Fix sort with revisions

app/controllers/instructeurs/procedures_controller.rb

@@ -74,14 +74,19 @@ module Instructeurs
 
       @dossiers = case statut
       when 'a-suivre'
+        dossiers_count = @a_suivre_count
         @a_suivre_dossiers
       when 'suivis'
+        dossiers_count = @suivis_count
         @followed_dossiers
       when 'traites'
+        dossiers_count = @traites_count
         @termines_dossiers
       when 'tous'
+        dossiers_count = @tous_count
         @all_state_dossiers
       when 'archives'
+        dossiers_count = @archives_count
         @archived_dossiers
       end
 
@@ -90,7 +95,7 @@ module Instructeurs
       @has_termine_notifications = notifications[:termines].present?
       @not_archived_notifications_dossier_ids = notifications[:en_cours] + notifications[:termines]
 
-      sorted_ids = procedure_presentation.sorted_ids(@dossiers, current_instructeur)
+      sorted_ids = procedure_presentation.sorted_ids(@dossiers, dossiers_count, current_instructeur)
 
       if @current_filters.count > 0
         filtered_ids = procedure_presentation.filtered_ids(@dossiers, statut)

app/models/procedure_presentation.rb

@@ -83,7 +83,7 @@ class ProcedurePresentation < ApplicationRecord
     ]
   end
 
-  def sorted_ids(dossiers, instructeur)
+  def sorted_ids(dossiers, count, instructeur)
     table, column, order = sort.values_at(TABLE, COLUMN, 'order')
 
     case table
@@ -97,15 +97,27 @@ class ProcedurePresentation < ApplicationRecord
             dossiers_id_with_notification
       end
     when TYPE_DE_CHAMP
-      dossiers
+      ids = dossiers
         .with_type_de_champ(column)
         .order("champs.value #{order}")
         .pluck(:id)
+      if ids.size != count
+        rest = dossiers.where.not(id: ids).order(id: order).pluck(:id)
+        order == 'asc' ? ids + rest : rest + ids
+      else
+        ids
+      end
     when TYPE_DE_CHAMP_PRIVATE
-      dossiers
+      ids = dossiers
         .with_type_de_champ_private(column)
         .order("champs.value #{order}")
         .pluck(:id)
+      if ids.size != count
+        rest = dossiers.where.not(id: ids).order(id: order).pluck(:id)
+        order == 'asc' ? ids + rest : rest + ids
+      else
+        ids
+      end
     when 'followers_instructeurs'
       assert_supported_column(table, column)
       # LEFT OUTER JOIN allows to keep dossiers without assignated instructeurs yet

spec/models/procedure_presentation_spec.rb

@@ -132,7 +132,7 @@ describe ProcedurePresentation do
     let(:sort) { { 'table' => table, 'column' => column, 'order' => order } }
     let(:procedure_presentation) { create(:procedure_presentation, assign_to: assign_to, sort: sort) }
 
-    subject { procedure_presentation.sorted_ids(procedure.dossiers, instructeur) }
+    subject { procedure_presentation.sorted_ids(procedure.dossiers, procedure.dossiers.count, instructeur) }
 
     context 'for notifications table' do
       let(:table) { 'notifications' }
@@ -205,9 +205,9 @@ describe ProcedurePresentation do
     end
 
     context 'for type_de_champ table' do
+      context 'with no revisions' do
         let(:table) { 'type_de_champ' }
         let(:column) { procedure.types_de_champ.first.stable_id.to_s }
-      let(:order) { 'desc' } # Asc works the same, no extra test required
 
         let(:beurre_dossier) { create(:dossier, procedure: procedure) }
         let(:tartine_dossier) { create(:dossier, procedure: procedure) }
@@ -217,13 +217,52 @@ describe ProcedurePresentation do
           tartine_dossier.champs.first.update(value: 'tartine')
         end
 
+        context 'asc' do
+          let(:order) { 'asc' }
+
+          it { is_expected.to eq([beurre_dossier, tartine_dossier].map(&:id)) }
+        end
+
+        context 'desc' do
+          let(:order) { 'desc' }
+
           it { is_expected.to eq([tartine_dossier, beurre_dossier].map(&:id)) }
         end
+      end
+
+      context 'with a revision adding a new type_de_champ' do
+        let!(:tdc) { { type_champ: :text, libelle: 'nouveau champ' } }
+        let(:table) { 'type_de_champ' }
+        let(:column) { procedure.types_de_champ.last.stable_id.to_s }
+
+        let(:nothing_dossier) { create(:dossier, procedure: procedure) }
+        let(:beurre_dossier) { create(:dossier, procedure: procedure) }
+        let(:tartine_dossier) { create(:dossier, procedure: procedure) }
+
+        before do
+          nothing_dossier
+          procedure.draft_revision.add_type_de_champ(tdc)
+          procedure.publish_revision!
+          beurre_dossier.champs.last.update(value: 'beurre')
+          tartine_dossier.champs.last.update(value: 'tartine')
+        end
+
+        context 'asc' do
+          let(:order) { 'asc' }
+          it { is_expected.to eq([beurre_dossier, tartine_dossier, nothing_dossier].map(&:id)) }
+        end
+
+        context 'desc' do
+          let(:order) { 'desc' }
+          it { is_expected.to eq([nothing_dossier, tartine_dossier, beurre_dossier].map(&:id)) }
+        end
+      end
+    end
 
     context 'for type_de_champ_private table' do
+      context 'with no revisions' do
         let(:table) { 'type_de_champ_private' }
         let(:column) { procedure.types_de_champ_private.first.stable_id.to_s }
-      let(:order) { 'asc' } # Desc works the same, no extra test required
 
         let(:biere_dossier) { create(:dossier, procedure: procedure) }
         let(:vin_dossier) { create(:dossier, procedure: procedure) }
@@ -233,9 +272,48 @@ describe ProcedurePresentation do
           vin_dossier.champs_private.first.update(value: 'vin')
         end
 
+        context 'asc' do
+          let(:order) { 'asc' }
+
           it { is_expected.to eq([biere_dossier, vin_dossier].map(&:id)) }
         end
 
+        context 'desc' do
+          let(:order) { 'desc' }
+
+          it { is_expected.to eq([vin_dossier, biere_dossier].map(&:id)) }
+        end
+      end
+
+      context 'with a revision adding a new type_de_champ' do
+        let!(:tdc) { { type_champ: :text, private: true, libelle: 'nouveau champ' } }
+        let(:table) { 'type_de_champ_private' }
+        let(:column) { procedure.types_de_champ_private.last.stable_id.to_s }
+
+        let(:nothing_dossier) { create(:dossier, procedure: procedure) }
+        let(:biere_dossier) { create(:dossier, procedure: procedure) }
+        let(:vin_dossier) { create(:dossier, procedure: procedure) }
+
+        before do
+          nothing_dossier
+          procedure.draft_revision.add_type_de_champ(tdc)
+          procedure.publish_revision!
+          biere_dossier.champs_private.last.update(value: 'biere')
+          vin_dossier.champs_private.last.update(value: 'vin')
+        end
+
+        context 'asc' do
+          let(:order) { 'asc' }
+          it { is_expected.to eq([biere_dossier, vin_dossier, nothing_dossier].map(&:id)) }
+        end
+
+        context 'desc' do
+          let(:order) { 'desc' }
+          it { is_expected.to eq([nothing_dossier, vin_dossier, biere_dossier].map(&:id)) }
+        end
+      end
+    end
+
     context 'for individual table' do
       let(:table) { 'individual' }
       let(:order) { 'asc' } # Desc works the same, no extra test required