Merge branch 'nfriend-add-merge-train-warning-dialog' into 'master'

Add warning dialog when users click "Merge immediately" merge train option

Closes #12679

See merge request gitlab-org/gitlab!20054

app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue

@@ -2,9 +2,9 @@
 import _ from 'underscore';
 import successSvg from 'icons/_icon_status_success.svg';
 import warningSvg from 'icons/_icon_status_warning.svg';
+import readyToMergeMixin from 'ee_else_ce/vue_merge_request_widget/mixins/ready_to_merge';
 import simplePoll from '~/lib/utils/simple_poll';
 import { __, sprintf } from '~/locale';
-import readyToMergeMixin from 'ee_else_ce/vue_merge_request_widget/mixins/ready_to_merge';
 import { GlIcon } from '@gitlab/ui';
 import MergeRequest from '../../../merge_request';
 import { refreshUserMergeRequestCounts } from '~/commons/nav/user_merge_requests';
@@ -26,6 +26,10 @@ export default {
     CommitEdit,
     CommitMessageDropdown,
     GlIcon,
+    MergeImmediatelyConfirmationDialog: () =>
+      import(
+        'ee_component/vue_merge_request_widget/components/merge_immediately_confirmation_dialog.vue'
+      ),
   },
   mixins: [readyToMergeMixin],
   props: {
@@ -165,6 +169,16 @@ export default {
           new Flash(__('Something went wrong. Please try again.')); // eslint-disable-line
         });
     },
+    handleMergeImmediatelyButtonClick() {
+      if (this.isMergeImmediatelyDangerous) {
+        this.$refs.confirmationDialog.show();
+      } else {
+        this.handleMergeButtonClick(false, true);
+      }
+    },
+    onMergeImmediatelyConfirmation() {
+      this.handleMergeButtonClick(false, true);
+    },
     initiateMergePolling() {
       simplePoll(
         (continuePolling, stopPolling) => {
@@ -286,11 +300,16 @@ export default {
                 </a>
               </li>
               <li>
+                <merge-immediately-confirmation-dialog
+                  ref="confirmationDialog"
+                  :docs-url="mr.mergeImmediatelyDocsPath"
+                  @mergeImmediately="onMergeImmediatelyConfirmation"
+                />
                 <a
-                  class="accept-merge-request"
+                  class="accept-merge-request js-merge-immediately-button"
                   data-qa-selector="merge_immediately_option"
                   href="#"
-                  @click.prevent="handleMergeButtonClick(false, true)"
+                  @click.prevent="handleMergeImmediatelyButtonClick"
                 >
                   <span class="media">
                     <span class="merge-opt-icon" aria-hidden="true" v-html="warningSvg"></span>

app/assets/javascripts/vue_merge_request_widget/mixins/ready_to_merge.js

@@ -23,5 +23,8 @@ export default {
     shouldShowMergeImmediatelyDropdown() {
       return this.mr.isPipelineActive && !this.mr.onlyAllowMergeIfPipelineSucceeds;
     },
+    isMergeImmediatelyDangerous() {
+      return false;
+    },
   },
 };

app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js

@@ -173,6 +173,7 @@ export default class MergeRequestStore {
     this.ciEnvironmentsStatusPath = data.ci_environments_status_path;
     this.securityApprovalsHelpPagePath = data.security_approvals_help_page_path;
     this.eligibleApproversDocsPath = data.eligible_approvers_docs_path;
+    this.mergeImmediatelyDocsPath = data.merge_immediately_docs_path;
   }
 
   get isNothingToMergeState() {

changelogs/unreleased/nfriend-add-merge-train-warning-dialog.yml

+---
+title: Add warning dialog when users click the "Merge immediately" merge train option
+merge_request: 20054
+author:
+type: added

ee/app/assets/javascripts/vue_merge_request_widget/components/merge_immediately_confirmation_dialog.vue

+<script>
+import { GlModal, GlButton } from '@gitlab/ui';
+import { __, sprintf } from '~/locale';
+import _ from 'underscore';
+
+export default {
+  name: 'MergeImmediatelyConfirmationDialog',
+  components: {
+    GlModal,
+    GlButton,
+  },
+  props: {
+    docsUrl: {
+      type: String,
+      required: true,
+    },
+  },
+  computed: {
+    bodyText() {
+      return sprintf(
+        __(
+          "Merging immediately isn't recommended as it may negatively impact the existing merge train. Read the %{docsLinkStart}documentation%{docsLinkEnd} for more information.",
+        ),
+        {
+          docsLinkStart: `<a href="${_.escape(
+            this.docsUrl,
+          )}" target="_blank" rel="noopener noreferrer">`,
+          docsLinkEnd: '</a>',
+        },
+        false,
+      );
+    },
+  },
+  methods: {
+    show() {
+      this.$refs.modal.show();
+    },
+    cancel() {
+      this.hide();
+    },
+    mergeImmediately() {
+      this.$emit('mergeImmediately');
+      this.hide();
+    },
+    hide() {
+      this.$refs.modal.hide();
+    },
+    focusCancelButton() {
+      this.$refs.cancelButton.$el.focus();
+    },
+  },
+};
+</script>
+<template>
+  <gl-modal
+    ref="modal"
+    modal-id="merge-immediately-confirmation-dialog"
+    :title="__('Merge immediately')"
+    @shown="focusCancelButton"
+  >
+    <p v-html="bodyText"></p>
+    <p>{{ __('Are you sure you want to merge immediately?') }}</p>
+    <template v-slot:modal-footer>
+      <gl-button ref="cancelButton" @click="cancel">{{ __('Cancel') }}</gl-button>
+      <gl-button variant="danger" @click="mergeImmediately">{{
+        __('Merge immediately')
+      }}</gl-button>
+    </template>
+  </gl-modal>
+</template>

ee/app/assets/javascripts/vue_merge_request_widget/mixins/ready_to_merge.js

@@ -49,5 +49,8 @@ export default {
 
       return this.mr.isPipelineActive && !this.mr.onlyAllowMergeIfPipelineSucceeds;
     },
+    isMergeImmediatelyDangerous() {
+      return this.mr.preferredAutoMergeStrategy === MT_MERGE_STRATEGY;
+    },
   },
 };

ee/app/presenters/ee/merge_request_presenter.rb

@@ -45,6 +45,10 @@ module EE
       help_page_path('ci/merge_request_pipelines/pipelines_for_merged_results/merge_trains/index.md', anchor: 'startadd-to-merge-train-when-pipeline-succeeds')
     end
 
+    def merge_immediately_docs_path
+      help_page_path('ci/merge_request_pipelines/pipelines_for_merged_results/merge_trains/index.md', anchor: 'immediately-merge-a-merge-request-with-a-merge-train')
+    end
+
     def target_project
       merge_request.target_project.present(current_user: current_user)
     end

ee/app/serializers/ee/merge_request_widget_entity.rb

@@ -150,6 +150,10 @@ module EE
         presenter(merge_request).merge_train_when_pipeline_succeeds_docs_path
       end
 
+      expose :merge_immediately_docs_path do |merge_request|
+        presenter(merge_request).merge_immediately_docs_path
+      end
+
       expose :blocking_merge_requests, if: -> (mr, _) { mr&.target_project&.feature_available?(:blocking_merge_requests) }
 
       private

ee/spec/features/merge_request/user_merges_immediately_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe 'Merge requests > User merges immediately', :js do
+  let_it_be(:project) { create(:project, :repository) }
+  let_it_be(:user) { create(:user) }
+
+  let_it_be(:merge_request) do
+    create(:merge_request, :with_merge_request_pipeline,
+      source_project: project, source_branch: 'feature',
+      target_project: project, target_branch: 'master')
+  end
+
+  let_it_be(:ci_yaml) do
+    { test: { stage: 'test', script: 'echo', only: ['merge_requests'] } }
+  end
+
+  before(:all) do
+    project.add_maintainer(user)
+    project.update!(merge_pipelines_enabled: true)
+    merge_request.all_pipelines.first.succeed!
+    merge_request.update_head_pipeline
+  end
+
+  before do
+    stub_licensed_features(merge_pipelines: true, merge_trains: true)
+    stub_ci_pipeline_yaml_file(YAML.dump(ci_yaml))
+
+    sign_in(user)
+    visit project_merge_request_path(project, merge_request)
+    wait_for_requests
+  end
+
+  context 'when the merge request is on the merge train' do
+    def merge_button
+      find('.mr-widget-body .accept-merge-request.btn-info')
+    end
+
+    def open_warning_dialog
+      find('.mr-widget-body .dropdown-toggle').click
+
+      click_link 'Merge immediately'
+
+      expect(page).to have_selector('#merge-immediately-confirmation-dialog')
+    end
+
+    it 'shows a warning dialog and does nothing if the user selects "Cancel"' do
+      Sidekiq::Testing.fake! do
+        open_warning_dialog
+
+        find(':focus').send_keys :enter
+
+        expect(merge_button).to have_no_content('Merge in progress')
+      end
+    end
+
+    it 'shows a warning dialog and merges immediately after the user confirms' do
+      Sidekiq::Testing.fake! do
+        open_warning_dialog
+
+        click_button 'Merge immediately'
+
+        expect(merge_button).to have_content('Merge in progress')
+      end
+    end
+  end
+end

ee/spec/frontend/vue_mr_widget/components/merge_immediately_confirmation_dialog_spec.js

+import { shallowMount } from '@vue/test-utils';
+import MergeImmediatelyConfirmationDialog from 'ee/vue_merge_request_widget/components/merge_immediately_confirmation_dialog.vue';
+import { trimText } from 'helpers/text_helper';
+
+describe('MergeImmediatelyConfirmationDialog', () => {
+  const docsUrl = 'path/to/merge/immediately/docs';
+  let wrapper;
+
+  beforeEach(() => {
+    wrapper = shallowMount(MergeImmediatelyConfirmationDialog, {
+      propsData: { docsUrl },
+      sync: false,
+    });
+
+    return wrapper.vm.$nextTick();
+  });
+
+  it('should render informational text explaining why merging immediately can be dangerous', () => {
+    expect(trimText(wrapper.text())).toContain(
+      "Merging immediately isn't recommended as it may negatively impact the existing merge train. Read the documentation for more information. Are you sure you want to merge immediately?",
+    );
+  });
+
+  it('should render a link to the documentation', () => {
+    const docsLink = wrapper.find(`a[href="${docsUrl}"]`);
+
+    expect(docsLink.exists()).toBe(true);
+    expect(docsLink.attributes().rel).toBe('noopener noreferrer');
+    expect(trimText(docsLink.text())).toBe('documentation');
+  });
+
+  afterEach(() => {
+    wrapper.destroy();
+    wrapper = null;
+  });
+});

ee/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js → ee/spec/frontend/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js

 import { shallowMount, createLocalVue } from '@vue/test-utils';
 import { MERGE_DISABLED_TEXT_UNAPPROVED } from 'ee/vue_merge_request_widget/mixins/ready_to_merge';
+import MergeImmediatelyConfirmationDialog from 'ee/vue_merge_request_widget/components/merge_immediately_confirmation_dialog.vue';
 import ReadyToMerge from '~/vue_merge_request_widget/components/states/ready_to_merge.vue';
 import {
   MWPS_MERGE_STRATEGY,
@@ -38,6 +39,7 @@ describe('ReadyToMerge', () => {
     targetBranch: 'master',
     preferredAutoMergeStrategy: MWPS_MERGE_STRATEGY,
     availableAutoMergeStrategies: [MWPS_MERGE_STRATEGY],
+    mergeImmediatelyDocsPath: 'path/to/merge/immediately/docs',
   };
 
   const factory = (mrUpdates = {}) => {
@@ -48,6 +50,9 @@ describe('ReadyToMerge', () => {
       },
       localVue,
       sync: false,
+      stubs: {
+        MergeImmediatelyConfirmationDialog,
+      },
     });
 
     ({ vm } = wrapper);
@@ -55,6 +60,8 @@ describe('ReadyToMerge', () => {
 
   const findResolveItemsMessage = () => wrapper.find('.js-resolve-mr-widget-items-message');
   const findMergeButton = () => wrapper.find('.qa-merge-button');
+  const findMergeButtonDropdown = () => wrapper.find('.js-merge-moment');
+  const findMergeImmediatelyButton = () => wrapper.find('.js-merge-immediately-button');
 
   afterEach(() => {
     wrapper.destroy();
@@ -145,6 +152,20 @@ describe('ReadyToMerge', () => {
         expect(vm.autoMergeText).toEqual('Add to merge train when pipeline succeeds');
       });
     });
+
+    describe('isMergeImmediatelyDangerous', () => {
+      it('should return false if the preferred auto merge strategy is not merge trains', () => {
+        factory({ preferredAutoMergeStrategy: MWPS_MERGE_STRATEGY });
+
+        expect(vm.isMergeImmediatelyDangerous).toBe(false);
+      });
+
+      it('should return true if the preferred auto merge strategy is merge trains', () => {
+        factory({ preferredAutoMergeStrategy: MT_MERGE_STRATEGY });
+
+        expect(vm.isMergeImmediatelyDangerous).toBe(true);
+      });
+    });
   });
 
   describe('shouldShowMergeImmediatelyDropdown', () => {
@@ -186,6 +207,45 @@ describe('ReadyToMerge', () => {
     });
   });
 
+  describe('merge immediately warning dialog', () => {
+    let dialog;
+
+    const clickMergeImmediately = () => {
+      dialog = wrapper.find(MergeImmediatelyConfirmationDialog);
+
+      expect(dialog.exists()).toBe(true);
+      dialog.vm.show = jest.fn();
+      vm.handleMergeButtonClick = jest.fn();
+      findMergeButtonDropdown().trigger('click');
+      findMergeImmediatelyButton().trigger('click');
+    };
+
+    it('should show a warning dialog asking for confirmation if the user is trying to skip the merge train', () => {
+      factory({ preferredAutoMergeStrategy: MT_MERGE_STRATEGY });
+      clickMergeImmediately();
+
+      expect(dialog.vm.show).toHaveBeenCalled();
+      expect(vm.handleMergeButtonClick).not.toHaveBeenCalled();
+    });
+
+    it('should perform the merge when the user confirms their intent to merge immediately', () => {
+      factory({ preferredAutoMergeStrategy: MT_MERGE_STRATEGY });
+      clickMergeImmediately();
+
+      dialog.vm.$emit('mergeImmediately');
+
+      expect(vm.handleMergeButtonClick).toHaveBeenCalled();
+    });
+
+    it('should not ask for confirmation in non-merge train scenarios', () => {
+      factory({ isPipelineActive: true, onlyAllowMergeIfPipelineSucceeds: false });
+      clickMergeImmediately();
+
+      expect(dialog.vm.show).not.toHaveBeenCalled();
+      expect(vm.handleMergeButtonClick).toHaveBeenCalled();
+    });
+  });
+
   describe('cannot merge', () => {
     describe('when isMergeAllowed=false', () => {
       beforeEach(() => {

locale/gitlab.pot

@@ -2041,6 +2041,9 @@ msgstr ""
 msgid "Are you sure you want to lose your issue information?"
 msgstr ""
 
+msgid "Are you sure you want to merge immediately?"
+msgstr ""
+
 msgid "Are you sure you want to permanently delete this license?"
 msgstr ""
 
@@ -11004,6 +11007,9 @@ msgstr ""
 msgid "Merges this merge request when the pipeline succeeds."
 msgstr ""
 
+msgid "Merging immediately isn't recommended as it may negatively impact the existing merge train. Read the %{docsLinkStart}documentation%{docsLinkEnd} for more information."
+msgstr ""
+
 msgid "Messages"
 msgstr ""
 

spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js

 import Vue from 'vue';
+import { createLocalVue, shallowMount } from '@vue/test-utils';
 import ReadyToMerge from '~/vue_merge_request_widget/components/states/ready_to_merge.vue';
 import SquashBeforeMerge from '~/vue_merge_request_widget/components/states/squash_before_merge.vue';
 import CommitsHeader from '~/vue_merge_request_widget/components/states/commits_header.vue';
 import CommitEdit from '~/vue_merge_request_widget/components/states/commit_edit.vue';
 import CommitMessageDropdown from '~/vue_merge_request_widget/components/states/commit_message_dropdown.vue';
 import eventHub from '~/vue_merge_request_widget/event_hub';
-import { createLocalVue, shallowMount } from '@vue/test-utils';
 import { MWPS_MERGE_STRATEGY, MTWPS_MERGE_STRATEGY } from '~/vue_merge_request_widget/constants';
 
 const commitMessage = 'This is the commit message';
@@ -288,6 +288,12 @@ describe('ReadyToMerge', () => {
         expect(vm.isMergeButtonDisabled).toBe(true);
       });
     });
+
+    describe('isMergeImmediatelyDangerous', () => {
+      it('should always return false in CE', () => {
+        expect(vm.isMergeImmediatelyDangerous).toBe(false);
+      });
+    });
   });
 
   describe('methods', () => {