Merge branch '334131-improve-merge-train-messaging' into 'master'

Improve merge train help text [RUN-AS-IF-FOSS]

See merge request gitlab-org/gitlab!71687

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

@@ -103,8 +103,8 @@ export default {
     GlDropdownItem,
     GlFormCheckbox,
     GlSkeletonLoader,
-    MergeTrainHelperText: () =>
-      import('ee_component/vue_merge_request_widget/components/merge_train_helper_text.vue'),
+    MergeTrainHelperIcon: () =>
+      import('ee_component/vue_merge_request_widget/components/merge_train_helper_icon.vue'),
     MergeImmediatelyConfirmationDialog: () =>
       import(
         'ee_component/vue_merge_request_widget/components/merge_immediately_confirmation_dialog.vue'
@@ -238,7 +238,7 @@ export default {
       return CONFIRM;
     },
     iconClass() {
-      if (this.shouldRenderMergeTrainHelperText && !this.mr.preventMerge) {
+      if (this.shouldRenderMergeTrainHelperIcon && !this.mr.preventMerge) {
         return PIPELINE_RUNNING_STATE;
       }
 
@@ -504,7 +504,7 @@ export default {
       </div>
     </div>
     <template v-else>
-      <div class="mr-widget-body media" :class="{ 'gl-pb-3': shouldRenderMergeTrainHelperText }">
+      <div class="mr-widget-body media">
         <status-icon :status="iconClass" />
         <div class="media-body">
           <div class="mr-widget-body-controls gl-display-flex gl-align-items-center">
@@ -575,6 +575,13 @@ export default {
                 :is-disabled="isSquashReadOnly"
                 class="gl-mx-3"
               />
+
+              <merge-train-helper-icon
+                v-if="shouldRenderMergeTrainHelperIcon"
+                :merge-train-when-pipeline-succeeds-docs-path="
+                  mr.mergeTrainWhenPipelineSucceedsDocsPath
+                "
+              />
             </div>
             <template v-else>
               <div class="bold js-resolve-mr-widget-items-message gl-ml-3">
@@ -605,13 +612,6 @@ export default {
           </div>
         </div>
       </div>
-      <merge-train-helper-text
-        v-if="shouldRenderMergeTrainHelperText"
-        :pipeline-id="pipelineId"
-        :pipeline-link="pipeline.path"
-        :merge-train-length="stateData.mergeTrainsCount"
-        :merge-train-when-pipeline-succeeds-docs-path="mr.mergeTrainWhenPipelineSucceedsDocsPath"
-      />
       <template v-if="shouldShowMergeControls">
         <div
           v-if="!shouldShowMergeEdit"

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

@@ -32,7 +32,7 @@ export default {
     isMergeImmediatelyDangerous() {
       return false;
     },
-    shouldRenderMergeTrainHelperText() {
+    shouldRenderMergeTrainHelperIcon() {
       return false;
     },
     pipelineId() {

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

+<script>
+import { GlIcon, GlLink, GlPopover } from '@gitlab/ui';
+import { s__ } from '~/locale';
+
+export default {
+  name: 'MergeTrainHelperIcon',
+  components: {
+    GlIcon,
+    GlLink,
+    GlPopover,
+  },
+  props: {
+    mergeTrainWhenPipelineSucceedsDocsPath: {
+      type: String,
+      required: true,
+    },
+  },
+
+  i18n: {
+    popoverTitle: s__('mrWidget|What is a merge train?'),
+    popoverContent: s__(
+      'mrWidget|A merge train is a queued list of merge requests waiting to be merged into the target branch. The changes in each merge request are combined with the changes in earlier merge requests and tested before merge.',
+    ),
+    moreInfo: s__('mrWidget|More information'),
+    learnMore: s__('mrWidget|Learn more'),
+  },
+  popoverConstants: {
+    target: 'merge-train-help',
+    container: 'merge-train-help-container',
+  },
+};
+</script>
+
+<template>
+  <div :id="$options.popoverConstants.container">
+    <gl-icon
+      :id="$options.popoverConstants.target"
+      name="question-o"
+      class="gl-text-blue-600"
+      :aria-label="$options.i18n.moreInfo"
+      data-testid="merge-train-helper-icon"
+    />
+    <gl-popover
+      :target="$options.popoverConstants.target"
+      :container="$options.popoverConstants.container"
+      placement="top"
+      :title="$options.i18n.popoverTitle"
+      triggers="hover focus"
+    >
+      <p data-testid="merge-train-helper-content">{{ $options.i18n.popoverContent }}</p>
+      <gl-link
+        class="gl-mt-3"
+        :href="mergeTrainWhenPipelineSucceedsDocsPath"
+        target="_blank"
+        rel="noopener noreferrer"
+      >
+        {{ $options.i18n.learnMore }}
+      </gl-link>
+    </gl-popover>
+  </div>
+</template>

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

-<script>
-import { GlLink, GlSprintf } from '@gitlab/ui';
-import { s__ } from '~/locale';
-
-export default {
-  name: 'MergeTrainHelperText',
-  components: {
-    GlLink,
-    GlSprintf,
-  },
-  props: {
-    pipelineId: {
-      type: Number,
-      required: true,
-    },
-    pipelineLink: {
-      type: String,
-      required: true,
-    },
-    mergeTrainWhenPipelineSucceedsDocsPath: {
-      type: String,
-      required: true,
-    },
-    mergeTrainLength: {
-      type: Number,
-      required: true,
-    },
-  },
-  computed: {
-    helperMessage() {
-      return this.mergeTrainLength === 0
-        ? s__(
-            'mrWidget|This action will start a merge train when pipeline %{pipelineLink} succeeds.',
-          )
-        : s__(
-            'mrWidget|This action will add the merge request to the merge train when pipeline %{pipelineLink} succeeds.',
-          );
-    },
-  },
-};
-</script>
-
-<template>
-  <section class="js-merge-train-helper-text gl-px-5 gl-pb-5">
-    <div class="gl-pl-7">
-      <gl-sprintf :message="helperMessage">
-        <template #pipelineLink>
-          <gl-link data-testid="pipeline-link" :href="pipelineLink">#{{ pipelineId }}</gl-link>
-        </template>
-      </gl-sprintf>
-      <gl-link
-        :href="mergeTrainWhenPipelineSucceedsDocsPath"
-        target="_blank"
-        rel="noopener noreferrer"
-        data-testid="documentation-link"
-      >
-        {{ s__('mrWidget|More information') }}
-      </gl-link>
-    </div>
-  </section>
-</template>

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

@@ -61,7 +61,7 @@ export default {
 
       return this.pipeline.id;
     },
-    shouldRenderMergeTrainHelperText() {
+    shouldRenderMergeTrainHelperIcon() {
       return (
         this.pipeline &&
         isNumber(this.pipelineId) &&

ee/spec/features/merge_trains/user_adds_to_merge_train_when_pipeline_succeeds_spec.rb

@@ -25,16 +25,15 @@ RSpec.describe 'User adds to merge train when pipeline succeeds', :js do
     sign_in(user)
   end
 
-  it 'shows Start merge train when pipeline succeeds button and helper texts' do
+  it 'shows Start merge train when pipeline succeeds button and helper icon' do
     visit project_merge_request_path(project, merge_request)
 
     expect(page).to have_button('Start merge train when pipeline succeeds')
 
-    within('.js-merge-train-helper-text') do
-      expect(page).to have_content("This action will start a merge train when pipeline ##{pipeline.id} succeeds.")
-      expect(page).to have_link('More information',
-        href: MergeRequestPresenter.new(merge_request).merge_train_when_pipeline_succeeds_docs_path)
-    end
+    find('[data-testid="merge-train-helper-icon"]').hover
+
+    expect(page).to have_selector('[data-testid="merge-train-helper-content"]')
+    expect(page).to have_link('Learn more', href: help_page_path('ci/pipelines/merge_trains.md', anchor: 'add-a-merge-request-to-a-merge-train'))
   end
 
   context 'when merge_trains EEP license is not available' do
@@ -46,6 +45,7 @@ RSpec.describe 'User adds to merge train when pipeline succeeds', :js do
       visit project_merge_request_path(project, merge_request)
 
       expect(page).not_to have_button('Start merge train when pipeline succeeds')
+      expect(page).not_to have_selector('[data-testid="merge-train-helper-icon"]')
     end
   end
 

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

+import { GlLink, GlPopover, GlIcon } from '@gitlab/ui';
+import { shallowMount } from '@vue/test-utils';
+import MergeTrainHelperIcon from 'ee/vue_merge_request_widget/components/merge_train_helper_icon.vue';
+
+describe('MergeTrainHelperIcon', () => {
+  let wrapper;
+
+  const helpLink = 'path/to/help';
+
+  const findIcon = () => wrapper.findComponent(GlIcon);
+  const findPopover = () => wrapper.findComponent(GlPopover);
+  const findLink = () => wrapper.findComponent(GlLink);
+
+  const createComponent = () => {
+    wrapper = shallowMount(MergeTrainHelperIcon, {
+      propsData: {
+        mergeTrainWhenPipelineSucceedsDocsPath: helpLink,
+      },
+    });
+  };
+
+  beforeEach(() => {
+    createComponent();
+  });
+
+  afterEach(() => {
+    wrapper.destroy();
+  });
+
+  it('displays the correct icon', () => {
+    expect(findIcon().props('name')).toBe('question-o');
+  });
+
+  it('displays the correct help link', () => {
+    expect(findLink().attributes('href')).toBe(helpLink);
+  });
+
+  it('displays the correct popover title', () => {
+    expect(findPopover().props('title')).toBe('What is a merge train?');
+  });
+
+  it('displays the correct popover content', () => {
+    expect(wrapper.text()).toContain(
+      'A merge train is a queued list of merge requests waiting to be merged into the target branch. The changes in each merge request are combined with the changes in earlier merge requests and tested before merge',
+    );
+  });
+});

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

-import { GlLink, GlSprintf } from '@gitlab/ui';
-import { shallowMount } from '@vue/test-utils';
-import MergeTrainHelperText from 'ee/vue_merge_request_widget/components/merge_train_helper_text.vue';
-import { trimText } from 'helpers/text_helper';
-
-describe('MergeTrainHelperText', () => {
-  let wrapper;
-
-  const defaultProps = {
-    pipelineId: 123,
-    pipelineLink: 'path/to/pipeline',
-    mergeTrainWhenPipelineSucceedsDocsPath: 'path/to/help',
-    mergeTrainLength: 2,
-  };
-
-  const findDocumentationLink = () => wrapper.find('[data-testid="documentation-link"]');
-  const findPipelineLink = () => wrapper.find('[data-testid="pipeline-link"]');
-
-  const createWrapper = (propsData) => {
-    wrapper = shallowMount(MergeTrainHelperText, {
-      propsData: {
-        ...defaultProps,
-        ...propsData,
-      },
-      stubs: {
-        GlSprintf,
-        GlLink,
-      },
-    });
-  };
-
-  afterEach(() => {
-    wrapper.destroy();
-    wrapper = null;
-  });
-
-  it('should return the "start" version of the message if there is no existing merge train', () => {
-    createWrapper({ mergeTrainLength: 0 });
-
-    expect(trimText(wrapper.text())).toBe(
-      'This action will start a merge train when pipeline #123 succeeds. More information',
-    );
-  });
-
-  it('should render the correct pipeline link in the helper text', () => {
-    createWrapper();
-
-    const pipelineLink = findPipelineLink();
-
-    expect(pipelineLink.exists()).toBe(true);
-    expect(pipelineLink.text()).toContain('#123');
-    expect(pipelineLink.attributes('href')).toBe(defaultProps.pipelineLink);
-  });
-
-  it('should render the correct documentation link in the helper text', () => {
-    createWrapper();
-
-    expect(findDocumentationLink().exists()).toBe(true);
-    expect(findDocumentationLink().attributes('href')).toBe(
-      defaultProps.mergeTrainWhenPipelineSucceedsDocsPath,
-    );
-  });
-});

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

@@ -2,7 +2,7 @@ import { GlLink, GlSprintf } from '@gitlab/ui';
 import { mount, shallowMount } from '@vue/test-utils';
 import MergeImmediatelyConfirmationDialog from 'ee/vue_merge_request_widget/components/merge_immediately_confirmation_dialog.vue';
 import MergeTrainFailedPipelineConfirmationDialog from 'ee/vue_merge_request_widget/components/merge_train_failed_pipeline_confirmation_dialog.vue';
-import MergeTrainHelperText from 'ee/vue_merge_request_widget/components/merge_train_helper_text.vue';
+import MergeTrainHelperIcon from 'ee/vue_merge_request_widget/components/merge_train_helper_icon.vue';
 import { MERGE_DISABLED_TEXT_UNAPPROVED } from 'ee/vue_merge_request_widget/mixins/ready_to_merge';
 import ReadyToMerge from '~/vue_merge_request_widget/components/states/ready_to_merge.vue';
 import {
@@ -67,7 +67,7 @@ describe('ReadyToMerge', () => {
       },
       stubs: {
         MergeImmediatelyConfirmationDialog,
-        MergeTrainHelperText,
+        MergeTrainHelperIcon,
         GlSprintf,
         GlLink,
         MergeTrainFailedPipelineConfirmationDialog,
@@ -83,11 +83,7 @@ describe('ReadyToMerge', () => {
   const findMergeButton = () => wrapper.find('[data-testid="merge-button"]');
   const findMergeButtonDropdown = () => wrapper.find('.js-merge-moment');
   const findMergeImmediatelyButton = () => wrapper.find('.js-merge-immediately-button');
-  const findMergeTrainHelperText = () => wrapper.find(MergeTrainHelperText);
-  const findMergeTrainPipelineLink = () =>
-    findMergeTrainHelperText().find('[data-testid="pipeline-link"]');
-  const findMergeTrainDocumentationLink = () =>
-    findMergeTrainHelperText().find('[data-testid="documentation-link"]');
+  const findMergeTrainHelperIcon = () => wrapper.find(MergeTrainHelperIcon);
   const findFailedPipelineMergeTrainText = () =>
     wrapper.find('[data-testid="failed-pipeline-merge-train-text"]');
   const findMergeTrainFailedPipelineConfirmationDialog = () =>
@@ -207,84 +203,26 @@ describe('ReadyToMerge', () => {
     });
   });
 
-  describe('shouldRenderMergeTrainHelperText', () => {
-    it('should render the helper text if MTWPS is available and the user has not yet pressed the MTWPS button', () => {
+  describe('shouldRenderMergeTrainHelperIcon', () => {
+    it('should render the helper icon if MTWPS is available and the user has not yet pressed the MTWPS button', () => {
       factory({
         onlyAllowMergeIfPipelineSucceeds: true,
         preferredAutoMergeStrategy: MTWPS_MERGE_STRATEGY,
         autoMergeEnabled: false,
       });
 
-      expect(findMergeTrainHelperText().exists()).toBe(true);
+      expect(findMergeTrainHelperIcon().exists()).toBe(true);
     });
   });
 
-  describe('merge train helper text', () => {
-    it('does not render the merge train helper text if the MTWPS strategy is not available', () => {
+  describe('merge train helper icon', () => {
+    it('does not render the merge train helper icon if the MTWPS strategy is not available', () => {
       factory({
         availableAutoMergeStrategies: [MT_MERGE_STRATEGY],
         pipeline: activePipeline,
       });
 
-      expect(findMergeTrainHelperText().exists()).toBe(false);
-    });
-
-    it('renders the correct merge train helper text when there is an existing merge train', () => {
-      factory({
-        onlyAllowMergeIfPipelineSucceeds: true,
-        preferredAutoMergeStrategy: MTWPS_MERGE_STRATEGY,
-        autoMergeEnabled: false,
-        mergeTrainsCount: 2,
-        pipeline: activePipeline,
-      });
-
-      expect(findMergeTrainHelperText().text()).toContain(
-        `This action will add the merge request to the merge train when pipeline #${activePipeline.id} succeeds.`,
-      );
-    });
-
-    it('renders the correct merge train helper text when there is no existing merge train', () => {
-      factory({
-        onlyAllowMergeIfPipelineSucceeds: true,
-        preferredAutoMergeStrategy: MTWPS_MERGE_STRATEGY,
-        autoMergeEnabled: false,
-        mergeTrainsCount: 0,
-        pipeline: activePipeline,
-      });
-
-      expect(findMergeTrainHelperText().text()).toContain(
-        `This action will start a merge train when pipeline #${activePipeline.id} succeeds.`,
-      );
-    });
-
-    it('renders the correct pipeline link inside the message', () => {
-      factory({
-        onlyAllowMergeIfPipelineSucceeds: true,
-        preferredAutoMergeStrategy: MTWPS_MERGE_STRATEGY,
-        autoMergeEnabled: false,
-        mergeTrainsCount: 0,
-        pipeline: activePipeline,
-      });
-
-      const pipelineLink = findMergeTrainPipelineLink();
-
-      expect(pipelineLink.text()).toContain(activePipeline.id);
-      expect(pipelineLink.attributes('href')).toBe(activePipeline.path);
-    });
-
-    it('renders the documentation link inside the message', () => {
-      factory({
-        onlyAllowMergeIfPipelineSucceeds: true,
-        preferredAutoMergeStrategy: MTWPS_MERGE_STRATEGY,
-        autoMergeEnabled: false,
-        mergeTrainsCount: 0,
-        pipeline: activePipeline,
-      });
-
-      const pipelineLink = findMergeTrainDocumentationLink();
-
-      expect(pipelineLink.text()).toContain('More information');
-      expect(pipelineLink.attributes('href')).toBe(mr.mergeTrainWhenPipelineSucceedsDocsPath);
+      expect(findMergeTrainHelperIcon().exists()).toBe(false);
     });
   });
 

locale/gitlab.pot

@@ -40607,6 +40607,9 @@ msgstr ""
 msgid "mrWidget|%{prefixToLinkStart}No pipeline%{prefixToLinkEnd} %{addPipelineLinkStart}Add the .gitlab-ci.yml file%{addPipelineLinkEnd} to create one."
 msgstr ""
 
+msgid "mrWidget|A merge train is a queued list of merge requests waiting to be merged into the target branch. The changes in each merge request are combined with the changes in earlier merge requests and tested before merge."
+msgstr ""
+
 msgid "mrWidget|A new merge train has started and this merge request is the first of the queue."
 msgstr ""
 
@@ -40702,6 +40705,9 @@ msgstr ""
 msgid "mrWidget|Jump to first unresolved thread"
 msgstr ""
 
+msgid "mrWidget|Learn more"
+msgstr ""
+
 msgid "mrWidget|Loading deployment statistics"
 msgstr ""
 
@@ -40851,12 +40857,6 @@ msgstr ""
 msgid "mrWidget|There are merge conflicts"
 msgstr ""
 
-msgid "mrWidget|This action will add the merge request to the merge train when pipeline %{pipelineLink} succeeds."
-msgstr ""
-
-msgid "mrWidget|This action will start a merge train when pipeline %{pipelineLink} succeeds."
-msgstr ""
-
 msgid "mrWidget|This merge request failed to be merged automatically"
 msgstr ""
 
@@ -40872,6 +40872,9 @@ msgstr ""
 msgid "mrWidget|Use %{linkStart}CI pipelines to test your code%{linkEnd} by simply adding a GitLab CI configuration file to your project. It only takes a minute to make your code more secure and robust."
 msgstr ""
 
+msgid "mrWidget|What is a merge train?"
+msgstr ""
+
 msgid "mrWidget|You can merge after removing denied licenses"
 msgstr ""