From 9124f0b6015638ad08c0b643c29d927e1f76df62 Mon Sep 17 00:00:00 2001
From: Reuben Pereira <rpereira@gitlab.com>
Date: Tue, 18 Oct 2022 14:56:52 +0530
Subject: [PATCH] Display pipeline name on pipeline details and list pages

The pipeline name is displayed instead of the commit title on the
list page.
If a pipeline does not have a name, the commit title is displayed.

On the details page, the pipeline name is displayed as the header
instead of the commit title. The commit title is moved to the row
that currently only displays the commit SHA.
---
 .../pipelines_list/pipeline_url.vue           | 16 ++++++-
 app/serializers/ci/pipeline_entity.rb         |  1 +
 .../merge_requests/pipeline_entity.rb         |  1 +
 app/serializers/pipeline_serializer.rb        |  1 +
 app/views/projects/pipelines/_info.html.haml  | 27 ++++++++----
 ...ines#index-get_list_project_pipelines.json |  3 ++
 .../projects/pipelines/pipeline_spec.rb       | 43 ++++++++++++++++++-
 spec/frontend/pipelines/mock_data.js          |  2 +
 spec/frontend/pipelines/pipeline_url_spec.js  | 16 ++++++-
 spec/serializers/ci/pipeline_entity_spec.rb   | 13 +++++-
 .../merge_requests/pipeline_entity_spec.rb    | 14 +++++-
 spec/serializers/pipeline_serializer_spec.rb  |  1 +
 12 files changed, 122 insertions(+), 16 deletions(-)

diff --git a/app/assets/javascripts/pipelines/components/pipelines_list/pipeline_url.vue b/app/assets/javascripts/pipelines/components/pipelines_list/pipeline_url.vue
index 39d41415456277..fe2ef2c2d7138f 100644
--- a/app/assets/javascripts/pipelines/components/pipelines_list/pipeline_url.vue
+++ b/app/assets/javascripts/pipelines/components/pipelines_list/pipeline_url.vue
@@ -115,6 +115,9 @@ export default {
     commitTitle() {
       return this.pipeline?.commit?.title;
     },
+    pipelineName() {
+      return this.pipeline?.name;
+    },
   },
   methods: {
     trackClick(action) {
@@ -125,7 +128,18 @@ export default {
 </script>
 <template>
   <div class="pipeline-tags" data-testid="pipeline-url-table-cell">
-    <div class="commit-title gl-mb-2" data-testid="commit-title-container">
+    <div v-if="pipelineName" class="gl-mb-2" data-testid="pipeline-name-container">
+      <span class="gl-display-flex">
+        <tooltip-on-truncate
+          :title="pipelineName"
+          class="gl-flex-grow-1 gl-text-truncate gl-text-gray-900"
+        >
+          {{ pipelineName }}
+        </tooltip-on-truncate>
+      </span>
+    </div>
+
+    <div v-if="!pipelineName" 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="gl-flex-grow-1 gl-text-truncate">
           <gl-link
diff --git a/app/serializers/ci/pipeline_entity.rb b/app/serializers/ci/pipeline_entity.rb
index 8edb07e4f59382..143017c5159094 100644
--- a/app/serializers/ci/pipeline_entity.rb
+++ b/app/serializers/ci/pipeline_entity.rb
@@ -10,6 +10,7 @@ class Ci::PipelineEntity < Grape::Entity
   expose :iid
   expose :user, using: UserEntity
   expose :active?, as: :active
+  expose :name, if: -> (pipeline, _) { Feature.enabled?(:pipeline_name, pipeline.project) }
 
   # Coverage isn't always necessary (e.g. when displaying project pipelines in
   # the UI). Instead of creating an entirely different entity we just allow the
diff --git a/app/serializers/merge_requests/pipeline_entity.rb b/app/serializers/merge_requests/pipeline_entity.rb
index 9c2545a283410d..76e75a8ca6dfaf 100644
--- a/app/serializers/merge_requests/pipeline_entity.rb
+++ b/app/serializers/merge_requests/pipeline_entity.rb
@@ -5,6 +5,7 @@ class MergeRequests::PipelineEntity < Grape::Entity
 
   expose :id
   expose :active?, as: :active
+  expose :name, if: -> (pipeline, _) { Feature.enabled?(:pipeline_name, pipeline.project) }
 
   expose :path do |pipeline|
     project_pipeline_path(pipeline.project, pipeline)
diff --git a/app/serializers/pipeline_serializer.rb b/app/serializers/pipeline_serializer.rb
index 9cfc81e87059f2..b738438a78fdf1 100644
--- a/app/serializers/pipeline_serializer.rb
+++ b/app/serializers/pipeline_serializer.rb
@@ -40,6 +40,7 @@ def represent_stages(resource)
 
   def preloaded_relations
     [
+      :pipeline_metadata,
       :cancelable_statuses,
       :retryable_builds,
       :stages,
diff --git a/app/views/projects/pipelines/_info.html.haml b/app/views/projects/pipelines/_info.html.haml
index 7c4b0a5fdb9605..30cc7f9431169b 100644
--- a/app/views/projects/pipelines/_info.html.haml
+++ b/app/views/projects/pipelines/_info.html.haml
@@ -1,9 +1,13 @@
-.commit-box
-  %h3.commit-title
-    = markdown(commit.title, pipeline: :single_line)
-  - if commit.description.present?
-    %pre.commit-description<
-      = preserve(markdown(commit.description, pipeline: :single_line))
+- if Feature.enabled?(:pipeline_name, @pipeline.project) && @pipeline.name
+  %h3
+    = @pipeline.name
+- else
+  .commit-box
+    %h3.commit-title
+      = markdown(commit.title, pipeline: :single_line)
+    - if commit.description.present?
+      %pre.commit-description<
+        = preserve(markdown(commit.description, pipeline: :single_line))
 
 .info-well
   .well-segment.pipeline-info{ class: "gl-align-items-baseline!" }
@@ -45,11 +49,16 @@
       - if @pipeline.stuck?
         = gl_badge_tag s_('Pipelines|stuck'), { variant: :warning, size: :sm }, { class: 'js-pipeline-url-stuck has-tooltip' }
 
-  .well-segment
+  .well-segment{ 'data-testid': 'commit-row' }
     .icon-container.commit-icon
       = sprite_icon('commit', css_class: 'gl-top-0!')
-    = link_to commit.short_id, project_commit_path(@project, @pipeline.sha), class: "commit-sha"
-    = clipboard_button(text: @pipeline.sha, title: _("Copy commit SHA"))
+    - if Feature.enabled?(:pipeline_name, @pipeline.project) && @pipeline.name
+      = markdown(commit.title, pipeline: :single_line)
+      = clipboard_button(text: @pipeline.sha, title: _("Copy commit SHA"))
+      = link_to commit.short_id, project_commit_path(@project, @pipeline.sha), class: "commit-sha"
+    - else
+      = link_to commit.short_id, project_commit_path(@project, @pipeline.sha), class: "commit-sha"
+      = clipboard_button(text: @pipeline.sha, title: _("Copy commit SHA"))
 
   .well-segment.related-merge-request-info
     .icon-container
diff --git a/spec/contracts/contracts/project/pipeline/index/pipelines#index-get_list_project_pipelines.json b/spec/contracts/contracts/project/pipeline/index/pipelines#index-get_list_project_pipelines.json
index 2a01d85c0ee042..2ebfd1bfdf27b5 100644
--- a/spec/contracts/contracts/project/pipeline/index/pipelines#index-get_list_project_pipelines.json
+++ b/spec/contracts/contracts/project/pipeline/index/pipelines#index-get_list_project_pipelines.json
@@ -198,6 +198,9 @@
             "match": "regex",
             "regex": "^(push|web|trigger|schedule|api|external|pipeline|chat|webide|merge_request_event|external_pull_request_event|parent_pipeline|ondemand_dast_scan|ondemand_dast_validation)$"
           },
+          "$.body.pipelines[*].name": {
+            "match": "type"
+          },
           "$.body.pipelines[*].created_at": {
             "match": "regex",
             "regex": "^\\d{4}-[01]\\d-[0-3]\\dT[0-2]\\d:[0-5]\\d:[0-5]\\d\\.\\d+([+-][0-2]\\d(:?[0-5]\\d)?|Z)$"
diff --git a/spec/features/projects/pipelines/pipeline_spec.rb b/spec/features/projects/pipelines/pipeline_spec.rb
index 66916aea8d7e42..2d729af513a18e 100644
--- a/spec/features/projects/pipelines/pipeline_spec.rb
+++ b/spec/features/projects/pipelines/pipeline_spec.rb
@@ -64,7 +64,9 @@
     let_it_be(:group) { create(:group) }
     let_it_be(:project, reload: true) { create(:project, :repository, group: group) }
 
-    let(:pipeline) { create(:ci_pipeline, project: project, ref: 'master', sha: project.commit.id, user: user) }
+    let(:pipeline) do
+      create(:ci_pipeline, name: 'Build pipeline', project: project, ref: 'master', sha: project.commit.id, user: user)
+    end
 
     subject(:visit_pipeline) { visit project_pipeline_path(project, pipeline) }
 
@@ -96,6 +98,45 @@
       end
     end
 
+    context 'with pipeline_name feature flag enabled' do
+      before do
+        stub_feature_flags(pipeline_name: true)
+      end
+
+      it 'displays pipeline name instead of commit title' do
+        visit_pipeline
+
+        within 'h3' do
+          expect(page).to have_content(pipeline.name)
+        end
+
+        within '.well-segment[data-testid="commit-row"]' do
+          expect(page).to have_content(project.commit.title)
+          expect(page).to have_content(project.commit.short_id)
+        end
+      end
+    end
+
+    context 'with pipeline_name feature flag disabled' do
+      before do
+        stub_feature_flags(pipeline_name: false)
+      end
+
+      it 'displays commit title' do
+        visit_pipeline
+
+        within 'h3' do
+          expect(page).not_to have_content(pipeline.name)
+          expect(page).to have_content(project.commit.title)
+        end
+
+        within '.well-segment[data-testid="commit-row"]' do
+          expect(page).not_to have_content(project.commit.title)
+          expect(page).to have_content(project.commit.short_id)
+        end
+      end
+    end
+
     describe 'related merge requests' do
       context 'when there are no related merge requests' do
         it 'shows a "no related merge requests" message' do
diff --git a/spec/frontend/pipelines/mock_data.js b/spec/frontend/pipelines/mock_data.js
index 26e04903163c34..36bce65dd56609 100644
--- a/spec/frontend/pipelines/mock_data.js
+++ b/spec/frontend/pipelines/mock_data.js
@@ -730,6 +730,7 @@ export const mockPipelineTag = () => {
       },
       active: false,
       source: 'push',
+      name: 'Build pipeline',
       created_at: '2022-02-02T15:39:04.012Z',
       updated_at: '2022-02-02T15:40:59.573Z',
       path: '/root/mr-widgets/-/pipelines/311',
@@ -955,6 +956,7 @@ export const mockPipelineBranch = () => {
       },
       active: false,
       source: 'push',
+      name: 'Build pipeline',
       created_at: '2022-01-14T17:40:27.866Z',
       updated_at: '2022-01-14T18:02:35.850Z',
       path: '/root/mr-widgets/-/pipelines/268',
diff --git a/spec/frontend/pipelines/pipeline_url_spec.js b/spec/frontend/pipelines/pipeline_url_spec.js
index 1d66607e72b2f6..c62898f0c834d2 100644
--- a/spec/frontend/pipelines/pipeline_url_spec.js
+++ b/spec/frontend/pipelines/pipeline_url_spec.js
@@ -1,3 +1,4 @@
+import { merge } from 'lodash';
 import { mockTracking, unmockTracking } from 'helpers/tracking_helper';
 import { shallowMountExtended } from 'helpers/vue_test_utils_helper';
 import PipelineUrlComponent from '~/pipelines/components/pipelines_list/pipeline_url.vue';
@@ -20,6 +21,7 @@ describe('Pipeline Url Component', () => {
   const findCommitRefName = () => wrapper.findByTestId('commit-ref-name');
 
   const findCommitTitleContainer = () => wrapper.findByTestId('commit-title-container');
+  const findPipelineNameContainer = () => wrapper.findByTestId('pipeline-name-container');
   const findCommitTitle = (commitWrapper) => commitWrapper.find('[data-testid="commit-title"]');
 
   const defaultProps = mockPipeline(projectPath);
@@ -51,7 +53,16 @@ describe('Pipeline Url Component', () => {
     expect(findPipelineUrlLink().text()).toBe('#1');
   });
 
-  it('should render the commit title, commit reference and commit-short-sha', () => {
+  it('should render the pipeline name instead of commit title', () => {
+    createComponent(merge(mockPipeline(projectPath), { pipeline: { name: 'Build pipeline' } }));
+
+    expect(findCommitTitleContainer().exists()).toBe(false);
+    expect(findPipelineNameContainer().exists()).toBe(true);
+    expect(findRefName().exists()).toBe(true);
+    expect(findCommitShortSha().exists()).toBe(true);
+  });
+
+  it('should render the commit title when pipeline has no name', () => {
     createComponent();
 
     const commitWrapper = findCommitTitleContainer();
@@ -59,6 +70,7 @@ describe('Pipeline Url Component', () => {
     expect(findCommitTitle(commitWrapper).exists()).toBe(true);
     expect(findRefName().exists()).toBe(true);
     expect(findCommitShortSha().exists()).toBe(true);
+    expect(findPipelineNameContainer().exists()).toBe(false);
   });
 
   describe('commit user avatar', () => {
@@ -142,7 +154,7 @@ describe('Pipeline Url Component', () => {
     });
 
     it('tracks commit title click', () => {
-      createComponent(mockPipelineBranch());
+      createComponent(merge(mockPipelineBranch(), { pipeline: { name: null } }));
 
       findCommitTitle(findCommitTitleContainer()).vm.$emit('click');
 
diff --git a/spec/serializers/ci/pipeline_entity_spec.rb b/spec/serializers/ci/pipeline_entity_spec.rb
index 7889ac03f8fa4d..ff364918b4f380 100644
--- a/spec/serializers/ci/pipeline_entity_spec.rb
+++ b/spec/serializers/ci/pipeline_entity_spec.rb
@@ -15,12 +15,13 @@
     subject { entity.as_json }
 
     context 'when pipeline is empty' do
-      let(:pipeline) { create(:ci_empty_pipeline) }
+      let(:pipeline) { create(:ci_empty_pipeline, name: 'Build pipeline') }
 
       it 'contains required fields' do
         expect(subject).to include :id, :iid, :user, :path, :coverage, :source
         expect(subject).to include :ref, :commit
         expect(subject).to include :updated_at, :created_at
+        expect(subject[:name]).to eq('Build pipeline')
       end
 
       it 'excludes coverage data when disabled' do
@@ -48,6 +49,16 @@
           .to include :stuck, :auto_devops, :yaml_errors,
                       :retryable, :cancelable, :merge_request
       end
+
+      context 'when pipeline_name feature flag is disabled' do
+        before do
+          stub_feature_flags(pipeline_name: false)
+        end
+
+        it 'does not return name' do
+          is_expected.not_to include(:name)
+        end
+      end
     end
 
     context 'when default branch not protected' do
diff --git a/spec/serializers/merge_requests/pipeline_entity_spec.rb b/spec/serializers/merge_requests/pipeline_entity_spec.rb
index 31ebe7387438c2..a8f4fc44f1002a 100644
--- a/spec/serializers/merge_requests/pipeline_entity_spec.rb
+++ b/spec/serializers/merge_requests/pipeline_entity_spec.rb
@@ -5,7 +5,7 @@
 RSpec.describe MergeRequests::PipelineEntity do
   let_it_be(:project) { create(:project, :repository) }
   let_it_be(:user) { create(:user) }
-  let_it_be(:pipeline) { create(:ci_pipeline, project: project) }
+  let_it_be(:pipeline) { create(:ci_pipeline, project: project, name: 'Build pipeline') }
 
   let(:request) { double('request') }
 
@@ -29,7 +29,7 @@
 
       is_expected.to include(
         :id, :path, :active, :coverage, :ref, :commit, :details,
-        :flags, :triggered, :triggered_by
+        :flags, :triggered, :triggered_by, :name
       )
       expect(subject[:commit]).to include(:short_id, :commit_path)
       expect(subject[:ref]).to include(:branch)
@@ -51,5 +51,15 @@
 
       expect(entity.as_json).not_to include(:coverage)
     end
+
+    context 'when pipeline_name feature flag is disabled' do
+      before do
+        stub_feature_flags(pipeline_name: false)
+      end
+
+      it 'does not return name' do
+        is_expected.not_to include(:name)
+      end
+    end
   end
 end
diff --git a/spec/serializers/pipeline_serializer_spec.rb b/spec/serializers/pipeline_serializer_spec.rb
index 4d9bdc4bb178f9..29b7107fc02d2e 100644
--- a/spec/serializers/pipeline_serializer_spec.rb
+++ b/spec/serializers/pipeline_serializer_spec.rb
@@ -240,6 +240,7 @@ def create_pipeline(status)
         create(:ci_empty_pipeline,
                project: project,
                status: status,
+               name: 'Build pipeline',
                ref: 'feature').tap do |pipeline|
           Ci::Build::AVAILABLE_STATUSES.each do |build_status|
             create_build(pipeline, status, build_status)
-- 
GitLab