Merge branch 'mc/backstage/keep-latest-artifact-followup' into 'master'

Move artifact unlock into Ci::Ref transition

Closes #228850

See merge request gitlab-org/gitlab!36819

app/models/ci/pipeline.rb

@@ -249,14 +249,6 @@ module Ci
 
         pipeline.run_after_commit { AutoDevops::DisableWorker.perform_async(pipeline.id) }
       end
-
-      after_transition any => [:success] do |pipeline|
-        next unless Gitlab::Ci::Features.keep_latest_artifacts_for_ref_enabled?(pipeline.project)
-
-        pipeline.run_after_commit do
-          Ci::PipelineSuccessUnlockArtifactsWorker.perform_async(pipeline.id)
-        end
-      end
     end
 
     scope :internal, -> { where(source: internal_sources) }

app/models/ci/ref.rb

@@ -3,6 +3,7 @@
 module Ci
   class Ref < ApplicationRecord
     extend Gitlab::Ci::Model
+    include AfterCommitQueue
     include Gitlab::OptimisticLocking
 
     FAILING_STATUSES = %w[failed broken still_failing].freeze
@@ -15,6 +16,7 @@ module Ci
         transition unknown: :success
         transition fixed: :success
         transition %i[failed broken still_failing] => :fixed
+        transition success: same
       end
 
       event :do_fail do
@@ -29,6 +31,14 @@ module Ci
       state :fixed, value: 3
       state :broken, value: 4
       state :still_failing, value: 5
+
+      after_transition any => [:fixed, :success] do |ci_ref|
+        next unless ::Gitlab::Ci::Features.keep_latest_artifacts_for_ref_enabled?(ci_ref.project)
+
+        ci_ref.run_after_commit do
+          Ci::PipelineSuccessUnlockArtifactsWorker.perform_async(ci_ref.last_finished_pipeline_id)
+        end
+      end
     end
 
     class << self

app/services/git/tag_push_service.rb

@@ -26,9 +26,5 @@ module Git
     def removing_tag?
       Gitlab::Git.blank_ref?(newrev)
     end
-
-    def tag_name
-      Gitlab::Git.ref_name(ref)
-    end
   end
 end

spec/models/ci/pipeline_spec.rb

@@ -3266,32 +3266,6 @@ RSpec.describe Ci::Pipeline, :mailer do
         end
       end
     end
-
-    context 'when transitioning to success' do
-      context 'when feature is enabled' do
-        before do
-          stub_feature_flags(keep_latest_artifacts_for_ref: true)
-        end
-
-        it 'calls the PipelineSuccessUnlockArtifactsWorker' do
-          expect(Ci::PipelineSuccessUnlockArtifactsWorker).to receive(:perform_async).with(pipeline.id)
-
-          pipeline.succeed!
-        end
-      end
-
-      context 'when feature is disabled' do
-        before do
-          stub_feature_flags(keep_latest_artifacts_for_ref: false)
-        end
-
-        it 'does not call the PipelineSuccessUnlockArtifactsWorker' do
-          expect(Ci::PipelineSuccessUnlockArtifactsWorker).not_to receive(:perform_async)
-
-          pipeline.succeed!
-        end
-      end
-    end
   end
 
   describe '#default_branch?' do

spec/models/ci/ref_spec.rb

@@ -3,8 +3,69 @@
 require 'spec_helper'
 
 RSpec.describe Ci::Ref do
+  using RSpec::Parameterized::TableSyntax
+
   it { is_expected.to belong_to(:project) }
 
+  describe 'state machine transitions' do
+    context 'unlock artifacts transition' do
+      let(:ci_ref) { create(:ci_ref) }
+      let(:unlock_artifacts_worker_spy) { class_spy(::Ci::PipelineSuccessUnlockArtifactsWorker) }
+
+      before do
+        stub_const('Ci::PipelineSuccessUnlockArtifactsWorker', unlock_artifacts_worker_spy)
+      end
+
+      context 'when keep latest artifact feature is enabled' do
+        before do
+          stub_feature_flags(keep_latest_artifacts_for_ref: true)
+        end
+
+        where(:initial_state, :action, :count) do
+          :unknown | :succeed! | 1
+          :unknown | :do_fail! | 0
+          :success | :succeed! | 1
+          :success | :do_fail! | 0
+          :failed | :succeed! | 1
+          :failed | :do_fail! | 0
+          :fixed | :succeed! | 1
+          :fixed | :do_fail! | 0
+          :broken | :succeed! | 1
+          :broken | :do_fail! | 0
+          :still_failing | :succeed | 1
+          :still_failing | :do_fail | 0
+        end
+
+        with_them do
+          context "when transitioning states" do
+            before do
+              status_value = Ci::Ref.state_machines[:status].states[initial_state].value
+              ci_ref.update!(status: status_value)
+            end
+
+            it 'calls unlock artifacts service' do
+              ci_ref.send(action)
+
+              expect(unlock_artifacts_worker_spy).to have_received(:perform_async).exactly(count).times
+            end
+          end
+        end
+      end
+
+      context 'when keep latest artifact feature is not enabled' do
+        before do
+          stub_feature_flags(keep_latest_artifacts_for_ref: false)
+        end
+
+        it 'does not call unlock artifacts service' do
+          ci_ref.succeed!
+
+          expect(unlock_artifacts_worker_spy).not_to have_received(:perform_async)
+        end
+      end
+    end
+  end
+
   describe '.ensure_for' do
     let_it_be(:project) { create(:project, :repository) }