Merge branch '243563-move-approval-code-to-ce' into 'master'

Move approval MR filter and quick actions to CE

See merge request gitlab-org/gitlab!43326

app/assets/javascripts/filtered_search/add_extra_tokens_for_merge_requests.js

@@ -63,4 +63,47 @@ export default IssuableTokenKeys => {
 
   IssuableTokenKeys.tokenKeys.push(targetBranchToken);
   IssuableTokenKeys.tokenKeysWithAlternative.push(targetBranchToken);
+
+  const approvedBy = {
+    token: {
+      formattedKey: __('Approved-By'),
+      key: 'approved-by',
+      type: 'array',
+      param: 'usernames[]',
+      symbol: '@',
+      icon: 'approval',
+      tag: '@approved-by',
+    },
+    condition: [
+      {
+        url: 'approved_by_usernames[]=None',
+        tokenKey: 'approved-by',
+        value: __('None'),
+        operator: '=',
+      },
+      {
+        url: 'not[approved_by_usernames][]=None',
+        tokenKey: 'approved-by',
+        value: __('None'),
+        operator: '!=',
+      },
+      {
+        url: 'approved_by_usernames[]=Any',
+        tokenKey: 'approved-by',
+        value: __('Any'),
+        operator: '=',
+      },
+      {
+        url: 'not[approved_by_usernames][]=Any',
+        tokenKey: 'approved-by',
+        value: __('Any'),
+        operator: '!=',
+      },
+    ],
+  };
+
+  const tokenPosition = 2;
+  IssuableTokenKeys.tokenKeys.splice(tokenPosition, 0, ...[approvedBy.token]);
+  IssuableTokenKeys.tokenKeysWithAlternative.splice(tokenPosition, 0, ...[approvedBy.token]);
+  IssuableTokenKeys.conditions.push(...approvedBy.condition);
 };

app/assets/javascripts/filtered_search/available_dropdown_mappings.js

@@ -69,6 +69,11 @@ export default class AvailableDropdownMappings {
         gl: DropdownUser,
         element: this.container.querySelector('#js-dropdown-assignee'),
       },
+      'approved-by': {
+        reference: null,
+        gl: DropdownUser,
+        element: this.container.querySelector('#js-dropdown-approved-by'),
+      },
       milestone: {
         reference: null,
         gl: DropdownNonUser,

app/assets/javascripts/filtered_search/constants.js

-export const USER_TOKEN_TYPES = ['author', 'assignee'];
+export const USER_TOKEN_TYPES = ['author', 'assignee', 'approved-by'];
 
 export const DROPDOWN_TYPE = {
   hint: 'hint',

ee/app/finders/merge_requests/by_approvals_finder.rb → app/finders/merge_requests/by_approvals_finder.rb

@@ -47,7 +47,7 @@ module MergeRequests
 
     # Is param using special condition: "Any" ?
     #
-    # @return [Boolean] whether special condition "Any"" is being used
+    # @return [Boolean] whether special condition "Any" is being used
     def by_any_approvals?
       includes_special_label?(IssuableFinder::Params::FILTER_ANY)
     end

app/finders/merge_requests_finder.rb

@@ -33,7 +33,11 @@ class MergeRequestsFinder < IssuableFinder
   include MergedAtFilter
 
   def self.scalar_params
-    @scalar_params ||= super + [:wip, :draft, :target_branch, :merged_after, :merged_before]
+    @scalar_params ||= super + [:wip, :draft, :target_branch, :merged_after, :merged_before, :approved_by_ids]
+  end
+
+  def self.array_params
+    @array_params ||= super.merge(approved_by_usernames: [])
   end
 
   def klass
@@ -47,6 +51,7 @@ class MergeRequestsFinder < IssuableFinder
     items = by_draft(items)
     items = by_target_branch(items)
     items = by_merged_at(items)
+    items = by_approvals(items)
     by_source_project_id(items)
   end
 
@@ -131,6 +136,15 @@ class MergeRequestsFinder < IssuableFinder
   def deployment_id
     @deployment_id ||= params[:deployment_id].presence
   end
+
+  # Filter by merge requests that had been approved by specific users
+  # rubocop: disable CodeReuse/Finder
+  def by_approvals(items)
+    MergeRequests::ByApprovalsFinder
+      .new(params[:approved_by_usernames], params[:approved_by_ids])
+      .execute(items)
+  end
+  # rubocop: enable CodeReuse/Finder
 end
 
 MergeRequestsFinder.prepend_if_ee('EE::MergeRequestsFinder')

app/models/concerns/approvable_base.rb

@@ -2,10 +2,34 @@
 
 module ApprovableBase
   extend ActiveSupport::Concern
+  include FromUnion
 
   included do
     has_many :approvals, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent
     has_many :approved_by_users, through: :approvals, source: :user
+
+    scope :without_approvals, -> { left_outer_joins(:approvals).where(approvals: { id: nil }) }
+    scope :with_approvals, -> { joins(:approvals) }
+    scope :approved_by_users_with_ids, -> (*user_ids) do
+      with_approvals
+        .merge(Approval.with_user)
+        .where(users: { id: user_ids })
+        .group(:id)
+        .having("COUNT(users.id) = ?", user_ids.size)
+    end
+    scope :approved_by_users_with_usernames, -> (*usernames) do
+      with_approvals
+        .merge(Approval.with_user)
+        .where(users: { username: usernames })
+        .group(:id)
+        .having("COUNT(users.id) = ?", usernames.size)
+    end
+  end
+
+  class_methods do
+    def select_from_union(relations)
+      where(id: from_union(relations))
+    end
   end
 
   def approved_by?(user)

changelogs/unreleased/243563-move-approval-code-to-ce.yml

+---
+title: Move approval MR filter and quick actions to CE
+author: Pavel Kuznetsov
+merge_request: 43326
+type: changed

ee/app/assets/javascripts/filtered_search/add_extra_tokens_for_merge_requests.js

@@ -39,51 +39,11 @@ const approvers = {
   },
 };
 
-const approvedBy = {
-  condition: [
-    {
-      url: 'approved_by_usernames[]=None',
-      tokenKey: 'approved-by',
-      value: __('None'),
-      operator: '=',
-    },
-    {
-      url: 'not[approved_by_usernames][]=None',
-      tokenKey: 'approved-by',
-      value: __('None'),
-      operator: '!=',
-    },
-    {
-      url: 'approved_by_usernames[]=Any',
-      tokenKey: 'approved-by',
-      value: __('Any'),
-      operator: '=',
-    },
-    {
-      url: 'not[approved_by_usernames][]=Any',
-      tokenKey: 'approved-by',
-      value: __('Any'),
-      operator: '!=',
-    },
-  ],
-  token: {
-    formattedKey: __('Approved-By'),
-    key: 'approved-by',
-    type: 'array',
-    param: 'usernames[]',
-    symbol: '@',
-    icon: 'approval',
-    tag: '@approved-by',
-  },
-};
-
 export default IssuableTokenKeys => {
   addExtraTokensForMergeRequests(IssuableTokenKeys);
   const tokenPosition = 2;
-  const combinedTokens = [approvers.token, approvedBy.token];
-  const combinedConditions = [approvers.condition, approvedBy.condition];
 
-  IssuableTokenKeys.tokenKeys.splice(tokenPosition, 0, ...combinedTokens);
-  IssuableTokenKeys.tokenKeysWithAlternative.splice(tokenPosition, 0, ...combinedTokens);
-  IssuableTokenKeys.conditions.push(...combinedConditions);
+  IssuableTokenKeys.tokenKeys.splice(tokenPosition, 0, ...[approvers.token]);
+  IssuableTokenKeys.tokenKeysWithAlternative.splice(tokenPosition, 0, ...[approvers.token]);
+  IssuableTokenKeys.conditions.push(...approvers.condition);
 };

ee/app/assets/javascripts/filtered_search/available_dropdown_mappings.js

@@ -47,12 +47,6 @@ export default class AvailableDropdownMappings {
       element: this.container.querySelector('#js-dropdown-approver'),
     };
 
-    ceMappings['approved-by'] = {
-      reference: null,
-      gl: DropdownUser,
-      element: this.container.querySelector('#js-dropdown-approved-by'),
-    };
-
     ceMappings.weight = {
       reference: null,
       gl: DropdownWeight,

ee/app/assets/javascripts/filtered_search/constants.js

 import { USER_TOKEN_TYPES as CE_USER_TOKEN_TYPES } from '~/filtered_search/constants';
 
-export const USER_TOKEN_TYPES = [...CE_USER_TOKEN_TYPES, 'approver', 'approved-by'];
+export const USER_TOKEN_TYPES = [...CE_USER_TOKEN_TYPES, 'approver'];

ee/app/finders/ee/merge_requests_finder.rb

@@ -8,8 +8,7 @@ module EE
     override :filter_items
     def filter_items(items)
       items = super(items)
-      items = by_approvers(items)
-      by_approvals(items)
+      by_approvers(items)
     end
 
     # Filter by merge requests approval list that contains specified user directly or as part of group membership
@@ -19,24 +18,17 @@ module EE
         .execute(items)
     end
 
-    # Filter by merge requests that had been approved by specific users
-    def by_approvals(items)
-      ::MergeRequests::ByApprovalsFinder
-        .new(params[:approved_by_usernames], params[:approved_by_ids])
-        .execute(items)
-    end
-
     class_methods do
       extend ::Gitlab::Utils::Override
 
       override :scalar_params
       def scalar_params
-        @scalar_params ||= super + [:approver_ids, :approved_by_ids]
+        @scalar_params ||= super + [:approver_ids]
       end
 
       override :array_params
       def array_params
-        @array_params ||= super.merge(approver_usernames: [], approved_by_usernames: [])
+        @array_params ||= super.merge(approver_usernames: [])
       end
     end
   end

ee/app/models/ee/merge_request.rb

@@ -53,23 +53,6 @@ module EE
       delegate :merge_requests_author_approval?, to: :target_project, allow_nil: true
       delegate :merge_requests_disable_committers_approval?, to: :target_project, allow_nil: true
 
-      scope :without_approvals, -> { left_outer_joins(:approvals).where(approvals: { id: nil }) }
-      scope :with_approvals, -> { joins(:approvals) }
-      scope :approved_by_users_with_ids, -> (*user_ids) do
-        with_approvals
-          .merge(Approval.with_user)
-          .where(users: { id: user_ids })
-          .group(:id)
-          .having("COUNT(users.id) = ?", user_ids.size)
-      end
-      scope :approved_by_users_with_usernames, -> (*usernames) do
-        with_approvals
-          .merge(Approval.with_user)
-          .where(users: { username: usernames })
-          .group(:id)
-          .having("COUNT(users.id) = ?", usernames.size)
-      end
-
       accepts_nested_attributes_for :approval_rules, allow_destroy: true
 
       scope :order_review_time_desc, -> do
@@ -89,10 +72,6 @@ module EE
     end
 
     class_methods do
-      def select_from_union(relations)
-        where(id: from_union(relations))
-      end
-
       # This is an ActiveRecord scope in CE
       def with_api_entity_associations
         super.preload(:blocking_merge_requests)

ee/app/services/ee/quick_actions/interpret_service.rb

@@ -10,7 +10,6 @@ module EE
         # rubocop: disable Cop/InjectEnterpriseEditionModule
         include EE::Gitlab::QuickActions::EpicActions
         include EE::Gitlab::QuickActions::IssueActions
-        include EE::Gitlab::QuickActions::MergeRequestActions
         include EE::Gitlab::QuickActions::IssueAndMergeRequestActions
         # rubocop: enable Cop/InjectEnterpriseEditionModule
       end

ee/lib/ee/gitlab/quick_actions/merge_request_actions.rb

-# frozen_string_literal: true
-
-module EE
-  module Gitlab
-    module QuickActions
-      module MergeRequestActions
-        extend ActiveSupport::Concern
-        include ::Gitlab::QuickActions::Dsl
-
-        included do
-          desc _('Approve a merge request')
-          explanation _('Approve the current merge request.')
-          types MergeRequest
-          condition do
-            quick_action_target.persisted? && quick_action_target.can_approve?(current_user) && !quick_action_target.project.require_password_to_approve?
-          end
-          command :approve do
-            if quick_action_target.can_approve?(current_user)
-              ::MergeRequests::ApprovalService.new(quick_action_target.project, current_user).execute(quick_action_target)
-              @execution_message[:approve] = _('Approved the current merge request.')
-            end
-          end
-        end
-      end
-    end
-  end
-end

lib/gitlab/quick_actions/merge_request_actions.rb

@@ -121,6 +121,20 @@ module Gitlab
                                                  result[:message]
                                                end
         end
+
+        desc _('Approve a merge request')
+        explanation _('Approve the current merge request.')
+        types MergeRequest
+        condition do
+          quick_action_target.persisted? && quick_action_target.can_be_approved_by?(current_user)
+        end
+        command :approve do
+          success = MergeRequests::ApprovalService.new(quick_action_target.project, current_user).execute(quick_action_target)
+
+          next unless success
+
+          @execution_message[:approve] = _('Approved the current merge request.')
+        end
       end
 
       def merge_orchestration_service