From 4a947f9135a653c5e596fa7eb03f9653474dfa71 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Wed, 6 Oct 2021 19:33:49 +0200 Subject: [PATCH 1/9] feat(manager): add become administrateur button in manager (with 24h expiration) --- app/controllers/manager/procedures_controller.rb | 8 ++++---- .../cron/purge_manager_administrateur_sessions_job.rb | 7 +++++++ app/models/administrateurs_procedure.rb | 1 + app/views/manager/procedures/show.html.erb | 9 ++++++--- ...06164955_add_manager_to_administrateurs_procedures.rb | 5 +++++ db/schema.rb | 3 ++- 6 files changed, 25 insertions(+), 8 deletions(-) create mode 100644 app/jobs/cron/purge_manager_administrateur_sessions_job.rb create mode 100644 db/migrate/20211006164955_add_manager_to_administrateurs_procedures.rb diff --git a/app/controllers/manager/procedures_controller.rb b/app/controllers/manager/procedures_controller.rb index 9ca75b503..eb9d3c6ef 100644 --- a/app/controllers/manager/procedures_controller.rb +++ b/app/controllers/manager/procedures_controller.rb @@ -47,12 +47,12 @@ module Manager end def add_administrateur - administrateur = Administrateur.by_email(params[:email]) + administrateur = Administrateur.by_email(current_super_admin.email) if administrateur - procedure.administrateurs << administrateur - flash[:notice] = "L'administrateur \"#{params[:email]}\" est ajouté à la démarche." + AdministrateursProcedure.create(procedure: procedure, administrateur: administrateur, manager: true) + flash[:notice] = "L’administrateur \"#{administrateur.email}\" est ajouté à la démarche pour la journée." else - flash[:alert] = "L'administrateur \"#{params[:email]}\" est introuvable." + flash[:alert] = "Vous n’êtes pas connecté en tant qu’administrateur." end redirect_to manager_procedure_path(procedure) end diff --git a/app/jobs/cron/purge_manager_administrateur_sessions_job.rb b/app/jobs/cron/purge_manager_administrateur_sessions_job.rb new file mode 100644 index 000000000..29de55b38 --- /dev/null +++ b/app/jobs/cron/purge_manager_administrateur_sessions_job.rb @@ -0,0 +1,7 @@ +class Cron::PurgeManagerAdministrateurSessionsJob < Cron::CronJob + self.schedule_expression = "every day at 3 am" + + def perform + AdministrateursProcedure.where(manager: true).destroy_all + end +end diff --git a/app/models/administrateurs_procedure.rb b/app/models/administrateurs_procedure.rb index f0a883b04..1fd2002dc 100644 --- a/app/models/administrateurs_procedure.rb +++ b/app/models/administrateurs_procedure.rb @@ -2,6 +2,7 @@ # # Table name: administrateurs_procedures # +# manager :boolean # created_at :datetime not null # updated_at :datetime not null # administrateur_id :bigint not null diff --git a/app/views/manager/procedures/show.html.erb b/app/views/manager/procedures/show.html.erb index da0724ee8..492b787d7 100644 --- a/app/views/manager/procedures/show.html.erb +++ b/app/views/manager/procedures/show.html.erb @@ -64,9 +64,12 @@ as well as a link to its edit page.
<%= render_field attribute, page: page %> <% if attribute.name == 'administrateurs' %> - <%= form_tag(add_administrateur_manager_procedure_path(procedure), style: 'margin-top: 1rem;') do %> - <%= email_field_tag(:email, '', placeholder: 'Email', autocapitalize: 'off', autocorrect: 'off', spellcheck: 'false', style: 'margin-bottom: 1rem;width:24rem;') %> - + <% if procedure.administrateurs.find { |admin| admin.email == current_super_admin.email } %> +

Vous êtes déjà administrateur sur cette démarche

+ <% else %> + <%= form_tag(add_administrateur_manager_procedure_path(procedure), style: 'margin-top: 1rem;') do %> + + <% end %> <% end %> <% end %>
diff --git a/db/migrate/20211006164955_add_manager_to_administrateurs_procedures.rb b/db/migrate/20211006164955_add_manager_to_administrateurs_procedures.rb new file mode 100644 index 000000000..8e424a92b --- /dev/null +++ b/db/migrate/20211006164955_add_manager_to_administrateurs_procedures.rb @@ -0,0 +1,5 @@ +class AddManagerToAdministrateursProcedures < ActiveRecord::Migration[6.1] + def change + add_column :administrateurs_procedures, :manager, :boolean + end +end diff --git a/db/schema.rb b/db/schema.rb index 5c75e2aea..2d29640fd 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2021_10_01_143403) do +ActiveRecord::Schema.define(version: 2021_10_06_164955) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -76,6 +76,7 @@ ActiveRecord::Schema.define(version: 2021_10_01_143403) do t.bigint "procedure_id", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.boolean "manager" t.index ["administrateur_id", "procedure_id"], name: "index_unique_admin_proc_couple", unique: true t.index ["administrateur_id"], name: "index_administrateurs_procedures_on_administrateur_id" t.index ["procedure_id"], name: "index_administrateurs_procedures_on_procedure_id" From 195583c0bdd33f52f9f68b4ad8c75433d4dae769 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Mon, 4 Oct 2021 12:17:07 +0200 Subject: [PATCH 2/9] Enable manager specs --- spec/controllers/manager/instructeurs_controller_spec.rb | 2 +- spec/controllers/manager/procedures_controller_spec.rb | 2 +- spec/controllers/manager/users_controller_spec.rb | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/controllers/manager/instructeurs_controller_spec.rb b/spec/controllers/manager/instructeurs_controller_spec.rb index 6dedf3797..11279083b 100644 --- a/spec/controllers/manager/instructeurs_controller_spec.rb +++ b/spec/controllers/manager/instructeurs_controller_spec.rb @@ -1,4 +1,4 @@ -xdescribe Manager::InstructeursController, type: :controller do +describe Manager::InstructeursController, type: :controller do let(:super_admin) { create(:super_admin) } let(:instructeur) { create(:instructeur) } diff --git a/spec/controllers/manager/procedures_controller_spec.rb b/spec/controllers/manager/procedures_controller_spec.rb index 8391a59b6..ad27b784a 100644 --- a/spec/controllers/manager/procedures_controller_spec.rb +++ b/spec/controllers/manager/procedures_controller_spec.rb @@ -1,4 +1,4 @@ -xdescribe Manager::ProceduresController, type: :controller do +describe Manager::ProceduresController, type: :controller do let(:super_admin) { create :super_admin } before { sign_in super_admin } diff --git a/spec/controllers/manager/users_controller_spec.rb b/spec/controllers/manager/users_controller_spec.rb index c6be09477..3593a9dcc 100644 --- a/spec/controllers/manager/users_controller_spec.rb +++ b/spec/controllers/manager/users_controller_spec.rb @@ -1,4 +1,4 @@ -xdescribe Manager::UsersController, type: :controller do +describe Manager::UsersController, type: :controller do let(:super_admin) { create(:super_admin) } describe '#show' do @@ -40,7 +40,7 @@ xdescribe Manager::UsersController, type: :controller do subject expect(User.find_by(id: user.id).email).not_to eq(nouvel_email) - expect(flash[:error]).to match("« #{nouvel_email} » n'est pas une adresse valide.") + expect(flash[:error]).to match("« #{nouvel_email} » n’est pas une adresse valide.") end end end From c56199e8f746a2c708f5d86bf24a5d55282263cd Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Mon, 4 Oct 2021 13:10:14 +0200 Subject: [PATCH 3/9] spec cleaning --- spec/controllers/manager/users_controller_spec.rb | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/spec/controllers/manager/users_controller_spec.rb b/spec/controllers/manager/users_controller_spec.rb index 3593a9dcc..2e7ea86ce 100644 --- a/spec/controllers/manager/users_controller_spec.rb +++ b/spec/controllers/manager/users_controller_spec.rb @@ -1,6 +1,8 @@ describe Manager::UsersController, type: :controller do let(:super_admin) { create(:super_admin) } + before { sign_in super_admin } + describe '#show' do render_views @@ -8,7 +10,6 @@ describe Manager::UsersController, type: :controller do let(:user) { create(:user) } before do - sign_in(super_admin) get :show, params: { id: user.id } end @@ -16,11 +17,8 @@ describe Manager::UsersController, type: :controller do end describe '#update' do - let!(:user) { create(:user, email: 'ancien.email@domaine.fr') } + let(:user) { create(:user, email: 'ancien.email@domaine.fr') } - before { - sign_in super_admin - } subject { patch :update, params: { id: user.id, user: { email: nouvel_email } } } describe 'with a valid email' do @@ -46,9 +44,7 @@ describe Manager::UsersController, type: :controller do end describe '#delete' do - let!(:user) { create(:user) } - - before { sign_in super_admin } + let(:user) { create(:user) } subject { delete :delete, params: { id: user.id } } From 9a6a53349f877094c631fb28aace225475052ed7 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Mon, 4 Oct 2021 14:00:32 +0200 Subject: [PATCH 4/9] simple cases when the preexisting targeted account does not have instructeur or profile profile --- app/controllers/manager/users_controller.rb | 29 ++++++++--- .../manager/users_controller_spec.rb | 52 +++++++++++++++---- 2 files changed, 64 insertions(+), 17 deletions(-) diff --git a/app/controllers/manager/users_controller.rb b/app/controllers/manager/users_controller.rb index 5f1e52ad5..2586c5a51 100644 --- a/app/controllers/manager/users_controller.rb +++ b/app/controllers/manager/users_controller.rb @@ -2,14 +2,25 @@ module Manager class UsersController < Manager::ApplicationController def update user = User.find(params[:id]) - new_email = params[:user][:email] - user.skip_reconfirmation! - user.update(email: new_email) - if (user.valid?) - flash[:notice] = "L'email a été modifié en « #{new_email} » sans notification ni validation par email." + + preexisting_user = User.find_by(email: targeted_email) + + if preexisting_user.nil? + user.skip_reconfirmation! + user.update(email: targeted_email) + + if (user.valid?) + flash[:notice] = "L'email a été modifié en « #{targeted_email} » sans notification ni validation par email." + else + flash[:error] = user.errors.full_messages.to_sentence + end else - flash[:error] = "« #{new_email} » n’est pas une adresse valide." + user.dossiers.update_all(user_id: preexisting_user.id) + + user.instructeur&.update(user: preexisting_user) + user.expert&.update(user: preexisting_user) end + redirect_to edit_manager_user_path(user) end @@ -72,5 +83,11 @@ module Manager end redirect_to emails_manager_user_path(@user) end + + private + + def targeted_email + params[:user][:email] + end end end diff --git a/spec/controllers/manager/users_controller_spec.rb b/spec/controllers/manager/users_controller_spec.rb index 2e7ea86ce..4649b5d99 100644 --- a/spec/controllers/manager/users_controller_spec.rb +++ b/spec/controllers/manager/users_controller_spec.rb @@ -21,24 +21,54 @@ describe Manager::UsersController, type: :controller do subject { patch :update, params: { id: user.id, user: { email: nouvel_email } } } - describe 'with a valid email' do - let(:nouvel_email) { 'nouvel.email@domaine.fr' } + context 'when the targeted email does not exist' do + describe 'with a valid email' do + let(:nouvel_email) { 'nouvel.email@domaine.fr' } - it 'updates the user email' do - subject + it 'updates the user email' do + subject - expect(User.find_by(id: user.id).email).to eq(nouvel_email) + expect(User.find_by(id: user.id).email).to eq(nouvel_email) + end + end + + describe 'with an invalid email' do + let(:nouvel_email) { 'plop' } + + it 'does not update the user email' do + subject + + expect(User.find_by(id: user.id).email).not_to eq(nouvel_email) + expect(flash[:error]).to match("Courriel invalide") + end end end - describe 'with an invalid email' do - let(:nouvel_email) { 'plop' } + context 'when the targeted email exists' do + let(:preexisting_user) { create(:user, email: 'email.existant@domaine.fr') } + let(:nouvel_email) { preexisting_user.email } - it 'does not update the user email' do - subject + context 'and the old account has a dossier' do + let!(:dossier) { create(:dossier, user: user) } - expect(User.find_by(id: user.id).email).not_to eq(nouvel_email) - expect(flash[:error]).to match("« #{nouvel_email} » n’est pas une adresse valide.") + it 'transfers the dossier' do + subject + + expect(preexisting_user.dossiers).to match([dossier]) + end + end + + context 'and the old account belongs to an instructeur and expert' do + let!(:instructeur) { create(:instructeur, user: user) } + let!(:expert) { create(:expert, user: user) } + + it 'transfers instructeur account' do + subject + preexisting_user.reload + + expect(preexisting_user.instructeur).to match(instructeur) + expect(preexisting_user.expert).to match(expert) + end end end end From 136f29524ea5fc9ae596fa73387be854c88d9370 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Mon, 4 Oct 2021 15:26:01 +0200 Subject: [PATCH 5/9] merge instructeur --- app/controllers/manager/users_controller.rb | 7 ++++- app/models/administrateurs_instructeur.rb | 13 +++++++++ app/models/instructeur.rb | 12 +++++++- .../manager/users_controller_spec.rb | 29 +++++++++++++++++++ 4 files changed, 59 insertions(+), 2 deletions(-) create mode 100644 app/models/administrateurs_instructeur.rb diff --git a/app/controllers/manager/users_controller.rb b/app/controllers/manager/users_controller.rb index 2586c5a51..fcc19b17b 100644 --- a/app/controllers/manager/users_controller.rb +++ b/app/controllers/manager/users_controller.rb @@ -17,7 +17,12 @@ module Manager else user.dossiers.update_all(user_id: preexisting_user.id) - user.instructeur&.update(user: preexisting_user) + if preexisting_user.instructeur.nil? + user.instructeur&.update(user: preexisting_user) + else + preexisting_user.instructeur.merge(user.instructeur) + end + user.expert&.update(user: preexisting_user) end diff --git a/app/models/administrateurs_instructeur.rb b/app/models/administrateurs_instructeur.rb new file mode 100644 index 000000000..96a9320f7 --- /dev/null +++ b/app/models/administrateurs_instructeur.rb @@ -0,0 +1,13 @@ +# == Schema Information +# +# Table name: administrateurs_instructeurs +# +# created_at :datetime +# updated_at :datetime +# administrateur_id :integer +# instructeur_id :integer +# +class AdministrateursInstructeur < ApplicationRecord + belongs_to :administrateur + belongs_to :instructeur +end diff --git a/app/models/instructeur.rb b/app/models/instructeur.rb index cf2c0fefb..572900f3e 100644 --- a/app/models/instructeur.rb +++ b/app/models/instructeur.rb @@ -10,7 +10,8 @@ # updated_at :datetime # class Instructeur < ApplicationRecord - has_and_belongs_to_many :administrateurs + has_many :administrateurs_instructeurs + has_many :administrateurs, through: :administrateurs_instructeurs has_many :assign_to, dependent: :destroy has_many :groupe_instructeurs, through: :assign_to @@ -19,6 +20,7 @@ class Instructeur < ApplicationRecord has_many :assign_to_with_email_notifications, -> { with_email_notifications }, class_name: 'AssignTo', inverse_of: :instructeur has_many :groupe_instructeur_with_email_notifications, through: :assign_to_with_email_notifications, source: :groupe_instructeur + has_many :commentaires has_many :dossiers, -> { state_not_brouillon }, through: :groupe_instructeurs has_many :follows, -> { active }, inverse_of: :instructeur has_many :previous_follows, -> { inactive }, class_name: 'Follow', inverse_of: :instructeur @@ -249,6 +251,14 @@ class Instructeur < ApplicationRecord Dossier.connection.select_all(sanitized_query).first end + def merge(old_instructeur) + old_instructeur.assign_to.update_all(instructeur_id: id) + old_instructeur.follows.update_all(instructeur_id: id) + old_instructeur.administrateurs_instructeurs.update_all(instructeur_id: id) + old_instructeur.commentaires.update_all(instructeur_id: id) + old_instructeur.bulk_messages.update_all(instructeur_id: id) + end + private def annotations_hash(demande, annotations_privees, avis, messagerie) diff --git a/spec/controllers/manager/users_controller_spec.rb b/spec/controllers/manager/users_controller_spec.rb index 4649b5d99..c078ebc9b 100644 --- a/spec/controllers/manager/users_controller_spec.rb +++ b/spec/controllers/manager/users_controller_spec.rb @@ -69,6 +69,35 @@ describe Manager::UsersController, type: :controller do expect(preexisting_user.instructeur).to match(instructeur) expect(preexisting_user.expert).to match(expert) end + + context 'and the preexisting account owns an instructeur and expert as well' do + let!(:preexisting_instructeur) { create(:instructeur, user: preexisting_user) } + let!(:preexisting_expert) { create(:expert, user: preexisting_user) } + + context 'and the source instructeur has some procedures and dossiers' do + let!(:procedure) { create(:procedure, instructeurs: [instructeur]) } + let(:dossier) { create(:dossier) } + let(:administrateur) { create(:administrateur) } + let!(:commentaire) { create(:commentaire, instructeur: instructeur, dossier: dossier) } + let!(:bulk_message) { BulkMessage.create!(instructeur: instructeur, body: 'body', sent_at: Time.zone.now) } + + before do + user.instructeur.followed_dossiers << dossier + user.instructeur.administrateurs << administrateur + end + + it 'transferts all the stuff' do + subject + preexisting_user.reload + + expect(procedure.instructeurs).to match([preexisting_user.instructeur]) + expect(preexisting_user.instructeur.followed_dossiers).to match([dossier]) + expect(preexisting_user.instructeur.administrateurs).to match([administrateur]) + expect(preexisting_user.instructeur.commentaires).to match([commentaire]) + expect(preexisting_user.instructeur.bulk_messages).to match([bulk_message]) + end + end + end end end end From a480b31eb5582ec6fb378541287002d457af001c Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Mon, 4 Oct 2021 15:26:11 +0200 Subject: [PATCH 6/9] merge expert --- app/controllers/manager/users_controller.rb | 6 +++++- app/models/expert.rb | 6 ++++++ spec/controllers/manager/users_controller_spec.rb | 14 ++++++++++++++ 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/app/controllers/manager/users_controller.rb b/app/controllers/manager/users_controller.rb index fcc19b17b..7c1b9643a 100644 --- a/app/controllers/manager/users_controller.rb +++ b/app/controllers/manager/users_controller.rb @@ -23,7 +23,11 @@ module Manager preexisting_user.instructeur.merge(user.instructeur) end - user.expert&.update(user: preexisting_user) + if preexisting_user.expert.nil? + user.expert&.update(user: preexisting_user) + else + preexisting_user.expert.merge(user.expert) + end end redirect_to edit_manager_user_path(user) diff --git a/app/models/expert.rb b/app/models/expert.rb index 95d82cdbf..d2766a3f7 100644 --- a/app/models/expert.rb +++ b/app/models/expert.rb @@ -11,6 +11,7 @@ class Expert < ApplicationRecord has_many :experts_procedures has_many :avis, through: :experts_procedures has_many :dossiers, through: :avis + has_many :commentaires default_scope { eager_load(:user) } @@ -34,4 +35,9 @@ class Expert < ApplicationRecord @avis_summary = { unanswered: result.unanswered, total: result.total } end end + + def merge(old_expert) + old_expert.experts_procedures.update_all(expert_id: id) + old_expert.commentaires.update_all(expert_id: id) + end end diff --git a/spec/controllers/manager/users_controller_spec.rb b/spec/controllers/manager/users_controller_spec.rb index c078ebc9b..beb043ac3 100644 --- a/spec/controllers/manager/users_controller_spec.rb +++ b/spec/controllers/manager/users_controller_spec.rb @@ -97,6 +97,20 @@ describe Manager::UsersController, type: :controller do expect(preexisting_user.instructeur.bulk_messages).to match([bulk_message]) end end + + context 'and the source expert has some avis and commentaires' do + let(:dossier) { create(:dossier) } + let(:experts_procedure) { create(:experts_procedure, expert: user.expert, procedure: dossier.procedure) } + let!(:avis) { create(:avis, dossier: dossier, claimant: create(:instructeur), experts_procedure: experts_procedure) } + let!(:commentaire) { create(:commentaire, expert: expert, dossier: dossier) } + + it 'transfers the avis' do + subject + + expect(preexisting_user.expert.avis).to match([avis]) + expect(preexisting_user.expert.commentaires).to match([commentaire]) + end + end end end end From 5009c583ea10bebaef0e01c1c55e3ad958f6fa2b Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Mon, 4 Oct 2021 15:44:05 +0200 Subject: [PATCH 7/9] Add notice when merging account --- app/controllers/manager/users_controller.rb | 2 ++ spec/controllers/manager/users_controller_spec.rb | 1 + 2 files changed, 3 insertions(+) diff --git a/app/controllers/manager/users_controller.rb b/app/controllers/manager/users_controller.rb index 7c1b9643a..d107ca5c2 100644 --- a/app/controllers/manager/users_controller.rb +++ b/app/controllers/manager/users_controller.rb @@ -28,6 +28,8 @@ module Manager else preexisting_user.expert.merge(user.expert) end + + flash[:notice] = "Le compte « #{targeted_email} » a absorbé le compte « #{user.email} »." end redirect_to edit_manager_user_path(user) diff --git a/spec/controllers/manager/users_controller_spec.rb b/spec/controllers/manager/users_controller_spec.rb index beb043ac3..896cbc21b 100644 --- a/spec/controllers/manager/users_controller_spec.rb +++ b/spec/controllers/manager/users_controller_spec.rb @@ -68,6 +68,7 @@ describe Manager::UsersController, type: :controller do expect(preexisting_user.instructeur).to match(instructeur) expect(preexisting_user.expert).to match(expert) + expect(flash[:notice]).to match("Le compte « email.existant@domaine.fr » a absorbé le compte « ancien.email@domaine.fr ».") end context 'and the preexisting account owns an instructeur and expert as well' do From 77d14d4a60cff4a3345c4262eadbd3c720ed2d1b Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Mon, 4 Oct 2021 15:53:20 +0200 Subject: [PATCH 8/9] forbid admin merge yet --- app/controllers/manager/users_controller.rb | 5 +++-- spec/controllers/manager/users_controller_spec.rb | 11 +++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/app/controllers/manager/users_controller.rb b/app/controllers/manager/users_controller.rb index d107ca5c2..bb70d54d5 100644 --- a/app/controllers/manager/users_controller.rb +++ b/app/controllers/manager/users_controller.rb @@ -2,10 +2,11 @@ module Manager class UsersController < Manager::ApplicationController def update user = User.find(params[:id]) - preexisting_user = User.find_by(email: targeted_email) - if preexisting_user.nil? + if user.administrateur.present? + flash[:error] = "« #{targeted_email} » est un administrateur. On ne sait pas encore faire." + elsif preexisting_user.nil? user.skip_reconfirmation! user.update(email: targeted_email) diff --git a/spec/controllers/manager/users_controller_spec.rb b/spec/controllers/manager/users_controller_spec.rb index 896cbc21b..c1e43f856 100644 --- a/spec/controllers/manager/users_controller_spec.rb +++ b/spec/controllers/manager/users_controller_spec.rb @@ -30,6 +30,17 @@ describe Manager::UsersController, type: :controller do expect(User.find_by(id: user.id).email).to eq(nouvel_email) end + + context 'and the user is an administrateur' do + let(:user) { create(:administrateur).user } + + it 'rejects the modification' do + subject + + expect(flash[:error]).to match("« nouvel.email@domaine.fr » est un administrateur. On ne sait pas encore faire.") + expect(user.reload.email).not_to eq(nouvel_email) + end + end end describe 'with an invalid email' do From d7e621d16706813f67b906890f3b0e9fabfd465e Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Thu, 7 Oct 2021 14:34:01 +0200 Subject: [PATCH 9/9] beef up the merge methods --- app/models/expert.rb | 18 ++++++- app/models/instructeur.rb | 26 ++++++++-- spec/models/expert_spec.rb | 63 +++++++++++++++++++++++ spec/models/instructeur_spec.rb | 88 +++++++++++++++++++++++++++++++++ 4 files changed, 191 insertions(+), 4 deletions(-) diff --git a/app/models/expert.rb b/app/models/expert.rb index d2766a3f7..01042c840 100644 --- a/app/models/expert.rb +++ b/app/models/expert.rb @@ -9,6 +9,7 @@ class Expert < ApplicationRecord has_one :user has_many :experts_procedures + has_many :procedures, through: :experts_procedures has_many :avis, through: :experts_procedures has_many :dossiers, through: :avis has_many :commentaires @@ -37,7 +38,22 @@ class Expert < ApplicationRecord end def merge(old_expert) - old_expert.experts_procedures.update_all(expert_id: id) + procedure_with_new, procedure_without_new = old_expert + .procedures + .partition { |p| p.experts.exists?(id) } + + ExpertsProcedure + .where(expert_id: old_expert.id, procedure: procedure_without_new) + .update_all(expert_id: id) + + ExpertsProcedure + .where(expert_id: old_expert.id, procedure: procedure_with_new) + .destroy_all + old_expert.commentaires.update_all(expert_id: id) + + Avis + .where(claimant_id: old_expert.id, claimant_type: Expert.name) + .update_all(claimant_id: id) end end diff --git a/app/models/instructeur.rb b/app/models/instructeur.rb index 572900f3e..e9b4c5985 100644 --- a/app/models/instructeur.rb +++ b/app/models/instructeur.rb @@ -252,9 +252,29 @@ class Instructeur < ApplicationRecord end def merge(old_instructeur) - old_instructeur.assign_to.update_all(instructeur_id: id) - old_instructeur.follows.update_all(instructeur_id: id) - old_instructeur.administrateurs_instructeurs.update_all(instructeur_id: id) + old_instructeur + .assign_to + .where.not(groupe_instructeur_id: assign_to.pluck(:groupe_instructeur_id)) + .update_all(instructeur_id: id) + + old_instructeur + .follows + .where.not(dossier_id: follows.pluck(:dossier_id)) + .update_all(instructeur_id: id) + + admin_with_new_instructeur, admin_without_new_instructeur = old_instructeur + .administrateurs + .partition { |admin| admin.instructeurs.exists?(id) } + + admin_without_new_instructeur.each do |admin| + admin.instructeurs << self + admin.instructeurs.delete(old_instructeur) + end + + admin_with_new_instructeur.each do |admin| + admin.instructeurs.delete(old_instructeur) + end + old_instructeur.commentaires.update_all(instructeur_id: id) old_instructeur.bulk_messages.update_all(instructeur_id: id) end diff --git a/spec/models/expert_spec.rb b/spec/models/expert_spec.rb index 2b3dd1c78..5b3ebf607 100644 --- a/spec/models/expert_spec.rb +++ b/spec/models/expert_spec.rb @@ -12,4 +12,67 @@ RSpec.describe Expert, type: :model do it { expect(ExpertsProcedure.where(expert: expert, procedure: procedure).count).to eq(1) } it { expect(ExpertsProcedure.where(expert: expert, procedure: procedure).first.allow_decision_access).to be_falsy } end + + describe '#merge' do + let(:old_expert) { create(:expert) } + let(:new_expert) { create(:expert) } + + subject { new_expert.merge(old_expert) } + + context 'when an old expert access a procedure' do + let(:procedure) { create(:procedure) } + + before do + procedure.experts << old_expert + subject + end + + it 'transfers the access to the new expert' do + expect(procedure.reload.experts). to match_array(new_expert) + end + end + + context 'when both expert access a procedure' do + let(:procedure) { create(:procedure) } + + before do + procedure.experts << old_expert + procedure.experts << new_expert + subject + end + + it 'removes the old one' do + expect(procedure.reload.experts). to match_array(new_expert) + end + end + + context 'when an old expert has a commentaire' do + let(:dossier) { create(:dossier) } + let(:commentaire) { CommentaireService.build(old_expert, dossier, body: "Mon commentaire") } + + before do + commentaire.save + subject + end + + it 'transfers the commentaire to the new expert' do + expect(new_expert.reload.commentaires).to match_array(commentaire) + end + end + + context 'when an old expert claims for an avis' do + let!(:avis) { create(:avis, dossier: create(:dossier), claimant: old_expert) } + + before do + subject + end + + it 'transfers the claim to the new expert' do + avis_claimed_by_new_expert = Avis + .where(claimant_id: new_expert.id, claimant_type: Expert.name) + + expect(avis_claimed_by_new_expert).to match_array(avis) + end + end + end end diff --git a/spec/models/instructeur_spec.rb b/spec/models/instructeur_spec.rb index 065e3eb19..a9ffc2c4a 100644 --- a/spec/models/instructeur_spec.rb +++ b/spec/models/instructeur_spec.rb @@ -737,6 +737,94 @@ describe Instructeur, type: :model do end end + describe '#merge' do + let(:old_instructeur) { create(:instructeur) } + let(:new_instructeur) { create(:instructeur) } + + subject { new_instructeur.merge(old_instructeur) } + + context 'when an procedure is assigned to the old instructeur' do + let(:procedure) { create(:procedure) } + + before do + procedure.defaut_groupe_instructeur.instructeurs << old_instructeur + subject + end + + it 'transfers the assignment' do + expect(new_instructeur.procedures).to match_array(procedure) + end + end + + context 'when both instructeurs are assigned to the same procedure' do + let(:procedure) { create(:procedure) } + + before do + procedure.defaut_groupe_instructeur.instructeurs << old_instructeur + procedure.defaut_groupe_instructeur.instructeurs << new_instructeur + subject + end + + it 'keeps the assignment' do + expect(new_instructeur.procedures).to match_array(procedure) + end + end + + context 'when a dossier is followed by an old instructeur' do + let(:dossier) { create(:dossier) } + + before do + old_instructeur.followed_dossiers << dossier + subject + end + + it 'transfers the dossier' do + expect(new_instructeur.followed_dossiers).to match_array(dossier) + end + end + + context 'when both instructeurs follow the same dossier' do + let(:dossier) { create(:dossier) } + + before do + old_instructeur.followed_dossiers << dossier + new_instructeur.followed_dossiers << dossier + subject + end + + it 'does not change anything' do + expect(new_instructeur.followed_dossiers.pluck(:id)).to match_array(dossier.id) + end + end + + context 'when the old instructeur is on on admin list' do + let(:administrateur) { create(:administrateur) } + + before do + administrateur.instructeurs << old_instructeur + subject + end + + it 'is replaced by the new one' do + expect(administrateur.reload.instructeurs).to match_array(new_instructeur) + end + end + + context 'when both are on the same admin list' do + let(:administrateur) { create(:administrateur) } + + before do + administrateur.instructeurs << old_instructeur + administrateur.instructeurs << new_instructeur + subject + end + + it 'removes the old one' do + expect(administrateur.reload.instructeurs).to match_array(new_instructeur) + end + end + end + private def assign(procedure_to_assign, instructeur_assigne: instructeur)