Merge branch '328721-follow-up-from-update-code-review-metrics-definitions' into 'master'

Resolve "Follow-up from "Update code review metrics definitions""

See merge request gitlab-org/gitlab!60077

app/models/note.rb

@@ -107,6 +107,7 @@ class Note < ApplicationRecord
   scope :fresh, -> { order_created_asc.with_order_id_asc }
   scope :updated_after, ->(time) { where('updated_at > ?', time) }
   scope :with_updated_at, ->(time) { where(updated_at: time) }
+  scope :with_suggestions, -> { joins(:suggestions) }
   scope :inc_author_project, -> { includes(:project, :author) }
   scope :inc_author, -> { includes(:author) }
   scope :with_api_entity_associations, -> { preload(:note_diff_file, :author) }

ee/app/models/ee/note.rb

@@ -12,7 +12,6 @@ module EE
 
       scope :searchable, -> { where(system: false).includes(:noteable) }
       scope :by_humans, -> { user.joins(:author).merge(::User.humans) }
-      scope :with_suggestions, -> { joins(:suggestions) }
       scope :count_for_vulnerability_id, ->(vulnerability_id) do
         where(noteable_type: ::Vulnerability.name, noteable_id: vulnerability_id)
           .group(:noteable_id)

ee/lib/ee/gitlab/usage_data.rb

@@ -260,10 +260,6 @@ module EE
                                                :creator_id,
                                                start: minimum_id(::User),
                                                finish: maximum_id(::User)),
-            suggestions: distinct_count(::Note.with_suggestions.where(time_period),
-                                        :author_id,
-                                        start: minimum_id(::User),
-                                        finish: maximum_id(::User)),
             users_using_path_locks: distinct_count(PathLock.where(time_period), :user_id),
             users_using_lfs_locks: distinct_count(LfsFileLock.where(time_period), :user_id),
             total_number_of_path_locks: count(::PathLock.where(time_period)),

ee/spec/lib/ee/gitlab/usage_data_spec.rb

@@ -299,7 +299,6 @@ RSpec.describe Gitlab::UsageData do
         create(:approval_project_rule, project: project, users: [create(:user)], approvals_required: 1)
         protected_branch = create(:protected_branch, project: project)
         create(:approval_project_rule, protected_branches: [protected_branch], users: [create(:user)], approvals_required: 2, project: project)
-        create(:suggestion, note: create(:note, project: project))
         create(:code_owner_rule, merge_request: merge_request, approvals_required: 3)
         create(:code_owner_rule, merge_request: merge_request, approvals_required: 7, section: 'new_section')
         create(:approval_merge_request_rule, merge_request: merge_request)
@@ -337,7 +336,6 @@ RSpec.describe Gitlab::UsageData do
         projects_imported_from_github: 2,
         projects_with_repositories_enabled: 26,
         protected_branches: 2,
-        suggestions: 2,
         users_using_lfs_locks: 12,
         users_using_path_locks: 16,
         total_number_of_path_locks: 20,
@@ -357,7 +355,6 @@ RSpec.describe Gitlab::UsageData do
         projects_imported_from_github: 1,
         projects_with_repositories_enabled: 13,
         protected_branches: 1,
-        suggestions: 1,
         users_using_lfs_locks: 6,
         users_using_path_locks: 8,
         total_number_of_path_locks: 10,

ee/spec/models/note_spec.rb

@@ -91,18 +91,6 @@ RSpec.describe Note do
     end
   end
 
-  describe 'scopes' do
-    describe '.with_suggestions' do
-      it 'returns the correct note' do
-        note_with_suggestion = create(:note, suggestions: [create(:suggestion)])
-        note_without_suggestion = create(:note)
-
-        expect(described_class.with_suggestions).to include(note_with_suggestion)
-        expect(described_class.with_suggestions).not_to include(note_without_suggestion)
-      end
-    end
-  end
-
   context 'object storage with background upload' do
     before do
       stub_uploads_object_storage(AttachmentUploader, background_upload: true)

lib/gitlab/usage_data.rb

@@ -574,7 +574,11 @@ module Gitlab
           projects_with_disable_overriding_approvers_per_merge_request: count(::Project.where(time_period.merge(disable_overriding_approvers_per_merge_request: true))),
           projects_without_disable_overriding_approvers_per_merge_request: count(::Project.where(time_period.merge(disable_overriding_approvers_per_merge_request: [false, nil]))),
           remote_mirrors: distinct_count(::Project.with_remote_mirrors.where(time_period), :creator_id),
-          snippets: distinct_count(::Snippet.where(time_period), :author_id)
+          snippets: distinct_count(::Snippet.where(time_period), :author_id),
+          suggestions: distinct_count(::Note.with_suggestions.where(time_period),
+                                      :author_id,
+                                      start: minimum_id(::User),
+                                      finish: maximum_id(::User))
         }.tap do |h|
           if time_period.present?
             h[:merge_requests_users] = merge_requests_users(time_period)

spec/lib/gitlab/usage_data_spec.rb

@@ -172,6 +172,7 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do
         another_project = create(:project, :repository, creator: another_user)
         create(:remote_mirror, project: another_project, enabled: false)
         create(:snippet, author: user)
+        create(:suggestion, note: create(:note, project: project))
       end
 
       expect(described_class.usage_activity_by_stage_create({})).to include(
@@ -181,7 +182,8 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do
         projects_with_disable_overriding_approvers_per_merge_request: 2,
         projects_without_disable_overriding_approvers_per_merge_request: 6,
         remote_mirrors: 2,
-        snippets: 2
+        snippets: 2,
+        suggestions: 2
       )
       expect(described_class.usage_activity_by_stage_create(described_class.last_28_days_time_period)).to include(
         deploy_keys: 1,
@@ -190,7 +192,8 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do
         projects_with_disable_overriding_approvers_per_merge_request: 1,
         projects_without_disable_overriding_approvers_per_merge_request: 3,
         remote_mirrors: 1,
-        snippets: 1
+        snippets: 1,
+        suggestions: 1
       )
     end
   end

spec/models/note_spec.rb

@@ -1384,6 +1384,16 @@ RSpec.describe Note do
         expect(notes.second.id).to eq(note2.id)
       end
     end
+
+    describe '.with_suggestions' do
+      it 'returns the correct note' do
+        note_with_suggestion = create(:note, suggestions: [create(:suggestion)])
+        note_without_suggestion = create(:note)
+
+        expect(described_class.with_suggestions).to include(note_with_suggestion)
+        expect(described_class.with_suggestions).not_to include(note_without_suggestion)
+      end
+    end
   end
 
   describe '#noteable_assignee_or_author?' do