From 2f6cd317bf683aba63e88ecd537ce96070f21836 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Tue, 21 Dec 2021 17:25:14 +0000 Subject: [PATCH 1/2] fix(instructeur): set notification settings on all groupe instruteur for given procedure --- app/controllers/instructeurs/procedures_controller.rb | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/app/controllers/instructeurs/procedures_controller.rb b/app/controllers/instructeurs/procedures_controller.rb index 3cca930e6..fbc67b98d 100644 --- a/app/controllers/instructeurs/procedures_controller.rb +++ b/app/controllers/instructeurs/procedures_controller.rb @@ -216,11 +216,13 @@ module Instructeurs def email_notifications @procedure = procedure - @assign_to = assign_to + @assign_to = assign_tos.first end def update_email_notifications - assign_to.update!(assign_to_params) + assign_tos.each do |assign_to| + assign_to.update!(assign_to_params) + end flash.notice = 'Vos notifications sont enregistrées.' redirect_to instructeur_procedure_path(procedure) end @@ -290,10 +292,6 @@ module Instructeurs @exports = Export.find_for_groupe_instructeurs(groupe_instructeur_ids) end - def assign_to - current_instructeur.assign_to.joins(:groupe_instructeur).find_by(groupe_instructeurs: { procedure: procedure }) - end - def assign_tos @assign_tos ||= current_instructeur .assign_to From 4e779f445dc245d8aea637f643fa51a75133914d Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Tue, 21 Dec 2021 18:06:38 +0000 Subject: [PATCH 2/2] fix(instructeur): copy notifications settings from previous groupe instructeur fix #6763 --- .../groupe_instructeurs_controller.rb | 12 ++-- .../groupe_instructeurs_controller.rb | 4 +- app/models/groupe_instructeur.rb | 17 +++++ app/models/instructeur.rb | 15 ++-- spec/models/groupe_instructeur_spec.rb | 72 ++++++++++++++++++- spec/models/instructeur_spec.rb | 40 ++--------- 6 files changed, 111 insertions(+), 49 deletions(-) diff --git a/app/controllers/administrateurs/groupe_instructeurs_controller.rb b/app/controllers/administrateurs/groupe_instructeurs_controller.rb index 58d1cb379..5fc518620 100644 --- a/app/controllers/administrateurs/groupe_instructeurs_controller.rb +++ b/app/controllers/administrateurs/groupe_instructeurs_controller.rb @@ -125,7 +125,9 @@ module Administrateurs end if procedure.routee? - groupe_instructeur.instructeurs << instructeurs + instructeurs.each do |instructeur| + groupe_instructeur.add(instructeur) + end GroupeInstructeurMailer .add_instructeurs(groupe_instructeur, instructeurs, current_user.email) @@ -139,7 +141,9 @@ module Administrateurs else if instructeurs.present? - procedure.defaut_groupe_instructeur.instructeurs << instructeurs + instructeurs.each do |instructeur| + procedure.defaut_groupe_instructeur.add(instructeur) + end flash[:notice] = "Les instructeurs ont bien été affectés à la démarche" end end @@ -158,7 +162,7 @@ module Administrateurs else instructeur = Instructeur.find(instructeur_id) if procedure.routee? - if instructeur.remove_from_groupe_instructeur(groupe_instructeur) + if groupe_instructeur.remove(instructeur) flash[:notice] = "L’instructeur « #{instructeur.email} » a été retiré du groupe." GroupeInstructeurMailer .remove_instructeur(groupe_instructeur, instructeur, current_user.email) @@ -167,7 +171,7 @@ module Administrateurs flash[:alert] = "L’instructeur « #{instructeur.email} » n’est pas dans le groupe." end else - if instructeur.remove_from_groupe_instructeur(procedure.defaut_groupe_instructeur) + if procedure.defaut_groupe_instructeur.remove(instructeur) flash[:notice] = "L’instructeur a bien été désaffecté de la démarche" else flash[:alert] = "L’instructeur n’est pas affecté à la démarche" diff --git a/app/controllers/instructeurs/groupe_instructeurs_controller.rb b/app/controllers/instructeurs/groupe_instructeurs_controller.rb index c27457a49..1e4af490b 100644 --- a/app/controllers/instructeurs/groupe_instructeurs_controller.rb +++ b/app/controllers/instructeurs/groupe_instructeurs_controller.rb @@ -20,7 +20,7 @@ module Instructeurs if groupe_instructeur.instructeurs.include?(instructeur) flash[:alert] = "L’instructeur « #{instructeur_email} » est déjà dans le groupe." else - groupe_instructeur.instructeurs << instructeur + groupe_instructeur.add(instructeur) flash[:notice] = "L’instructeur « #{instructeur_email} » a été affecté au groupe." GroupeInstructeurMailer .add_instructeurs(groupe_instructeur, [instructeur], current_user.email) @@ -35,7 +35,7 @@ module Instructeurs flash[:alert] = "Suppression impossible : il doit y avoir au moins un instructeur dans le groupe" else instructeur = Instructeur.find(instructeur_id) - if instructeur.remove_from_groupe_instructeur(groupe_instructeur) + if groupe_instructeur.remove(instructeur) flash[:notice] = "L’instructeur « #{instructeur.email} » a été retiré du groupe." GroupeInstructeurMailer .remove_instructeur(groupe_instructeur, instructeur, current_user.email) diff --git a/app/models/groupe_instructeur.rb b/app/models/groupe_instructeur.rb index 899ab14ed..7c8775e82 100644 --- a/app/models/groupe_instructeur.rb +++ b/app/models/groupe_instructeur.rb @@ -24,4 +24,21 @@ class GroupeInstructeur < ApplicationRecord scope :without_group, -> (group) { where.not(id: group) } scope :for_api_v2, -> { includes(procedure: [:administrateurs]) } + + def add(instructeur) + return if in?(instructeur.groupe_instructeurs) + + default_notification_settings = instructeur.notification_settings(procedure_id) + instructeur.assign_to.create(groupe_instructeur: self, **default_notification_settings) + end + + def remove(instructeur) + return if !in?(instructeur.groupe_instructeurs) + + instructeur.groupe_instructeurs.destroy(self) + instructeur.follows + .joins(:dossier) + .where(dossiers: { groupe_instructeur: self }) + .update_all(unfollowed_at: Time.zone.now) + end end diff --git a/app/models/instructeur.rb b/app/models/instructeur.rb index 58fa00620..43ea54db6 100644 --- a/app/models/instructeur.rb +++ b/app/models/instructeur.rb @@ -81,14 +81,13 @@ class Instructeur < ApplicationRecord end end - def remove_from_groupe_instructeur(groupe_instructeur) - if groupe_instructeur.in?(groupe_instructeurs) - groupe_instructeurs.destroy(groupe_instructeur) - follows - .joins(:dossier) - .where(dossiers: { groupe_instructeur: groupe_instructeur }) - .update_all(unfollowed_at: Time.zone.now) - end + NOTIFICATION_SETTINGS = [:daily_email_notifications_enabled, :instant_email_dossier_notifications_enabled, :instant_email_message_notifications_enabled, :weekly_email_notifications_enabled] + + def notification_settings(procedure_id) + assign_to + .joins(:groupe_instructeur) + .find_by(groupe_instructeurs: { procedure_id: procedure_id }) + &.slice(*NOTIFICATION_SETTINGS) || {} end def last_week_overview diff --git a/spec/models/groupe_instructeur_spec.rb b/spec/models/groupe_instructeur_spec.rb index 6dc6d753d..9f00ccf7d 100644 --- a/spec/models/groupe_instructeur_spec.rb +++ b/spec/models/groupe_instructeur_spec.rb @@ -1,5 +1,17 @@ describe GroupeInstructeur, type: :model do - let(:procedure) { create(:procedure) } + let(:admin) { create :administrateur } + let(:procedure) { create :procedure, :published, administrateur: admin } + let(:procedure_2) { create :procedure, :published, administrateur: admin } + let(:procedure_3) { create :procedure, :published, administrateur: admin } + let(:instructeur) { create :instructeur, administrateurs: [admin] } + let(:procedure_assign) { assign(procedure) } + + before do + procedure_assign + assign(procedure_2) + procedure_3 + end + subject { GroupeInstructeur.new(label: label, procedure: procedure) } context 'with no label provided' do @@ -33,4 +45,62 @@ describe GroupeInstructeur, type: :model do it { is_expected.to be_invalid } end + + describe "#add" do + let(:another_groupe_instructeur) { create(:groupe_instructeur, procedure: procedure) } + + subject { another_groupe_instructeur.add(instructeur) } + + it 'adds the instructeur to the groupe instructeur' do + subject + expect(another_groupe_instructeur.reload.instructeurs).to include(instructeur) + end + + context 'when joining another groupe instructeur on the same procedure' do + before do + procedure_assign.update(daily_email_notifications_enabled: true) + subject + end + + it 'copies notifications settings from a previous group' do + expect(instructeur.assign_to.last.daily_email_notifications_enabled).to be_truthy + end + end + end + + describe "#remove" do + subject { procedure_to_remove.defaut_groupe_instructeur.remove(instructeur) } + + context "with an assigned procedure" do + let(:procedure_to_remove) { procedure } + let!(:procedure_presentation) { procedure_assign.procedure_presentation } + + it { is_expected.to be_truthy } + + describe "consequences" do + before do + procedure_assign.build_procedure_presentation + procedure_assign.save + subject + end + + it "removes the assign_to and procedure_presentation" do + expect(AssignTo.where(id: procedure_assign).count).to eq(0) + expect(ProcedurePresentation.where(assign_to_id: procedure_assign.id).count).to eq(0) + end + end + end + + context "with an already unassigned procedure" do + let(:procedure_to_remove) { procedure_3 } + + it { is_expected.to be_falsey } + 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 + end end diff --git a/spec/models/instructeur_spec.rb b/spec/models/instructeur_spec.rb index 7cdfd8304..bc0c87d94 100644 --- a/spec/models/instructeur_spec.rb +++ b/spec/models/instructeur_spec.rb @@ -1,13 +1,15 @@ describe Instructeur, type: :model do let(:admin) { create :administrateur } - let!(:procedure) { create :procedure, :published, administrateur: admin } - let!(:procedure_2) { create :procedure, :published, administrateur: admin } - let!(:procedure_3) { create :procedure, :published, administrateur: admin } + let(:procedure) { create :procedure, :published, administrateur: admin } + let(:procedure_2) { create :procedure, :published, administrateur: admin } + let(:procedure_3) { create :procedure, :published, administrateur: admin } let(:instructeur) { create :instructeur, administrateurs: [admin] } - let!(:procedure_assign) { assign(procedure) } + let(:procedure_assign) { assign(procedure) } before do + procedure_assign assign(procedure_2) + procedure_3 end describe 'follow' do @@ -84,36 +86,6 @@ describe Instructeur, type: :model do end end - describe "#remove_from_groupe_instructeur" do - subject { instructeur.remove_from_groupe_instructeur(procedure_to_remove.defaut_groupe_instructeur) } - - context "with an assigned procedure" do - let(:procedure_to_remove) { procedure } - let!(:procedure_presentation) { procedure_assign.procedure_presentation } - - it { is_expected.to be_truthy } - - describe "consequences" do - before do - procedure_assign.build_procedure_presentation - procedure_assign.save - subject - end - - it "removes the assign_to and procedure_presentation" do - expect(AssignTo.where(id: procedure_assign).count).to eq(0) - expect(ProcedurePresentation.where(assign_to_id: procedure_assign.id).count).to eq(0) - end - end - end - - context "with an already unassigned procedure" do - let(:procedure_to_remove) { procedure_3 } - - it { is_expected.to be_falsey } - end - end - describe 'last_week_overview' do let!(:instructeur2) { create(:instructeur) } subject { instructeur2.last_week_overview }