diff --git a/danger/bulk_database_actions/Dangerfile b/danger/bulk_database_actions/Dangerfile
new file mode 100644
index 0000000000000000000000000000000000000000..a8cc7bea000eab205451432314911df8d842a439
--- /dev/null
+++ b/danger/bulk_database_actions/Dangerfile
@@ -0,0 +1,3 @@
+# frozen_string_literal: true
+
+bulk_database_actions.add_comment_for_bulk_database_action_method_usage
diff --git a/danger/plugins/bulk_database_actions.rb b/danger/plugins/bulk_database_actions.rb
new file mode 100644
index 0000000000000000000000000000000000000000..cc1b21a6673ecd580b814630716f9b6298773cde
--- /dev/null
+++ b/danger/plugins/bulk_database_actions.rb
@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+require_relative '../../tooling/danger/bulk_database_actions'
+
+module Danger
+ class BulkDatabaseActions < ::Danger::Plugin
+ include Tooling::Danger::BulkDatabaseActions
+ end
+end
diff --git a/doc/development/database_review.md b/doc/development/database_review.md
index 42021a5ae95f4feac71d484dac06dea8aa048f2b..d742fe8a54b781efd879fb9e98fcdfc607094dfc 100644
--- a/doc/development/database_review.md
+++ b/doc/development/database_review.md
@@ -29,6 +29,8 @@ 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)
+ methods on an ActiveRecord object.
A database reviewer is expected to look out for overly complex
queries in the change and review those closer. If the author does not
@@ -216,6 +218,15 @@ 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`
+
+Using these 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. Danger will comment on a Merge Request Diff when these methods are used.
+
+Follow documentation for [preparation when adding or modifying queries](#preparation-when-adding-or-modifying-queries)
+to add the raw SQL query and query plan to the Merge Request description, and request a database review.
+
### How to review for database
- Check migrations
diff --git a/spec/tooling/danger/bulk_database_actions_spec.rb b/spec/tooling/danger/bulk_database_actions_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..584730ba30875fb3881f705ed662fc0c9d99d204
--- /dev/null
+++ b/spec/tooling/danger/bulk_database_actions_spec.rb
@@ -0,0 +1,109 @@
+# frozen_string_literal: true
+
+require 'gitlab-dangerfiles'
+require 'danger'
+require 'danger/plugins/internal/helper'
+require 'gitlab/dangerfiles/spec_helper'
+require 'rspec-parameterized'
+
+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"
+
+ let(:fake_danger) { DangerSpecHelper.fake_danger.include(described_class) }
+ 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(:file_diff) do
+ [
+ "+ def execute",
+ "+ pat_family.active.#{method_call}",
+ "+",
+ "+ ServiceResponse.success",
+ "+ end"
+ ]
+ end
+
+ 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)
+ end
+
+ subject(:bulk_database_actions) { fake_danger.new(helper: fake_helper) }
+
+ describe '#add_comment_for_bulk_database_action_method_usage' do
+ 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
+ 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
+ end
+ end
+ end
+
+ context 'for spec directories' do
+ let(:method_call) { 'update_all(revoked: true)' }
+
+ 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
+ end
+ end
+
+ 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'
+ end
+
+ context 'for JiHu spec file' do
+ let(:filename) { 'jh/spec/services/personal_access_tokens/revoke_token_family_service_spec.rb' }
+
+ it_behaves_like 'no Danger comment'
+ end
+ end
+ end
+end
diff --git a/tooling/danger/bulk_database_actions.rb b/tooling/danger/bulk_database_actions.rb
new file mode 100644
index 0000000000000000000000000000000000000000..9a26243fb2db7e8a2cd18d02ddc1f207f345cbc0
--- /dev/null
+++ b/tooling/danger/bulk_database_actions.rb
@@ -0,0 +1,34 @@
+# frozen_string_literal: true
+
+require_relative 'suggestor'
+
+module Tooling
+ module Danger
+ module BulkDatabaseActions
+ include ::Tooling::Danger::Suggestor
+
+ BULK_UPDATE_METHODS_REGEX = /\W(((update|delete|destroy)_all)|delete|update)(\(|\s+|$)/
+
+ 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
+
+ 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
+ end
+ end
+end