Merge pull request #8019 from tchak/fix-flaky-test

fix(groupe_instructeur): add explicit order to groupe_instructeurs

app/controllers/administrateurs/groupe_instructeurs_controller.rb

@@ -296,7 +296,6 @@ module Administrateurs
         .groupe_instructeurs
         .page(params[:page])
         .per(ITEMS_PER_PAGE)
-        .order(:label)
     end
 
     def paginated_instructeurs

app/controllers/instructeurs/groupe_instructeurs_controller.rb

@@ -77,7 +77,6 @@ module Instructeurs
         .where(procedure: procedure)
         .page(params[:page])
         .per(ITEMS_PER_PAGE)
-        .order(:label)
     end
 
     def paginated_instructeurs

app/models/bulk_message.rb

@@ -13,6 +13,6 @@
 #
 class BulkMessage < ApplicationRecord
   belongs_to :instructeur
-  has_and_belongs_to_many :groupe_instructeurs
+  has_and_belongs_to_many :groupe_instructeurs, -> { order(:label) }
   has_one_attached :piece_jointe
 end

app/models/instructeur.rb

@@ -15,14 +15,15 @@ class Instructeur < ApplicationRecord
   has_and_belongs_to_many :administrateurs
 
   has_many :assign_to, dependent: :destroy
-  has_many :groupe_instructeurs, through: :assign_to
+  has_many :groupe_instructeurs, -> { order(:label) }, through: :assign_to
-  has_many :procedures, -> { distinct }, through: :groupe_instructeurs
+  has_many :unordered_groupe_instructeurs, through: :assign_to, source: :groupe_instructeur
+  has_many :procedures, -> { distinct }, through: :unordered_groupe_instructeurs
 
   has_many :assign_to_with_email_notifications, -> { with_email_notifications }, class_name: 'AssignTo', inverse_of: :instructeur
   has_many :groupe_instructeur_with_email_notifications, through: :assign_to_with_email_notifications, source: :groupe_instructeur
 
   has_many :commentaires, inverse_of: :instructeur, dependent: :nullify
-  has_many :dossiers, -> { state_not_brouillon }, through: :groupe_instructeurs
+  has_many :dossiers, -> { state_not_brouillon }, through: :unordered_groupe_instructeurs
   has_many :follows, -> { active }, inverse_of: :instructeur
   has_many :previous_follows, -> { inactive }, class_name: 'Follow', inverse_of: :instructeur
   has_many :followed_dossiers, through: :follows, source: :dossier

app/models/procedure.rb

@@ -191,7 +191,7 @@ class Procedure < ApplicationRecord
 
   has_many :administrateurs_procedures, dependent: :delete_all
   has_many :administrateurs, through: :administrateurs_procedures, after_remove: -> (procedure, _admin) { procedure.validate! }
-  has_many :groupe_instructeurs, dependent: :destroy
+  has_many :groupe_instructeurs, -> { order(:label) }, inverse_of: :procedure, dependent: :destroy
   has_many :instructeurs, through: :groupe_instructeurs
 
   # This relationship is used in following dossiers through. We can not use revisions relationship
@@ -205,7 +205,7 @@ class Procedure < ApplicationRecord
   has_one :refused_mail, class_name: "Mails::RefusedMail", dependent: :destroy
   has_one :without_continuation_mail, class_name: "Mails::WithoutContinuationMail", dependent: :destroy
 
-  has_one :defaut_groupe_instructeur, -> { actif.order(:label) }, class_name: 'GroupeInstructeur', inverse_of: :procedure
+  has_one :defaut_groupe_instructeur, -> { actif.order(:label) }, class_name: 'GroupeInstructeur', inverse_of: false
 
   has_one_attached :logo
   has_one_attached :notice

app/views/shared/dossiers/_edit.html.haml

@@ -40,7 +40,7 @@
           = dossier.procedure.routing_criteria_name
           %span.mandatory *
         = f.select :groupe_instructeur_id,
-          dossier.procedure.groupe_instructeurs.where(closed: false).order(:label).map { |gi| [gi.label, gi.id] },
+          dossier.procedure.groupe_instructeurs.actif.map { |gi| [gi.label, gi.id] },
           { include_blank: dossier.brouillon? }
 
     - dossier.champs.each do |champ|

spec/controllers/administrateurs/groupe_instructeurs_controller_spec.rb

@@ -389,7 +389,7 @@ describe Administrateurs::GroupeInstructeursController, type: :controller do
       before { subject }
 
       it { expect(response.status).to eq(302) }
-      it { expect(procedure.groupe_instructeurs.last.label).to eq("Afrique") }
+      it { expect(procedure.groupe_instructeurs.first.label).to eq("Afrique") }
       it { expect(flash.alert).to be_present }
       it { expect(flash.alert).to eq("Import terminé. Cependant les emails suivants ne sont pas pris en compte: kara") }
     end
@@ -408,7 +408,7 @@ describe Administrateurs::GroupeInstructeursController, type: :controller do
 
       before { subject }
 
-      it { expect(procedure.groupe_instructeurs.pluck(:label)).to eq(["défaut", "Auvergne-Rhône-Alpes", "Vendée"]) }
+      it { expect(procedure.groupe_instructeurs.pluck(:label)).to match_array(["Auvergne-Rhône-Alpes", "Vendée", "défaut"]) }
       it { expect(flash.notice).to be_present }
       it { expect(flash.notice).to eq("La liste des instructeurs a été importée avec succès") }
     end

spec/controllers/instructeurs/procedures_controller_spec.rb

@@ -545,10 +545,11 @@ describe Instructeurs::ProceduresController, type: :controller do
 
   describe '#create_multiple_commentaire' do
     let(:instructeur) { create(:instructeur) }
-    let!(:gi_p1_1) { GroupeInstructeur.create(label: '1', procedure: procedure) }
+    let!(:gi_p1_1) { create(:groupe_instructeur, label: '1', procedure: procedure) }
-    let!(:gi_p1_2) { GroupeInstructeur.create(label: '2', procedure: procedure) }
+    let!(:gi_p1_2) { create(:groupe_instructeur, label: '2', procedure: procedure) }
     let(:body) { "avant\napres" }
-    let!(:dossier) { create(:dossier, state: "brouillon", procedure: procedure, groupe_instructeur: procedure.groupe_instructeurs.first) }
+    let(:bulk_message) { BulkMessage.first }
+    let!(:dossier) { create(:dossier, state: "brouillon", procedure: procedure, groupe_instructeur: gi_p1_1) }
     let!(:dossier_2) { create(:dossier, state: "brouillon", procedure: procedure, groupe_instructeur: gi_p1_1) }
     let!(:dossier_3) { create(:dossier, state: "brouillon", procedure: procedure, groupe_instructeur: gi_p1_2) }
     let!(:procedure) { create(:procedure, :published, instructeurs: [instructeur]) }
@@ -565,16 +566,16 @@ describe Instructeurs::ProceduresController, type: :controller do
     end
 
     it "creates a commentaire for 2 dossiers" do
-      expect(Commentaire.all.count).to eq(2)
+      expect(Commentaire.count).to eq(2)
       expect(dossier.commentaires.first.body).to eq("avant\napres")
       expect(dossier_2.commentaires.first.body).to eq("avant\napres")
       expect(dossier_3.commentaires).to eq([])
     end
 
     it "creates a Bulk Message for 2 groupes instructeurs" do
-      expect(BulkMessage.all.count).to eq(1)
+      expect(BulkMessage.count).to eq(1)
-      expect(BulkMessage.all.first.body).to eq("avant\napres")
+      expect(bulk_message.body).to eq("avant\napres")
-      expect(BulkMessage.all.first.groupe_instructeurs.sort).to match([procedure.groupe_instructeurs.first, gi_p1_1])
+      expect(bulk_message.groupe_instructeurs).to match([gi_p1_1])
     end
 
     it "creates a flash notice" do

spec/views/shared/dossiers/_champs.html.haml_spec.rb

@@ -59,7 +59,7 @@ describe 'shared/dossiers/champs.html.haml', type: :view do
         :routee,
         routing_criteria_name: 'departement')
     end
-    let(:dossier) { create(:dossier, procedure: procedure) }
+    let(:dossier) { create(:dossier, :en_construction, procedure: procedure) }
     let(:champs) { [] }
 
     it "renders the routing criteria name and its value" do