Merge branch '292822-track-comment-related-metrics-for-a-merge-request' into 'master'

Resolve "Track comment related metrics for a merge request"

See merge request gitlab-org/gitlab!50849

app/services/notes/create_service.rb

@@ -75,6 +75,7 @@ module Notes
       end
 
       track_note_creation_usage_for_issues(note) if note.for_issue?
+      track_note_creation_usage_for_merge_requests(note) if note.for_merge_request?
     end
 
     def do_commands(note, update_params, message, only_commands)
@@ -119,5 +120,9 @@ module Notes
     def track_note_creation_usage_for_issues(note)
       Gitlab::UsageDataCounters::IssueActivityUniqueCounter.track_issue_comment_added_action(author: note.author)
     end
+
+    def track_note_creation_usage_for_merge_requests(note)
+      Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter.track_create_comment_action(user: note.author)
+    end
   end
 end

app/services/notes/destroy_service.rb

@@ -9,6 +9,7 @@ module Notes
 
       clear_noteable_diffs_cache(note)
       track_note_removal_usage_for_issues(note) if note.for_issue?
+      track_note_removal_usage_for_merge_requests(note) if note.for_merge_request?
     end
 
     private
@@ -16,6 +17,10 @@ module Notes
     def track_note_removal_usage_for_issues(note)
       Gitlab::UsageDataCounters::IssueActivityUniqueCounter.track_issue_comment_removed_action(author: note.author)
     end
+
+    def track_note_removal_usage_for_merge_requests(note)
+      Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter.track_remove_comment_action(user: note.author)
+    end
   end
 end
 

app/services/notes/update_service.rb

@@ -15,6 +15,7 @@ module Notes
       end
 
       track_note_edit_usage_for_issues(note) if note.for_issue?
+      track_note_edit_usage_for_merge_requests(note) if note.for_merge_request?
 
       only_commands = false
 
@@ -95,6 +96,10 @@ module Notes
     def track_note_edit_usage_for_issues(note)
       Gitlab::UsageDataCounters::IssueActivityUniqueCounter.track_issue_comment_edited_action(author: note.author)
     end
+
+    def track_note_edit_usage_for_merge_requests(note)
+      Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter.track_edit_comment_action(user: note.author)
+    end
   end
 end
 

changelogs/unreleased/292822-track-comment-related-metrics-for-a-merge-request.yml

+---
+title: Add metrics to creating, editing or removing comments on merge requests
+merge_request: 50849
+author:
+type: other

config/feature_flags/development/usage_data_i_code_review_user_create_mr_comment.yml

+---
+name: usage_data_i_code_review_user_create_mr_comment
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/50849
+rollout_issue_url: 
+milestone: '13.8'
+type: development
+group: group::code review
+default_enabled: true

config/feature_flags/development/usage_data_i_code_review_user_edit_mr_comment.yml

+---
+name: usage_data_i_code_review_user_edit_mr_comment
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/50849
+rollout_issue_url: 
+milestone: '13.8'
+type: development
+group: group::code review
+default_enabled: true

config/feature_flags/development/usage_data_i_code_review_user_remove_mr_comment.yml

+---
+name: usage_data_i_code_review_user_remove_mr_comment
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/50849
+rollout_issue_url: 
+milestone: '13.8'
+type: development
+group: group::code review
+default_enabled: true

lib/gitlab/usage_data_counters/known_events/common.yml

@@ -466,6 +466,21 @@
   category: code_review
   aggregation: weekly
   feature_flag: usage_data_i_code_review_user_merge_mr
+- name: i_code_review_user_create_mr_comment
+  redis_slot: code_review
+  category: code_review
+  aggregation: weekly
+  feature_flag: usage_data_i_code_review_user_create_mr_comment
+- name: i_code_review_user_edit_mr_comment
+  redis_slot: code_review
+  category: code_review
+  aggregation: weekly
+  feature_flag: usage_data_i_code_review_user_edit_mr_comment
+- name: i_code_review_user_remove_mr_comment
+  redis_slot: code_review
+  category: code_review
+  aggregation: weekly
+  feature_flag: usage_data_i_code_review_user_remove_mr_comment
 # Terraform
 - name: p_terraform_state_api_unique_users
   category: terraform

lib/gitlab/usage_data_counters/merge_request_activity_unique_counter.rb

@@ -10,6 +10,9 @@ module Gitlab
       MR_CLOSE_ACTION = 'i_code_review_user_close_mr'
       MR_REOPEN_ACTION = 'i_code_review_user_reopen_mr'
       MR_MERGE_ACTION = 'i_code_review_user_merge_mr'
+      MR_CREATE_COMMENT_ACTION = 'i_code_review_user_create_mr_comment'
+      MR_EDIT_COMMENT_ACTION = 'i_code_review_user_edit_mr_comment'
+      MR_REMOVE_COMMENT_ACTION = 'i_code_review_user_remove_mr_comment'
 
       class << self
         def track_mr_diffs_action(merge_request:)
@@ -37,6 +40,18 @@ module Gitlab
           track_unique_action_by_user(MR_REOPEN_ACTION, user)
         end
 
+        def track_create_comment_action(user:)
+          track_unique_action_by_user(MR_CREATE_COMMENT_ACTION, user)
+        end
+
+        def track_edit_comment_action(user:)
+          track_unique_action_by_user(MR_EDIT_COMMENT_ACTION, user)
+        end
+
+        def track_remove_comment_action(user:)
+          track_unique_action_by_user(MR_REMOVE_COMMENT_ACTION, user)
+        end
+
         private
 
         def track_unique_action_by_merge_request(action, merge_request)

spec/lib/gitlab/usage_data_counters/merge_request_activity_unique_counter_spec.rb

@@ -71,4 +71,28 @@ RSpec.describe Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter, :cl
       let(:action) { described_class::MR_REOPEN_ACTION }
     end
   end
+
+  describe '.track_create_comment_action' do
+    subject { described_class.track_create_comment_action(user: user) }
+
+    it_behaves_like 'a tracked merge request unique event' do
+      let(:action) { described_class::MR_CREATE_COMMENT_ACTION }
+    end
+  end
+
+  describe '.track_edit_comment_action' do
+    subject { described_class.track_edit_comment_action(user: user) }
+
+    it_behaves_like 'a tracked merge request unique event' do
+      let(:action) { described_class::MR_EDIT_COMMENT_ACTION }
+    end
+  end
+
+  describe '.track_remove_comment_action' do
+    subject { described_class.track_remove_comment_action(user: user) }
+
+    it_behaves_like 'a tracked merge request unique event' do
+      let(:action) { described_class::MR_REMOVE_COMMENT_ACTION }
+    end
+  end
 end

spec/services/notes/create_service_spec.rb

@@ -78,6 +78,12 @@ RSpec.describe Notes::CreateService do
         end.to change { counter.unique_events(event_names: event, start_date: 1.day.ago, end_date: 1.day.from_now) }.by(1)
       end
 
+      it 'does not track merge request usage data' do
+        expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter).not_to receive(:track_create_comment_action)
+
+        described_class.new(project, user, opts).execute
+      end
+
       context 'in a merge request' do
         let_it_be(:project_with_repo) { create(:project, :repository) }
         let_it_be(:merge_request) do
@@ -85,18 +91,6 @@ RSpec.describe Notes::CreateService do
                  target_project: project_with_repo)
         end
 
-        context 'issue comment usage data' do
-          let(:opts) do
-            { note: 'Awesome comment', noteable_type: 'MergeRequest', noteable_id: merge_request.id }
-          end
-
-          it 'does not track' do
-            expect(Gitlab::UsageDataCounters::IssueActivityUniqueCounter).not_to receive(:track_issue_comment_added_action)
-
-            described_class.new(project, user, opts).execute
-          end
-        end
-
         context 'noteable highlight cache clearing' do
           let(:position) do
             Gitlab::Diff::Position.new(old_path: "files/ruby/popen.rb",
@@ -119,6 +113,18 @@ RSpec.describe Notes::CreateService do
               .to receive(:unfolded_diff?) { true }
           end
 
+          it 'does not track issue comment usage data' do
+            expect(Gitlab::UsageDataCounters::IssueActivityUniqueCounter).not_to receive(:track_issue_comment_added_action)
+
+            described_class.new(project_with_repo, user, new_opts).execute
+          end
+
+          it 'tracks merge request usage data' do
+            expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter).to receive(:track_create_comment_action).with(user: user)
+
+            described_class.new(project_with_repo, user, new_opts).execute
+          end
+
           it 'clears noteable diff cache when it was unfolded for the note position' do
             expect_any_instance_of(Gitlab::Diff::HighlightCache).to receive(:clear)
 

spec/services/notes/destroy_service_spec.rb

@@ -35,6 +35,14 @@ RSpec.describe Notes::DestroyService do
       end.to change { counter.unique_events(event_names: event, start_date: 1.day.ago, end_date: 1.day.from_now) }.by(1)
     end
 
+    it 'tracks merge request usage data' do
+      mr = create(:merge_request, source_project: project)
+      note = create(:note, project: project, noteable: mr)
+      expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter).to receive(:track_remove_comment_action).with(user: user)
+
+      described_class.new(project, user).execute(note)
+    end
+
     context 'in a merge request' do
       let_it_be(:repo_project) { create(:project, :repository) }
       let_it_be(:merge_request) do

spec/services/notes/update_service_spec.rb

@@ -49,11 +49,12 @@ RSpec.describe Notes::UpdateService do
 
     it 'does not track usage data when params is blank' do
       expect(Gitlab::UsageDataCounters::IssueActivityUniqueCounter).not_to receive(:track_issue_comment_edited_action)
+      expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter).not_to receive(:track_edit_comment_action)
 
       update_note({})
     end
 
-    it 'tracks usage data', :clean_gitlab_redis_shared_state do
+    it 'tracks issue usage data', :clean_gitlab_redis_shared_state do
       event = Gitlab::UsageDataCounters::IssueActivityUniqueCounter::ISSUE_COMMENT_EDITED
       counter = Gitlab::UsageDataCounters::HLLRedisCounter
 
@@ -63,6 +64,17 @@ RSpec.describe Notes::UpdateService do
       end.to change { counter.unique_events(event_names: event, start_date: 1.day.ago, end_date: 1.day.from_now) }.by(1)
     end
 
+    context 'when the notable is a merge request' do
+      let(:merge_request) { create(:merge_request, source_project: project) }
+      let(:note) { create(:note, project: project, noteable: merge_request, author: user, note: "Old note #{user2.to_reference}") }
+
+      it 'tracks merge request usage data' do
+        expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter).to receive(:track_edit_comment_action).with(user: user)
+
+        update_note(note: 'new text')
+      end
+    end
+
     context 'with system note' do
       before do
         note.update_column(:system, true)