Merge branch 'mo-dont-expose-artifacts-on-pipeline-index' into 'master'

Do not expose artifacts on pipeline index

See merge request gitlab-org/gitlab!60126

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

@@ -41,7 +41,7 @@ export default {
         this.pipeline.flags.retryable ||
         this.pipeline.flags.cancelable ||
         this.pipeline.details.manual_actions.length ||
-        this.pipeline.details.artifacts.length
+        this.pipeline.details.has_downloadable_artifacts
       );
     },
     actions() {
@@ -55,9 +55,7 @@ export default {
       return this.cancelingPipeline === this.pipeline.id;
     },
     showArtifacts() {
-      return (
-        this.pipeline.details.artifacts?.length || this.pipeline.details.has_downloadable_artifacts
-      );
+      return this.pipeline.details.has_downloadable_artifacts;
     },
   },
   watch: {

app/controllers/projects/pipelines_controller.rb

@@ -218,7 +218,7 @@ class Projects::PipelinesController < Projects::ApplicationController
     PipelineSerializer
       .new(project: @project, current_user: @current_user)
       .with_pagination(request, response)
-      .represent(@pipelines, disable_coverage: true, preload: true)
+      .represent(@pipelines, disable_coverage: true, preload: true, disable_artifacts: true)
   end
 
   def render_show

app/models/ci/pipeline.rb

@@ -1076,6 +1076,14 @@ module Ci
       complete? && builds.latest.with_exposed_artifacts.exists?
     end
 
+    def has_downloadable_artifacts?
+      if downloadable_artifacts.loaded?
+        downloadable_artifacts.any?
+      else
+        downloadable_artifacts.exists?
+      end
+    end
+
     def branch_updated?
       strong_memoize(:branch_updated) do
         push_details.branch_updated?

app/serializers/pipeline_details_entity.rb

@@ -8,7 +8,8 @@ class PipelineDetailsEntity < Ci::PipelineEntity
   end
 
   expose :details do
-    expose :artifacts do |pipeline, options|
+    expose :has_downloadable_artifacts?, as: :has_downloadable_artifacts
+    expose :artifacts, unless: proc { options[:disable_artifacts] } do |pipeline, options|
       rel = pipeline.downloadable_artifacts
 
       if Feature.enabled?(:non_public_artifacts, type: :development)

changelogs/unreleased/mo-dont-expose-artifacts-on-pipeline-index.yml

+---
+title: Stop exposing artifacts on pipelines.json
+merge_request: 60126
+author:
+type: performance

spec/features/projects/pipelines/pipelines_spec.rb

@@ -460,20 +460,6 @@ RSpec.describe 'Pipelines', :js do
           it 'has artifacts dropdown' do
             expect(page).to have_selector('[data-testid="pipeline-multi-actions-dropdown"]')
           end
-
-          it 'has artifacts download dropdown' do
-            find('[data-testid="pipeline-multi-actions-dropdown"]').click
-
-            expect(page).to have_link(with_artifacts.file_type)
-          end
-
-          it 'has download attribute on download links' do
-            find('[data-testid="pipeline-multi-actions-dropdown"]').click
-            expect(page).to have_selector('a', text: 'Download')
-            page.all('[data-testid="artifact-item"]', text: 'Download').each do |link|
-              expect(link[:download]).to eq ''
-            end
-          end
         end
 
         context 'with artifacts expired' do

spec/models/ci/pipeline_spec.rb

@@ -4437,4 +4437,18 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do
         .not_to exceed_query_limit(control_count)
     end
   end
+
+  describe '#has_downloadable_artifacts?' do
+    it 'returns false when when pipeline does not have downloadable artifacts' do
+      pipeline = create(:ci_pipeline, :success)
+
+      expect(pipeline.has_downloadable_artifacts?). to eq(false)
+    end
+
+    it 'returns false when when pipeline does not have downloadable artifacts' do
+      pipeline = create(:ci_pipeline, :with_codequality_reports)
+
+      expect(pipeline.has_downloadable_artifacts?). to eq(true)
+    end
+  end
 end

spec/serializers/pipeline_details_entity_spec.rb

@@ -32,7 +32,7 @@ RSpec.describe PipelineDetailsEntity do
         expect(subject[:details])
           .to include :duration, :finished_at
         expect(subject[:details])
-          .to include :stages, :artifacts, :manual_actions, :scheduled_actions
+          .to include :stages, :artifacts, :has_downloadable_artifacts, :manual_actions, :scheduled_actions
         expect(subject[:details][:status]).to include :icon, :favicon, :text, :label
       end
 
@@ -186,5 +186,35 @@ RSpec.describe PipelineDetailsEntity do
     end
 
     it_behaves_like 'public artifacts'
+
+    context 'when pipeline has downloadable artifacts' do
+      subject(:entity) { described_class.represent(pipeline, request: request, disable_artifacts: disable_artifacts).as_json }
+
+      let_it_be(:pipeline) { create(:ci_pipeline, :with_codequality_reports) }
+
+      context 'when disable_artifacts is true' do
+        subject(:entity) { described_class.represent(pipeline, request: request, disable_artifacts: true).as_json }
+
+        it 'excludes artifacts data' do
+          expect(entity[:details]).not_to include(:artifacts)
+        end
+
+        it 'returns true for has_downloadable_artifacts' do
+          expect(entity[:details][:has_downloadable_artifacts]).to eq(true)
+        end
+      end
+
+      context 'when disable_artifacts is false' do
+        subject(:entity) { described_class.represent(pipeline, request: request, disable_artifacts: false).as_json }
+
+        it 'includes artifacts data' do
+          expect(entity[:details]).to include(:artifacts)
+        end
+
+        it 'returns true for has_downloadable_artifacts' do
+          expect(entity[:details][:has_downloadable_artifacts]).to eq(true)
+        end
+      end
+    end
   end
 end