Commit 38e438cd authored by Jan Provaznik's avatar Jan Provaznik

Merge branch '229918-issuedata-comments' into 'master'

Add Issue usagedata for add/remove/edit comments

See merge request gitlab-org/gitlab!45609
parents 0e17360d 21aa5833
...@@ -73,6 +73,8 @@ module Notes ...@@ -73,6 +73,8 @@ module Notes
if note.for_merge_request? && note.diff_note? && note.start_of_discussion? if note.for_merge_request? && note.diff_note? && note.start_of_discussion?
Discussions::CaptureDiffNotePositionService.new(note.noteable, note.diff_file&.paths).execute(note.discussion) Discussions::CaptureDiffNotePositionService.new(note.noteable, note.diff_file&.paths).execute(note.discussion)
end end
track_note_creation_usage_for_issues(note) if note.for_issue?
end end
def do_commands(note, update_params, message, only_commands) def do_commands(note, update_params, message, only_commands)
...@@ -113,5 +115,9 @@ module Notes ...@@ -113,5 +115,9 @@ module Notes
track_usage_event(:incident_management_incident_comment, user.id) track_usage_event(:incident_management_incident_comment, user.id)
end end
def track_note_creation_usage_for_issues(note)
Gitlab::UsageDataCounters::IssueActivityUniqueCounter.track_issue_comment_added_action(author: note.author)
end
end end
end end
...@@ -8,6 +8,13 @@ module Notes ...@@ -8,6 +8,13 @@ module Notes
end end
clear_noteable_diffs_cache(note) clear_noteable_diffs_cache(note)
track_note_removal_usage_for_issues(note) if note.for_issue?
end
private
def track_note_removal_usage_for_issues(note)
Gitlab::UsageDataCounters::IssueActivityUniqueCounter.track_issue_comment_removed_action(author: note.author)
end end
end end
end end
......
...@@ -14,6 +14,8 @@ module Notes ...@@ -14,6 +14,8 @@ module Notes
note.save note.save
end end
track_note_edit_usage_for_issues(note) if note.for_issue?
only_commands = false only_commands = false
quick_actions_service = QuickActionsService.new(project, current_user) quick_actions_service = QuickActionsService.new(project, current_user)
...@@ -89,6 +91,10 @@ module Notes ...@@ -89,6 +91,10 @@ module Notes
Note.id_in(note.discussion.notes.map(&:id)).update_all(confidential: params[:confidential]) Note.id_in(note.discussion.notes.map(&:id)).update_all(confidential: params[:confidential])
end end
def track_note_edit_usage_for_issues(note)
Gitlab::UsageDataCounters::IssueActivityUniqueCounter.track_issue_comment_edited_action(author: note.author)
end
end end
end end
......
---
title: UsageData for issues added/removed/edited
merge_request: 45609
author:
type: added
...@@ -33,6 +33,9 @@ module Gitlab ...@@ -33,6 +33,9 @@ module Gitlab
ISSUE_DUE_DATE_CHANGED = 'g_project_management_issue_due_date_changed' ISSUE_DUE_DATE_CHANGED = 'g_project_management_issue_due_date_changed'
ISSUE_TIME_ESTIMATE_CHANGED = 'g_project_management_issue_time_estimate_changed' ISSUE_TIME_ESTIMATE_CHANGED = 'g_project_management_issue_time_estimate_changed'
ISSUE_TIME_SPENT_CHANGED = 'g_project_management_issue_time_spent_changed' ISSUE_TIME_SPENT_CHANGED = 'g_project_management_issue_time_spent_changed'
ISSUE_COMMENT_ADDED = 'g_project_management_issue_comment_added'
ISSUE_COMMENT_EDITED = 'g_project_management_issue_comment_edited'
ISSUE_COMMENT_REMOVED = 'g_project_management_issue_comment_removed'
class << self class << self
def track_issue_created_action(author:, time: Time.zone.now) def track_issue_created_action(author:, time: Time.zone.now)
...@@ -147,6 +150,18 @@ module Gitlab ...@@ -147,6 +150,18 @@ module Gitlab
track_unique_action(ISSUE_TIME_SPENT_CHANGED, author, time) track_unique_action(ISSUE_TIME_SPENT_CHANGED, author, time)
end end
def track_issue_comment_added_action(author:, time: Time.zone.now)
track_unique_action(ISSUE_COMMENT_ADDED, author, time)
end
def track_issue_comment_edited_action(author:, time: Time.zone.now)
track_unique_action(ISSUE_COMMENT_EDITED, author, time)
end
def track_issue_comment_removed_action(author:, time: Time.zone.now)
track_unique_action(ISSUE_COMMENT_REMOVED, author, time)
end
private private
def track_unique_action(action, author, time) def track_unique_action(action, author, time)
......
...@@ -303,3 +303,15 @@ ...@@ -303,3 +303,15 @@
category: issues_edit category: issues_edit
redis_slot: project_management redis_slot: project_management
aggregation: daily aggregation: daily
- name: g_project_management_issue_comment_added
category: issues_edit
redis_slot: project_management
aggregation: daily
- name: g_project_management_issue_comment_edited
category: issues_edit
redis_slot: project_management
aggregation: daily
- name: g_project_management_issue_comment_removed
category: issues_edit
redis_slot: project_management
aggregation: daily
...@@ -292,6 +292,36 @@ RSpec.describe Gitlab::UsageDataCounters::IssueActivityUniqueCounter, :clean_git ...@@ -292,6 +292,36 @@ RSpec.describe Gitlab::UsageDataCounters::IssueActivityUniqueCounter, :clean_git
end end
end end
context 'for Issue comment added actions' do
it_behaves_like 'tracks and counts action' do
let(:action) { described_class::ISSUE_COMMENT_ADDED }
def track_action(params)
described_class.track_issue_comment_added_action(**params)
end
end
end
context 'for Issue comment edited actions' do
it_behaves_like 'tracks and counts action' do
let(:action) { described_class::ISSUE_COMMENT_EDITED }
def track_action(params)
described_class.track_issue_comment_edited_action(**params)
end
end
end
context 'for Issue comment removed actions' do
it_behaves_like 'tracks and counts action' do
let(:action) { described_class::ISSUE_COMMENT_REMOVED }
def track_action(params)
described_class.track_issue_comment_removed_action(**params)
end
end
end
it 'can return the count of actions per user deduplicated', :aggregate_failures do it 'can return the count of actions per user deduplicated', :aggregate_failures do
described_class.track_issue_title_changed_action(author: user1) described_class.track_issue_title_changed_action(author: user1)
described_class.track_issue_description_changed_action(author: user1) described_class.track_issue_description_changed_action(author: user1)
......
...@@ -67,15 +67,37 @@ RSpec.describe Notes::CreateService do ...@@ -67,15 +67,37 @@ RSpec.describe Notes::CreateService do
let(:current_user) { user } let(:current_user) { user }
end end
end end
it 'tracks issue comment usage data', :clean_gitlab_redis_shared_state do
event = Gitlab::UsageDataCounters::IssueActivityUniqueCounter::ISSUE_COMMENT_ADDED
counter = Gitlab::UsageDataCounters::HLLRedisCounter
expect(Gitlab::UsageDataCounters::IssueActivityUniqueCounter).to receive(:track_issue_comment_added_action).with(author: user).and_call_original
expect do
described_class.new(project, user, opts).execute
end.to change { counter.unique_events(event_names: event, start_date: 1.day.ago, end_date: 1.day.from_now) }.by(1)
end end
context 'noteable highlight cache clearing' do context 'in a merge request' do
let(:project_with_repo) { create(:project, :repository) } let_it_be(:project_with_repo) { create(:project, :repository) }
let(:merge_request) do let_it_be(:merge_request) do
create(:merge_request, source_project: project_with_repo, create(:merge_request, source_project: project_with_repo,
target_project: project_with_repo) target_project: project_with_repo)
end end
context 'issue comment usage data' do
let(:opts) do
{ note: 'Awesome comment', noteable_type: 'MergeRequest', noteable_id: merge_request.id }
end
it 'does not track' do
expect(Gitlab::UsageDataCounters::IssueActivityUniqueCounter).not_to receive(:track_issue_comment_added_action)
described_class.new(project, user, opts).execute
end
end
context 'noteable highlight cache clearing' do
let(:position) do let(:position) do
Gitlab::Diff::Position.new(old_path: "files/ruby/popen.rb", Gitlab::Diff::Position.new(old_path: "files/ruby/popen.rb",
new_path: "files/ruby/popen.rb", new_path: "files/ruby/popen.rb",
...@@ -121,13 +143,6 @@ RSpec.describe Notes::CreateService do ...@@ -121,13 +143,6 @@ RSpec.describe Notes::CreateService do
end end
context 'note diff file' do context 'note diff file' do
let(:project_with_repo) { create(:project, :repository) }
let(:merge_request) do
create(:merge_request,
source_project: project_with_repo,
target_project: project_with_repo)
end
let(:line_number) { 14 } let(:line_number) { 14 }
let(:position) do let(:position) do
Gitlab::Diff::Position.new(old_path: "files/ruby/popen.rb", Gitlab::Diff::Position.new(old_path: "files/ruby/popen.rb",
...@@ -212,6 +227,8 @@ RSpec.describe Notes::CreateService do ...@@ -212,6 +227,8 @@ RSpec.describe Notes::CreateService do
end end
end end
end end
end
end
context 'note with commands' do context 'note with commands' do
context 'all quick actions' do context 'all quick actions' do
......
...@@ -24,18 +24,35 @@ RSpec.describe Notes::DestroyService do ...@@ -24,18 +24,35 @@ RSpec.describe Notes::DestroyService do
.to change { user.todos_pending_count }.from(1).to(0) .to change { user.todos_pending_count }.from(1).to(0)
end end
context 'noteable highlight cache clearing' do it 'tracks issue comment removal usage data', :clean_gitlab_redis_shared_state do
let(:repo_project) { create(:project, :repository) } note = create(:note, project: project, noteable: issue)
let(:merge_request) do event = Gitlab::UsageDataCounters::IssueActivityUniqueCounter::ISSUE_COMMENT_REMOVED
counter = Gitlab::UsageDataCounters::HLLRedisCounter
expect(Gitlab::UsageDataCounters::IssueActivityUniqueCounter).to receive(:track_issue_comment_removed_action).with(author: user).and_call_original
expect do
described_class.new(project, user).execute(note)
end.to change { counter.unique_events(event_names: event, start_date: 1.day.ago, end_date: 1.day.from_now) }.by(1)
end
context 'in a merge request' do
let_it_be(:repo_project) { create(:project, :repository) }
let_it_be(:merge_request) do
create(:merge_request, source_project: repo_project, create(:merge_request, source_project: repo_project,
target_project: repo_project) target_project: repo_project)
end end
let_it_be(:note) do
let(:note) do
create(:diff_note_on_merge_request, project: repo_project, create(:diff_note_on_merge_request, project: repo_project,
noteable: merge_request) noteable: merge_request)
end end
it 'does not track issue comment removal usage data' do
expect(Gitlab::UsageDataCounters::IssueActivityUniqueCounter).not_to receive(:track_issue_comment_removed_action)
described_class.new(repo_project, user).execute(note)
end
context 'noteable highlight cache clearing' do
before do before do
allow(note.position).to receive(:unfolded_diff?) { true } allow(note.position).to receive(:unfolded_diff?) { true }
end end
...@@ -57,4 +74,5 @@ RSpec.describe Notes::DestroyService do ...@@ -57,4 +74,5 @@ RSpec.describe Notes::DestroyService do
end end
end end
end end
end
end end
...@@ -47,6 +47,22 @@ RSpec.describe Notes::UpdateService do ...@@ -47,6 +47,22 @@ RSpec.describe Notes::UpdateService do
end end
end end
it 'does not track usage data when params is blank' do
expect(Gitlab::UsageDataCounters::IssueActivityUniqueCounter).not_to receive(:track_issue_comment_edited_action)
update_note({})
end
it 'tracks usage data', :clean_gitlab_redis_shared_state do
event = Gitlab::UsageDataCounters::IssueActivityUniqueCounter::ISSUE_COMMENT_EDITED
counter = Gitlab::UsageDataCounters::HLLRedisCounter
expect(Gitlab::UsageDataCounters::IssueActivityUniqueCounter).to receive(:track_issue_comment_edited_action).with(author: user).and_call_original
expect do
update_note(note: 'new text')
end.to change { counter.unique_events(event_names: event, start_date: 1.day.ago, end_date: 1.day.from_now) }.by(1)
end
context 'with system note' do context 'with system note' do
before do before do
note.update_column(:system, true) note.update_column(:system, true)
...@@ -55,6 +71,12 @@ RSpec.describe Notes::UpdateService do ...@@ -55,6 +71,12 @@ RSpec.describe Notes::UpdateService do
it 'does not update the note' do it 'does not update the note' do
expect { update_note(note: 'new text') }.not_to change { note.reload.note } expect { update_note(note: 'new text') }.not_to change { note.reload.note }
end end
it 'does not track usage data' do
expect(Gitlab::UsageDataCounters::IssueActivityUniqueCounter).not_to receive(:track_issue_comment_edited_action)
update_note(note: 'new text')
end
end end
context 'suggestions' do context 'suggestions' do
...@@ -220,6 +242,12 @@ RSpec.describe Notes::UpdateService do ...@@ -220,6 +242,12 @@ RSpec.describe Notes::UpdateService do
expect(note).not_to receive(:create_new_cross_references!) expect(note).not_to receive(:create_new_cross_references!)
update_note({ note: "Updated with new reference: #{issue.to_reference}" }) update_note({ note: "Updated with new reference: #{issue.to_reference}" })
end end
it 'does not track usage data' do
expect(Gitlab::UsageDataCounters::IssueActivityUniqueCounter).not_to receive(:track_issue_comment_edited_action)
update_note(note: 'new text')
end
end end
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