Commit 0d02d9c7 authored by Heinrich Lee Yu's avatar Heinrich Lee Yu Committed by Mayra Cabrera

Track issue health status changes in usage ping

Adds health status change events to the events we track for issues
parent 654f971a
...@@ -15,6 +15,8 @@ module EE ...@@ -15,6 +15,8 @@ module EE
health_status = noteable.health_status&.humanize(capitalize: false) health_status = noteable.health_status&.humanize(capitalize: false)
body = health_status ? "changed health status to **#{health_status}**" : 'removed the health status' body = health_status ? "changed health status to **#{health_status}**" : 'removed the health status'
issue_activity_counter.track_issue_health_status_changed_action(author: author) if noteable.is_a?(Issue)
create_note(NoteSummary.new(noteable, project, author, body, action: 'health_status')) create_note(NoteSummary.new(noteable, project, author, body, action: 'health_status'))
end end
......
---
title: Track issue health status changes in usage ping
merge_request: 44417
author:
type: other
# frozen_string_literal: true
module EE
module Gitlab
module UsageDataCounters
module IssueActivityUniqueCounter
extend ActiveSupport::Concern
ISSUE_HEALTH_STATUS_CHANGED = 'g_project_management_issue_health_status_changed'
class_methods do
def track_issue_health_status_changed_action(author:, time: Time.zone.now)
track_unique_action(ISSUE_HEALTH_STATUS_CHANGED, author, time)
end
end
end
end
end
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Gitlab::UsageDataCounters::IssueActivityUniqueCounter, :clean_gitlab_redis_shared_state do
let(:user1) { build(:user, id: 1) }
let(:user2) { build(:user, id: 2) }
let(:user3) { build(:user, id: 3) }
let(:time) { Time.zone.now }
context 'for Issue health status changed actions' do
it_behaves_like 'a tracked issue edit event' do
let(:action) { described_class::ISSUE_HEALTH_STATUS_CHANGED }
def track_action(params)
described_class.track_issue_health_status_changed_action(**params)
end
end
end
end
...@@ -14,11 +14,11 @@ RSpec.describe ::SystemNotes::IssuablesService do ...@@ -14,11 +14,11 @@ RSpec.describe ::SystemNotes::IssuablesService do
let(:service) { described_class.new(noteable: noteable, project: project, author: author) } let(:service) { described_class.new(noteable: noteable, project: project, author: author) }
describe '#change_health_status_note' do describe '#change_health_status_note' do
subject { service.change_health_status_note }
context 'when health_status changed' do context 'when health_status changed' do
let(:noteable) { create(:issue, project: project, title: 'Lorem ipsum', health_status: 'at_risk') } let(:noteable) { create(:issue, project: project, title: 'Lorem ipsum', health_status: 'at_risk') }
subject { service.change_health_status_note }
it_behaves_like 'a system note' do it_behaves_like 'a system note' do
let(:action) { 'health_status' } let(:action) { 'health_status' }
end end
...@@ -31,8 +31,6 @@ RSpec.describe ::SystemNotes::IssuablesService do ...@@ -31,8 +31,6 @@ RSpec.describe ::SystemNotes::IssuablesService do
context 'when health_status removed' do context 'when health_status removed' do
let(:noteable) { create(:issue, project: project, title: 'Lorem ipsum', health_status: nil) } let(:noteable) { create(:issue, project: project, title: 'Lorem ipsum', health_status: nil) }
subject { service.change_health_status_note }
it_behaves_like 'a system note' do it_behaves_like 'a system note' do
let(:action) { 'health_status' } let(:action) { 'health_status' }
end end
...@@ -41,6 +39,12 @@ RSpec.describe ::SystemNotes::IssuablesService do ...@@ -41,6 +39,12 @@ RSpec.describe ::SystemNotes::IssuablesService do
expect(subject.note).to eq 'removed the health status' expect(subject.note).to eq 'removed the health status'
end end
end end
it 'tracks the issue event in usage ping' do
expect(Gitlab::UsageDataCounters::IssueActivityUniqueCounter).to receive(:track_issue_health_status_changed_action).with(author: author)
subject
end
end end
describe '#publish_issue_to_status_page' do describe '#publish_issue_to_status_page' do
......
...@@ -174,3 +174,5 @@ module Gitlab ...@@ -174,3 +174,5 @@ module Gitlab
end end
end end
end end
Gitlab::UsageDataCounters::IssueActivityUniqueCounter.prepend_if_ee('EE::Gitlab::UsageDataCounters::IssueActivityUniqueCounter')
...@@ -315,3 +315,7 @@ ...@@ -315,3 +315,7 @@
category: issues_edit category: issues_edit
redis_slot: project_management redis_slot: project_management
aggregation: daily aggregation: daily
- name: g_project_management_issue_health_status_changed
category: issues_edit
redis_slot: project_management
aggregation: daily
...@@ -88,7 +88,7 @@ RSpec.describe Gitlab::ExclusiveLeaseHelpers, :clean_gitlab_redis_shared_state d ...@@ -88,7 +88,7 @@ RSpec.describe Gitlab::ExclusiveLeaseHelpers, :clean_gitlab_redis_shared_state d
let(:options) { { retries: 0 } } let(:options) { { retries: 0 } }
it 'never sleeps' do it 'never sleeps' do
expect(class_instance).not_to receive(:sleep) expect_any_instance_of(Gitlab::ExclusiveLeaseHelpers::SleepingLock).not_to receive(:sleep)
expect { subject }.to raise_error('Failed to obtain a lock') expect { subject }.to raise_error('Failed to obtain a lock')
end end
...@@ -98,7 +98,7 @@ RSpec.describe Gitlab::ExclusiveLeaseHelpers, :clean_gitlab_redis_shared_state d ...@@ -98,7 +98,7 @@ RSpec.describe Gitlab::ExclusiveLeaseHelpers, :clean_gitlab_redis_shared_state d
let(:options) { { retries: 1, sleep_sec: 0.05.seconds } } let(:options) { { retries: 1, sleep_sec: 0.05.seconds } }
it 'receives the specified argument' do it 'receives the specified argument' do
expect_any_instance_of(Object).to receive(:sleep).with(0.05.seconds).once expect_any_instance_of(Gitlab::ExclusiveLeaseHelpers::SleepingLock).to receive(:sleep).with(0.05.seconds).once
expect { subject }.to raise_error('Failed to obtain a lock') expect { subject }.to raise_error('Failed to obtain a lock')
end end
...@@ -108,8 +108,8 @@ RSpec.describe Gitlab::ExclusiveLeaseHelpers, :clean_gitlab_redis_shared_state d ...@@ -108,8 +108,8 @@ RSpec.describe Gitlab::ExclusiveLeaseHelpers, :clean_gitlab_redis_shared_state d
let(:options) { { retries: 2, sleep_sec: ->(num) { 0.1 + num } } } let(:options) { { retries: 2, sleep_sec: ->(num) { 0.1 + num } } }
it 'receives the specified argument' do it 'receives the specified argument' do
expect_any_instance_of(Object).to receive(:sleep).with(1.1.seconds).once expect_any_instance_of(Gitlab::ExclusiveLeaseHelpers::SleepingLock).to receive(:sleep).with(1.1.seconds).once
expect_any_instance_of(Object).to receive(:sleep).with(2.1.seconds).once expect_any_instance_of(Gitlab::ExclusiveLeaseHelpers::SleepingLock).to receive(:sleep).with(2.1.seconds).once
expect { subject }.to raise_error('Failed to obtain a lock') expect { subject }.to raise_error('Failed to obtain a lock')
end end
......
...@@ -59,7 +59,7 @@ RSpec.describe Gitlab::ImportExport::AttributesFinder do ...@@ -59,7 +59,7 @@ RSpec.describe Gitlab::ImportExport::AttributesFinder do
end end
before do before do
allow_any_instance_of(Gitlab::ImportExport).to receive(:config_file).and_return(test_config) allow(Gitlab::ImportExport).to receive(:config_file).and_return(test_config)
end end
it 'generates hash from project tree config' do it 'generates hash from project tree config' do
......
...@@ -12,7 +12,7 @@ RSpec.describe Gitlab::ImportExport::Group::LegacyTreeSaver do ...@@ -12,7 +12,7 @@ RSpec.describe Gitlab::ImportExport::Group::LegacyTreeSaver do
before do before do
group.add_maintainer(user) group.add_maintainer(user)
allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path) allow(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path)
end end
after do after do
......
...@@ -12,7 +12,7 @@ RSpec.describe Gitlab::ImportExport::Importer do ...@@ -12,7 +12,7 @@ RSpec.describe Gitlab::ImportExport::Importer do
subject(:importer) { described_class.new(project) } subject(:importer) { described_class.new(project) }
before do before do
allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(test_path) allow(Gitlab::ImportExport).to receive(:storage_path).and_return(test_path)
allow_any_instance_of(Gitlab::ImportExport::FileImporter).to receive(:remove_import_file) allow_any_instance_of(Gitlab::ImportExport::FileImporter).to receive(:remove_import_file)
stub_uploads_object_storage(FileUploader) stub_uploads_object_storage(FileUploader)
......
...@@ -13,7 +13,7 @@ RSpec.describe Gitlab::ImportExport::LfsRestorer do ...@@ -13,7 +13,7 @@ RSpec.describe Gitlab::ImportExport::LfsRestorer do
subject(:restorer) { described_class.new(project: project, shared: shared) } subject(:restorer) { described_class.new(project: project, shared: shared) }
before do before do
allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path) allow(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path)
FileUtils.mkdir_p(shared.export_path) FileUtils.mkdir_p(shared.export_path)
end end
......
...@@ -26,7 +26,7 @@ RSpec.describe API::ProjectExport, :clean_gitlab_redis_cache do ...@@ -26,7 +26,7 @@ RSpec.describe API::ProjectExport, :clean_gitlab_redis_cache do
let(:export_path) { "#{Dir.tmpdir}/project_export_spec" } let(:export_path) { "#{Dir.tmpdir}/project_export_spec" }
before do before do
allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path) allow(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path)
allow_next_instance_of(ProjectExportWorker) do |job| allow_next_instance_of(ProjectExportWorker) do |job|
allow(job).to receive(:jid).and_return(SecureRandom.hex(8)) allow(job).to receive(:jid).and_return(SecureRandom.hex(8))
end end
......
...@@ -15,7 +15,7 @@ module ImportExport ...@@ -15,7 +15,7 @@ module ImportExport
export_path = [prefix, 'spec', 'fixtures', 'lib', 'gitlab', 'import_export', name].compact export_path = [prefix, 'spec', 'fixtures', 'lib', 'gitlab', 'import_export', name].compact
export_path = File.join(*export_path) export_path = File.join(*export_path)
allow_any_instance_of(Gitlab::ImportExport).to receive(:export_path) { export_path } allow(Gitlab::ImportExport).to receive(:export_path) { export_path }
end end
def setup_reader(reader) def setup_reader(reader)
......
# frozen_string_literal: true
# This patch allows stubbing of prepended methods
# Based on https://github.com/rspec/rspec-mocks/pull/1218
module RSpec
module Mocks
module InstanceMethodStasherForPrependedMethods
private
def method_owned_by_klass?
owner = @klass.instance_method(@method).owner
owner = owner.class unless Module === owner
owner == @klass ||
# When `extend self` is used, and not under any instance of
(owner.singleton_class == @klass && !Mocks.space.any_instance_recorder_for(owner, true)) ||
!method_defined_on_klass?(owner)
end
end
end
end
module RSpec
module Mocks
module MethodDoubleForPrependedMethods
def restore_original_method
return show_frozen_warning if object_singleton_class.frozen?
return unless @method_is_proxied
remove_method_from_definition_target
if @method_stasher.method_is_stashed?
@method_stasher.restore
restore_original_visibility
end
@method_is_proxied = false
end
def restore_original_visibility
method_owner.__send__(@original_visibility, @method_name)
end
private
def method_owner
@method_owner ||= Object.instance_method(:method).bind(object).call(@method_name).owner
end
end
end
end
RSpec::Mocks::InstanceMethodStasher.prepend(RSpec::Mocks::InstanceMethodStasherForPrependedMethods)
RSpec::Mocks::MethodDouble.prepend(RSpec::Mocks::MethodDoubleForPrependedMethods)
# frozen_string_literal: true
RSpec.shared_examples 'a tracked issue edit event' do |event|
before do
stub_application_setting(usage_ping_enabled: true)
end
def count_unique(date_from:, date_to:)
Gitlab::UsageDataCounters::HLLRedisCounter.unique_events(event_names: action, start_date: date_from, end_date: date_to)
end
specify do
aggregate_failures do
expect(track_action(author: user1)).to be_truthy
expect(track_action(author: user1)).to be_truthy
expect(track_action(author: user2)).to be_truthy
expect(track_action(author: user3, time: time - 3.days)).to be_truthy
expect(count_unique(date_from: time, date_to: time)).to eq(2)
expect(count_unique(date_from: time - 5.days, date_to: 1.day.since(time))).to eq(3)
end
end
it 'does not track edit actions if author is not present' do
expect(track_action(author: nil)).to be_nil
end
context 'when feature flag track_issue_activity_actions is disabled' do
it 'does not track edit actions' do
stub_feature_flags(track_issue_activity_actions: false)
expect(track_action(author: user1)).to be_nil
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