Merge branch '337496-link-by-name-not-id-pipelines' into 'master'

Link by commit and name for pipeline

See merge request gitlab-org/gitlab!80648

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

@@ -136,8 +136,8 @@ export default {
           return __('Branch');
       }
     },
-    commitTitle() {
-      return this.pipeline?.commit?.title;
+    commitTitleText() {
+      return this.pipeline?.commit?.title || __("Can't find HEAD commit for this branch");
     },
     hasAuthor() {
       return (
@@ -159,27 +159,22 @@ export default {
   <div class="pipeline-tags" data-testid="pipeline-url-table-cell">
     <template v-if="rearrangePipelinesTable">
       <div class="commit-title gl-mb-2" data-testid="commit-title-container">
-        <span v-if="commitTitle" class="gl-display-flex">
-          <tooltip-on-truncate :title="commitTitle" class="flex-truncate-child gl-flex-grow-1">
+        <span class="gl-display-flex">
+          <tooltip-on-truncate :title="commitTitleText" class="flex-truncate-child gl-flex-grow-1">
             <gl-link
-              :href="commitUrl"
-              class="commit-row-message gl-text-gray-900"
+              :href="pipeline.path"
+              class="commit-row-message gl-text-blue-600!"
               data-testid="commit-title"
-              >{{ commitTitle }}</gl-link
+              data-qa-selector="pipeline_url_link"
+              >{{ commitTitleText }}</gl-link
             >
           </tooltip-on-truncate>
         </span>
-        <span v-else>{{ __("Can't find HEAD commit for this branch") }}</span>
       </div>
       <div class="gl-mb-2">
-        <gl-link
-          :href="pipeline.path"
-          class="gl-text-decoration-underline gl-text-blue-600!"
-          data-testid="pipeline-url-link"
-          data-qa-selector="pipeline_url_link"
-        >
+        <span class="gl-font-weight-bold gl-text-gray-500" data-testid="pipeline-identifier">
           #{{ pipeline[pipelineKey] }}
-        </gl-link>
+        </span>
         <!--Commit row-->
         <div class="icon-container gl-display-inline-block">
           <gl-icon

spec/features/merge_request/user_sees_merge_request_pipelines_spec.rb

@@ -64,7 +64,7 @@ RSpec.describe 'Merge request > User sees pipelines triggered by merge request',
     it 'sees branch pipelines and detached merge request pipelines in correct order' do
       page.within('.ci-table') do
         expect(page).to have_selector('.ci-created', count: 2)
-        expect(first('[data-testid="pipeline-url-link"]')).to have_content("##{detached_merge_request_pipeline.id}")
+        expect(first('[data-testid="pipeline-identifier"]')).to have_content("##{detached_merge_request_pipeline.id}")
       end
     end
 
@@ -101,16 +101,16 @@ RSpec.describe 'Merge request > User sees pipelines triggered by merge request',
         page.within('.ci-table') do
           expect(page).to have_selector('.ci-pending', count: 4)
 
-          expect(all('[data-testid="pipeline-url-link"]')[0])
+          expect(all('[data-testid="pipeline-identifier"]')[0])
             .to have_content("##{detached_merge_request_pipeline_2.id}")
 
-          expect(all('[data-testid="pipeline-url-link"]')[1])
+          expect(all('[data-testid="pipeline-identifier"]')[1])
             .to have_content("##{detached_merge_request_pipeline.id}")
 
-          expect(all('[data-testid="pipeline-url-link"]')[2])
+          expect(all('[data-testid="pipeline-identifier"]')[2])
             .to have_content("##{push_pipeline_2.id}")
 
-          expect(all('[data-testid="pipeline-url-link"]')[3])
+          expect(all('[data-testid="pipeline-identifier"]')[3])
             .to have_content("##{push_pipeline.id}")
         end
       end
@@ -201,7 +201,7 @@ RSpec.describe 'Merge request > User sees pipelines triggered by merge request',
       it 'sees a branch pipeline in pipeline tab' do
         page.within('.ci-table') do
           expect(page).to have_selector('.ci-created', count: 1)
-          expect(first('[data-testid="pipeline-url-link"]')).to have_content("##{push_pipeline.id}")
+          expect(first('[data-testid="pipeline-identifier"]')).to have_content("##{push_pipeline.id}")
         end
       end
 
@@ -252,7 +252,7 @@ RSpec.describe 'Merge request > User sees pipelines triggered by merge request',
     it 'sees branch pipelines and detached merge request pipelines in correct order' do
       page.within('.ci-table') do
         expect(page).to have_selector('.ci-pending', count: 2)
-        expect(first('[data-testid="pipeline-url-link"]')).to have_content("##{detached_merge_request_pipeline.id}")
+        expect(first('[data-testid="pipeline-identifier"]')).to have_content("##{detached_merge_request_pipeline.id}")
       end
     end
 
@@ -295,16 +295,16 @@ RSpec.describe 'Merge request > User sees pipelines triggered by merge request',
         page.within('.ci-table') do
           expect(page).to have_selector('.ci-pending', count: 4)
 
-          expect(all('[data-testid="pipeline-url-link"]')[0])
+          expect(all('[data-testid="pipeline-identifier"]')[0])
             .to have_content("##{detached_merge_request_pipeline_2.id}")
 
-          expect(all('[data-testid="pipeline-url-link"]')[1])
+          expect(all('[data-testid="pipeline-identifier"]')[1])
             .to have_content("##{detached_merge_request_pipeline.id}")
 
-          expect(all('[data-testid="pipeline-url-link"]')[2])
+          expect(all('[data-testid="pipeline-identifier"]')[2])
             .to have_content("##{push_pipeline_2.id}")
 
-          expect(all('[data-testid="pipeline-url-link"]')[3])
+          expect(all('[data-testid="pipeline-identifier"]')[3])
             .to have_content("##{push_pipeline.id}")
         end
       end

spec/features/merge_request/user_sees_pipelines_spec.rb

@@ -134,7 +134,7 @@ RSpec.describe 'Merge request > User sees pipelines', :js do
         create_merge_request_pipeline
         act_on_security_warning(action: 'Run pipeline')
 
-        check_pipeline(expected_project: parent_project)
+        check_pipeline(expected_project: parent_project, link_selector: 'pipeline-url-link')
         check_head_pipeline(expected_project: parent_project)
       end
 
@@ -179,13 +179,13 @@ RSpec.describe 'Merge request > User sees pipelines', :js do
       click_button('Run pipeline')
     end
 
-    def check_pipeline(expected_project:)
+    def check_pipeline(expected_project:, link_selector: 'commit-title')
       page.within('.ci-table') do
         expect(page).to have_selector('.commit', count: 2)
 
         page.within(first('.commit')) do
           page.within('.pipeline-tags') do
-            expect(page.find('[data-testid="pipeline-url-link"]')[:href]).to include(expected_project.full_path)
+            expect(page.find("[data-testid=#{link_selector}]")[:href]).to include(expected_project.full_path)
             expect(page).to have_content('detached')
           end
           page.within('.pipeline-triggerer') do

spec/features/projects/pipelines/pipelines_spec.rb

@@ -711,7 +711,7 @@ RSpec.describe 'Pipelines', :js do
           end
 
           expect(page.find('[data-testid="pipeline-th"]')).to have_content 'Pipeline'
-          expect(page.find('[data-testid="pipeline-url-link"]')).to have_content "##{pipeline.iid}"
+          expect(page.find('[data-testid="pipeline-identifier"]')).to have_content "##{pipeline.iid}"
         end
       end
     end

spec/frontend/pipelines/mock_data.js

@@ -676,7 +676,7 @@ export const mockPipeline = (projectPath) => {
         short_id: 'fd6df5b3',
         created_at: '2021-10-19T21:17:12.000+00:00',
         parent_ids: ['7147906b84306e83cb3fec6582a25390b75713c6'],
-        title: 'Commit',
+        title: 'Commit Title',
         message: 'Commit',
         author_name: 'Administrator',
         author_email: 'admin@example.com',
@@ -1141,3 +1141,176 @@ export const mockPipelineBranch = () => {
     viewType: 'root',
   };
 };
+
+export const mockPipelineNoCommit = () => {
+  return {
+    pipeline: {
+      id: 268,
+      iid: 34,
+      user: {
+        id: 1,
+        username: 'root',
+        name: 'Administrator',
+        state: 'active',
+        avatar_url:
+          'https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon',
+        web_url: 'http://gdk.test:3000/root',
+        show_status: false,
+        path: '/root',
+      },
+      active: false,
+      source: 'push',
+      created_at: '2022-01-14T17:40:27.866Z',
+      updated_at: '2022-01-14T18:02:35.850Z',
+      path: '/root/mr-widgets/-/pipelines/268',
+      flags: {
+        stuck: false,
+        auto_devops: false,
+        merge_request: false,
+        yaml_errors: false,
+        retryable: true,
+        cancelable: false,
+        failure_reason: false,
+        detached_merge_request_pipeline: false,
+        merge_request_pipeline: false,
+        merge_train_pipeline: false,
+        latest: true,
+      },
+      details: {
+        status: {
+          icon: 'status_warning',
+          text: 'passed',
+          label: 'passed with warnings',
+          group: 'success-with-warnings',
+          tooltip: 'passed',
+          has_details: true,
+          details_path: '/root/mr-widgets/-/pipelines/268',
+          illustration: null,
+          favicon:
+            '/assets/ci_favicons/favicon_status_success-8451333011eee8ce9f2ab25dc487fe24a8758c694827a582f17f42b0a90446a2.png',
+        },
+        stages: [
+          {
+            name: 'validate',
+            title: 'validate: passed with warnings',
+            status: {
+              icon: 'status_warning',
+              text: 'passed',
+              label: 'passed with warnings',
+              group: 'success-with-warnings',
+              tooltip: 'passed',
+              has_details: true,
+              details_path: '/root/mr-widgets/-/pipelines/268#validate',
+              illustration: null,
+              favicon:
+                '/assets/ci_favicons/favicon_status_success-8451333011eee8ce9f2ab25dc487fe24a8758c694827a582f17f42b0a90446a2.png',
+            },
+            path: '/root/mr-widgets/-/pipelines/268#validate',
+            dropdown_path: '/root/mr-widgets/-/pipelines/268/stage.json?stage=validate',
+          },
+          {
+            name: 'test',
+            title: 'test: passed',
+            status: {
+              icon: 'status_success',
+              text: 'passed',
+              label: 'passed',
+              group: 'success',
+              tooltip: 'passed',
+              has_details: true,
+              details_path: '/root/mr-widgets/-/pipelines/268#test',
+              illustration: null,
+              favicon:
+                '/assets/ci_favicons/favicon_status_success-8451333011eee8ce9f2ab25dc487fe24a8758c694827a582f17f42b0a90446a2.png',
+            },
+            path: '/root/mr-widgets/-/pipelines/268#test',
+            dropdown_path: '/root/mr-widgets/-/pipelines/268/stage.json?stage=test',
+          },
+          {
+            name: 'build',
+            title: 'build: passed',
+            status: {
+              icon: 'status_success',
+              text: 'passed',
+              label: 'passed',
+              group: 'success',
+              tooltip: 'passed',
+              has_details: true,
+              details_path: '/root/mr-widgets/-/pipelines/268#build',
+              illustration: null,
+              favicon:
+                '/assets/ci_favicons/favicon_status_success-8451333011eee8ce9f2ab25dc487fe24a8758c694827a582f17f42b0a90446a2.png',
+            },
+            path: '/root/mr-widgets/-/pipelines/268#build',
+            dropdown_path: '/root/mr-widgets/-/pipelines/268/stage.json?stage=build',
+          },
+        ],
+        duration: 75,
+        finished_at: '2022-01-14T18:02:35.842Z',
+        name: 'Pipeline',
+        manual_actions: [],
+        scheduled_actions: [],
+      },
+      ref: {
+        name: 'update-ci',
+        path: '/root/mr-widgets/-/commits/update-ci',
+        tag: false,
+        branch: true,
+        merge_request: false,
+      },
+      retry_path: '/root/mr-widgets/-/pipelines/268/retry',
+      delete_path: '/root/mr-widgets/-/pipelines/268',
+      failed_builds: [
+        {
+          id: 1260,
+          name: 'fmt',
+          started: '2022-01-14T17:40:36.435Z',
+          complete: true,
+          archived: false,
+          build_path: '/root/mr-widgets/-/jobs/1260',
+          retry_path: '/root/mr-widgets/-/jobs/1260/retry',
+          playable: false,
+          scheduled: false,
+          created_at: '2022-01-14T17:40:27.879Z',
+          updated_at: '2022-01-14T17:40:42.129Z',
+          status: {
+            icon: 'status_warning',
+            text: 'failed',
+            label: 'failed (allowed to fail)',
+            group: 'failed-with-warnings',
+            tooltip: 'failed - (script failure) (allowed to fail)',
+            has_details: true,
+            details_path: '/root/mr-widgets/-/jobs/1260',
+            illustration: {
+              image:
+                '/assets/illustrations/skipped-job_empty-29a8a37d8a61d1b6f68cf3484f9024e53cd6eb95e28eae3554f8011a1146bf27.svg',
+              size: 'svg-430',
+              title: 'This job does not have a trace.',
+            },
+            favicon:
+              '/assets/ci_favicons/favicon_status_failed-41304d7f7e3828808b0c26771f0309e55296819a9beea3ea9fbf6689d9857c12.png',
+            action: {
+              icon: 'retry',
+              title: 'Retry',
+              path: '/root/mr-widgets/-/jobs/1260/retry',
+              method: 'post',
+              button_title: 'Retry this job',
+            },
+          },
+          recoverable: false,
+        },
+      ],
+      project: {
+        id: 23,
+        name: 'mr-widgets',
+        full_path: '/root/mr-widgets',
+        full_name: 'Administrator / mr-widgets',
+      },
+      triggered_by: null,
+      triggered: [],
+    },
+    pipelineScheduleUrl: 'foo',
+    pipelineKey: 'id',
+    viewType: 'root',
+  };
+};

spec/frontend/pipelines/pipeline_url_spec.js

 import { shallowMountExtended } from 'helpers/vue_test_utils_helper';
 import { trimText } from 'helpers/text_helper';
 import PipelineUrlComponent from '~/pipelines/components/pipelines_list/pipeline_url.vue';
-import { mockPipeline, mockPipelineBranch, mockPipelineTag } from './mock_data';
+import {
+  mockPipeline,
+  mockPipelineBranch,
+  mockPipelineTag,
+  mockPipelineNoCommit,
+} from './mock_data';
 
 const projectPath = 'test/test';
 
@@ -26,7 +31,7 @@ describe('Pipeline Url Component', () => {
   const findCommitIconType = () => wrapper.findByTestId('commit-icon-type');
 
   const findCommitTitleContainer = () => wrapper.findByTestId('commit-title-container');
-  const findCommitTitle = (commitWrapper) => commitWrapper.find('[data-testid="commit-title"]');
+  const findCommitTitle = () => wrapper.findByTestId('commit-title');
 
   const defaultProps = mockPipeline(projectPath);
 
@@ -232,5 +237,33 @@ describe('Pipeline Url Component', () => {
         expect(findCommitIconType().attributes('title')).toBe(expectedTitle);
       },
     );
+
+    describe('with commit', () => {
+      beforeEach(() => {
+        createComponent({}, true);
+      });
+
+      it('displays commit title with link to pipeline', () => {
+        expect(findCommitTitle().attributes('href')).toBe(defaultProps.pipeline.path);
+      });
+
+      it('displays commit title text', () => {
+        expect(findCommitTitle().text()).toBe(defaultProps.pipeline.commit.title);
+      });
+    });
+
+    describe('without commit', () => {
+      beforeEach(() => {
+        createComponent(mockPipelineNoCommit(), true);
+      });
+
+      it('displays cant find head commit text', () => {
+        expect(findCommitTitle().text()).toBe("Can't find HEAD commit for this branch");
+      });
+
+      it('displays link to pipeline', () => {
+        expect(findCommitTitle().attributes('href')).toBe(mockPipelineNoCommit().pipeline.path);
+      });
+    });
   });
 });