Merge branch 'mc/feature/fix-keep-path-artifact-locking' into 'master'

Show keep path even for expired locked artifacts

See merge request gitlab-org/gitlab!43866

app/models/ci/build.rb

@@ -782,6 +782,11 @@ module Ci
         end
     end
 
+    def has_expired_locked_archive_artifacts?
+      locked_artifacts? &&
+        artifacts_expire_at.present? && artifacts_expire_at < Time.current
+    end
+
     def has_expiring_archive_artifacts?
       has_expiring_artifacts? && job_artifacts_archive.present?
     end

app/serializers/build_details_entity.rb

@@ -35,7 +35,7 @@ class BuildDetailsEntity < JobEntity
       browse_project_job_artifacts_path(project, build)
     end
 
-    expose :keep_path, if: -> (*) { (build.locked_artifacts? || build.has_expiring_archive_artifacts?) && can?(current_user, :update_build, build) } do |build|
+    expose :keep_path, if: -> (*) { (build.has_expired_locked_archive_artifacts? || build.has_expiring_archive_artifacts?) && can?(current_user, :update_build, build) } do |build|
       keep_project_job_artifacts_path(project, build)
     end
 

changelogs/unreleased/mc-feature-fix-keep-path-artifact-locking.yml

+---
+title: Show keep path for expired locked artifacts.
+merge_request: 43866
+author:
+type: changed

spec/controllers/projects/jobs_controller_spec.rb

@@ -197,16 +197,40 @@ RSpec.describe Projects::JobsController, :clean_gitlab_redis_shared_state do
         context 'with not expiry date' do
           let(:job) { create(:ci_build, :success, :artifacts, pipeline: pipeline) }
 
-          it 'exposes needed information' do
-            get_show_json
+          context 'when artifacts are unlocked' do
+            before do
+              job.pipeline.unlocked!
+            end
 
-            expect(response).to have_gitlab_http_status(:ok)
-            expect(response).to match_response_schema('job/job_details')
-            expect(json_response['artifact']['download_path']).to match(%r{artifacts/download})
-            expect(json_response['artifact']['browse_path']).to match(%r{artifacts/browse})
-            expect(json_response['artifact']).not_to have_key('keep_path')
-            expect(json_response['artifact']).not_to have_key('expired')
-            expect(json_response['artifact']).not_to have_key('expired_at')
+            it 'exposes needed information' do
+              get_show_json
+
+              expect(response).to have_gitlab_http_status(:ok)
+              expect(response).to match_response_schema('job/job_details')
+              expect(json_response['artifact']['download_path']).to match(%r{artifacts/download})
+              expect(json_response['artifact']['browse_path']).to match(%r{artifacts/browse})
+              expect(json_response['artifact']).not_to have_key('keep_path')
+              expect(json_response['artifact']).not_to have_key('expired')
+              expect(json_response['artifact']).not_to have_key('expired_at')
+            end
+          end
+
+          context 'when artifacts are locked' do
+            before do
+              job.pipeline.artifacts_locked!
+            end
+
+            it 'exposes needed information' do
+              get_show_json
+
+              expect(response).to have_gitlab_http_status(:ok)
+              expect(response).to match_response_schema('job/job_details')
+              expect(json_response['artifact']['download_path']).to match(%r{artifacts/download})
+              expect(json_response['artifact']['browse_path']).to match(%r{artifacts/browse})
+              expect(json_response['artifact']).not_to have_key('keep_path')
+              expect(json_response['artifact']).not_to have_key('expired')
+              expect(json_response['artifact']).not_to have_key('expired_at')
+            end
           end
         end
 

spec/models/ci/build_spec.rb

@@ -2307,6 +2307,54 @@ RSpec.describe Ci::Build do
     end
   end
 
+  describe '#has_expired_locked_archive_artifacts?' do
+    subject { build.has_expired_locked_archive_artifacts? }
+
+    context 'when build does not have artifacts' do
+      it { is_expected.to eq(nil) }
+    end
+
+    context 'when build has artifacts' do
+      before do
+        create(:ci_job_artifact, :archive, job: build)
+      end
+
+      context 'when artifacts are unlocked' do
+        before do
+          build.pipeline.unlocked!
+        end
+
+        it { is_expected.to eq(false) }
+      end
+
+      context 'when artifacts are locked' do
+        before do
+          build.pipeline.artifacts_locked!
+        end
+
+        context 'when artifacts do not expire' do
+          it { is_expected.to eq(false) }
+        end
+
+        context 'when artifacts expire in the future' do
+          before do
+            build.update!(artifacts_expire_at: 1.day.from_now)
+          end
+
+          it { is_expected.to eq(false) }
+        end
+
+        context 'when artifacts expired in the past' do
+          before do
+            build.update!(artifacts_expire_at: 1.day.ago)
+          end
+
+          it { is_expected.to eq(true) }
+        end
+      end
+    end
+  end
+
   describe '#has_expiring_archive_artifacts?' do
     context 'when artifacts have expiration date set' do
       before do