From e28176bf851608d8c9194447b456f040a5bd11bf Mon Sep 17 00:00:00 2001
From: Phil Hughes <me@iamphill.com>
Date: Mon, 4 Jul 2022 11:56:22 +0100
Subject: [PATCH 1/4] Added approval checkbox to submit review dropdown

https://gitlab.com/gitlab-org/gitlab/-/issues/10869
---
 .../components/submit_dropdown.vue            | 84 ++++++++++++++-----
 .../stores/modules/batch_comments/actions.js  |  6 +-
 .../notes/components/note_header.vue          |  2 +-
 .../page_bundles/merge_requests.scss          |  4 +-
 .../merge_requests/drafts_controller.rb       | 18 +++-
 .../merge_request_noteable_entity.rb          |  6 ++
 .../components/approval_password.vue          | 34 ++++++++
 .../merge_requests/drafts_controller.rb       | 17 ++++
 .../ee/merge_request_noteable_entity.rb       | 19 +++++
 .../merge_request/draft_comments_spec.rb      | 80 ++++++++++++++++++
 .../entities/merge_request_noteable.json      | 42 ++++++++++
 .../components/submit_dropdown_spec.js        | 64 ++++++++++++++
 .../__snapshots__/event_item_spec.js.snap     |  3 +-
 locale/gitlab.pot                             | 12 +++
 .../merge_requests/drafts_controller_spec.rb  | 34 +++++++-
 .../entities/merge_request_noteable.json      |  6 +-
 .../components/submit_dropdown_spec.js        | 20 ++++-
 .../modules/batch_comments/actions_spec.js    |  3 +-
 18 files changed, 419 insertions(+), 35 deletions(-)
 create mode 100644 ee/app/assets/javascripts/batch_comments/components/approval_password.vue
 create mode 100644 ee/app/controllers/ee/projects/merge_requests/drafts_controller.rb
 create mode 100644 ee/app/serializers/ee/merge_request_noteable_entity.rb
 create mode 100644 ee/spec/features/merge_request/draft_comments_spec.rb
 create mode 100644 ee/spec/fixtures/api/schemas/entities/merge_request_noteable.json
 create mode 100644 ee/spec/frontend/batch_comments/components/submit_dropdown_spec.js

diff --git a/app/assets/javascripts/batch_comments/components/submit_dropdown.vue b/app/assets/javascripts/batch_comments/components/submit_dropdown.vue
index 54b9953270b856..bb9ec43c6b1881 100644
--- a/app/assets/javascripts/batch_comments/components/submit_dropdown.vue
+++ b/app/assets/javascripts/batch_comments/components/submit_dropdown.vue
@@ -1,7 +1,16 @@
 <script>
 import $ from 'jquery';
-import { GlDropdown, GlButton, GlIcon, GlForm, GlFormGroup, GlLink } from '@gitlab/ui';
+import {
+  GlDropdown,
+  GlButton,
+  GlIcon,
+  GlForm,
+  GlFormGroup,
+  GlLink,
+  GlFormCheckbox,
+} from '@gitlab/ui';
 import { mapGetters, mapActions } from 'vuex';
+import { createAlert } from '~/flash';
 import MarkdownField from '~/vue_shared/components/markdown/field.vue';
 import { scrollToElement } from '~/lib/utils/common_utils';
 import Autosave from '~/autosave';
@@ -15,29 +24,46 @@ export default {
     GlForm,
     GlFormGroup,
     GlLink,
+    GlFormCheckbox,
     MarkdownField,
+    ApprovalPassword: () => import('ee_component/batch_comments/components/approval_password.vue'),
   },
   data() {
     return {
       isSubmitting: false,
-      note: '',
+      noteData: {
+        noteable_type: '',
+        noteable_id: '',
+        note: '',
+        approve: false,
+        approval_password: '',
+      },
     };
   },
   computed: {
     ...mapGetters(['getNotesData', 'getNoteableData', 'noteableType', 'getCurrentUserLastNote']),
   },
+  watch: {
+    'noteData.approve': function noteDataApproveWatch() {
+      setTimeout(() => {
+        this.repositionDropdown();
+      });
+    },
+  },
   mounted() {
     this.autosave = new Autosave(
       $(this.$refs.textarea),
       `submit_review_dropdown/${this.getNoteableData.id}`,
     );
+    this.noteData.noteable_type = this.noteableType;
+    this.noteData.noteable_id = this.getNoteableData.id;
 
     // We override the Bootstrap Vue click outside behaviour
     // to allow for clicking in the autocomplete dropdowns
     // without this override the submit dropdown will close
     // whenever a item in the autocomplete dropdown is clicked
-    const originalClickOutHandler = this.$refs.dropdown.$refs.dropdown.clickOutHandler;
-    this.$refs.dropdown.$refs.dropdown.clickOutHandler = (e) => {
+    const originalClickOutHandler = this.$refs.submitDropdown.$refs.dropdown.clickOutHandler;
+    this.$refs.submitDropdown.$refs.dropdown.clickOutHandler = (e) => {
       if (!e.target.closest('.atwho-container')) {
         originalClickOutHandler(e);
       }
@@ -45,26 +71,29 @@ export default {
   },
   methods: {
     ...mapActions('batchComments', ['publishReview']),
+    repositionDropdown() {
+      this.$refs.submitDropdown?.$refs.dropdown?.updatePopper();
+    },
     async submitReview() {
-      const noteData = {
-        noteable_type: this.noteableType,
-        noteable_id: this.getNoteableData.id,
-        note: this.note,
-      };
-
       this.isSubmitting = true;
 
-      await this.publishReview(noteData);
+      try {
+        await this.publishReview(this.noteData);
 
-      this.autosave.reset();
+        this.autosave.reset();
 
-      if (window.mrTabs && this.note) {
-        window.location.hash = `note_${this.getCurrentUserLastNote.id}`;
-        window.mrTabs.tabShown('show');
+        if (window.mrTabs && this.note) {
+          window.location.hash = `note_${this.getCurrentUserLastNote.id}`;
+          window.mrTabs.tabShown('show');
 
-        setTimeout(() =>
-          scrollToElement(document.getElementById(`note_${this.getCurrentUserLastNote.id}`)),
-        );
+          setTimeout(() =>
+            scrollToElement(document.getElementById(`note_${this.getCurrentUserLastNote.id}`)),
+          );
+        }
+      } catch (e) {
+        if (e.data?.message) {
+          createAlert({ message: e.data.message });
+        }
       }
 
       this.isSubmitting = false;
@@ -79,8 +108,9 @@ export default {
 
 <template>
   <gl-dropdown
-    ref="dropdown"
+    ref="submitDropdown"
     right
+    dropup
     class="submit-review-dropdown"
     data-qa-selector="submit_review_dropdown"
     variant="info"
@@ -110,7 +140,7 @@ export default {
             <markdown-field
               :is-submitting="isSubmitting"
               :add-spacing-classes="false"
-              :textarea-value="note"
+              :textarea-value="noteData.note"
               :markdown-preview-path="getNoteableData.preview_note_path"
               :markdown-docs-path="getNotesData.markdownDocsPath"
               :quick-actions-docs-path="getNotesData.quickActionsDocsPath"
@@ -122,7 +152,7 @@ export default {
                 <textarea
                   id="review-note-body"
                   ref="textarea"
-                  v-model="note"
+                  v-model="noteData.note"
                   dir="auto"
                   :disabled="isSubmitting"
                   name="review[note]"
@@ -139,6 +169,18 @@ export default {
           </div>
         </div>
       </gl-form-group>
+      <template v-if="getNoteableData.current_user.can_approve">
+        <gl-form-checkbox v-model="noteData.approve" data-testid="approve_merge_request">
+          {{ __('Approve merge request') }}
+        </gl-form-checkbox>
+        <approval-password
+          v-if="getNoteableData.require_password_to_approve"
+          v-show="noteData.approve"
+          v-model="noteData.approval_password"
+          class="gl-mt-3"
+          data-testid="approve_password"
+        />
+      </template>
       <div class="gl-display-flex gl-justify-content-start gl-mt-5">
         <gl-button
           :loading="isSubmitting"
diff --git a/app/assets/javascripts/batch_comments/stores/modules/batch_comments/actions.js b/app/assets/javascripts/batch_comments/stores/modules/batch_comments/actions.js
index a44b9827fe9b57..2b0aaa74e8397e 100644
--- a/app/assets/javascripts/batch_comments/stores/modules/batch_comments/actions.js
+++ b/app/assets/javascripts/batch_comments/stores/modules/batch_comments/actions.js
@@ -84,7 +84,11 @@ export const publishReview = ({ commit, dispatch, getters }, noteData = {}) => {
     .publish(getters.getNotesData.draftsPublishPath, noteData)
     .then(() => dispatch('updateDiscussionsAfterPublish'))
     .then(() => commit(types.RECEIVE_PUBLISH_REVIEW_SUCCESS))
-    .catch(() => commit(types.RECEIVE_PUBLISH_REVIEW_ERROR));
+    .catch((e) => {
+      commit(types.RECEIVE_PUBLISH_REVIEW_ERROR);
+
+      throw e.response;
+    });
 };
 
 export const updateDiscussionsAfterPublish = async ({ dispatch, getters, rootGetters }) => {
diff --git a/app/assets/javascripts/notes/components/note_header.vue b/app/assets/javascripts/notes/components/note_header.vue
index 9917249f0dbf66..fb50884b1293bb 100644
--- a/app/assets/javascripts/notes/components/note_header.vue
+++ b/app/assets/javascripts/notes/components/note_header.vue
@@ -183,7 +183,6 @@ export default {
         :data-user-id="author.id"
         :data-username="author.username"
       >
-        <slot name="note-header-info"></slot>
         <user-name-with-status
           :name="authorName"
           :availability="userAvailability(author)"
@@ -207,6 +206,7 @@ export default {
           @mouseleave="handleUsernameMouseLeave"
           ><span class="note-headline-light">@{{ author.username }}</span>
         </a>
+        <slot name="note-header-info"></slot>
         <gitlab-team-member-badge v-if="author && author.is_gitlab_employee" />
       </span>
     </template>
diff --git a/app/assets/stylesheets/page_bundles/merge_requests.scss b/app/assets/stylesheets/page_bundles/merge_requests.scss
index 05397906b764ac..baed36943066e1 100644
--- a/app/assets/stylesheets/page_bundles/merge_requests.scss
+++ b/app/assets/stylesheets/page_bundles/merge_requests.scss
@@ -771,6 +771,7 @@ $tabs-holder-z-index: 250;
   &.show .dropdown-menu {
     width: calc(100vw - 20px);
     max-width: 650px;
+    max-height: calc(100vh - 50px);
 
     .gl-new-dropdown-inner {
       max-height: none !important;
@@ -812,8 +813,7 @@ $tabs-holder-z-index: 250;
   }
 
   .md-preview-holder {
-    max-height: 180px;
-    height: 180px;
+    max-height: 172px;
   }
 }
 
diff --git a/app/controllers/projects/merge_requests/drafts_controller.rb b/app/controllers/projects/merge_requests/drafts_controller.rb
index ff6b6bfaf274e5..58d462e8c9f737 100644
--- a/app/controllers/projects/merge_requests/drafts_controller.rb
+++ b/app/controllers/projects/merge_requests/drafts_controller.rb
@@ -49,8 +49,16 @@ def destroy
   def publish
     result = DraftNotes::PublishService.new(merge_request, current_user).execute(draft_note(allow_nil: true))
 
-    if Feature.enabled?(:mr_review_submit_comment, @project) && create_note_params[:note]
-      Notes::CreateService.new(@project, current_user, create_note_params).execute
+    if Feature.enabled?(:mr_review_submit_comment, @project)
+      ::Notes::CreateService.new(@project, current_user, create_note_params).execute if create_note_params[:note]
+
+      if Gitlab::Utils.to_boolean(approve_params[:approve])
+        success = ::MergeRequests::ApprovalService.new(project: @project, current_user: current_user, params: approve_params).execute(merge_request)
+
+        unless success
+          return render json: { message: _('An error occurred while approving, please try again.') }, status: :internal_server_error
+        end
+      end
     end
 
     if result[:status] == :success
@@ -115,6 +123,10 @@ def create_note_params
     end
   end
 
+  def approve_params
+    params.permit(:approve)
+  end
+
   def prepare_notes_for_rendering(notes)
     return [] unless notes
 
@@ -148,3 +160,5 @@ def authorize_create_note!
     access_denied! unless can?(current_user, :create_note, merge_request)
   end
 end
+
+Projects::MergeRequests::DraftsController.prepend_mod
diff --git a/app/serializers/merge_request_noteable_entity.rb b/app/serializers/merge_request_noteable_entity.rb
index f8c8e3538daa45..8154865d51d639 100644
--- a/app/serializers/merge_request_noteable_entity.rb
+++ b/app/serializers/merge_request_noteable_entity.rb
@@ -40,6 +40,10 @@ class MergeRequestNoteableEntity < IssuableEntity
     expose :can_update do |merge_request|
       can?(current_user, :update_merge_request, merge_request)
     end
+
+    expose :can_approve do |merge_request|
+      can?(current_user, :approve_merge_request, merge_request)
+    end
   end
 
   expose :locked_discussion_docs_path, if: -> (merge_request) { merge_request.discussion_locked? } do |merge_request|
@@ -65,3 +69,5 @@ def presenter(merge_request)
     @presenters[merge_request] ||= MergeRequestPresenter.new(merge_request, current_user: current_user) # rubocop: disable CodeReuse/Presenter
   end
 end
+
+MergeRequestNoteableEntity.prepend_mod_with('MergeRequestNoteableEntity')
diff --git a/ee/app/assets/javascripts/batch_comments/components/approval_password.vue b/ee/app/assets/javascripts/batch_comments/components/approval_password.vue
new file mode 100644
index 00000000000000..5760adb0c9b634
--- /dev/null
+++ b/ee/app/assets/javascripts/batch_comments/components/approval_password.vue
@@ -0,0 +1,34 @@
+<script>
+import { GlFormGroup, GlFormInput } from '@gitlab/ui';
+
+export default {
+  components: {
+    GlFormGroup,
+    GlFormInput,
+  },
+  props: {
+    value: {
+      type: String,
+      required: true,
+    },
+  },
+};
+</script>
+
+<template>
+  <gl-form-group
+    :label="__('Your password')"
+    :label-description="
+      __(
+        'To approve this merge request, please enter your password. This project requires all approvals to be authenticated.',
+      )
+    "
+  >
+    <gl-form-input
+      type="password"
+      :value="value"
+      :placeholder="__('Password')"
+      @input="(e) => $emit('input', e)"
+    />
+  </gl-form-group>
+</template>
diff --git a/ee/app/controllers/ee/projects/merge_requests/drafts_controller.rb b/ee/app/controllers/ee/projects/merge_requests/drafts_controller.rb
new file mode 100644
index 00000000000000..10b21fd9dbd5ce
--- /dev/null
+++ b/ee/app/controllers/ee/projects/merge_requests/drafts_controller.rb
@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+module EE
+  module Projects
+    module MergeRequests
+      module DraftsController
+        extend ActiveSupport::Concern
+
+        private
+
+        def approve_params
+          super.merge(params.permit(:approval_password))
+        end
+      end
+    end
+  end
+end
diff --git a/ee/app/serializers/ee/merge_request_noteable_entity.rb b/ee/app/serializers/ee/merge_request_noteable_entity.rb
new file mode 100644
index 00000000000000..a9b1ea3c02b203
--- /dev/null
+++ b/ee/app/serializers/ee/merge_request_noteable_entity.rb
@@ -0,0 +1,19 @@
+# frozen_string_literal: true
+
+module EE
+  module MergeRequestNoteableEntity
+    extend ActiveSupport::Concern
+
+    prepended do
+      expose :require_password_to_approve do |merge_request|
+        merge_request.target_project.require_password_to_approve?
+      end
+
+      expose :current_user do
+        expose :can_approve do |merge_request|
+          merge_request.can_approve?(current_user)
+        end
+      end
+    end
+  end
+end
diff --git a/ee/spec/features/merge_request/draft_comments_spec.rb b/ee/spec/features/merge_request/draft_comments_spec.rb
new file mode 100644
index 00000000000000..4e19075b2faa04
--- /dev/null
+++ b/ee/spec/features/merge_request/draft_comments_spec.rb
@@ -0,0 +1,80 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe 'Merge request > Batch comments', :js do
+  include MergeRequestDiffHelpers
+  include RepoHelpers
+
+  let(:project) { create(:project, :public, :repository, merge_requests_author_approval: true) }
+  let(:current_user) { project.owner }
+  let(:merge_request) do
+    create(:merge_request_with_diffs, source_project: project, target_project: project, source_branch: 'merge-test')
+  end
+
+  before do
+    create(:draft_note, merge_request: merge_request, author: current_user)
+
+    sign_in(current_user)
+
+    visit project_merge_request_path(merge_request.project, merge_request)
+
+    wait_for_requests
+  end
+
+  context 'approval' do
+    context 'user does not have permission to approve' do
+      let(:current_user) { create(:user) }
+
+      it 'does not allow user to approve' do
+        click_button 'Finish review'
+
+        expect(page).not_to have_selector('[data-testid="approve_merge_request"]')
+      end
+    end
+
+    context 'user has permission to approve' do
+      it 'allows user to approve' do
+        click_button 'Finish review'
+
+        find('[data-testid="approve_merge_request"]').click
+        click_button 'Submit review'
+
+        wait_for_requests
+
+        expect(page).to have_content('approved this merge request')
+      end
+
+      context 'require password for approval' do
+        let(:project) do
+          # rubocop:disable Layout/LineLength
+          create(:project, :public, :repository, require_password_to_approve: true, merge_requests_author_approval: true)
+          # rubocop:enable Layout/LineLength
+        end
+
+        it 'does not allow user to approve without password' do
+          click_button 'Finish review'
+
+          find('[data-testid="approve_merge_request"]').click
+          click_button 'Submit review'
+
+          wait_for_requests
+
+          expect(page).to have_content('An error occurred while approving, please try again.')
+        end
+
+        it 'allows user to approve' do
+          click_button 'Finish review'
+
+          find('[data-testid="approve_merge_request"]').click
+          fill_in(type: 'password', with: '12345678')
+          click_button 'Submit review'
+
+          wait_for_requests
+
+          expect(page).to have_content('approved this merge request')
+        end
+      end
+    end
+  end
+end
diff --git a/ee/spec/fixtures/api/schemas/entities/merge_request_noteable.json b/ee/spec/fixtures/api/schemas/entities/merge_request_noteable.json
new file mode 100644
index 00000000000000..570c06f7542802
--- /dev/null
+++ b/ee/spec/fixtures/api/schemas/entities/merge_request_noteable.json
@@ -0,0 +1,42 @@
+{
+  "type": "object",
+  "required": ["id", "iid", "title", "description", "merge_params", "state", "source_branch", "target_branch",
+    "diff_head_sha", "create_note_path", "preview_note_path", "can_receive_suggestion", "create_issue_to_resolve_discussions_path",
+    "new_blob_path", "current_user", "is_project_archived"],
+  "properties": {
+    "id": { "type": "integer" },
+    "iid": { "type": "integer" },
+    "title": { "type": "string" },
+    "description": { "type": "string" },
+    "merge_params": { "type": ["object", "null"] },
+    "state": { "type": "string" },
+    "source_branch": { "type": "string" },
+    "target_branch": { "type": "string" },
+    "diff_head_sha": { "type": "string" },
+    "create_note_path": { "type": ["string", "null"] },
+    "preview_note_path": { "type": ["string", "null"] },
+    "create_issue_to_resolve_discussions_path": { "type": ["string", "null"] },
+    "new_blob_path": { "type": ["string", "null"] },
+    "can_receive_suggestion": { "type": "boolean" },
+    "current_user": {
+      "type": "object",
+      "required": [
+        "can_create_note",
+        "can_update",
+        "can_approve"
+      ],
+      "properties": {
+        "can_create_note": { "type": "boolean" },
+        "can_update": { "type": "boolean" },
+        "can_approve": { "type": "boolean" }
+      },
+      "additionalProperties": false
+    },
+    "is_project_archived": { "type":  "boolean" },
+    "locked_discussion_docs_path": { "type":  "string" },
+    "archived_project_docs_path": { "type":  "string" },
+    "project_id": { "type": "integer"},
+    "require_password_to_approve": { "type": "boolean" }
+  },
+  "additionalProperties": false
+}
diff --git a/ee/spec/frontend/batch_comments/components/submit_dropdown_spec.js b/ee/spec/frontend/batch_comments/components/submit_dropdown_spec.js
new file mode 100644
index 00000000000000..c1ae716420abf9
--- /dev/null
+++ b/ee/spec/frontend/batch_comments/components/submit_dropdown_spec.js
@@ -0,0 +1,64 @@
+import Vue from 'vue';
+import Vuex from 'vuex';
+import { mountExtended } from 'helpers/vue_test_utils_helper';
+import waitForPromises from 'helpers/wait_for_promises';
+import SubmitDropdown from '~/batch_comments/components/submit_dropdown.vue';
+
+Vue.use(Vuex);
+
+let wrapper;
+let publishReview;
+
+function factory({ canApprove = true, requirePasswordToApprove = true } = {}) {
+  publishReview = jest.fn();
+
+  const store = new Vuex.Store({
+    getters: {
+      getNotesData: () => ({
+        markdownDocsPath: '/markdown/docs',
+        quickActionsDocsPath: '/quickactions/docs',
+      }),
+      getNoteableData: () => ({
+        id: 1,
+        preview_note_path: '/preview',
+        current_user: {
+          can_approve: canApprove,
+        },
+        require_password_to_approve: requirePasswordToApprove,
+      }),
+      noteableType: () => 'merge_request',
+    },
+    modules: {
+      batchComments: {
+        namespaced: true,
+        actions: {
+          publishReview,
+        },
+      },
+    },
+  });
+  wrapper = mountExtended(SubmitDropdown, {
+    store,
+  });
+}
+
+describe('Batch comments submit dropdown', () => {
+  afterEach(() => {
+    wrapper.destroy();
+  });
+
+  it.each`
+    requirePasswordToApprove | exists   | existsText
+    ${true}                  | ${true}  | ${'shows'}
+    ${false}                 | ${false} | ${'hides'}
+  `(
+    'it $existsText approve password if require_password_to_approve is $requirePasswordToApprove',
+    async ({ requirePasswordToApprove, exists }) => {
+      factory({ requirePasswordToApprove });
+
+      await waitForPromises();
+
+      expect(wrapper.findByTestId('approve_password').exists()).toBe(exists);
+    },
+  );
+});
diff --git a/ee/spec/frontend/vue_shared/security_reports/components/__snapshots__/event_item_spec.js.snap b/ee/spec/frontend/vue_shared/security_reports/components/__snapshots__/event_item_spec.js.snap
index cbced6a0809cd7..b8a06c1e0cb376 100644
--- a/ee/spec/frontend/vue_shared/security_reports/components/__snapshots__/event_item_spec.js.snap
+++ b/ee/spec/frontend/vue_shared/security_reports/components/__snapshots__/event_item_spec.js.snap
@@ -32,7 +32,6 @@ exports[`Event Item with action buttons renders the action buttons 1`] = `
         class="author-name-link js-user-link"
         data-username="gitlab"
       >
-         
         <span
           class="note-header-author-name gl-font-weight-bold"
         >
@@ -57,7 +56,7 @@ exports[`Event Item with action buttons renders the action buttons 1`] = `
             @gitlab
           </span>
         </a>
-         
+          
         <!---->
       </span>
        
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 628e6e1af6461c..ea17959b300c6b 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -4035,6 +4035,9 @@ msgstr ""
 msgid "An error occurred while adding formatted title for epic"
 msgstr ""
 
+msgid "An error occurred while approving, please try again."
+msgstr ""
+
 msgid "An error occurred while authorizing your role"
 msgstr ""
 
@@ -4873,6 +4876,9 @@ msgstr ""
 msgid "Approve a merge request"
 msgstr ""
 
+msgid "Approve merge request"
+msgstr ""
+
 msgid "Approve the current merge request."
 msgstr ""
 
@@ -40838,6 +40844,9 @@ msgstr ""
 msgid "To add the entry manually, provide the following details to the application on your phone."
 msgstr ""
 
+msgid "To approve this merge request, please enter your password. This project requires all approvals to be authenticated."
+msgstr ""
+
 msgid "To complete registration, we need additional details from you."
 msgstr ""
 
@@ -45554,6 +45563,9 @@ msgstr ""
 msgid "Your new comment"
 msgstr ""
 
+msgid "Your password"
+msgstr ""
+
 msgid "Your password reset token has expired."
 msgstr ""
 
diff --git a/spec/controllers/projects/merge_requests/drafts_controller_spec.rb b/spec/controllers/projects/merge_requests/drafts_controller_spec.rb
index b9ede84157d824..ea782309f93419 100644
--- a/spec/controllers/projects/merge_requests/drafts_controller_spec.rb
+++ b/spec/controllers/projects/merge_requests/drafts_controller_spec.rb
@@ -5,7 +5,7 @@
   include RepoHelpers
 
   let(:project)       { create(:project, :repository) }
-  let(:merge_request) { create(:merge_request_with_diffs, target_project: project, source_project: project) }
+  let(:merge_request) { create(:merge_request_with_diffs, target_project: project, source_project: project, author: create(:user)) }
   let(:user)          { project.first_owner }
   let(:user2)         { create(:user) }
 
@@ -417,6 +417,38 @@ def create_reply(discussion_id, resolves: false)
         end
       end
     end
+
+    context 'approve merge request' do
+      before do
+        create(:draft_note, merge_request: merge_request, author: user)
+      end
+
+      context 'when feature flag is disabled' do
+        before do
+          stub_feature_flags(mr_review_submit_comment: false)
+        end
+
+        it 'does not approve' do
+          post :publish, params: params.merge!(approve: true)
+
+          expect(merge_request.approvals.reload.size).to be(0)
+        end
+      end
+
+      context 'when feature flag is enabled' do
+        it 'approves merge request' do
+          post :publish, params: params.merge!(approve: true)
+
+          expect(merge_request.approvals.reload.size).to be(1)
+        end
+
+        it 'does not approve merge request' do
+          post :publish, params: params.merge!(approve: false)
+
+          expect(merge_request.approvals.reload.size).to be(0)
+        end
+      end
+    end
   end
 
   describe 'DELETE #destroy' do
diff --git a/spec/fixtures/api/schemas/entities/merge_request_noteable.json b/spec/fixtures/api/schemas/entities/merge_request_noteable.json
index 4ef19ed32c2984..705ebcbd843e15 100644
--- a/spec/fixtures/api/schemas/entities/merge_request_noteable.json
+++ b/spec/fixtures/api/schemas/entities/merge_request_noteable.json
@@ -22,11 +22,13 @@
       "type": "object",
       "required": [
         "can_create_note",
-        "can_update"
+        "can_update",
+        "can_approve"
       ],
       "properties": {
         "can_create_note": { "type": "boolean" },
-        "can_update": { "type": "boolean" }
+        "can_update": { "type": "boolean" },
+        "can_approve": { "type": "boolean" }
       },
       "additionalProperties": false
     },
diff --git a/spec/frontend/batch_comments/components/submit_dropdown_spec.js b/spec/frontend/batch_comments/components/submit_dropdown_spec.js
index 4f5ff797230d6c..dc7ecb8e44db2f 100644
--- a/spec/frontend/batch_comments/components/submit_dropdown_spec.js
+++ b/spec/frontend/batch_comments/components/submit_dropdown_spec.js
@@ -8,7 +8,7 @@ Vue.use(Vuex);
 let wrapper;
 let publishReview;
 
-function factory() {
+function factory({ canApprove = true } = {}) {
   publishReview = jest.fn();
 
   const store = new Vuex.Store({
@@ -17,7 +17,11 @@ function factory() {
         markdownDocsPath: '/markdown/docs',
         quickActionsDocsPath: '/quickactions/docs',
       }),
-      getNoteableData: () => ({ id: 1, preview_note_path: '/preview' }),
+      getNoteableData: () => ({
+        id: 1,
+        preview_note_path: '/preview',
+        current_user: { can_approve: canApprove },
+      }),
       noteableType: () => 'merge_request',
     },
     modules: {
@@ -54,6 +58,8 @@ describe('Batch comments submit dropdown', () => {
       noteable_type: 'merge_request',
       noteable_id: 1,
       note: 'Hello world',
+      approve: false,
+      approval_password: '',
     });
   });
 
@@ -66,4 +72,14 @@ describe('Batch comments submit dropdown', () => {
 
     expect(findSubmitButton().props('loading')).toBe(true);
   });
+
+  it.each`
+    canApprove | exists   | existsText
+    ${true}    | ${true}  | ${'shows'}
+    ${false}   | ${false} | ${'hides'}
+  `('it $existsText approve checkbox if can_approve is $canApprove', ({ canApprove, exists }) => {
+    factory({ canApprove });
+
+    expect(wrapper.findByTestId('approve_merge_request').exists()).toBe(exists);
+  });
 });
diff --git a/spec/frontend/batch_comments/stores/modules/batch_comments/actions_spec.js b/spec/frontend/batch_comments/stores/modules/batch_comments/actions_spec.js
index 9f50b12bac2448..6369ea9aa15fcc 100644
--- a/spec/frontend/batch_comments/stores/modules/batch_comments/actions_spec.js
+++ b/spec/frontend/batch_comments/stores/modules/batch_comments/actions_spec.js
@@ -180,6 +180,7 @@ describe('Batch comments store actions', () => {
     });
 
     it('calls service with notes data', () => {
+      mock.onAny().reply(200);
       jest.spyOn(axios, 'post');
 
       return actions
@@ -192,7 +193,7 @@ describe('Batch comments store actions', () => {
     it('dispatches error commits', () => {
       mock.onAny().reply(500);
 
-      return actions.publishReview({ dispatch, commit, getters, rootGetters }).then(() => {
+      return actions.publishReview({ dispatch, commit, getters, rootGetters }).catch(() => {
         expect(commit.mock.calls[0]).toEqual(['REQUEST_PUBLISH_REVIEW']);
         expect(commit.mock.calls[1]).toEqual(['RECEIVE_PUBLISH_REVIEW_ERROR']);
       });
-- 
GitLab


From c1078f78dbc49d73d605f3ac17cafd6ad146400b Mon Sep 17 00:00:00 2001
From: Patrick Bajao <ebajao@gitlab.com>
Date: Thu, 11 Aug 2022 11:37:10 +0800
Subject: [PATCH 2/4] Fix rubocop failures on draft_comments_spec.rb

---
 .../features/merge_request/draft_comments_spec.rb    | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/ee/spec/features/merge_request/draft_comments_spec.rb b/ee/spec/features/merge_request/draft_comments_spec.rb
index 4e19075b2faa04..170a9f423ff1f1 100644
--- a/ee/spec/features/merge_request/draft_comments_spec.rb
+++ b/ee/spec/features/merge_request/draft_comments_spec.rb
@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe 'Merge request > Batch comments', :js do
+RSpec.describe 'Merge request > Batch comments', :js, :sidekiq_inline do
   include MergeRequestDiffHelpers
   include RepoHelpers
 
@@ -22,8 +22,8 @@
     wait_for_requests
   end
 
-  context 'approval' do
-    context 'user does not have permission to approve' do
+  context 'with approval' do
+    context 'when user does not have permission to approve' do
       let(:current_user) { create(:user) }
 
       it 'does not allow user to approve' do
@@ -33,7 +33,7 @@
       end
     end
 
-    context 'user has permission to approve' do
+    context 'when user has permission to approve' do
       it 'allows user to approve' do
         click_button 'Finish review'
 
@@ -45,7 +45,7 @@
         expect(page).to have_content('approved this merge request')
       end
 
-      context 'require password for approval' do
+      context 'when password is required for approval' do
         let(:project) do
           # rubocop:disable Layout/LineLength
           create(:project, :public, :repository, require_password_to_approve: true, merge_requests_author_approval: true)
@@ -67,7 +67,7 @@
           click_button 'Finish review'
 
           find('[data-testid="approve_merge_request"]').click
-          fill_in(type: 'password', with: '12345678')
+          fill_in(type: 'password', with: current_user.password)
           click_button 'Submit review'
 
           wait_for_requests
-- 
GitLab


From 82fbe3822f3453f291046fdede8bfa3fc1807048 Mon Sep 17 00:00:00 2001
From: Patrick Bajao <ebajao@gitlab.com>
Date: Thu, 25 Aug 2022 11:45:12 +0800
Subject: [PATCH 3/4] Call MergeRequest#can_be_approved_by? in serializer

This is needed because we recently unify the approvable check to
`#can_be_approved_by?` and `#can_approve?` has been removed.
---
 app/serializers/merge_request_noteable_entity.rb       | 2 +-
 ee/app/serializers/ee/merge_request_noteable_entity.rb | 6 ------
 2 files changed, 1 insertion(+), 7 deletions(-)

diff --git a/app/serializers/merge_request_noteable_entity.rb b/app/serializers/merge_request_noteable_entity.rb
index 8154865d51d639..bd0de3a72b9fef 100644
--- a/app/serializers/merge_request_noteable_entity.rb
+++ b/app/serializers/merge_request_noteable_entity.rb
@@ -42,7 +42,7 @@ class MergeRequestNoteableEntity < IssuableEntity
     end
 
     expose :can_approve do |merge_request|
-      can?(current_user, :approve_merge_request, merge_request)
+      merge_request.can_be_approved_by?(current_user)
     end
   end
 
diff --git a/ee/app/serializers/ee/merge_request_noteable_entity.rb b/ee/app/serializers/ee/merge_request_noteable_entity.rb
index a9b1ea3c02b203..88bc42b82066b6 100644
--- a/ee/app/serializers/ee/merge_request_noteable_entity.rb
+++ b/ee/app/serializers/ee/merge_request_noteable_entity.rb
@@ -8,12 +8,6 @@ module MergeRequestNoteableEntity
       expose :require_password_to_approve do |merge_request|
         merge_request.target_project.require_password_to_approve?
       end
-
-      expose :current_user do
-        expose :can_approve do |merge_request|
-          merge_request.can_approve?(current_user)
-        end
-      end
     end
   end
 end
-- 
GitLab


From a1eac2b5e4f15b137eb6448f9c7bbb564e1822f6 Mon Sep 17 00:00:00 2001
From: Phil Hughes <me@iamphill.com>
Date: Thu, 25 Aug 2022 14:30:38 +0100
Subject: [PATCH 4/4] Added captureError to createAlert call

---
 .../javascripts/batch_comments/components/submit_dropdown.vue   | 2 +-
 .../javascripts/batch_comments/components/approval_password.vue | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/assets/javascripts/batch_comments/components/submit_dropdown.vue b/app/assets/javascripts/batch_comments/components/submit_dropdown.vue
index bb9ec43c6b1881..c5b713b54474ff 100644
--- a/app/assets/javascripts/batch_comments/components/submit_dropdown.vue
+++ b/app/assets/javascripts/batch_comments/components/submit_dropdown.vue
@@ -92,7 +92,7 @@ export default {
         }
       } catch (e) {
         if (e.data?.message) {
-          createAlert({ message: e.data.message });
+          createAlert({ message: e.data.message, captureError: true });
         }
       }
 
diff --git a/ee/app/assets/javascripts/batch_comments/components/approval_password.vue b/ee/app/assets/javascripts/batch_comments/components/approval_password.vue
index 5760adb0c9b634..c738092cc4d44d 100644
--- a/ee/app/assets/javascripts/batch_comments/components/approval_password.vue
+++ b/ee/app/assets/javascripts/batch_comments/components/approval_password.vue
@@ -28,7 +28,7 @@ export default {
       type="password"
       :value="value"
       :placeholder="__('Password')"
-      @input="(e) => $emit('input', e)"
+      @input="$emit('input', $event)"
     />
   </gl-form-group>
 </template>
-- 
GitLab