Commit b423307e authored by Marc Shaw's avatar Marc Shaw

Track when reviews are started and published

MR: gitlab.com/gitlab-org/gitlab/-/merge_requests/51521
Issue: gitlab.com/gitlab-org/gitlab/-/issues/292822
parent 4262129a
...@@ -8,6 +8,10 @@ module DraftNotes ...@@ -8,6 +8,10 @@ module DraftNotes
@merge_request, @current_user, @params = merge_request, current_user, params.dup @merge_request, @current_user, @params = merge_request, current_user, params.dup
end end
def merge_request_activity_counter
Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter
end
private private
def draft_notes def draft_notes
......
...@@ -31,6 +31,10 @@ module DraftNotes ...@@ -31,6 +31,10 @@ module DraftNotes
merge_request.diffs.clear_cache merge_request.diffs.clear_cache
end end
if draft_note.persisted?
merge_request_activity_counter.track_create_review_note_action(user: current_user)
end
draft_note draft_note
end end
......
...@@ -9,6 +9,7 @@ module DraftNotes ...@@ -9,6 +9,7 @@ module DraftNotes
publish_draft_note(draft) publish_draft_note(draft)
else else
publish_draft_notes publish_draft_notes
merge_request_activity_counter.track_publish_review_action(user: current_user)
end end
success success
......
---
title: Add metrics to starting and publishing a review
merge_request: 51521
author:
type: other
---
name: usage_data_i_code_review_user_create_review_note
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/51521
rollout_issue_url:
milestone: '13.8'
type: development
group: group::code review
default_enabled: true
---
name: usage_data_i_code_review_user_publish_review
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/51351
rollout_issue_url:
milestone: '13.8'
type: development
group: group::code review
default_enabled: true
...@@ -486,6 +486,16 @@ ...@@ -486,6 +486,16 @@
category: code_review category: code_review
aggregation: weekly aggregation: weekly
feature_flag: usage_data_i_code_review_user_remove_mr_comment feature_flag: usage_data_i_code_review_user_remove_mr_comment
- name: i_code_review_user_create_review_note
redis_slot: code_review
category: code_review
aggregation: weekly
feature_flag: usage_data_i_code_review_user_create_review_note
- name: i_code_review_user_publish_review
redis_slot: code_review
category: code_review
aggregation: weekly
feature_flag: usage_data_i_code_review_user_publish_review
# Terraform # Terraform
- name: p_terraform_state_api_unique_users - name: p_terraform_state_api_unique_users
category: terraform category: terraform
......
...@@ -13,6 +13,8 @@ module Gitlab ...@@ -13,6 +13,8 @@ module Gitlab
MR_CREATE_COMMENT_ACTION = 'i_code_review_user_create_mr_comment' MR_CREATE_COMMENT_ACTION = 'i_code_review_user_create_mr_comment'
MR_EDIT_COMMENT_ACTION = 'i_code_review_user_edit_mr_comment' MR_EDIT_COMMENT_ACTION = 'i_code_review_user_edit_mr_comment'
MR_REMOVE_COMMENT_ACTION = 'i_code_review_user_remove_mr_comment' 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'
class << self class << self
def track_mr_diffs_action(merge_request:) def track_mr_diffs_action(merge_request:)
...@@ -52,6 +54,14 @@ module Gitlab ...@@ -52,6 +54,14 @@ module Gitlab
track_unique_action_by_user(MR_REMOVE_COMMENT_ACTION, user) track_unique_action_by_user(MR_REMOVE_COMMENT_ACTION, user)
end end
def track_create_review_note_action(user:)
track_unique_action_by_user(MR_CREATE_REVIEW_NOTE_ACTION, user)
end
def track_publish_review_action(user:)
track_unique_action_by_user(MR_PUBLISH_REVIEW_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)
......
...@@ -95,4 +95,20 @@ RSpec.describe Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter, :cl ...@@ -95,4 +95,20 @@ RSpec.describe Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter, :cl
let(:action) { described_class::MR_REMOVE_COMMENT_ACTION } let(:action) { described_class::MR_REMOVE_COMMENT_ACTION }
end end
end end
describe '.track_create_review_note_action' do
subject { described_class.track_create_review_note_action(user: user) }
it_behaves_like 'a tracked merge request unique event' do
let(:action) { described_class::MR_CREATE_REVIEW_NOTE_ACTION }
end
end
describe '.track_publish_review_action' do
subject { described_class.track_publish_review_action(user: user) }
it_behaves_like 'a tracked merge request unique event' do
let(:action) { described_class::MR_PUBLISH_REVIEW_ACTION }
end
end
end end
...@@ -20,6 +20,23 @@ RSpec.describe DraftNotes::CreateService do ...@@ -20,6 +20,23 @@ RSpec.describe DraftNotes::CreateService do
expect(draft.discussion_id).to be_nil expect(draft.discussion_id).to be_nil
end end
it 'tracks the start event when the draft is persisted' do
expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
.to receive(:track_create_review_note_action)
.with(user: user)
draft = create_draft(note: 'This is a test')
expect(draft).to be_persisted
end
it 'does not track the start event when the draft is not persisted' do
expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
.not_to receive(:track_create_review_note_action)
draft = create_draft(note: 'Not a reply!', resolve_discussion: true)
expect(draft).not_to be_persisted
end
it 'cannot resolve when there is nothing to resolve' do it 'cannot resolve when there is nothing to resolve' do
draft = create_draft(note: 'Not a reply!', resolve_discussion: true) draft = create_draft(note: 'Not a reply!', resolve_discussion: true)
......
...@@ -43,6 +43,13 @@ RSpec.describe DraftNotes::PublishService do ...@@ -43,6 +43,13 @@ RSpec.describe DraftNotes::PublishService do
expect(result[:status]).to eq(:success) expect(result[:status]).to eq(:success)
end end
it 'does not track the publish event' do
expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
.not_to receive(:track_publish_review_action)
publish(draft: drafts.first)
end
context 'commit_id is set' do context 'commit_id is set' do
let(:commit_id) { commit.id } let(:commit_id) { commit.id }
...@@ -74,6 +81,13 @@ RSpec.describe DraftNotes::PublishService do ...@@ -74,6 +81,13 @@ RSpec.describe DraftNotes::PublishService do
expect { publish }.not_to change { DraftNote.count } expect { publish }.not_to change { DraftNote.count }
end end
it 'does not track the publish event' do
expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
.not_to receive(:track_publish_review_action)
publish
end
it 'returns an error' do it 'returns an error' do
result = publish result = publish
...@@ -105,6 +119,14 @@ RSpec.describe DraftNotes::PublishService do ...@@ -105,6 +119,14 @@ RSpec.describe DraftNotes::PublishService do
publish publish
end end
it 'tracks the publish event' do
expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
.to receive(:track_publish_review_action)
.with(user: user)
publish
end
context 'commit_id is set' do context 'commit_id is set' do
let(:commit_id) { commit.id } let(:commit_id) { commit.id }
......
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