From 58ef36ff571910892453231c859d9a92884d431c Mon Sep 17 00:00:00 2001 From: Christophe Robillard Date: Mon, 6 Jan 2020 17:33:09 +0100 Subject: [PATCH 1/7] tells if a user can be deleted --- app/models/user.rb | 4 ++++ spec/models/user_spec.rb | 16 ++++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/app/models/user.rb b/app/models/user.rb index 41a4984b3..84daa920f 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -96,6 +96,10 @@ class User < ApplicationRecord last_sign_in_at.present? end + def can_be_deleted? + dossiers.state_instruction_commencee.empty? + end + private def link_invites! diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 54eb0dcf3..154666d47 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -209,4 +209,20 @@ describe User, type: :model do it { is_expected.to be true } end end + + describe '#can_be_deleted?' do + let(:user) { create(:user) } + + subject { user.can_be_deleted? } + + context 'when the user has a dossier in instruction' do + let!(:dossier) { create(:dossier, :en_instruction, user: user) } + + it { is_expected.to be false } + end + + context 'when the user has no dossier in instruction' do + it { is_expected.to be true } + end + end end From 9a62d3fe0cce3d00c9bec00ff947ad1d92344170 Mon Sep 17 00:00:00 2001 From: Christophe Robillard Date: Wed, 8 Jan 2020 10:50:16 +0100 Subject: [PATCH 2/7] delete a user --- app/controllers/manager/users_controller.rb | 13 ++++++++ app/models/user.rb | 9 ++++++ app/views/manager/users/show.html.erb | 8 ++--- config/routes.rb | 1 + .../manager/users_controller_spec.rb | 17 ++++++++++ spec/models/user_spec.rb | 31 +++++++++++++++++++ 6 files changed, 73 insertions(+), 6 deletions(-) create mode 100644 spec/controllers/manager/users_controller_spec.rb diff --git a/app/controllers/manager/users_controller.rb b/app/controllers/manager/users_controller.rb index e80d22404..9e1a9fc54 100644 --- a/app/controllers/manager/users_controller.rb +++ b/app/controllers/manager/users_controller.rb @@ -20,5 +20,18 @@ module Manager head :ok end + + def delete + user = User.find(params[:id]) + if !user.can_be_deleted? + fail "Impossible de supprimer cet utilisateur car il a des dossiers en instruction" + end + user.delete_and_keep_track_dossiers(current_administration) + + logger.info("L'utilisateur #{user.id} est supprimé par #{current_administration.id}") + flash[:notice] = "L'utilisateur #{user.id} est supprimé" + + redirect_to manager_users_path + end end end diff --git a/app/models/user.rb b/app/models/user.rb index 84daa920f..9f95d1075 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -100,6 +100,15 @@ class User < ApplicationRecord dossiers.state_instruction_commencee.empty? end + def delete_and_keep_track_dossiers(administration) + if can_be_deleted? + dossiers.each do |dossier| + dossier.delete_and_keep_track(administration) + end + destroy + end + end + private def link_invites! diff --git a/app/views/manager/users/show.html.erb b/app/views/manager/users/show.html.erb index 079866322..ab51789a6 100644 --- a/app/views/manager/users/show.html.erb +++ b/app/views/manager/users/show.html.erb @@ -25,13 +25,9 @@ as well as a link to its edit page.
- <%= link_to( - t("administrate.actions.edit_resource", name: page.page_title), - [:edit, namespace, page.resource], - class: "button", - ) if valid_action?(:edit) && show_action?(:edit, page.resource) %> -
+ <%= 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é" %> +
<% if !user.confirmed? %> <%= link_to('Renvoyer l’email de confirmation', [:resend_confirmation_instructions, namespace, page.resource], method: :post, class: 'button') %> diff --git a/config/routes.rb b/config/routes.rb index 293d0d85e..8c7994777 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -25,6 +25,7 @@ Rails.application.routes.draw do end resources :users, only: [:index, :show] do + delete 'delete', on: :member post 'resend_confirmation_instructions', on: :member put 'enable_feature', on: :member end diff --git a/spec/controllers/manager/users_controller_spec.rb b/spec/controllers/manager/users_controller_spec.rb new file mode 100644 index 000000000..9a703a82d --- /dev/null +++ b/spec/controllers/manager/users_controller_spec.rb @@ -0,0 +1,17 @@ +describe Manager::UsersController, type: :controller do + let(:administration) { create(:administration) } + + describe '#delete' do + let!(:user) { create(:user) } + + before { sign_in administration } + + subject { delete :delete, params: { id: user.id } } + + it 'deletes the user' do + subject + + expect(User.find_by(id: user.id)).to be_nil + end + end +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 154666d47..f4d805132 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -225,4 +225,35 @@ describe User, type: :model do it { is_expected.to be true } end end + + describe '#delete_and_keep_track_dossiers' do + let(:administration) { create(:administration) } + let(:user) { create(:user) } + + context 'avec un dossier en instruction' do + let!(:dossier_en_instruction) { create(:dossier, :en_instruction, user: user) } + it 'ne supprime rien si dossier en instruction' do + + user.delete_and_keep_track_dossiers(administration) + + expect(Dossier.find_by(id: dossier_en_instruction.id)).to be_present + expect(User.find_by(id: user.id)).to be_present + end + + end + + context 'sans dossier en instruction' do + let!(:dossier_en_construction) { create(:dossier, :en_construction, user: user) } + let!(:dossier_brouillon) { create(:dossier, user: user) } + + it "garde une trace des dossiers et supprime l'utilisateur" 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 + end end From c72ce977f7639c8156c29736e2911cba4a571ba9 Mon Sep 17 00:00:00 2001 From: Christophe Robillard Date: Wed, 8 Jan 2020 11:18:44 +0100 Subject: [PATCH 3/7] refactor delete_user_account task --- lib/tasks/support.rake | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/tasks/support.rake b/lib/tasks/support.rake index 7d96ac8e2..7facf3e2d 100644 --- a/lib/tasks/support.rake +++ b/lib/tasks/support.rake @@ -15,13 +15,11 @@ namespace :support do user = User.find_by!(email: user_email) administration = Administration.find_by!(email: administration_email) - if user.dossiers.state_instruction_commencee.any? + if !user.can_be_deleted? fail "Cannot delete this user because instruction has started for some dossiers" end - user.dossiers.each do |dossier| - dossier.delete_and_keep_track(administration) - end + user.delete_and_keep_track_dossiers(administration) user.destroy end From a0b4d97d08490f10cf586cfae736d16bc2201a2a Mon Sep 17 00:00:00 2001 From: Christophe Robillard Date: Wed, 8 Jan 2020 15:09:03 +0100 Subject: [PATCH 4/7] fix rubocop offenses --- spec/models/user_spec.rb | 3 --- 1 file changed, 3 deletions(-) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index f4d805132..22a742584 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -233,13 +233,11 @@ describe User, type: :model do context 'avec un dossier en instruction' do let!(:dossier_en_instruction) { create(:dossier, :en_instruction, user: user) } it 'ne supprime rien si dossier en instruction' do - user.delete_and_keep_track_dossiers(administration) expect(Dossier.find_by(id: dossier_en_instruction.id)).to be_present expect(User.find_by(id: user.id)).to be_present end - end context 'sans dossier en instruction' do @@ -247,7 +245,6 @@ describe User, type: :model do let!(:dossier_brouillon) { create(:dossier, user: user) } it "garde une trace des dossiers et supprime l'utilisateur" do - user.delete_and_keep_track_dossiers(administration) expect(DeletedDossier.find_by(dossier_id: dossier_en_construction)).to be_present From 696a058280c05c7385086dba6f1864f4e7bb768a Mon Sep 17 00:00:00 2001 From: Christophe Robillard Date: Wed, 8 Jan 2020 16:08:47 +0100 Subject: [PATCH 5/7] delete adminis with procs managed by other admins --- app/models/administrateur.rb | 2 +- .../manager/administrateurs/show.html.erb | 2 +- spec/models/administrateur_spec.rb | 25 +++++++++++++++++++ 3 files changed, 27 insertions(+), 2 deletions(-) diff --git a/app/models/administrateur.rb b/app/models/administrateur.rb index 16a41be91..4d4b84d4a 100644 --- a/app/models/administrateur.rb +++ b/app/models/administrateur.rb @@ -68,6 +68,6 @@ class Administrateur < ApplicationRecord end def can_be_deleted? - dossiers.state_instruction_commencee.none? && procedures.none? + dossiers.state_instruction_commencee.none? && procedures.all? { |p| p.administrateurs.count > 1 } end end diff --git a/app/views/manager/administrateurs/show.html.erb b/app/views/manager/administrateurs/show.html.erb index 34b4b9857..73019d37f 100644 --- a/app/views/manager/administrateurs/show.html.erb +++ b/app/views/manager/administrateurs/show.html.erb @@ -36,7 +36,7 @@ as well as a link to its edit page. <% if page.resource.invitation_expired? %> <%= link_to "renvoyer l'invitation", reinvite_manager_administrateur_path(page.resource), method: :post, class: "button" %> <% end %> - <%= button_to "supprimer", delete_manager_administrateur_path(page.resource), method: :delete, disabled: !page.resource.can_be_deleted?, class: "button", data: { confirm: "Confirmez-vous la suppression de l'administrateur ?" }, title: page.resource.can_be_deleted? ? "Supprimer" : "Cet administrateur a des dossiers ou des procédures et ne peut être supprimé" %> + <%= button_to "supprimer", delete_manager_administrateur_path(page.resource), method: :delete, disabled: !page.resource.can_be_deleted?, class: "button", data: { confirm: "Confirmez-vous la suppression de l'administrateur ?" }, title: page.resource.can_be_deleted? ? "Supprimer" : "Cet administrateur a des dossiers ou des procédures dont il est le seul admin et ne peut être supprimé" %>
diff --git a/spec/models/administrateur_spec.rb b/spec/models/administrateur_spec.rb index 4b9fef1f0..9f0c30923 100644 --- a/spec/models/administrateur_spec.rb +++ b/spec/models/administrateur_spec.rb @@ -21,6 +21,31 @@ describe Administrateur, type: :model do end end + describe "#can_be_deleted?" do + subject { administrateur.can_be_deleted? } + + context 'when the administrateur has a dossier in instruction' do + let!(:dossier) { create(:dossier, :en_instruction) } + let(:administrateur) { dossier.procedure.administrateurs.first } + + it { is_expected.to be false } + end + + context "when the administrateur's procedures have other administrateurs" do + let!(:administrateur) { create(:administrateur) } + let!(:autre_administrateur) { create(:administrateur) } + let!(:procedure) { create(:procedure, administrateurs: [administrateur, autre_administrateur]) } + + it { is_expected.to be true } + end + + context "when the administrateur has no procedure" do + let!(:administrateur) { create(:administrateur) } + + it { is_expected.to be true } + 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'] } From 0f881f942be89c0b8359513d476f06c8a4e2dec4 Mon Sep 17 00:00:00 2001 From: Christophe Robillard Date: Wed, 8 Jan 2020 17:00:00 +0100 Subject: [PATCH 6/7] raise if trying to del a user who can't be deleted --- app/models/user.rb | 4 ++++ lib/tasks/support.rake | 4 ---- spec/models/user_spec.rb | 7 ++----- 3 files changed, 6 insertions(+), 9 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 9f95d1075..4838da20d 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -101,6 +101,10 @@ class User < ApplicationRecord end def delete_and_keep_track_dossiers(administration) + if !can_be_deleted? + raise "Cannot delete this user because instruction has started for some dossiers" + end + if can_be_deleted? dossiers.each do |dossier| dossier.delete_and_keep_track(administration) diff --git a/lib/tasks/support.rake b/lib/tasks/support.rake index 7facf3e2d..3ad4aee42 100644 --- a/lib/tasks/support.rake +++ b/lib/tasks/support.rake @@ -15,10 +15,6 @@ namespace :support do user = User.find_by!(email: user_email) administration = Administration.find_by!(email: administration_email) - if !user.can_be_deleted? - fail "Cannot delete this user because instruction has started for some dossiers" - end - user.delete_and_keep_track_dossiers(administration) user.destroy end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 22a742584..7181878ad 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -232,11 +232,8 @@ describe User, type: :model do context 'avec un dossier en instruction' do let!(:dossier_en_instruction) { create(:dossier, :en_instruction, user: user) } - it 'ne supprime rien si dossier en instruction' do - user.delete_and_keep_track_dossiers(administration) - - expect(Dossier.find_by(id: dossier_en_instruction.id)).to be_present - expect(User.find_by(id: user.id)).to be_present + it 'raises' do + expect { user.delete_and_keep_track_dossiers(administration) }.to raise_error end end From d289c21a8b20beb4e8d7da9d01eeb0ea5b197946 Mon Sep 17 00:00:00 2001 From: Christophe Robillard Date: Wed, 8 Jan 2020 17:05:04 +0100 Subject: [PATCH 7/7] speak english for test descriptions --- spec/models/user_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 7181878ad..b127d35c6 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -230,18 +230,18 @@ describe User, type: :model do let(:administration) { create(:administration) } let(:user) { create(:user) } - context 'avec un dossier en instruction' do + context 'with a dossier in instruction' do let!(:dossier_en_instruction) { create(:dossier, :en_instruction, user: user) } it 'raises' do expect { user.delete_and_keep_track_dossiers(administration) }.to raise_error end end - context 'sans dossier en instruction' do + context 'without a dossier in instruction' do let!(:dossier_en_construction) { create(:dossier, :en_construction, user: user) } let!(:dossier_brouillon) { create(:dossier, user: user) } - it "garde une trace des dossiers et supprime l'utilisateur" do + 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