Merge branch 'ph/345368/attentionQuickAction' into 'master'

Added request/remove attention quick action

See merge request gitlab-org/gitlab!79542

app/assets/javascripts/gfm_auto_complete.js

@@ -275,6 +275,8 @@ class GfmAutoComplete {
       UNASSIGN_REVIEWER: '/unassign_reviewer',
       REASSIGN: '/reassign',
       CC: '/cc',
+      ATTENTION: '/attention',
+      REMOVE_ATTENTION: '/remove_attention',
     };
     let assignees = [];
     let reviewers = [];
@@ -353,6 +355,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;

app/services/merge_requests/base_service.rb

@@ -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)

app/services/merge_requests/bulk_remove_attention_requested_service.rb

@@ -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

config/metrics/counts_28d/20220131143209_i_quickactions_attention_monthly.yml

+---
+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

config/metrics/counts_28d/20220131153230_i_quickactions_remove_attention_monthly.yml

+---
+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

config/metrics/counts_7d/20220131143201_i_quickactions_attention_weekly.yml

+---
+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

config/metrics/counts_7d/20220131153223_i_quickactions_remove_attention_weekly.yml

+---
+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

ee/spec/services/quick_actions/interpret_service_spec.rb

@@ -1231,6 +1231,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

lib/gitlab/quick_actions/merge_request_actions.rb

@@ -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)

lib/gitlab/usage_data_counters/known_events/quickactions.yml

@@ -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

locale/gitlab.pot

@@ -15023,6 +15023,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 ""
 
@@ -15035,6 +15038,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 ""
 
@@ -30016,6 +30022,9 @@ msgstr ""
 msgid "Remove attention request"
 msgstr ""
 
+msgid "Remove attention request(s)"
+msgstr ""
+
 msgid "Remove avatar"
 msgstr ""
 
@@ -30154,6 +30163,9 @@ msgstr ""
 msgid "Removed an issue from an epic."
 msgstr ""
 
+msgid "Removed attention from %{users_sentence}."
+msgstr ""
+
 msgid "Removed attention request from @%{username}"
 msgstr ""
 
@@ -30205,6 +30217,9 @@ msgstr ""
 msgid "Removes an issue from an epic."
 msgstr ""
 
+msgid "Removes attention from %{users_sentence}."
+msgstr ""
+
 msgid "Removes parent epic %{epic_ref}."
 msgstr ""
 
@@ -30605,6 +30620,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 ""
 
@@ -30629,6 +30653,9 @@ msgstr ""
 msgid "Requested %{time_ago}"
 msgstr ""
 
+msgid "Requested attention from %{users_sentence}."
+msgstr ""
+
 msgid "Requested attention from @%{username}"
 msgstr ""
 

spec/services/merge_requests/bulk_remove_attention_requested_service_spec.rb

@@ -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

spec/services/quick_actions/interpret_service_spec.rb

@@ -701,6 +701,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 }
@@ -2283,6 +2304,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