Merge branch '292824-track-mr-lock-changes' into 'master'

Track usage pings when MR gets locked/unlocked [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!55069

app/services/merge_requests/update_service.rb

@@ -47,6 +47,7 @@ module MergeRequests
       handle_draft_status_change(merge_request, changed_fields)
 
       track_title_and_desc_edits(changed_fields)
+      track_discussion_lock_toggle(merge_request, changed_fields)
 
       notify_if_labels_added(merge_request, old_labels)
       notify_if_mentions_added(merge_request, old_mentioned_users)
@@ -95,6 +96,16 @@ module MergeRequests
       end
     end
 
+    def track_discussion_lock_toggle(merge_request, changed_fields)
+      return unless changed_fields.include?('discussion_locked')
+
+      if merge_request.discussion_locked
+        merge_request_activity_counter.track_discussion_locked_action(user: current_user)
+      else
+        merge_request_activity_counter.track_discussion_unlocked_action(user: current_user)
+      end
+    end
+
     def notify_if_labels_added(merge_request, old_labels)
       added_labels = merge_request.labels - old_labels
 

changelogs/unreleased/292824-track-mr-lock-changes.yml

+---
+title: Track usage pings when MR gets locked/unlocked
+merge_request: 55069
+author:
+type: other

config/feature_flags/development/usage_data_i_code_review_user_mr_discussion_locked.yml

+---
+name: usage_data_i_code_review_user_mr_discussion_locked
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/55069
+rollout_issue_url:
+milestone: '13.10'
+type: development
+group: group::code review
+default_enabled: true

config/feature_flags/development/usage_data_i_code_review_user_mr_discussion_unlocked.yml

+---
+name: usage_data_i_code_review_user_mr_discussion_unlocked
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/55069
+rollout_issue_url:
+milestone: '13.10'
+type: development
+group: group::code review
+default_enabled: true

config/metrics/counts_28d/20210301103859_i_code_review_user_mr_discussion_locked_monthly.yml

+---
+key_path: redis_hll_counters.code_review.i_code_review_user_mr_discussion_locked_monthly
+description: Count of unique users per month who locked a MR
+product_section: dev
+product_stage: create
+product_group: group::code review
+product_category: code_review
+value_type: number
+status: implemented
+milestone: "13.10"
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/55069
+time_frame: 28d
+data_source: redis_hll
+distribution:
+- ce
+- ee
+tier:
+- free
+- premium
+- ultimate

config/metrics/counts_28d/20210301103925_i_code_review_user_mr_discussion_unlocked_monthly.yml

+---
+key_path: redis_hll_counters.code_review.i_code_review_user_mr_discussion_unlocked_monthly
+description: Count of unique users per month who unlocked a MR
+product_section: dev
+product_stage: create
+product_group: group::code review
+product_category: code_review
+value_type: number
+status: implemented
+milestone: "13.10"
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/55069
+time_frame: 28d
+data_source: redis_hll
+distribution:
+- ce
+- ee
+tier:
+- free
+- premium
+- ultimate

config/metrics/counts_7d/20210302105258_i_code_review_user_mr_discussion_unlocked_weekly.yml

+---
+key_path: redis_hll_counters.code_review.i_code_review_user_mr_discussion_unlocked_weekly
+description: Count of unique users per week who unlocked a MR
+product_section: dev
+product_stage: create
+product_group: group::code review
+product_category: code_review
+value_type: number
+status: implemented
+milestone: "13.10"
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/55069
+time_frame: 7d
+data_source: redis_hll
+distribution:
+- ce
+- ee
+tier:
+- free
+- premium
+- ultimate

config/metrics/counts_7d/20210302105318_i_code_review_user_mr_discussion_locked_weekly.yml

+---
+key_path: redis_hll_counters.code_review.i_code_review_user_mr_discussion_locked_weekly
+description: Count of unique users per week who locked a MR
+product_section: dev
+product_stage: create
+product_group: group::code review
+product_category: code_review
+value_type: number
+status: implemented
+milestone: "13.10"
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/55069
+time_frame: 7d
+data_source: redis_hll
+distribution:
+- ce
+- ee
+tier:
+- free
+- premium
+- ultimate

doc/development/usage_ping/dictionary.md

@@ -13384,6 +13384,86 @@ Count of unique users per week|month who merged a MR
 | `tier` |  |
 | `skip_validation` | true |
 
+## `redis_hll_counters.code_review.i_code_review_user_mr_discussion_locked_monthly`
+
+Count of unique users per month who locked a MR
+
+| field | value |
+| --- | --- |
+| `key_path` | **`redis_hll_counters.code_review.i_code_review_user_mr_discussion_locked_monthly`** |
+| `product_section` | dev |
+| `product_stage` | create |
+| `product_group` | `group::code review` |
+| `product_category` | `code_review` |
+| `value_type` | number |
+| `status` | implemented |
+| `milestone` | 13.10 |
+| `introduced_by_url` | [Introduced by](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/55069) |
+| `time_frame` | 28d |
+| `data_source` | Redis_hll |
+| `distribution` | ce, ee |
+| `tier` | free, premium, ultimate |
+
+## `redis_hll_counters.code_review.i_code_review_user_mr_discussion_locked_weekly`
+
+Count of unique users per week who locked a MR
+
+| field | value |
+| --- | --- |
+| `key_path` | **`redis_hll_counters.code_review.i_code_review_user_mr_discussion_locked_weekly`** |
+| `product_section` | dev |
+| `product_stage` | create |
+| `product_group` | `group::code review` |
+| `product_category` | `code_review` |
+| `value_type` | number |
+| `status` | implemented |
+| `milestone` | 13.10 |
+| `introduced_by_url` | [Introduced by](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/55069) |
+| `time_frame` | 7d |
+| `data_source` | Redis_hll |
+| `distribution` | ce, ee |
+| `tier` | free, premium, ultimate |
+
+## `redis_hll_counters.code_review.i_code_review_user_mr_discussion_unlocked_monthly`
+
+Count of unique users per month who unlocked a MR
+
+| field | value |
+| --- | --- |
+| `key_path` | **`redis_hll_counters.code_review.i_code_review_user_mr_discussion_unlocked_monthly`** |
+| `product_section` | dev |
+| `product_stage` | create |
+| `product_group` | `group::code review` |
+| `product_category` | `code_review` |
+| `value_type` | number |
+| `status` | implemented |
+| `milestone` | 13.10 |
+| `introduced_by_url` | [Introduced by](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/55069) |
+| `time_frame` | 28d |
+| `data_source` | Redis_hll |
+| `distribution` | ce, ee |
+| `tier` | free, premium, ultimate |
+
+## `redis_hll_counters.code_review.i_code_review_user_mr_discussion_unlocked_weekly`
+
+Count of unique users per week who unlocked a MR
+
+| field | value |
+| --- | --- |
+| `key_path` | **`redis_hll_counters.code_review.i_code_review_user_mr_discussion_unlocked_weekly`** |
+| `product_section` | dev |
+| `product_stage` | create |
+| `product_group` | `group::code review` |
+| `product_category` | `code_review` |
+| `value_type` | number |
+| `status` | implemented |
+| `milestone` | 13.10 |
+| `introduced_by_url` | [Introduced by](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/55069) |
+| `time_frame` | 7d |
+| `data_source` | Redis_hll |
+| `distribution` | ce, ee |
+| `tier` | free, premium, ultimate |
+
 ## `redis_hll_counters.code_review.i_code_review_user_publish_review_monthly`
 
 Missing description

lib/gitlab/usage_data_counters/aggregated_metrics/code_review.yml

@@ -42,7 +42,9 @@
     'i_code_review_user_approval_rule_edited',
     'i_code_review_user_vs_code_api_request',
     'i_code_review_user_toggled_task_item_status',
-    'i_code_review_user_create_mr_from_issue'
+    'i_code_review_user_create_mr_from_issue',
+    'i_code_review_user_mr_discussion_locked',
+    'i_code_review_user_mr_discussion_unlocked'
   ]
 - name: code_review_category_monthly_active_users
   operator: OR
@@ -78,7 +80,9 @@
     'i_code_review_user_approval_rule_deleted',
     'i_code_review_user_approval_rule_edited',
     'i_code_review_user_toggled_task_item_status',
-    'i_code_review_user_create_mr_from_issue'
+    'i_code_review_user_create_mr_from_issue',
+    'i_code_review_user_mr_discussion_locked',
+    'i_code_review_user_mr_discussion_unlocked'
   ]
 - name: code_review_extension_category_monthly_active_users
   operator: OR

lib/gitlab/usage_data_counters/known_events/code_review_events.yml

@@ -164,3 +164,13 @@
   category: code_review
   aggregation: weekly
   feature_flag: usage_data_i_code_review_user_create_mr_from_issue
+- name: i_code_review_user_mr_discussion_locked
+  redis_slot: code_review
+  category: code_review
+  aggregation: weekly
+  feature_flag: usage_data_i_code_review_user_mr_discussion_locked
+- name: i_code_review_user_mr_discussion_unlocked
+  redis_slot: code_review
+  category: code_review
+  aggregation: weekly
+  feature_flag: usage_data_i_code_review_user_mr_discussion_unlocked

lib/gitlab/usage_data_counters/merge_request_activity_unique_counter.rb

@@ -35,6 +35,8 @@ module Gitlab
       MR_EDIT_MR_TITLE_ACTION = 'i_code_review_edit_mr_title'
       MR_EDIT_MR_DESC_ACTION = 'i_code_review_edit_mr_desc'
       MR_CREATE_FROM_ISSUE_ACTION = 'i_code_review_user_create_mr_from_issue'
+      MR_DISCUSSION_LOCKED_ACTION = 'i_code_review_user_mr_discussion_locked'
+      MR_DISCUSSION_UNLOCKED_ACTION = 'i_code_review_user_mr_discussion_unlocked'
 
       class << self
         def track_mr_diffs_action(merge_request:)
@@ -153,6 +155,14 @@ module Gitlab
           track_unique_action_by_user(MR_CREATE_FROM_ISSUE_ACTION, user)
         end
 
+        def track_discussion_locked_action(user:)
+          track_unique_action_by_user(MR_DISCUSSION_LOCKED_ACTION, user)
+        end
+
+        def track_discussion_unlocked_action(user:)
+          track_unique_action_by_user(MR_DISCUSSION_UNLOCKED_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

@@ -284,4 +284,20 @@ RSpec.describe Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter, :cl
       let(:action) { described_class::MR_CREATE_FROM_ISSUE_ACTION }
     end
   end
+
+  describe '.track_discussion_locked_action' do
+    subject { described_class.track_discussion_locked_action(user: user) }
+
+    it_behaves_like 'a tracked merge request unique event' do
+      let(:action) { described_class::MR_DISCUSSION_LOCKED_ACTION }
+    end
+  end
+
+  describe '.track_discussion_unlocked_action' do
+    subject { described_class.track_discussion_unlocked_action(user: user) }
+
+    it_behaves_like 'a tracked merge request unique event' do
+      let(:action) { described_class::MR_DISCUSSION_UNLOCKED_ACTION }
+    end
+  end
 end

spec/services/merge_requests/update_service_spec.rb

@@ -48,6 +48,8 @@ RSpec.describe MergeRequests::UpdateService, :mailer do
     end
 
     context 'valid params' do
+      let(:locked) { true }
+
       let(:opts) do
         {
           title: 'New title',
@@ -58,7 +60,7 @@ RSpec.describe MergeRequests::UpdateService, :mailer do
           label_ids: [label.id],
           target_branch: 'target',
           force_remove_source_branch: '1',
-          discussion_locked: true
+          discussion_locked: locked
         }
       end
 
@@ -117,6 +119,56 @@ RSpec.describe MergeRequests::UpdateService, :mailer do
 
           MergeRequests::UpdateService.new(project, user, opts).execute(draft_merge_request)
         end
+
+        context 'when MR is locked' do
+          context 'when locked again' do
+            it 'does not track discussion locking' do
+              expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
+                .not_to receive(:track_discussion_locked_action)
+
+              opts[:discussion_locked] = true
+
+              MergeRequests::UpdateService.new(project, user, opts).execute(merge_request)
+            end
+          end
+
+          context 'when unlocked' do
+            it 'tracks dicussion unlocking' do
+              expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
+                .to receive(:track_discussion_unlocked_action).once.with(user: user)
+
+              opts[:discussion_locked] = false
+
+              MergeRequests::UpdateService.new(project, user, opts).execute(merge_request)
+            end
+          end
+        end
+
+        context 'when MR is unlocked' do
+          let(:locked) { false }
+
+          context 'when unlocked again' do
+            it 'does not track discussion unlocking' do
+              expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
+                .not_to receive(:track_discussion_unlocked_action)
+
+              opts[:discussion_locked] = false
+
+              MergeRequests::UpdateService.new(project, user, opts).execute(merge_request)
+            end
+          end
+
+          context 'when locked' do
+            it 'tracks dicussion locking' do
+              expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
+                .to receive(:track_discussion_locked_action).once.with(user: user)
+
+              opts[:discussion_locked] = true
+
+              MergeRequests::UpdateService.new(project, user, opts).execute(merge_request)
+            end
+          end
+        end
       end
 
       context 'updating milestone' do