refactor(declarative): process dossier in job isolation

app/jobs/cron/stalled_declarative_procedures_job.rb

@@ -1,13 +1,10 @@
 class Cron::StalledDeclarativeProceduresJob < Cron::CronJob
-  self.schedule_expression = "every 10 minute"
+  self.schedule_expression = "every 10 minutes"
 
-  def perform(*args)
+  def perform
     Procedure.declarative.find_each do |procedure|
-      begin
+      procedure.dossiers.state_en_construction.where(declarative_triggered_at: nil).find_each do |dossier|
-        procedure.process_stalled_dossiers!
+        ProcessStalledDeclarativeDossierJob.perform_later(dossier)
-      rescue => e
-        Sentry.set_tags(procedure: procedure.id)
-        Sentry.capture_exception(e)
       end
     end
   end

app/jobs/process_stalled_declarative_dossier_job.rb

@@ -0,0 +1,20 @@
+class ProcessStalledDeclarativeDossierJob < ApplicationJob
+  def perform(dossier)
+    return if dossier.declarative_triggered_at.present?
+
+    case dossier.procedure.declarative_with_state
+    when Procedure.declarative_with_states.fetch(:en_instruction)
+      if !dossier.en_instruction?
+        dossier.passer_automatiquement_en_instruction!
+      end
+    when Procedure.declarative_with_states.fetch(:accepte)
+      if dossier.may_accepter_automatiquement?
+        dossier.accepter_automatiquement!
+      end
+    end
+  end
+
+  def max_attempts
+    3 # this job is enqueued by a cron, so it's better to not retry too much
+  end
+end

app/models/procedure.rb

@@ -490,23 +490,6 @@ class Procedure < ApplicationRecord
     end
   end
 
-  def process_stalled_dossiers!
-    case declarative_with_state
-    when Procedure.declarative_with_states.fetch(:en_instruction)
-      dossiers
-        .state_en_construction
-        .where(declarative_triggered_at: nil)
-        .find_each(&:passer_automatiquement_en_instruction!)
-    when Procedure.declarative_with_states.fetch(:accepte)
-      dossiers
-        .state_en_construction
-        .where(declarative_triggered_at: nil)
-        .find_each do |dossier|
-          dossier.accepter_automatiquement! if dossier.can_accepter_automatiquement?
-        end
-    end
-  end
-
   def feature_enabled?(feature)
     Flipper.enabled?(feature, self)
   end

spec/jobs/cron/stalled_declarative_procedures_job_spec.rb

@@ -1,126 +1,47 @@
 RSpec.describe Cron::StalledDeclarativeProceduresJob, type: :job do
   describe "perform" do
-    let(:date) { Time.utc(2017, 9, 1, 10, 5, 0) }
-    let(:instruction_date) { date + 120 }
-
     let(:state) { nil }
     let(:procedure) { create(:procedure, :published, :for_individual, :with_instructeur, declarative_with_state: state) }
-    let(:nouveau_dossier1) { create(:dossier, :en_construction, :with_individual, :with_attestation, procedure: procedure) }
+    let!(:brouillon) { create(:dossier, :brouillon, procedure:) }
-    let(:nouveau_dossier2) { create(:dossier, :en_construction, :with_individual, :with_attestation, procedure: procedure) }
+    let!(:en_construction) { create(:dossier, :en_construction, :with_individual, :with_attestation, procedure:) }
-    let(:dossier_recu) { create(:dossier, :en_instruction, :with_individual, procedure: procedure) }
+    let!(:en_construction_triggered) { create(:dossier, :en_construction, :with_individual, :with_attestation, procedure:, declarative_triggered_at: 1.minute.ago) }
-    let(:dossier_brouillon) { create(:dossier, procedure: procedure) }
+    let!(:en_instruction) { create(:dossier, :en_instruction, :with_individual, procedure:) }
-    let(:dossier_repasse_en_construction) { create(:dossier, :en_construction, :with_individual, procedure: procedure) }
-
-    before do
-        Timecop.freeze(date)
-        dossier_repasse_en_construction&.touch(:declarative_triggered_at)
-      end
 
     subject(:perform_job) do
-      dossiers = [
+      described_class.perform_now
-        nouveau_dossier1,
-        nouveau_dossier2,
-        dossier_recu,
-        dossier_brouillon,
-        dossier_repasse_en_construction
-      ].compact
-
-      Cron::StalledDeclarativeProceduresJob.new.perform
-
-      dossiers.each(&:reload)
     end
 
-    after { Timecop.return }
+    context "declarative en instruction" do
-
-    context "with some dossiers" do
-      context "en_construction" do
       let(:state) { Dossier.states.fetch(:en_instruction) }
-        let(:last_operation) { nouveau_dossier1.dossier_operation_logs.last }
 
       it {
         perform_job
-          expect(nouveau_dossier1.en_instruction?).to be_truthy
+        expect(ProcessStalledDeclarativeDossierJob).not_to have_been_enqueued.with(brouillon)
-          expect(nouveau_dossier1.en_instruction_at).to eq(date)
+        expect(ProcessStalledDeclarativeDossierJob).to have_been_enqueued.with(en_construction)
-          expect(last_operation.operation).to eq('passer_en_instruction')
+        expect(ProcessStalledDeclarativeDossierJob).not_to have_been_enqueued.with(en_construction_triggered)
-
+        expect(ProcessStalledDeclarativeDossierJob).not_to have_been_enqueued.with(en_instruction)
-          expect(last_operation.automatic_operation?).to be_truthy
-
-          expect(nouveau_dossier2.en_instruction?).to be_truthy
-          expect(nouveau_dossier2.en_instruction_at).to eq(date)
-
-          expect(dossier_recu.en_instruction?).to be_truthy
-          expect(dossier_recu.en_instruction_at).to eq(instruction_date)
-
-          expect(dossier_brouillon.brouillon?).to be_truthy
-          expect(dossier_brouillon.en_instruction_at).to eq(nil)
-
-          expect(dossier_repasse_en_construction.en_construction?).to be_truthy
       }
     end
 
-      context "accepte" do
+    context "declarative accepte" do
       let(:state) { Dossier.states.fetch(:accepte) }
-        let(:last_operation) { nouveau_dossier1.dossier_operation_logs.last }
 
       it {
         perform_job
-          expect(nouveau_dossier1.accepte?).to be true
+        expect(ProcessStalledDeclarativeDossierJob).not_to have_been_enqueued.with(brouillon)
-          expect(nouveau_dossier1.en_instruction_at).to eq(date)
+        expect(ProcessStalledDeclarativeDossierJob).to have_been_enqueued.with(en_construction)
-          expect(nouveau_dossier1.processed_at).to eq(date)
+        expect(ProcessStalledDeclarativeDossierJob).not_to have_been_enqueued.with(en_construction_triggered)
-          expect(nouveau_dossier1.attestation).to be_present
+        expect(ProcessStalledDeclarativeDossierJob).not_to have_been_enqueued.with(en_instruction)
-          expect(last_operation.operation).to eq('accepter')
-          expect(last_operation.automatic_operation?).to be_truthy
-
-          expect(nouveau_dossier2.accepte?).to be true
-          expect(nouveau_dossier2.en_instruction_at).to eq(date)
-          expect(nouveau_dossier2.processed_at).to eq(date)
-          expect(nouveau_dossier2.attestation).to be_present
-
-          expect(dossier_recu.en_instruction?).to be true
-          expect(dossier_recu.en_instruction_at).to eq(instruction_date)
-          expect(dossier_recu.processed_at).to eq(nil)
-
-          expect(dossier_brouillon.brouillon?).to be true
-          expect(dossier_brouillon.en_instruction_at).to eq(nil)
-          expect(dossier_brouillon.processed_at).to eq(nil)
       }
-
-        context "having etablissement in degraded_mode" do
-          let(:procedure) { create(:procedure, :published, :with_instructeur, for_individual: false, declarative_with_state: state) }
-
-          let(:nouveau_dossier1) { create(:dossier, :en_construction, :with_entreprise, :with_attestation, procedure: procedure, as_degraded_mode: false) }
-          let(:nouveau_dossier2) { create(:dossier, :en_construction, :with_entreprise, :with_attestation, procedure: procedure, as_degraded_mode: true) }
-          let(:dossier_recu) { nil }
-          let(:dossier_repasse_en_construction) { nil }
-
-          before do
-            expect(nouveau_dossier2).to_not receive(:accepter_automatiquement!)
-            expect(Sentry).to_not receive(:capture_exception)
     end
 
+    context "not declarative" do
+      let(:state) { nil }
+
       it {
         perform_job
-            expect(nouveau_dossier1).to be_accepte
+        expect(ProcessStalledDeclarativeDossierJob).not_to have_been_enqueued
-            expect(nouveau_dossier2).to be_en_construction
       }
     end
   end
-    end
-  end
-
-  describe 'safer perform' do
-    let(:state) { Dossier.states.fetch(:en_instruction) }
-
-    it 'works no matter if one raise' do
-      procedure_1 = instance_double("Procedure", id: 1)
-      expect(procedure_1).to receive(:process_stalled_dossiers!)
-      procedure_2 = instance_double("Procedure", id: 2)
-      expect(procedure_2).to receive(:process_stalled_dossiers!).and_raise("boom")
-      procedure_3 = double(process_stalled_dossiers!: true)
-      expect(procedure_3).to receive(:process_stalled_dossiers!)
-
-      expect(Procedure).to receive_message_chain(:declarative, :find_each).and_yield(procedure_1).and_yield(procedure_2).and_yield(procedure_3)
-      Cron::StalledDeclarativeProceduresJob.perform_now
-    end
-  end
 end

spec/jobs/process_stalled_declarative_dossier_job_spec.rb

@@ -0,0 +1,105 @@
+RSpec.describe ProcessStalledDeclarativeDossierJob, type: :job do
+  include ActiveSupport::Testing::TimeHelpers
+
+  describe "perform" do
+    let(:procedure) { create(:procedure, :published, :for_individual, :with_instructeur, declarative_with_state: state) }
+    let(:last_operation) { dossier.dossier_operation_logs.last }
+
+    subject(:perform_job) do
+      described_class.perform_now(dossier)
+    end
+
+    before do
+      freeze_time
+      perform_job
+      dossier.reload
+    end
+
+    context 'declarative en instruction' do
+      let(:state) { Dossier.states.fetch(:en_instruction) }
+
+      context 'dossier en_construction' do
+        let(:dossier) { create(:dossier, :en_construction, :with_individual, :with_attestation, procedure:) }
+
+        it {
+          expect(dossier.state).to eq('en_instruction')
+          expect(dossier.en_instruction_at).to eq(Time.current)
+          expect(last_operation.operation).to eq('passer_en_instruction')
+          expect(last_operation.automatic_operation?).to be_truthy
+        }
+
+        context 'dossier repasse en_construction' do
+          let(:dossier) { create(:dossier, :en_construction, :with_individual, procedure:, declarative_triggered_at: 1.day.ago) }
+
+          it { expect(dossier.state).to eq('en_construction') }
+        end
+      end
+
+      context 'dossier already en_instruction' do
+        let(:dossier) { create(:dossier, :en_instruction, :with_individual, procedure:, en_instruction_at: 2.days.ago) }
+
+        it {
+          expect(dossier.state).to eq('en_instruction')
+          expect(dossier.en_instruction_at).to eq(2.days.ago)
+          expect(dossier.processed_at).to be_nil
+        }
+      end
+    end
+
+    context "declarative accepte" do
+      let(:state) { Dossier.states.fetch(:accepte) }
+
+      context 'dossier en_construction' do
+        let(:dossier) { create(:dossier, :en_construction, :with_individual, :with_attestation, procedure:) }
+
+        it {
+          expect(dossier.state).to eq('accepte')
+          expect(dossier.en_instruction_at).to eq(Time.current)
+          expect(dossier.processed_at).to eq(Time.current)
+          expect(dossier.attestation).to be_present
+          expect(last_operation.operation).to eq('accepter')
+          expect(last_operation.automatic_operation?).to be_truthy
+        }
+      end
+
+      context 'dossier en_instruction' do
+        let(:dossier) { create(:dossier, :en_instruction, :with_individual, procedure:, en_instruction_at: 2.days.ago) }
+
+        it {
+          expect(dossier.state).to eq('en_instruction')
+          expect(dossier.en_instruction_at).to eq(2.days.ago)
+          expect(dossier.processed_at).to be_nil
+        }
+      end
+
+      context 'dossier brouillon' do
+        let(:dossier) { create(:dossier, :brouillon) }
+
+        it {
+          expect(dossier.state).to eq('brouillon')
+          expect(dossier.en_instruction_at).to be_nil
+          expect(dossier.processed_at).to be_nil
+        }
+      end
+
+      context 'for entreprise' do
+        let(:procedure) { create(:procedure, :published, :with_instructeur, for_individual: false, declarative_with_state: state) }
+
+        let(:dossier) { create(:dossier, :en_construction, :with_entreprise, :with_attestation, procedure:, as_degraded_mode: false) }
+
+        it { expect(dossier).to be_accepte }
+
+        context 'having etablissement in degraded_mode' do
+          let(:dossier) { create(:dossier, :en_construction, :with_entreprise, :with_attestation, procedure:, as_degraded_mode: true) }
+
+          before do
+            expect(dossier).to_not receive(:accepter_automatiquement!)
+            expect(Sentry).to_not receive(:capture_exception)
+          end
+
+          it { expect(dossier).to be_en_construction }
+        end
+      end
+    end
+  end
+end