diff options
21 files changed, 175 insertions, 164 deletions
diff --git a/app/assets/javascripts/comment_templates/components/form.vue b/app/assets/javascripts/comment_templates/components/form.vue index 5a5d221591a..0e92f20f0c1 100644 --- a/app/assets/javascripts/comment_templates/components/form.vue +++ b/app/assets/javascripts/comment_templates/components/form.vue @@ -1,11 +1,12 @@ <!-- eslint-disable vue/multi-word-component-names --> <script> import { GlButton, GlForm, GlFormGroup, GlFormInput, GlAlert } from '@gitlab/ui'; -import MarkdownField from '~/vue_shared/components/markdown/field.vue'; +import MarkdownEditor from '~/vue_shared/components/markdown/markdown_editor.vue'; import { helpPagePath } from '~/helpers/help_page_helper'; import { logError } from '~/lib/logger'; import { __ } from '~/locale'; import { InternalEvents } from '~/tracking'; +import Api from '~/api'; import createSavedReplyMutation from '../queries/create_saved_reply.mutation.graphql'; import updateSavedReplyMutation from '../queries/update_saved_reply.mutation.graphql'; @@ -16,7 +17,7 @@ export default { GlFormGroup, GlFormInput, GlAlert, - MarkdownField, + MarkdownEditor, }, mixins: [InternalEvents.mixin()], props: { @@ -45,6 +46,14 @@ export default { name: this.name, content: this.content, }, + formFieldProps: { + id: 'comment-template-content', + name: 'comment-template-content', + 'aria-label': __('Content'), + placeholder: __('Write comment template content here…'), + 'data-testid': 'comment-template-content-input', + class: 'note-textarea js-gfm-input js-autosize markdown-area', + }, }; }, computed: { @@ -61,6 +70,9 @@ export default { isValid() { return this.isNameValid && this.isContentValid; }, + markdownPath() { + return Api.buildUrl(Api.markdownPath); + }, }, methods: { onCancel() { @@ -116,7 +128,6 @@ export default { }); }, }, - restrictedToolbarItems: ['full-screen'], markdownDocsPath: helpPagePath('user/markdown'), }; </script> @@ -156,30 +167,19 @@ export default { data-testid="comment-template-content-form-group" class="gl-lg-max-w-80p" > - <markdown-field - :enable-preview="false" + <markdown-editor + v-model="updateCommentTemplate.content" + class="js-no-autosize" :is-submitting="saving" - :add-spacing-classes="false" - :textarea-value="updateCommentTemplate.content" + :disable-attachments="true" + :render-markdown-path="markdownPath" :markdown-docs-path="$options.markdownDocsPath" + :form-field-props="formFieldProps" :restricted-tool-bar-items="$options.restrictedToolbarItems" :force-autosize="false" - class="js-no-autosize gl-border-gray-400!" - > - <template #textarea> - <textarea - v-model="updateCommentTemplate.content" - dir="auto" - class="note-textarea js-gfm-input js-autosize markdown-area" - data-supports-quick-actions="false" - :aria-label="__('Content')" - :placeholder="__('Write comment template content here…')" - data-testid="comment-template-content-input" - @keydown.meta.enter="onSubmit" - @keydown.ctrl.enter="onSubmit" - ></textarea> - </template> - </markdown-field> + @keydown.meta.enter="onSubmit" + @keydown.ctrl.enter="onSubmit" + /> </gl-form-group> <gl-button variant="confirm" diff --git a/app/assets/javascripts/vue_shared/components/markdown/markdown_editor.vue b/app/assets/javascripts/vue_shared/components/markdown/markdown_editor.vue index 7aa3be9a6bd..8fedc816502 100644 --- a/app/assets/javascripts/vue_shared/components/markdown/markdown_editor.vue +++ b/app/assets/javascripts/vue_shared/components/markdown/markdown_editor.vue @@ -55,7 +55,7 @@ export default { uploadsPath: { type: String, required: false, - default: () => window.uploads_path, + default: () => window.uploads_path || '', }, enableContentEditor: { type: Boolean, @@ -192,7 +192,7 @@ export default { { render_quick_actions: this.supportsQuickActions }, joinPaths(window.location.origin, this.renderMarkdownPath), ); - return axios.post(url, { text: markdown }).then(({ data }) => data.body); + return axios.post(url, { text: markdown }).then(({ data }) => data.body || data.html); }, onEditingModeChange(editingMode) { this.editingMode = editingMode; diff --git a/app/finders/issuable_finder/params.rb b/app/finders/issuable_finder/params.rb index bc136848dd5..6b56c966025 100644 --- a/app/finders/issuable_finder/params.rb +++ b/app/finders/issuable_finder/params.rb @@ -135,14 +135,9 @@ class IssuableFinder strong_memoize(:projects) do next Array.wrap(project) if project? - projects = - if current_user && params[:authorized_only].presence && !current_user_related? - current_user.authorized_projects(min_access_level) - else - projects_public_or_visible_to_user - end - - projects.with_feature_available_for_user(klass.base_class, current_user).reorder(nil) # rubocop: disable CodeReuse/ActiveRecord + projects_public_or_visible_to_user + .with_feature_available_for_user(klass.base_class, current_user) + .without_order end end diff --git a/app/models/project.rb b/app/models/project.rb index ea5eff7e6a6..2b79dee5b45 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -3217,6 +3217,11 @@ class Project < ApplicationRecord false end + # Overridden in EE + def on_demand_dast_available? + false + end + private # overridden in EE diff --git a/danger/bulk_database_actions/Dangerfile b/danger/bulk_database_actions/Dangerfile index a8cc7bea000..5b28ceca029 100644 --- a/danger/bulk_database_actions/Dangerfile +++ b/danger/bulk_database_actions/Dangerfile @@ -1,3 +1,8 @@ # frozen_string_literal: true -bulk_database_actions.add_comment_for_bulk_database_action_method_usage +helper.all_changed_files.each do |filename| + next unless filename.end_with?('.rb') + next if filename.start_with?('spec/', 'ee/spec/', 'jh/spec/') + + bulk_database_actions.add_suggestions_for(filename) +end diff --git a/danger/plugins/bulk_database_actions.rb b/danger/plugins/bulk_database_actions.rb index cc1b21a6673..5ae47119fb8 100644 --- a/danger/plugins/bulk_database_actions.rb +++ b/danger/plugins/bulk_database_actions.rb @@ -4,6 +4,8 @@ require_relative '../../tooling/danger/bulk_database_actions' module Danger class BulkDatabaseActions < ::Danger::Plugin - include Tooling::Danger::BulkDatabaseActions + def add_suggestions_for(filename) + Tooling::Danger::BulkDatabaseActions.new(filename, context: self).suggest + end end end diff --git a/doc/api/integrations.md b/doc/api/integrations.md index f94108475c9..babb8ec40df 100644 --- a/doc/api/integrations.md +++ b/doc/api/integrations.md @@ -696,11 +696,11 @@ Get the GitHub integration settings for a project. GET /projects/:id/integrations/github ``` -## GitLab for Slack app +## Slack notifications -### Set up the GitLab for Slack app +### Set up Slack notifications -Set up the GitLab for Slack app for a project. +Set up Slack notifications for a project. ```plaintext PUT /projects/:id/integrations/slack @@ -745,17 +745,17 @@ Parameters: | `wiki_page_channel` | string | false | The name of the channel to receive notifications for wiki page events. | | `wiki_page_events` | boolean | false | Enable notifications for wiki page events. | -### Disable the GitLab for Slack app +### Disable Slack notifications -Disable the GitLab for Slack app for a project. Integration settings are reset. +Disable Slack notifications for a project. Integration settings are reset. ```plaintext DELETE /projects/:id/integrations/slack ``` -### Get the GitLab for Slack app settings +### Get Slack notifications settings -Get the GitLab for Slack app settings for a project. +Get the Slack notifications settings for a project. ```plaintext GET /projects/:id/integrations/slack diff --git a/doc/ci/runners/runner_fleet_dashboard.md b/doc/ci/runners/runner_fleet_dashboard.md index ce487ff7386..a79f5ab5713 100644 --- a/doc/ci/runners/runner_fleet_dashboard.md +++ b/doc/ci/runners/runner_fleet_dashboard.md @@ -39,24 +39,25 @@ These features require [setting up an additional infrastructure](#enable-more-ci ## Enable more CI analytics features with ClickHouse **(ULTIMATE EXPERIMENT)** -> [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/11180) in GitLab 16.7 behind several [feature flags](#enable-clickhouse-integration-and-features). +> - [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/11180) in GitLab 16.7 with the [flags](../../administration/feature_flags.md) named `ci_data_ingestion_to_click_house` and `clickhouse_ci_analytics`. Disabled by default. +> - [Enabled by default](https://gitlab.com/gitlab-org/gitlab/-/issues/424866) in GitLab 16.8. Feature flag `clickhouse_ci_analytics` removed. This feature is an [Experiment](../../policy/experiment-beta-support.md). To test it, we have launched an early adopters program. To join the list of users testing this feature, see [epic 11180](https://gitlab.com/groups/gitlab-org/-/epics/11180). -### Enable ClickHouse integration and features +### Enable ClickHouse integration To enable additional CI analytics features: 1. [Configure ClickHouse integration](../../integration/clickhouse.md) 1. [Enable](../../administration/feature_flags.md#how-to-enable-and-disable-features-behind-flags) the following feature flags: - | Feature flag name | Purpose | - |------------------------------------|---------------------------------------------------------------------------| - | `ci_data_ingestion_to_click_house` | Enables synchronization of new finished CI builds to ClickHouse database. | - | `clickhouse_ci_analytics` | Enables the **Wait time to pick a job** chart. | + | Feature flag name | Purpose | Status | + |------------------------------------|---------------------------------------------------------------------------|------------------------------------| + | `ci_data_ingestion_to_click_house` | Enables synchronization of new finished CI builds to ClickHouse database. | Enabled by default in GitLab 16.8. | + | `clickhouse_ci_analytics` | Enables the **Wait time to pick a job** chart. | Removed in 16.8. | <i class="fa fa-youtube-play youtube" aria-hidden="true"></i> For a video walkthrough, see [Setting up Runner Fleet Dashboard with ClickHouse](https://www.youtube.com/watch?v=YpGV95Ctbpk). diff --git a/doc/ci/variables/index.md b/doc/ci/variables/index.md index 8e8f9367c08..49f5f1edf41 100644 --- a/doc/ci/variables/index.md +++ b/doc/ci/variables/index.md @@ -638,7 +638,7 @@ To disable variable expansion for the variable: ## CI/CD variable precedence -> Scan Execution Policies variable precedence was [changed](https://gitlab.com/gitlab-org/gitlab/-/issues/424028) in GitLab 16.7 [with a flag](../../administration/feature_flags.md) named `security_policies_variables_precedence`. Enabled by default. +> Scan Execution Policies variable precedence was [changed](https://gitlab.com/gitlab-org/gitlab/-/issues/424028) in GitLab 16.7 [with a flag](../../administration/feature_flags.md) named `security_policies_variables_precedence`. Enabled by default. [Feature flag removed in GitLab 16.8](https://gitlab.com/gitlab-org/gitlab/-/issues/435727). You can use CI/CD variables with the same name in different places, but the values can overwrite each other. The type of variable and where they are defined determines diff --git a/doc/development/database_review.md b/doc/development/database_review.md index f4fd81f85ec..2d02d7e25de 100644 --- a/doc/development/database_review.md +++ b/doc/development/database_review.md @@ -29,7 +29,7 @@ A database review is required for: These metrics could have complex queries over large tables. See the [Analytics Instrumentation Guide](https://about.gitlab.com/handbook/product/analytics-instrumentation-guide/) for implementation details. -- Changes that use [`update`, `delete`, `update_all`, `delete_all` or `destroy_all`](#preparation-when-using-update-delete-update_all-delete_all-or-destroy_all) +- Changes that use [`update`, `upsert`, `delete`, `update_all`, `upsert_all`, `delete_all` or `destroy_all`](#preparation-when-using-bulk-update-operations) methods on an ActiveRecord object. A database reviewer is expected to look out for overly complex @@ -227,9 +227,10 @@ Include in the MR description: - If you're adding a composite index, another index might become redundant, so remove that in the same migration. For example adding `index(column_A, column_B, column_C)` makes the indexes `index(column_A, column_B)` and `index(column_A)` redundant. -#### Preparation when using `update`, `delete`, `update_all`, `delete_all` or `destroy_all` +#### Preparation when using bulk update operations -Using these ActiveRecord methods requires extra care because they modify data and can perform poorly, or they +Using `update`, `upsert`, `delete`, `update_all`, `upsert_all`, `delete_all` or `destroy_all` +ActiveRecord methods requires extra care because they modify data and can perform poorly, or they can destroy data if improperly scoped. These methods are also [incompatible with Common Table Expression (CTE) statements](sql.md#when-to-use-common-table-expressions). Danger will comment on a Merge Request Diff when these methods are used. diff --git a/doc/user/application_security/policies/scan-execution-policies.md b/doc/user/application_security/policies/scan-execution-policies.md index 7250b00ce90..767a2207c15 100644 --- a/doc/user/application_security/policies/scan-execution-policies.md +++ b/doc/user/application_security/policies/scan-execution-policies.md @@ -205,7 +205,7 @@ The keys for a schedule rule are: ## `scan` action type -> Scan Execution Policies variable precedence was [changed](https://gitlab.com/gitlab-org/gitlab/-/issues/424028) in GitLab 16.6 [with a flag](../../../administration/feature_flags.md) named `security_policies_variables_precedence`. Disabled by default. +> Scan Execution Policies variable precedence was [changed](https://gitlab.com/gitlab-org/gitlab/-/issues/424028) in GitLab 16.7 [with a flag](../../../administration/feature_flags.md) named `security_policies_variables_precedence`. Enabled by default. [Feature flag removed in GitLab 16.8](https://gitlab.com/gitlab-org/gitlab/-/issues/435727). This action executes the selected `scan` with additional parameters when conditions for at least one rule in the defined policy are met. diff --git a/lib/gitlab/ci/templates/Security/DAST-On-Demand-Scan.gitlab-ci.yml b/lib/gitlab/ci/templates/Security/DAST-On-Demand-Scan.gitlab-ci.yml index 1ed4cd86e82..4b60298353d 100644 --- a/lib/gitlab/ci/templates/Security/DAST-On-Demand-Scan.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Security/DAST-On-Demand-Scan.gitlab-ci.yml @@ -21,7 +21,7 @@ variables: dast: stage: dast image: - name: "$SECURE_ANALYZERS_PREFIX/dast:$DAST_VERSION" + name: "$SECURE_ANALYZERS_PREFIX/dast:$DAST_VERSION$DAST_IMAGE_SUFFIX" variables: GIT_STRATEGY: none allow_failure: true @@ -30,3 +30,10 @@ dast: artifacts: reports: dast: gl-dast-report.json + rules: + - if: $CI_GITLAB_FIPS_MODE == "true" + variables: + DAST_IMAGE_SUFFIX: "-fips" + - if: $CI_GITLAB_FIPS_MODE != "true" + variables: + DAST_IMAGE_SUFFIX: "" diff --git a/lib/gitlab/ci/templates/Security/DAST-Runner-Validation.gitlab-ci.yml b/lib/gitlab/ci/templates/Security/DAST-Runner-Validation.gitlab-ci.yml index c75ff2e9ff8..8043b6a95cc 100644 --- a/lib/gitlab/ci/templates/Security/DAST-Runner-Validation.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Security/DAST-Runner-Validation.gitlab-ci.yml @@ -18,9 +18,16 @@ variables: validation: stage: dast image: - name: "$CI_TEMPLATE_REGISTRY_HOST/security-products/dast-runner-validation:$DAST_RUNNER_VALIDATION_VERSION" + name: "$CI_TEMPLATE_REGISTRY_HOST/security-products/dast-runner-validation:$DAST_RUNNER_VALIDATION_VERSION$DAST_IMAGE_SUFFIX" variables: GIT_STRATEGY: none allow_failure: false script: - ~/validate.sh + rules: + - if: $CI_GITLAB_FIPS_MODE == "true" + variables: + DAST_IMAGE_SUFFIX: "-fips" + - if: $CI_GITLAB_FIPS_MODE != "true" + variables: + DAST_IMAGE_SUFFIX: "" diff --git a/locale/gitlab.pot b/locale/gitlab.pot index c2da2cf6deb..051c3918d7c 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -20508,6 +20508,9 @@ msgstr "" msgid "Failed to create a to-do item for the design." msgstr "" +msgid "Failed to create branch target" +msgstr "" + msgid "Failed to create framework" msgstr "" @@ -20523,9 +20526,6 @@ msgstr "" msgid "Failed to create resources" msgstr "" -msgid "Failed to create target branch rule" -msgstr "" - msgid "Failed to create wiki" msgstr "" @@ -56469,7 +56469,7 @@ msgstr "" msgid "You have insufficient permissions to create a Todo for this alert" msgstr "" -msgid "You have insufficient permissions to create a target branch rule" +msgid "You have insufficient permissions to create a branch target" msgstr "" msgid "You have insufficient permissions to create an HTTP integration for this project" diff --git a/spec/controllers/concerns/issuable_collections_spec.rb b/spec/controllers/concerns/issuable_collections_spec.rb index 9eb0f36cb37..051172ea6da 100644 --- a/spec/controllers/concerns/issuable_collections_spec.rb +++ b/spec/controllers/concerns/issuable_collections_spec.rb @@ -74,7 +74,6 @@ RSpec.describe IssuableCollections do assignee_username: 'user1', author_id: '2', author_username: 'user2', - authorized_only: 'yes', confidential: true, due_date: '2017-01-01', group_id: '3', diff --git a/spec/frontend/comment_templates/components/form_spec.js b/spec/frontend/comment_templates/components/form_spec.js index 05ddf94b72b..ab368a42483 100644 --- a/spec/frontend/comment_templates/components/form_spec.js +++ b/spec/frontend/comment_templates/components/form_spec.js @@ -135,6 +135,18 @@ describe('Comment templates form component', () => { expect(findSubmitBtn().props('loading')).toBe(false); }); + + it('shows markdown preview button', () => { + wrapper = createComponent(); + + expect(wrapper.text()).toContain('Preview'); + }); + + it('allows switching to rich text editor', () => { + wrapper = createComponent(); + + expect(wrapper.text()).toContain('Switch to rich text editing'); + }); }); describe('updates saved reply', () => { diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 7b8b6376aea..1743c9bd89d 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -9006,6 +9006,14 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr it { is_expected.to eq(false) } end + describe '#on_demand_dast_available?' do + let_it_be(:project) { create(:project) } + + subject(:on_demand_dast_available?) { project.on_demand_dast_available? } + + it { is_expected.to be_falsy } + end + private def finish_job(export_job) diff --git a/spec/tooling/danger/bulk_database_actions_spec.rb b/spec/tooling/danger/bulk_database_actions_spec.rb index eba3eacb212..18a46f663c0 100644 --- a/spec/tooling/danger/bulk_database_actions_spec.rb +++ b/spec/tooling/danger/bulk_database_actions_spec.rb @@ -8,115 +8,87 @@ require_relative '../../../tooling/danger/bulk_database_actions' require_relative '../../../tooling/danger/project_helper' RSpec.describe Tooling::Danger::BulkDatabaseActions, feature_category: :tooling do - include_context "with dangerfile" + include_context 'with dangerfile' - let(:fake_danger) { DangerSpecHelper.fake_danger.include(described_class) } + let(:fake_danger) { DangerSpecHelper.fake_danger } let(:fake_project_helper) { instance_double(Tooling::Danger::ProjectHelper) } - - let(:mr_url) { 'https://gitlab.com/gitlab-org/gitlab/-/merge_requests/1' } - let(:doc_link) { described_class::DOCUMENTATION_LINK } - - let(:comment_text) { "\n#{described_class::COMMENT_TEXT}" } - - let(:file_lines) do - file_diff.map { |line| line.delete_prefix('+') } - end + let(:comment_text) { "\n#{described_class::SUGGESTION}" } + let(:file_lines) { file_diff.map { |line| line.delete_prefix('+') } } before do allow(bulk_database_actions).to receive(:project_helper).and_return(fake_project_helper) allow(bulk_database_actions.project_helper).to receive(:file_lines).and_return(file_lines) allow(bulk_database_actions.helper).to receive(:added_files).and_return([filename]) allow(bulk_database_actions.helper).to receive(:changed_lines).with(filename).and_return(file_diff) - allow(bulk_database_actions.helper).to receive(:mr_web_url).and_return(mr_url) + + bulk_database_actions.define_singleton_method(:add_suggestions_for) do |filename| + Tooling::Danger::BulkDatabaseActions.new(filename, context: self).suggest + end end subject(:bulk_database_actions) { fake_danger.new(helper: fake_helper) } - shared_examples 'no Danger comment' do - it 'does not comment on the bulk update action usage' do - expect(bulk_database_actions).not_to receive(:markdown) - - bulk_database_actions.add_comment_for_bulk_database_action_method_usage + context 'for single line method call' do + let(:file_diff) do + <<~DIFF.split("\n") + + def execute + + #{code} + + + + ServiceResponse.success + + end + DIFF end - end - describe '#add_comment_for_bulk_database_action_method_usage' do - context 'for single line method call' do - let(:file_diff) do - [ - "+ def execute", - "+ pat_family.active.#{method_call}", - "+", - "+ ServiceResponse.success", - "+ end" - ] - end + context 'when file is a non-spec Ruby file' do + let(:filename) { 'app/services/personal_access_tokens/revoke_token_family_service.rb' } - context 'when file is a non-spec Ruby file' do - let(:filename) { 'app/services/personal_access_tokens/revoke_token_family_service.rb' } - - using RSpec::Parameterized::TableSyntax - - where(:method_call, :expect_comment?) do - 'update_all(revoked: true)' | true - 'destroy_all' | true - 'delete_all' | true - 'update(revoked: true)' | true - 'delete' | true - 'update_two_factor' | false - 'delete_keys(key)' | false - 'destroy_hook(hook)' | false - 'destroy_all_merged' | false - 'update_all_mirrors' | false + using RSpec::Parameterized::TableSyntax + + context 'when comment is expected' do + where(:code) do + [ + 'update_all(revoked: true)', + 'destroy_all', + 'delete_all', + 'update(revoked: true)', + 'delete', + 'upsert', + 'upsert_all', + 'User.upsert', + 'User.last.destroy', + ' .destroy' + ] end with_them do - it "correctly handles potential bulk database action" do - if expect_comment? - expect(bulk_database_actions).to receive(:markdown).with(comment_text, file: filename, line: 2) - else - expect(bulk_database_actions).not_to receive(:markdown) - end - - bulk_database_actions.add_comment_for_bulk_database_action_method_usage + specify do + expect(bulk_database_actions).to receive(:markdown).with(comment_text.chomp, file: filename, line: 2) + + bulk_database_actions.add_suggestions_for(filename) end end end - context 'for spec directories' do - let(:method_call) { 'update_all(revoked: true)' } - - context 'for FOSS spec file' do - let(:filename) { 'spec/services/personal_access_tokens/revoke_token_family_service_spec.rb' } - - it_behaves_like 'no Danger comment' - end - - context 'for EE spec file' do - let(:filename) { 'ee/spec/services/personal_access_tokens/revoke_token_family_service_spec.rb' } - - it_behaves_like 'no Danger comment' + context 'when no comment is expected' do + where(:code) do + [ + 'we update bob', + 'update_two_factor', + 'delete_keys(key)', + 'destroy_hook(hook)', + 'destroy_all_merged', + 'update_all_mirrors' + ] end - context 'for JiHu spec file' do - let(:filename) { 'jh/spec/services/personal_access_tokens/revoke_token_family_service_spec.rb' } + with_them do + specify do + expect(bulk_database_actions).not_to receive(:markdown) - it_behaves_like 'no Danger comment' + bulk_database_actions.add_suggestions_for(filename) + end end end end - - context 'for strings' do - let(:filename) { 'app/services/personal_access_tokens/revoke_token_family_service.rb' } - let(:file_diff) do - [ - '+ expect { subject }.to output(', - '+ "ERROR: Could not update tag"', - '+ ).to_stderr' - ] - end - - it_behaves_like 'no Danger comment' - end end end diff --git a/spec/tooling/danger/project_helper_spec.rb b/spec/tooling/danger/project_helper_spec.rb index a41aba17f56..4eff248a6f0 100644 --- a/spec/tooling/danger/project_helper_spec.rb +++ b/spec/tooling/danger/project_helper_spec.rb @@ -251,6 +251,11 @@ RSpec.describe Tooling::Danger::ProjectHelper, feature_category: :tooling do [:backend, :analytics_instrumentation] | '+ count(User.active)' | ['lib/gitlab/usage_data/topology.rb'] [:backend, :analytics_instrumentation] | '+ foo_count(User.active)' | ['lib/gitlab/usage_data.rb'] [:backend] | '+ count(User.active)' | ['user.rb'] + [:database, :backend] | '+ User.upsert({ name: "blah" })' | ['app/foo/bar.rb'] + [:database, :backend] | '+ upsert({ name: "blah" })' | ['app/foo/bar.rb'] + [:database, :backend] | '+ .upsert({ name: "blah" })' | ['app/foo/bar.rb'] + [:database, :backend] | '+ .delete_all' | ['app/foo/bar.rb'] + [:database, :backend] | '+ .destroy_all' | ['app/foo/bar.rb'] [:import_integrate_be, :database] | '+ add_column :integrations, :foo, :text' | ['db/migrate/foo.rb'] [:import_integrate_be, :database] | '+ create_table :zentao_tracker_data do |t|' | ['ee/db/post_migrate/foo.rb'] [:import_integrate_be, :backend] | '+ Integrations::Foo' | ['app/foo/bar.rb'] diff --git a/tooling/danger/bulk_database_actions.rb b/tooling/danger/bulk_database_actions.rb index 0f74e31cdde..7f3edaf7663 100644 --- a/tooling/danger/bulk_database_actions.rb +++ b/tooling/danger/bulk_database_actions.rb @@ -1,34 +1,25 @@ # frozen_string_literal: true -require_relative 'suggestor' +require_relative 'suggestion' module Tooling module Danger - module BulkDatabaseActions - include ::Tooling::Danger::Suggestor + class BulkDatabaseActions < Suggestion + MATCH = %r{\A\+\s+(\S*\.)?((update|upsert|delete|destroy)(_all)?)\b} + REPLACEMENT = nil + DOCUMENTATION_LINK = 'https://docs.gitlab.com/ee/development/database_review.html#preparation-when-using-bulk-update-operations' - BULK_UPDATE_METHODS_REGEX = /\.((update|delete|destroy)(_all)?)\b/ + SUGGESTION = <<~MESSAGE_MARKDOWN.freeze + When using `update`, `upsert`, `delete`, `update_all`, `upsert_all`, `delete_all` or `destroy_all` + you must include the full database query and query execution plan in the merge request description, + and request a ~database review. - DOCUMENTATION_LINK = 'https://docs.gitlab.com/ee/development/database_review.html#preparation-when-using-update-delete-update_all-and-destroy_all' - COMMENT_TEXT = - "When using `update`, `delete`, `update_all`, `delete_all` or `destroy_all` you must include the full " \ - "database query and query execution plan in the merge request description, and request a ~database review. " \ - "This comment can be ignored if the object is not an ActiveRecord class, since no database query " \ - "would be generated. For more information, see [Database Review documentation](#{DOCUMENTATION_LINK}).".freeze + This comment can be ignored if the object is not an ActiveRecord class, since no database query would be generated. - def add_comment_for_bulk_database_action_method_usage - changed_ruby_files.each do |filename| - add_suggestion( - filename: filename, - regex: BULK_UPDATE_METHODS_REGEX, - comment_text: COMMENT_TEXT - ) - end - end + ---- - def changed_ruby_files - helper.added_files.select { |f| f.end_with?('.rb') && !f.start_with?('spec/', 'ee/spec/', 'jh/spec/') } - end + For more information, see [Database Review documentation](#{DOCUMENTATION_LINK}). + MESSAGE_MARKDOWN end end end diff --git a/tooling/danger/project_helper.rb b/tooling/danger/project_helper.rb index e0953d59dad..633516591a8 100644 --- a/tooling/danger/project_helper.rb +++ b/tooling/danger/project_helper.rb @@ -105,6 +105,7 @@ module Tooling %r{\A(app/models/project_authorization|app/services/users/refresh_authorized_projects_service)(/|\.rb)} => [:database, :backend], %r{\A((ee|jh)/)?app/finders/} => [:database, :backend], %r{\Arubocop/cop/migration(/|\.rb)} => :database, + [%r{\A((ee|jh)/)?(app|lib)/.+\.rb}, %r{\A\+\s+(\w*\.)?(update_all|upsert|upsert_all|delete_all|destroy_all)(\(.*\))?\s*\z}] => [:database, :backend], %r{\Alib/gitlab/ci/templates} => :ci_template, |