Merge branch 'change-pipeline-widget-when-no-pipeline-ran-for-commit' into 'master'

Show loading in MR pipeline widget when no pipelines

See merge request gitlab-org/gitlab!35980

app/assets/javascripts/pipelines/components/pipelines_list/pipelines_table_row.vue

@@ -309,7 +309,8 @@ export default {
           <div
             v-for="(stage, index) in pipeline.details.stages"
             :key="index"
-            class="stage-container dropdown js-mini-pipeline-graph"
+            class="stage-container dropdown"
+            data-testid="widget-mini-pipeline-graph"
           >
             <pipeline-stage
               :type="$options.pipelinesTable"

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

 <script>
 /* eslint-disable vue/require-default-prop */
-import { GlTooltipDirective, GlLink } from '@gitlab/ui';
+import { GlIcon, GlLink, GlLoadingIcon, GlSprintf, GlTooltipDirective } from '@gitlab/ui';
 import mrWidgetPipelineMixin from 'ee_else_ce/vue_merge_request_widget/mixins/mr_widget_pipeline';
-import { sprintf, s__ } from '~/locale';
+import { s__ } from '~/locale';
 import PipelineStage from '~/pipelines/components/pipelines_list/stage.vue';
 import CiIcon from '~/vue_shared/components/ci_icon.vue';
-import Icon from '~/vue_shared/components/icon.vue';
 import TooltipOnTruncate from '~/vue_shared/components/tooltip_on_truncate.vue';
 
 export default {
   name: 'MRWidgetPipeline',
   components: {
-    PipelineStage,
     CiIcon,
-    Icon,
-    TooltipOnTruncate,
     GlLink,
+    GlLoadingIcon,
+    GlIcon,
+    GlSprintf,
+    PipelineStage,
+    TooltipOnTruncate,
     LinkedPipelinesMiniList: () =>
       import('ee_component/vue_shared/components/linked_pipelines_mini_list.vue'),
   },
@@ -54,7 +55,11 @@ export default {
       type: String,
       required: false,
     },
-    troubleshootingDocsPath: {
+    mrTroubleshootingDocsPath: {
+      type: String,
+      required: true,
+    },
+    ciTroubleshootingDocsPath: {
       type: String,
       required: true,
     },
@@ -64,10 +69,7 @@ export default {
       return this.pipeline && Object.keys(this.pipeline).length > 0;
     },
     hasCIError() {
-      return (this.hasCi && !this.ciStatus) || this.hasPipelineMustSucceedConflict;
-    },
-    hasPipelineMustSucceedConflict() {
-      return !this.hasCi && this.pipelineMustSucceed;
+      return this.hasPipeline && !this.ciStatus;
     },
     status() {
       return this.pipeline.details && this.pipeline.details.status
@@ -82,22 +84,6 @@ export default {
     hasCommitInfo() {
       return this.pipeline.commit && Object.keys(this.pipeline.commit).length > 0;
     },
-    errorText() {
-      if (this.hasPipelineMustSucceedConflict) {
-        return s__('Pipeline|No pipeline has been run for this commit.');
-      }
-
-      return sprintf(
-        s__(
-          'Pipeline|Could not retrieve the pipeline status. For troubleshooting steps, read the %{linkStart}documentation%{linkEnd}.',
-        ),
-        {
-          linkStart: `<a href="${this.troubleshootingDocsPath}">`,
-          linkEnd: '</a>',
-        },
-        false,
-      );
-    },
     isTriggeredByMergeRequest() {
       return Boolean(this.pipeline.merge_request);
     },
@@ -118,15 +104,51 @@ export default {
       return '';
     },
   },
+  errorText: s__(
+    'Pipeline|Could not retrieve the pipeline status. For troubleshooting steps, read the %{linkStart}documentation%{linkEnd}.',
+  ),
+  monitoringPipelineText: s__('Pipeline|Checking pipeline status.'),
 };
 </script>
 <template>
-  <div class="ci-widget media js-ci-widget">
-    <template v-if="!hasPipeline || hasCIError">
-      <div class="add-border ci-status-icon ci-status-icon-failed ci-error js-ci-error">
-        <icon :size="24" name="status_failed_borderless" />
+  <div class="ci-widget media">
+    <template v-if="hasCIError">
+      <gl-icon name="status_failed" class="gl-text-red-500" :size="24" />
+      <div
+        class="gl-flex-fill-1 gl-ml-5"
+        tabindex="0"
+        role="text"
+        :aria-label="$options.errorText"
+        data-testid="ci-error-message"
+      >
+        <gl-sprintf :message="$options.errorText">
+          <template #link="{content}">
+            <gl-link :href="mrTroubleshootingDocsPath">{{ content }}</gl-link>
+          </template>
+        </gl-sprintf>
+      </div>
+    </template>
+    <template v-else-if="!hasPipeline">
+      <gl-loading-icon size="md" />
+      <div class="gl-flex-fill-1 gl-display-flex gl-ml-5" data-testid="monitoring-pipeline-message">
+        <span tabindex="0" role="text" :aria-label="$options.monitoringPipelineText">
+          <gl-sprintf :message="$options.monitoringPipelineText" />
+        </span>
+        <gl-link
+          :href="ciTroubleshootingDocsPath"
+          target="_blank"
+          class="gl-display-flex gl-align-items-center gl-ml-2"
+          tabindex="0"
+        >
+          <gl-icon
+            name="question"
+            :small="12"
+            tabindex="0"
+            role="text"
+            :aria-label="__('Link to go to GitLab pipeline documentation')"
+          />
+        </gl-link>
       </div>
-      <div class="media-body gl-ml-3" v-html="errorText"></div>
     </template>
     <template v-else-if="hasPipeline">
       <a :href="status.details_path" class="align-self-start gl-mr-3">
@@ -136,13 +158,15 @@ export default {
         <div class="ci-widget-content">
           <div class="media-body">
             <div
-              class="font-weight-bold js-pipeline-info-container"
+              class="gl-font-weight-bold"
+              data-testid="pipeline-info-container"
               data-qa-selector="merge_request_pipeline_info_content"
             >
               {{ pipeline.details.name }}
               <gl-link
                 :href="pipeline.path"
-                class="pipeline-id font-weight-normal pipeline-number"
+                class="pipeline-id gl-font-weight-normal pipeline-number"
+                data-testid="pipeline-id"
                 data-qa-selector="pipeline_link"
                 >#{{ pipeline.id }}</gl-link
               >
@@ -151,7 +175,8 @@ export default {
                 {{ s__('Pipeline|for') }}
                 <gl-link
                   :href="pipeline.commit.commit_path"
-                  class="commit-sha js-commit-link font-weight-normal"
+                  class="commit-sha gl-font-weight-normal"
+                  data-testid="commit-link"
                   >{{ pipeline.commit.short_id }}</gl-link
                 >
               </template>
@@ -160,18 +185,18 @@ export default {
                 <tooltip-on-truncate
                   :title="sourceBranch"
                   truncate-target="child"
-                  class="label-branch label-truncate font-weight-normal"
+                  class="label-branch label-truncate gl-font-weight-normal"
                   v-html="sourceBranchLink"
                 />
               </template>
             </div>
-            <div v-if="pipeline.coverage" class="coverage">
+            <div v-if="pipeline.coverage" class="coverage" data-testid="pipeline-coverage">
               {{ s__('Pipeline|Coverage') }} {{ pipeline.coverage }}%
 
               <span
                 v-if="pipelineCoverageDelta"
-                class="js-pipeline-coverage-delta"
                 :class="coverageDeltaClass"
+                data-testid="pipeline-coverage-delta"
               >
                 ({{ pipelineCoverageDelta }}%)
               </span>
@@ -189,13 +214,13 @@ export default {
                   :class="{
                     'has-downstream': hasDownstream(i),
                   }"
-                  class="stage-container dropdown js-mini-pipeline-graph mr-widget-pipeline-stages"
+                  class="stage-container dropdown mr-widget-pipeline-stages"
+                  data-testid="widget-mini-pipeline-graph"
                 >
                   <pipeline-stage :stage="stage" />
                 </div>
               </template>
             </span>
-
             <linked-pipelines-mini-list v-if="triggered.length" :triggered="triggered" />
           </span>
         </div>

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

@@ -82,7 +82,8 @@ export default {
       :pipeline-must-succeed="mr.onlyAllowMergeIfPipelineSucceeds"
       :source-branch="branch"
       :source-branch-link="branchLink"
-      :troubleshooting-docs-path="mr.troubleshootingDocsPath"
+      :mr-troubleshooting-docs-path="mr.mrTroubleshootingDocsPath"
+      :ci-troubleshooting-docs-path="mr.ciTroubleshootingDocsPath"
     />
     <template #footer>
       <div v-if="mr.exposedArtifactsPath" class="js-exposed-artifacts">

app/assets/javascripts/vue_merge_request_widget/constants.js

+export const SUCCESS = 'success';
 export const WARNING = 'warning';
 export const DANGER = 'danger';
 

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

@@ -163,7 +163,8 @@ export default class MergeRequestStore {
   setPaths(data) {
     // Paths are set on the first load of the page and not auto-refreshed
     this.squashBeforeMergeHelpPath = data.squash_before_merge_help_path;
-    this.troubleshootingDocsPath = data.troubleshooting_docs_path;
+    this.mrTroubleshootingDocsPath = data.mr_troubleshooting_docs_path;
+    this.ciTroubleshootingDocsPath = data.ci_troubleshooting_docs_path;
     this.pipelineMustSucceedDocsPath = data.pipeline_must_succeed_docs_path;
     this.mergeRequestBasicPath = data.merge_request_basic_path;
     this.mergeRequestWidgetPath = data.merge_request_widget_path;

app/views/projects/merge_requests/_widget.html.haml

@@ -7,7 +7,8 @@
     window.gl.mrWidgetData = #{serialize_issuable(@merge_request, serializer: 'widget', issues_links: true)}
 
     window.gl.mrWidgetData.squash_before_merge_help_path = '#{help_page_path("user/project/merge_requests/squash_and_merge")}';
-    window.gl.mrWidgetData.troubleshooting_docs_path = '#{help_page_path('user/project/merge_requests/reviewing_and_managing_merge_requests.md', anchor: 'troubleshooting')}';
+    window.gl.mrWidgetData.ci_troubleshooting_docs_path = '#{help_page_path('ci/troubleshooting.md')}';
+    window.gl.mrWidgetData.mr_troubleshooting_docs_path = '#{help_page_path('user/project/merge_requests/reviewing_and_managing_merge_requests.md', anchor: 'troubleshooting')}';
     window.gl.mrWidgetData.pipeline_must_succeed_docs_path = '#{help_page_path('user/project/merge_requests/merge_when_pipeline_succeeds.md', anchor: 'only-allow-merge-requests-to-be-merged-if-the-pipeline-succeeds')}';
     window.gl.mrWidgetData.security_approvals_help_page_path = '#{help_page_path('user/application_security/index.md', anchor: 'security-approvals-in-merge-requests-ultimate')}';
     window.gl.mrWidgetData.eligible_approvers_docs_path = '#{help_page_path('user/project/merge_requests/merge_request_approvals', anchor: 'eligible-approvers')}';

changelogs/unreleased/change-pipeline-widget-when-no-pipeline-ran-for-commit.yml

+---
+title: Add generic message when no pipeline in MR
+merge_request: 35980
+author:
+type: fixed

doc/ci/troubleshooting.md

+---
+stage: Verify
+group: Continuous Integration
+info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers
+type: reference
+---
+
+# Troubleshooting CI/CD
+
+## Merge request pipeline widget
+
+The merge request pipeline widget shows information about the pipeline status in a Merge Request. It's displayed above the [merge request ability to merge widget](#merge-request-ability-to-merge-widget).
+
+There are several messages that can be displayed depending on the status of the pipeline.
+
+### "Checking pipeline status"
+
+This message is shown when the merge request has no pipeline associated with the latest commit yet and [Pipelines must succeed](../user/project/merge_requests/merge_when_pipeline_succeeds.md#only-allow-merge-requests-to-be-merged-if-the-pipeline-succeeds) is turned on. This might be because:
+
+- GitLab hasn't finished creating the pipeline yet.
+- You are using an external CI service and GitLab hasn't heard back from the service yet.
+- You are not using CI/CD pipelines in your project.
+
+After the pipeline is created, the message will update with the pipeline status.
+
+Note: Currently if you delete the latest pipeline of a Merge Request, this message will be shown instead of a meaningful error message. This is a known issue and should be resolved soon.
+
+## Merge request ability to merge widget
+
+The merge request status widget shows the **Merge** button and whether or not a merge request is ready to merge. If the merge request can't be merged, the reason for this is displayed.
+
+If the pipeline is still running, the **Merge** button is replaced with the **Merge when pipeline succeeds** button.
+
+If [**Merge Trains**](merge_request_pipelines/pipelines_for_merged_results/merge_trains/index.md) are enabled, the button is either **Add to merge train** or **Add to merge train when pipeline succeeds**. **(PREMIUM)**
+
+### "A CI/CD pipeline must run and be successful before merge"
+
+This message is shown if the [Pipelines must succeed](../user/project/merge_requests/merge_when_pipeline_succeeds.md#only-allow-merge-requests-to-be-merged-if-the-pipeline-succeeds) setting is enabled in the project and a pipeline has not yet run successfully. This also applies if the pipeline has not been created yet, or if you are waiting for an external CI service. If you don't use pipelines for your project, then you should disable **Pipelines must succeed** so you can accept merge requests.

ee/spec/frontend/vue_mr_widget/mr_widget_pipeline_spec.js

@@ -7,6 +7,8 @@ import mockLinkedPipelines from '../vue_shared/components/linked_pipelines_mock_
 describe('MRWidgetPipeline', () => {
   let wrapper;
 
+  const findPipelineInfoContainer = () => wrapper.find('[data-testid="pipeline-info-container"');
+
   function createComponent(pipeline) {
     wrapper = mount(pipelineComponent, {
       propsData: {
@@ -16,7 +18,8 @@ describe('MRWidgetPipeline', () => {
         ciStatus: 'success',
         sourceBranchLink: undefined,
         sourceBranch: undefined,
-        troubleshootingDocsPath: 'help',
+        mrTroubleshootingDocsPath: 'help',
+        ciTroubleshootingDocsPath: 'help2',
       },
     });
   }
@@ -73,7 +76,7 @@ describe('MRWidgetPipeline', () => {
         createComponent(pipeline);
 
         const expected = `Merge train pipeline #${pipeline.id} ${pipeline.details.status.label} for ${pipeline.commit.short_id}`;
-        const actual = trimText(wrapper.find('.js-pipeline-info-container').text());
+        const actual = trimText(findPipelineInfoContainer().text());
 
         expect(actual).toBe(expected);
       });
@@ -87,7 +90,7 @@ describe('MRWidgetPipeline', () => {
         createComponent(pipeline);
 
         const expected = `Merged result pipeline #${pipeline.id} ${pipeline.details.status.label} for ${pipeline.commit.short_id}`;
-        const actual = trimText(wrapper.find('.js-pipeline-info-container').text());
+        const actual = trimText(findPipelineInfoContainer().text());
 
         expect(actual).toBe(expected);
       });

locale/gitlab.pot

@@ -13795,6 +13795,9 @@ msgstr ""
 msgid "Link title is required"
 msgstr ""
 
+msgid "Link to go to GitLab pipeline documentation"
+msgstr ""
+
 msgid "Linked emails (%{email_count})"
 msgstr ""
 
@@ -16878,6 +16881,9 @@ msgstr ""
 msgid "Pipeline|Canceled"
 msgstr ""
 
+msgid "Pipeline|Checking pipeline status."
+msgstr ""
+
 msgid "Pipeline|Commit"
 msgstr ""
 
@@ -16917,9 +16923,6 @@ msgstr ""
 msgid "Pipeline|Merged result pipeline"
 msgstr ""
 
-msgid "Pipeline|No pipeline has been run for this commit."
-msgstr ""
-
 msgid "Pipeline|Passed"
 msgstr ""
 

spec/features/merge_request/user_sees_merge_widget_spec.rb

@@ -268,7 +268,7 @@ RSpec.describe 'Merge request > User sees merge widget', :js do
     end
   end
 
-  context 'view merge request where project has CI set up but no CI status' do
+  context 'view merge request where there is no pipeline yet' do
     before do
       pipeline = create(:ci_pipeline, project: project,
                                       sha: merge_request.diff_head_sha,
@@ -278,11 +278,11 @@ RSpec.describe 'Merge request > User sees merge widget', :js do
       visit project_merge_request_path(project, merge_request)
     end
 
-    it 'has pipeline error text' do
+    it 'has pipeline loading state' do
       # Wait for the `ci_status` and `merge_check` requests
       wait_for_requests
 
-      expect(page).to have_text("Could not retrieve the pipeline status. For troubleshooting steps, read the documentation.")
+      expect(page).to have_text("Checking pipeline status")
     end
   end
 
@@ -889,9 +889,9 @@ RSpec.describe 'Merge request > User sees merge widget', :js do
       visit project_merge_request_path(project, merge_request)
     end
 
-    it 'renders a CI pipeline error' do
+    it 'renders a CI pipeline loading state' do
       within '.ci-widget' do
-        expect(page).to have_content('Could not retrieve the pipeline status.')
+        expect(page).to have_content('Checking pipeline status')
       end
     end
   end

spec/features/merge_request/user_sees_pipelines_spec.rb

@@ -38,14 +38,6 @@ RSpec.describe 'Merge request > User sees pipelines', :js do
         expect(page).to have_selector('.stage-cell')
       end
 
-      it 'pipeline sha does not equal last commit sha' do
-        pipeline.update_attribute(:sha, '19e2e9b4ef76b422ce1154af39a91323ccc57434')
-        visit project_merge_request_path(project, merge_request)
-        wait_for_requests
-
-        expect(page.find('.ci-widget')).to have_text("Could not retrieve the pipeline status. For troubleshooting steps, read the documentation.")
-      end
-
       context 'with a detached merge request pipeline' do
         let(:merge_request) { create(:merge_request, :with_detached_merge_request_pipeline) }
 

spec/features/projects/pipelines/pipelines_spec.rb

@@ -528,7 +528,7 @@ RSpec.describe 'Pipelines', :js do
         end
 
         it 'renders a mini pipeline graph' do
-          expect(page).to have_selector('.js-mini-pipeline-graph')
+          expect(page).to have_selector('[data-testid="widget-mini-pipeline-graph"]')
           expect(page).to have_selector('.js-builds-dropdown-button')
         end
 

spec/frontend/fixtures/static/mini_dropdown_graph.html

-<div class="js-builds-dropdown-tests dropdown dropdown js-mini-pipeline-graph">
-<button class="js-builds-dropdown-button" data-toggle="dropdown" data-stage-endpoint="foobar">
-Dropdown
-</button>
-<ul class="dropdown-menu mini-pipeline-graph-dropdown-menu js-builds-dropdown-container">
-<li class="js-builds-dropdown-list scrollable-menu">
-<ul></ul>
-</li>
-<li class="js-builds-dropdown-loading hidden">
-<span class="fa fa-spinner"></span>
-</li>
-</ul>
+<div class="js-builds-dropdown-tests dropdown dropdown" data-testid="widget-mini-pipeline-graph">
+  <button class="js-builds-dropdown-button" data-toggle="dropdown" data-stage-endpoint="foobar">
+    Dropdown
+  </button>
+  <ul class="dropdown-menu mini-pipeline-graph-dropdown-menu js-builds-dropdown-container">
+    <li class="js-builds-dropdown-list scrollable-menu">
+      <ul></ul>
+    </li>
+    <li class="js-builds-dropdown-loading hidden">
+      <span class="fa fa-spinner"></span>
+    </li>
+  </ul>
 </div>

spec/frontend/vue_mr_widget/mock_data.js

@@ -239,7 +239,8 @@ export default {
   commit_change_content_path: '/root/acets-app/-/merge_requests/22/commit_change_content',
   merge_commit_path:
     'http://localhost:3000/root/acets-app/commit/53027d060246c8f47e4a9310fb332aa52f221775',
-  troubleshooting_docs_path: 'help',
+  mr_troubleshooting_docs_path: 'help',
+  ci_troubleshooting_docs_path: 'help2',
   merge_request_pipelines_docs_path: '/help/ci/merge_request_pipelines/index.md',
   merge_train_when_pipeline_succeeds_docs_path:
     '/help/ci/merge_request_pipelines/pipelines_for_merged_results/merge_trains/#startadd-to-merge-train-when-pipeline-succeeds',
@@ -312,7 +313,8 @@ export const mockStore = {
     { id: 0, name: 'prod', status: SUCCESS },
     { id: 1, name: 'prod-docs', status: SUCCESS },
   ],
-  troubleshootingDocsPath: 'troubleshooting-docs-path',
+  mrTroubleshootingDocsPath: 'mr-troubleshooting-docs-path',
+  ciTroubleshootingDocsPath: 'ci-troubleshooting-docs-path',
   ciStatus: 'ci-status',
   hasCI: true,
   exposedArtifactsPath: 'exposed_artifacts.json',