Automatically revert unsuccessful conversion to semantic mode

app/workers/project_identifiers/convert_instance_to_semantic_ids_job.rb

@@ -35,7 +35,7 @@ class ProjectIdentifiers::ConvertInstanceToSemanticIdsJob < ApplicationJob
   queue_with_priority :above_normal
 
   def perform
-    GoodJob::Batch.enqueue(on_success: ProjectIdentifiers::FinishSemanticConversionJob) do
+    GoodJob::Batch.enqueue(on_finish: ProjectIdentifiers::FinishSemanticConversionJob) do
       ProjectIdentifiers::PendingProjectsFinder.project_ids.each do |project_id|
         ProjectIdentifiers::ConvertProjectToSemanticIdsJob.perform_later(project_id)
       end

app/workers/project_identifiers/finish_semantic_conversion_job.rb

@@ -28,30 +28,41 @@
 # See COPYRIGHT and LICENSE files for more details.
 #++
 
-# GoodJob on_success callback invoked after a ConvertInstanceToSemanticIdsJob
-# batch completes. Performs up to MAX_SWEEPS synchronous sweeps to catch any
-# projects created or modified during the batch run, then enables semantic mode.
-# If projects still remain after MAX_SWEEPS sweeps a RuntimeError is raised to
-# abort the job (leaving the setting as classic); the instance is either under
+# GoodJob on_finish callback invoked after a ConvertInstanceToSemanticIdsJob batch
+# completes (regardless of per-project job failures or discards). Performs up to
+# MAX_SWEEPS synchronous sweeps to catch any projects created or modified during
+# the batch run, then enables semantic mode. If projects still remain after
+# MAX_SWEEPS sweeps the instance reverts to classic mode; it is either under
 # active load or there is a code bug.
 class ProjectIdentifiers::FinishSemanticConversionJob < ApplicationJob
   queue_with_priority :high
 
   MAX_SWEEPS = 5
 
-  def perform(*)
+  class ConversionFailed < StandardError; end
+
+  def perform(batch, *)
+    raise ConversionFailed, "Batch had failures" if batch.discarded?
+
     corrective_sweep
     set_semantic_mode!
+  rescue ConversionFailed => e
+    revert_to_classic!(reason: e.message)
   end
 
   private
 
+  def revert_to_classic!(reason:)
+    Rails.logger.error "[#{self.class.name}] #{reason} - reverting to classic mode."
+    ProjectIdentifiers::RevertInstanceToClassicIdsJob.perform_later
+  end
+
   def set_semantic_mode!
     result = Settings::UpdateService
                .new(user: User.system)
                .call(work_packages_identifier: Setting::WorkPackageIdentifier::SEMANTIC)
 
-    raise "[FinishSemanticConversionJob] Failed to enable semantic mode: #{result.message}" unless result.success?
+    raise ConversionFailed, "Failed to enable semantic mode: #{result.message}" unless result.success?
   end
 
   def corrective_sweep
@@ -69,10 +80,8 @@ def corrective_sweep
 
     return if pending_project_ids.empty?
 
-    message = "[FinishSemanticConversionJob] Giving up after #{MAX_SWEEPS} sweeps — " \
-              "projects still remain pending. The instance may be under active load or there is a bug."
-    Rails.logger.warn message
-    raise message
+    raise ConversionFailed, "Giving up after #{MAX_SWEEPS} sweeps — " \
+                            "projects still remain pending. The instance may be under active load or there is a bug."
   end
 
   def pending_project_ids

spec/workers/project_identifiers/convert_instance_to_semantic_ids_job_spec.rb

@@ -46,11 +46,11 @@
         expect(GoodJob::Job.where(job_class: ProjectIdentifiers::ConvertProjectToSemanticIdsJob.name).count).to eq(2)
       end
 
-      it "sets FinishSemanticConversionJob as the on_success callback" do
+      it "sets FinishSemanticConversionJob as the on_finish callback" do
         allow(GoodJob::Batch).to receive(:enqueue).and_call_original
         job.perform
         expect(GoodJob::Batch).to have_received(:enqueue)
-          .with(hash_including(on_success: ProjectIdentifiers::FinishSemanticConversionJob))
+          .with(hash_including(on_finish: ProjectIdentifiers::FinishSemanticConversionJob))
       end
     end
 

spec/workers/project_identifiers/finish_semantic_conversion_job_spec.rb

@@ -33,10 +33,12 @@
 RSpec.describe ProjectIdentifiers::FinishSemanticConversionJob do
   subject(:job) { described_class.new }
 
+  let(:batch) { instance_double(GoodJob::Batch, discarded?: false) }
   let(:update_service) { instance_double(Settings::UpdateService, call: ServiceResult.success) }
 
   before do
     allow(Settings::UpdateService).to receive(:new).with(user: User.system).and_return(update_service)
+    allow(ProjectIdentifiers::RevertInstanceToClassicIdsJob).to receive(:perform_later)
   end
 
   describe "#perform" do
@@ -45,7 +47,7 @@
 
       it "enables semantic mode without running any conversion" do
         allow(ProjectIdentifiers::ConvertProjectToSemanticService).to receive(:new)
-        job.perform
+        job.perform(batch)
         expect(ProjectIdentifiers::ConvertProjectToSemanticService).not_to have_received(:new)
         expect(update_service).to have_received(:call)
           .with(work_packages_identifier: Setting::WorkPackageIdentifier::SEMANTIC)
@@ -63,7 +65,7 @@
       end
 
       it "runs one conversion sweep then enables semantic mode" do
-        job.perform
+        job.perform(batch)
         expect(service).to have_received(:call).once
         expect(update_service).to have_received(:call)
           .with(work_packages_identifier: Setting::WorkPackageIdentifier::SEMANTIC)
@@ -82,7 +84,7 @@
       end
 
       it "enables semantic mode after the final sweep clears all projects" do
-        job.perform
+        job.perform(batch)
         expect(service).to have_received(:call).exactly(described_class::MAX_SWEEPS).times
         expect(update_service).to have_received(:call)
           .with(work_packages_identifier: Setting::WorkPackageIdentifier::SEMANTIC)
@@ -97,16 +99,24 @@
         allow(ProjectIdentifiers::PendingProjectsFinder).to receive(:project_ids).and_return(Set[1])
         allow(Project).to receive(:find_by).with(id: 1).and_return(project)
         allow(ProjectIdentifiers::ConvertProjectToSemanticService).to receive(:new).with(project).and_return(service)
+        allow(Rails.logger).to receive(:error)
       end
 
-      it "raises after MAX_SWEEPS sweeps, logging a warning and not enabling semantic mode" do
-        allow(Rails.logger).to receive(:warn)
-        give_up_pattern = /Giving up after #{described_class::MAX_SWEEPS} sweeps/o
+      it "does not enable semantic mode and reverts to classic" do
+        job.perform(batch)
+        expect(update_service).not_to have_received(:call)
+        expect(ProjectIdentifiers::RevertInstanceToClassicIdsJob).to have_received(:perform_later)
+      end
+
+      it "logs the failure reason" do
+        job.perform(batch)
+        expect(Rails.logger).to have_received(:error)
+          .with(/Giving up after #{described_class::MAX_SWEEPS} sweeps/o)
+      end
 
-        expect { job.perform }.to raise_error(RuntimeError, give_up_pattern)
+      it "runs the maximum number of sweeps before giving up" do
+        job.perform(batch)
         expect(service).to have_received(:call).exactly(described_class::MAX_SWEEPS).times
-        expect(Rails.logger).to have_received(:warn).with(give_up_pattern)
-        expect(update_service).not_to have_received(:call)
       end
     end
 
@@ -118,11 +128,57 @@
       end
 
       it "skips the missing project and still enables semantic mode" do
-        job.perform
+        job.perform(batch)
         expect(ProjectIdentifiers::ConvertProjectToSemanticService).not_to have_received(:new)
         expect(update_service).to have_received(:call)
           .with(work_packages_identifier: Setting::WorkPackageIdentifier::SEMANTIC)
       end
     end
+
+    context "when enabling semantic mode fails" do
+      before do
+        allow(ProjectIdentifiers::PendingProjectsFinder).to receive(:project_ids).and_return(Set.new)
+        allow(update_service).to receive(:call)
+          .and_return(instance_double(ServiceResult, success?: false, message: "update failed"))
+        allow(Rails.logger).to receive(:error)
+      end
+
+      it "reverts to classic" do
+        job.perform(batch)
+        expect(ProjectIdentifiers::RevertInstanceToClassicIdsJob).to have_received(:perform_later)
+      end
+
+      it "logs the failure reason" do
+        job.perform(batch)
+        expect(Rails.logger).to have_received(:error).with(/Failed to enable semantic mode/)
+      end
+    end
+
+    context "when the batch had failures" do
+      let(:batch) { instance_double(GoodJob::Batch, discarded?: true) }
+
+      before { allow(Rails.logger).to receive(:error) }
+
+      it "does not run a corrective sweep" do
+        allow(ProjectIdentifiers::PendingProjectsFinder).to receive(:project_ids)
+        job.perform(batch)
+        expect(ProjectIdentifiers::PendingProjectsFinder).not_to have_received(:project_ids)
+      end
+
+      it "does not enable semantic mode" do
+        job.perform(batch)
+        expect(update_service).not_to have_received(:call)
+      end
+
+      it "enqueues RevertInstanceToClassicIdsJob" do
+        job.perform(batch)
+        expect(ProjectIdentifiers::RevertInstanceToClassicIdsJob).to have_received(:perform_later)
+      end
+
+      it "logs the failure reason" do
+        job.perform(batch)
+        expect(Rails.logger).to have_received(:error).with(/Batch had failures/)
+      end
+    end
   end
 end