Merge pull request #10772 from demarches-simplifiees/fix-10751

ETQ Instructeur, jamais connecté, je ne veux pas recevoir 10 mails si j'ai été ajouté à 10 groupes

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(&:should_receive_email_activation?).each do
+          InstructeurMailer.confirm_and_notify_added_instructeur(_1, groupe_instructeur, current_administrateur.email).deliver_later
+        end
 
         if known_instructeurs.present?
           GroupeInstructeurMailer

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.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
       end
 
       redirect_to instructeur_groupe_path(procedure, groupe_instructeur)

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

spec/controllers/administrateurs/groupe_instructeurs_controller_spec.rb

@@ -387,7 +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(: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, { 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)
@@ -397,6 +399,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 +424,18 @@ 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
+        )
+
+        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
 

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,38 @@ 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
+
+    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
 
@@ -105,6 +142,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 +153,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

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