Merge branch 'remove-artifact-paths-redux' into 'master'

Remove duplicate paths from an internal API serializer [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!55800

app/serializers/build_artifact_entity.rb

@@ -21,11 +21,17 @@ class BuildArtifactEntity < Grape::Entity
     )
   end
 
-  expose :keep_path, if: -> (*) { artifact.expiring? } do |artifact|
+  expose :keep_path, if: -> (*) { artifact.expiring? && show_duplicated_paths?(artifact.project) } do |artifact|
     fast_keep_project_job_artifacts_path(artifact.project, artifact.job)
   end
 
-  expose :browse_path do |artifact|
+  expose :browse_path, if: -> (*) { show_duplicated_paths?(artifact.project) } do |artifact|
     fast_browse_project_job_artifacts_path(artifact.project, artifact.job)
   end
+
+  private
+
+  def show_duplicated_paths?(project)
+    !Gitlab::Ci::Features.remove_duplicate_artifact_exposure_paths?(project)
+  end
 end

config/feature_flags/development/remove_duplicate_artifact_exposure_paths.yml

+---
+name: remove_duplicate_artifact_exposure_paths
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/54611
+rollout_issue_url:
+milestone: '13.10'
+type: development
+group: group::testing
+default_enabled: false

lib/gitlab/ci/features.rb

@@ -71,6 +71,10 @@ module Gitlab
       def self.ci_commit_pipeline_mini_graph_vue_enabled?(project)
         ::Feature.enabled?(:ci_commit_pipeline_mini_graph_vue, project, default_enabled: :yaml)
       end
+
+      def self.remove_duplicate_artifact_exposure_paths?(project)
+        ::Feature.enabled?(:remove_duplicate_artifact_exposure_paths, project, default_enabled: :yaml)
+      end
     end
   end
 end

spec/serializers/build_artifact_entity_spec.rb

@@ -21,15 +21,30 @@ RSpec.describe BuildArtifactEntity do
       expect(subject).to include(:expired, :expire_at)
     end
 
-    it 'contains paths to the artifacts' do
-      expect(subject[:path])
-        .to include "jobs/#{job.id}/artifacts/download?file_type=codequality"
+    it 'exposes the artifact download path' do
+      expect(subject[:path]).to include "jobs/#{job.id}/artifacts/download?file_type=codequality"
+    end
+
+    context 'with remove_duplicate_artifact_exposure_paths enabled' do
+      before do
+        stub_feature_flags(remove_duplicate_artifact_exposure_paths: true)
+      end
+
+      it 'has no keep or browse path' do
+        expect(subject).not_to include(:keep_path)
+        expect(subject).not_to include(:browse_path)
+      end
+    end
 
-      expect(subject[:keep_path])
-        .to include "jobs/#{job.id}/artifacts/keep"
+    context 'with remove_duplicate_artifact_exposure_paths disabled' do
+      before do
+        stub_feature_flags(remove_duplicate_artifact_exposure_paths: false)
+      end
 
-      expect(subject[:browse_path])
-        .to include "jobs/#{job.id}/artifacts/browse"
+      it 'has keep and browse paths' do
+        expect(subject[:keep_path]).to be_present
+        expect(subject[:browse_path]).to be_present
+      end
     end
   end
 end