Commit cadc4528 authored by Heinrich Lee Yu's avatar Heinrich Lee Yu

Merge branch '292830-track-approval-rule-changes' into 'master'

Track approval rule changes [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!53761
parents 5e2365f5 d7a4f0fc
---
name: usage_data_i_code_review_user_approval_rule_added
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/issues/320966
rollout_issue_url:
milestone: '13.9'
type: development
group: group::code review
default_enabled: true
---
name: usage_data_i_code_review_user_approval_rule_deleted
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/issues/320966
rollout_issue_url:
milestone: '13.9'
type: development
group: group::code review
default_enabled: true
---
name: usage_data_i_code_review_user_approval_rule_edited
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/issues/320966
rollout_issue_url:
milestone: '13.9'
type: development
group: group::code review
default_enabled: true
......@@ -23,5 +23,9 @@ module ApprovalRules
def success(*args, &blk)
super.tap { |hsh| hsh[:rule] = rule }
end
def merge_request_activity_counter
Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter
end
end
end
......@@ -26,6 +26,9 @@ module ApprovalRules
def success
track_onboarding_progress
merge_request_activity_counter
.track_approval_rule_added_action(user: current_user)
super
end
......
......@@ -17,5 +17,12 @@ module ApprovalRules
error(rule.errors.messages)
end
end
def success
merge_request_activity_counter
.track_approval_rule_deleted_action(user: current_user)
super
end
end
end
......@@ -22,5 +22,12 @@ module ApprovalRules
params[:groups] += GroupFinder.new(rule, current_user).hidden_groups
end
end
def success
merge_request_activity_counter
.track_approval_rule_edited_action(user: current_user)
super
end
end
end
......@@ -10,22 +10,33 @@ RSpec.describe ApprovalRules::CreateService do
let(:new_approvers) { create_list(:user, 2) }
let(:new_groups) { create_list(:group, 2, :private) }
it 'creates approval, excluding non-eligible users and groups' do
result = described_class.new(target, user, {
name: 'security',
approvals_required: 1,
user_ids: new_approvers.map(&:id),
group_ids: new_groups.map(&:id)
}).execute
expect(result[:status]).to eq(:success)
rule = result[:rule]
expect(rule.name).to eq('security')
expect(rule.approvals_required).to eq(1)
expect(rule.users).to be_empty
expect(rule.groups).to be_empty
context 'basic creation action' do
let(:result) do
described_class.new(target, user, {
name: 'security',
approvals_required: 1,
user_ids: new_approvers.map(&:id),
group_ids: new_groups.map(&:id)
}).execute
end
it 'creates approval, excluding non-eligible users and groups' do
expect(result[:status]).to eq(:success)
rule = result[:rule]
expect(rule.name).to eq('security')
expect(rule.approvals_required).to eq(1)
expect(rule.users).to be_empty
expect(rule.groups).to be_empty
end
it 'tracks creation event via a usage counter' do
expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
.to receive(:track_approval_rule_added_action).once.with(user: user)
result
end
end
context 'when some users and groups are eligible' do
......
......@@ -32,6 +32,13 @@ RSpec.describe ApprovalRules::MergeRequestRuleDestroyService do
it 'returns successful status' do
expect(result[:status]).to eq(:success)
end
it 'tracks delete event via a usage counter' do
expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
.to receive(:track_approval_rule_deleted_action).once.with(user: user)
result
end
end
context 'when rule not successfully deleted' do
......@@ -42,6 +49,13 @@ RSpec.describe ApprovalRules::MergeRequestRuleDestroyService do
it 'returns error status' do
expect(result[:status]).to eq(:error)
end
it 'does not track delete event via a usage counter' do
expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
.not_to receive(:track_approval_rule_deleted_action)
result
end
end
end
end
......@@ -11,22 +11,33 @@ RSpec.describe ApprovalRules::UpdateService do
let(:new_approvers) { create_list(:user, 2) }
let(:new_groups) { create_list(:group, 2, :private) }
it 'updates approval, excluding non-eligible users and groups' do
result = described_class.new(approval_rule, user, {
name: 'security',
approvals_required: 1,
user_ids: new_approvers.map(&:id),
group_ids: new_groups.map(&:id)
}).execute
expect(result[:status]).to eq(:success)
rule = result[:rule]
expect(rule.name).to eq('security')
expect(rule.approvals_required).to eq(1)
expect(rule.users).to be_empty
expect(rule.groups).to be_empty
context 'basic update action' do
let(:result) do
described_class.new(approval_rule, user, {
name: 'security',
approvals_required: 1,
user_ids: new_approvers.map(&:id),
group_ids: new_groups.map(&:id)
}).execute
end
it 'updates approval, excluding non-eligible users and groups' do
expect(result[:status]).to eq(:success)
rule = result[:rule]
expect(rule.name).to eq('security')
expect(rule.approvals_required).to eq(1)
expect(rule.users).to be_empty
expect(rule.groups).to be_empty
end
it 'tracks update event via a usage counter' do
expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
.to receive(:track_approval_rule_edited_action).once.with(user: user)
result
end
end
context 'when some users and groups are eligible' do
......
......@@ -124,3 +124,18 @@
category: code_review
aggregation: weekly
feature_flag: usage_data_i_code_review_user_review_requested
- name: i_code_review_user_approval_rule_added
redis_slot: code_review
category: code_review
aggregation: weekly
feature_flag: usage_data_i_code_review_user_approval_rule_added
- name: i_code_review_user_approval_rule_deleted
redis_slot: code_review
category: code_review
aggregation: weekly
feature_flag: usage_data_i_code_review_user_approval_rule_deleted
- name: i_code_review_user_approval_rule_edited
redis_slot: code_review
category: code_review
aggregation: weekly
feature_flag: usage_data_i_code_review_user_approval_rule_edited
......@@ -26,6 +26,9 @@ module Gitlab
MR_UNRESOLVE_THREAD_ACTION = 'i_code_review_user_unresolve_thread'
MR_ASSIGNED_USERS_ACTION = 'i_code_review_user_assigned'
MR_REVIEW_REQUESTED_USERS_ACTION = 'i_code_review_user_review_requested'
MR_APPROVAL_RULE_ADDED_USERS_ACTION = 'i_code_review_user_approval_rule_added'
MR_APPROVAL_RULE_EDITED_USERS_ACTION = 'i_code_review_user_approval_rule_edited'
MR_APPROVAL_RULE_DELETED_USERS_ACTION = 'i_code_review_user_approval_rule_deleted'
MR_EDIT_MR_TITLE_ACTION = 'i_code_review_edit_mr_title'
MR_EDIT_MR_DESC_ACTION = 'i_code_review_edit_mr_desc'
......@@ -118,6 +121,18 @@ module Gitlab
track_unique_action_by_user(MR_EDIT_MR_DESC_ACTION, user)
end
def track_approval_rule_added_action(user:)
track_unique_action_by_user(MR_APPROVAL_RULE_ADDED_USERS_ACTION, user)
end
def track_approval_rule_edited_action(user:)
track_unique_action_by_user(MR_APPROVAL_RULE_EDITED_USERS_ACTION, user)
end
def track_approval_rule_deleted_action(user:)
track_unique_action_by_user(MR_APPROVAL_RULE_DELETED_USERS_ACTION, user)
end
private
def track_unique_action_by_merge_request(action, merge_request)
......
......@@ -228,4 +228,28 @@ RSpec.describe Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter, :cl
let(:action) { described_class::MR_REVIEW_REQUESTED_USERS_ACTION }
end
end
describe '.track_approval_rule_added_action' do
subject { described_class.track_approval_rule_added_action(user: user) }
it_behaves_like 'a tracked merge request unique event' do
let(:action) { described_class::MR_APPROVAL_RULE_ADDED_USERS_ACTION }
end
end
describe '.track_approval_rule_edited_action' do
subject { described_class.track_approval_rule_edited_action(user: user) }
it_behaves_like 'a tracked merge request unique event' do
let(:action) { described_class::MR_APPROVAL_RULE_EDITED_USERS_ACTION }
end
end
describe '.track_approval_rule_deleted_action' do
subject { described_class.track_approval_rule_deleted_action(user: user) }
it_behaves_like 'a tracked merge request unique event' do
let(:action) { described_class::MR_APPROVAL_RULE_DELETED_USERS_ACTION }
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