Merge branch 'am-add-estimate_batch_distinct_count-changes-to-dangerbot' into 'master'

Add estimate_batch_distinct_count changes to dangerbot database

See merge request gitlab-org/gitlab!50149

doc/development/database_review.md

@@ -25,7 +25,7 @@ A database review is required for:
   generally up to the author of a merge request to decide whether or
   not complex queries are being introduced and if they require a
   database review.
-- Changes in usage data metrics that use `count` and `distinct_count`.
+- Changes in usage data metrics that use `count`, `distinct_count` and `estimate_batch_distinct_count`.
   These metrics could have complex queries over large tables.
   See the [Product Analytics Guide](https://about.gitlab.com/handbook/product/product-analytics-guide/)
   for implementation details.

lib/gitlab/danger/helper.rb

@@ -128,7 +128,7 @@ module Gitlab
       }.freeze
       # First-match win, so be sure to put more specific regex at the top...
       CATEGORIES = {
-        [%r{usage_data\.rb}, %r{^(\+|-).*(count|distinct_count)\(.*\)(.*)$}] => [:database, :backend],
+        [%r{usage_data\.rb}, %r{^(\+|-).*\s+(count|distinct_count|estimate_batch_distinct_count)\(.*\)(.*)$}] => [:database, :backend],
 
         %r{\Adoc/.*(\.(md|png|gif|jpg))\z} => :docs,
         %r{\A(CONTRIBUTING|LICENSE|MAINTENANCE|PHILOSOPHY|PROCESS|README)(\.md)?\z} => :docs,

spec/lib/gitlab/danger/helper_spec.rb

@@ -351,33 +351,23 @@ RSpec.describe Gitlab::Danger::Helper do
     end
 
     context 'having specific changes' do
-      it 'has database and backend categories' do
-        changed_files = ['usage_data.rb', 'lib/gitlab/usage_data.rb', 'ee/lib/ee/gitlab/usage_data.rb']
-
-        changed_files.each do |file|
-          allow(fake_git).to receive(:diff_for_file).with(file) { double(:diff, patch: "+ count(User.active)") }
-
-          expect(helper.categories_for_file(file)).to eq([:database, :backend])
-        end
-      end
-
-      it 'has backend category' do
-        allow(fake_git).to receive(:diff_for_file).with('usage_data.rb') { double(:diff, patch: "+ alt_usage_data(User.active)") }
-
-        expect(helper.categories_for_file('usage_data.rb')).to eq([:backend])
+      where(:expected_categories, :patch, :changed_files) do
+        [:database, :backend] | '+ count(User.active)'                         | ['usage_data.rb', 'lib/gitlab/usage_data.rb', 'ee/lib/ee/gitlab/usage_data.rb']
+        [:database, :backend] | '+ estimate_batch_distinct_count(User.active)' | ['usage_data.rb']
+        [:backend]            | '+ alt_usage_data(User.active)'                | ['usage_data.rb']
+        [:backend]            | '+ count(User.active)'                         | ['user.rb']
+        [:backend]            | '+ count(User.active)'                         | ['usage_data/topology.rb']
+        [:backend]            | '+ foo_count(User.active)'                     | ['usage_data.rb']
       end
 
-      it 'has backend category for changes outside usage_data files' do
-        allow(fake_git).to receive(:diff_for_file).with('user.rb') { double(:diff, patch: "+ count(User.active)") }
-
-        expect(helper.categories_for_file('user.rb')).to eq([:backend])
-      end
+      with_them do
+        it 'has the correct categories' do
+          changed_files.each do |file|
+            allow(fake_git).to receive(:diff_for_file).with(file) { double(:diff, patch: patch) }
 
-      it 'has backend category for files that are not usage_data.rb' do
-        changed_file = 'usage_data/topology.rb'
-        allow(fake_git).to receive(:diff_for_file).with(changed_file) { double(:diff, patch: "+ count(User.active)") }
-
-        expect(helper.categories_for_file(changed_file)).to eq([:backend])
+            expect(helper.categories_for_file(file)).to eq(expected_categories)
+          end
+        end
       end
     end
   end