From d7811fca402de896d3658bc03ab1a8a36de7fa41 Mon Sep 17 00:00:00 2001 From: Mathieu Magnin Date: Tue, 10 Sep 2024 16:15:30 +0200 Subject: [PATCH 1/3] [#10751] Only send the first invitation mail to the instructeur if email is not verified --- .../groupe_instructeurs_controller.rb | 3 ++- .../groupe_instructeurs_controller_spec.rb | 26 +++++++++++++++++++ spec/factories/instructeur.rb | 8 +++++- 3 files changed, 35 insertions(+), 2 deletions(-) diff --git a/app/controllers/instructeurs/groupe_instructeurs_controller.rb b/app/controllers/instructeurs/groupe_instructeurs_controller.rb index 7fbf5dbd2..3c6fccf22 100644 --- a/app/controllers/instructeurs/groupe_instructeurs_controller.rb +++ b/app/controllers/instructeurs/groupe_instructeurs_controller.rb @@ -50,9 +50,10 @@ module Instructeurs GroupeInstructeurMailer .notify_added_instructeurs(groupe_instructeur, [instructeur], current_user.email) .deliver_later - else + elsif instructeur.previously_new_record? InstructeurMailer.confirm_and_notify_added_instructeur(instructeur, groupe_instructeur, current_user.email).deliver_later end + # else instructeur already exists and email is not verified, so do not spam them end redirect_to instructeur_groupe_path(procedure, groupe_instructeur) diff --git a/spec/controllers/instructeurs/groupe_instructeurs_controller_spec.rb b/spec/controllers/instructeurs/groupe_instructeurs_controller_spec.rb index 7e52d38e5..5187da647 100644 --- a/spec/controllers/instructeurs/groupe_instructeurs_controller_spec.rb +++ b/spec/controllers/instructeurs/groupe_instructeurs_controller_spec.rb @@ -78,6 +78,11 @@ describe Instructeurs::GroupeInstructeursController, type: :controller do end describe '#add_instructeur' do + before do + allow(InstructeurMailer).to receive(:confirm_and_notify_added_instructeur).and_call_original + allow(GroupeInstructeurMailer).to receive(:notify_added_instructeurs).and_call_original + end + subject do post :add_instructeur, params: { @@ -95,6 +100,23 @@ describe Instructeurs::GroupeInstructeursController, type: :controller do expect(gi_1_2.instructeurs.map(&:email)).to include(new_instructeur_email) expect(flash.notice).to be_present expect(response).to redirect_to(instructeur_groupe_path(procedure, gi_1_2)) + expect(InstructeurMailer).to have_received(:confirm_and_notify_added_instructeur).with(instance_of(Instructeur), gi_1_2, anything) + expect(GroupeInstructeurMailer).not_to have_received(:notify_added_instructeurs) + end + end + + context 'of an instructeur already known and with email verified' do + let(:known_instructeur) { create(:instructeur, :email_verified) } + let(:new_instructeur_email) { known_instructeur.email } + + before { subject } + + it "works" do + expect(gi_1_2.instructeurs.map(&:email)).to include(new_instructeur_email) + expect(flash.notice).to be_present + expect(response).to redirect_to(instructeur_groupe_path(procedure, gi_1_2)) + expect(InstructeurMailer).not_to have_received(:confirm_and_notify_added_instructeur) + expect(GroupeInstructeurMailer).to have_received(:notify_added_instructeurs) end end @@ -105,6 +127,8 @@ describe Instructeurs::GroupeInstructeursController, type: :controller do it "works" do expect(flash.alert).to be_present expect(response).to redirect_to(instructeur_groupe_path(procedure, gi_1_2)) + expect(InstructeurMailer).not_to have_received(:confirm_and_notify_added_instructeur) + expect(GroupeInstructeurMailer).not_to have_received(:notify_added_instructeurs) end end @@ -114,6 +138,8 @@ describe Instructeurs::GroupeInstructeursController, type: :controller do it "works" do expect { subject }.not_to enqueue_email expect(flash.alert).to include(new_instructeur_email) + expect(InstructeurMailer).not_to have_received(:confirm_and_notify_added_instructeur) + expect(GroupeInstructeurMailer).not_to have_received(:notify_added_instructeurs) end end end diff --git a/spec/factories/instructeur.rb b/spec/factories/instructeur.rb index a26627619..31d9e8b02 100644 --- a/spec/factories/instructeur.rb +++ b/spec/factories/instructeur.rb @@ -10,7 +10,13 @@ FactoryBot.define do transient do email { generate(:instructeur_email) } - password { 'somethingverycomplated!' } + password { '{my-%s3cure[]-p4$$w0rd' } + end + + trait :email_verified do + after(:create) do |instructeur| + instructeur.user.update(email_verified_at: Time.zone.now) + end end trait :with_agent_connect_information do From 790177c758d448491f1f8e3c9be2b58f9e9ffb58 Mon Sep 17 00:00:00 2001 From: Mathieu Magnin Date: Tue, 10 Sep 2024 17:41:55 +0200 Subject: [PATCH 2/3] [#10751] Only send the first invitation mail to the instructeur if email is not verified (from admin controller) --- .../administrateurs/groupe_instructeurs_controller.rb | 6 ++++-- .../groupe_instructeurs_controller_spec.rb | 10 +++++++++- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/app/controllers/administrateurs/groupe_instructeurs_controller.rb b/app/controllers/administrateurs/groupe_instructeurs_controller.rb index 8bc9fd900..630f5fb1a 100644 --- a/app/controllers/administrateurs/groupe_instructeurs_controller.rb +++ b/app/controllers/administrateurs/groupe_instructeurs_controller.rb @@ -252,9 +252,11 @@ module Administrateurs "Les instructeurs ont bien été affectés à la démarche" end - known_instructeurs, new_instructeurs = instructeurs.partition { |instructeur| instructeur.user.email_verified_at } + known_instructeurs, not_verified_instructeurs = instructeurs.partition { |instructeur| instructeur.user.email_verified_at } - new_instructeurs.each { InstructeurMailer.confirm_and_notify_added_instructeur(_1, groupe_instructeur, current_administrateur.email).deliver_later } + not_verified_instructeurs.filter(&:previously_new_record?).each do + InstructeurMailer.confirm_and_notify_added_instructeur(_1, groupe_instructeur, current_administrateur.email).deliver_later + end if known_instructeurs.present? GroupeInstructeurMailer diff --git a/spec/controllers/administrateurs/groupe_instructeurs_controller_spec.rb b/spec/controllers/administrateurs/groupe_instructeurs_controller_spec.rb index 1269856da..b69a93f2b 100644 --- a/spec/controllers/administrateurs/groupe_instructeurs_controller_spec.rb +++ b/spec/controllers/administrateurs/groupe_instructeurs_controller_spec.rb @@ -387,7 +387,8 @@ describe Administrateurs::GroupeInstructeursController, type: :controller do context 'of news instructeurs' do let!(:user_email_verified) { create(:user, :with_email_verified) } let!(:instructeur_email_verified) { create(:instructeur, user: user_email_verified) } - let(:new_instructeur_emails) { ['new_i1@gmail.com', 'new_i2@gmail.com', instructeur_email_verified.email] } + let!(:instructeur_email_not_verified) { create(:instructeur, user: create(:user)) } + let(:new_instructeur_emails) { ['new_i1@gmail.com', 'new_i2@gmail.com', instructeur_email_verified.email, instructeur_email_not_verified.email] } before do allow(GroupeInstructeurMailer).to receive(:notify_added_instructeurs) @@ -397,6 +398,7 @@ describe Administrateurs::GroupeInstructeursController, type: :controller do .and_return(double(deliver_later: true)) do_request end + it 'validates changes and responses' do expect(gi_1_2.instructeurs.pluck(:email)).to include(*new_instructeur_emails) expect(flash.notice).to be_present @@ -421,6 +423,12 @@ describe Administrateurs::GroupeInstructeursController, type: :controller do gi_1_2, admin.email ) + + expect(InstructeurMailer).not_to have_received(:confirm_and_notify_added_instructeur).with( + instructeur_email_not_verified, + gi_1_2, + admin.email + ) end end From a99fae168c1de3d77602552f737e946d2f5538b4 Mon Sep 17 00:00:00 2001 From: Mathieu Magnin Date: Wed, 11 Sep 2024 18:42:29 +0200 Subject: [PATCH 3/3] [#10751] Allow sending a new invitation if the user's previous one has expired --- .../groupe_instructeurs_controller.rb | 2 +- .../groupe_instructeurs_controller.rb | 2 +- app/models/instructeur.rb | 5 +++++ .../groupe_instructeurs_controller_spec.rb | 11 +++++++++-- .../groupe_instructeurs_controller_spec.rb | 15 +++++++++++++++ 5 files changed, 31 insertions(+), 4 deletions(-) diff --git a/app/controllers/administrateurs/groupe_instructeurs_controller.rb b/app/controllers/administrateurs/groupe_instructeurs_controller.rb index 630f5fb1a..ec7011dd6 100644 --- a/app/controllers/administrateurs/groupe_instructeurs_controller.rb +++ b/app/controllers/administrateurs/groupe_instructeurs_controller.rb @@ -254,7 +254,7 @@ module Administrateurs known_instructeurs, not_verified_instructeurs = instructeurs.partition { |instructeur| instructeur.user.email_verified_at } - not_verified_instructeurs.filter(&:previously_new_record?).each do + not_verified_instructeurs.filter(&:should_receive_email_activation?).each do InstructeurMailer.confirm_and_notify_added_instructeur(_1, groupe_instructeur, current_administrateur.email).deliver_later end diff --git a/app/controllers/instructeurs/groupe_instructeurs_controller.rb b/app/controllers/instructeurs/groupe_instructeurs_controller.rb index 3c6fccf22..2d3771e18 100644 --- a/app/controllers/instructeurs/groupe_instructeurs_controller.rb +++ b/app/controllers/instructeurs/groupe_instructeurs_controller.rb @@ -50,7 +50,7 @@ module Instructeurs GroupeInstructeurMailer .notify_added_instructeurs(groupe_instructeur, [instructeur], current_user.email) .deliver_later - elsif instructeur.previously_new_record? + elsif instructeur.should_receive_email_activation? InstructeurMailer.confirm_and_notify_added_instructeur(instructeur, groupe_instructeur, current_user.email).deliver_later end # else instructeur already exists and email is not verified, so do not spam them diff --git a/app/models/instructeur.rb b/app/models/instructeur.rb index 77e266017..8d54f0baf 100644 --- a/app/models/instructeur.rb +++ b/app/models/instructeur.rb @@ -214,6 +214,11 @@ class Instructeur < ApplicationRecord trusted_device_token&.token_young? end + def should_receive_email_activation? + # if was recently created or received an activation email more than 7 days ago + previously_new_record? || user.reset_password_sent_at.nil? || user.reset_password_sent_at < Devise.reset_password_within.ago + end + def can_be_deleted? user.administrateur.nil? && procedures.all? { |p| p.defaut_groupe_instructeur.instructeurs.count > 1 } end diff --git a/spec/controllers/administrateurs/groupe_instructeurs_controller_spec.rb b/spec/controllers/administrateurs/groupe_instructeurs_controller_spec.rb index b69a93f2b..ac89dd43d 100644 --- a/spec/controllers/administrateurs/groupe_instructeurs_controller_spec.rb +++ b/spec/controllers/administrateurs/groupe_instructeurs_controller_spec.rb @@ -387,8 +387,9 @@ describe Administrateurs::GroupeInstructeursController, type: :controller do context 'of news instructeurs' do let!(:user_email_verified) { create(:user, :with_email_verified) } let!(:instructeur_email_verified) { create(:instructeur, user: user_email_verified) } - let!(:instructeur_email_not_verified) { create(:instructeur, user: create(:user)) } - let(:new_instructeur_emails) { ['new_i1@gmail.com', 'new_i2@gmail.com', instructeur_email_verified.email, instructeur_email_not_verified.email] } + let!(:instructeur_email_not_verified) { create(:instructeur, user: create(:user, { reset_password_sent_at: 1.day.ago })) } + let!(:instructeur_email_not_verified_but_received_invitation_long_time_ago) { create(:instructeur, user: create(:user, { reset_password_sent_at: 10.days.ago })) } + let(:new_instructeur_emails) { ['new_i1@gmail.com', 'new_i2@gmail.com', instructeur_email_verified.email, instructeur_email_not_verified.email, instructeur_email_not_verified_but_received_invitation_long_time_ago.email] } before do allow(GroupeInstructeurMailer).to receive(:notify_added_instructeurs) @@ -429,6 +430,12 @@ describe Administrateurs::GroupeInstructeursController, type: :controller do gi_1_2, admin.email ) + + expect(InstructeurMailer).to have_received(:confirm_and_notify_added_instructeur).with( + instructeur_email_not_verified_but_received_invitation_long_time_ago, + gi_1_2, + admin.email + ) end end diff --git a/spec/controllers/instructeurs/groupe_instructeurs_controller_spec.rb b/spec/controllers/instructeurs/groupe_instructeurs_controller_spec.rb index 5187da647..043d20071 100644 --- a/spec/controllers/instructeurs/groupe_instructeurs_controller_spec.rb +++ b/spec/controllers/instructeurs/groupe_instructeurs_controller_spec.rb @@ -120,6 +120,21 @@ describe Instructeurs::GroupeInstructeursController, type: :controller do end end + context 'of an instructeur already known that has received a invitation email long time ago' do + let(:known_instructeur) { create(:instructeur, user: create(:user, { reset_password_sent_at: 10.days.ago })) } + let(:new_instructeur_email) { known_instructeur.email } + + before { subject } + + it "works" do + expect(gi_1_2.instructeurs.map(&:email)).to include(new_instructeur_email) + expect(flash.notice).to be_present + expect(response).to redirect_to(instructeur_groupe_path(procedure, gi_1_2)) + expect(InstructeurMailer).to have_received(:confirm_and_notify_added_instructeur) + expect(GroupeInstructeurMailer).not_to have_received(:notify_added_instructeurs) + end + end + context 'of an instructeur already in the group' do let(:new_instructeur_email) { instructeur.email } before { subject }