Merge branch 'display-artifacts-dropdown-on-mr-widget' into 'master'

Display Artifacts Dropdown on MR Pipeline Widget

See merge request gitlab-org/gitlab!50998

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

 <script>
-/* eslint-disable @gitlab/vue-require-i18n-strings */
-import { GlLink, GlTooltipDirective, GlIcon } from '@gitlab/ui';
+import { GlDropdown, GlDropdownItem, GlSprintf, GlTooltipDirective } from '@gitlab/ui';
+import { __ } from '~/locale';
 
 export default {
   directives: {
     GlTooltip: GlTooltipDirective,
   },
   components: {
-    GlIcon,
-    GlLink,
+    GlDropdown,
+    GlDropdownItem,
+    GlSprintf,
+  },
+  translations: {
+    artifacts: __('Artifacts'),
+    downloadArtifact: __('Download %{name} artifact'),
   },
   props: {
     artifacts: {
@@ -19,24 +24,25 @@ export default {
 };
 </script>
 <template>
-  <div class="btn-group" role="group">
-    <button
-      v-gl-tooltip
-      type="button"
-      class="dropdown-toggle build-artifacts btn btn-default js-pipeline-dropdown-download"
-      :title="__('Artifacts')"
-      data-toggle="dropdown"
-      :aria-label="__('Artifacts')"
+  <gl-dropdown
+    v-gl-tooltip
+    class="build-artifacts js-pipeline-dropdown-download"
+    :title="$options.translations.artifacts"
+    :text="$options.translations.artifacts"
+    :aria-label="$options.translations.artifacts"
+    icon="download"
+    text-sr-only
+  >
+    <gl-dropdown-item
+      v-for="(artifact, i) in artifacts"
+      :key="i"
+      :href="artifact.path"
+      rel="nofollow"
+      download
     >
-      <gl-icon name="download" />
-      <gl-icon name="chevron-down" />
-    </button>
-    <ul class="dropdown-menu dropdown-menu-right">
-      <li v-for="(artifact, i) in artifacts" :key="i">
-        <gl-link :href="artifact.path" rel="nofollow" download
-          >Download {{ artifact.name }} artifact</gl-link
-        >
-      </li>
-    </ul>
-  </div>
+      <gl-sprintf :message="$options.translations.downloadArtifact">
+        <template #name>{{ artifact.name }}</template>
+      </gl-sprintf>
+    </gl-dropdown-item>
+  </gl-dropdown>
 </template>

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

@@ -346,7 +346,6 @@ export default {
         <pipelines-artifacts-component
           v-if="pipeline.details.artifacts.length"
           :artifacts="pipeline.details.artifacts"
-          class="d-md-block"
         />
 
         <gl-button

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

@@ -11,6 +11,7 @@ import {
 import mrWidgetPipelineMixin from 'ee_else_ce/vue_merge_request_widget/mixins/mr_widget_pipeline';
 import { s__, n__ } from '~/locale';
 import PipelineStage from '~/pipelines/components/pipelines_list/stage.vue';
+import PipelineArtifacts from '~/pipelines/components/pipelines_list/pipelines_artifacts.vue';
 import CiIcon from '~/vue_shared/components/ci_icon.vue';
 import TooltipOnTruncate from '~/vue_shared/components/tooltip_on_truncate.vue';
 
@@ -23,6 +24,7 @@ export default {
     GlIcon,
     GlSprintf,
     GlTooltip,
+    PipelineArtifacts,
     PipelineStage,
     TooltipOnTruncate,
     LinkedPipelinesMiniList: () =>
@@ -97,6 +99,9 @@ export default {
     hasCommitInfo() {
       return this.pipeline.commit && Object.keys(this.pipeline.commit).length > 0;
     },
+    hasArtifacts() {
+      return this.pipeline?.details?.artifacts?.length > 0;
+    },
     isMergeRequestPipeline() {
       return Boolean(this.pipeline.flags && this.pipeline.flags.merge_request_pipeline);
     },
@@ -218,7 +223,6 @@ export default {
                 data-testid="pipeline-coverage-delta"
                 >({{ pipelineCoverageDelta }}%)</span
               >
-
               {{ pipelineCoverageJobNumberText }}
               <span ref="pipelineCoverageQuestion">
                 <gl-icon name="question" :size="12" />
@@ -258,6 +262,11 @@ export default {
               </template>
             </span>
             <linked-pipelines-mini-list v-if="triggered.length" :triggered="triggered" />
+            <pipeline-artifacts
+              v-if="hasArtifacts"
+              :artifacts="pipeline.details.artifacts"
+              class="gl-ml-3"
+            />
           </span>
         </div>
       </div>

app/serializers/merge_requests/pipeline_entity.rb

@@ -21,6 +21,16 @@ class MergeRequests::PipelineEntity < Grape::Entity
       pipeline.present.name
     end
 
+    expose :artifacts do |pipeline, options|
+      rel = pipeline.downloadable_artifacts
+
+      if Feature.enabled?(:non_public_artifacts, type: :development)
+        rel = rel.select { |artifact| can?(request.current_user, :read_job_artifacts, artifact.job) }
+      end
+
+      BuildArtifactEntity.represent(rel, options)
+    end
+
     expose :detailed_status, as: :status, with: DetailedStatusEntity do |pipeline|
       pipeline.detailed_status(request.current_user)
     end

changelogs/unreleased/display-artifacts-dropdown-on-mr-widget.yml

+---
+title: Display Artifacts Dropdown on MR Pipeline Widget
+merge_request: 50998
+author:
+type: added

locale/gitlab.pot

@@ -10200,6 +10200,9 @@ msgstr ""
 msgid "Download %{format}:"
 msgstr ""
 
+msgid "Download %{name} artifact"
+msgstr ""
+
 msgid "Download CSV"
 msgstr ""
 

spec/frontend/pipelines/pipelines_artifacts_spec.js

-import { shallowMount } from '@vue/test-utils';
-import { GlLink } from '@gitlab/ui';
+import { mount } from '@vue/test-utils';
+import { GlDropdown, GlDropdownItem } from '@gitlab/ui';
 import PipelineArtifacts from '~/pipelines/components/pipelines_list/pipelines_artifacts.vue';
 
 describe('Pipelines Artifacts dropdown', () => {
   let wrapper;
 
   const createComponent = () => {
-    wrapper = shallowMount(PipelineArtifacts, {
+    wrapper = mount(PipelineArtifacts, {
       propsData: {
         artifacts: [
           {
@@ -22,8 +22,8 @@ describe('Pipelines Artifacts dropdown', () => {
     });
   };
 
-  const findGlLink = () => wrapper.find(GlLink);
-  const findAllGlLinks = () => wrapper.find('.dropdown-menu').findAll(GlLink);
+  const findFirstGlDropdownItem = () => wrapper.find(GlDropdownItem);
+  const findAllGlDropdownItems = () => wrapper.find(GlDropdown).findAll(GlDropdownItem);
 
   beforeEach(() => {
     createComponent();
@@ -35,12 +35,12 @@ describe('Pipelines Artifacts dropdown', () => {
   });
 
   it('should render a dropdown with all the provided artifacts', () => {
-    expect(findAllGlLinks()).toHaveLength(2);
+    expect(findAllGlDropdownItems()).toHaveLength(2);
   });
 
   it('should render a link with the provided path', () => {
-    expect(findGlLink().attributes('href')).toEqual('/download/path');
+    expect(findFirstGlDropdownItem().find('a').attributes('href')).toEqual('/download/path');
 
-    expect(findGlLink().text()).toContain('artifact');
+    expect(findFirstGlDropdownItem().text()).toContain('artifact');
   });
 });

spec/frontend/vue_mr_widget/components/artifacts_list_app_spec.js

@@ -7,7 +7,7 @@ import { TEST_HOST as FAKE_ENDPOINT } from 'helpers/test_constants';
 import axios from '~/lib/utils/axios_utils';
 import ArtifactsListApp from '~/vue_merge_request_widget/components/artifacts_list_app.vue';
 import { getStoreConfig } from '~/vue_merge_request_widget/stores/artifacts_list';
-import { artifactsList } from './mock_data';
+import { artifacts } from '../mock_data';
 
 Vue.use(Vuex);
 
@@ -78,9 +78,9 @@ describe('Merge Requests Artifacts list app', () => {
   describe('with results', () => {
     beforeEach(() => {
       createComponent();
-      mock.onGet(FAKE_ENDPOINT).reply(200, artifactsList, {});
+      mock.onGet(FAKE_ENDPOINT).reply(200, artifacts, {});
       store.dispatch('receiveArtifactsSuccess', {
-        data: artifactsList,
+        data: artifacts,
         status: 200,
       });
       return nextTick();

spec/frontend/vue_mr_widget/components/artifacts_list_spec.js

 import { shallowMount } from '@vue/test-utils';
 import { GlLink } from '@gitlab/ui';
 import ArtifactsList from '~/vue_merge_request_widget/components/artifacts_list.vue';
-import { artifactsList } from './mock_data';
+import { artifacts } from '../mock_data';
 
 describe('Artifacts List', () => {
   let wrapper;
 
   const data = {
-    artifacts: artifactsList,
+    artifacts,
   };
 
   const mountComponent = (props) => {

spec/frontend/vue_mr_widget/components/mock_data.js

-export const artifactsList = [
-  {
-    text: 'result.txt',
-    url: 'bar',
-    job_name: 'generate-artifact',
-    job_path: 'bar',
-  },
-  {
-    text: 'foo.txt',
-    url: 'foo',
-    job_name: 'foo-artifact',
-    job_path: 'foo',
-  },
-];

spec/frontend/vue_mr_widget/mock_data.js

 import { SUCCESS } from '~/vue_merge_request_widget/components/deployment/constants';
 
+export const artifacts = [
+  {
+    text: 'result.txt',
+    url: 'bar',
+    job_name: 'generate-artifact',
+    job_path: 'bar',
+  },
+  {
+    text: 'foo.txt',
+    url: 'foo',
+    job_name: 'foo-artifact',
+    job_path: 'foo',
+  },
+];
+
 export default {
   id: 132,
   iid: 22,
@@ -84,6 +99,7 @@ export default {
     coverage: '92.16',
     path: '/root/acets-app/pipelines/172',
     details: {
+      artifacts,
       status: {
         icon: 'status_success',
         favicon: 'favicon_status_success',
@@ -127,7 +143,6 @@ export default {
           dropdown_path: '/root/acets-app/pipelines/172/stage.json?stage=review',
         },
       ],
-      artifacts: [],
       manual_actions: [
         {
           name: 'stop_review',
@@ -275,6 +290,7 @@ export const mockStore = {
   pipeline: {
     id: 0,
     details: {
+      artifacts,
       status: {
         details_path: '/root/review-app-tester/pipelines/66',
         favicon:
@@ -294,6 +310,7 @@ export const mockStore = {
   mergePipeline: {
     id: 1,
     details: {
+      artifacts,
       status: {
         details_path: '/root/review-app-tester/pipelines/66',
         favicon:

spec/frontend/vue_mr_widget/stores/artifacts_list/getters_spec.js

 import { title } from '~/vue_merge_request_widget/stores/artifacts_list/getters';
 import state from '~/vue_merge_request_widget/stores/artifacts_list/state';
-import { artifactsList } from '../../components/mock_data';
+import { artifacts } from '../../mock_data';
 
 describe('Artifacts Store Getters', () => {
   let localState;
@@ -24,7 +24,7 @@ describe('Artifacts Store Getters', () => {
     });
     describe('when it has artifacts', () => {
       it('returns artifacts message', () => {
-        localState.artifacts = artifactsList;
+        localState.artifacts = artifacts;
         expect(title(localState)).toBe('View 2 exposed artifacts');
       });
     });

spec/serializers/merge_requests/pipeline_entity_spec.rb

@@ -30,7 +30,7 @@ RSpec.describe MergeRequests::PipelineEntity do
       )
       expect(subject[:commit]).to include(:short_id, :commit_path)
       expect(subject[:ref]).to include(:branch)
-      expect(subject[:details]).to include(:name, :status, :stages)
+      expect(subject[:details]).to include(:artifacts, :name, :status, :stages)
       expect(subject[:details][:status]).to include(:icon, :favicon, :text, :label, :tooltip)
       expect(subject[:flags]).to include(:merge_request_pipeline)
     end
@@ -42,4 +42,6 @@ RSpec.describe MergeRequests::PipelineEntity do
       expect(entity.as_json).not_to include(:coverage)
     end
   end
+
+  it_behaves_like 'public artifacts'
 end

spec/serializers/pipeline_details_entity_spec.rb

@@ -184,25 +184,6 @@ RSpec.describe PipelineDetailsEntity do
       end
     end
 
-    context 'when a pipeline belongs to a public project' do
-      let(:project) { create(:project, :public) }
-      let(:pipeline) { create(:ci_empty_pipeline, status: :success, project: project) }
-
-      context 'that has artifacts' do
-        let!(:build) { create(:ci_build, :success, :artifacts, pipeline: pipeline) }
-
-        it 'contains information about artifacts' do
-          expect(subject[:details][:artifacts].length).to eq(1)
-        end
-      end
-
-      context 'that has non public artifacts' do
-        let!(:build) { create(:ci_build, :success, :artifacts, :non_public_artifacts, pipeline: pipeline) }
-
-        it 'does not contain information about artifacts' do
-          expect(subject[:details][:artifacts].length).to eq(0)
-        end
-      end
-    end
+    it_behaves_like 'public artifacts'
   end
 end

spec/support/shared_examples/serializers/pipeline_artifacts_shared_example.rb

+# frozen_string_literal: true
+RSpec.shared_examples 'public artifacts' do
+  let_it_be(:project) { create(:project, :public) }
+  let(:pipeline) { create(:ci_empty_pipeline, status: :success, project: project) }
+
+  context 'that has artifacts' do
+    let!(:build) { create(:ci_build, :success, :artifacts, pipeline: pipeline) }
+
+    it 'contains information about artifacts' do
+      expect(subject[:details][:artifacts].length).to eq(1)
+    end
+  end
+
+  context 'that has non public artifacts' do
+    let!(:build) { create(:ci_build, :success, :artifacts, :non_public_artifacts, pipeline: pipeline) }
+
+    it 'does not contain information about artifacts' do
+      expect(subject[:details][:artifacts].length).to eq(0)
+    end
+  end
+end