diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2022-04-07 15:10:21 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2022-04-07 15:10:21 +0300 |
commit | 630c555b11c0fc64d3f0c1ec13b314e2efe3f7ee (patch) | |
tree | 5873240ec56a19ddb3794154b0efb48efeab73d3 | |
parent | f6f4bc2bc0f369b0837f01c45c15421f7f3868a3 (diff) |
Add latest changes from gitlab-org/gitlab@master
28 files changed, 533 insertions, 303 deletions
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" /> </div> <div v-else> 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" ></div> - <!-- eslint-disable vue/no-mutating-props --> + <textarea v-if="descriptionText" - v-model="descriptionText" + :value="descriptionText" :data-update-url="updateUrl" class="hidden js-task-list-field" dir="auto" data-testid="textarea" > </textarea> - <!-- eslint-enable vue/no-mutating-props --> + <gl-modal ref="modal" modal-id="create-task-modal" diff --git a/app/assets/javascripts/issues/show/components/fields/description.vue b/app/assets/javascripts/issues/show/components/fields/description.vue index abf9ae2690e..0bb5e7cb2ee 100644 --- a/app/assets/javascripts/issues/show/components/fields/description.vue +++ b/app/assets/javascripts/issues/show/components/fields/description.vue @@ -9,8 +9,8 @@ export default { }, mixins: [updateMixin], props: { - formState: { - type: Object, + value: { + type: String, required: true, }, markdownPreviewPath: { @@ -52,24 +52,23 @@ export default { :quick-actions-docs-path="quickActionsDocsPath" :can-attach-file="canAttachFile" :enable-autocomplete="enableAutocomplete" - :textarea-value="formState.description" + :textarea-value="value" > <template #textarea> - <!-- eslint-disable vue/no-mutating-props --> <textarea id="issue-description" ref="textarea" - v-model="formState.description" + :value="value" class="note-textarea js-gfm-input js-autosize markdown-area qa-description-textarea" dir="auto" data-supports-quick-actions="true" :aria-label="__('Description')" :placeholder="__('Write a comment or drag your files here…')" + @input="$emit('input', $event.target.value)" @keydown.meta.enter="updateIssuable" @keydown.ctrl.enter="updateIssuable" > </textarea> - <!-- eslint-enable vue/no-mutating-props --> </template> </markdown-field> </div> 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 { <template> <fieldset> <label class="sr-only" for="issuable-title">{{ __('Title') }}</label> - <!-- eslint-disable vue/no-mutating-props --> <input id="issuable-title" ref="input" - v-model="formState.title" + :value="value" class="form-control qa-title-input gl-border-gray-200" dir="auto" type="text" :placeholder="__('Title')" :aria-label="__('Title')" + @input="$emit('input', $event.target.value)" @keydown.meta.enter="updateIssuable" @keydown.ctrl.enter="updateIssuable" /> - <!-- eslint-enable vue/no-mutating-props --> </fieldset> </template> 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 { > <div class="row gl-mb-3"> <div class="col-12"> - <issuable-title-field ref="title" :form-state="formState" /> + <issuable-title-field ref="title" v-model="formData.title" /> </div> </div> <div class="row"> <div v-if="isIssueType" class="col-12 col-md-4 pr-md-0"> <issuable-type-field ref="issue-type" /> </div> + <div v-if="hasIssuableTemplates" class="col-12 col-md-4 pl-md-2"> <description-template-field - :form-state="formState" + v-model="formData.description" :issuable-templates="issuableTemplates" :project-path="projectPath" :project-id="projectId" @@ -208,14 +221,16 @@ export default { /> </div> </div> + <description-field ref="description" - :form-state="formState" + v-model="formData.description" :markdown-preview-path="markdownPreviewPath" :markdown-docs-path="markdownDocsPath" :can-attach-file="canAttachFile" :enable-autocomplete="enableAutocomplete" /> + <edit-actions :endpoint="endpoint" :form-state="formState" diff --git a/app/assets/javascripts/issues/show/mixins/update.js b/app/assets/javascripts/issues/show/mixins/update.js index 72be65b426f..31b29de580c 100644 --- a/app/assets/javascripts/issues/show/mixins/update.js +++ b/app/assets/javascripts/issues/show/mixins/update.js @@ -3,7 +3,6 @@ import eventHub from '../event_hub'; export default { methods: { updateIssuable() { - this.formState.updateLoading = true; eventHub.$emit('update.issuable'); }, }, diff --git a/app/models/concerns/featurable.rb b/app/models/concerns/featurable.rb index 70d67fc7559..08189d83534 100644 --- a/app/models/concerns/featurable.rb +++ b/app/models/concerns/featurable.rb @@ -50,7 +50,7 @@ module Featurable end def available_features - @available_features + @available_features || [] end def access_level_attribute(feature) @@ -74,6 +74,12 @@ module Featurable STRING_OPTIONS.key(level) end + def required_minimum_access_level(feature) + ensure_feature!(feature) + + Gitlab::Access::GUEST + end + def ensure_feature!(feature) feature = feature.model_name.plural if feature.respond_to?(:model_name) feature = feature.to_sym @@ -91,8 +97,8 @@ module Featurable public_send(self.class.access_level_attribute(feature)) # rubocop:disable GitlabSecurity/PublicSend end - def feature_available?(feature, user) - get_permission(user, feature) + def feature_available?(feature, user = nil) + has_permission?(user, feature) end def string_access_level(feature) @@ -115,4 +121,30 @@ module Featurable def feature_validation_exclusion [] end + + def has_permission?(user, feature) + case access_level(feature) + when DISABLED + false + when PRIVATE + member?(user, feature) + when ENABLED + true + when PUBLIC + true + else + true + end + end + + def member?(user, feature) + return false unless user + return true if user.can_read_all_resources? + + resource_member?(user, feature) + end + + def resource_member?(user, feature) + raise NotImplementedError + end end diff --git a/app/models/environment.rb b/app/models/environment.rb index 829c4f4159d..54323c8bbd8 100644 --- a/app/models/environment.rb +++ b/app/models/environment.rb @@ -273,7 +273,15 @@ class Environment < ApplicationRecord return unless available? stop! - stop_action&.play(current_user) + + return unless stop_action + + Gitlab::OptimisticLocking.retry_lock( + stop_action, + name: 'environment_stop_with_action' + ) do |build| + build&.play(current_user) + end end def reset_auto_stop diff --git a/app/models/group.rb b/app/models/group.rb index 02e4e91da23..ff794472eb7 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -106,6 +106,7 @@ class Group < Namespace has_one :crm_settings, class_name: 'Group::CrmSettings', inverse_of: :group accepts_nested_attributes_for :variables, allow_destroy: true + accepts_nested_attributes_for :group_feature, update_only: true validate :visibility_level_allowed_by_projects validate :visibility_level_allowed_by_sub_groups @@ -835,6 +836,17 @@ class Group < Namespace end end + # Check for enabled features, similar to `Project#feature_available?` + # NOTE: We still want to keep this after removing `Namespace#feature_available?`. + override :feature_available? + def feature_available?(feature, user = nil) + if ::Groups::FeatureSetting.available_features.include?(feature) + group_feature.feature_available?(feature, user) # rubocop:disable Gitlab/FeatureAvailableUsage + else + super + end + end + private def max_member_access(user_ids) diff --git a/app/models/groups/feature_setting.rb b/app/models/groups/feature_setting.rb index 5db5cda2d56..72d0851ea85 100644 --- a/app/models/groups/feature_setting.rb +++ b/app/models/groups/feature_setting.rb @@ -2,11 +2,23 @@ module Groups class FeatureSetting < ApplicationRecord + include Featurable + extend ::Gitlab::Utils::Override + self.primary_key = :group_id self.table_name = 'group_features' belongs_to :group validates :group, presence: true + + private + + override :resource_member? + def resource_member?(user, feature) + group.member?(user, ::Groups::FeatureSetting.required_minimum_access_level(feature)) + end end end + +::Groups::FeatureSetting.prepend_mod_with('Groups::FeatureSetting') diff --git a/app/models/namespace.rb b/app/models/namespace.rb index 5f7b8d1c8b1..5dcf240b6c3 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -373,7 +373,7 @@ class Namespace < ApplicationRecord end # Deprecated, use #licensed_feature_available? instead. Remove once Namespace#feature_available? isn't used anymore. - def feature_available?(feature) + def feature_available?(feature, _user = nil) licensed_feature_available?(feature) end diff --git a/app/models/project_feature.rb b/app/models/project_feature.rb index 0d3e50837ab..33783d31355 100644 --- a/app/models/project_feature.rb +++ b/app/models/project_feature.rb @@ -3,6 +3,7 @@ class ProjectFeature < ApplicationRecord include Featurable extend Gitlab::ConfigHelper + extend ::Gitlab::Utils::Override # When updating this array, make sure to update rubocop/cop/gitlab/feature_available_usage.rb as well. FEATURES = %i[ @@ -155,31 +156,14 @@ class ProjectFeature < ApplicationRecord %i(merge_requests_access_level builds_access_level).each(&validator) end - def get_permission(user, feature) - case access_level(feature) - when DISABLED - false - when PRIVATE - team_access?(user, feature) - when ENABLED - true - when PUBLIC - true - else - true - end + def feature_validation_exclusion + %i(pages) end - def team_access?(user, feature) - return unless user - return true if user.can_read_all_resources? - + override :resource_member? + def resource_member?(user, feature) project.team.member?(user, ProjectFeature.required_minimum_access_level(feature)) end - - def feature_validation_exclusion - %i(pages) - end end ProjectFeature.prepend_mod_with('ProjectFeature') diff --git a/app/models/wiki.rb b/app/models/wiki.rb index 622070abd88..cb572fb5971 100644 --- a/app/models/wiki.rb +++ b/app/models/wiki.rb @@ -16,6 +16,13 @@ class Wiki 'Org' => :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 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; + let wrapper; + const defaultOptions = { + propsData: { + value: 'test', + issuableTemplates: { + test: [{ name: 'test', id: 'test', project_path: '/', namespace_path: '/' }], + }, + projectId: 1, + projectPath: '/', + namespacePath: '/', + projectNamespace: '/', + }, + }; - beforeEach(() => { - const Component = Vue.extend(descriptionTemplate); - formState = { - description: 'test', - }; + const findIssuableSelector = () => wrapper.find('.js-issuable-selector'); - vm = new Component({ - propsData: { - formState, - issuableTemplates: { - test: [{ name: 'test', id: 'test', project_path: '/', namespace_path: '/' }], - }, - projectId: 1, - projectPath: '/', - namespacePath: '/', - projectNamespace: '/', - }, - }).$mount(); + 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'; - - expect(vm.issuableTemplate.editor.getValue()).toBe('testing new template'); + it('returns value with editor getValue', () => { + createComponent(); + expect(wrapper.vm.issuableTemplate.editor.getValue()).toBe('test'); }); -}); - -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) - 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) - end - end + def feature1_access_level + Featurable::DISABLED + end - describe '.access_level_attribute' do - it { expect(feature_class.access_level_attribute(:wiki)).to eq :wiki_access_level } + def feature2_access_level + Featurable::ENABLED + end - it 'raises error for unspecified feature' do - expect { feature_class.access_level_attribute(:unknown) } - .to raise_error(ArgumentError, /invalid feature: unknown/) + def feature3_access_level + Featurable::PRIVATE + end end end - describe '.set_available_features' do - let!(:klass) do - Class.new(ApplicationRecord) do - include Featurable + subject { klass.new } - self.table_name = 'project_features' - - set_available_features %i(feature1 feature2) + describe '.set_available_features' do + it { expect(klass.available_features).to match_array [:feature1, :feature2, :feature3] } + end - def feature1_access_level - Featurable::DISABLED - end + describe '#*_enabled?' do + it { expect(subject.feature1_enabled?).to be_falsey } + it { expect(subject.feature2_enabled?).to be_truthy } + end - def feature2_access_level - Featurable::ENABLED - end - end + describe '.quoted_access_level_column' do + it 'returns the table name and quoted column name for a feature' do + expect(klass.quoted_access_level_column(:feature1)).to eq('"project_features"."feature1_access_level"') 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) } + describe '.access_level_attribute' do + it { expect(klass.access_level_attribute(:feature1)).to eq :feature1_access_level } + + it 'raises error for unspecified feature' do + expect { klass.access_level_attribute(:unknown) } + .to raise_error(ArgumentError, /invalid feature: unknown/) + end 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" - end + before do + expect(subject).to receive(:member?).and_call_original 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), "#{feature} failed" + 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 member of project group" do - group = create(:group) - project = create(:project, namespace: group) - group.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 - 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) + context 'when user cannot read all resources' do + it 'raises NotImplementedError exception' do + expect(subject).to receive(:resource_member?).and_call_original - 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') } + + 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 - def update_page - subject.update_page( - page.page, - content: 'some other content', - format: :markdown, - message: 'updated page' - ) + 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 + + 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' - expect(page.raw_content).to eq('some other content') + '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 + + 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 - expect(page.version.message).to eq('updated page') + it_behaves_like 'update_page tests' do + include_context 'common examples' + include_context 'extended examples' end - it 'sets the correct commit email' do - update_page + context 'when format is invalid' do + let!(:page) { create(:wiki_page, wiki: subject, title: 'test 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 '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 - it 'runs after_wiki_activity callbacks' do - page + context 'when page path does not have a default extension' do + let!(:page) { create(:wiki_page, wiki: subject, title: 'test page') } - expect(subject).to receive(:after_wiki_activity) + 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) - update_page + 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 |