[#2750] Move sorted_ids to ProcedurePresentation

app/controllers/new_gestionnaire/procedures_controller.rb

@@ -69,8 +69,7 @@ module NewGestionnaire
         @archived_dossiers
       end
 
-      dossier_field_service = DossierFieldService.new
+      sorted_ids = procedure_presentation.sorted_ids(@dossiers, current_gestionnaire)
-      sorted_ids = dossier_field_service.sorted_ids(@dossiers, procedure_presentation, current_gestionnaire)
 
       if @current_filters.count > 0
         filtered_ids = procedure_presentation.filtered_ids(@dossiers, statut)

app/models/procedure_presentation.rb

@@ -52,10 +52,44 @@ class ProcedurePresentation < ApplicationRecord
     end
   end
 
+  def sorted_ids(dossiers, gestionnaire)
+    table = sort['table']
+    column = sanitized_column(sort)
+    order = sort['order']
+    assert_valid_order(order)
+
+    case table
+    when 'notifications'
+      dossiers_id_with_notification = gestionnaire.notifications_for_procedure(procedure)
+      if order == 'desc'
+        return dossiers_id_with_notification +
+            (dossiers.order('dossiers.updated_at desc').ids - dossiers_id_with_notification)
+      else
+        return (dossiers.order('dossiers.updated_at asc').ids - dossiers_id_with_notification) +
+            dossiers_id_with_notification
+      end
+    when 'self'
+      return dossiers
+          .order("#{column} #{order}")
+          .pluck(:id)
+    when 'type_de_champ', 'type_de_champ_private'
+      return dossiers
+          .includes(table == 'type_de_champ' ? :champs : :champs_private)
+          .where("champs.type_de_champ_id = #{sort['column'].to_i}")
+          .order("champs.value #{order}")
+          .pluck(:id)
+    else
+      return dossiers
+          .includes(table)
+          .order("#{column} #{order}")
+          .pluck(:id)
+    end
+  end
+
   def filtered_ids(dossiers, statut)
     filters[statut].map do |filter|
       table = filter['table']
-      column = dossier_field_service.sanitized_column(filter)
+      column = sanitized_column(filter)
       case table
       when 'self'
         dossiers.where("? ILIKE ?", filter['column'], "%#{filter['value']}%")
@@ -92,6 +126,20 @@ class ProcedurePresentation < ApplicationRecord
 
   private
 
+  def assert_valid_order(order)
+    if !["asc", "desc"].include?(order)
+      raise "Invalid order #{order}"
+    end
+  end
+
+  def sanitized_column(field)
+    table = field['table']
+    table = ActiveRecord::Base.connection.quote_column_name((table == 'self' ? 'dossier' : table).pluralize)
+    column = ActiveRecord::Base.connection.quote_column_name(field['column'])
+
+    table + '.' + column
+  end
+
   def dossier_field_service
     @dossier_field_service ||= DossierFieldService.new
   end

app/services/dossier_field_service.rb

@@ -67,49 +67,6 @@ class DossierFieldService
     valid_columns_for_table(procedure, table).include?(column)
   end
 
-  def sorted_ids(dossiers, procedure_presentation, gestionnaire)
-    table = procedure_presentation.sort['table']
-    column = sanitized_column(procedure_presentation.sort)
-    order = procedure_presentation.sort['order']
-    assert_valid_order(order)
-
-    case table
-    when 'notifications'
-      procedure = procedure_presentation.assign_to.procedure
-      dossiers_id_with_notification = gestionnaire.notifications_for_procedure(procedure)
-      if order == 'desc'
-        return dossiers_id_with_notification +
-            (dossiers.order('dossiers.updated_at desc').ids - dossiers_id_with_notification)
-      else
-        return (dossiers.order('dossiers.updated_at asc').ids - dossiers_id_with_notification) +
-            dossiers_id_with_notification
-      end
-    when 'self'
-      return dossiers
-          .order("#{column} #{order}")
-          .pluck(:id)
-    when 'type_de_champ', 'type_de_champ_private'
-      return dossiers
-          .includes(table == 'type_de_champ' ? :champs : :champs_private)
-          .where("champs.type_de_champ_id = #{procedure_presentation.sort['column'].to_i}")
-          .order("champs.value #{order}")
-          .pluck(:id)
-    else
-      return dossiers
-          .includes(table)
-          .order("#{column} #{order}")
-          .pluck(:id)
-    end
-  end
-
-  def sanitized_column(field)
-    table = field['table']
-    table = ActiveRecord::Base.connection.quote_column_name((table == 'self' ? 'dossier' : table).pluralize)
-    column = ActiveRecord::Base.connection.quote_column_name(field['column'])
-
-    table + '.' + column
-  end
-
   private
 
   def valid_columns_for_table(procedure, table)
@@ -123,12 +80,6 @@ class DossierFieldService
     @column_whitelist[procedure.id][table] || []
   end
 
-  def assert_valid_order(order)
-    if !["asc", "desc"].include?(order)
-      raise "Invalid order #{order}"
-    end
-  end
-
   def field_hash(label, table, column)
     {
       'label' => label,

spec/models/procedure_presentation_spec.rb

@@ -1,7 +1,8 @@
 require 'spec_helper'
 
 describe ProcedurePresentation do
-  let(:assign_to) { create(:assign_to, procedure: create(:procedure, :with_type_de_champ)) }
+  let(:procedure) { create(:procedure, :with_type_de_champ, :with_type_de_champ_private) }
+  let(:assign_to) { create(:assign_to, procedure: procedure) }
   let(:first_type_de_champ_id) { assign_to.procedure.types_de_champ.first.id.to_s }
   let (:procedure_presentation_id) {
     ProcedurePresentation.create(
@@ -107,8 +108,104 @@ describe ProcedurePresentation do
     it { expect(subject.fields_for_select).to eq([["label1", "table1/column1"], ["label2", "table2/column2"]]) }
   end
 
+  describe '#sorted_ids' do
+    let(:gestionnaire) { create(:gestionnaire) }
+    let(:assign_to) { create(:assign_to, procedure: procedure, gestionnaire: gestionnaire) }
+    let(:sort) { { 'table' => table, 'column' => column, 'order' => order } }
+    let(:procedure_presentation) { ProcedurePresentation.create(assign_to: assign_to, sort: sort) }
+
+    subject { procedure_presentation.sorted_ids(procedure.dossiers, gestionnaire) }
+
+    context 'for notifications table' do
+      let(:table) { 'notifications' }
+      let(:column) { 'notifications' }
+
+      let!(:notified_dossier) { create(:dossier, :en_construction, procedure: procedure) }
+      let!(:recent_dossier) { create(:dossier, :en_construction, procedure: procedure) }
+      let!(:older_dossier) { create(:dossier, :en_construction, procedure: procedure) }
+
+      before do
+        notified_dossier.champs.first.touch(time: DateTime.new(2018, 9, 20))
+        create(:follow, gestionnaire: gestionnaire, dossier: notified_dossier, demande_seen_at: DateTime.new(2018, 9, 10))
+        recent_dossier.touch(time: DateTime.new(2018, 9, 25))
+        older_dossier.touch(time: DateTime.new(2018, 5, 13))
+      end
+
+      context 'in ascending order' do
+        let(:order) { 'asc' }
+
+        it { is_expected.to eq([older_dossier, recent_dossier, notified_dossier].map(&:id)) }
+      end
+
+      context 'in descending order' do
+        let(:order) { 'desc' }
+
+        it { is_expected.to eq([notified_dossier, recent_dossier, older_dossier].map(&:id)) }
+      end
+    end
+
+    context 'for self table' do
+      let(:table) { 'self' }
+      let(:column) { 'updated_at' } # All other columns work the same, no extra test required
+      let(:order) { 'asc' } # Desc works the same, no extra test required
+
+      let(:recent_dossier) { create(:dossier, procedure: procedure) }
+      let(:older_dossier) { create(:dossier, procedure: procedure) }
+
+      before do
+        recent_dossier.touch(time: DateTime.new(2018, 9, 25))
+        older_dossier.touch(time: DateTime.new(2018, 5, 13))
+      end
+
+      it { is_expected.to eq([older_dossier, recent_dossier].map(&:id)) }
+    end
+
+    context 'for type_de_champ table' do
+      let(:table) { 'type_de_champ' }
+      let(:column) { procedure.types_de_champ.first.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) }
+
+      before do
+        beurre_dossier.champs.first.update(value: 'beurre')
+        tartine_dossier.champs.first.update(value: 'tartine')
+      end
+
+      it { is_expected.to eq([tartine_dossier, beurre_dossier].map(&:id)) }
+    end
+
+    context 'for type_de_champ_private table' do
+      let(:table) { 'type_de_champ_private' }
+      let(:column) { procedure.types_de_champ_private.first.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) }
+
+      before do
+        biere_dossier.champs_private.first.update(value: 'biere')
+        vin_dossier.champs_private.first.update(value: 'vin')
+      end
+
+      it { is_expected.to eq([biere_dossier, vin_dossier].map(&:id)) }
+    end
+
+    context 'for other tables' do
+      # All other columns and tables work the same so it’s ok to test only one
+      let(:table) { 'etablissement' }
+      let(:column) { 'code_postal' }
+      let(:order) { 'asc' } # Desc works the same, no extra test required
+
+      let!(:huitieme_dossier) { create(:dossier, procedure: procedure, etablissement: create(:etablissement, code_postal: '75008')) }
+      let!(:vingtieme_dossier) { create(:dossier, procedure: procedure, etablissement: create(:etablissement, code_postal: '75020')) }
+
+      it { is_expected.to eq([huitieme_dossier, vingtieme_dossier].map(&:id)) }
+    end
+  end
+
   describe '#filtered_ids' do
-    let(:procedure) { create(:procedure, :with_type_de_champ, :with_type_de_champ_private) }
     let(:procedure_presentation) { create(:procedure_presentation, assign_to: create(:assign_to, procedure: procedure), filters: { "suivis" => filter }) }
 
     subject { procedure_presentation.filtered_ids(procedure.dossiers, 'suivis') }

spec/services/dossier_field_service_spec.rb

@@ -3,103 +3,6 @@ require 'spec_helper'
 describe DossierFieldService do
   let(:procedure) { create(:procedure, :with_type_de_champ, :with_type_de_champ_private) }
 
-  describe '#sorted_ids' do
-    let(:gestionnaire) { create(:gestionnaire) }
-    let(:assign_to) { create(:assign_to, procedure: procedure, gestionnaire: gestionnaire) }
-    let(:sort) { { 'table' => table, 'column' => column, 'order' => order } }
-    let(:procedure_presentation) { ProcedurePresentation.create(assign_to: assign_to, sort: sort) }
-
-    subject { described_class.new.sorted_ids(procedure.dossiers, procedure_presentation, gestionnaire) }
-
-    context 'for notifications table' do
-      let(:table) { 'notifications' }
-      let(:column) { 'notifications' }
-
-      let!(:notified_dossier) { create(:dossier, :en_construction, procedure: procedure) }
-      let!(:recent_dossier) { create(:dossier, :en_construction, procedure: procedure) }
-      let!(:older_dossier) { create(:dossier, :en_construction, procedure: procedure) }
-
-      before do
-        notified_dossier.champs.first.touch(time: DateTime.new(2018, 9, 20))
-        create(:follow, gestionnaire: gestionnaire, dossier: notified_dossier, demande_seen_at: DateTime.new(2018, 9, 10))
-        recent_dossier.touch(time: DateTime.new(2018, 9, 25))
-        older_dossier.touch(time: DateTime.new(2018, 5, 13))
-      end
-
-      context 'in ascending order' do
-        let(:order) { 'asc' }
-
-        it { is_expected.to eq([older_dossier, recent_dossier, notified_dossier].map(&:id)) }
-      end
-
-      context 'in descending order' do
-        let(:order) { 'desc' }
-
-        it { is_expected.to eq([notified_dossier, recent_dossier, older_dossier].map(&:id)) }
-      end
-    end
-
-    context 'for self table' do
-      let(:table) { 'self' }
-      let(:column) { 'updated_at' } # All other columns work the same, no extra test required
-      let(:order) { 'asc' } # Desc works the same, no extra test required
-
-      let(:recent_dossier) { create(:dossier, procedure: procedure) }
-      let(:older_dossier) { create(:dossier, procedure: procedure) }
-
-      before do
-        recent_dossier.touch(time: DateTime.new(2018, 9, 25))
-        older_dossier.touch(time: DateTime.new(2018, 5, 13))
-      end
-
-      it { is_expected.to eq([older_dossier, recent_dossier].map(&:id)) }
-    end
-
-    context 'for type_de_champ table' do
-      let(:table) { 'type_de_champ' }
-      let(:column) { procedure.types_de_champ.first.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) }
-
-      before do
-        beurre_dossier.champs.first.update(value: 'beurre')
-        tartine_dossier.champs.first.update(value: 'tartine')
-      end
-
-      it { is_expected.to eq([tartine_dossier, beurre_dossier].map(&:id)) }
-    end
-
-    context 'for type_de_champ_private table' do
-      let(:table) { 'type_de_champ_private' }
-      let(:column) { procedure.types_de_champ_private.first.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) }
-
-      before do
-        biere_dossier.champs_private.first.update(value: 'biere')
-        vin_dossier.champs_private.first.update(value: 'vin')
-      end
-
-      it { is_expected.to eq([biere_dossier, vin_dossier].map(&:id)) }
-    end
-
-    context 'for other tables' do
-      # All other columns and tables work the same so it’s ok to test only one
-      let(:table) { 'etablissement' }
-      let(:column) { 'code_postal' }
-      let(:order) { 'asc' } # Desc works the same, no extra test required
-
-      let!(:huitieme_dossier) { create(:dossier, procedure: procedure, etablissement: create(:etablissement, code_postal: '75008')) }
-      let!(:vingtieme_dossier) { create(:dossier, procedure: procedure, etablissement: create(:etablissement, code_postal: '75020')) }
-
-      it { is_expected.to eq([huitieme_dossier, vingtieme_dossier].map(&:id)) }
-    end
-  end
-
   describe '#get_value' do
     subject { described_class.new.get_value(dossier, table, column) }