use join table instead of arrays

app/components/dossiers/batch_alert_component/batch_alert_component.html.haml

@@ -1,9 +1,9 @@
 .fr-mb-5v
   - if @batch.finished_at.present?
-    = render Dsfr::AlertComponent.new(title: t(".#{batch.operation}.finish.title"), state: (@batch.failed_dossier_ids.size.positive? ? :warning : :success), heading_level: 'h2') do |c|
+    = render Dsfr::AlertComponent.new(title: t(".#{batch.operation}.finish.title"), state: (@batch.errors? ? :warning : :success), heading_level: 'h2') do |c|
       - c.body do
         %p
-          = t(".#{batch.operation}.finish.text_success", count: @batch.total_count, success_count: @batch.success_dossier_ids.size)
+          = t(".#{batch.operation}.finish.text_success", count: @batch.total_count, success_count: @batch.success_count)
 
 
   - else

app/controllers/instructeurs/batch_operations_controller.rb

@@ -15,7 +15,6 @@ module Instructeurs
       params.require(:batch_operation)
         .permit(:operation, dossier_ids: [])
         .merge(instructeur: current_instructeur)
-        .merge(groupe_instructeurs: current_instructeur.groupe_instructeurs.where(procedure_id: @procedure.id))
     end
 
     def set_procedure

app/models/batch_operation.rb

@@ -22,11 +22,14 @@ class BatchOperation < ApplicationRecord
   }
 
   has_many :dossiers, dependent: :nullify
-  has_and_belongs_to_many :groupe_instructeurs
+  has_many :dossier_operations, class_name: 'DossierBatchOperation', dependent: :destroy
+  has_many :groupe_instructeurs, through: :dossier_operations
   belongs_to :instructeur
 
   validates :operation, presence: true
 
+  before_create :build_operations
+
   RETENTION_DURATION = 4.hours
   MAX_DUREE_GENERATION = 24.hours
 
@@ -67,28 +70,17 @@ class BatchOperation < ApplicationRecord
     end
   end
 
-  # use Arel::UpdateManager for array_append/array_remove (inspired by atomic_append)
-  #   see: https://www.rubydoc.info/gems/arel/Arel/UpdateManager
-  #   we use this approach to ensure atomicity
   def track_processed_dossier(success, dossier)
     transaction do
-      dossier.update(batch_operation: nil)
-      manager = Arel::UpdateManager.new.table(arel_table).where(arel_table[:id].eq(id))
-      values = []
-      values.push([arel_table[:run_at], Time.zone.now]) if called_for_first_time?
-      values.push([arel_table[:finished_at], Time.zone.now]) if called_for_last_time?(dossier)
-      values.push([arel_table[:updated_at], Time.zone.now])
+      dossiers.delete(dossier)
+      touch(:run_at) if called_for_first_time?
+      touch(:finished_at) if called_for_last_time?(dossier)
 
-      # NOTE: ensure to append BigInteger to SQL array by casting IDs
       if success
-        values.push([arel_table[:success_dossier_ids], Arel::Nodes::NamedFunction.new('array_append', [arel_table[:success_dossier_ids], Arel::Nodes::SqlLiteral.new("#{dossier.id}::BIGINT")])])
-        values.push([arel_table[:failed_dossier_ids], Arel::Nodes::NamedFunction.new('array_remove', [arel_table[:failed_dossier_ids], Arel::Nodes::SqlLiteral.new("#{dossier.id}::BIGINT")])])
+        dossier_operation(dossier).done!
       else
-        values.push([arel_table[:failed_dossier_ids], Arel::Nodes::NamedFunction.new('array_append', [arel_table[:failed_dossier_ids], Arel::Nodes::SqlLiteral.new("#{dossier.id}::BIGINT")])])
+        dossier_operation(dossier).fail!
       end
-
-      manager.set(values)
-      ActiveRecord::Base.connection.update(manager.to_sql)
     end
   end
 
@@ -112,27 +104,33 @@ class BatchOperation < ApplicationRecord
     run_at.nil?
   end
 
-  # beware, must be reloaded first
   def called_for_last_time?(dossier_to_ignore)
-    dossiers.where.not(id: dossier_to_ignore.id).empty?
+    dossiers.count.zero?
   end
 
   def total_count
-    total = failed_dossier_ids.size + success_dossier_ids.size
-
-    if finished_at.blank?
-      total += dossiers.count
-    end
-    total
+    dossier_operations.size
   end
 
   def progress_count
-    failed_dossier_ids.size + success_dossier_ids.size
+    dossier_operations.pending.size
+  end
+
+  def success_count
+    dossier_operations.success.size
+  end
+
+  def errors?
+    dossier_operations.error.present?
   end
 
   private
 
-  def arel_table
-    BatchOperation.arel_table
+  def dossier_operation(dossier)
+    dossier_operations.find_by!(dossier:)
+  end
+
+  def build_operations
+    dossier_operations.build(dossiers.map { { dossier: _1 } })
   end
 end

app/models/dossier.rb

@@ -146,6 +146,8 @@ class Dossier < ApplicationRecord
   belongs_to :user, optional: true
   belongs_to :parent_dossier, class_name: 'Dossier', optional: true
   belongs_to :batch_operation, optional: true
+  has_many :dossier_batch_operations
+  has_many :batch_operations, through: :dossier_batch_operations
   has_one :france_connect_information, through: :user
 
   has_one :procedure, through: :revision

app/models/dossier_batch_operation.rb

@@ -0,0 +1,39 @@
+# == Schema Information
+#
+# Table name: dossier_batch_operations
+#
+#  id                 :bigint           not null, primary key
+#  state              :string           default("pending"), not null
+#  created_at         :datetime         not null
+#  updated_at         :datetime         not null
+#  batch_operation_id :bigint           not null
+#  dossier_id         :bigint           not null
+#
+class DossierBatchOperation < ApplicationRecord
+  belongs_to :dossier
+  belongs_to :batch_operation
+  has_many :groupe_instructeurs, through: :dossier
+
+  enum state: {
+    pending: 'pending',
+    success: 'success',
+    error:   'error'
+  }
+
+  include AASM
+
+  aasm whiny_persistence: true, column: :state, enum: true do
+    state :pending, initial: true
+    state :success
+    state :error
+
+    event :done do
+      transitions from: :pending, to: :success
+      transitions from: :error, to: :success
+    end
+
+    event :fail do
+      transitions from: :pending, to: :error
+    end
+  end
+end

app/models/groupe_instructeur.rb

@@ -16,9 +16,9 @@ class GroupeInstructeur < ApplicationRecord
   has_many :instructeurs, through: :assign_tos
   has_many :dossiers
   has_many :deleted_dossiers
+  has_many :batch_operations, through: :dossiers, source: :batch_operations
   has_and_belongs_to_many :exports, dependent: :destroy
   has_and_belongs_to_many :bulk_messages, dependent: :destroy
-  has_and_belongs_to_many :batch_operations, dependent: :destroy
 
   validates :label, presence: true, allow_nil: false
   validates :label, uniqueness: { scope: :procedure }

db/migrate/20221222204553_create_dossier_batch_operations.rb

@@ -0,0 +1,10 @@
+class CreateDossierBatchOperations < ActiveRecord::Migration[6.1]
+  def change
+    create_table :dossier_batch_operations do |t|
+      t.references :dossier, null: false, foreign_key: true
+      t.references :batch_operation, null: false, foreign_key: true
+      t.string :state, null: false, default: DossierBatchOperation.states.fetch(:pending)
+      t.timestamps
+    end
+  end
+end

db/schema.rb

@@ -297,6 +297,16 @@ ActiveRecord::Schema.define(version: 2022_12_27_084442) do
     t.index ["procedure_id"], name: "index_deleted_dossiers_on_procedure_id"
   end
 
+  create_table "dossier_batch_operations", force: :cascade do |t|
+    t.bigint "batch_operation_id", null: false
+    t.datetime "created_at", precision: 6, null: false
+    t.bigint "dossier_id", null: false
+    t.string "state", default: "pending", null: false
+    t.datetime "updated_at", precision: 6, null: false
+    t.index ["batch_operation_id"], name: "index_dossier_batch_operations_on_batch_operation_id"
+    t.index ["dossier_id"], name: "index_dossier_batch_operations_on_dossier_id"
+  end
+
   create_table "dossier_operation_logs", force: :cascade do |t|
     t.boolean "automatic_operation", default: false, null: false
     t.bigint "bill_signature_id"

spec/jobs/batch_operation_process_one_job_spec.rb

@@ -11,7 +11,7 @@ describe BatchOperationProcessOneJob, type: :job do
     it 'when it works' do
       allow_any_instance_of(BatchOperation).to receive(:process_one).with(dossier_job).and_return(true)
       expect { subject.perform_now }
-        .to change { batch_operation.reload.success_dossier_ids }
+        .to change { batch_operation.dossier_operations.success.pluck(:dossier_id) }
         .from([])
         .to([dossier_job.id])
     end
@@ -20,7 +20,7 @@ describe BatchOperationProcessOneJob, type: :job do
       allow_any_instance_of(BatchOperation).to receive(:process_one).with(dossier_job).and_raise("boom")
       expect { subject.perform_now }.to raise_error('boom')
 
-      expect(batch_operation.reload.failed_dossier_ids).to eq([dossier_job.id])
+      expect(batch_operation.dossier_operations.error.pluck(:dossier_id)).to eq([dossier_job.id])
     end
 
     context 'when operation is "archiver"' do

spec/models/batch_operation_spec.rb

@@ -2,14 +2,12 @@ describe BatchOperation, type: :model do
   describe 'association' do
     it { is_expected.to have_many(:dossiers) }
     it { is_expected.to belong_to(:instructeur) }
-    it { is_expected.to have_and_belong_to_many(:groupe_instructeurs) }
+    it { is_expected.to have_many(:dossier_operations) }
   end
 
   describe 'attributes' do
     subject { BatchOperation.new }
     it { expect(subject.payload).to eq({}) }
-    it { expect(subject.failed_dossier_ids).to eq([]) }
-    it { expect(subject.success_dossier_ids).to eq([]) }
     it { expect(subject.run_at).to eq(nil) }
     it { expect(subject.finished_at).to eq(nil) }
     it { expect(subject.operation).to eq(nil) }
@@ -56,17 +54,20 @@ describe BatchOperation, type: :model do
     context 'when it succeed' do
       it 'pushes dossier_job id to batch_operation.success_dossier_ids' do
         expect { batch_operation.track_processed_dossier(true, dossier) }
-          .to change { batch_operation.reload.success_dossier_ids }
+          .to change { batch_operation.dossier_operations.success.pluck(:dossier_id) }
           .from([])
           .to([dossier.id])
       end
     end
 
     context 'when it succeed after a failure' do
-      let(:batch_operation) { create(:batch_operation, operation: :archiver, instructeur: instructeur, dossiers: [dossier], failed_dossier_ids: [dossier.id]) }
+      let(:batch_operation) { create(:batch_operation, operation: :archiver, instructeur: instructeur, dossiers: [dossier]) }
+      before do
+        batch_operation.track_processed_dossier(false, dossier)
+      end
       it 'remove former dossier id from failed_dossier_ids' do
         expect { batch_operation.track_processed_dossier(true, dossier) }
-          .to change { batch_operation.reload.failed_dossier_ids }
+          .to change { batch_operation.dossier_operations.error.pluck(:dossier_id) }
           .from([dossier.id])
           .to([])
       end
@@ -75,7 +76,7 @@ describe BatchOperation, type: :model do
     context 'when it fails' do
       it 'pushes dossier_job id to batch_operation.failed_dossier_ids' do
         expect { batch_operation.track_processed_dossier(false, dossier) }
-          .to change { batch_operation.reload.failed_dossier_ids }
+          .to change { batch_operation.dossier_operations.error.pluck(:dossier_id) }
           .from([])
           .to([dossier.id])
       end