diff --git a/app/controllers/administrateurs/groupe_instructeurs_controller.rb b/app/controllers/administrateurs/groupe_instructeurs_controller.rb index 707f77cc7..399f5997e 100644 --- a/app/controllers/administrateurs/groupe_instructeurs_controller.rb +++ b/app/controllers/administrateurs/groupe_instructeurs_controller.rb @@ -40,7 +40,7 @@ module Administrateurs @instructeurs = paginated_instructeurs @groupes_instructeurs = paginated_groupe_instructeurs - flash[:alert] = "le nom « #{@groupe_instructeur.label} » est déjà pris par un autre groupe." + flash[:alert] = @groupe_instructeur.errors.full_messages render :index end end @@ -56,7 +56,7 @@ module Administrateurs @instructeurs = paginated_instructeurs @available_instructeur_emails = available_instructeur_emails - flash[:alert] = @groupe_instructeur.errors.full_messages.to_sentence + flash[:alert] = @groupe_instructeur.errors.full_messages render :show end end @@ -64,7 +64,7 @@ module Administrateurs def destroy @groupe_instructeur = groupe_instructeur - if !@groupe_instructeur.dossiers.empty? + if @groupe_instructeur.dossiers.present? flash[:alert] = "Impossible de supprimer un groupe avec des dossiers. Il faut le réaffecter avant" elsif procedure.groupe_instructeurs.one? flash[:alert] = "Suppression impossible : il doit y avoir au moins un groupe instructeur sur chaque procédure" @@ -109,47 +109,30 @@ module Administrateurs def add_instructeur emails = params['emails'].presence || [].to_json - emails = JSON.parse(emails).map(&:strip).map(&:downcase) + emails = JSON.parse(emails) - correct_emails, bad_emails = emails - .partition { |email| URI::MailTo::EMAIL_REGEXP.match?(email) } + instructeurs, invalid_emails = Instructeur.find_or_invite(emails:, groupe_instructeur:) - if bad_emails.present? + if invalid_emails.present? flash[:alert] = t('.wrong_address', - count: bad_emails.count, - value: bad_emails.join(', ')) + count: invalid_emails.size, + emails: invalid_emails.join(', ')) end - email_to_adds = correct_emails - groupe_instructeur.instructeurs.map(&:email) - - if email_to_adds.present? - instructeurs = email_to_adds.map do |instructeur_email| - Instructeur.by_email(instructeur_email) || - create_instructeur(instructeur_email) - end - - if procedure.routing_enabled? - instructeurs.each do |instructeur| - groupe_instructeur.add(instructeur) - end + if instructeurs.present? + instructeurs.each { groupe_instructeur.add(_1) } + flash[:notice] = if procedure.routing_enabled? GroupeInstructeurMailer - .add_instructeurs(groupe_instructeur, instructeurs, current_user.email) + .add_instructeurs(groupe_instructeur, instructeurs, current_administrateur.email) .deliver_later - flash[:notice] = t('.assignment', - count: email_to_adds.count, - value: email_to_adds.join(', '), + t('.assignment', + count: instructeurs.size, + emails: instructeurs.map(&:email).join(', '), groupe: groupe_instructeur.label) - else - - if instructeurs.present? - instructeurs.each do |instructeur| - procedure.defaut_groupe_instructeur.add(instructeur) - end - flash[:notice] = "Les instructeurs ont bien été affectés à la démarche" - end + "Les instructeurs ont bien été affectés à la démarche" end end @@ -164,21 +147,27 @@ module Administrateurs if groupe_instructeur.instructeurs.one? flash[:alert] = "Suppression impossible : il doit y avoir au moins un instructeur dans le groupe" else - instructeur = Instructeur.find(instructeur_id) - if procedure.routing_enabled? - if groupe_instructeur.remove(instructeur) - flash[:notice] = "L’instructeur « #{instructeur.email} » a été retiré du groupe." + instructeur = groupe_instructeur.instructeurs.find_by(id: instructeur_id) + + if groupe_instructeur.remove(instructeur) + flash[:notice] = if procedure.routing_enabled? GroupeInstructeurMailer - .remove_instructeur(groupe_instructeur, instructeur, current_user.email) + .remove_instructeurs(groupe_instructeur, [instructeur], current_administrateur.email) .deliver_later + + "L’instructeur « #{instructeur.email} » a été retiré du groupe." else - flash[:alert] = "L’instructeur « #{instructeur.email} » n’est pas dans le groupe." + "L’instructeur a bien été désaffecté de la démarche" end else - if procedure.defaut_groupe_instructeur.remove(instructeur) - flash[:notice] = "L’instructeur a bien été désaffecté de la démarche" + flash[:alert] = if procedure.routing_enabled? + if instructeur.present? + "L’instructeur « #{instructeur.email} » n’est pas dans le groupe." + else + "L’instructeur n’est pas dans le groupe." + end else - flash[:alert] = "L’instructeur n’est pas affecté à la démarche" + "L’instructeur n’est pas affecté à la démarche" end end end @@ -258,16 +247,6 @@ module Administrateurs private - def create_instructeur(email) - user = User.create_or_promote_to_instructeur( - email, - SecureRandom.hex, - administrateurs: [current_administrateur] - ) - user.invite! - user.instructeur - end - def procedure current_administrateur .procedures diff --git a/app/mailers/groupe_instructeur_mailer.rb b/app/mailers/groupe_instructeur_mailer.rb index e045169bf..e0d0712ef 100644 --- a/app/mailers/groupe_instructeur_mailer.rb +++ b/app/mailers/groupe_instructeur_mailer.rb @@ -12,6 +12,17 @@ class GroupeInstructeurMailer < ApplicationMailer mail(bcc: emails, subject: subject) end + def remove_instructeurs(group, removed_instructeurs, current_instructeur_email) + @removed_instructeur_emails = removed_instructeurs.map(&:email) + @group = group + @current_instructeur_email = current_instructeur_email + + subject = "Suppression d’un instructeur dans le groupe \"#{group.label}\"" + + emails = @group.instructeurs.map(&:email) + mail(bcc: emails, subject: subject) + end + def remove_instructeur(group, instructeur, current_instructeur_email) @email = instructeur.email @group = group diff --git a/app/models/groupe_instructeur.rb b/app/models/groupe_instructeur.rb index 1ddda5c40..aef00f7dc 100644 --- a/app/models/groupe_instructeur.rb +++ b/app/models/groupe_instructeur.rb @@ -33,6 +33,7 @@ class GroupeInstructeur < ApplicationRecord scope :closed, -> { where(closed: true) } def add(instructeur) + return if instructeur.nil? return if in?(instructeur.groupe_instructeurs) default_notification_settings = instructeur.notification_settings(procedure_id) @@ -40,6 +41,7 @@ class GroupeInstructeur < ApplicationRecord end def remove(instructeur) + return if instructeur.nil? return if !in?(instructeur.groupe_instructeurs) instructeur.groupe_instructeurs.destroy(self) diff --git a/app/models/instructeur.rb b/app/models/instructeur.rb index 170df6da7..362870a3a 100644 --- a/app/models/instructeur.rb +++ b/app/models/instructeur.rb @@ -49,7 +49,45 @@ class Instructeur < ApplicationRecord default_scope { eager_load(:user) } def self.by_email(email) - Instructeur.eager_load(:user).find_by(users: { email: email }) + find_by(users: { email: email }) + end + + def self.find_or_invite(emails: [], ids: [], groupe_instructeur:) + valid_emails, invalid_emails = emails + .map(&:strip) + .map(&:downcase) + .partition { URI::MailTo::EMAIL_REGEXP.match?(_1) } + valid_ids = ids.map { _1.is_a?(String) ? id_from_typed_id(_1) : _1 } + + instructeurs_by_ids = where(id: valid_ids) + instructeurs_by_emails = where(users: { email: valid_emails }) + + instructeurs = if valid_ids.present? && valid_emails.present? + instructeurs_by_ids.or(instructeurs_by_emails).distinct(:id) + elsif valid_emails.present? + instructeurs_by_emails + else + instructeurs_by_ids + end + + not_found_emails = valid_emails - instructeurs.map(&:email) + + # Send invitations to users without account + if not_found_emails.present? + administrateurs = groupe_instructeur.procedure.administrateurs + instructeurs += not_found_emails.map { invite(_1, administrateurs) } + end + + # We dont't want to assign a user to a groupe_instructeur if they are already assigned to it + instructeurs -= groupe_instructeur.instructeurs + + [instructeurs, invalid_emails] + end + + def self.invite(email, administrateurs) + user = User.create_or_promote_to_instructeur(email, SecureRandom.hex, administrateurs:) + user.invite! + user.instructeur end def email diff --git a/app/views/groupe_instructeur_mailer/add_instructeurs.html.haml b/app/views/groupe_instructeur_mailer/add_instructeurs.html.haml index 31a5f3c10..ee899f212 100644 --- a/app/views/groupe_instructeur_mailer/add_instructeurs.html.haml +++ b/app/views/groupe_instructeur_mailer/add_instructeurs.html.haml @@ -2,7 +2,7 @@ Bonjour, %p - #{t('administrateurs.groupe_instructeurs.add_instructeur.assignment', count: @new_instructeur_emails.count, value: @new_instructeur_emails.join(', '), groupe: @group.label).chomp('.')} par « #{@current_instructeur_email} », en charge de la démarche « #{@group.procedure.libelle} ». + = t('administrateurs.groupe_instructeurs.add_instructeur.email_body', count: @new_instructeur_emails.size, emails: @new_instructeur_emails.join(', '), groupe: @group.label, email: @current_instructeur_email, procedure: @group.procedure.libelle) %p Cliquez sur le lien ci-dessous pour voir la liste des instructeurs de ce groupe : diff --git a/app/views/groupe_instructeur_mailer/remove_instructeur.html.haml b/app/views/groupe_instructeur_mailer/remove_instructeur.html.haml index 6a8e1cc94..9e7770919 100644 --- a/app/views/groupe_instructeur_mailer/remove_instructeur.html.haml +++ b/app/views/groupe_instructeur_mailer/remove_instructeur.html.haml @@ -2,7 +2,7 @@ Bonjour, %p - L’instructeur « #{@email} » a été retiré du groupe « #{@group.label} » par « #{@current_instructeur_email} », en charge de la démarche « #{@group.procedure.libelle} ». + = t('administrateurs.groupe_instructeurs.remove_instructeur.email_body', count: 1, emails: @email, groupe: @group.label, email: @current_instructeur_email, procedure: @group.procedure.libelle) %p Cliquez sur le lien ci-dessous pour voir la liste des instructeurs de ce groupe : diff --git a/app/views/groupe_instructeur_mailer/remove_instructeurs.html.haml b/app/views/groupe_instructeur_mailer/remove_instructeurs.html.haml new file mode 100644 index 000000000..d5a73e165 --- /dev/null +++ b/app/views/groupe_instructeur_mailer/remove_instructeurs.html.haml @@ -0,0 +1,11 @@ +%p + Bonjour, + +%p + = t('administrateurs.groupe_instructeurs.remove_instructeur.email_body', count: @removed_instructeur_emails.size, emails: @removed_instructeur_emails.join(', '), groupe: @group.label, email: @current_instructeur_email, procedure: @group.procedure.libelle) + +%p + Cliquez sur le lien ci-dessous pour voir la liste des instructeurs de ce groupe : + = link_to(@group.label, admin_procedure_groupe_instructeur_url(@group.procedure, @group)) + += render partial: "layouts/mailers/signature" diff --git a/config/locales/views/administrateurs/groupe_instructeurs/en.yml b/config/locales/views/administrateurs/groupe_instructeurs/en.yml index 3f7d89d42..c9c85e230 100644 --- a/config/locales/views/administrateurs/groupe_instructeurs/en.yml +++ b/config/locales/views/administrateurs/groupe_instructeurs/en.yml @@ -11,11 +11,18 @@ en: instructors_group: Group of instructors add_instructeur: wrong_address: - one: "%{value} is not a valid email address" - other: "%{value} are not valid email addresses" + one: "%{emails} is not a valid email address" + other: "%{emails} are not valid email addresses" assignment: - one: "The instructor %{value} was assigned to the group « %{groupe} »." - other: "The instructors %{value} were assigned to the group « %{groupe} »." + one: "The instructor %{emails} was assigned to the group « %{groupe} »." + other: "The instructors %{emails} were assigned to the group « %{groupe} »." + email_body: + one: "The instructor %{emails} was assigned to the group « %{groupe} » by « %{email} », in charge of procedure « %{procedure} »." + other: "The instructors %{emails} were assigned to the group « %{groupe} » by « %{email} », in charge of procedure « %{procedure} »." + remove_instructeur: + email_body: + one: "The instructor %{emails} was removed from the group « %{groupe} » by « %{email} », in charge of procedure « %{procedure} »." + other: "The instructors %{emails} were removed from the group « %{groupe} » by « %{email} », in charge of procedure « %{procedure} »." reaffecter_dossiers: existing_groupe: one: "%{count} group exist" diff --git a/config/locales/views/administrateurs/groupe_instructeurs/fr.yml b/config/locales/views/administrateurs/groupe_instructeurs/fr.yml index f92ac1567..4f38d70dc 100644 --- a/config/locales/views/administrateurs/groupe_instructeurs/fr.yml +++ b/config/locales/views/administrateurs/groupe_instructeurs/fr.yml @@ -17,11 +17,18 @@ fr: other: "%{count} groupes existent" add_instructeur: wrong_address: - one: "%{value} n’est pas une adresse email valide" - other: "%{value} ne sont pas des adresses emails valides" + one: "%{emails} n’est pas une adresse email valide" + other: "%{emails} ne sont pas des adresses emails valides" assignment: - one: "L’instructeur %{value} a été affecté au groupe « %{groupe} »." - other: "Les instructeurs %{value} ont été affectés au groupe « %{groupe} »." + one: "L’instructeur %{emails} a été affecté au groupe « %{groupe} »." + other: "Les instructeurs %{emails} ont été affectés au groupe « %{groupe} »." + email_body: + one: "L’instructeur %{emails} a été affecté au groupe « %{groupe} » par « %{email} », en charge de la démarche « %{procedure} »." + other: "Les instructeurs %{emails} ont été affectés au groupe « %{groupe} » par « %{email} », en charge de la démarche « %{procedure} »." + remove_instructeur: + email_body: + one: "L’instructeur %{emails} a été retiré du groupe « %{groupe} » par « %{email} », en charge de la démarche « %{procedure} »." + other: "Les instructeurs %{emails} ont été retirés du groupe « %{groupe} » par « %{email} », en charge de la démarche « %{procedure} »." reaffecter_dossiers: existing_groupe: one: "%{count} groupe existe" diff --git a/spec/controllers/administrateurs/groupe_instructeurs_controller_spec.rb b/spec/controllers/administrateurs/groupe_instructeurs_controller_spec.rb index 13905cbe6..e0facf03b 100644 --- a/spec/controllers/administrateurs/groupe_instructeurs_controller_spec.rb +++ b/spec/controllers/administrateurs/groupe_instructeurs_controller_spec.rb @@ -206,7 +206,7 @@ describe Administrateurs::GroupeInstructeursController, type: :controller do it { expect(response).to render_template(:show) } it { expect(gi_1_1.label).not_to eq(new_name) } it { expect(gi_1_1.closed).to eq(false) } - it { expect(flash.alert).to eq('Il doit y avoir au moins un groupe instructeur actif sur chaque démarche') } + it { expect(flash.alert).to eq(['Il doit y avoir au moins un groupe instructeur actif sur chaque démarche']) } end context 'when the name is already taken' do @@ -214,7 +214,7 @@ describe Administrateurs::GroupeInstructeursController, type: :controller do let(:new_name) { gi_1_2.label } it { expect(gi_1_1.label).not_to eq(new_name) } - it { expect(flash.alert).to eq('Le libellé est déjà utilisé(e)') } + it { expect(flash.alert).to eq(['Le libellé est déjà utilisé(e)']) } end end