Merge branch 'unappprove_quick_action' into 'master'

Added unapprove quick-action

See merge request gitlab-org/gitlab!69225

app/models/concerns/approvable_base.rb

@@ -54,4 +54,8 @@ module ApprovableBase
   def can_be_approved_by?(user)
     user && !approved_by?(user) && user.can?(:approve_merge_request, self)
   end
+
+  def can_be_unapproved_by?(user)
+    user && approved_by?(user) && user.can?(:approve_merge_request, self)
+  end
 end

config/metrics/counts_28d/20210902191057_i_quickactions_unapprove_monthly.yml

+---
+data_category: optional
+key_path: redis_hll_counters.quickactions.i_quickactions_unapprove_monthly
+description: Count of MAU using the `/unapprove` quick action
+product_section: dev
+product_stage: create
+product_group: group::code review
+product_category: code_review
+value_type: number
+status: data_available
+milestone: "14.3"
+time_frame: 28d
+data_source: redis_hll
+instrumentation_class: RedisHLLMetric
+options:
+  events:
+    - i_quickactions_unapprove
+instrumentation_class: RedisHLLMetric
+options:
+  events:
+    - i_quickactions_unapprove
+distribution:
+  - ce
+  - ee
+tier:
+  - free
+  - premium
+  - ultimate

config/metrics/counts_7d/20210902191054_i_quickactions_unapprove_weekly.yml

+---
+data_category: optional
+key_path: redis_hll_counters.quickactions.i_quickactions_unapprove_weekly
+description: Count of WAU using the `/unapprove` quick action
+product_section: dev
+product_stage: create
+product_group: group::code review
+product_category: code_review
+value_type: number
+status: data_available
+milestone: "14.3"
+time_frame: 7d
+data_source: redis_hll
+instrumentation_class: RedisHLLMetric
+options:
+  events:
+    - i_quickactions_unapprove
+instrumentation_class: RedisHLLMetric
+options:
+  events:
+    - i_quickactions_unapprove
+distribution:
+  - ce
+  - ee
+tier:
+  - free
+  - premium
+  - ultimate

doc/user/project/quick_actions.md

@@ -103,6 +103,7 @@ threads. Some quick actions might not be available to all subscription tiers.
 | `/target_branch <local branch name>`  | **{dotted-circle}** No | **{check-circle}** Yes | **{dotted-circle}** No | Set target branch. |
 | `/title <new title>`                  | **{check-circle}** Yes | **{check-circle}** Yes | **{check-circle}** Yes | Change title. |
 | `/todo`                               | **{check-circle}** Yes | **{check-circle}** Yes | **{check-circle}** Yes | Add a to-do item. |
+| `/unapprove`                          | **{dotted-circle}** No | **{check-circle}** Yes | **{dotted-circle}** No | Unapprove the merge request. ([introduced in GitLab 14.3](https://gitlab.com/gitlab-org/gitlab/-/issues/8003)|
 | `/unassign @user1 @user2`             | **{check-circle}** Yes | **{check-circle}** Yes | **{dotted-circle}** No | Remove specific assignees. |
 | `/unassign`                           | **{dotted-circle}** No | **{check-circle}** Yes | **{dotted-circle}** No | Remove all assignees. |
 | `/unassign_reviewer @user1 @user2` or `/remove_reviewer @user1 @user2`    | **{dotted-circle}** No | **{check-circle}** Yes | **{dotted-circle}** No | Remove specific reviewers. |

ee/spec/services/quick_actions/interpret_service_spec.rb

@@ -952,30 +952,6 @@ RSpec.describe QuickActions::InterpretService do
       end
     end
 
-    context 'approve command' do
-      let(:merge_request) { create(:merge_request, source_project: project) }
-      let(:content) { '/approve' }
-
-      it 'approves the current merge request' do
-        service.execute(content, merge_request)
-
-        expect(merge_request.approved_by_users).to eq([current_user])
-      end
-
-      context "when the user can't approve" do
-        before do
-          project.team.truncate
-          project.add_guest(current_user)
-        end
-
-        it 'does not approve the MR' do
-          service.execute(content, merge_request)
-
-          expect(merge_request.approved_by_users).to be_empty
-        end
-      end
-    end
-
     shared_examples 'weight command' do
       it 'populates weight specified by the /weight command' do
         _, updates = service.execute(content, issuable)

lib/gitlab/quick_actions/merge_request_actions.rb

@@ -155,6 +155,20 @@ module Gitlab
           @execution_message[:approve] = _('Approved the current merge request.')
         end
 
+        desc _('Unapprove a merge request')
+        explanation _('Unapprove the current merge request.')
+        types MergeRequest
+        condition do
+          quick_action_target.persisted? && quick_action_target.can_be_unapproved_by?(current_user)
+        end
+        command :unapprove do
+          success = MergeRequests::RemoveApprovalService.new(project: quick_action_target.project, current_user: current_user).execute(quick_action_target)
+
+          next unless success
+
+          @execution_message[:unapprove] = _('Unapproved the current merge request.')
+        end
+
         desc do
           if quick_action_target.allows_multiple_reviewers?
             _('Assign reviewer(s)')

lib/gitlab/usage_data_counters/known_events/quickactions.yml

@@ -3,6 +3,10 @@
   category: quickactions
   redis_slot: quickactions
   aggregation: weekly
+- name: i_quickactions_unapprove
+  category: quickactions
+  redis_slot: quickactions
+  aggregation: weekly
 - name: i_quickactions_assign_single
   category: quickactions
   redis_slot: quickactions

locale/gitlab.pot

@@ -35727,6 +35727,15 @@ msgstr ""
 msgid "Unable to update this issue at this time."
 msgstr ""
 
+msgid "Unapprove a merge request"
+msgstr ""
+
+msgid "Unapprove the current merge request."
+msgstr ""
+
+msgid "Unapproved the current merge request."
+msgstr ""
+
 msgid "Unarchive project"
 msgstr ""
 

spec/models/concerns/approvable_base_spec.rb

@@ -60,6 +60,34 @@ RSpec.describe ApprovableBase do
     end
   end
 
+  describe '#can_be_unapproved_by?' do
+    subject { merge_request.can_be_unapproved_by?(user) }
+
+    before do
+      merge_request.project.add_developer(user)
+    end
+
+    it 'returns false' do
+      is_expected.to be_falsy
+    end
+
+    context 'when a user has approved' do
+      let!(:approval) { create(:approval, merge_request: merge_request, user: user) }
+
+      it 'returns true' do
+        is_expected.to be_truthy
+      end
+    end
+
+    context 'when a user is nil' do
+      let(:user) { nil }
+
+      it 'returns false' do
+        is_expected.to be_falsy
+      end
+    end
+  end
+
   describe '.not_approved_by_users_with_usernames' do
     subject { MergeRequest.not_approved_by_users_with_usernames([user.username, user2.username]) }
 

spec/services/quick_actions/interpret_service_spec.rb

@@ -624,6 +624,18 @@ RSpec.describe QuickActions::InterpretService do
       end
     end
 
+    shared_examples 'approve command unavailable' do
+      it 'is not part of the available commands' do
+        expect(service.available_commands(issuable)).not_to include(a_hash_including(name: :approve))
+      end
+    end
+
+    shared_examples 'unapprove command unavailable' do
+      it 'is not part of the available commands' do
+        expect(service.available_commands(issuable)).not_to include(a_hash_including(name: :unapprove))
+      end
+    end
+
     shared_examples 'shrug command' do
       it 'appends ¯\_(ツ)_/¯ to the comment' do
         new_content, _, _ = service.execute(content, issuable)
@@ -2135,6 +2147,66 @@ RSpec.describe QuickActions::InterpretService do
         end
       end
     end
+
+    context 'approve command' do
+      let(:merge_request) { create(:merge_request, source_project: project) }
+      let(:content) { '/approve' }
+
+      it 'approves the current merge request' do
+        service.execute(content, merge_request)
+
+        expect(merge_request.approved_by_users).to eq([developer])
+      end
+
+      context "when the user can't approve" do
+        before do
+          project.team.truncate
+          project.add_guest(developer)
+        end
+
+        it 'does not approve the MR' do
+          service.execute(content, merge_request)
+
+          expect(merge_request.approved_by_users).to be_empty
+        end
+      end
+
+      it_behaves_like 'approve command unavailable' do
+        let(:issuable) { issue }
+      end
+    end
+
+    context 'unapprove command' do
+      let!(:merge_request) { create(:merge_request, source_project: project) }
+      let(:content) { '/unapprove' }
+
+      before do
+        service.execute('/approve', merge_request)
+      end
+
+      it 'unapproves the current merge request' do
+        service.execute(content, merge_request)
+
+        expect(merge_request.approved_by_users).to be_empty
+      end
+
+      context "when the user can't unapprove" do
+        before do
+          project.team.truncate
+          project.add_guest(developer)
+        end
+
+        it 'does not unapprove the MR' do
+          service.execute(content, merge_request)
+
+          expect(merge_request.approved_by_users).to eq([developer])
+        end
+
+        it_behaves_like 'unapprove command unavailable' do
+          let(:issuable) { issue }
+        end
+      end
+    end
   end
 
   describe '#explain' do