From a63c3dbfc4e8ff52475e5616bb671df5dbd2e2dc Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Tue, 21 Jan 2020 16:49:55 +0100 Subject: [PATCH 1/7] app: improve wording of the strike banner Make it clearer that the strike affects only the DS staff. --- app/views/layouts/_strike_banner.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/layouts/_strike_banner.html.haml b/app/views/layouts/_strike_banner.html.haml index a585bb51e..c9587ebad 100644 --- a/app/views/layouts/_strike_banner.html.haml +++ b/app/views/layouts/_strike_banner.html.haml @@ -3,6 +3,6 @@ .site-banner-icon ⚠️ .site-banner-text %strong - En raison d’une grève nationale interprofessionnelle, une partie des personnels ne travaille pas. + En raison d’une grève nationale interprofessionnelle, une partie du personnel de demarches-simplifiees.fr ne travaille pas. %br Les délais de réponse aux questions techniques pourront être perturbés pendant les prochains jours. From 6a47458112f9af76f3a6f2415642d9e3f6bb0119 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Tue, 21 Jan 2020 17:24:42 +0100 Subject: [PATCH 2/7] specs: use order-insentitive matching for arrays MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The specs sometime failes locally, because the order of the returned objects is undefined. This is an acceptable behavior – but it means we should test for collection membership, rather than for a specific array order. Replace `match` by `match_array` to have unordered matching. --- .../procedures_controller_spec.rb | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/spec/controllers/instructeurs/procedures_controller_spec.rb b/spec/controllers/instructeurs/procedures_controller_spec.rb index 0fdb90e5e..a8168d821 100644 --- a/spec/controllers/instructeurs/procedures_controller_spec.rb +++ b/spec/controllers/instructeurs/procedures_controller_spec.rb @@ -256,10 +256,10 @@ describe Instructeurs::ProceduresController, type: :controller do get :show, params: { procedure_id: procedure.id } end - it { expect(assigns(:a_suivre_dossiers)).to match([new_unfollow_dossier]) } + 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([new_unfollow_dossier]) } + it { expect(assigns(:all_state_dossiers)).to match_array([new_unfollow_dossier]) } it { expect(assigns(:archived_dossiers)).to be_empty } context 'and dossiers without follower on each of the others groups' do @@ -270,8 +270,8 @@ describe Instructeurs::ProceduresController, type: :controller do get :show, params: { procedure_id: procedure.id } end - it { expect(assigns(:a_suivre_dossiers)).to match([new_unfollow_dossier, new_unfollow_dossier_on_gi_2]) } - it { expect(assigns(:all_state_dossiers)).to match([new_unfollow_dossier, new_unfollow_dossier_on_gi_2]) } + 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]) } end end @@ -284,9 +284,9 @@ describe Instructeurs::ProceduresController, type: :controller do end it { expect(assigns(:a_suivre_dossiers)).to be_empty } - it { expect(assigns(:followed_dossiers)).to match([new_followed_dossier]) } + 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([new_followed_dossier]) } + it { expect(assigns(:all_state_dossiers)).to match_array([new_followed_dossier]) } it { expect(assigns(:archived_dossiers)).to be_empty } context 'and dossier with a follower on each of the others groups' do @@ -313,8 +313,8 @@ describe Instructeurs::ProceduresController, type: :controller do it { expect(assigns(:a_suivre_dossiers)).to be_empty } it { expect(assigns(:followed_dossiers)).to be_empty } - it { expect(assigns(:termines_dossiers)).to match([termine_dossier]) } - it { expect(assigns(:all_state_dossiers)).to match([termine_dossier]) } + 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 } context 'and terminer dossiers on each of the others groups' do @@ -325,8 +325,8 @@ describe Instructeurs::ProceduresController, type: :controller do get :show, params: { procedure_id: procedure.id } end - it { expect(assigns(:termines_dossiers)).to match([termine_dossier, termine_dossier_on_gi_2]) } - it { expect(assigns(:all_state_dossiers)).to match([termine_dossier, termine_dossier_on_gi_2]) } + 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]) } end end @@ -341,7 +341,7 @@ describe Instructeurs::ProceduresController, type: :controller do 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([archived_dossier]) } + it { expect(assigns(:archived_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, groupe_instructeur: gi_2, state: Dossier.states.fetch(:en_instruction), archived: true) } @@ -351,7 +351,7 @@ describe Instructeurs::ProceduresController, type: :controller do get :show, params: { procedure_id: procedure.id } end - it { expect(assigns(:archived_dossiers)).to match([archived_dossier, archived_dossier_on_gi_2]) } + it { expect(assigns(:archived_dossiers)).to match_array([archived_dossier, archived_dossier_on_gi_2]) } end end @@ -369,7 +369,7 @@ describe Instructeurs::ProceduresController, type: :controller do context 'when statut is empty' do let(:statut) { nil } - it { expect(assigns(:dossiers)).to match([a_suivre__dossier]) } + it { expect(assigns(:dossiers)).to match_array([a_suivre__dossier]) } it { expect(assigns(:statut)).to eq('a-suivre') } end @@ -377,21 +377,21 @@ describe Instructeurs::ProceduresController, type: :controller do let(:statut) { 'a-suivre' } it { expect(assigns(:statut)).to eq('a-suivre') } - it { expect(assigns(:dossiers)).to match([a_suivre__dossier]) } + it { expect(assigns(:dossiers)).to match_array([a_suivre__dossier]) } end context 'when statut is suivis' do let(:statut) { 'suivis' } it { expect(assigns(:statut)).to eq('suivis') } - it { expect(assigns(:dossiers)).to match([new_followed_dossier]) } + it { expect(assigns(:dossiers)).to match_array([new_followed_dossier]) } end context 'when statut is traites' do let(:statut) { 'traites' } it { expect(assigns(:statut)).to eq('traites') } - it { expect(assigns(:dossiers)).to match([termine_dossier]) } + it { expect(assigns(:dossiers)).to match_array([termine_dossier]) } end context 'when statut is tous' do @@ -405,7 +405,7 @@ describe Instructeurs::ProceduresController, type: :controller do let(:statut) { 'archives' } it { expect(assigns(:statut)).to eq('archives') } - it { expect(assigns(:dossiers)).to match([archived_dossier]) } + it { expect(assigns(:dossiers)).to match_array([archived_dossier]) } end end end From 00e5b736fb4e9c3d2bf0e7952b759864829bd3f1 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Wed, 8 Jan 2020 09:45:05 +0000 Subject: [PATCH 3/7] factories: mark the parent procedure as `for_individual` This ensures that we don't get an inconsistency, where a dossier is `for_individual` while its procedure is not `for_individual`. --- spec/factories/dossier.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/spec/factories/dossier.rb b/spec/factories/dossier.rb index 73224330c..7847029f9 100644 --- a/spec/factories/dossier.rb +++ b/spec/factories/dossier.rb @@ -37,9 +37,13 @@ FactoryBot.define do end trait :for_individual do - after(:build) do |dossier, _evaluator| + after(:build) do |dossier, evaluator| + # If the procedure was implicitely created by the factory, + # mark it automatically as for_individual. + if evaluator.procedure.nil? + dossier.procedure.update(for_individual: true) + end dossier.individual = create(:individual) - dossier.save end end From 724ff5036358d5f9b2d269906f01afb18e7a7299 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Tue, 7 Jan 2020 17:47:22 +0100 Subject: [PATCH 4/7] factories: rename dossier `for_individual` to `with_individual` This clarifies the distinction between ``` create :procedure, :for_individual ``` and ``` create :dossier, :with_individual ``` --- .../api/v2/graphql_controller_spec.rb | 24 +++++++++---------- .../users/dossiers_controller_spec.rb | 2 +- spec/factories/dossier.rb | 2 +- spec/features/users/dossier_details_spec.rb | 6 ++--- spec/features/users/invite_spec.rb | 4 ++-- spec/mailers/notification_mailer_spec.rb | 2 +- spec/models/dossier_spec.rb | 2 +- .../services/procedure_export_service_spec.rb | 10 ++++---- spec/views/commencer/show.html.haml_spec.rb | 4 ++-- 9 files changed, 28 insertions(+), 28 deletions(-) diff --git a/spec/controllers/api/v2/graphql_controller_spec.rb b/spec/controllers/api/v2/graphql_controller_spec.rb index d92e06160..1177464eb 100644 --- a/spec/controllers/api/v2/graphql_controller_spec.rb +++ b/spec/controllers/api/v2/graphql_controller_spec.rb @@ -8,14 +8,14 @@ describe API::V2::GraphqlController do dossier = create(:dossier, :en_construction, :with_all_champs, - :for_individual, + :with_individual, procedure: procedure) create(:commentaire, :with_file, dossier: dossier, email: 'test@test.com') dossier end - let(:dossier1) { create(:dossier, :en_construction, :for_individual, procedure: procedure, en_construction_at: 1.day.ago) } - let(:dossier2) { create(:dossier, :en_construction, :for_individual, procedure: procedure, en_construction_at: 3.days.ago) } - let(:dossier_brouillon) { create(:dossier, :for_individual, procedure: procedure) } + let(:dossier1) { create(:dossier, :en_construction, :with_individual, procedure: procedure, en_construction_at: 1.day.ago) } + let(:dossier2) { create(:dossier, :en_construction, :with_individual, procedure: procedure, en_construction_at: 3.days.ago) } + let(:dossier_brouillon) { create(:dossier, :with_individual, procedure: procedure) } let(:dossiers) { [dossier2, dossier1, dossier] } let(:instructeur) { create(:instructeur, followed_dossiers: dossiers) } @@ -416,7 +416,7 @@ describe API::V2::GraphqlController do end describe 'dossierPasserEnInstruction' do - let(:dossier) { create(:dossier, :en_construction, :for_individual, procedure: procedure) } + let(:dossier) { create(:dossier, :en_construction, :with_individual, procedure: procedure) } let(:query) do "mutation { dossierPasserEnInstruction(input: { @@ -451,7 +451,7 @@ describe API::V2::GraphqlController do end context 'validation error' do - let(:dossier) { create(:dossier, :en_instruction, :for_individual, procedure: procedure) } + let(:dossier) { create(:dossier, :en_instruction, :with_individual, procedure: procedure) } it "should fail" do expect(gql_errors).to eq(nil) @@ -464,7 +464,7 @@ describe API::V2::GraphqlController do end describe 'dossierClasserSansSuite' do - let(:dossier) { create(:dossier, :en_instruction, :for_individual, procedure: procedure) } + let(:dossier) { create(:dossier, :en_instruction, :with_individual, procedure: procedure) } let(:query) do "mutation { dossierClasserSansSuite(input: { @@ -501,7 +501,7 @@ describe API::V2::GraphqlController do end context 'validation error' do - let(:dossier) { create(:dossier, :accepte, :for_individual, procedure: procedure) } + let(:dossier) { create(:dossier, :accepte, :with_individual, procedure: procedure) } it "should fail" do expect(gql_errors).to eq(nil) @@ -514,7 +514,7 @@ describe API::V2::GraphqlController do end describe 'dossierRefuser' do - let(:dossier) { create(:dossier, :en_instruction, :for_individual, procedure: procedure) } + let(:dossier) { create(:dossier, :en_instruction, :with_individual, procedure: procedure) } let(:query) do "mutation { dossierRefuser(input: { @@ -551,7 +551,7 @@ describe API::V2::GraphqlController do end context 'validation error' do - let(:dossier) { create(:dossier, :sans_suite, :for_individual, procedure: procedure) } + let(:dossier) { create(:dossier, :sans_suite, :with_individual, procedure: procedure) } it "should fail" do expect(gql_errors).to eq(nil) @@ -564,7 +564,7 @@ describe API::V2::GraphqlController do end describe 'dossierAccepter' do - let(:dossier) { create(:dossier, :en_instruction, :for_individual, procedure: procedure) } + let(:dossier) { create(:dossier, :en_instruction, :with_individual, procedure: procedure) } let(:query) do "mutation { dossierAccepter(input: { @@ -634,7 +634,7 @@ describe API::V2::GraphqlController do end context 'validation error' do - let(:dossier) { create(:dossier, :refuse, :for_individual, procedure: procedure) } + let(:dossier) { create(:dossier, :refuse, :with_individual, procedure: procedure) } it "should fail" do expect(gql_errors).to eq(nil) diff --git a/spec/controllers/users/dossiers_controller_spec.rb b/spec/controllers/users/dossiers_controller_spec.rb index 3671ae490..cdd9f3d90 100644 --- a/spec/controllers/users/dossiers_controller_spec.rb +++ b/spec/controllers/users/dossiers_controller_spec.rb @@ -170,7 +170,7 @@ describe Users::DossiersController, type: :controller do end context 'when the identite cannot be updated by the user' do - let(:dossier) { create(:dossier, :for_individual, :en_instruction, user: user, procedure: procedure) } + let(:dossier) { create(:dossier, :with_individual, :en_instruction, user: user, procedure: procedure) } let(:individual_params) { { gender: 'M', nom: 'Mouse', prenom: 'Mickey' } } it 'redirects to the dossiers list' do diff --git a/spec/factories/dossier.rb b/spec/factories/dossier.rb index 7847029f9..5369a4b8b 100644 --- a/spec/factories/dossier.rb +++ b/spec/factories/dossier.rb @@ -36,7 +36,7 @@ FactoryBot.define do end end - trait :for_individual do + trait :with_individual do after(:build) do |dossier, evaluator| # If the procedure was implicitely created by the factory, # mark it automatically as for_individual. diff --git a/spec/features/users/dossier_details_spec.rb b/spec/features/users/dossier_details_spec.rb index 99332413a..2f37ce02c 100644 --- a/spec/features/users/dossier_details_spec.rb +++ b/spec/features/users/dossier_details_spec.rb @@ -3,7 +3,7 @@ require 'features/users/dossier_shared_examples.rb' describe 'Dossier details:' do let(:user) { create(:user) } let(:procedure) { create(:simple_procedure) } - let(:dossier) { create(:dossier, :en_construction, :for_individual, :with_commentaires, user: user, procedure: procedure) } + let(:dossier) { create(:dossier, :en_construction, :with_individual, :with_commentaires, user: user, procedure: procedure) } before do visit_dossier dossier @@ -17,7 +17,7 @@ describe 'Dossier details:' do end describe "the user can see the mean time they are expected to wait" do - let(:other_dossier) { create(:dossier, :accepte, :for_individual, procedure: procedure, en_construction_at: 10.days.ago, en_instruction_at: 9.days.ago, processed_at: Time.zone.now) } + let(:other_dossier) { create(:dossier, :accepte, :with_individual, procedure: procedure, en_construction_at: 10.days.ago, en_instruction_at: 9.days.ago, processed_at: Time.zone.now) } context "when the dossier is in construction" do it "displays the estimated wait duration" do @@ -28,7 +28,7 @@ describe 'Dossier details:' do end context "when the dossier is in instruction" do - let(:dossier) { create(:dossier, :en_instruction, :for_individual, :with_commentaires, user: user, procedure: procedure) } + let(:dossier) { create(:dossier, :en_instruction, :with_individual, :with_commentaires, user: user, procedure: procedure) } it "displays the estimated wait duration" do other_dossier diff --git a/spec/features/users/invite_spec.rb b/spec/features/users/invite_spec.rb index 2a7d4a26f..e1d9cebd0 100644 --- a/spec/features/users/invite_spec.rb +++ b/spec/features/users/invite_spec.rb @@ -8,7 +8,7 @@ feature 'Invitations' do let(:invite) { create(:invite, user: invited_user, dossier: dossier) } context 'when the dossier is a brouillon' do - let!(:dossier) { create(:dossier, :for_individual, state: Dossier.states.fetch(:brouillon), user: owner, procedure: procedure) } + let!(:dossier) { create(:dossier, :with_individual, state: Dossier.states.fetch(:brouillon), user: owner, procedure: procedure) } scenario 'on the form, the owner of a dossier can invite another user to collaborate on the dossier', js: true do log_in(owner) @@ -69,7 +69,7 @@ feature 'Invitations' do end context 'when the dossier is en_construction' do - let!(:dossier) { create(:dossier, :for_individual, :en_construction, user: owner, procedure: procedure) } + let!(:dossier) { create(:dossier, :with_individual, :en_construction, user: owner, procedure: procedure) } scenario 'on dossier details, the owner of a dossier can invite another user to collaborate on the dossier', js: true do log_in(owner) diff --git a/spec/mailers/notification_mailer_spec.rb b/spec/mailers/notification_mailer_spec.rb index d5e0ee23a..4301b27b0 100644 --- a/spec/mailers/notification_mailer_spec.rb +++ b/spec/mailers/notification_mailer_spec.rb @@ -3,7 +3,7 @@ require "spec_helper" RSpec.describe NotificationMailer, type: :mailer do let(:user) { create(:user) } let(:procedure) { create(:simple_procedure) } - let(:dossier) { create(:dossier, :en_construction, :for_individual, :with_service, user: user, procedure: procedure) } + let(:dossier) { create(:dossier, :en_construction, :with_individual, :with_service, user: user, procedure: procedure) } describe '.send_dossier_received' do let(:email_template) { create(:received_mail, subject: 'Email subject', body: 'Your dossier was processed. Thanks.') } diff --git a/spec/models/dossier_spec.rb b/spec/models/dossier_spec.rb index d1ae1d8ca..0f6c67409 100644 --- a/spec/models/dossier_spec.rb +++ b/spec/models/dossier_spec.rb @@ -589,7 +589,7 @@ describe Dossier do end context 'when there is an individual' do - let(:dossier) { create(:dossier, :for_individual, procedure: procedure) } + let(:dossier) { create(:dossier, :with_individual, procedure: procedure) } it { is_expected.to eq("#{dossier.individual.nom} #{dossier.individual.prenom}") } end diff --git a/spec/services/procedure_export_service_spec.rb b/spec/services/procedure_export_service_spec.rb index eb58aed07..7d24508c8 100644 --- a/spec/services/procedure_export_service_spec.rb +++ b/spec/services/procedure_export_service_spec.rb @@ -33,7 +33,7 @@ describe ProcedureExportService do end context 'with dossier' do - let!(:dossier) { create(:dossier, :en_instruction, :with_all_champs, :for_individual, procedure: procedure) } + let!(:dossier) { create(:dossier, :en_instruction, :with_all_champs, :with_individual, procedure: procedure) } let(:nominal_headers) do [ @@ -286,7 +286,7 @@ describe ProcedureExportService do end context 'with avis' do - let!(:dossier) { create(:dossier, :en_instruction, :with_all_champs, :for_individual, procedure: procedure) } + let!(:dossier) { create(:dossier, :en_instruction, :with_all_champs, :with_individual, procedure: procedure) } let!(:avis) { create(:avis, :with_answer, dossier: dossier) } it 'should have headers' do @@ -307,8 +307,8 @@ describe ProcedureExportService do context 'with repetitions' do let!(:dossiers) do [ - create(:dossier, :en_instruction, :with_all_champs, :for_individual, procedure: procedure), - create(:dossier, :en_instruction, :with_all_champs, :for_individual, procedure: procedure) + create(:dossier, :en_instruction, :with_all_champs, :with_individual, procedure: procedure), + create(:dossier, :en_instruction, :with_all_champs, :with_individual, procedure: procedure) ] end let(:champ_repetition) { dossiers.first.champs.find { |champ| champ.type_champ == 'repetition' } } @@ -341,7 +341,7 @@ describe ProcedureExportService do end context 'with non unique labels' do - let(:dossier) { create(:dossier, :en_instruction, :with_all_champs, :for_individual, procedure: procedure) } + let(:dossier) { create(:dossier, :en_instruction, :with_all_champs, :with_individual, procedure: procedure) } let(:champ_repetition) { dossier.champs.find { |champ| champ.type_champ == 'repetition' } } let(:type_de_champ_repetition) { create(:type_de_champ_repetition, procedure: procedure, libelle: champ_repetition.libelle) } let!(:another_champ_repetition) { create(:champ_repetition, type_de_champ: type_de_champ_repetition, dossier: dossier) } diff --git a/spec/views/commencer/show.html.haml_spec.rb b/spec/views/commencer/show.html.haml_spec.rb index c8119d7d9..22fc05505 100644 --- a/spec/views/commencer/show.html.haml_spec.rb +++ b/spec/views/commencer/show.html.haml_spec.rb @@ -53,7 +53,7 @@ RSpec.describe 'commencer/show.html.haml', type: :view do context 'and they have a submitted dossier' do let!(:brouillon) { create(:dossier, user: user, procedure: procedure) } - let!(:dossier) { create(:dossier, :en_construction, :for_individual, user: user, procedure: procedure) } + let!(:dossier) { create(:dossier, :en_construction, :with_individual, user: user, procedure: procedure) } it_behaves_like 'it renders a link to create a new dossier', 'Commencer un nouveau dossier' @@ -66,7 +66,7 @@ RSpec.describe 'commencer/show.html.haml', type: :view do context 'and they have several submitted dossiers' do let!(:brouillon) { create(:dossier, user: user, procedure: procedure) } - let!(:dossiers) { create_list(:dossier, 2, :en_construction, :for_individual, user: user, procedure: procedure) } + let!(:dossiers) { create_list(:dossier, 2, :en_construction, :with_individual, user: user, procedure: procedure) } it_behaves_like 'it renders a link to create a new dossier', 'Commencer un nouveau dossier' From 0efb62f03a30ec28c9f47f00f3dbee86548ff616 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Tue, 7 Jan 2020 17:45:39 +0100 Subject: [PATCH 5/7] factories: ensure that dossiers `with_individual` are consistent --- spec/factories/dossier.rb | 3 +++ spec/models/dossier_spec.rb | 3 ++- spec/views/commencer/show.html.haml_spec.rb | 2 +- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/spec/factories/dossier.rb b/spec/factories/dossier.rb index 5369a4b8b..504405f08 100644 --- a/spec/factories/dossier.rb +++ b/spec/factories/dossier.rb @@ -43,6 +43,9 @@ FactoryBot.define do if evaluator.procedure.nil? dossier.procedure.update(for_individual: true) end + if !dossier.procedure.for_individual? + raise 'Inconsistent factory: attempting to create a dossier :with_individual on a procedure that is not `for_individual?`' + end dossier.individual = create(:individual) end end diff --git a/spec/models/dossier_spec.rb b/spec/models/dossier_spec.rb index 0f6c67409..85376e90e 100644 --- a/spec/models/dossier_spec.rb +++ b/spec/models/dossier_spec.rb @@ -573,7 +573,7 @@ describe Dossier do end describe '#owner_name' do - let!(:procedure) { create(:procedure) } + let(:procedure) { create(:procedure) } subject { dossier.owner_name } context 'when there is no entreprise or individual' do @@ -589,6 +589,7 @@ describe Dossier do end context 'when there is an individual' do + let(:procedure) { create(:procedure, :for_individual) } let(:dossier) { create(:dossier, :with_individual, procedure: procedure) } it { is_expected.to eq("#{dossier.individual.nom} #{dossier.individual.prenom}") } diff --git a/spec/views/commencer/show.html.haml_spec.rb b/spec/views/commencer/show.html.haml_spec.rb index 22fc05505..3d758d664 100644 --- a/spec/views/commencer/show.html.haml_spec.rb +++ b/spec/views/commencer/show.html.haml_spec.rb @@ -3,7 +3,7 @@ require 'rails_helper' RSpec.describe 'commencer/show.html.haml', type: :view do include Rails.application.routes.url_helpers - let(:procedure) { create(:procedure, :with_service, :published) } + let(:procedure) { create(:procedure, :published, :for_individual, :with_service) } before do assign(:procedure, procedure) From 792ba73643de37f58404464b21ab4a16d1ef1ea1 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Tue, 7 Jan 2020 17:50:49 +0100 Subject: [PATCH 6/7] factories: ensure that dossiers `with_entreprise` are consistent --- spec/controllers/api/v2/graphql_controller_spec.rb | 4 ++-- spec/factories/dossier.rb | 3 +++ spec/views/users/dossiers/brouillon.html.haml_spec.rb | 4 ++-- spec/views/users/dossiers/identite.html.haml_spec.rb | 4 ++-- 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/spec/controllers/api/v2/graphql_controller_spec.rb b/spec/controllers/api/v2/graphql_controller_spec.rb index 1177464eb..6c7d7e7f8 100644 --- a/spec/controllers/api/v2/graphql_controller_spec.rb +++ b/spec/controllers/api/v2/graphql_controller_spec.rb @@ -288,8 +288,8 @@ describe API::V2::GraphqlController do end context "with entreprise" do - let(:procedure) { create(:procedure, :published, administrateurs: [admin]) } - let(:dossier) { create(:dossier, :en_construction, :with_entreprise, procedure: procedure) } + let(:procedure_for_entreprise) { create(:procedure, :published, administrateurs: [admin]) } + let(:dossier) { create(:dossier, :en_construction, :with_entreprise, procedure: procedure_for_entreprise) } let(:query) do "{ diff --git a/spec/factories/dossier.rb b/spec/factories/dossier.rb index 504405f08..c0b563521 100644 --- a/spec/factories/dossier.rb +++ b/spec/factories/dossier.rb @@ -25,6 +25,9 @@ FactoryBot.define do trait :with_entreprise do after(:build) do |dossier, _evaluator| + if dossier.procedure.for_individual? + raise 'Inconsistent factory: attempting to create a dossier :with_entreprise on a procedure that is `for_individual?`' + end etablissement = create(:etablissement) dossier.etablissement = etablissement end diff --git a/spec/views/users/dossiers/brouillon.html.haml_spec.rb b/spec/views/users/dossiers/brouillon.html.haml_spec.rb index dc13af78a..1d66d554f 100644 --- a/spec/views/users/dossiers/brouillon.html.haml_spec.rb +++ b/spec/views/users/dossiers/brouillon.html.haml_spec.rb @@ -1,8 +1,8 @@ require 'spec_helper' describe 'users/dossiers/brouillon.html.haml', type: :view do - let(:procedure) { create(:procedure, :with_type_de_champ, :with_notice, :with_service, for_individual: true) } - let(:dossier) { create(:dossier, :with_entreprise, state: Dossier.states.fetch(:brouillon), procedure: procedure) } + let(:procedure) { create(:procedure, :with_type_de_champ, :with_notice, :with_service) } + let(:dossier) { create(:dossier, state: Dossier.states.fetch(:brouillon), procedure: procedure) } let(:footer) { view.content_for(:footer) } before do diff --git a/spec/views/users/dossiers/identite.html.haml_spec.rb b/spec/views/users/dossiers/identite.html.haml_spec.rb index b5d954c67..2f01ef41c 100644 --- a/spec/views/users/dossiers/identite.html.haml_spec.rb +++ b/spec/views/users/dossiers/identite.html.haml_spec.rb @@ -1,8 +1,8 @@ require 'spec_helper' describe 'users/dossiers/identite.html.haml', type: :view do - let(:procedure) { create(:simple_procedure, for_individual: true) } - let(:dossier) { create(:dossier, :with_entreprise, :with_service, state: Dossier.states.fetch(:brouillon), procedure: procedure) } + let(:procedure) { create(:simple_procedure, :for_individual) } + let(:dossier) { create(:dossier, :with_service, state: Dossier.states.fetch(:brouillon), procedure: procedure) } before do sign_in dossier.user From 751f24f7bb79c7d9d0af4ef1b7afe7aeabb4cdbc Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Tue, 21 Jan 2020 18:57:54 +0100 Subject: [PATCH 7/7] Revert "4127 fix superadmin supprime compte usager" --- .../manager/administrateurs_controller.rb | 6 ++- .../manager/instructeurs_controller.rb | 14 ------ app/controllers/manager/users_controller.rb | 2 +- app/models/administrateur.rb | 13 ------ app/models/instructeur.rb | 8 +--- app/models/user.rb | 11 ++--- app/views/manager/instructeurs/show.html.erb | 3 -- app/views/manager/users/show.html.erb | 2 +- config/routes.rb | 1 - .../manager/instructeurs_controller_spec.rb | 17 ------- spec/models/administrateur_spec.rb | 15 ------- spec/models/instructeur_spec.rb | 35 +-------------- spec/models/user_spec.rb | 45 +++---------------- 13 files changed, 22 insertions(+), 150 deletions(-) delete mode 100644 spec/controllers/manager/instructeurs_controller_spec.rb diff --git a/app/controllers/manager/administrateurs_controller.rb b/app/controllers/manager/administrateurs_controller.rb index 10dec19d5..f976f3cf6 100644 --- a/app/controllers/manager/administrateurs_controller.rb +++ b/app/controllers/manager/administrateurs_controller.rb @@ -22,7 +22,11 @@ module Manager def delete administrateur = Administrateur.find(params[:id]) - administrateur.delete_and_transfer_services + if !administrateur.can_be_deleted? + fail "Impossible de supprimer cet administrateur car il a des dossiers ou des procédures" + end + administrateur.dossiers.each(&:delete_and_keep_track) + administrateur.destroy logger.info("L'administrateur #{administrateur.id} est supprimé par #{current_administration.id}") flash[:notice] = "L'administrateur #{administrateur.id} est supprimé" diff --git a/app/controllers/manager/instructeurs_controller.rb b/app/controllers/manager/instructeurs_controller.rb index ed2107c45..82b1e11eb 100644 --- a/app/controllers/manager/instructeurs_controller.rb +++ b/app/controllers/manager/instructeurs_controller.rb @@ -6,19 +6,5 @@ module Manager flash[:notice] = "Instructeur réinvité." redirect_to manager_instructeur_path(instructeur) end - - def delete - instructeur = Instructeur.find(params[:id]) - - if !instructeur.can_be_deleted? - fail "Impossible de supprimer cet instructeur car il est administrateur ou il est le seul instructeur sur une démarche" - end - instructeur.destroy! - - logger.info("L'instructeur #{instructeur.id} est supprimé par #{current_administration.id}") - flash[:notice] = "L'instructeur #{instructeur.id} est supprimé" - - redirect_to manager_instructeurs_path - end end end diff --git a/app/controllers/manager/users_controller.rb b/app/controllers/manager/users_controller.rb index eb2c9e169..9e1a9fc54 100644 --- a/app/controllers/manager/users_controller.rb +++ b/app/controllers/manager/users_controller.rb @@ -24,7 +24,7 @@ module Manager def delete user = User.find(params[:id]) if !user.can_be_deleted? - fail "Impossible de supprimer cet utilisateur. Il a des dossiers en instruction ou il est administrateur." + fail "Impossible de supprimer cet utilisateur car il a des dossiers en instruction" end user.delete_and_keep_track_dossiers(current_administration) diff --git a/app/models/administrateur.rb b/app/models/administrateur.rb index 1174d4b60..4d4b84d4a 100644 --- a/app/models/administrateur.rb +++ b/app/models/administrateur.rb @@ -70,17 +70,4 @@ class Administrateur < ApplicationRecord def can_be_deleted? dossiers.state_instruction_commencee.none? && procedures.all? { |p| p.administrateurs.count > 1 } end - - def delete_and_transfer_services - if !can_be_deleted? - fail "Impossible de supprimer cet administrateur car il a des dossiers ou des procédures" - end - dossiers.each(&:delete_and_keep_track) - - procedures.each do |procedure| - next_administrateur = procedure.administrateurs.where.not(id: self.id).first - procedure.service.update(administrateur: next_administrateur) - end - destroy - end end diff --git a/app/models/instructeur.rb b/app/models/instructeur.rb index 97b2b0a49..4c142b870 100644 --- a/app/models/instructeur.rb +++ b/app/models/instructeur.rb @@ -17,9 +17,9 @@ class Instructeur < ApplicationRecord has_many :previously_followed_dossiers, -> { distinct }, through: :previous_follows, source: :dossier has_many :avis has_many :dossiers_from_avis, through: :avis, source: :dossier - has_many :trusted_device_tokens, dependent: :destroy + has_many :trusted_device_tokens - has_one :user, dependent: :nullify + has_one :user default_scope { eager_load(:user) } @@ -176,10 +176,6 @@ class Instructeur < ApplicationRecord trusted_device_token&.token_young? end - def can_be_deleted? - user.administrateur.nil? && procedures.all? { |p| p.defaut_groupe_instructeur.instructeurs.count > 1 } - end - private def annotations_hash(demande, annotations_privees, avis, messagerie) diff --git a/app/models/user.rb b/app/models/user.rb index 35c0edd5e..4838da20d 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -97,7 +97,7 @@ class User < ApplicationRecord end def can_be_deleted? - administrateur.nil? && instructeur.nil? && dossiers.state_instruction_commencee.empty? + dossiers.state_instruction_commencee.empty? end def delete_and_keep_track_dossiers(administration) @@ -105,11 +105,12 @@ class User < ApplicationRecord raise "Cannot delete this user because instruction has started for some dossiers" end - dossiers.each do |dossier| - dossier.delete_and_keep_track(administration) + if can_be_deleted? + dossiers.each do |dossier| + dossier.delete_and_keep_track(administration) + end + destroy end - dossiers.unscoped.destroy_all - destroy! end private diff --git a/app/views/manager/instructeurs/show.html.erb b/app/views/manager/instructeurs/show.html.erb index 983e171e9..e423414a9 100644 --- a/app/views/manager/instructeurs/show.html.erb +++ b/app/views/manager/instructeurs/show.html.erb @@ -34,10 +34,7 @@ as well as a link to its edit page.
<%= link_to 'Réinviter', reinvite_manager_instructeur_path(instructeur), method: :post, class: 'button' %> -
- <%= button_to "Supprimer", delete_manager_instructeur_path(page.resource), method: :delete, disabled: !page.resource.can_be_deleted?, class: "button", data: { confirm: "Confirmez-vous la suppression de l'instructeur ?" }, title: page.resource.can_be_deleted? ? "Supprimer" : "Cet instructeur est administrateur ou a des démarches dont il est le seul instructeur et ne peut être supprimé" %> -
diff --git a/app/views/manager/users/show.html.erb b/app/views/manager/users/show.html.erb index 833e79723..ab51789a6 100644 --- a/app/views/manager/users/show.html.erb +++ b/app/views/manager/users/show.html.erb @@ -25,7 +25,7 @@ as well as a link to its edit page.
- <%= button_to "supprimer", delete_manager_user_path(page.resource), method: :delete, disabled: !page.resource.can_be_deleted?, class: "button", data: { confirm: "Confirmez-vous la suppression de l'utilisateur ?" }, title: page.resource.can_be_deleted? ? "Supprimer" : "Cet utilisateur ne peut être supprimé. Il a des dossiers dont l'instruction a commencé ou il est administrateur ou instructeur" %> + <%= button_to "supprimer", delete_manager_user_path(page.resource), method: :delete, disabled: !page.resource.can_be_deleted?, class: "button", data: { confirm: "Confirmez-vous la suppression de l'utilisateur ?" }, title: page.resource.can_be_deleted? ? "Supprimer" : "Cet utilisateur a des dossiers dont l'instruction a commencé et ne peut être supprimé" %>
diff --git a/config/routes.rb b/config/routes.rb index e69a44a60..09f0d8fb8 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -32,7 +32,6 @@ Rails.application.routes.draw do resources :instructeurs, only: [:index, :show] do post 'reinvite', on: :member - delete 'delete', on: :member end resources :dossiers, only: [:show] diff --git a/spec/controllers/manager/instructeurs_controller_spec.rb b/spec/controllers/manager/instructeurs_controller_spec.rb deleted file mode 100644 index 3a0164d4b..000000000 --- a/spec/controllers/manager/instructeurs_controller_spec.rb +++ /dev/null @@ -1,17 +0,0 @@ -describe Manager::InstructeursController, type: :controller do - let(:administration) { create(:administration) } - - describe '#delete' do - let!(:instructeur) { create(:instructeur) } - - before { sign_in administration } - - subject { delete :delete, params: { id: instructeur.id } } - - it 'deletes the instructeur' do - subject - - expect(Instructeur.find_by(id: instructeur.id)).to be_nil - end - end -end diff --git a/spec/models/administrateur_spec.rb b/spec/models/administrateur_spec.rb index 3e6864466..9f0c30923 100644 --- a/spec/models/administrateur_spec.rb +++ b/spec/models/administrateur_spec.rb @@ -46,21 +46,6 @@ describe Administrateur, type: :model do end end - describe '#delete_and_transfer_services' do - let!(:administrateur) { create(:administrateur) } - let!(:autre_administrateur) { create(:administrateur) } - let!(:procedure) { create(:procedure, :with_service, administrateurs: [administrateur, autre_administrateur]) } - let(:service) { procedure.service } - - it "delete and transfer services to other admin" do - service.update(administrateur: administrateur) - administrateur.delete_and_transfer_services - - expect(Administrateur.find_by(id: administrateur.id)).to be_nil - expect(service.reload.administrateur).to eq(autre_administrateur) - end - end - # describe '#password_complexity' do # let(:email) { 'mail@beta.gouv.fr' } # let(:passwords) { ['pass', '12pass23', 'démarches ', 'démarches-simple', 'démarches-simplifiées-pwd'] } diff --git a/spec/models/instructeur_spec.rb b/spec/models/instructeur_spec.rb index 78737bf89..37917bf18 100644 --- a/spec/models/instructeur_spec.rb +++ b/spec/models/instructeur_spec.rb @@ -424,40 +424,9 @@ describe Instructeur, type: :model do it { expect(instructeur_a.procedures.all.to_ary).to eq([procedure_a]) } end - describe "#can_be_deleted?" do - subject { instructeur.can_be_deleted? } - - context 'when the instructeur is an administrateur' do - let!(:administrateur) { create(:administrateur) } - let(:instructeur) { administrateur.instructeur } - - it { is_expected.to be false } - end - - context "when the instructeur's procedures have other instructeurs" do - let(:instructeur_not_admin) { create(:instructeur) } - let(:autre_instructeur) { create(:instructeur) } - - it "can be deleted" do - assign(procedure, instructeur_assigne: instructeur_not_admin) - assign(procedure, instructeur_assigne: autre_instructeur) - expect(autre_instructeur.can_be_deleted?).to be_truthy - end - end - - context "when the instructeur's procedures is the only one" do - let(:instructeur_not_admin) { create :instructeur } - let(:autre_procedure) { create :procedure } - it "can be deleted" do - assign(autre_procedure, instructeur_assigne: instructeur_not_admin) - expect(instructeur_not_admin.can_be_deleted?).to be_falsy - end - end - end - private - def assign(procedure_to_assign, instructeur_assigne: instructeur) - create :assign_to, instructeur: instructeur_assigne, procedure: procedure_to_assign, groupe_instructeur: procedure_to_assign.defaut_groupe_instructeur + def assign(procedure_to_assign) + create :assign_to, instructeur: instructeur, procedure: procedure_to_assign, groupe_instructeur: procedure_to_assign.defaut_groupe_instructeur end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 19b317f40..b127d35c6 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -224,24 +224,6 @@ describe User, type: :model do context 'when the user has no dossier in instruction' do it { is_expected.to be true } end - - context 'when the user is an administrateur' do - it 'cannot be deleted' do - administrateur = create(:administrateur) - user = administrateur.user - - expect(user.can_be_deleted?).to be_falsy - end - end - - context 'when the user is an instructeur' do - it 'cannot be deleted' do - instructeur = create(:instructeur) - user = instructeur.user - - expect(user.can_be_deleted?).to be_falsy - end - end end describe '#delete_and_keep_track_dossiers' do @@ -259,29 +241,12 @@ describe User, type: :model do let!(:dossier_en_construction) { create(:dossier, :en_construction, user: user) } let!(:dossier_brouillon) { create(:dossier, user: user) } - context 'without a hidden dossier' do - it "keep track of dossiers and delete user" do - user.delete_and_keep_track_dossiers(administration) + it "keep track of dossiers and delete user" do + user.delete_and_keep_track_dossiers(administration) - expect(DeletedDossier.find_by(dossier_id: dossier_en_construction)).to be_present - expect(DeletedDossier.find_by(dossier_id: dossier_brouillon)).to be_present - expect(User.find_by(id: user.id)).to be_nil - end - end - - context 'with a hidden dossier' do - let!(:dossier_cache) do - create(:dossier, :en_construction, user: user) - end - - it "keep track of dossiers and delete user" do - dossier_cache.delete_and_keep_track(administration) - user.delete_and_keep_track_dossiers(administration) - - expect(DeletedDossier.find_by(dossier_id: dossier_en_construction)).to be_present - expect(DeletedDossier.find_by(dossier_id: dossier_brouillon)).to be_present - expect(User.find_by(id: user.id)).to be_nil - end + expect(DeletedDossier.find_by(dossier_id: dossier_en_construction)).to be_present + expect(DeletedDossier.find_by(dossier_id: dossier_brouillon)).to be_present + expect(User.find_by(id: user.id)).to be_nil end end end