From 6475cdff7a2a28672f906b6d57ce66a0d106b35e Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Fri, 23 Jul 2021 09:26:13 +0200 Subject: [PATCH 1/2] Revert "Suppression de la clef `"migrated": true` sur les filtres des ProcedurePresentation" --- ...39_remove_migration_status_on_filters.rake | 25 ----------- lib/tasks/task_helper.rb | 11 ----- ...remove_migration_status_on_filters_spec.rb | 45 ------------------- 3 files changed, 81 deletions(-) delete mode 100644 lib/tasks/deployment/20210720133539_remove_migration_status_on_filters.rake delete mode 100644 spec/lib/tasks/deployment/20210720133539_remove_migration_status_on_filters_spec.rb diff --git a/lib/tasks/deployment/20210720133539_remove_migration_status_on_filters.rake b/lib/tasks/deployment/20210720133539_remove_migration_status_on_filters.rake deleted file mode 100644 index 6ddb56443..000000000 --- a/lib/tasks/deployment/20210720133539_remove_migration_status_on_filters.rake +++ /dev/null @@ -1,25 +0,0 @@ -namespace :after_party do - desc 'Deployment task: remove_migration_status_on_filters' - task remove_migration_status_on_filters: :environment do - rake_puts "Running deploy task 'remove_migration_status_on_filters'" - - # In a9a4f6e2a801b19b127aae8eaec0d1f384b1a53a, a task to migrate ProcedurePresentation's filters - # was added. - # This task added a "migrated: true" key to all migrated filters. - # - # Now that this task has run, we can safely remove the extra key. - - procedure_presentations = ProcedurePresentation.where("filters -> 'migrated' IS NOT NULL") - progress = ProgressReport.new(procedure_presentations.count) - - procedure_presentations.find_each do |pp| - pp.filters.delete('migrated') - pp.save! - progress.inc - end - progress.finish - - AfterParty::TaskRecord - .create version: AfterParty::TaskRecorder.new(__FILE__).timestamp - end -end diff --git a/lib/tasks/task_helper.rb b/lib/tasks/task_helper.rb index 8323773d7..ef6a0bd95 100644 --- a/lib/tasks/task_helper.rb +++ b/lib/tasks/task_helper.rb @@ -15,17 +15,6 @@ def rake_print(*args) end end -# Display progress of a long-running Rake task. -# -# Usage: -# -# ``` -# progress = ProgressReport.new(100) -# (0..100).times do -# progress.inc -# end -# progress.finish -# ```` class ProgressReport def initialize(total) @start = Time.zone.now diff --git a/spec/lib/tasks/deployment/20210720133539_remove_migration_status_on_filters_spec.rb b/spec/lib/tasks/deployment/20210720133539_remove_migration_status_on_filters_spec.rb deleted file mode 100644 index e57f7a2d9..000000000 --- a/spec/lib/tasks/deployment/20210720133539_remove_migration_status_on_filters_spec.rb +++ /dev/null @@ -1,45 +0,0 @@ -describe '20201001161931_migrate_filters_to_use_stable_id' do - let(:rake_task) { Rake::Task['after_party:remove_migration_status_on_filters'] } - - let(:procedure) { create(:simple_procedure) } - let(:instructeur_1) { create(:instructeur) } - let(:instructeur_2) { create(:instructeur) } - - let(:assign_to_1) { create(:assign_to, procedure: procedure, instructeur: instructeur_1) } - let(:assign_to_2) { create(:assign_to, procedure: procedure, instructeur: instructeur_2) } - - let(:procedure_presentation_with_migration) { create(:procedure_presentation, assign_to: assign_to_1, filters: filters.merge('migrated': true)) } - let(:procedure_presentation_without_migration) { create(:procedure_presentation, assign_to: assign_to_2, filters: filters) } - - let(:filters) do - { "suivis" => [{ "table" => "user", "column" => "email", "value" => "test@example.com" }] } - end - - before do - procedure_presentation_with_migration - procedure_presentation_without_migration - - rake_task.invoke - - procedure_presentation_with_migration.reload - procedure_presentation_without_migration.reload - end - - after { rake_task.reenable } - - context 'when the procedure presentation has a "migrated" key' do - it 'removes the "migrated" key' do - expect(procedure_presentation_with_migration.filters).not_to have_key('migrated') - end - - it 'leaves other keys unchanged' do - expect(procedure_presentation_with_migration.filters['suivis']).to be_present - end - end - - context 'when the procedure presentation doesn’t have a "migrated" key' do - it 'leaves keys unchanged' do - expect(procedure_presentation_without_migration.filters['suivis']).to be_present - end - end -end From 64cfb4d64e30ee9c2110fd9d94e50f9af60abe22 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Thu, 22 Jul 2021 18:16:04 +0200 Subject: [PATCH 2/2] Fix sort with revisions --- .../instructeurs/procedures_controller.rb | 7 +- app/models/procedure_presentation.rb | 18 ++- spec/models/procedure_presentation_spec.rb | 116 +++++++++++++++--- 3 files changed, 118 insertions(+), 23 deletions(-) diff --git a/app/controllers/instructeurs/procedures_controller.rb b/app/controllers/instructeurs/procedures_controller.rb index b9c3e85bb..54e4ce7bf 100644 --- a/app/controllers/instructeurs/procedures_controller.rb +++ b/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) diff --git a/app/models/procedure_presentation.rb b/app/models/procedure_presentation.rb index eeb37160e..b5204ea2d 100644 --- a/app/models/procedure_presentation.rb +++ b/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 diff --git a/spec/models/procedure_presentation_spec.rb b/spec/models/procedure_presentation_spec.rb index 30e0209bb..c3d45668a 100644 --- a/spec/models/procedure_presentation_spec.rb +++ b/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,35 +205,113 @@ describe ProcedurePresentation do end context 'for type_de_champ table' 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 + context 'with no revisions' do + let(:table) { 'type_de_champ' } + let(:column) { procedure.types_de_champ.first.stable_id.to_s } - let(:beurre_dossier) { create(:dossier, procedure: procedure) } - let(:tartine_dossier) { create(:dossier, procedure: procedure) } + 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') + before do + beurre_dossier.champs.first.update(value: 'beurre') + 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 - it { is_expected.to eq([tartine_dossier, beurre_dossier].map(&:id)) } + 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 - 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 + context 'with no revisions' do + let(:table) { 'type_de_champ_private' } + let(:column) { procedure.types_de_champ_private.first.stable_id.to_s } - let(:biere_dossier) { create(:dossier, procedure: procedure) } - let(:vin_dossier) { create(:dossier, procedure: procedure) } + 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') + before do + biere_dossier.champs_private.first.update(value: 'biere') + 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 - it { is_expected.to eq([biere_dossier, vin_dossier].map(&:id)) } + 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