Merge branch...

Merge branch '345883-destroying-a-ci-jobartifact-causes-a-cross-database-modification-across-unsupported-tables-error' into 'master'

Fixes cross-db modification when destroying a Ci::JobArtifact

See merge request gitlab-org/gitlab!74737

ee/app/models/ee/ci/job_artifact.rb

@@ -15,7 +15,7 @@ module EE
       # After destroy callbacks are often skipped because of FastDestroyAll.
       # All destroy callbacks should be implemented in `Ci::JobArtifacts::DestroyBatchService`
       # See https://gitlab.com/gitlab-org/gitlab/-/issues/297472
-      after_destroy :log_geo_deleted_event
+      after_commit :log_geo_deleted_event, on: :destroy
 
       LICENSE_SCANNING_REPORT_FILE_TYPES = %w[license_scanning].freeze
       DEPENDENCY_LIST_REPORT_FILE_TYPES = %w[dependency_scanning].freeze

ee/spec/models/ee/ci/job_artifact_spec.rb

@@ -12,14 +12,27 @@ RSpec.describe Ci::JobArtifact do
     let_it_be(:primary) { create(:geo_node, :primary) }
     let_it_be(:secondary) { create(:geo_node) }
 
-    it 'creates a JobArtifactDeletedEvent' do
+    before do
       stub_current_geo_node(primary)
+    end
+
+    it 'creates a JobArtifactDeletedEvent' do
+      job_artifact = create(:ee_ci_job_artifact, :archive)
+
+      expect { job_artifact.destroy! }.to change { Geo::JobArtifactDeletedEvent.count }.by(1)
+    end
 
+    context 'JobArtifact destroy fails' do
+      it 'does not create a JobArtifactDeletedEvent' do
         job_artifact = create(:ee_ci_job_artifact, :archive)
 
-      expect do
-        job_artifact.destroy!
-      end.to change { Geo::JobArtifactDeletedEvent.count }.by(1)
+        allow(job_artifact).to receive(:destroy!)
+                           .and_raise(ActiveRecord::RecordNotDestroyed)
+
+        expect { job_artifact.destroy! }.to raise_error(ActiveRecord::RecordNotDestroyed)
+                                        .and not_change { Ci::JobArtifact.count }
+                                        .and not_change { Geo::JobArtifactDeletedEvent.count }
+      end
     end
   end