Commit 4e59373a authored by Marc Shaw's avatar Marc Shaw

Track multiline comments on a merge request

MR: gitlab.com/gitlab-org/gitlab/-/merge_requests/51098
Issue: gitlab.com/gitlab-org/gitlab/-/issues/292822
parent 8ccc32f6
......@@ -111,6 +111,10 @@ class DiffNote < Note
super.merge(suggestions_filter_enabled: true)
end
def multiline?
position&.multiline?
end
private
def enqueue_diff_file_creation_job
......
......@@ -122,7 +122,7 @@ module Notes
end
def track_note_creation_usage_for_merge_requests(note)
Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter.track_create_comment_action(user: note.author)
Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter.track_create_comment_action(note: note)
end
end
end
......@@ -19,7 +19,7 @@ module Notes
end
def track_note_removal_usage_for_merge_requests(note)
Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter.track_remove_comment_action(user: note.author)
Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter.track_remove_comment_action(note: note)
end
end
end
......
......@@ -98,7 +98,7 @@ module Notes
end
def track_note_edit_usage_for_merge_requests(note)
Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter.track_edit_comment_action(user: note.author)
Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter.track_edit_comment_action(note: note)
end
end
end
......
---
title: Add metrics to creating, editing or removing multiline comments on merge requests
merge_request: 51098
author:
type: other
---
name: usage_data_i_code_review_user_create_multiline_mr_comment
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/51098
rollout_issue_url:
milestone: '13.8'
type: development
group: group::code review
default_enabled: true
---
name: usage_data_i_code_review_user_edit_multiline_mr_comment
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/51098
rollout_issue_url:
milestone: '13.8'
type: development
group: group::code review
default_enabled: true
---
name: usage_data_i_code_review_user_remove_multiline_mr_comment
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/51098
rollout_issue_url:
milestone: '13.8'
type: development
group: group::code review
default_enabled: true
......@@ -19,6 +19,7 @@ module Gitlab
:height,
:x,
:y,
:line_range,
:position_type, to: :formatter
# A position can belong to a text line or to an image coordinate
......@@ -167,6 +168,12 @@ module Gitlab
end
end
def multiline?
return unless on_text? && line_range
line_range['start'] != line_range['end']
end
private
def find_diff_file(repository)
......
......@@ -496,6 +496,21 @@
category: code_review
aggregation: weekly
feature_flag: usage_data_i_code_review_user_publish_review
- name: i_code_review_user_create_multiline_mr_comment
redis_slot: code_review
category: code_review
aggregation: weekly
feature_flag: usage_data_i_code_review_user_create_multiline_mr_comment
- name: i_code_review_user_edit_multiline_mr_comment
redis_slot: code_review
category: code_review
aggregation: weekly
feature_flag: usage_data_i_code_review_user_edit_multiline_mr_comment
- name: i_code_review_user_remove_multiline_mr_comment
redis_slot: code_review
category: code_review
aggregation: weekly
feature_flag: usage_data_i_code_review_user_remove_multiline_mr_comment
# Terraform
- name: p_terraform_state_api_unique_users
category: terraform
......
......@@ -15,6 +15,9 @@ module Gitlab
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'
MR_CREATE_MULTILINE_COMMENT_ACTION = 'i_code_review_user_create_multiline_mr_comment'
MR_EDIT_MULTILINE_COMMENT_ACTION = 'i_code_review_user_edit_multiline_mr_comment'
MR_REMOVE_MULTILINE_COMMENT_ACTION = 'i_code_review_user_remove_multiline_mr_comment'
class << self
def track_mr_diffs_action(merge_request:)
......@@ -42,16 +45,19 @@ module Gitlab
track_unique_action_by_user(MR_REOPEN_ACTION, user)
end
def track_create_comment_action(user:)
track_unique_action_by_user(MR_CREATE_COMMENT_ACTION, user)
def track_create_comment_action(note:)
track_unique_action_by_user(MR_CREATE_COMMENT_ACTION, note.author)
track_multiline_unique_action(MR_CREATE_MULTILINE_COMMENT_ACTION, note)
end
def track_edit_comment_action(user:)
track_unique_action_by_user(MR_EDIT_COMMENT_ACTION, user)
def track_edit_comment_action(note:)
track_unique_action_by_user(MR_EDIT_COMMENT_ACTION, note.author)
track_multiline_unique_action(MR_EDIT_MULTILINE_COMMENT_ACTION, note)
end
def track_remove_comment_action(user:)
track_unique_action_by_user(MR_REMOVE_COMMENT_ACTION, user)
def track_remove_comment_action(note:)
track_unique_action_by_user(MR_REMOVE_COMMENT_ACTION, note.author)
track_multiline_unique_action(MR_REMOVE_MULTILINE_COMMENT_ACTION, note)
end
def track_create_review_note_action(user:)
......@@ -77,6 +83,12 @@ module Gitlab
def track_unique_action(action, value)
Gitlab::UsageDataCounters::HLLRedisCounter.track_usage_event(action, value)
end
def track_multiline_unique_action(action, note)
return unless note.is_a?(DiffNote) && note.multiline?
track_unique_action_by_user(action, note.author)
end
end
end
end
......
......@@ -752,4 +752,62 @@ RSpec.describe Gitlab::Diff::Position do
expect(subject.file_hash).to eq(Digest::SHA1.hexdigest(subject.file_path))
end
end
describe '#multiline?' do
let(:end_line_code) { "ab09011fa121d0a2bb9fa4ca76094f2482b902b7_#{end_old_line}_#{end_new_line}" }
let(:line_range) do
{
"start" => {
"line_code" => "ab09011fa121d0a2bb9fa4ca76094f2482b902b7_18_18",
"type" => nil,
"old_line" => 18,
"new_line" => 18
},
"end" => {
"line_code" => end_line_code,
"type" => nil,
"old_line" => end_old_line,
"new_line" => end_new_line
}
}
end
subject(:multiline) do
described_class.new(
line_range: line_range,
position_type: position_type
)
end
let(:end_old_line) { 20 }
let(:end_new_line) { 20 }
context 'when the position type is text' do
let(:position_type) { "text" }
context 'when the start lines equal the end lines' do
let(:end_old_line) { 18 }
let(:end_new_line) { 18 }
it "returns true" do
expect(subject.multiline?).to be_falsey
end
end
context 'when the start lines do not equal the end lines' do
it "returns true" do
expect(subject.multiline?).to be_truthy
end
end
end
context 'when the position type is not text' do
let(:position_type) { "image" }
it "returns false" do
expect(subject.multiline?).to be_falsey
end
end
end
end
......@@ -5,6 +5,7 @@ require 'spec_helper'
RSpec.describe Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter, :clean_gitlab_redis_shared_state do
let(:merge_request) { build(:merge_request, id: 1) }
let(:user) { build(:user, id: 1) }
let(:note) { build(:note, author: user) }
shared_examples_for 'a tracked merge request unique event' do
specify do
......@@ -73,27 +74,63 @@ RSpec.describe Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter, :cl
end
describe '.track_create_comment_action' do
subject { described_class.track_create_comment_action(user: user) }
subject { described_class.track_create_comment_action(note: note) }
it_behaves_like 'a tracked merge request unique event' do
let(:action) { described_class::MR_CREATE_COMMENT_ACTION }
end
context 'when the note is multiline diff note' do
let(:note) { build(:diff_note_on_merge_request, author: user) }
before do
allow(note).to receive(:multiline?).and_return(true)
end
it_behaves_like 'a tracked merge request unique event' do
let(:action) { described_class::MR_CREATE_MULTILINE_COMMENT_ACTION }
end
end
end
describe '.track_edit_comment_action' do
subject { described_class.track_edit_comment_action(user: user) }
subject { described_class.track_edit_comment_action(note: note) }
it_behaves_like 'a tracked merge request unique event' do
let(:action) { described_class::MR_EDIT_COMMENT_ACTION }
end
context 'when the note is multiline diff note' do
let(:note) { build(:diff_note_on_merge_request, author: user) }
before do
allow(note).to receive(:multiline?).and_return(true)
end
it_behaves_like 'a tracked merge request unique event' do
let(:action) { described_class::MR_EDIT_MULTILINE_COMMENT_ACTION }
end
end
end
describe '.track_remove_comment_action' do
subject { described_class.track_remove_comment_action(user: user) }
subject { described_class.track_remove_comment_action(note: note) }
it_behaves_like 'a tracked merge request unique event' do
let(:action) { described_class::MR_REMOVE_COMMENT_ACTION }
end
context 'when the note is multiline diff note' do
let(:note) { build(:diff_note_on_merge_request, author: user) }
before do
allow(note).to receive(:multiline?).and_return(true)
end
it_behaves_like 'a tracked merge request unique event' do
let(:action) { described_class::MR_REMOVE_MULTILINE_COMMENT_ACTION }
end
end
end
describe '.track_create_review_note_action' do
......
......@@ -120,7 +120,7 @@ RSpec.describe Notes::CreateService do
end
it 'tracks merge request usage data' do
expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter).to receive(:track_create_comment_action).with(user: user)
expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter).to receive(:track_create_comment_action).with(note: kind_of(Note))
described_class.new(project_with_repo, user, new_opts).execute
end
......
......@@ -38,7 +38,7 @@ RSpec.describe Notes::DestroyService do
it 'tracks merge request usage data' do
mr = create(:merge_request, source_project: project)
note = create(:note, project: project, noteable: mr)
expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter).to receive(:track_remove_comment_action).with(user: user)
expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter).to receive(:track_remove_comment_action).with(note: note)
described_class.new(project, user).execute(note)
end
......
......@@ -69,7 +69,7 @@ RSpec.describe Notes::UpdateService do
let(:note) { create(:note, project: project, noteable: merge_request, author: user, note: "Old note #{user2.to_reference}") }
it 'tracks merge request usage data' do
expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter).to receive(:track_edit_comment_action).with(user: user)
expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter).to receive(:track_edit_comment_action).with(note: note)
update_note(note: 'new text')
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