Use dossier.assign_to_groupe_instructeur

app/controllers/new_administrateur/groupe_instructeurs_controller.rb

@@ -70,7 +70,11 @@ module NewAdministrateur
 
     def reaffecter
       target_group = procedure.groupe_instructeurs.find(params[:target_group])
-      groupe_instructeur.dossiers.update_all(groupe_instructeur_id: target_group.id)
+
+      groupe_instructeur.dossiers.find_each do |dossier|
+        dossier.assign_to_groupe_instructeur(target_group, current_administrateur)
+      end
+
       flash[:notice] = "Les dossiers du groupe « #{groupe_instructeur.label} » ont été réaffectés au groupe « #{target_group.label} »."
       redirect_to procedure_groupe_instructeurs_path(procedure)
     end

app/controllers/users/dossiers_controller.rb

@@ -297,14 +297,13 @@ module Users
     end
 
     # FIXME: require(:dossier) when all the champs are united
-    def champs_and_groupe_instructeurs_params
+    def champs_params
-      params.permit(dossier: [
+      params.permit(dossier: {
-        :groupe_instructeur_id,
         champs_attributes: [
           :id, :value, :primary_value, :secondary_value, :piece_justificative_file, value: [],
           champs_attributes: [:id, :_destroy, :value, :primary_value, :secondary_value, :piece_justificative_file, value: []]
         ]
-      ])
+      })
     end
 
     def dossier
@@ -315,11 +314,20 @@ module Users
       Dossier.with_champs.find(params[:id])
     end
 
+    def change_groupe_instructeur?
+      params[:dossier][:groupe_instructeur_id].present? && @dossier.groupe_instructeur_id != params[:dossier][:groupe_instructeur_id].to_i
+    end
+
     def update_dossier_and_compute_errors
       errors = []
 
-      if champs_and_groupe_instructeurs_params[:dossier] && !@dossier.update(champs_and_groupe_instructeurs_params[:dossier])
+      if champs_params[:dossier]
-        errors += @dossier.errors.full_messages
+        if !@dossier.update(champs_params[:dossier])
+          errors += @dossier.errors.full_messages
+        elsif change_groupe_instructeur?
+          groupe_instructeur = @dossier.procedure.groupe_instructeurs.find(params[:dossier][:groupe_instructeur_id])
+          @dossier.assign_to_groupe_instructeur(groupe_instructeur)
+        end
       end
 
       if !save_draft?

app/models/dossier.rb

@@ -213,7 +213,6 @@ class Dossier < ApplicationRecord
   before_save :build_default_champs, if: Proc.new { groupe_instructeur_id_was.nil? }
   before_save :update_search_terms
 
-  after_save :unfollow_stale_instructeurs
   after_save :send_dossier_received
   after_save :send_web_hook
   after_create :send_draft_notification_email
@@ -300,6 +299,22 @@ class Dossier < ApplicationRecord
     instruction_commencee? && retention_end_date <= Time.zone.now
   end
 
+  def assign_to_groupe_instructeur(groupe_instructeur, author = nil)
+    if groupe_instructeur.procedure == procedure && groupe_instructeur != self.groupe_instructeur
+      if update(groupe_instructeur: groupe_instructeur)
+        unfollow_stale_instructeurs
+
+        if author.present?
+          log_dossier_operation(author, :changer_groupe_instructeur, self)
+        end
+
+        true
+      end
+    else
+      false
+    end
+  end
+
   def text_summary
     if brouillon?
       parts = [
@@ -647,14 +662,11 @@ class Dossier < ApplicationRecord
   end
 
   def unfollow_stale_instructeurs
-    if saved_change_to_groupe_instructeur_id? && saved_change_to_groupe_instructeur_id[0].present?
+    followers_instructeurs.each do |instructeur|
-      followers_instructeurs.each do |instructeur|
+      if instructeur.groupe_instructeurs.exclude?(groupe_instructeur)
-        if instructeur.groupe_instructeurs.exclude?(groupe_instructeur)
+        instructeur.unfollow(self)
-          instructeur.unfollow(self)
+        DossierMailer.notify_groupe_instructeur_changed(instructeur, self).deliver_later
-          DossierMailer.notify_groupe_instructeur_changed(instructeur, self).deliver_later
-        end
       end
-      log_dossier_operation(user, :changer_groupe_instructeur, self)
     end
   end
 

spec/models/dossier_spec.rb

@@ -398,12 +398,29 @@ describe Dossier do
     it { is_expected.to match([dossier3, dossier4, dossier2]) }
   end
 
+  describe "#assign_to_groupe_instructeur" do
+    let(:procedure) { create(:procedure) }
+    let(:new_groupe_instructeur_new_procedure) { create(:groupe_instructeur) }
+    let(:new_groupe_instructeur) { create(:groupe_instructeur, procedure: procedure) }
+    let(:dossier) { create(:dossier, :en_construction, procedure: procedure) }
+
+    it "can change groupe instructeur" do
+      expect(dossier.assign_to_groupe_instructeur(new_groupe_instructeur_new_procedure)).to be_falsey
+      expect(dossier.groupe_instructeur).not_to eq(new_groupe_instructeur_new_procedure)
+    end
+
+    it "can not change groupe instructeur if new groupe is from another procedure" do
+      expect(dossier.assign_to_groupe_instructeur(new_groupe_instructeur)).to be_truthy
+      expect(dossier.groupe_instructeur).to eq(new_groupe_instructeur)
+    end
+  end
+
   describe "#unfollow_stale_instructeurs" do
     let(:procedure) { create(:procedure) }
     let(:instructeur) { create(:instructeur) }
-    let(:new_groupe_instructeur) { create(:groupe_instructeur) }
+    let(:new_groupe_instructeur) { create(:groupe_instructeur, procedure: procedure) }
     let(:instructeur2) { create(:instructeur, groupe_instructeurs: [procedure.defaut_groupe_instructeur, new_groupe_instructeur]) }
-    let(:dossier) { create(:dossier, procedure: procedure, state: Dossier.states.fetch(:en_construction)) }
+    let(:dossier) { create(:dossier, :en_construction, procedure: procedure) }
     let(:last_operation) { DossierOperationLog.last }
 
     before do
@@ -413,7 +430,7 @@ describe Dossier do
     it "unfollows stale instructeurs when groupe instructeur change" do
       instructeur.follow(dossier)
       instructeur2.follow(dossier)
-      dossier.reload.update(groupe_instructeur: new_groupe_instructeur)
+      dossier.reload.assign_to_groupe_instructeur(new_groupe_instructeur, procedure.administrateurs.first)
 
       expect(dossier.reload.followers_instructeurs).not_to include(instructeur)
       expect(dossier.reload.followers_instructeurs).to include(instructeur2)