Merge branch '229677-merge-modal' into 'master'

Resolve "Migrate data-toggle modal to GlModal in mr_widget_header.vue"

See merge request gitlab-org/gitlab!47889

app/assets/javascripts/vue_merge_request_widget/components/mr_widget_header.vue

@@ -7,12 +7,14 @@ import {
   GlDropdownSectionHeader,
   GlDropdownItem,
   GlTooltipDirective,
+  GlModalDirective,
 } from '@gitlab/ui';
 import { n__, s__, sprintf } from '~/locale';
 import { mergeUrlParams, webIDEUrl } from '~/lib/utils/url_utility';
 import clipboardButton from '~/vue_shared/components/clipboard_button.vue';
 import TooltipOnTruncate from '~/vue_shared/components/tooltip_on_truncate.vue';
 import MrWidgetIcon from './mr_widget_icon.vue';
+import MrWidgetHowToMergeModal from './mr_widget_how_to_merge_modal.vue';
 
 export default {
   name: 'MRWidgetHeader',
@@ -20,6 +22,7 @@ export default {
     clipboardButton,
     TooltipOnTruncate,
     MrWidgetIcon,
+    MrWidgetHowToMergeModal,
     GlButton,
     GlDropdown,
     GlDropdownSectionHeader,
@@ -27,6 +30,7 @@ export default {
   },
   directives: {
     GlTooltip: GlTooltipDirective,
+    GlModalDirective,
   },
   props: {
     mr: {
@@ -82,6 +86,9 @@ export default {
           )
         : '';
     },
+    isFork() {
+      return this.mr.sourceProjectFullPath !== this.mr.targetProjectFullPath;
+    },
   },
 };
 </script>
@@ -140,13 +147,22 @@ export default {
             </gl-button>
           </span>
           <gl-button
+            v-gl-modal-directive="'modal-merge-info'"
             :disabled="mr.sourceBranchRemoved"
-            data-target="#modal_merge_info"
-            data-toggle="modal"
             class="js-check-out-branch gl-mr-3"
           >
             {{ s__('mrWidget|Check out branch') }}
           </gl-button>
+          <mr-widget-how-to-merge-modal
+            :is-fork="isFork"
+            :can-merge="mr.canMerge"
+            :source-branch="mr.sourceBranch"
+            :source-project="mr.sourceProject"
+            :source-project-path="mr.sourceProjectFullPath"
+            :target-branch="mr.targetBranch"
+            :source-project-default-url="mr.sourceProjectDefaultUrl"
+            :reviewing-docs-path="mr.reviewingDocsPath"
+          />
         </template>
         <gl-dropdown
           v-gl-tooltip

app/assets/javascripts/vue_merge_request_widget/components/mr_widget_how_to_merge_modal.vue

+<script>
+/* eslint-disable @gitlab/require-i18n-strings */
+import { GlModal, GlLink, GlSprintf } from '@gitlab/ui';
+import ClipboardButton from '~/vue_shared/components/clipboard_button.vue';
+import { __ } from '~/locale';
+
+export default {
+  i18n: {
+    steps: {
+      step1: {
+        label: __('Step 1.'),
+        help: __('Fetch and check out the branch for this merge request'),
+      },
+      step2: {
+        label: __('Step 2.'),
+        help: __('Review the changes locally'),
+      },
+      step3: {
+        label: __('Step 3.'),
+        help: __('Merge the branch and fix any conflicts that come up'),
+      },
+      step4: {
+        label: __('Step 4.'),
+        help: __('Push the result of the merge to GitLab'),
+      },
+    },
+    copyCommands: __('Copy commands'),
+    tip: __(
+      '%{strongStart}Tip:%{strongEnd} You can also checkout merge requests locally by %{linkStart}following these guidelines%{linkEnd}',
+    ),
+    title: __('Check out, review, and merge locally'),
+  },
+  components: {
+    GlModal,
+    ClipboardButton,
+    GlLink,
+    GlSprintf,
+  },
+  props: {
+    canMerge: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
+    isFork: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
+    sourceBranch: {
+      type: String,
+      required: false,
+      default: '',
+    },
+    sourceProjectPath: {
+      type: String,
+      required: false,
+      default: '',
+    },
+    targetBranch: {
+      type: String,
+      required: false,
+      default: '',
+    },
+    sourceProjectDefaultUrl: {
+      type: String,
+      required: false,
+      default: '',
+    },
+    reviewingDocsPath: {
+      type: String,
+      required: false,
+      default: null,
+    },
+  },
+  computed: {
+    mergeInfo1() {
+      return this.isFork
+        ? `git fetch "${this.sourceProjectDefaultUrl}" ${this.sourceBranch}\ngit checkout -b "${this.sourceProjectPath}-${this.sourceBranch}" FETCH_HEAD`
+        : `git fetch origin\ngit checkout -b "${this.sourceBranch}" "origin/${this.sourceBranch}"`;
+    },
+    mergeInfo2() {
+      return this.isFork
+        ? `git fetch origin\ngit checkout "${this.targetBranch}"\ngit merge --no-ff "${this.sourceProjectPath}-${this.sourceBranch}"`
+        : `git fetch origin\ngit checkout "${this.targetBranch}"\ngit merge --no-ff " ${this.sourceBranch}"`;
+    },
+    mergeInfo3() {
+      return this.canMerge
+        ? `git push origin "${this.targetBranch}"`
+        : __('Note that pushing to GitLab requires write access to this repository.');
+    },
+  },
+};
+</script>
+
+<template>
+  <gl-modal modal-id="modal-merge-info" :title="$options.i18n.title" no-fade hide-footer>
+    <p>
+      <strong>
+        {{ $options.i18n.steps.step1.label }}
+      </strong>
+      {{ $options.i18n.steps.step1.help }}
+    </p>
+    <div class="gl-display-flex">
+      <pre class="gl-overflow-scroll gl-w-full" data-testid="how-to-merge-instructions">{{
+        mergeInfo1
+      }}</pre>
+      <clipboard-button
+        :text="mergeInfo1"
+        :title="$options.i18n.copyCommands"
+        class="gl-shadow-none! gl-bg-transparent! gl-flex-shrink-0"
+      />
+    </div>
+
+    <p>
+      <strong>
+        {{ $options.i18n.steps.step2.label }}
+      </strong>
+      {{ $options.i18n.steps.step2.help }}
+    </p>
+    <p>
+      <strong>
+        {{ $options.i18n.steps.step3.label }}
+      </strong>
+      {{ $options.i18n.steps.step3.help }}
+    </p>
+    <div class="gl-display-flex">
+      <pre class="gl-overflow-scroll gl-w-full" data-testid="how-to-merge-instructions">{{
+        mergeInfo2
+      }}</pre>
+      <clipboard-button
+        :text="mergeInfo2"
+        :title="$options.i18n.copyCommands"
+        class="gl-shadow-none! gl-bg-transparent! gl-flex-shrink-0"
+      />
+    </div>
+    <p>
+      <strong>
+        {{ $options.i18n.steps.step4.label }}
+      </strong>
+      {{ $options.i18n.steps.step4.help }}
+    </p>
+    <div class="gl-display-flex">
+      <pre class="gl-overflow-scroll gl-w-full" data-testid="how-to-merge-instructions">{{
+        mergeInfo3
+      }}</pre>
+      <clipboard-button
+        :text="mergeInfo3"
+        :title="$options.i18n.copyCommands"
+        class="gl-shadow-none! gl-bg-transparent! gl-flex-shrink-0"
+      />
+    </div>
+    <p v-if="reviewingDocsPath">
+      <gl-sprintf :message="$options.i18n.tip">
+        <template #strong="{ content }">
+          <strong>{{ content }}</strong>
+        </template>
+        <template #link="{ content }">
+          <gl-link class="gl-display-inline-block" :href="reviewingDocsPath" target="_blank">{{
+            content
+          }}</gl-link>
+        </template>
+      </gl-sprintf>
+    </p>
+  </gl-modal>
+</template>

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

@@ -220,6 +220,7 @@ export default class MergeRequestStore {
     this.sourceProjectFullPath = data.source_project_full_path;
     this.mergeRequestPipelinesHelpPath = data.merge_request_pipelines_docs_path;
     this.conflictsDocsPath = data.conflicts_docs_path;
+    this.reviewingDocsPath = data.reviewing_and_managing_merge_requests_docs_path;
     this.ciEnvironmentsStatusPath = data.ci_environments_status_path;
     this.securityApprovalsHelpPagePath = data.security_approvals_help_page_path;
     this.eligibleApproversDocsPath = data.eligible_approvers_docs_path;
@@ -229,6 +230,7 @@ export default class MergeRequestStore {
     this.pipelinesEmptySvgPath = data.pipelines_empty_svg_path;
     this.humanAccess = data.human_access;
     this.newPipelinePath = data.new_project_pipeline_path;
+    this.sourceProjectDefaultUrl = data.source_project_default_url;
     this.userCalloutsPath = data.user_callouts_path;
     this.suggestPipelineFeatureId = data.suggest_pipeline_feature_id;
     this.isDismissedSuggestPipeline = data.is_dismissed_suggest_pipeline;

app/serializers/merge_request_widget_entity.rb

@@ -2,6 +2,9 @@
 
 class MergeRequestWidgetEntity < Grape::Entity
   include RequestAwareEntity
+  include ProjectsHelper
+  include ApplicationHelper
+  include ApplicationSettingsHelper
 
   SUGGEST_PIPELINE = 'suggest_pipeline'
 
@@ -48,6 +51,10 @@ class MergeRequestWidgetEntity < Grape::Entity
     help_page_path('user/project/merge_requests/resolve_conflicts.md')
   end
 
+  expose :reviewing_and_managing_merge_requests_docs_path do |merge_request|
+    help_page_path('user/project/merge_requests/reviewing_and_managing_merge_requests.md', anchor: "checkout-merge-requests-locally-through-the-head-ref")
+  end
+
   expose :merge_request_pipelines_docs_path do |merge_request|
     help_page_path('ci/merge_request_pipelines/index.md')
   end
@@ -87,6 +94,10 @@ class MergeRequestWidgetEntity < Grape::Entity
     new_project_pipeline_path(merge_request.project)
   end
 
+  expose :source_project_default_url do |merge_request|
+    merge_request.source_project && default_url_to_repo(merge_request.source_project)
+  end
+
   # Rendering and redacting Markdown can be expensive. These links are
   # just nice to have in the merge request widget, so only
   # include them if they are explicitly requested on first load.

changelogs/unreleased/229677-merge-modal.yml

+---
+title: Make "How to merge" modal in merge requests conform to correct modal styling
+merge_request: 47889
+author:
+type: changed

locale/gitlab.pot

@@ -785,6 +785,9 @@ msgstr ""
 msgid "%{strongStart}Note:%{strongEnd} Once a custom stage has been added you can re-order stages by dragging them into the desired position."
 msgstr ""
 
+msgid "%{strongStart}Tip:%{strongEnd} You can also checkout merge requests locally by %{linkStart}following these guidelines%{linkEnd}"
+msgstr ""
+
 msgid "%{strong_start}%{branch_count}%{strong_end} Branch"
 msgid_plural "%{strong_start}%{branch_count}%{strong_end} Branches"
 msgstr[0] ""
@@ -5149,6 +5152,9 @@ msgstr ""
 msgid "Check feature availability on namespace plan"
 msgstr ""
 
+msgid "Check out, review, and merge locally"
+msgstr ""
+
 msgid "Check the %{docs_link_start}documentation%{docs_link_end}."
 msgstr ""
 
@@ -11860,6 +11866,9 @@ msgstr ""
 msgid "February"
 msgstr ""
 
+msgid "Fetch and check out the branch for this merge request"
+msgstr ""
+
 msgid "Fetching incoming email"
 msgstr ""
 
@@ -16992,6 +17001,9 @@ msgstr ""
 msgid "Merge requests are read-only in a secondary Geo node"
 msgstr ""
 
+msgid "Merge the branch and fix any conflicts that come up"
+msgstr ""
+
 msgid "Merge when pipeline succeeds"
 msgstr ""
 
@@ -18751,6 +18763,9 @@ msgstr ""
 msgid "Note parameters are invalid: %{errors}"
 msgstr ""
 
+msgid "Note that pushing to GitLab requires write access to this repository."
+msgstr ""
+
 msgid "Note that this invitation was sent to %{mail_to_invite_email}, but you are signed in as %{link_to_current_user} with email %{mail_to_current_user}."
 msgstr ""
 
@@ -22206,6 +22221,9 @@ msgstr ""
 msgid "Push project from command line"
 msgstr ""
 
+msgid "Push the result of the merge to GitLab"
+msgstr ""
+
 msgid "Push to create a project"
 msgstr ""
 
@@ -23352,6 +23370,9 @@ msgstr ""
 msgid "Review requested from %{name}"
 msgstr ""
 
+msgid "Review the changes locally"
+msgstr ""
+
 msgid "Review the process for configuring service providers in your identity provider — in this case, GitLab is the \"service provider\" or \"relying party\"."
 msgstr ""
 
@@ -26016,6 +26037,18 @@ msgstr ""
 msgid "Stay updated about the performance and health of your environment by configuring Prometheus to monitor your deployments."
 msgstr ""
 
+msgid "Step 1."
+msgstr ""
+
+msgid "Step 2."
+msgstr ""
+
+msgid "Step 3."
+msgstr ""
+
+msgid "Step 4."
+msgstr ""
+
 msgid "Still, we recommend keeping a backup saved somewhere. Otherwise, if you ever need it and have lost it, you will need to request GitLab Inc. to send it to you again."
 msgstr ""
 

spec/features/merge_request/user_sees_check_out_branch_modal_spec.rb

@@ -18,8 +18,8 @@ RSpec.describe 'Merge request > User sees check out branch modal', :js do
     expect(page).to have_content('Check out, review, and merge locally')
   end
 
-  it 'closes the check out branch modal with escape keypress' do
-    find('#modal_merge_info').send_keys(:escape)
+  it 'closes the check out branch modal with the close action' do
+    find('.modal button[aria-label="Close"]').click
 
     expect(page).not_to have_content('Check out, review, and merge locally')
   end

spec/fixtures/api/schemas/entities/merge_request_widget.json

@@ -14,6 +14,7 @@
         "merge_request_cached_widget_path": { "type": "string" },
         "commit_change_content_path": { "type": "string" },
         "conflicts_docs_path": { "type": ["string", "null"] },
+        "reviewing_and_managing_merge_requests_docs_path": { "type": ["string", "null"] },
         "merge_request_pipelines_docs_path": { "type": ["string", "null"] },
         "ci_environments_status_path": { "type": "string" },
         "issues_links": {

spec/frontend/vue_mr_widget/components/mr_widget_header_spec.js

@@ -164,9 +164,7 @@ describe('MRWidgetHeader', () => {
       it('renders checkout branch button with modal trigger', () => {
         const button = vm.$el.querySelector('.js-check-out-branch');
 
-        expect(button.textContent.trim()).toEqual('Check out branch');
-        expect(button.getAttribute('data-target')).toEqual('#modal_merge_info');
-        expect(button.getAttribute('data-toggle')).toEqual('modal');
+        expect(button.textContent.trim()).toBe('Check out branch');
       });
 
       it('renders web ide button', () => {

spec/frontend/vue_mr_widget/mock_data.js

@@ -41,6 +41,7 @@ export default {
   user_callouts_path: 'some/callout/path',
   suggest_pipeline_feature_id: 'suggest_pipeline',
   new_project_pipeline_path: '/group2/project2/pipelines/new',
+  source_project_default_url: '/gitlab-org/html5-boilerplate.git',
   metrics: {
     merged_by: {
       name: 'Administrator',

spec/frontend/vue_mr_widget/mr_widget_how_to_merge_modal_spec.js

+import { GlModal, GlSprintf } from '@gitlab/ui';
+import { shallowMount } from '@vue/test-utils';
+import MrWidgetHowToMergeModal from '~/vue_merge_request_widget/components/mr_widget_how_to_merge_modal.vue';
+
+describe('MRWidgetHowToMerge', () => {
+  let wrapper;
+
+  function mountComponent({ data = {}, props = {} } = {}) {
+    wrapper = shallowMount(MrWidgetHowToMergeModal, {
+      data() {
+        return { ...data };
+      },
+      propsData: {
+        ...props,
+      },
+      stubs: {},
+    });
+  }
+
+  afterEach(() => {
+    wrapper.destroy();
+    wrapper = null;
+  });
+
+  beforeEach(() => {
+    mountComponent();
+  });
+
+  const findModal = () => wrapper.find(GlModal);
+  const findInstructionsFields = () =>
+    wrapper.findAll('[ data-testid="how-to-merge-instructions"]');
+  const findTipLink = () => wrapper.find(GlSprintf);
+
+  it('renders a modal', () => {
+    expect(findModal().exists()).toBe(true);
+  });
+
+  it('renders a selection of markdown fields', () => {
+    expect(findInstructionsFields().length).toBe(3);
+  });
+
+  it('renders a tip including a link to docs when a valid link is present', () => {
+    mountComponent({ props: { reviewingDocsPath: '/gitlab-org/help' } });
+    expect(findTipLink().exists()).toBe(true);
+  });
+
+  it('should not render a tip including a link to docs when a valid link is not present', () => {
+    expect(findTipLink().exists()).toBe(false);
+  });
+
+  it('should render different instructions based on if the user can merge', () => {
+    mountComponent({ props: { canMerge: true } });
+    expect(
+      findInstructionsFields()
+        .at(2)
+        .text(),
+    ).toContain('git push origin');
+  });
+
+  it('should render different instructions based on if the merge is based off a fork', () => {
+    mountComponent({ props: { isFork: true } });
+    expect(
+      findInstructionsFields()
+        .at(0)
+        .text(),
+    ).toContain('FETCH_HEAD');
+  });
+});

spec/frontend/vue_mr_widget/stores/mr_widget_store_spec.js

@@ -141,6 +141,12 @@ describe('MergeRequestStore', () => {
       expect(store.newPipelinePath).toBe('/group2/project2/pipelines/new');
     });
 
+    it('should set sourceProjectDefaultUrl', () => {
+      store.setPaths({ ...mockData });
+
+      expect(store.sourceProjectDefaultUrl).toBe('/gitlab-org/html5-boilerplate.git');
+    });
+
     it('should set securityReportsDocsPath', () => {
       store.setPaths({ ...mockData });
 

spec/serializers/merge_request_widget_entity_spec.rb

@@ -351,4 +351,18 @@ RSpec.describe MergeRequestWidgetEntity do
   it 'has security_reports_docs_path' do
     expect(subject[:security_reports_docs_path]).not_to be_nil
   end
+
+  describe 'has source_project_default_url' do
+    it 'returns the default url to the source project' do
+      expect(subject[:source_project_default_url]).to eq project.http_url_to_repo
+    end
+
+    context 'when source project is nil' do
+      it 'returns nil' do
+        allow(resource).to receive(:source_project).and_return(nil)
+
+        expect(subject[:source_project_default_url]).to be_nil
+      end
+    end
+  end
 end