code review changes

app/controllers/new_administrateur/groupe_instructeurs_controller.rb

@@ -49,13 +49,14 @@ module NewAdministrateur
     end
 
     def destroy
-      if procedure.groupe_instructeurs.one?
+      if !groupe_instructeur.dossiers.empty?
+        flash[:alert] = "Impossible de supprimer un groupe avec des dossiers. Il faut le réaffecter avant"
+      elsif procedure.groupe_instructeurs.one?
         flash[:alert] = "Suppression impossible : il doit y avoir au moins un groupe instructeur sur chaque procédure"
-      elsif groupe_instructeur == procedure.defaut_groupe_instructeur
-        flash[:alert] = "Impossible de supprimer le groupe par défaut"
       else
-        flash[:notice] = "le groupe « #{groupe_instructeur.label} » a été supprimé."
+        label = groupe_instructeur.label
-        groupe_instructeur.destroy
+        groupe_instructeur.destroy!
+        flash[:notice] = "le groupe « #{label} » a été supprimé."
       end
       redirect_to procedure_groupe_instructeurs_path(procedure)
     end
@@ -68,16 +69,9 @@ module NewAdministrateur
     end
 
     def reaffecter
-      target_group = GroupeInstructeur.find(params[:target_group])
+      target_group = procedure.groupe_instructeurs.find(params[:target_group])
-      if target_group.blank? || !procedure.groupe_instructeurs.include?(target_group)
+      groupe_instructeur.dossiers.update_all(groupe_instructeur_id: target_group.id)
-        flash[:notice] = "Impossible de réaffecter les dossiers au groupe demandé."
-      else
-        groupe_instructeur.dossiers.each do |d|
-          d.update(groupe_instructeur: target_group)
-        end
       flash[:notice] = "Les dossiers du groupe « #{groupe_instructeur.label} » ont été réaffectés au groupe « #{target_group.label} »."
-
-      end
       redirect_to procedure_groupe_instructeurs_path(procedure)
     end
 

app/views/new_administrateur/groupe_instructeurs/index.html.haml

@@ -34,12 +34,9 @@
           %tr
             %td= group.label
             %td.actions= link_to "voir", procedure_groupe_instructeur_path(@procedure, group)
-            - if @groupes_instructeurs.count > 1
+            - if @groupes_instructeurs.many?
-              - if group.dossiers.count == 0
+              - if group.dossiers.empty?
                 %td.actions
-                  - if group == @procedure.defaut_groupe_instructeur
-                    Groupe par défaut − ne peut être supprimé
-                  - else
                   = link_to procedure_groupe_instructeur_path(@procedure, group), { method: :delete, class: 'button', data: { confirm: "Êtes-vous sûr de vouloir supprimer le groupe « #{group.label} » ?" }} do
                     %span.icon.delete
                     supprimer ce groupe

app/views/new_administrateur/groupe_instructeurs/reaffecter_dossiers.html.haml

@@ -7,7 +7,7 @@
 .container.groupe-instructeur
 
   .card
-    .card-title Réaffectation des dossiers du groupe « #{@groupe_instructeur.label} »
+    .card-title Réaffectation des dossiers du groupe « {@groupe_instructeur.label} »
     %p
       Le groupe « #{@groupe_instructeur.label} » contient des dossiers. Afin de procéder à sa suppression, vous devez réaffecter ses dossiers à un autre groupe instructeur.
 

spec/controllers/new_administrateur/groupe_instructeurs_controller_spec.rb

@@ -78,21 +78,22 @@ describe NewAdministrateur::GroupeInstructeursController, type: :controller do
 
     context 'with many groups' do
       let!(:gi_1_2) { procedure.groupe_instructeurs.create(label: 'groupe instructeur 2') }
-      context 'of the default group' do
-        before { delete_group procedure.defaut_groupe_instructeur }
-
-        it { expect(flash.alert).to be_present }
-        it { expect(procedure.groupe_instructeurs.count).to eq(2) }
-        it { expect(response).to redirect_to(procedure_groupe_instructeurs_path(procedure)) }
-      end
 
       context 'of a group that can be deleted' do
         before { delete_group gi_1_2 }
-
         it { expect(flash.notice).to be_present }
         it { expect(procedure.groupe_instructeurs.count).to eq(1) }
         it { expect(response).to redirect_to(procedure_groupe_instructeurs_path(procedure)) }
       end
+
+      context 'of a group with dossiers, that cannot be deleted' do
+        let!(:dossier12) { create(:dossier, procedure: procedure, state: Dossier.states.fetch(:en_construction), groupe_instructeur: gi_1_2) }
+        before { delete_group gi_1_2 }
+
+        it { expect(flash.alert).to be_present }
+        it { expect(procedure.groupe_instructeurs.count).to eq(2) }
+        it { expect(response).to redirect_to(procedure_groupe_instructeurs_path(procedure)) }
+      end
     end
   end
 
@@ -140,8 +141,8 @@ describe NewAdministrateur::GroupeInstructeursController, type: :controller do
       it { expect(dossier12.groupe_instructeur.id).to be(gi_1_2.id) }
     end
 
-    describe 'when the new group is not a possible group' do
+    describe 'when the target group is not a possible group' do
-      before do
+      subject {
         post :reaffecter,
           params:
             {
@@ -149,14 +150,12 @@ describe NewAdministrateur::GroupeInstructeursController, type: :controller do
               id: gi_1_1.id,
               target_group: gi_2_2.id
             }
+      }
+      before do
         dossier12.reload
       end
 
-      it { expect(response).to redirect_to(procedure_groupe_instructeurs_path(procedure)) }
+      it { expect { subject }.to raise_error(ActiveRecord::RecordNotFound) }
-      it { expect(gi_1_1.dossiers.count).to be(1) }
-      it { expect(gi_2_2.dossiers.count).to be(0) }
-      it { expect(gi_1_1.dossiers.last.id).to be(dossier12.id) }
-      it { expect(dossier12.groupe_instructeur.id).to be(gi_1_1.id) }
     end
   end