From 706c648be87660d066d2e0ef0bec13d06e5a52e7 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Thu, 31 Mar 2022 16:16:38 +0200 Subject: [PATCH 1/4] refactor(procedure): remove unused code --- app/models/procedure.rb | 21 --------------------- spec/models/procedure_spec.rb | 11 ----------- 2 files changed, 32 deletions(-) diff --git a/app/models/procedure.rb b/app/models/procedure.rb index f1df0ab39..2fe7dd2e6 100644 --- a/app/models/procedure.rb +++ b/app/models/procedure.rb @@ -513,27 +513,6 @@ class Procedure < ApplicationRecord self.dossiers.state_not_brouillon.size end - def export_filename(format) - procedure_identifier = path || "procedure-#{id}" - "dossiers_#{procedure_identifier}_#{Time.zone.now.strftime('%Y-%m-%d_%H-%M')}.#{format}" - end - - def export(dossiers) - ProcedureExportService.new(self, dossiers) - end - - def to_csv(dossiers) - export(dossiers).to_csv - end - - def to_xlsx(dossiers) - export(dossiers).to_xlsx - end - - def to_ods(dossiers) - export(dossiers).to_ods - end - def procedure_overview(start_date, groups) ProcedureOverview.new(self, start_date, groups) end diff --git a/spec/models/procedure_spec.rb b/spec/models/procedure_spec.rb index 9bd6e6e85..b5327b1ff 100644 --- a/spec/models/procedure_spec.rb +++ b/spec/models/procedure_spec.rb @@ -1059,17 +1059,6 @@ describe Procedure do it { expect(Procedure.default_sort).to eq({ "table" => "self", "column" => "id", "order" => "desc" }) } end - describe "#export_filename" do - before { Timecop.freeze(Time.zone.local(2018, 1, 2, 23, 11, 14)) } - after { Timecop.return } - - subject { procedure.export_filename(:csv) } - - let(:procedure) { create(:procedure, :published) } - - it { is_expected.to eq("dossiers_#{procedure.path}_2018-01-02_23-11.csv") } - end - describe '#new_dossier' do let(:procedure) do create(:procedure, From f71c89aa913a1267873d37ad607986572ed69087 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Thu, 31 Mar 2022 16:31:05 +0200 Subject: [PATCH 2/4] refactor(procedure): cleanup tests --- spec/controllers/instructeurs/procedures_controller_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/controllers/instructeurs/procedures_controller_spec.rb b/spec/controllers/instructeurs/procedures_controller_spec.rb index e73ae4b1a..fa1249f7b 100644 --- a/spec/controllers/instructeurs/procedures_controller_spec.rb +++ b/spec/controllers/instructeurs/procedures_controller_spec.rb @@ -482,7 +482,7 @@ describe Instructeurs::ProceduresController, type: :controller do let(:instructeur) { create(:instructeur) } let!(:procedure) { create(:procedure) } let!(:gi_0) { procedure.defaut_groupe_instructeur } - let!(:gi_1) { GroupeInstructeur.create(label: 'gi_1', procedure: procedure, instructeurs: [instructeur]) } + let!(:gi_1) { create(:groupe_instructeur, label: 'gi_1', procedure: procedure, instructeurs: [instructeur]) } before { sign_in(instructeur.user) } @@ -511,7 +511,7 @@ describe Instructeurs::ProceduresController, type: :controller do end context 'when the export is ready' do - let!(:export) { create(:export, groupe_instructeurs: [gi_1]) } + let(:export) { create(:export, groupe_instructeurs: [gi_1]) } before do export.file.attach(io: StringIO.new('export'), filename: 'file.csv') @@ -524,7 +524,7 @@ describe Instructeurs::ProceduresController, type: :controller do end context 'when another export is ready' do - let!(:export) { create(:export, groupe_instructeurs: [gi_0, gi_1]) } + let(:export) { create(:export, groupe_instructeurs: [gi_0, gi_1]) } before do export.file.attach(io: StringIO.new('export'), filename: 'file.csv') From 6da54936b739f744de0dcc60b8f4de6985002a20 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Thu, 31 Mar 2022 16:32:45 +0200 Subject: [PATCH 3/4] refactor(procedure_presentation): use internal reference to instructeur --- app/controllers/instructeurs/procedures_controller.rb | 2 +- app/models/instructeur.rb | 6 +++++- app/models/procedure_presentation.rb | 5 ++--- spec/models/procedure_presentation_spec.rb | 2 +- 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/app/controllers/instructeurs/procedures_controller.rb b/app/controllers/instructeurs/procedures_controller.rb index a2e30afe3..7756c80a9 100644 --- a/app/controllers/instructeurs/procedures_controller.rb +++ b/app/controllers/instructeurs/procedures_controller.rb @@ -111,7 +111,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, dossiers_count, current_instructeur) + sorted_ids = procedure_presentation.sorted_ids(@dossiers, dossiers_count) if @current_filters.count > 0 filtered_ids = procedure_presentation.filtered_ids(@dossiers, statut) diff --git a/app/models/instructeur.rb b/app/models/instructeur.rb index 6c74b242c..4a0304e72 100644 --- a/app/models/instructeur.rb +++ b/app/models/instructeur.rb @@ -110,7 +110,11 @@ class Instructeur < ApplicationRecord end def procedure_presentation_and_errors_for_procedure_id(procedure_id) - assign_to.joins(:groupe_instructeur).find_by(groupe_instructeurs: { procedure_id: procedure_id }).procedure_presentation_or_default_and_errors + assign_to + .joins(:groupe_instructeur) + .includes(:instructeur, :procedure) + .find_by(groupe_instructeurs: { procedure_id: procedure_id }) + .procedure_presentation_or_default_and_errors end def notifications_for_dossier(dossier) diff --git a/app/models/procedure_presentation.rb b/app/models/procedure_presentation.rb index d85f20340..4a4a29bca 100644 --- a/app/models/procedure_presentation.rb +++ b/app/models/procedure_presentation.rb @@ -25,8 +25,7 @@ class ProcedurePresentation < ApplicationRecord FILTERS_VALUE_MAX_LENGTH = 100 belongs_to :assign_to, optional: false - - delegate :procedure, to: :assign_to + delegate :procedure, :instructeur, to: :assign_to validate :check_allowed_displayed_fields validate :check_allowed_sort_column @@ -84,7 +83,7 @@ class ProcedurePresentation < ApplicationRecord ] end - def sorted_ids(dossiers, count, instructeur) + def sorted_ids(dossiers, count) table, column, order = sort.values_at(TABLE, COLUMN, 'order') case table diff --git a/spec/models/procedure_presentation_spec.rb b/spec/models/procedure_presentation_spec.rb index dbd0c728c..eaa5395e3 100644 --- a/spec/models/procedure_presentation_spec.rb +++ b/spec/models/procedure_presentation_spec.rb @@ -133,7 +133,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, procedure.dossiers.count, instructeur) } + subject { procedure_presentation.sorted_ids(procedure.dossiers, procedure.dossiers.count) } context 'for notifications table' do let(:table) { 'notifications' } From 0daae815d8fbf2bef308475796f930f14dcc2260 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Thu, 31 Mar 2022 16:33:14 +0200 Subject: [PATCH 4/4] refactor(dossier): use by_statut --- .../instructeurs/procedures_controller.rb | 42 ++++------- app/models/dossier.rb | 24 +++++++ .../procedures_controller_spec.rb | 69 ++++++++----------- 3 files changed, 63 insertions(+), 72 deletions(-) diff --git a/app/controllers/instructeurs/procedures_controller.rb b/app/controllers/instructeurs/procedures_controller.rb index 7756c80a9..641bc076d 100644 --- a/app/controllers/instructeurs/procedures_controller.rb +++ b/app/controllers/instructeurs/procedures_controller.rb @@ -63,47 +63,29 @@ module Instructeurs @can_download_dossiers = (@tous_count + @archives_count) > 0 dossiers = Dossier.where(groupe_instructeur_id: groupe_instructeur_ids) - dossiers_visibles = dossiers.visible_by_administration - @a_suivre_dossiers = dossiers_visibles - .without_followers - .en_cours - - @followed_dossiers = current_instructeur + @followed_dossiers_id = current_instructeur .followed_dossiers .en_cours - .merge(dossiers_visibles) + .merge(dossiers.visible_by_administration) + .pluck(:id) - @followed_dossiers_id = @followed_dossiers.pluck(:id) - - @termines_dossiers = dossiers_visibles.termine - @all_state_dossiers = dossiers_visibles.all_state - @archived_dossiers = dossiers_visibles.archived - @expirant_dossiers = dossiers_visibles.termine_or_en_construction_close_to_expiration - @supprimes_recemment_dossiers = dossiers.hidden_by_administration.termine - - @dossiers = case statut + @dossiers = dossiers.by_statut(current_instructeur, statut) + dossiers_count = case statut when 'a-suivre' - dossiers_count = @a_suivre_count - @a_suivre_dossiers + @a_suivre_count when 'suivis' - dossiers_count = @suivis_count - @followed_dossiers + @suivis_count when 'traites' - dossiers_count = @traites_count - @termines_dossiers + @traites_count when 'tous' - dossiers_count = @tous_count - @all_state_dossiers + @tous_count when 'supprimes_recemment' - dossiers_count = @supprimes_recemment_count - @supprimes_recemment_dossiers + @supprimes_recemment_count when 'archives' - dossiers_count = @archives_count - @archived_dossiers + @archives_count when 'expirant' - dossiers_count = @expirant_count - @expirant_dossiers + @expirant_count end notifications = current_instructeur.notifications_for_groupe_instructeurs(groupe_instructeur_ids) diff --git a/app/models/dossier.rb b/app/models/dossier.rb index d27e058c7..4a3109c90 100644 --- a/app/models/dossier.rb +++ b/app/models/dossier.rb @@ -393,6 +393,30 @@ class Dossier < ApplicationRecord .distinct end + scope :by_statut, -> (instructeur, statut = 'tous') do + case statut + when 'a-suivre' + visible_by_administration + .without_followers + .en_cours + when 'suivis' + instructeur + .followed_dossiers + .en_cours + .merge(visible_by_administration) + when 'traites' + visible_by_administration.termine + when 'tous' + visible_by_administration.all_state + when 'supprimes_recemment' + hidden_by_administration.termine + when 'archives' + visible_by_administration.archived + when 'expirant' + visible_by_administration.termine_or_en_construction_close_to_expiration + end + end + accepts_nested_attributes_for :individual delegate :siret, :siren, to: :etablissement, allow_nil: true diff --git a/spec/controllers/instructeurs/procedures_controller_spec.rb b/spec/controllers/instructeurs/procedures_controller_spec.rb index fa1249f7b..937414ef6 100644 --- a/spec/controllers/instructeurs/procedures_controller_spec.rb +++ b/spec/controllers/instructeurs/procedures_controller_spec.rb @@ -274,38 +274,34 @@ describe Instructeurs::ProceduresController, type: :controller do let!(:new_unfollow_dossier) { create(:dossier, :en_instruction, procedure: procedure) } before { subject } - it { expect(assigns(:a_suivre_dossiers)).to match_array([new_unfollow_dossier]) } - it { expect(assigns(:followed_dossiers)).to be_empty } - it { expect(assigns(:termines_dossiers)).to be_empty } - it { expect(assigns(:all_state_dossiers)).to match_array([new_unfollow_dossier]) } - it { expect(assigns(:archived_dossiers)).to be_empty } + it { expect(assigns(:dossiers)).to match_array([new_unfollow_dossier]) } context 'with a dossier en contruction hidden by user' do - let!(:hidden_dossier) { create(:dossier, groupe_instructeur: gi_2, state: Dossier.states.fetch(:en_construction), hidden_by_user_at: 1.hour.ago) } + let!(:hidden_dossier) { create(:dossier, :en_construction, groupe_instructeur: gi_2, hidden_by_user_at: 1.hour.ago) } before { subject } - it { expect(assigns(:all_state_dossiers)).to match_array([new_unfollow_dossier]) } + it { expect(assigns(:dossiers)).to match_array([new_unfollow_dossier]) } end context 'with a dossier en contruction not hidden by user' do - let!(:en_construction_dossier) { create(:dossier, groupe_instructeur: gi_2, state: Dossier.states.fetch(:en_construction)) } + let!(:en_construction_dossier) { create(:dossier, :en_construction, groupe_instructeur: gi_2) } before { subject } - it { expect(assigns(:all_state_dossiers)).to match_array([new_unfollow_dossier, en_construction_dossier]) } + it { expect(assigns(:dossiers)).to match_array([new_unfollow_dossier, en_construction_dossier]) } end context 'and dossiers without follower on each of the others groups' do - let!(:new_unfollow_dossier_on_gi_2) { create(:dossier, groupe_instructeur: gi_2, state: Dossier.states.fetch(:en_instruction)) } - let!(:new_unfollow_dossier_on_gi_3) { create(:dossier, groupe_instructeur: gi_3, state: Dossier.states.fetch(:en_instruction)) } + let!(:new_unfollow_dossier_on_gi_2) { create(:dossier, :en_instruction, groupe_instructeur: gi_2) } + let!(:new_unfollow_dossier_on_gi_3) { create(:dossier, :en_instruction, groupe_instructeur: gi_3) } before { subject } - it { expect(assigns(:a_suivre_dossiers)).to match_array([new_unfollow_dossier, new_unfollow_dossier_on_gi_2]) } - it { expect(assigns(:all_state_dossiers)).to match_array([new_unfollow_dossier, new_unfollow_dossier_on_gi_2]) } + it { expect(assigns(:dossiers)).to match_array([new_unfollow_dossier, new_unfollow_dossier_on_gi_2]) } end end context 'with a new dossier with a follower' do + let(:statut) { 'suivis' } let!(:new_followed_dossier) { create(:dossier, :en_instruction, procedure: procedure) } before do @@ -313,15 +309,11 @@ describe Instructeurs::ProceduresController, type: :controller do subject end - it { expect(assigns(:a_suivre_dossiers)).to be_empty } - it { expect(assigns(:followed_dossiers)).to match_array([new_followed_dossier]) } - it { expect(assigns(:termines_dossiers)).to be_empty } - it { expect(assigns(:all_state_dossiers)).to match_array([new_followed_dossier]) } - it { expect(assigns(:archived_dossiers)).to be_empty } + it { expect(assigns(:dossiers)).to match_array([new_followed_dossier]) } context 'and dossier with a follower on each of the others groups' do - let!(:new_follow_dossier_on_gi_2) { create(:dossier, groupe_instructeur: gi_2, state: Dossier.states.fetch(:en_instruction)) } - let!(:new_follow_dossier_on_gi_3) { create(:dossier, groupe_instructeur: gi_3, state: Dossier.states.fetch(:en_instruction)) } + let!(:new_follow_dossier_on_gi_2) { create(:dossier, :en_instruction, groupe_instructeur: gi_2) } + let!(:new_follow_dossier_on_gi_3) { create(:dossier, :en_instruction, groupe_instructeur: gi_3) } before do instructeur.followed_dossiers << new_follow_dossier_on_gi_2 << new_follow_dossier_on_gi_3 @@ -329,44 +321,36 @@ describe Instructeurs::ProceduresController, type: :controller do end # followed dossiers on another groupe should not be displayed - it { expect(assigns(:followed_dossiers)).to contain_exactly(new_followed_dossier, new_follow_dossier_on_gi_2) } - it { expect(assigns(:all_state_dossiers)).to contain_exactly(new_followed_dossier, new_follow_dossier_on_gi_2) } + it { expect(assigns(:dossiers)).to contain_exactly(new_followed_dossier, new_follow_dossier_on_gi_2) } end end context 'with a termine dossier with a follower' do + let(:statut) { 'traites' } let!(:termine_dossier) { create(:dossier, :accepte, procedure: procedure) } before { subject } - it { expect(assigns(:a_suivre_dossiers)).to be_empty } - it { expect(assigns(:followed_dossiers)).to be_empty } - it { expect(assigns(:termines_dossiers)).to match_array([termine_dossier]) } - it { expect(assigns(:all_state_dossiers)).to match_array([termine_dossier]) } - it { expect(assigns(:archived_dossiers)).to be_empty } + it { expect(assigns(:dossiers)).to match_array([termine_dossier]) } context 'and terminer dossiers on each of the others groups' do - let!(:termine_dossier_on_gi_2) { create(:dossier, groupe_instructeur: gi_2, state: Dossier.states.fetch(:accepte)) } - let!(:termine_dossier_on_gi_3) { create(:dossier, groupe_instructeur: gi_3, state: Dossier.states.fetch(:accepte)) } + let!(:termine_dossier_on_gi_2) { create(:dossier, :accepte, groupe_instructeur: gi_2) } + let!(:termine_dossier_on_gi_3) { create(:dossier, :accepte, groupe_instructeur: gi_3) } before { subject } - it { expect(assigns(:termines_dossiers)).to match_array([termine_dossier, termine_dossier_on_gi_2]) } - it { expect(assigns(:all_state_dossiers)).to match_array([termine_dossier, termine_dossier_on_gi_2]) } + it { expect(assigns(:dossiers)).to match_array([termine_dossier, termine_dossier_on_gi_2]) } end end context 'with an archived dossier' do + let(:statut) { 'archives' } let!(:archived_dossier) { create(:dossier, :en_instruction, procedure: procedure, archived: true) } let!(:archived_dossier_deleted) { create(:dossier, :en_instruction, procedure: procedure, archived: true, hidden_by_administration_at: 2.days.ago) } before { subject } - it { expect(assigns(:a_suivre_dossiers)).to be_empty } - it { expect(assigns(:followed_dossiers)).to be_empty } - it { expect(assigns(:termines_dossiers)).to be_empty } - it { expect(assigns(:all_state_dossiers)).to be_empty } - it { expect(assigns(:archived_dossiers)).to match_array([archived_dossier]) } + it { expect(assigns(:dossiers)).to match_array([archived_dossier]) } context 'and terminer dossiers on each of the others groups' do let!(:archived_dossier_on_gi_2) { create(:dossier, :en_instruction, groupe_instructeur: gi_2, archived: true) } @@ -374,22 +358,23 @@ describe Instructeurs::ProceduresController, type: :controller do before { subject } - it { expect(assigns(:archived_dossiers)).to match_array([archived_dossier, archived_dossier_on_gi_2]) } + it { expect(assigns(:dossiers)).to match_array([archived_dossier, archived_dossier_on_gi_2]) } end end context 'with an expirants dossier' do + let(:statut) { 'expirant' } let!(:expiring_dossier_termine_deleted) { create(:dossier, :accepte, procedure: procedure, processed_at: 175.days.ago, hidden_by_administration_at: 2.days.ago) } let!(:expiring_dossier_termine) { create(:dossier, :accepte, procedure: procedure, processed_at: 175.days.ago) } let!(:expiring_dossier_en_construction) { create(:dossier, :en_construction, procedure: procedure, en_construction_at: 175.days.ago) } before { subject } - it { expect(assigns(:expirant_dossiers)).to match_array([expiring_dossier_termine, expiring_dossier_en_construction]) } + it { expect(assigns(:dossiers)).to match_array([expiring_dossier_termine, expiring_dossier_en_construction]) } end describe 'statut' do - let!(:a_suivre__dossier) { Timecop.freeze(1.day.ago) { create(:dossier, :en_instruction, procedure: procedure) } } + let!(:a_suivre_dossier) { Timecop.freeze(1.day.ago) { create(:dossier, :en_instruction, procedure: procedure) } } let!(:new_followed_dossier) { Timecop.freeze(2.days.ago) { create(:dossier, :en_instruction, procedure: procedure) } } let!(:termine_dossier) { Timecop.freeze(3.days.ago) { create(:dossier, :accepte, procedure: procedure) } } let!(:archived_dossier) { Timecop.freeze(4.days.ago) { create(:dossier, :en_instruction, procedure: procedure, archived: true) } } @@ -402,7 +387,7 @@ describe Instructeurs::ProceduresController, type: :controller do context 'when statut is empty' do let(:statut) { nil } - it { expect(assigns(:dossiers)).to match_array([a_suivre__dossier]) } + it { expect(assigns(:dossiers)).to match_array([a_suivre_dossier]) } it { expect(assigns(:statut)).to eq('a-suivre') } end @@ -410,7 +395,7 @@ describe Instructeurs::ProceduresController, type: :controller do let(:statut) { 'a-suivre' } it { expect(assigns(:statut)).to eq('a-suivre') } - it { expect(assigns(:dossiers)).to match_array([a_suivre__dossier]) } + it { expect(assigns(:dossiers)).to match_array([a_suivre_dossier]) } end context 'when statut is suivis' do @@ -431,7 +416,7 @@ describe Instructeurs::ProceduresController, type: :controller do let(:statut) { 'tous' } it { expect(assigns(:statut)).to eq('tous') } - it { expect(assigns(:dossiers)).to match_array([a_suivre__dossier, new_followed_dossier, termine_dossier]) } + it { expect(assigns(:dossiers)).to match_array([a_suivre_dossier, new_followed_dossier, termine_dossier]) } end context 'when statut is archives' do