Merge branch '292825-track-approver-metrics-for-a-merge-request' into 'master'

Track approver metrics for a merge request

See merge request gitlab-org/gitlab!53201

app/services/merge_requests/approval_service.rb

@@ -14,6 +14,7 @@ module MergeRequests
       create_approval_note(merge_request)
       mark_pending_todos_as_done(merge_request)
       execute_approval_hooks(merge_request, current_user)
+      merge_request_activity_counter.track_approve_mr_action(user: current_user)
 
       success
     end

app/services/merge_requests/remove_approval_service.rb

@@ -16,6 +16,7 @@ module MergeRequests
 
         reset_approvals_cache(merge_request)
         create_note(merge_request)
+        merge_request_activity_counter.track_unapprove_mr_action(user: current_user)
       end
 
       success

changelogs/unreleased/292825-track-approver-metrics-for-a-merge-request.yml

+---
+title: Add metrics for merge request approvals and revoking approvals
+merge_request: 53201
+author:
+type: added

config/feature_flags/development/usage_data_i_code_review_user_approve_mr.yml

+---
+name: usage_data_i_code_review_user_approve_mr
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/53201
+rollout_issue_url:
+milestone: '13.9'
+type: development
+group: group::code review
+default_enabled: true

config/feature_flags/development/usage_data_i_code_review_user_unapprove_mr.yml

+---
+name: usage_data_i_code_review_user_unapprove_mr
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/53201
+rollout_issue_url:
+milestone: '13.9'
+type: development
+group: group::code review
+default_enabled: true

lib/gitlab/usage_data_counters/known_events/common.yml

@@ -486,6 +486,16 @@
   category: code_review
   aggregation: weekly
   feature_flag: usage_data_i_code_review_user_reopen_mr
+- name: i_code_review_user_approve_mr
+  redis_slot: code_review
+  category: code_review
+  aggregation: weekly
+  feature_flag: usage_data_i_code_review_user_approve_mr
+- name: i_code_review_user_unapprove_mr
+  redis_slot: code_review
+  category: code_review
+  aggregation: weekly
+  feature_flag: usage_data_i_code_review_user_unapprove_mr
 - name: i_code_review_user_resolve_thread
   redis_slot: code_review
   category: code_review

lib/gitlab/usage_data_counters/merge_request_activity_unique_counter.rb

@@ -10,6 +10,8 @@ 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_APPROVE_ACTION = 'i_code_review_user_approve_mr'
+      MR_UNAPPROVE_ACTION = 'i_code_review_user_unapprove_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'
@@ -53,6 +55,14 @@ module Gitlab
           track_unique_action_by_user(MR_REOPEN_ACTION, user)
         end
 
+        def track_approve_mr_action(user:)
+          track_unique_action_by_user(MR_APPROVE_ACTION, user)
+        end
+
+        def track_unapprove_mr_action(user:)
+          track_unique_action_by_user(MR_UNAPPROVE_ACTION, user)
+        end
+
         def track_resolve_thread_action(user:)
           track_unique_action_by_user(MR_RESOLVE_THREAD_ACTION, user)
         end

spec/lib/gitlab/usage_data_counters/merge_request_activity_unique_counter_spec.rb

@@ -73,6 +73,22 @@ RSpec.describe Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter, :cl
     end
   end
 
+  describe '.track_approve_mr_action' do
+    subject { described_class.track_approve_mr_action(user: user) }
+
+    it_behaves_like 'a tracked merge request unique event' do
+      let(:action) { described_class::MR_APPROVE_ACTION }
+    end
+  end
+
+  describe '.track_unapprove_mr_action' do
+    subject { described_class.track_unapprove_mr_action(user: user) }
+
+    it_behaves_like 'a tracked merge request unique event' do
+      let(:action) { described_class::MR_UNAPPROVE_ACTION }
+    end
+  end
+
   describe '.track_resolve_thread_action' do
     subject { described_class.track_resolve_thread_action(user: user) }
 

spec/services/merge_requests/approval_service_spec.rb

@@ -31,6 +31,13 @@ RSpec.describe MergeRequests::ApprovalService do
 
         expect(todo.reload).to be_pending
       end
+
+      it 'does not track merge request approve action' do
+        expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
+          .not_to receive(:track_approve_mr_action).with(user: user)
+
+        service.execute(merge_request)
+      end
     end
 
     context 'with valid approval' do
@@ -59,6 +66,13 @@ RSpec.describe MergeRequests::ApprovalService do
           service.execute(merge_request)
         end
       end
+
+      it 'tracks merge request approve action' do
+        expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
+          .to receive(:track_approve_mr_action).with(user: user)
+
+        service.execute(merge_request)
+      end
     end
 
     context 'user cannot update the merge request' do

spec/services/merge_requests/remove_approval_service_spec.rb

@@ -32,6 +32,13 @@ RSpec.describe MergeRequests::RemoveApprovalService do
 
         execute!
       end
+
+      it 'tracks merge request unapprove action' do
+        expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
+          .to receive(:track_unapprove_mr_action).with(user: user)
+
+        execute!
+      end
     end
 
     context 'with a user who has not approved' do
@@ -41,6 +48,13 @@ RSpec.describe MergeRequests::RemoveApprovalService do
 
         execute!
       end
+
+      it 'does not track merge request unapprove action' do
+        expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
+          .not_to receive(:track_unapprove_mr_action).with(user: user)
+
+        execute!
+      end
     end
   end
 end