Merge branch 'make-bridge-pipelines-clickable' into 'master'

Make bridge/child pipelines clickable

See merge request gitlab-org/gitlab!41263

app/assets/javascripts/pipelines/components/graph/job_item.vue

@@ -132,8 +132,9 @@ export default {
       v-gl-tooltip="{ boundary, placement: 'bottom' }"
       :href="status.details_path"
       :title="tooltipText"
-      :class="cssClassJobName"
+      :class="jobClasses"
       class="js-pipeline-graph-job-link qa-job-link menu-item"
+      data-testid="job-with-link"
     >
       <job-name-component :name="job.name" :status="job.status" />
     </gl-link>

changelogs/unreleased/make-bridge-pipelines-clickable.yml

+---
+title: Make bridge/child pipelines clickable
+merge_request: 41263
+author:
+type: added

config/feature_flags/development/ci_bridge_pipeline_details.yml

+---
+name: ci_bridge_pipeline_details
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/41263
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/250683
+group: group::memory
+type: development
+default_enabled: true

lib/gitlab/ci/status/bridge/common.rb

@@ -10,14 +10,28 @@ module Gitlab
           end
 
           def has_details?
-            false
+            !!details_path
+          end
+
+          def details_path
+            return unless Feature.enabled?(:ci_bridge_pipeline_details, subject.project, default_enabled: true)
+            return unless can?(user, :read_pipeline, downstream_pipeline)
+
+            project_pipeline_path(downstream_project, downstream_pipeline)
           end
 
           def has_action?
             false
           end
 
-          def details_path
+          private
+
+          def downstream_pipeline
+            subject.downstream_pipeline
+          end
+
+          def downstream_project
+            downstream_pipeline&.project
           end
         end
       end

spec/frontend/pipelines/graph/job_item_spec.js

@@ -6,6 +6,7 @@ describe('pipeline graph job item', () => {
   let wrapper;
 
   const findJobWithoutLink = () => wrapper.find('[data-testid="job-without-link"]');
+  const findJobWithLink = () => wrapper.find('[data-testid="job-with-link"]');
 
   const createWrapper = propsData => {
     wrapper = mount(JobItem, {
@@ -36,7 +37,7 @@ describe('pipeline graph job item', () => {
   };
   const mockJobWithoutDetails = {
     id: 4257,
-    name: 'test',
+    name: 'job_without_details',
     status: {
       icon: 'status_success',
       text: 'passed',
@@ -60,7 +61,7 @@ describe('pipeline graph job item', () => {
 
         expect(link.attributes('href')).toBe(mockJob.status.details_path);
 
-        expect(link.attributes('title')).toEqual(`${mockJob.name} - ${mockJob.status.label}`);
+        expect(link.attributes('title')).toBe(`${mockJob.name} - ${mockJob.status.label}`);
 
         expect(wrapper.find('.ci-status-icon-success').exists()).toBe(true);
 
@@ -84,11 +85,10 @@ describe('pipeline graph job item', () => {
       expect(wrapper.find('.ci-status-icon-success').exists()).toBe(true);
       expect(wrapper.find('a').exists()).toBe(false);
 
-      expect(trimText(wrapper.find('.ci-status-text').text())).toEqual(mockJob.name);
+      expect(trimText(wrapper.find('.ci-status-text').text())).toBe(mockJobWithoutDetails.name);
     });
 
     it('should apply hover class and provided class name', () => {
-      expect(findJobWithoutLink().classes()).toContain(triggerActiveClass);
       expect(findJobWithoutLink().classes()).toContain('css-class-job-name');
     });
   });
@@ -139,9 +139,7 @@ describe('pipeline graph job item', () => {
         },
       });
 
-      expect(wrapper.find('.js-job-component-tooltip').attributes('title')).toEqual(
-        'test - success',
-      );
+      expect(wrapper.find('.js-job-component-tooltip').attributes('title')).toBe('test - success');
     });
   });
 
@@ -151,29 +149,39 @@ describe('pipeline graph job item', () => {
         job: delayedJobFixture,
       });
 
-      expect(wrapper.find('.js-pipeline-graph-job-link').attributes('title')).toEqual(
+      expect(findJobWithLink().attributes('title')).toBe(
         `delayed job - delayed manual action (${wrapper.vm.remainingTime})`,
       );
     });
   });
 
   describe('trigger job highlighting', () => {
-    it('trigger job should stay highlighted when downstream is expanded', () => {
-      createWrapper({
-        job: mockJobWithoutDetails,
-        pipelineExpanded: { jobName: mockJob.name, expanded: true },
-      });
-
-      expect(findJobWithoutLink().classes()).toContain(triggerActiveClass);
-    });
-
-    it('trigger job should not be highlighted when downstream is closed', () => {
-      createWrapper({
-        job: mockJobWithoutDetails,
-        pipelineExpanded: { jobName: mockJob.name, expanded: false },
-      });
-
-      expect(findJobWithoutLink().classes()).not.toContain(triggerActiveClass);
-    });
+    it.each`
+      job                      | jobName                       | expanded | link
+      ${mockJob}               | ${mockJob.name}               | ${true}  | ${true}
+      ${mockJobWithoutDetails} | ${mockJobWithoutDetails.name} | ${true}  | ${false}
+    `(
+      `trigger job should stay highlighted when downstream is expanded`,
+      ({ job, jobName, expanded, link }) => {
+        createWrapper({ job, pipelineExpanded: { jobName, expanded } });
+        const findJobEl = link ? findJobWithLink : findJobWithoutLink;
+
+        expect(findJobEl().classes()).toContain(triggerActiveClass);
+      },
+    );
+
+    it.each`
+      job                      | jobName                       | expanded | link
+      ${mockJob}               | ${mockJob.name}               | ${false} | ${true}
+      ${mockJobWithoutDetails} | ${mockJobWithoutDetails.name} | ${false} | ${false}
+    `(
+      `trigger job should not be highlighted when downstream is not expanded`,
+      ({ job, jobName, expanded, link }) => {
+        createWrapper({ job, pipelineExpanded: { jobName, expanded } });
+        const findJobEl = link ? findJobWithLink : findJobWithoutLink;
+
+        expect(findJobEl().classes()).not.toContain(triggerActiveClass);
+      },
+    );
   });
 });

spec/lib/gitlab/ci/status/bridge/common_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Ci::Status::Bridge::Common do
+  let_it_be(:user) { create(:user) }
+  let_it_be(:bridge) { create(:ci_bridge) }
+  let_it_be(:downstream_pipeline) { create(:ci_pipeline) }
+
+  before_all do
+    create(:ci_sources_pipeline,
+      source_pipeline: bridge.pipeline,
+      source_project: bridge.pipeline.project,
+      source_job: bridge,
+      pipeline: downstream_pipeline,
+      project: downstream_pipeline.project)
+  end
+
+  subject do
+    Gitlab::Ci::Status::Core
+      .new(bridge, user)
+      .extend(described_class)
+  end
+
+  describe '#details_path' do
+    context 'when user has access to read downstream pipeline' do
+      before do
+        downstream_pipeline.project.add_developer(user)
+      end
+
+      it { expect(subject).to have_details }
+      it { expect(subject.details_path).to include "pipelines/#{downstream_pipeline.id}" }
+
+      context 'when ci_bridge_pipeline_details is disabled' do
+        before do
+          stub_feature_flags(ci_bridge_pipeline_details: false)
+        end
+
+        it { expect(subject).not_to have_details }
+        it { expect(subject.details_path).to be_nil }
+      end
+    end
+
+    context 'when user does not have access to read downstream pipeline' do
+      it { expect(subject).not_to have_details }
+      it { expect(subject.details_path).to be_nil }
+    end
+  end
+end