Detect model scope changes as database changes in Danger

Relates to https://gitlab.com/gitlab-org/gitlab/-/issues/419001

danger/database/Dangerfile

@@ -67,9 +67,7 @@ end
 return unless helper.ci?
 return if helper.mr_labels.include?(DATABASE_APPROVED_LABEL)
 
-db_paths_to_review = helper.changes_by_category[:database]
-
-if helper.mr_labels.include?('database') || db_paths_to_review.any?
+if helper.mr_labels.include?('database') || database.changes.any?
   message 'This merge request adds or changes files that require a ' \
     'review from the [Database team](https://gitlab.com/groups/gl-database/-/group_members).'
 

spec/tooling/danger/database_spec.rb

 # frozen_string_literal: true
 
+require 'rspec-parameterized'
 require 'gitlab-dangerfiles'
 require 'danger'
 require 'danger/plugins/internal/helper'
@@ -41,11 +42,98 @@
 
   let(:cutoff) { Date.parse('2022-10-01') - 21 }
 
-  subject(:database) { fake_danger.new }
+  subject(:database) { fake_danger.new(helper: fake_helper) }
 
   describe '#find_migration_files_before' do
     it 'returns migrations that are before the cutoff' do
       expect(database.find_migration_files_before(migration_files, cutoff).length).to eq(8)
     end
   end
+
+  describe '#changes' do
+    using RSpec::Parameterized::TableSyntax
+
+    where do
+      {
+        'with database changes to a migration file' => {
+          modified_files: %w[
+            db/migrate/20230720114001_test_migration.rb
+            db/schema_migrations/20230720114001
+            db/structure.sql
+            app/models/test.rb
+          ],
+          changed_lines: [],
+          changes_by_category: {
+            database: %w[
+              db/migrate/20230720114001_test_migration.rb
+              db/schema_migrations/20230720114001
+              db/structure.sql
+            ]
+          },
+          impacted_files: %w[
+            db/migrate/20230720114001_test_migration.rb
+            db/schema_migrations/20230720114001
+            db/structure.sql
+          ]
+        },
+        'with non-database changes' => {
+          modified_files: %w[
+            app/models/test.rb
+          ],
+          changed_lines: %w[
+            +# Comment explaining scope :blah
+          ],
+          changes_by_category: {
+            database: []
+          },
+          impacted_files: []
+        },
+        'with database changes in a doc' => {
+          modified_files: %w[doc/development/database/test.md],
+          changed_lines: [
+            '+scope :blah, ->() { where(hidden: false) }'
+          ],
+          changes_by_category: {
+            database: []
+          },
+          impacted_files: []
+        },
+        'with database changes in a model' => {
+          modified_files: %w[app/models/test.rb],
+          changed_lines: [
+            '+# Comment explaining scope :blah',
+            '+scope :blah, ->() { where(hidden: false) }'
+          ],
+          changes_by_category: {
+            database: []
+          },
+          impacted_files: %w[app/models/test.rb]
+        },
+        'with database changes in a concern' => {
+          modified_files: %w[app/models/concerns/test.rb],
+          changed_lines: [
+            '-  .where(hidden: false)',
+            '+  .where(hidden: true)'
+          ],
+          changes_by_category: {
+            database: []
+          },
+          impacted_files: %w[app/models/concerns/test.rb]
+        }
+      }
+    end
+
+    with_them do
+      before do
+        allow(fake_helper).to receive(:modified_files).and_return(modified_files)
+        allow(fake_helper).to receive(:all_changed_files).and_return(modified_files)
+        allow(fake_helper).to receive(:changed_lines).and_return(changed_lines)
+        allow(fake_helper).to receive(:changes_by_category).and_return(changes_by_category)
+      end
+
+      it 'returns database changes' do
+        expect(database.changes).to match impacted_files
+      end
+    end
+  end
 end

tooling/danger/database.rb

@@ -5,6 +5,8 @@ module Danger
     module Database
       TIMESTAMP_MATCHER = /(?<timestamp>\d{14})/
       MIGRATION_MATCHER = %r{\A(ee/)?db/(geo/)?(post_)?migrate/}
+      MODEL_PATHS = %r{\A(ee/)?app/models/}
+      MODEL_CHANGES = %r{^[^#\n]*?(?:scope :|where\(|joins\()}
 
       def find_migration_files_before(file_names, cutoff)
         migrations = file_names.select { |f| f.match?(MIGRATION_MATCHER) }
@@ -15,6 +17,20 @@ def find_migration_files_before(file_names, cutoff)
           timestamp < cutoff
         end
       end
+
+      def changes
+        changed_database_paths + changed_model_paths
+      end
+
+      def changed_database_paths
+        helper.changes_by_category[:database]
+      end
+
+      def changed_model_paths
+        helper.all_changed_files.grep(MODEL_PATHS).select do |file|
+          helper.changed_lines(file).any? { |change| change =~ MODEL_CHANGES }
+        end
+      end
     end
   end
 end