Merge branch...

Merge branch '292833-track-rebase-and-resolve-conflicts-metrics-for-a-merge-request-2' into 'master'

Resolve "Track resolve conflicts metrics for a merge request"

See merge request gitlab-org/gitlab!61654

app/controllers/projects/merge_requests/conflicts_controller.rb

@@ -9,6 +9,7 @@ class Projects::MergeRequests::ConflictsController < Projects::MergeRequests::Ap
     respond_to do |format|
       format.html do
         @issuable_sidebar = serializer.represent(@merge_request, serializer: 'sidebar')
+        Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter.track_loading_conflict_ui_action(user: current_user)
       end
 
       format.json do
@@ -42,6 +43,8 @@ class Projects::MergeRequests::ConflictsController < Projects::MergeRequests::Ap
   def resolve_conflicts
     return render_404 unless @conflicts_list.can_be_resolved_in_ui?
 
+    Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter.track_resolve_conflict_action(user: current_user)
+
     if @merge_request.can_be_merged?
       render status: :bad_request, json: { message: _('The merge conflicts for this merge request have already been resolved.') }
       return

changelogs/unreleased/292833-track-rebase-and-resolve-conflicts-metrics-for-a-merge-request-2.yml

+---
+title: Track usage of the resolve conflict UI
+merge_request: 61654
+author:
+type: other

config/metrics/aggregates/code_review.yml

@@ -63,6 +63,8 @@
     - 'i_code_review_diff_hide_whitespace'
     - 'i_code_review_diff_single_file'
     - 'i_code_review_diff_multiple_files'
+    - 'i_code_review_user_load_conflict_ui'
+    - 'i_code_review_user_resolve_conflict'
 - name: code_review_category_monthly_active_users
   operator: OR
   feature_flag: usage_data_code_review_aggregation
@@ -118,6 +120,8 @@
     - 'i_code_review_diff_hide_whitespace'
     - 'i_code_review_diff_single_file'
     - 'i_code_review_diff_multiple_files'
+    - 'i_code_review_user_load_conflict_ui'
+    - 'i_code_review_user_resolve_conflict'
 - name: code_review_extension_category_monthly_active_users
   operator: OR
   feature_flag: usage_data_code_review_aggregation

config/metrics/counts_28d/20210514013545_i_code_review_user_resolve_conflict_monthly.yml

+---
+key_path: redis_hll_counters.code_review.i_code_review_user_resolve_conflict_monthly
+name: resolve_conflict
+description: Count of unique users per week who attempt to resolve a conflict through the ui
+product_section:
+product_stage: create
+product_group: group::code review
+product_category: code_review
+value_type: number
+status: implemented
+milestone: "13.12"
+time_frame: 28d
+data_source: redis_hll
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/61654
+distribution:
+- ce
+- ee
+tier:
+- free
+- premium
+- ultimate

config/metrics/counts_28d/20210514013549_i_code_review_user_load_conflict_ui_monthly.yml

+---
+key_path: redis_hll_counters.code_review.i_code_review_user_load_conflict_ui_monthly
+name: load_conflict_ui
+description: Count of unique users per week who load the conflict resolution page
+product_section:
+product_stage: create
+product_group: group::code review
+product_category: code_review
+value_type: number
+status: implemented
+milestone: "13.12"
+time_frame: 28d
+data_source: redis_hll
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/61654
+distribution:
+- ce
+- ee
+tier:
+- free
+- premium
+- ultimate

config/metrics/counts_7d/20210514013544_i_code_review_user_load_conflict_ui_weekly.yml

+---
+key_path: redis_hll_counters.code_review.i_code_review_user_load_conflict_ui_weekly
+name: load_conflict_ui
+description: Count of unique users per week who load the conflict resolution page
+product_section:
+product_stage: create
+product_group: group::code review
+product_category: code_review
+value_type: number
+status: implemented
+milestone: "13.12"
+time_frame: 7d
+data_source: redis_hll
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/61654
+distribution:
+- ce
+- ee
+tier:
+- free
+- premium
+- ultimate

config/metrics/counts_7d/20210514013545_i_code_review_user_resolve_conflict_weekly.yml

+---
+key_path: redis_hll_counters.code_review.i_code_review_user_resolve_conflict_weekly
+name: resolve_conflict
+description: Count of unique users per week who attempt to resolve a conflict through the ui
+product_section:
+product_stage: create
+product_group: group::code review
+product_category: code_review
+value_type: number
+status: implemented
+milestone: "13.12"
+time_frame: 28d
+data_source: redis_hll
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/61654
+distribution:
+- ce
+- ee
+tier:
+- free
+- premium
+- ultimate

doc/development/usage_ping/dictionary.md

@@ -9346,6 +9346,30 @@ Status: `data_available`
 
 Tiers: `free`, `premium`, `ultimate`
 
+### `redis_hll_counters.code_review.i_code_review_user_load_conflict_ui_monthly`
+
+Count of unique users per week who load the conflict resolution page
+
+[YAML definition](https://gitlab.com/gitlab-org/gitlab/-/blob/master/config/metrics/counts_28d/20210514013549_i_code_review_user_load_conflict_ui_monthly.yml)
+
+Group: `group::code review`
+
+Status: `implemented`
+
+Tiers: `free`, `premium`, `ultimate`
+
+### `redis_hll_counters.code_review.i_code_review_user_load_conflict_ui_weekly`
+
+Count of unique users per week who load the conflict resolution page
+
+[YAML definition](https://gitlab.com/gitlab-org/gitlab/-/blob/master/config/metrics/counts_7d/20210514013544_i_code_review_user_load_conflict_ui_weekly.yml)
+
+Group: `group::code review`
+
+Status: `implemented`
+
+Tiers: `free`, `premium`, `ultimate`
+
 ### `redis_hll_counters.code_review.i_code_review_user_marked_as_draft_monthly`
 
 Count of unique users per month who mark a merge request as a draft
@@ -9562,6 +9586,30 @@ Status: `data_available`
 
 Tiers: `free`, `premium`, `ultimate`
 
+### `redis_hll_counters.code_review.i_code_review_user_resolve_conflict_monthly`
+
+Count of unique users per week who attempt to resolve a conflict through the ui
+
+[YAML definition](https://gitlab.com/gitlab-org/gitlab/-/blob/master/config/metrics/counts_28d/20210514013545_i_code_review_user_resolve_conflict_monthly.yml)
+
+Group: `group::code review`
+
+Status: `implemented`
+
+Tiers: `free`, `premium`, `ultimate`
+
+### `redis_hll_counters.code_review.i_code_review_user_resolve_conflict_weekly`
+
+Count of unique users per week who attempt to resolve a conflict through the ui
+
+[YAML definition](https://gitlab.com/gitlab-org/gitlab/-/blob/master/config/metrics/counts_7d/20210514013545_i_code_review_user_resolve_conflict_weekly.yml)
+
+Group: `group::code review`
+
+Status: `implemented`
+
+Tiers: `free`, `premium`, `ultimate`
+
 ### `redis_hll_counters.code_review.i_code_review_user_resolve_thread_monthly`
 
 Count of unique users per month who resolve a thread in a merge request

lib/gitlab/usage_data_counters/known_events/code_review_events.yml

@@ -219,3 +219,11 @@
   category: code_review
   aggregation: weekly
   feature_flag: diff_settings_usage_data
+- name: i_code_review_user_load_conflict_ui
+  redis_slot: code_review
+  category: code_review
+  aggregation: weekly
+- name: i_code_review_user_resolve_conflict
+  redis_slot: code_review
+  category: code_review
+  aggregation: weekly

lib/gitlab/usage_data_counters/merge_request_activity_unique_counter.rb

@@ -44,6 +44,8 @@ module Gitlab
       MR_INCLUDING_CI_CONFIG_ACTION = 'o_pipeline_authoring_unique_users_pushing_mr_ciconfigfile'
       MR_MILESTONE_CHANGED_ACTION = 'i_code_review_user_milestone_changed'
       MR_LABELS_CHANGED_ACTION = 'i_code_review_user_labels_changed'
+      MR_LOAD_CONFLICT_UI_ACTION = 'i_code_review_user_load_conflict_ui'
+      MR_RESOLVE_CONFLICT_ACTION = 'i_code_review_user_resolve_conflict'
 
       class << self
         def track_mr_diffs_action(merge_request:)
@@ -201,6 +203,14 @@ module Gitlab
           track_unique_action_by_user(MR_LABELS_CHANGED_ACTION, user)
         end
 
+        def track_loading_conflict_ui_action(user:)
+          track_unique_action_by_user(MR_LOAD_CONFLICT_UI_ACTION, user)
+        end
+
+        def track_resolve_conflict_action(user:)
+          track_unique_action_by_user(MR_RESOLVE_CONFLICT_ACTION, user)
+        end
+
         private
 
         def track_unique_action_by_merge_request(action, merge_request)

spec/controllers/projects/merge_requests/conflicts_controller_spec.rb

@@ -17,8 +17,31 @@ RSpec.describe Projects::MergeRequests::ConflictsController do
   end
 
   describe 'GET show' do
+    context 'when the request is html' do
+      before do
+        allow(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
+          .to receive(:track_loading_conflict_ui_action)
+
+        get :show,
+            params: {
+              namespace_id: merge_request_with_conflicts.project.namespace.to_param,
+              project_id: merge_request_with_conflicts.project,
+              id: merge_request_with_conflicts.iid
+            },
+            format: 'html'
+      end
+
+      it 'does tracks the resolve call' do
+        expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
+          .to have_received(:track_loading_conflict_ui_action).with(user: user)
+      end
+    end
+
     context 'when the conflicts cannot be resolved in the UI' do
       before do
+        allow(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
+          .to receive(:track_loading_conflict_ui_action)
+
         allow(Gitlab::Git::Conflict::Parser).to receive(:parse)
           .and_raise(Gitlab::Git::Conflict::Parser::UnmergeableFile)
 
@@ -38,6 +61,11 @@ RSpec.describe Projects::MergeRequests::ConflictsController do
       it 'returns JSON with a message' do
         expect(json_response.keys).to contain_exactly('message', 'type')
       end
+
+      it 'does not track the resolve call' do
+        expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
+          .not_to have_received(:track_loading_conflict_ui_action).with(user: user)
+      end
     end
 
     context 'with valid conflicts' do
@@ -145,20 +173,19 @@ RSpec.describe Projects::MergeRequests::ConflictsController do
         conflict_for_path(path)
       end
 
-      it 'returns a 200 status code' do
-        expect(response).to have_gitlab_http_status(:ok)
-      end
-
-      it 'returns the file in JSON format' do
+      it 'returns a 200 and the file in JSON format' do
         content = MergeRequests::Conflicts::ListService.new(merge_request_with_conflicts)
                     .file_for_path(path, path)
                     .content
 
-        expect(json_response).to include('old_path' => path,
-                                         'new_path' => path,
-                                         'blob_icon' => 'doc-text',
-                                         'blob_path' => a_string_ending_with(path),
-                                         'content' => content)
+        aggregate_failures do
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(json_response).to include('old_path' => path,
+                                           'new_path' => path,
+                                           'blob_icon' => 'doc-text',
+                                           'blob_path' => a_string_ending_with(path),
+                                           'content' => content)
+        end
       end
     end
   end
@@ -166,6 +193,11 @@ RSpec.describe Projects::MergeRequests::ConflictsController do
   context 'POST resolve_conflicts' do
     let!(:original_head_sha) { merge_request_with_conflicts.diff_head_sha }
 
+    before do
+      allow(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
+        .to receive(:track_resolve_conflict_action)
+    end
+
     def resolve_conflicts(files)
       post :resolve_conflicts,
            params: {
@@ -201,13 +233,16 @@ RSpec.describe Projects::MergeRequests::ConflictsController do
         resolve_conflicts(resolved_files)
       end
 
-      it 'creates a new commit on the branch' do
-        expect(original_head_sha).not_to eq(merge_request_with_conflicts.source_branch_head.sha)
-        expect(merge_request_with_conflicts.source_branch_head.message).to include('Commit message')
-      end
+      it 'handles the success case' do
+        aggregate_failures do
+          # creates a new commit on the branch
+          expect(original_head_sha).not_to eq(merge_request_with_conflicts.source_branch_head.sha)
+          expect(merge_request_with_conflicts.source_branch_head.message).to include('Commit message')
 
-      it 'returns an OK response' do
-        expect(response).to have_gitlab_http_status(:ok)
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
+            .to have_received(:track_resolve_conflict_action).with(user: user)
+        end
       end
     end
 
@@ -232,16 +267,17 @@ RSpec.describe Projects::MergeRequests::ConflictsController do
         resolve_conflicts(resolved_files)
       end
 
-      it 'returns a 400 error' do
-        expect(response).to have_gitlab_http_status(:bad_request)
-      end
-
-      it 'has a message with the name of the first missing section' do
-        expect(json_response['message']).to include('6eb14e00385d2fb284765eb1cd8d420d33d63fc9_21_21')
-      end
+      it 'handles the error case' do
+        aggregate_failures do
+          # has a message with the name of the first missing section
+          expect(json_response['message']).to include('6eb14e00385d2fb284765eb1cd8d420d33d63fc9_21_21')
+          # does not create a new commit
+          expect(original_head_sha).to eq(merge_request_with_conflicts.source_branch_head.sha)
 
-      it 'does not create a new commit' do
-        expect(original_head_sha).to eq(merge_request_with_conflicts.source_branch_head.sha)
+          expect(response).to have_gitlab_http_status(:bad_request)
+          expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
+            .to have_received(:track_resolve_conflict_action).with(user: user)
+        end
       end
     end
 
@@ -262,16 +298,17 @@ RSpec.describe Projects::MergeRequests::ConflictsController do
         resolve_conflicts(resolved_files)
       end
 
-      it 'returns a 400 error' do
-        expect(response).to have_gitlab_http_status(:bad_request)
-      end
+      it 'handles the error case' do
+        aggregate_failures do
+          # has a message with the name of the missing file
+          expect(json_response['message']).to include('files/ruby/popen.rb')
+          # does not create a new commit
+          expect(original_head_sha).to eq(merge_request_with_conflicts.source_branch_head.sha)
 
-      it 'has a message with the name of the missing file' do
-        expect(json_response['message']).to include('files/ruby/popen.rb')
-      end
-
-      it 'does not create a new commit' do
-        expect(original_head_sha).to eq(merge_request_with_conflicts.source_branch_head.sha)
+          expect(response).to have_gitlab_http_status(:bad_request)
+          expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
+            .to have_received(:track_resolve_conflict_action).with(user: user)
+        end
       end
     end
 
@@ -300,16 +337,17 @@ RSpec.describe Projects::MergeRequests::ConflictsController do
         resolve_conflicts(resolved_files)
       end
 
-      it 'returns a 400 error' do
-        expect(response).to have_gitlab_http_status(:bad_request)
-      end
-
-      it 'has a message with the path of the problem file' do
-        expect(json_response['message']).to include('files/ruby/popen.rb')
-      end
+      it 'handles the error case' do
+        aggregate_failures do
+          # has a message with the path of the problem file
+          expect(json_response['message']).to include('files/ruby/popen.rb')
+          # does not create a new commit
+          expect(original_head_sha).to eq(merge_request_with_conflicts.source_branch_head.sha)
 
-      it 'does not create a new commit' do
-        expect(original_head_sha).to eq(merge_request_with_conflicts.source_branch_head.sha)
+          expect(response).to have_gitlab_http_status(:bad_request)
+          expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
+            .to have_received(:track_resolve_conflict_action).with(user: user)
+        end
       end
     end
   end

spec/lib/gitlab/usage_data_counters/merge_request_activity_unique_counter_spec.rb

@@ -386,4 +386,20 @@ RSpec.describe Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter, :cl
       let(:action) { described_class::MR_LABELS_CHANGED_ACTION }
     end
   end
+
+  describe '.track_loading_conflict_ui_action' do
+    subject { described_class.track_loading_conflict_ui_action(user: user) }
+
+    it_behaves_like 'a tracked merge request unique event' do
+      let(:action) { described_class::MR_LOAD_CONFLICT_UI_ACTION }
+    end
+  end
+
+  describe '.track_resolve_conflict_action' do
+    subject { described_class.track_resolve_conflict_action(user: user) }
+
+    it_behaves_like 'a tracked merge request unique event' do
+      let(:action) { described_class::MR_RESOLVE_CONFLICT_ACTION }
+    end
+  end
 end