diff --git a/app/assets/javascripts/issues/show/components/app.vue b/app/assets/javascripts/issues/show/components/app.vue
index 55e39165885..456a2029703 100644
--- a/app/assets/javascripts/issues/show/components/app.vue
+++ b/app/assets/javascripts/issues/show/components/app.vue
@@ -327,9 +327,12 @@ export default {
});
},
+ updateFormState(state) {
+ this.store.setFormState(state);
+ },
+
updateAndShowForm(templates = {}) {
if (!this.showForm) {
- this.showForm = true;
this.store.setFormState({
title: this.state.titleText,
description: this.state.descriptionText,
@@ -338,6 +341,7 @@ export default {
updateLoading: false,
issuableTemplates: templates,
});
+ this.showForm = true;
}
},
@@ -369,6 +373,10 @@ export default {
},
updateIssuable() {
+ this.store.setFormState({
+ updateLoading: true,
+ });
+
const {
store: { formState },
issueState,
@@ -376,7 +384,9 @@ export default {
const issuablePayload = issueState.isDirty
? { ...formState, issue_type: issueState.issueType }
: formState;
+
this.clearFlash();
+
return this.service
.updateIssuable(issuablePayload)
.then((res) => res.data)
@@ -473,6 +483,7 @@ export default {
:can-attach-file="canAttachFile"
:enable-autocomplete="enableAutocomplete"
:issuable-type="issuableType"
+ @updateForm="updateFormState"
/>
diff --git a/app/assets/javascripts/issues/show/components/description.vue b/app/assets/javascripts/issues/show/components/description.vue
index ddfef35ad6f..0b7e128c47b 100644
--- a/app/assets/javascripts/issues/show/components/description.vue
+++ b/app/assets/javascripts/issues/show/components/description.vue
@@ -288,17 +288,17 @@ export default {
}"
class="md"
>
-
+
-
+
-
-
diff --git a/app/assets/javascripts/issues/show/components/fields/description_template.vue b/app/assets/javascripts/issues/show/components/fields/description_template.vue
index d528641dcb6..98f92c97f77 100644
--- a/app/assets/javascripts/issues/show/components/fields/description_template.vue
+++ b/app/assets/javascripts/issues/show/components/fields/description_template.vue
@@ -8,8 +8,8 @@ export default {
GlIcon,
},
props: {
- formState: {
- type: Object,
+ value: {
+ type: String,
required: true,
},
issuableTemplates: {
@@ -39,10 +39,9 @@ export default {
// Create the editor for the template
const editor = document.querySelector('.detail-page-description .note-textarea') || {};
editor.setValue = (val) => {
- // eslint-disable-next-line vue/no-mutating-props
- this.formState.description = val;
+ this.$emit('input', val);
};
- editor.getValue = () => this.formState.description;
+ editor.getValue = () => this.value;
this.issuableTemplate = new IssuableTemplateSelectors({
$dropdowns: $(this.$refs.toggle),
diff --git a/app/assets/javascripts/issues/show/components/fields/title.vue b/app/assets/javascripts/issues/show/components/fields/title.vue
index a73926575d0..594d1a65700 100644
--- a/app/assets/javascripts/issues/show/components/fields/title.vue
+++ b/app/assets/javascripts/issues/show/components/fields/title.vue
@@ -4,8 +4,8 @@ import updateMixin from '../../mixins/update';
export default {
mixins: [updateMixin],
props: {
- formState: {
- type: Object,
+ value: {
+ type: String,
required: true,
},
},
@@ -15,19 +15,18 @@ export default {
diff --git a/app/assets/javascripts/issues/show/components/form.vue b/app/assets/javascripts/issues/show/components/form.vue
index 6447ec85b4e..e2c12edf46d 100644
--- a/app/assets/javascripts/issues/show/components/form.vue
+++ b/app/assets/javascripts/issues/show/components/form.vue
@@ -86,6 +86,10 @@ export default {
},
data() {
return {
+ formData: {
+ title: this.formState.title,
+ description: this.formState.description,
+ },
showOutdatedDescriptionWarning: false,
};
},
@@ -100,6 +104,14 @@ export default {
return this.issuableType === IssuableType.Issue;
},
},
+ watch: {
+ formData: {
+ handler(value) {
+ this.$emit('updateForm', value);
+ },
+ deep: true,
+ },
+ },
created() {
eventHub.$on('delete.issuable', this.resetAutosave);
eventHub.$on('update.issuable', this.resetAutosave);
@@ -191,16 +203,17 @@ export default {
>
+
+
:org
}.freeze unless defined?(MARKUPS)
+ DEFAULT_MARKUP_EXTENSIONS = { # rubocop:disable Style/MultilineIfModifier
+ markdown: 'md',
+ rdoc: 'rdoc',
+ asciidoc: 'asciidoc',
+ org: 'org'
+ }.freeze unless defined?(DEFAULT_MARKUP_EXTENSIONS)
+
CouldNotCreateWikiError = Class.new(StandardError)
HOMEPAGE = 'home'
@@ -184,12 +191,37 @@ class Wiki
end
def update_page(page, content:, title: nil, format: :markdown, message: nil)
- commit = commit_details(:updated, message, page.title)
+ if Feature.enabled?(:gitaly_replace_wiki_update_page, container, default_enabled: :yaml)
+ with_valid_format(format) do |default_extension|
+ title = title.presence || Pathname(page.path).sub_ext('').to_s
- wiki.update_page(page.path, title || page.name, format.to_sym, content, commit)
- after_wiki_activity
+ # If the format is the same we keep the former extension. This check is for formats
+ # that can have more than one extension like Markdown (.md, .markdown)
+ # If we don't do this we will override the existing extension.
+ extension = page.format != format.to_sym ? default_extension : File.extname(page.path).downcase[1..]
- true
+ capture_git_error(:updated) do
+ repository.update_file(
+ user,
+ sluggified_full_path(title, extension),
+ content,
+ previous_path: page.path,
+ **multi_commit_options(:updated, message, title))
+
+ after_wiki_activity
+
+ true
+ end
+ end
+ else
+ commit = commit_details(:updated, message, page.title)
+
+ wiki.update_page(page.path, title || page.name, format.to_sym, content, commit)
+
+ after_wiki_activity
+
+ true
+ end
end
def delete_page(page, message = nil)
@@ -296,7 +328,7 @@ class Wiki
git_user = Gitlab::Git::User.from_gitlab(user)
{
- branch_name: repository.root_ref,
+ branch_name: repository.root_ref || default_branch,
message: commit_message,
author_email: git_user.email,
author_name: git_user.name
@@ -321,6 +353,24 @@ class Wiki
def default_message(action, title)
"#{user.username} #{action} page: #{title}"
end
+
+ def with_valid_format(format, &block)
+ unless Wiki::MARKUPS.value?(format.to_sym)
+ @error_message = _('Invalid format selected')
+
+ return false
+ end
+
+ yield Wiki::DEFAULT_MARKUP_EXTENSIONS[format.to_sym]
+ end
+
+ def sluggified_full_path(title, extension)
+ sluggified_title(title) + '.' + extension
+ end
+
+ def sluggified_title(title)
+ Gitlab::EncodingHelper.encode_utf8_no_detect(title).tr(' ', '-')
+ end
end
Wiki.prepend_mod_with('Wiki')
diff --git a/app/services/ci/after_requeue_job_service.rb b/app/services/ci/after_requeue_job_service.rb
index bc70dd3bea4..1ae4639751b 100644
--- a/app/services/ci/after_requeue_job_service.rb
+++ b/app/services/ci/after_requeue_job_service.rb
@@ -22,15 +22,9 @@ module Ci
end
def dependent_jobs
- dependent_jobs = stage_dependent_jobs
- .or(needs_dependent_jobs)
- .ordered_by_stage
-
- if ::Feature.enabled?(:ci_fix_order_of_subsequent_jobs, @processable.pipeline.project, default_enabled: :yaml)
- dependent_jobs = ordered_by_dag(dependent_jobs)
- end
-
- dependent_jobs
+ ordered_by_dag(
+ stage_dependent_jobs.or(needs_dependent_jobs).ordered_by_stage
+ )
end
def process(job)
diff --git a/config/feature_flags/development/ci_fix_order_of_subsequent_jobs.yml b/config/feature_flags/development/gitaly_replace_wiki_update_page.yml
similarity index 60%
rename from config/feature_flags/development/ci_fix_order_of_subsequent_jobs.yml
rename to config/feature_flags/development/gitaly_replace_wiki_update_page.yml
index 9a98604d0a8..9fabf5edde8 100644
--- a/config/feature_flags/development/ci_fix_order_of_subsequent_jobs.yml
+++ b/config/feature_flags/development/gitaly_replace_wiki_update_page.yml
@@ -1,8 +1,8 @@
---
-name: ci_fix_order_of_subsequent_jobs
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/74394
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/345587
-milestone: '14.9'
+name: gitaly_replace_wiki_update_page
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/83833
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/357246
+milestone: '14.10'
type: development
-group: group::pipeline authoring
+group: group::editor
default_enabled: false
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 7a58c9358c4..417871f6484 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -20500,6 +20500,9 @@ msgstr ""
msgid "Invalid file."
msgstr ""
+msgid "Invalid format selected"
+msgstr ""
+
msgid "Invalid hash"
msgstr ""
diff --git a/spec/frontend/issues/show/components/fields/description_spec.js b/spec/frontend/issues/show/components/fields/description_spec.js
index 188f6807087..0dcd70ac19b 100644
--- a/spec/frontend/issues/show/components/fields/description_spec.js
+++ b/spec/frontend/issues/show/components/fields/description_spec.js
@@ -15,9 +15,7 @@ describe('Description field component', () => {
markdownPreviewPath: '/',
markdownDocsPath: '/',
quickActionsDocsPath: '/',
- formState: {
- description,
- },
+ value: description,
},
stubs: {
MarkdownField,
diff --git a/spec/frontend/issues/show/components/fields/description_template_spec.js b/spec/frontend/issues/show/components/fields/description_template_spec.js
index abe2805e5b2..79a3bfa9840 100644
--- a/spec/frontend/issues/show/components/fields/description_template_spec.js
+++ b/spec/frontend/issues/show/components/fields/description_template_spec.js
@@ -1,74 +1,65 @@
-import Vue from 'vue';
+import { shallowMount } from '@vue/test-utils';
import descriptionTemplate from '~/issues/show/components/fields/description_template.vue';
describe('Issue description template component with templates as hash', () => {
- let vm;
- let formState;
-
- beforeEach(() => {
- const Component = Vue.extend(descriptionTemplate);
- formState = {
- description: 'test',
- };
-
- vm = new Component({
- propsData: {
- formState,
- issuableTemplates: {
- test: [{ name: 'test', id: 'test', project_path: '/', namespace_path: '/' }],
- },
- projectId: 1,
- projectPath: '/',
- namespacePath: '/',
- projectNamespace: '/',
+ let wrapper;
+ const defaultOptions = {
+ propsData: {
+ value: 'test',
+ issuableTemplates: {
+ test: [{ name: 'test', id: 'test', project_path: '/', namespace_path: '/' }],
},
- }).$mount();
+ projectId: 1,
+ projectPath: '/',
+ namespacePath: '/',
+ projectNamespace: '/',
+ },
+ };
+
+ const findIssuableSelector = () => wrapper.find('.js-issuable-selector');
+
+ const createComponent = (options = defaultOptions) => {
+ wrapper = shallowMount(descriptionTemplate, options);
+ };
+
+ afterEach(() => {
+ wrapper.destroy();
});
it('renders templates as JSON hash in data attribute', () => {
- expect(vm.$el.querySelector('.js-issuable-selector').getAttribute('data-data')).toBe(
+ createComponent();
+ expect(findIssuableSelector().attributes('data-data')).toBe(
'{"test":[{"name":"test","id":"test","project_path":"/","namespace_path":"/"}]}',
);
});
- it('updates formState when changing template', () => {
- vm.issuableTemplate.editor.setValue('test new template');
+ it('emits input event', () => {
+ createComponent();
+ wrapper.vm.issuableTemplate.editor.setValue('test new template');
- expect(formState.description).toBe('test new template');
+ expect(wrapper.emitted('input')).toEqual([['test new template']]);
});
- it('returns formState description with editor getValue', () => {
- formState.description = 'testing new template';
+ it('returns value with editor getValue', () => {
+ createComponent();
+ expect(wrapper.vm.issuableTemplate.editor.getValue()).toBe('test');
+ });
- expect(vm.issuableTemplate.editor.getValue()).toBe('testing new template');
- });
-});
-
-describe('Issue description template component with templates as array', () => {
- let vm;
- let formState;
-
- beforeEach(() => {
- const Component = Vue.extend(descriptionTemplate);
- formState = {
- description: 'test',
- };
-
- vm = new Component({
- propsData: {
- formState,
- issuableTemplates: [{ name: 'test', id: 'test', project_path: '/', namespace_path: '/' }],
- projectId: 1,
- projectPath: '/',
- namespacePath: '/',
- projectNamespace: '/',
- },
- }).$mount();
- });
-
- it('renders templates as JSON array in data attribute', () => {
- expect(vm.$el.querySelector('.js-issuable-selector').getAttribute('data-data')).toBe(
- '[{"name":"test","id":"test","project_path":"/","namespace_path":"/"}]',
- );
+ describe('Issue description template component with templates as array', () => {
+ it('renders templates as JSON array in data attribute', () => {
+ createComponent({
+ propsData: {
+ value: 'test',
+ issuableTemplates: [{ name: 'test', id: 'test', project_path: '/', namespace_path: '/' }],
+ projectId: 1,
+ projectPath: '/',
+ namespacePath: '/',
+ projectNamespace: '/',
+ },
+ });
+ expect(findIssuableSelector().attributes('data-data')).toBe(
+ '[{"name":"test","id":"test","project_path":"/","namespace_path":"/"}]',
+ );
+ });
});
});
diff --git a/spec/frontend/issues/show/components/fields/title_spec.js b/spec/frontend/issues/show/components/fields/title_spec.js
index efd0b6fbd30..de04405d89b 100644
--- a/spec/frontend/issues/show/components/fields/title_spec.js
+++ b/spec/frontend/issues/show/components/fields/title_spec.js
@@ -12,9 +12,7 @@ describe('Title field component', () => {
wrapper = shallowMount(TitleField, {
propsData: {
- formState: {
- title: 'test',
- },
+ value: 'test',
},
});
});
diff --git a/spec/models/concerns/featurable_spec.rb b/spec/models/concerns/featurable_spec.rb
index 453b6f7f29a..bf104fe1b30 100644
--- a/spec/models/concerns/featurable_spec.rb
+++ b/spec/models/concerns/featurable_spec.rb
@@ -3,171 +3,101 @@
require 'spec_helper'
RSpec.describe Featurable do
- let_it_be(:user) { create(:user) }
+ let!(:klass) do
+ Class.new(ApplicationRecord) do
+ include Featurable
- let(:project) { create(:project) }
- let(:feature_class) { subject.class }
- let(:features) { feature_class::FEATURES }
+ self.table_name = 'project_features'
- subject { project.project_feature }
+ set_available_features %i(feature1 feature2 feature3)
+
+ def feature1_access_level
+ Featurable::DISABLED
+ end
+
+ def feature2_access_level
+ Featurable::ENABLED
+ end
+
+ def feature3_access_level
+ Featurable::PRIVATE
+ end
+ end
+ end
+
+ subject { klass.new }
+
+ describe '.set_available_features' do
+ it { expect(klass.available_features).to match_array [:feature1, :feature2, :feature3] }
+ end
+
+ describe '#*_enabled?' do
+ it { expect(subject.feature1_enabled?).to be_falsey }
+ it { expect(subject.feature2_enabled?).to be_truthy }
+ end
describe '.quoted_access_level_column' do
it 'returns the table name and quoted column name for a feature' do
- expected = '"project_features"."issues_access_level"'
-
- expect(feature_class.quoted_access_level_column(:issues)).to eq(expected)
+ expect(klass.quoted_access_level_column(:feature1)).to eq('"project_features"."feature1_access_level"')
end
end
describe '.access_level_attribute' do
- it { expect(feature_class.access_level_attribute(:wiki)).to eq :wiki_access_level }
+ it { expect(klass.access_level_attribute(:feature1)).to eq :feature1_access_level }
it 'raises error for unspecified feature' do
- expect { feature_class.access_level_attribute(:unknown) }
+ expect { klass.access_level_attribute(:unknown) }
.to raise_error(ArgumentError, /invalid feature: unknown/)
end
end
- describe '.set_available_features' do
- let!(:klass) do
- Class.new(ApplicationRecord) do
- include Featurable
-
- self.table_name = 'project_features'
-
- set_available_features %i(feature1 feature2)
-
- def feature1_access_level
- Featurable::DISABLED
- end
-
- def feature2_access_level
- Featurable::ENABLED
- end
- end
- end
-
- let!(:instance) { klass.new }
-
- it { expect(klass.available_features).to eq [:feature1, :feature2] }
- it { expect(instance.feature1_enabled?).to be_falsey }
- it { expect(instance.feature2_enabled?).to be_truthy }
- end
-
- describe '.available_features' do
- it { expect(feature_class.available_features).to include(*features) }
- end
-
describe '#access_level' do
it 'returns access level' do
- expect(subject.access_level(:wiki)).to eq(subject.wiki_access_level)
+ expect(subject.access_level(:feature1)).to eq(subject.feature1_access_level)
end
end
describe '#feature_available?' do
- let(:features) { %w(issues wiki builds merge_requests snippets repository pages metrics_dashboard) }
-
context 'when features are disabled' do
- it "returns false" do
- update_all_project_features(project, features, ProjectFeature::DISABLED)
-
- features.each do |feature|
- expect(project.feature_available?(feature.to_sym, user)).to eq(false), "#{feature} failed"
- end
+ it 'returns false' do
+ expect(subject.feature_available?(:feature1)).to eq(false)
end
end
context 'when features are enabled only for team members' do
- it "returns false when user is not a team member" do
- update_all_project_features(project, features, ProjectFeature::PRIVATE)
+ let_it_be(:user) { create(:user) }
- features.each do |feature|
- expect(project.feature_available?(feature.to_sym, user)).to eq(false), "#{feature} failed"
+ before do
+ expect(subject).to receive(:member?).and_call_original
+ end
+
+ context 'when user is not present' do
+ it 'returns false' do
+ expect(subject.feature_available?(:feature3)).to eq(false)
end
end
- it "returns true when user is a team member" do
- project.add_developer(user)
+ context 'when user can read all resources' do
+ it 'returns true' do
+ allow(user).to receive(:can_read_all_resources?).and_return(true)
- update_all_project_features(project, features, ProjectFeature::PRIVATE)
-
- features.each do |feature|
- expect(project.feature_available?(feature.to_sym, user)).to eq(true), "#{feature} failed"
+ expect(subject.feature_available?(:feature3, user)).to eq(true)
end
end
- it "returns true when user is a member of project group" do
- group = create(:group)
- project = create(:project, namespace: group)
- group.add_developer(user)
+ context 'when user cannot read all resources' do
+ it 'raises NotImplementedError exception' do
+ expect(subject).to receive(:resource_member?).and_call_original
- update_all_project_features(project, features, ProjectFeature::PRIVATE)
-
- features.each do |feature|
- expect(project.feature_available?(feature.to_sym, user)).to eq(true), "#{feature} failed"
- end
- end
-
- context 'when admin mode is enabled', :enable_admin_mode do
- it "returns true if user is an admin" do
- user.update_attribute(:admin, true)
-
- update_all_project_features(project, features, ProjectFeature::PRIVATE)
-
- features.each do |feature|
- expect(project.feature_available?(feature.to_sym, user)).to eq(true), "#{feature} failed"
- end
- end
- end
-
- context 'when admin mode is disabled' do
- it "returns false when user is an admin" do
- user.update_attribute(:admin, true)
-
- update_all_project_features(project, features, ProjectFeature::PRIVATE)
-
- features.each do |feature|
- expect(project.feature_available?(feature.to_sym, user)).to eq(false), "#{feature} failed"
- end
+ expect { subject.feature_available?(:feature3, user) }.to raise_error(NotImplementedError)
end
end
end
context 'when feature is enabled for everyone' do
- it "returns true" do
- expect(project.feature_available?(:issues, user)).to eq(true)
+ it 'returns true' do
+ expect(subject.feature_available?(:feature2)).to eq(true)
end
end
end
-
- describe '#*_enabled?' do
- let(:features) { %w(wiki builds merge_requests) }
-
- it "returns false when feature is disabled" do
- update_all_project_features(project, features, ProjectFeature::DISABLED)
-
- features.each do |feature|
- expect(project.public_send("#{feature}_enabled?")).to eq(false), "#{feature} failed"
- end
- end
-
- it "returns true when feature is enabled only for team members" do
- update_all_project_features(project, features, ProjectFeature::PRIVATE)
-
- features.each do |feature|
- expect(project.public_send("#{feature}_enabled?")).to eq(true), "#{feature} failed"
- end
- end
-
- it "returns true when feature is enabled for everyone" do
- features.each do |feature|
- expect(project.public_send("#{feature}_enabled?")).to eq(true), "#{feature} failed"
- end
- end
- end
-
- def update_all_project_features(project, features, value)
- project_feature_attributes = features.to_h { |f| ["#{f}_access_level", value] }
- project.project_feature.update!(project_feature_attributes)
- end
end
diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb
index 172e0c4ddee..b20c91c53c1 100644
--- a/spec/models/environment_spec.rb
+++ b/spec/models/environment_spec.rb
@@ -586,6 +586,31 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do
expect(subject.user).to eq(user)
end
end
+
+ context 'close action does not raise ActiveRecord::StaleObjectError' do
+ let!(:close_action) do
+ create(:ci_build, :manual, pipeline: pipeline, name: 'close_app')
+ end
+
+ before do
+ # preload the build
+ environment.stop_action
+
+ # Update record as the other process. This makes `environment.stop_action` stale.
+ close_action.drop!
+ end
+
+ it 'successfully plays the build even if the build was a stale object' do
+ # Since build is droped.
+ expect(close_action.processed).to be_falsey
+
+ # it encounters the StaleObjectError at first, but reloads the object and runs `build.play`
+ expect { subject }.not_to raise_error(ActiveRecord::StaleObjectError)
+
+ # Now the build should be processed.
+ expect(close_action.reload.processed).to be_truthy
+ end
+ end
end
end
end
diff --git a/spec/models/groups/feature_setting_spec.rb b/spec/models/groups/feature_setting_spec.rb
new file mode 100644
index 00000000000..f1e66744b90
--- /dev/null
+++ b/spec/models/groups/feature_setting_spec.rb
@@ -0,0 +1,13 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Groups::FeatureSetting do
+ describe 'associations' do
+ it { is_expected.to belong_to(:group) }
+ end
+
+ describe 'validations' do
+ it { is_expected.to validate_presence_of(:group) }
+ end
+end
diff --git a/spec/models/project_feature_spec.rb b/spec/models/project_feature_spec.rb
index 75e43ed9a67..941f6c0a49d 100644
--- a/spec/models/project_feature_spec.rb
+++ b/spec/models/project_feature_spec.rb
@@ -5,8 +5,8 @@ require 'spec_helper'
RSpec.describe ProjectFeature do
using RSpec::Parameterized::TableSyntax
- let(:project) { create(:project) }
- let(:user) { create(:user) }
+ let_it_be_with_reload(:project) { create(:project) }
+ let_it_be(:user) { create(:user) }
it { is_expected.to belong_to(:project) }
@@ -242,4 +242,95 @@ RSpec.describe ProjectFeature do
end
end
end
+
+ # rubocop:disable Gitlab/FeatureAvailableUsage
+ describe '#feature_available?' do
+ let(:features) { ProjectFeature::FEATURES }
+
+ context 'when features are disabled' do
+ it 'returns false' do
+ update_all_project_features(project, features, ProjectFeature::DISABLED)
+
+ features.each do |feature|
+ expect(project.feature_available?(feature.to_sym, user)).to eq(false), "#{feature} failed"
+ end
+ end
+ end
+
+ context 'when features are enabled only for team members' do
+ it 'returns false when user is not a team member' do
+ update_all_project_features(project, features, ProjectFeature::PRIVATE)
+
+ features.each do |feature|
+ expect(project.feature_available?(feature.to_sym, user)).to eq(false), "#{feature} failed"
+ end
+ end
+
+ it 'returns true when user is a team member' do
+ project.add_developer(user)
+
+ update_all_project_features(project, features, ProjectFeature::PRIVATE)
+
+ features.each do |feature|
+ expect(project.feature_available?(feature.to_sym, user)).to eq(true)
+ end
+ end
+
+ it 'returns true when user is a member of project group' do
+ group = create(:group)
+ project = create(:project, namespace: group)
+ group.add_developer(user)
+
+ update_all_project_features(project, features, ProjectFeature::PRIVATE)
+
+ features.each do |feature|
+ expect(project.feature_available?(feature.to_sym, user)).to eq(true)
+ end
+ end
+
+ context 'when admin mode is enabled', :enable_admin_mode do
+ it 'returns true if user is an admin' do
+ user.update_attribute(:admin, true)
+
+ update_all_project_features(project, features, ProjectFeature::PRIVATE)
+
+ features.each do |feature|
+ expect(project.feature_available?(feature.to_sym, user)).to eq(true)
+ end
+ end
+ end
+
+ context 'when admin mode is disabled' do
+ it 'returns false when user is an admin' do
+ user.update_attribute(:admin, true)
+
+ update_all_project_features(project, features, ProjectFeature::PRIVATE)
+
+ features.each do |feature|
+ expect(project.feature_available?(feature.to_sym, user)).to eq(false), "#{feature} failed"
+ end
+ end
+ end
+ end
+
+ context 'when feature is enabled for everyone' do
+ it 'returns true' do
+ expect(project.feature_available?(:issues, user)).to eq(true)
+ end
+ end
+
+ context 'when feature has any other value' do
+ it 'returns true' do
+ project.project_feature.update_attribute(:issues_access_level, 200)
+
+ expect(project.feature_available?(:issues)).to eq(true)
+ end
+ end
+
+ def update_all_project_features(project, features, value)
+ project_feature_attributes = features.to_h { |f| ["#{f}_access_level", value] }
+ project.project_feature.update!(project_feature_attributes)
+ end
+ end
+ # rubocop:enable Gitlab/FeatureAvailableUsage
end
diff --git a/spec/models/wiki_page_spec.rb b/spec/models/wiki_page_spec.rb
index 0016d2f517b..51970064c54 100644
--- a/spec/models/wiki_page_spec.rb
+++ b/spec/models/wiki_page_spec.rb
@@ -473,6 +473,21 @@ RSpec.describe WikiPage do
end
end
+ describe 'in subdir' do
+ it 'keeps the page in the same dir when the content is updated' do
+ title = 'foo/Existing Page'
+ page = create_wiki_page(title: title)
+
+ expect(page.slug).to eq 'foo/Existing-Page'
+ expect(page.update(title: title, content: 'new_content')).to be_truthy
+
+ page = wiki.find_page(title)
+
+ expect(page.slug).to eq 'foo/Existing-Page'
+ expect(page.content).to eq 'new_content'
+ end
+ end
+
context 'when renaming a page' do
it 'raises an error if the page already exists' do
existing_page = create_wiki_page
diff --git a/spec/services/ci/after_requeue_job_service_spec.rb b/spec/services/ci/after_requeue_job_service_spec.rb
index c9e01437d16..c9bd44f78e2 100644
--- a/spec/services/ci/after_requeue_job_service_spec.rb
+++ b/spec/services/ci/after_requeue_job_service_spec.rb
@@ -196,25 +196,6 @@ RSpec.describe Ci::AfterRequeueJobService, :sidekiq_inline do
c2: 'created'
)
end
-
- context 'when the FF ci_fix_order_of_subsequent_jobs is disabled' do
- before do
- stub_feature_flags(ci_fix_order_of_subsequent_jobs: false)
- end
-
- it 'does not mark b1 as processable', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/356571' do
- execute_after_requeue_service(a1)
-
- check_jobs_statuses(
- a1: 'pending',
- a2: 'created',
- b1: 'skipped',
- b2: 'created',
- c1: 'created',
- c2: 'created'
- )
- end
- end
end
private
diff --git a/spec/support/shared_examples/models/wiki_shared_examples.rb b/spec/support/shared_examples/models/wiki_shared_examples.rb
index b3f79d9fe6e..1b7da3b0ef1 100644
--- a/spec/support/shared_examples/models/wiki_shared_examples.rb
+++ b/spec/support/shared_examples/models/wiki_shared_examples.rb
@@ -427,45 +427,122 @@ RSpec.shared_examples 'wiki model' do
end
describe '#update_page' do
- let(:page) { create(:wiki_page, wiki: subject, title: 'update-page') }
+ shared_examples 'update_page tests' do
+ with_them do
+ let!(:page) { create(:wiki_page, wiki: subject, title: original_title, format: original_format, content: 'original content') }
- def update_page
- subject.update_page(
- page.page,
- content: 'some other content',
- format: :markdown,
- message: 'updated page'
- )
+ let(:message) { 'updated page' }
+ let(:updated_content) { 'updated content' }
+
+ def update_page
+ subject.update_page(
+ page.page,
+ content: updated_content,
+ title: updated_title,
+ format: updated_format,
+ message: message
+ )
+ end
+
+ specify :aggregate_failures do
+ expect(subject).to receive(:after_wiki_activity)
+ expect(update_page).to eq true
+
+ page = subject.find_page(updated_title.presence || original_title)
+
+ expect(page.raw_content).to eq(updated_content)
+ expect(page.path).to eq(expected_path)
+ expect(page.version.message).to eq(message)
+ expect(user.commit_email).not_to eq(user.email)
+ expect(commit.author_email).to eq(user.commit_email)
+ expect(commit.committer_email).to eq(user.commit_email)
+ end
+ end
end
- it 'updates the content of the page' do
- update_page
- page = subject.find_page('update-page')
+ shared_context 'common examples' do
+ using RSpec::Parameterized::TableSyntax
- expect(page.raw_content).to eq('some other content')
+ where(:original_title, :original_format, :updated_title, :updated_format, :expected_path) do
+ 'test page' | :markdown | 'new test page' | :markdown | 'new-test-page.md'
+ 'test page' | :markdown | 'test page' | :markdown | 'test-page.md'
+ 'test page' | :markdown | 'test page' | :asciidoc | 'test-page.asciidoc'
+
+ 'test page' | :markdown | 'new dir/new test page' | :markdown | 'new-dir/new-test-page.md'
+ 'test page' | :markdown | 'new dir/test page' | :markdown | 'new-dir/test-page.md'
+
+ 'test dir/test page' | :markdown | 'new dir/new test page' | :markdown | 'new-dir/new-test-page.md'
+ 'test dir/test page' | :markdown | 'test dir/test page' | :markdown | 'test-dir/test-page.md'
+ 'test dir/test page' | :markdown | 'test dir/test page' | :asciidoc | 'test-dir/test-page.asciidoc'
+
+ 'test dir/test page' | :markdown | 'new test page' | :markdown | 'new-test-page.md'
+ 'test dir/test page' | :markdown | 'test page' | :markdown | 'test-page.md'
+
+ 'test page' | :markdown | nil | :markdown | 'test-page.md'
+ 'test.page' | :markdown | nil | :markdown | 'test.page.md'
+ end
end
- it 'sets the correct commit message' do
- update_page
- page = subject.find_page('update-page')
+ # There are two bugs in Gollum. THe first one is when the title and the format are updated
+ # at the same time https://gitlab.com/gitlab-org/gitlab/-/issues/243519.
+ # The second one is when the wiki page is within a dir and the `title` argument
+ # we pass to the update method is `nil`. Gollum will remove the dir and move the page.
+ #
+ # We can include this context into the former once it is fixed
+ # or when Gollum is removed since the Gitaly approach already fixes it.
+ shared_context 'extended examples' do
+ using RSpec::Parameterized::TableSyntax
- expect(page.version.message).to eq('updated page')
+ where(:original_title, :original_format, :updated_title, :updated_format, :expected_path) do
+ 'test page' | :markdown | 'new test page' | :asciidoc | 'new-test-page.asciidoc'
+ 'test page' | :markdown | 'new dir/new test page' | :asciidoc | 'new-dir/new-test-page.asciidoc'
+ 'test dir/test page' | :markdown | 'new dir/new test page' | :asciidoc | 'new-dir/new-test-page.asciidoc'
+ 'test dir/test page' | :markdown | 'new test page' | :asciidoc | 'new-test-page.asciidoc'
+ 'test page' | :markdown | nil | :asciidoc | 'test-page.asciidoc'
+ 'test dir/test page' | :markdown | nil | :asciidoc | 'test-dir/test-page.asciidoc'
+ 'test dir/test page' | :markdown | nil | :markdown | 'test-dir/test-page.md'
+ 'test page' | :markdown | '' | :markdown | 'test-page.md'
+ 'test.page' | :markdown | '' | :markdown | 'test.page.md'
+ end
end
- it 'sets the correct commit email' do
- update_page
-
- expect(user.commit_email).not_to eq(user.email)
- expect(commit.author_email).to eq(user.commit_email)
- expect(commit.committer_email).to eq(user.commit_email)
+ it_behaves_like 'update_page tests' do
+ include_context 'common examples'
+ include_context 'extended examples'
end
- it 'runs after_wiki_activity callbacks' do
- page
+ context 'when format is invalid' do
+ let!(:page) { create(:wiki_page, wiki: subject, title: 'test page') }
- expect(subject).to receive(:after_wiki_activity)
+ it 'returns false and sets error message' do
+ expect(subject.update_page(page.page, content: 'new content', format: :foobar)).to eq false
+ expect(subject.error_message).to match(/Invalid format selected/)
+ end
+ end
- update_page
+ context 'when page path does not have a default extension' do
+ let!(:page) { create(:wiki_page, wiki: subject, title: 'test page') }
+
+ context 'when format is not different' do
+ it 'does not change the default extension' do
+ path = 'test-page.markdown'
+ page.page.instance_variable_set(:@path, path)
+
+ expect(subject.repository).to receive(:update_file).with(user, path, anything, anything)
+
+ subject.update_page(page.page, content: 'new content', format: :markdown)
+ end
+ end
+ end
+
+ context 'when feature flag :gitaly_replace_wiki_update_page is disabled' do
+ before do
+ stub_feature_flags(gitaly_replace_wiki_update_page: false)
+ end
+
+ it_behaves_like 'update_page tests' do
+ include_context 'common examples'
+ end
end
end
diff --git a/spec/support/shared_examples/policies/wiki_policies_shared_examples.rb b/spec/support/shared_examples/policies/wiki_policies_shared_examples.rb
index 58822f4309b..991d6289373 100644
--- a/spec/support/shared_examples/policies/wiki_policies_shared_examples.rb
+++ b/spec/support/shared_examples/policies/wiki_policies_shared_examples.rb
@@ -107,10 +107,4 @@ RSpec.shared_examples 'model with wiki policies' do
expect_disallowed(*disallowed_permissions)
end
end
-
- # TODO: Remove this helper once we implement group features
- # https://gitlab.com/gitlab-org/gitlab/-/issues/208412
- def set_access_level(access_level)
- raise NotImplementedError
- end
end