Commit 925ea029 authored by Heinrich Lee Yu's avatar Heinrich Lee Yu

Fix tracking of issue label change events

We cannot use an after_save hook here because ChangeLabelsService uses
bulk_insert. This was working unexpectedly because after_save was being
triggered when the markdown cache was being updated.
parent e6cee6a1
......@@ -15,7 +15,6 @@ class ResourceLabelEvent < ResourceEvent
validate :exactly_one_issuable
after_save :expire_etag_cache
after_save :usage_metrics
after_destroy :expire_etag_cache
enum action: {
......@@ -114,16 +113,6 @@ class ResourceLabelEvent < ResourceEvent
def discussion_id_key
[self.class.name, created_at, user_id]
end
def for_issue?
issue_id.present?
end
def usage_metrics
return unless for_issue?
Gitlab::UsageDataCounters::IssueActivityUniqueCounter.track_issue_label_changed_action(author: user)
end
end
ResourceLabelEvent.prepend_if_ee('EE::ResourceLabelEvent')
......@@ -24,6 +24,8 @@ module ResourceEvents
Gitlab::Database.bulk_insert(ResourceLabelEvent.table_name, labels) # rubocop:disable Gitlab/BulkInsert
resource.expire_note_etag_cache
Gitlab::UsageDataCounters::IssueActivityUniqueCounter.track_issue_label_changed_action(author: user) if resource.is_a?(Issue)
end
private
......
......@@ -51,14 +51,6 @@ RSpec.describe ResourceLabelEvent, type: :model do
end
context 'callbacks' do
describe '#usage_metrics' do
it 'tracks changed labels' do
expect(Gitlab::UsageDataCounters::IssueActivityUniqueCounter).to receive(:track_issue_label_changed_action)
subject.save!
end
end
describe '#expire_etag_cache' do
def expect_expiration(issue)
expect_next_instance_of(Gitlab::EtagCaching::Store) do |instance|
......
......@@ -10,7 +10,7 @@ RSpec.describe ResourceEvents::ChangeLabelsService do
describe '.change_labels' do
subject { described_class.new(resource, author).execute(added_labels: added, removed_labels: removed) }
let(:labels) { create_list(:label, 2, project: project) }
let_it_be(:labels) { create_list(:label, 2, project: project) }
def expect_label_event(event, label, action)
expect(event.user).to eq(author)
......@@ -57,5 +57,28 @@ RSpec.describe ResourceEvents::ChangeLabelsService do
expect { subject }.to change { resource.resource_label_events.count }.from(0).to(2)
end
end
describe 'usage data' do
let(:added) { [labels[0]] }
let(:removed) { [labels[1]] }
context 'when resource is an issue' do
it 'tracks changed labels' do
expect(Gitlab::UsageDataCounters::IssueActivityUniqueCounter).to receive(:track_issue_label_changed_action)
subject
end
end
context 'when resource is a merge request' do
let(:resource) { create(:merge_request, source_project: project) }
it 'does not track changed labels' do
expect(Gitlab::UsageDataCounters::IssueActivityUniqueCounter).not_to receive(:track_issue_label_changed_action)
subject
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