Commit 100aa2a6 authored by Phil Hughes's avatar Phil Hughes

Added request/remove attention quick action

Adds a request and a remove attention quick action.

Closes https://gitlab.com/gitlab-org/gitlab/-/issues/345368
parent f9e06ba8
......@@ -266,6 +266,8 @@ class GfmAutoComplete {
UNASSIGN_REVIEWER: '/unassign_reviewer',
REASSIGN: '/reassign',
CC: '/cc',
ATTENTION: '/attention',
REMOVE_ATTENTION: '/remove_attention',
};
let assignees = [];
let reviewers = [];
......@@ -344,6 +346,23 @@ class GfmAutoComplete {
} else if (command === MEMBER_COMMAND.UNASSIGN_REVIEWER) {
// Only include members which are not assigned as a reviewer to Issuable currently
return data.filter((member) => reviewers.includes(member.search));
} else if (
command === MEMBER_COMMAND.ATTENTION ||
command === MEMBER_COMMAND.REMOVE_ATTENTION
) {
const attentionUsers = [
...(SidebarMediator.singleton?.store?.assignees || []),
...(SidebarMediator.singleton?.store?.reviewers || []),
];
const attentionRequested = command === MEMBER_COMMAND.REMOVE_ATTENTION;
return data.filter((member) =>
attentionUsers.find(
(u) =>
createMemberSearchString(u).includes(member.search) &&
u.attention_requested === attentionRequested,
),
);
}
return data;
......
......@@ -246,7 +246,9 @@ module MergeRequests
def remove_all_attention_requests(merge_request)
return unless merge_request.attention_requested_enabled?
::MergeRequests::BulkRemoveAttentionRequestedService.new(project: merge_request.project, current_user: current_user, merge_request: merge_request).execute
users = merge_request.reviewers + merge_request.assignees
::MergeRequests::BulkRemoveAttentionRequestedService.new(project: merge_request.project, current_user: current_user, merge_request: merge_request, users: users.uniq).execute
end
def remove_attention_requested(merge_request, user)
......
......@@ -3,20 +3,24 @@
module MergeRequests
class BulkRemoveAttentionRequestedService < MergeRequests::BaseService
attr_accessor :merge_request
attr_accessor :users
def initialize(project:, current_user:, merge_request:)
def initialize(project:, current_user:, merge_request:, users:)
super(project: project, current_user: current_user)
@merge_request = merge_request
@users = users
end
# rubocop: disable CodeReuse/ActiveRecord
def execute
return error("Invalid permissions") unless can?(current_user, :update_merge_request, merge_request)
merge_request.merge_request_assignees.update_all(state: :reviewed)
merge_request.merge_request_reviewers.update_all(state: :reviewed)
merge_request.merge_request_assignees.where(user_id: users).update_all(state: :reviewed)
merge_request.merge_request_reviewers.where(user_id: users).update_all(state: :reviewed)
success
end
# rubocop: enable CodeReuse/ActiveRecord
end
end
---
key_path: redis_hll_counters.quickactions.i_quickactions_attention_monthly
description: Count of MAU using the `/attention` quick action
product_section: dev
product_stage: create
product_group: group::code review
product_category: code_review
value_type: number
status: active
milestone: '14.8'
introduced_by_url:
time_frame: 28d
data_source: redis_hll
data_category: optional
instrumentation_class: RedisHLLMetric
options:
events:
- i_quickactions_attention
distribution:
- ce
- ee
tier:
- free
- premium
- ultimate
---
key_path: redis_hll_counters.quickactions.i_quickactions_remove_attention_monthly
description: Count of MAU using the `/remove_attention` quick action
product_section: dev
product_stage: create
product_group: group::code review
product_category: code_review
value_type: number
status: active
milestone: '14.8'
introduced_by_url:
time_frame: 28d
data_source: redis_hll
data_category: optional
instrumentation_class: RedisHLLMetric
options:
events:
- i_quickactions_remove_attention
distribution:
- ce
- ee
tier:
- free
- premium
- ultimate
---
key_path: redis_hll_counters.quickactions.i_quickactions_attention_weekly
description: Count of WAU using the `/attention` quick action
product_section: dev
product_stage: create
product_group: group::code review
product_category: code_review
value_type: number
status: active
milestone: '14.8'
introduced_by_url:
time_frame: 7d
data_source: redis_hll
data_category: optional
instrumentation_class: RedisHLLMetric
options:
events:
- i_quickactions_attention
distribution:
- ce
- ee
tier:
- free
- premium
- ultimate
---
key_path: redis_hll_counters.quickactions.i_quickactions_remove_attention_weekly
description: Count of WAU using the `/remove_attention` quick action
product_section: dev
product_stage: create
product_group: group::code review
product_category: code_review
value_type: number
status: active
milestone: '14.8'
introduced_by_url:
time_frame: 7d
data_source: redis_hll
data_category: optional
instrumentation_class: RedisHLLMetric
options:
events:
- i_quickactions_remove_attention
distribution:
- ce
- ee
tier:
- free
- premium
- ultimate
......@@ -1213,6 +1213,49 @@ RSpec.describe QuickActions::InterpretService do
end
end
end
describe 'attention command' do
let(:issuable) { create(:merge_request, reviewers: [developer, developer2], source_project: project) }
let(:reviewer) { issuable.merge_request_reviewers.find_by(user_id: developer.id) }
let(:reviewer2) { issuable.merge_request_reviewers.find_by(user_id: developer2.id) }
let(:content) { "/attention @#{developer.username} @#{developer2.username}" }
context 'with multiple users' do
before do
reviewer.update!(state: :reviewed)
reviewer2.update!(state: :reviewed)
end
it 'updates reviewers attention status' do
_, _, message = service.execute(content, issuable)
expect(message).to eq("Requested attention from #{developer.to_reference} and #{developer2.to_reference}.")
reviewer.reload
reviewer2.reload
expect(reviewer).to be_attention_requested
expect(reviewer2).to be_attention_requested
end
end
end
describe 'remove attention command' do
let(:issuable) { create(:merge_request, reviewers: [developer, developer2], source_project: project) }
let(:reviewer) { issuable.merge_request_reviewers.find_by(user_id: developer.id) }
let(:reviewer2) { issuable.merge_request_reviewers.find_by(user_id: developer2.id) }
let(:content) { "/remove_attention @#{developer.username} @#{developer2.username}" }
context 'with multiple users' do
it 'updates reviewers attention status' do
_, _, message = service.execute(content, issuable)
expect(message).to eq("Removed attention from #{developer.to_reference} and #{developer2.to_reference}.")
expect(reviewer).not_to be_attention_requested
expect(reviewer2).not_to be_attention_requested
end
end
end
end
describe '#explain' do
......
......@@ -249,6 +249,76 @@ module Gitlab
@updates[:reviewer_ids] = []
end
end
desc do
if quick_action_target.allows_multiple_reviewers?
_('Request attention from assignee(s) or reviewer(s)')
else
_('Request attention from assignee or reviewer')
end
end
explanation do |users|
_('Request attention from %{users_sentence}.') % { users_sentence: reviewer_users_sentence(users) }
end
execution_message do |users = nil|
if users.blank?
_("Failed to request attention because no user was found.")
else
_('Requested attention from %{users_sentence}.') % { users_sentence: reviewer_users_sentence(users) }
end
end
params do
quick_action_target.allows_multiple_reviewers? ? '@user1 @user2' : '@user'
end
types MergeRequest
condition do
Feature.enabled?(:mr_attention_requests, project, default_enabled: :yaml) &&
current_user.can?(:"admin_#{quick_action_target.to_ability_name}", project)
end
parse_params do |attention_param|
extract_users(attention_param)
end
command :attention do |users|
next if users.empty?
users.each do |user|
::MergeRequests::ToggleAttentionRequestedService.new(project: quick_action_target.project, merge_request: quick_action_target, current_user: current_user, user: user).execute
end
end
desc do
if quick_action_target.allows_multiple_reviewers?
_('Remove attention request(s)')
else
_('Remove attention request')
end
end
explanation do |users|
_('Removes attention from %{users_sentence}.') % { users_sentence: reviewer_users_sentence(users) }
end
execution_message do |users = nil|
if users.blank?
_("Failed to remove attention because no user was found.")
else
_('Removed attention from %{users_sentence}.') % { users_sentence: reviewer_users_sentence(users) }
end
end
params do
quick_action_target.allows_multiple_reviewers? ? '@user1 @user2' : '@user'
end
types MergeRequest
condition do
Feature.enabled?(:mr_attention_requests, project, default_enabled: :yaml) &&
current_user.can?(:"admin_#{quick_action_target.to_ability_name}", project)
end
parse_params do |attention_param|
extract_users(attention_param)
end
command :remove_attention do |users|
next if users.empty?
::MergeRequests::BulkRemoveAttentionRequestedService.new(project: quick_action_target.project, merge_request: quick_action_target, current_user: current_user, users: users).execute
end
end
def reviewer_users_sentence(users)
......
......@@ -295,3 +295,11 @@
category: quickactions
redis_slot: quickactions
aggregation: weekly
- name: i_quickactions_attention
category: quickactions
redis_slot: quickactions
aggregation: weekly
- name: i_quickactions_remove_attention
category: quickactions
redis_slot: quickactions
aggregation: weekly
......@@ -14875,6 +14875,9 @@ msgstr ""
msgid "Failed to remove a to-do item for the design."
msgstr ""
msgid "Failed to remove attention because no user was found."
msgstr ""
msgid "Failed to remove mirror."
msgstr ""
......@@ -14887,6 +14890,9 @@ msgstr ""
msgid "Failed to remove user key."
msgstr ""
msgid "Failed to request attention because no user was found."
msgstr ""
msgid "Failed to reset key. Please try again."
msgstr ""
......@@ -29754,6 +29760,9 @@ msgstr ""
msgid "Remove attention request"
msgstr ""
msgid "Remove attention request(s)"
msgstr ""
msgid "Remove avatar"
msgstr ""
......@@ -29892,6 +29901,9 @@ msgstr ""
msgid "Removed an issue from an epic."
msgstr ""
msgid "Removed attention from %{users_sentence}."
msgstr ""
msgid "Removed attention request from @%{username}"
msgstr ""
......@@ -29943,6 +29955,9 @@ msgstr ""
msgid "Removes an issue from an epic."
msgstr ""
msgid "Removes attention from %{users_sentence}."
msgstr ""
msgid "Removes parent epic %{epic_ref}."
msgstr ""
......@@ -30343,6 +30358,15 @@ msgstr ""
msgid "Request attention"
msgstr ""
msgid "Request attention from %{users_sentence}."
msgstr ""
msgid "Request attention from assignee or reviewer"
msgstr ""
msgid "Request attention from assignee(s) or reviewer(s)"
msgstr ""
msgid "Request attention to review"
msgstr ""
......@@ -30367,6 +30391,9 @@ msgstr ""
msgid "Requested %{time_ago}"
msgstr ""
msgid "Requested attention from %{users_sentence}."
msgstr ""
msgid "Requested attention from @%{username}"
msgstr ""
......
......@@ -10,7 +10,7 @@ RSpec.describe MergeRequests::BulkRemoveAttentionRequestedService do
let(:reviewer) { merge_request.find_reviewer(user) }
let(:assignee) { merge_request.find_assignee(assignee_user) }
let(:project) { merge_request.project }
let(:service) { described_class.new(project: project, current_user: current_user, merge_request: merge_request) }
let(:service) { described_class.new(project: project, current_user: current_user, merge_request: merge_request, users: [user, assignee_user]) }
let(:result) { service.execute }
before do
......@@ -20,7 +20,7 @@ RSpec.describe MergeRequests::BulkRemoveAttentionRequestedService do
describe '#execute' do
context 'invalid permissions' do
let(:service) { described_class.new(project: project, current_user: create(:user), merge_request: merge_request) }
let(:service) { described_class.new(project: project, current_user: create(:user), merge_request: merge_request, users: [user]) }
it 'returns an error' do
expect(result[:status]).to eq :error
......
......@@ -703,6 +703,27 @@ RSpec.describe QuickActions::InterpretService do
end
end
shared_examples 'attention command' do
it 'updates reviewers attention status' do
_, _, message = service.execute(content, issuable)
expect(message).to eq("Requested attention from #{developer.to_reference}.")
reviewer.reload
expect(reviewer).to be_attention_requested
end
end
shared_examples 'remove attention command' do
it 'updates reviewers attention status' do
_, _, message = service.execute(content, issuable)
expect(message).to eq("Removed attention from #{developer.to_reference}.")
expect(reviewer).not_to be_attention_requested
end
end
it_behaves_like 'reopen command' do
let(:content) { '/reopen' }
let(:issuable) { issue }
......@@ -2279,6 +2300,82 @@ RSpec.describe QuickActions::InterpretService do
expect(message).to eq('One or more contacts were successfully removed.')
end
end
describe 'attention command' do
let(:issuable) { create(:merge_request, reviewers: [developer], source_project: project) }
let(:reviewer) { issuable.merge_request_reviewers.find_by(user_id: developer.id) }
let(:content) { "/attention @#{developer.username}" }
context 'with one user' do
before do
reviewer.update!(state: :reviewed)
end
it_behaves_like 'attention command'
end
context 'with no user' do
let(:content) { "/attention" }
it_behaves_like 'failed command', 'Failed to request attention because no user was found.'
end
context 'with incorrect permissions' do
let(:service) { described_class.new(project, create(:user)) }
it_behaves_like 'failed command', 'Could not apply attention command.'
end
context 'with feature flag disabled' do
before do
stub_feature_flags(mr_attention_requests: false)
end
it_behaves_like 'failed command', 'Could not apply attention command.'
end
context 'with an issue instead of a merge request' do
let(:issuable) { issue }
it_behaves_like 'failed command', 'Could not apply attention command.'
end
end
describe 'remove attention command' do
let(:issuable) { create(:merge_request, reviewers: [developer], source_project: project) }
let(:reviewer) { issuable.merge_request_reviewers.find_by(user_id: developer.id) }
let(:content) { "/remove_attention @#{developer.username}" }
context 'with one user' do
it_behaves_like 'remove attention command'
end
context 'with no user' do
let(:content) { "/remove_attention" }
it_behaves_like 'failed command', 'Failed to remove attention because no user was found.'
end
context 'with incorrect permissions' do
let(:service) { described_class.new(project, create(:user)) }
it_behaves_like 'failed command', 'Could not apply remove_attention command.'
end
context 'with feature flag disabled' do
before do
stub_feature_flags(mr_attention_requests: false)
end
it_behaves_like 'failed command', 'Could not apply remove_attention command.'
end
context 'with an issue instead of a merge request' do
let(:issuable) { issue }
it_behaves_like 'failed command', 'Could not apply remove_attention command.'
end
end
end
describe '#explain' do
......
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