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

Resolve "Track comment related metrics for a merge request"

See merge request gitlab-org/gitlab!51098

app/models/diff_note.rb

@@ -111,6 +111,10 @@ class DiffNote < Note
     super.merge(suggestions_filter_enabled: true)
   end
 
+  def multiline?
+    position&.multiline?
+  end
+
   private
 
   def enqueue_diff_file_creation_job

app/services/notes/create_service.rb

@@ -122,7 +122,7 @@ module Notes
     end
 
     def track_note_creation_usage_for_merge_requests(note)
-      Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter.track_create_comment_action(user: note.author)
+      Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter.track_create_comment_action(note: note)
     end
   end
 end

app/services/notes/destroy_service.rb

@@ -19,7 +19,7 @@ module Notes
     end
 
     def track_note_removal_usage_for_merge_requests(note)
-      Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter.track_remove_comment_action(user: note.author)
+      Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter.track_remove_comment_action(note: note)
     end
   end
 end

app/services/notes/update_service.rb

@@ -98,7 +98,7 @@ module Notes
     end
 
     def track_note_edit_usage_for_merge_requests(note)
-      Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter.track_edit_comment_action(user: note.author)
+      Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter.track_edit_comment_action(note: note)
     end
   end
 end

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

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

config/feature_flags/development/usage_data_i_code_review_user_create_multiline_mr_comment.yml

+---
+name: usage_data_i_code_review_user_create_multiline_mr_comment
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/51098
+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_multiline_mr_comment.yml

+---
+name: usage_data_i_code_review_user_edit_multiline_mr_comment
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/51098
+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_multiline_mr_comment.yml

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

lib/gitlab/diff/position.rb

@@ -19,6 +19,7 @@ module Gitlab
                :height,
                :x,
                :y,
+               :line_range,
                :position_type, to: :formatter
 
       # A position can belong to a text line or to an image coordinate
@@ -167,6 +168,12 @@ module Gitlab
         end
       end
 
+      def multiline?
+        return unless on_text? && line_range
+
+        line_range['start'] != line_range['end']
+      end
+
       private
 
       def find_diff_file(repository)

lib/gitlab/usage_data_counters/known_events/common.yml

@@ -496,6 +496,21 @@
   category: code_review
   aggregation: weekly
   feature_flag: usage_data_i_code_review_user_publish_review
+- name: i_code_review_user_create_multiline_mr_comment
+  redis_slot: code_review
+  category: code_review
+  aggregation: weekly
+  feature_flag: usage_data_i_code_review_user_create_multiline_mr_comment
+- name: i_code_review_user_edit_multiline_mr_comment
+  redis_slot: code_review
+  category: code_review
+  aggregation: weekly
+  feature_flag: usage_data_i_code_review_user_edit_multiline_mr_comment
+- name: i_code_review_user_remove_multiline_mr_comment
+  redis_slot: code_review
+  category: code_review
+  aggregation: weekly
+  feature_flag: usage_data_i_code_review_user_remove_multiline_mr_comment
 # Terraform
 - name: p_terraform_state_api_unique_users
   category: terraform

lib/gitlab/usage_data_counters/merge_request_activity_unique_counter.rb

@@ -15,6 +15,9 @@ module Gitlab
       MR_REMOVE_COMMENT_ACTION = 'i_code_review_user_remove_mr_comment'
       MR_CREATE_REVIEW_NOTE_ACTION = 'i_code_review_user_create_review_note'
       MR_PUBLISH_REVIEW_ACTION = 'i_code_review_user_publish_review'
+      MR_CREATE_MULTILINE_COMMENT_ACTION = 'i_code_review_user_create_multiline_mr_comment'
+      MR_EDIT_MULTILINE_COMMENT_ACTION = 'i_code_review_user_edit_multiline_mr_comment'
+      MR_REMOVE_MULTILINE_COMMENT_ACTION = 'i_code_review_user_remove_multiline_mr_comment'
 
       class << self
         def track_mr_diffs_action(merge_request:)
@@ -42,16 +45,19 @@ 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)
+        def track_create_comment_action(note:)
+          track_unique_action_by_user(MR_CREATE_COMMENT_ACTION, note.author)
+          track_multiline_unique_action(MR_CREATE_MULTILINE_COMMENT_ACTION, note)
         end
 
-        def track_edit_comment_action(user:)
-          track_unique_action_by_user(MR_EDIT_COMMENT_ACTION, user)
+        def track_edit_comment_action(note:)
+          track_unique_action_by_user(MR_EDIT_COMMENT_ACTION, note.author)
+          track_multiline_unique_action(MR_EDIT_MULTILINE_COMMENT_ACTION, note)
         end
 
-        def track_remove_comment_action(user:)
-          track_unique_action_by_user(MR_REMOVE_COMMENT_ACTION, user)
+        def track_remove_comment_action(note:)
+          track_unique_action_by_user(MR_REMOVE_COMMENT_ACTION, note.author)
+          track_multiline_unique_action(MR_REMOVE_MULTILINE_COMMENT_ACTION, note)
         end
 
         def track_create_review_note_action(user:)
@@ -77,6 +83,12 @@ module Gitlab
         def track_unique_action(action, value)
           Gitlab::UsageDataCounters::HLLRedisCounter.track_usage_event(action, value)
         end
+
+        def track_multiline_unique_action(action, note)
+          return unless note.is_a?(DiffNote) && note.multiline?
+
+          track_unique_action_by_user(action, note.author)
+        end
       end
     end
   end

spec/lib/gitlab/diff/position_spec.rb

@@ -752,4 +752,62 @@ RSpec.describe Gitlab::Diff::Position do
       expect(subject.file_hash).to eq(Digest::SHA1.hexdigest(subject.file_path))
     end
   end
+
+  describe '#multiline?' do
+    let(:end_line_code) { "ab09011fa121d0a2bb9fa4ca76094f2482b902b7_#{end_old_line}_#{end_new_line}" }
+
+    let(:line_range) do
+      {
+        "start" => {
+          "line_code" => "ab09011fa121d0a2bb9fa4ca76094f2482b902b7_18_18",
+          "type" => nil,
+          "old_line" => 18,
+          "new_line" => 18
+        },
+         "end" => {
+          "line_code" => end_line_code,
+          "type" => nil,
+          "old_line" => end_old_line,
+          "new_line" => end_new_line
+        }
+      }
+    end
+
+    subject(:multiline) do
+      described_class.new(
+        line_range: line_range,
+        position_type: position_type
+      )
+    end
+
+    let(:end_old_line) { 20 }
+    let(:end_new_line) { 20 }
+
+    context 'when the position type is text' do
+      let(:position_type) { "text" }
+
+      context 'when the start lines equal the end lines' do
+        let(:end_old_line) { 18 }
+        let(:end_new_line) { 18 }
+
+        it "returns true" do
+          expect(subject.multiline?).to be_falsey
+        end
+      end
+
+      context 'when the start lines do not equal the end lines' do
+        it "returns true" do
+          expect(subject.multiline?).to be_truthy
+        end
+      end
+    end
+
+    context 'when the position type is not text' do
+      let(:position_type) { "image" }
+
+      it "returns false" do
+        expect(subject.multiline?).to be_falsey
+      end
+    end
+  end
 end

spec/lib/gitlab/usage_data_counters/merge_request_activity_unique_counter_spec.rb

@@ -5,6 +5,7 @@ require 'spec_helper'
 RSpec.describe Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter, :clean_gitlab_redis_shared_state do
   let(:merge_request) { build(:merge_request, id: 1) }
   let(:user) { build(:user, id: 1) }
+  let(:note) { build(:note, author: user) }
 
   shared_examples_for 'a tracked merge request unique event' do
     specify do
@@ -73,27 +74,63 @@ RSpec.describe Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter, :cl
   end
 
   describe '.track_create_comment_action' do
-    subject { described_class.track_create_comment_action(user: user) }
+    subject { described_class.track_create_comment_action(note: note) }
 
     it_behaves_like 'a tracked merge request unique event' do
       let(:action) { described_class::MR_CREATE_COMMENT_ACTION }
     end
+
+    context 'when the note is multiline diff note' do
+      let(:note) { build(:diff_note_on_merge_request, author: user) }
+
+      before do
+        allow(note).to receive(:multiline?).and_return(true)
+      end
+
+      it_behaves_like 'a tracked merge request unique event' do
+        let(:action) { described_class::MR_CREATE_MULTILINE_COMMENT_ACTION }
+      end
+    end
   end
 
   describe '.track_edit_comment_action' do
-    subject { described_class.track_edit_comment_action(user: user) }
+    subject { described_class.track_edit_comment_action(note: note) }
 
     it_behaves_like 'a tracked merge request unique event' do
       let(:action) { described_class::MR_EDIT_COMMENT_ACTION }
     end
+
+    context 'when the note is multiline diff note' do
+      let(:note) { build(:diff_note_on_merge_request, author: user) }
+
+      before do
+        allow(note).to receive(:multiline?).and_return(true)
+      end
+
+      it_behaves_like 'a tracked merge request unique event' do
+        let(:action) { described_class::MR_EDIT_MULTILINE_COMMENT_ACTION }
+      end
+    end
   end
 
   describe '.track_remove_comment_action' do
-    subject { described_class.track_remove_comment_action(user: user) }
+    subject { described_class.track_remove_comment_action(note: note) }
 
     it_behaves_like 'a tracked merge request unique event' do
       let(:action) { described_class::MR_REMOVE_COMMENT_ACTION }
     end
+
+    context 'when the note is multiline diff note' do
+      let(:note) { build(:diff_note_on_merge_request, author: user) }
+
+      before do
+        allow(note).to receive(:multiline?).and_return(true)
+      end
+
+      it_behaves_like 'a tracked merge request unique event' do
+        let(:action) { described_class::MR_REMOVE_MULTILINE_COMMENT_ACTION }
+      end
+    end
   end
 
   describe '.track_create_review_note_action' do

spec/services/notes/create_service_spec.rb

@@ -120,7 +120,7 @@ RSpec.describe Notes::CreateService do
           end
 
           it 'tracks merge request usage data' do
-            expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter).to receive(:track_create_comment_action).with(user: user)
+            expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter).to receive(:track_create_comment_action).with(note: kind_of(Note))
 
             described_class.new(project_with_repo, user, new_opts).execute
           end

spec/services/notes/destroy_service_spec.rb

@@ -38,7 +38,7 @@ RSpec.describe Notes::DestroyService do
     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)
+      expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter).to receive(:track_remove_comment_action).with(note: note)
 
       described_class.new(project, user).execute(note)
     end

spec/services/notes/update_service_spec.rb

@@ -69,7 +69,7 @@ RSpec.describe Notes::UpdateService do
       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)
+        expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter).to receive(:track_edit_comment_action).with(note: note)
 
         update_note(note: 'new text')
       end