Commit 35c8e30f authored by Marc Shaw's avatar Marc Shaw

Track usage of the resolve UI

MR: gitlab.com/gitlab-org/gitlab/-/merge_requests/61654
Issue: gitlab.com/gitlab-org/gitlab/-/issues/292833

Changelog: other
parent e3be5309
...@@ -9,6 +9,7 @@ class Projects::MergeRequests::ConflictsController < Projects::MergeRequests::Ap ...@@ -9,6 +9,7 @@ class Projects::MergeRequests::ConflictsController < Projects::MergeRequests::Ap
respond_to do |format| respond_to do |format|
format.html do format.html do
@issuable_sidebar = serializer.represent(@merge_request, serializer: 'sidebar') @issuable_sidebar = serializer.represent(@merge_request, serializer: 'sidebar')
Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter.track_loading_conflict_ui_action(user: current_user)
end end
format.json do format.json do
...@@ -42,6 +43,8 @@ class Projects::MergeRequests::ConflictsController < Projects::MergeRequests::Ap ...@@ -42,6 +43,8 @@ class Projects::MergeRequests::ConflictsController < Projects::MergeRequests::Ap
def resolve_conflicts def resolve_conflicts
return render_404 unless @conflicts_list.can_be_resolved_in_ui? 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? if @merge_request.can_be_merged?
render status: :bad_request, json: { message: _('The merge conflicts for this merge request have already been resolved.') } render status: :bad_request, json: { message: _('The merge conflicts for this merge request have already been resolved.') }
return return
......
---
title: Track usage of the resolve conflict UI
merge_request: 61654
author:
type: other
...@@ -63,6 +63,8 @@ ...@@ -63,6 +63,8 @@
- 'i_code_review_diff_hide_whitespace' - 'i_code_review_diff_hide_whitespace'
- 'i_code_review_diff_single_file' - 'i_code_review_diff_single_file'
- 'i_code_review_diff_multiple_files' - '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 - name: code_review_category_monthly_active_users
operator: OR operator: OR
feature_flag: usage_data_code_review_aggregation feature_flag: usage_data_code_review_aggregation
...@@ -118,6 +120,8 @@ ...@@ -118,6 +120,8 @@
- 'i_code_review_diff_hide_whitespace' - 'i_code_review_diff_hide_whitespace'
- 'i_code_review_diff_single_file' - 'i_code_review_diff_single_file'
- 'i_code_review_diff_multiple_files' - '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 - name: code_review_extension_category_monthly_active_users
operator: OR operator: OR
feature_flag: usage_data_code_review_aggregation feature_flag: usage_data_code_review_aggregation
......
---
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
---
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
---
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
---
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
...@@ -9382,6 +9382,30 @@ Status: `data_available` ...@@ -9382,6 +9382,30 @@ Status: `data_available`
Tiers: `free`, `premium`, `ultimate` 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` ### `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 Count of unique users per month who mark a merge request as a draft
...@@ -9598,6 +9622,30 @@ Status: `data_available` ...@@ -9598,6 +9622,30 @@ Status: `data_available`
Tiers: `free`, `premium`, `ultimate` 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` ### `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 Count of unique users per month who resolve a thread in a merge request
......
...@@ -219,3 +219,11 @@ ...@@ -219,3 +219,11 @@
category: code_review category: code_review
aggregation: weekly aggregation: weekly
feature_flag: diff_settings_usage_data 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
...@@ -44,6 +44,8 @@ module Gitlab ...@@ -44,6 +44,8 @@ module Gitlab
MR_INCLUDING_CI_CONFIG_ACTION = 'o_pipeline_authoring_unique_users_pushing_mr_ciconfigfile' MR_INCLUDING_CI_CONFIG_ACTION = 'o_pipeline_authoring_unique_users_pushing_mr_ciconfigfile'
MR_MILESTONE_CHANGED_ACTION = 'i_code_review_user_milestone_changed' MR_MILESTONE_CHANGED_ACTION = 'i_code_review_user_milestone_changed'
MR_LABELS_CHANGED_ACTION = 'i_code_review_user_labels_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 class << self
def track_mr_diffs_action(merge_request:) def track_mr_diffs_action(merge_request:)
...@@ -201,6 +203,14 @@ module Gitlab ...@@ -201,6 +203,14 @@ module Gitlab
track_unique_action_by_user(MR_LABELS_CHANGED_ACTION, user) track_unique_action_by_user(MR_LABELS_CHANGED_ACTION, user)
end 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 private
def track_unique_action_by_merge_request(action, merge_request) def track_unique_action_by_merge_request(action, merge_request)
......
...@@ -17,8 +17,31 @@ RSpec.describe Projects::MergeRequests::ConflictsController do ...@@ -17,8 +17,31 @@ RSpec.describe Projects::MergeRequests::ConflictsController do
end end
describe 'GET show' do 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 context 'when the conflicts cannot be resolved in the UI' do
before do before do
allow(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
.to receive(:track_loading_conflict_ui_action)
allow(Gitlab::Git::Conflict::Parser).to receive(:parse) allow(Gitlab::Git::Conflict::Parser).to receive(:parse)
.and_raise(Gitlab::Git::Conflict::Parser::UnmergeableFile) .and_raise(Gitlab::Git::Conflict::Parser::UnmergeableFile)
...@@ -38,6 +61,11 @@ RSpec.describe Projects::MergeRequests::ConflictsController do ...@@ -38,6 +61,11 @@ RSpec.describe Projects::MergeRequests::ConflictsController do
it 'returns JSON with a message' do it 'returns JSON with a message' do
expect(json_response.keys).to contain_exactly('message', 'type') expect(json_response.keys).to contain_exactly('message', 'type')
end 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 end
context 'with valid conflicts' do context 'with valid conflicts' do
...@@ -145,20 +173,19 @@ RSpec.describe Projects::MergeRequests::ConflictsController do ...@@ -145,20 +173,19 @@ RSpec.describe Projects::MergeRequests::ConflictsController do
conflict_for_path(path) conflict_for_path(path)
end end
it 'returns a 200 status code' do it 'returns a 200 and the file in JSON format' do
expect(response).to have_gitlab_http_status(:ok)
end
it 'returns the file in JSON format' do
content = MergeRequests::Conflicts::ListService.new(merge_request_with_conflicts) content = MergeRequests::Conflicts::ListService.new(merge_request_with_conflicts)
.file_for_path(path, path) .file_for_path(path, path)
.content .content
expect(json_response).to include('old_path' => path, aggregate_failures do
'new_path' => path, expect(response).to have_gitlab_http_status(:ok)
'blob_icon' => 'doc-text', expect(json_response).to include('old_path' => path,
'blob_path' => a_string_ending_with(path), 'new_path' => path,
'content' => content) 'blob_icon' => 'doc-text',
'blob_path' => a_string_ending_with(path),
'content' => content)
end
end end
end end
end end
...@@ -166,6 +193,11 @@ RSpec.describe Projects::MergeRequests::ConflictsController do ...@@ -166,6 +193,11 @@ RSpec.describe Projects::MergeRequests::ConflictsController do
context 'POST resolve_conflicts' do context 'POST resolve_conflicts' do
let!(:original_head_sha) { merge_request_with_conflicts.diff_head_sha } 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) def resolve_conflicts(files)
post :resolve_conflicts, post :resolve_conflicts,
params: { params: {
...@@ -201,13 +233,17 @@ RSpec.describe Projects::MergeRequests::ConflictsController do ...@@ -201,13 +233,17 @@ RSpec.describe Projects::MergeRequests::ConflictsController do
resolve_conflicts(resolved_files) resolve_conflicts(resolved_files)
end end
it 'creates a new commit on the branch' do it 'handles the success case' do
expect(original_head_sha).not_to eq(merge_request_with_conflicts.source_branch_head.sha) aggregate_failures do
expect(merge_request_with_conflicts.source_branch_head.message).to include('Commit message') # creates a new commit on the branch
end 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 # returns an OK response
expect(response).to have_gitlab_http_status(:ok) expect(response).to have_gitlab_http_status(:ok)
# it 'tracks the resolve call' do
expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
.to have_received(:track_resolve_conflict_action).with(user: user)
end
end end
end end
...@@ -232,16 +268,18 @@ RSpec.describe Projects::MergeRequests::ConflictsController do ...@@ -232,16 +268,18 @@ RSpec.describe Projects::MergeRequests::ConflictsController do
resolve_conflicts(resolved_files) resolve_conflicts(resolved_files)
end end
it 'returns a 400 error' do it 'handles the error case' do
expect(response).to have_gitlab_http_status(:bad_request) aggregate_failures do
end # returns a 400 error
expect(response).to have_gitlab_http_status(:bad_request)
it 'has a message with the name of the first missing section' do # has a message with the name of the first missing section
expect(json_response['message']).to include('6eb14e00385d2fb284765eb1cd8d420d33d63fc9_21_21') expect(json_response['message']).to include('6eb14e00385d2fb284765eb1cd8d420d33d63fc9_21_21')
end # 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 # tracks the resolve call
expect(original_head_sha).to eq(merge_request_with_conflicts.source_branch_head.sha) expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
.to have_received(:track_resolve_conflict_action).with(user: user)
end
end end
end end
...@@ -262,16 +300,18 @@ RSpec.describe Projects::MergeRequests::ConflictsController do ...@@ -262,16 +300,18 @@ RSpec.describe Projects::MergeRequests::ConflictsController do
resolve_conflicts(resolved_files) resolve_conflicts(resolved_files)
end end
it 'returns a 400 error' do it 'handles the error case' do
expect(response).to have_gitlab_http_status(:bad_request) aggregate_failures do
end # returns a 400 error
expect(response).to have_gitlab_http_status(:bad_request)
it 'has a message with the name of the missing file' do # has a message with the name of the missing file
expect(json_response['message']).to include('files/ruby/popen.rb') expect(json_response['message']).to include('files/ruby/popen.rb')
end # 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 # tracks the resolve call
expect(original_head_sha).to eq(merge_request_with_conflicts.source_branch_head.sha) expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
.to have_received(:track_resolve_conflict_action).with(user: user)
end
end end
end end
...@@ -300,16 +340,18 @@ RSpec.describe Projects::MergeRequests::ConflictsController do ...@@ -300,16 +340,18 @@ RSpec.describe Projects::MergeRequests::ConflictsController do
resolve_conflicts(resolved_files) resolve_conflicts(resolved_files)
end end
it 'returns a 400 error' do it 'handles the error case' do
expect(response).to have_gitlab_http_status(:bad_request) aggregate_failures do
end # returns a 400 error
expect(response).to have_gitlab_http_status(:bad_request)
it 'has a message with the path of the problem file' do # has a message with the path of the problem file
expect(json_response['message']).to include('files/ruby/popen.rb') expect(json_response['message']).to include('files/ruby/popen.rb')
end # 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 # tracks the resolve call
expect(original_head_sha).to eq(merge_request_with_conflicts.source_branch_head.sha) expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
.to have_received(:track_resolve_conflict_action).with(user: user)
end
end end
end end
end end
......
...@@ -386,4 +386,20 @@ RSpec.describe Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter, :cl ...@@ -386,4 +386,20 @@ RSpec.describe Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter, :cl
let(:action) { described_class::MR_LABELS_CHANGED_ACTION } let(:action) { described_class::MR_LABELS_CHANGED_ACTION }
end end
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 end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment