From c8906d20b837f213df762a9d8c7b4760e1df2547 Mon Sep 17 00:00:00 2001 From: Lisa Durand Date: Wed, 3 Jul 2024 16:09:34 +0200 Subject: [PATCH 1/5] add reusable code in concern for typo detection email --- .../experts_procedures_controller.rb | 17 +--------------- .../concerns/email_sanitizable_concern.rb | 20 +++++++++++++++++++ 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/app/controllers/administrateurs/experts_procedures_controller.rb b/app/controllers/administrateurs/experts_procedures_controller.rb index 64abc5c38..d86f00ca2 100644 --- a/app/controllers/administrateurs/experts_procedures_controller.rb +++ b/app/controllers/administrateurs/experts_procedures_controller.rb @@ -10,16 +10,10 @@ module Administrateurs def create emails = params['emails'].presence || [].to_json - emails = JSON.parse(emails).map { EmailSanitizer.sanitize(_1) } - @maybe_typos, no_suggestions = emails - .map { |email| [email, EmailChecker.check(email:)[:suggestions]&.first] } - .partition { _1[1].present? } + emails = check_if_typo(emails) errors = Array.wrap(generate_emails_suggestions_message(@maybe_typos)) - emails = no_suggestions.map(&:first) - emails << EmailSanitizer.sanitize(params['final_email']) if params['final_email'].present? - valid_users, invalid_users = emails .map { |email| User.create_or_promote_to_expert(email, SecureRandom.hex) } .partition(&:valid?) @@ -78,14 +72,5 @@ module Administrateurs def expert_procedure_params params.require(:experts_procedure).permit(:allow_decision_access) end - - def generate_emails_suggestions_message(suggestions) - return if suggestions.empty? - - typo_list = suggestions.map(&:first).join(', ') - verification_link = view_context.link_to("vérifier l’orthographe", "#maybe_typos_errors") - - "Attention, nous pensons avoir identifié une faute de frappe dans les invitations : #{typo_list}. Veuillez #{verification_link} des invitations." - end end end diff --git a/app/models/concerns/email_sanitizable_concern.rb b/app/models/concerns/email_sanitizable_concern.rb index d143c7c7d..b8818b1d2 100644 --- a/app/models/concerns/email_sanitizable_concern.rb +++ b/app/models/concerns/email_sanitizable_concern.rb @@ -8,6 +8,26 @@ module EmailSanitizableConcern end end + def generate_emails_suggestions_message(suggestions) + return if suggestions.empty? + + typo_list = suggestions.map(&:first).join(', ') + verification_link = view_context.link_to("vérifier l’orthographe", "#maybe_typos_errors") + + "Attention, nous pensons avoir identifié une faute de frappe dans les invitations : #{typo_list}. Veuillez #{verification_link} des invitations." + end + + def check_if_typo(emails) + emails = JSON.parse(emails).map { EmailSanitizer.sanitize(_1) } + @maybe_typos, no_suggestions = emails + .map { |email| [email, EmailChecker.check(email:)[:suggestions]&.first] } + .partition { _1[1].present? } + + emails = no_suggestions.map(&:first) + emails << EmailSanitizer.sanitize(params['final_email']) if params['final_email'].present? + emails + end + class EmailSanitizer def self.sanitize(value) value.gsub(/[[:space:]]/, ' ').strip.downcase From f163c04da614994dbd36344be783708e5057a3d5 Mon Sep 17 00:00:00 2001 From: Lisa Durand Date: Wed, 3 Jul 2024 16:53:23 +0200 Subject: [PATCH 2/5] add typo detection and suggestion for admin adding instructeurs --- .../groupe_instructeurs_controller.rb | 23 ++++++++++++++----- .../_instructeurs.html.haml | 1 + 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/app/controllers/administrateurs/groupe_instructeurs_controller.rb b/app/controllers/administrateurs/groupe_instructeurs_controller.rb index 50a940690..90cb20245 100644 --- a/app/controllers/administrateurs/groupe_instructeurs_controller.rb +++ b/app/controllers/administrateurs/groupe_instructeurs_controller.rb @@ -1,6 +1,7 @@ module Administrateurs class GroupeInstructeursController < AdministrateurController include ActiveSupport::NumberHelper + include EmailSanitizableConcern include Logic include UninterlacePngConcern include GroupeInstructeursSignatureConcern @@ -219,18 +220,20 @@ module Administrateurs def add_instructeur emails = params['emails'].presence || [].to_json - emails = JSON.parse(emails).map { EmailSanitizableConcern::EmailSanitizer.sanitize(_1) } + + emails = check_if_typo(emails) + errors = Array.wrap(generate_emails_suggestions_message(@maybe_typos)) instructeurs, invalid_emails = groupe_instructeur.add_instructeurs(emails:) if invalid_emails.present? - flash[:alert] = t('.wrong_address', + errors += [t('.wrong_address', count: invalid_emails.size, - emails: invalid_emails.join(', ')) + emails: invalid_emails.join(', '))] end if instructeurs.present? - flash[:notice] = if procedure.routing_enabled? + flash.now[:notice] = if procedure.routing_enabled? t('.assignment', count: instructeurs.size, emails: instructeurs.map(&:email).join(', '), @@ -250,10 +253,18 @@ module Administrateurs end end + flash.now[:alert] = errors.join(". ") if !errors.empty? + + @procedure = procedure + @instructeurs = paginated_instructeurs + @available_instructeur_emails = available_instructeur_emails + if procedure.routing_enabled? - redirect_to admin_procedure_groupe_instructeur_path(procedure, groupe_instructeur) + @groupe_instructeur = groupe_instructeur + render :show else - redirect_to admin_procedure_groupe_instructeurs_path(procedure) + @groupes_instructeurs = paginated_groupe_instructeurs + render :index end end diff --git a/app/views/administrateurs/groupe_instructeurs/_instructeurs.html.haml b/app/views/administrateurs/groupe_instructeurs/_instructeurs.html.haml index 3e8dd7643..77470a328 100644 --- a/app/views/administrateurs/groupe_instructeurs/_instructeurs.html.haml +++ b/app/views/administrateurs/groupe_instructeurs/_instructeurs.html.haml @@ -1,5 +1,6 @@ .card + = render Procedure::InvitationWithTypoComponent.new(maybe_typos: @maybe_typos, url: add_instructeur_admin_procedure_groupe_instructeur_path(@procedure, groupe_instructeur.id), title: "Avant d'ajouter l'email, veuillez confirmer" ) .card-title Affectation des instructeurs = form_for :instructeur, url: { action: :add_instructeur, id: groupe_instructeur.id }, html: { class: 'form' } do |f| .instructeur-wrapper From 67be27e18f21ee40db4adfd94a9b0e576fb6474a Mon Sep 17 00:00:00 2001 From: Lisa Durand Date: Thu, 4 Jul 2024 13:40:01 +0200 Subject: [PATCH 3/5] add typo detection and suggestion for instructeur adding instructeur --- .../groupe_instructeurs_controller.rb | 26 ++++++++++++++----- .../groupe_instructeurs/show.html.haml | 1 + 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/app/controllers/instructeurs/groupe_instructeurs_controller.rb b/app/controllers/instructeurs/groupe_instructeurs_controller.rb index 3193d9455..d75254b5c 100644 --- a/app/controllers/instructeurs/groupe_instructeurs_controller.rb +++ b/app/controllers/instructeurs/groupe_instructeurs_controller.rb @@ -1,5 +1,6 @@ module Instructeurs class GroupeInstructeursController < InstructeurController + include EmailSanitizableConcern include UninterlacePngConcern include GroupeInstructeursSignatureConcern @@ -19,16 +20,29 @@ module Instructeurs end def add_instructeur - instructeur = Instructeur.by_email(instructeur_email) || - create_instructeur(instructeur_email) + email = instructeur_email.present? ? [instructeur_email].to_json : [].to_json + email = check_if_typo(email)&.first + errors = Array.wrap(generate_emails_suggestions_message(@maybe_typos)) + + if !errors.empty? + flash.now[:alert] = errors.join(". ") if !errors.empty? + + @procedure = procedure + @groupe_instructeur = groupe_instructeur + @instructeurs = paginated_instructeurs + return render :show + end + + instructeur = Instructeur.by_email(email) || + create_instructeur(email) if instructeur.blank? - flash[:alert] = "L’adresse email « #{instructeur_email} » n’est pas valide." + flash[:alert] = "L’adresse email « #{email} » n’est pas valide." elsif groupe_instructeur.instructeurs.include?(instructeur) - flash[:alert] = "L’instructeur « #{instructeur_email} » est déjà dans le groupe." + flash[:alert] = "L’instructeur « #{email} » est déjà dans le groupe." else groupe_instructeur.add(instructeur) - flash[:notice] = "L’instructeur « #{instructeur_email} » a été affecté au groupe." + flash[:notice] = "L’instructeur « #{email} » a été affecté au groupe." if instructeur.user.email_verified_at GroupeInstructeurMailer @@ -100,7 +114,7 @@ module Instructeurs end def instructeur_email - params[:instructeur][:email].strip.downcase + params.dig('instructeur', 'email')&.strip&.downcase end def instructeur_id diff --git a/app/views/instructeurs/groupe_instructeurs/show.html.haml b/app/views/instructeurs/groupe_instructeurs/show.html.haml index 1197d93bd..e0a42aa46 100644 --- a/app/views/instructeurs/groupe_instructeurs/show.html.haml +++ b/app/views/instructeurs/groupe_instructeurs/show.html.haml @@ -21,6 +21,7 @@ Démarche « #{@procedure.libelle} » .card.fr-mt-2w + = render Procedure::InvitationWithTypoComponent.new(maybe_typos: @maybe_typos, url: add_instructeur_instructeur_groupe_path(@procedure, @groupe_instructeur.id), title: "Avant d'ajouter l'email, veuillez confirmer" ) %h2.fr-h3 Gestion des instructeurs = form_for(Instructeur.new(user: User.new), url: { action: :add_instructeur }, html: { class: 'form' }) do |f| %h3.fr-h4 Affecter un nouvel instructeur From 8ff2bbbaab81bd7ec3240bdc792d712409e28d15 Mon Sep 17 00:00:00 2001 From: Lisa Durand Date: Thu, 4 Jul 2024 14:25:09 +0200 Subject: [PATCH 4/5] fix specs --- .../groupe_instructeurs_controller.rb | 8 ++++--- .../groupe_instructeurs_controller_spec.rb | 24 +++++++++---------- .../groupe_instructeurs_controller_spec.rb | 2 +- 3 files changed, 18 insertions(+), 16 deletions(-) diff --git a/app/controllers/administrateurs/groupe_instructeurs_controller.rb b/app/controllers/administrateurs/groupe_instructeurs_controller.rb index 90cb20245..16378ca5c 100644 --- a/app/controllers/administrateurs/groupe_instructeurs_controller.rb +++ b/app/controllers/administrateurs/groupe_instructeurs_controller.rb @@ -227,9 +227,11 @@ module Administrateurs instructeurs, invalid_emails = groupe_instructeur.add_instructeurs(emails:) if invalid_emails.present? - errors += [t('.wrong_address', - count: invalid_emails.size, - emails: invalid_emails.join(', '))] + errors += [ + t('.wrong_address', + count: invalid_emails.size, + emails: invalid_emails.join(', ')) + ] end if instructeurs.present? diff --git a/spec/controllers/administrateurs/groupe_instructeurs_controller_spec.rb b/spec/controllers/administrateurs/groupe_instructeurs_controller_spec.rb index 7727cbfa0..21cf35fb5 100644 --- a/spec/controllers/administrateurs/groupe_instructeurs_controller_spec.rb +++ b/spec/controllers/administrateurs/groupe_instructeurs_controller_spec.rb @@ -341,7 +341,7 @@ describe Administrateurs::GroupeInstructeursController, type: :controller do context 'when all emails are valid' do let(:emails) { ['test@b.gouv.fr', 'test2@b.gouv.fr'].to_json } it do - expect(subject).to redirect_to admin_procedure_groupe_instructeurs_path(procedure_non_routee) + expect(subject).to render_template(:index) expect(subject.request.flash[:alert]).to be_nil expect(subject.request.flash[:notice]).to be_present end @@ -350,7 +350,7 @@ describe Administrateurs::GroupeInstructeursController, type: :controller do context 'when there is at least one bad email' do let(:emails) { ['badmail', 'instructeur2@gmail.com'].to_json } it do - expect(subject).to redirect_to admin_procedure_groupe_instructeurs_path(procedure_non_routee) + expect(subject).to render_template(:index) expect(subject.request.flash[:alert]).to be_present expect(subject.request.flash[:notice]).to be_present end @@ -360,7 +360,7 @@ describe Administrateurs::GroupeInstructeursController, type: :controller do let(:instructeur) { create(:instructeur) } before { procedure_non_routee.groupe_instructeurs.first.add_instructeurs(emails: [instructeur.user.email]) } let(:emails) { [instructeur.email].to_json } - it { expect(subject).to redirect_to admin_procedure_groupe_instructeurs_path(procedure_non_routee) } + it { expect(subject).to render_template(:index) } end context 'when signed in admin comes from manager' do @@ -385,7 +385,7 @@ 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@mail.com', 'new_i2@mail.com', instructeur_email_verified.email] } + let(:new_instructeur_emails) { ['new_i1@gmail.com', 'new_i2@gmail.com', instructeur_email_verified.email] } before do allow(GroupeInstructeurMailer).to receive(:notify_added_instructeurs) @@ -398,7 +398,7 @@ describe Administrateurs::GroupeInstructeursController, type: :controller do it 'validates changes and responses' do expect(gi_1_2.instructeurs.pluck(:email)).to include(*new_instructeur_emails) expect(flash.notice).to be_present - expect(response).to redirect_to(admin_procedure_groupe_instructeur_path(procedure, gi_1_2)) + expect(response).to render_template(:show) expect(procedure.routing_enabled?).to be_truthy expect(GroupeInstructeurMailer).to have_received(:notify_added_instructeurs).with( gi_1_2, @@ -409,13 +409,13 @@ describe Administrateurs::GroupeInstructeursController, type: :controller do it "calls InstructeurMailer with the right params" do expect(InstructeurMailer).to have_received(:confirm_and_notify_added_instructeur).with( - User.find_by(email: 'new_i1@mail.com').instructeur, + User.find_by(email: 'new_i1@gmail.com').instructeur, gi_1_2, admin.email ) expect(InstructeurMailer).to have_received(:confirm_and_notify_added_instructeur).with( - User.find_by(email: 'new_i2@mail.com').instructeur, + User.find_by(email: 'new_i2@gmail.com').instructeur, gi_1_2, admin.email ) @@ -425,22 +425,22 @@ describe Administrateurs::GroupeInstructeursController, type: :controller do context 'of an instructeur already in the group' do let(:new_instructeur_emails) { [instructeur.email] } before { do_request } - it { expect(response).to redirect_to(admin_procedure_groupe_instructeur_path(procedure, gi_1_2)) } + it { expect(response).to render_template(:show) } end context 'of badly formed email' do let(:new_instructeur_emails) { ['badly_formed_email'] } before { do_request } it do - expect(flash.alert).to be_present - expect(response).to redirect_to(admin_procedure_groupe_instructeur_path(procedure, gi_1_2)) - end + expect(flash.alert).to be_present + expect(response).to render_template(:show) + end end context 'of an empty string' do let(:new_instructeur_emails) { [''] } before { do_request } - it { expect(response).to redirect_to(admin_procedure_groupe_instructeur_path(procedure, gi_1_2)) } + it { expect(response).to render_template(:show) } end context 'when connected as an administrateur from manager' do diff --git a/spec/controllers/instructeurs/groupe_instructeurs_controller_spec.rb b/spec/controllers/instructeurs/groupe_instructeurs_controller_spec.rb index bc5bf8e2b..5fa6cf44f 100644 --- a/spec/controllers/instructeurs/groupe_instructeurs_controller_spec.rb +++ b/spec/controllers/instructeurs/groupe_instructeurs_controller_spec.rb @@ -86,7 +86,7 @@ describe Instructeurs::GroupeInstructeursController, type: :controller do end context 'of a new instructeur' do - let(:new_instructeur_email) { 'new_instructeur@mail.com' } + let(:new_instructeur_email) { 'new_instructeur@gmail.com' } before { subject } it "works" do From 7539d2ddea3e427d6f2e19a408c38aded2d8aeb9 Mon Sep 17 00:00:00 2001 From: Lisa Durand Date: Wed, 10 Jul 2024 15:54:12 +0200 Subject: [PATCH 5/5] fix specs --- .../administrateurs/experts_procedures_controller.rb | 2 +- .../administrateurs/groupe_instructeurs_controller.rb | 3 +-- app/controllers/instructeurs/groupe_instructeurs_controller.rb | 2 +- app/models/concerns/email_sanitizable_concern.rb | 2 +- .../administrateurs/groupe_instructeurs_controller_spec.rb | 2 +- 5 files changed, 5 insertions(+), 6 deletions(-) diff --git a/app/controllers/administrateurs/experts_procedures_controller.rb b/app/controllers/administrateurs/experts_procedures_controller.rb index d86f00ca2..54895580b 100644 --- a/app/controllers/administrateurs/experts_procedures_controller.rb +++ b/app/controllers/administrateurs/experts_procedures_controller.rb @@ -9,7 +9,7 @@ module Administrateurs end def create - emails = params['emails'].presence || [].to_json + emails = params['emails'].presence || [] emails = check_if_typo(emails) errors = Array.wrap(generate_emails_suggestions_message(@maybe_typos)) diff --git a/app/controllers/administrateurs/groupe_instructeurs_controller.rb b/app/controllers/administrateurs/groupe_instructeurs_controller.rb index 16378ca5c..aa422b281 100644 --- a/app/controllers/administrateurs/groupe_instructeurs_controller.rb +++ b/app/controllers/administrateurs/groupe_instructeurs_controller.rb @@ -219,8 +219,7 @@ module Administrateurs end def add_instructeur - emails = params['emails'].presence || [].to_json - + emails = params['emails'].presence || [] emails = check_if_typo(emails) errors = Array.wrap(generate_emails_suggestions_message(@maybe_typos)) diff --git a/app/controllers/instructeurs/groupe_instructeurs_controller.rb b/app/controllers/instructeurs/groupe_instructeurs_controller.rb index d75254b5c..932564f83 100644 --- a/app/controllers/instructeurs/groupe_instructeurs_controller.rb +++ b/app/controllers/instructeurs/groupe_instructeurs_controller.rb @@ -20,7 +20,7 @@ module Instructeurs end def add_instructeur - email = instructeur_email.present? ? [instructeur_email].to_json : [].to_json + email = instructeur_email.present? ? [instructeur_email] : [] email = check_if_typo(email)&.first errors = Array.wrap(generate_emails_suggestions_message(@maybe_typos)) diff --git a/app/models/concerns/email_sanitizable_concern.rb b/app/models/concerns/email_sanitizable_concern.rb index b8818b1d2..80b598eec 100644 --- a/app/models/concerns/email_sanitizable_concern.rb +++ b/app/models/concerns/email_sanitizable_concern.rb @@ -18,7 +18,7 @@ module EmailSanitizableConcern end def check_if_typo(emails) - emails = JSON.parse(emails).map { EmailSanitizer.sanitize(_1) } + emails = emails.map { EmailSanitizer.sanitize(_1) } @maybe_typos, no_suggestions = emails .map { |email| [email, EmailChecker.check(email:)[:suggestions]&.first] } .partition { _1[1].present? } diff --git a/spec/controllers/administrateurs/groupe_instructeurs_controller_spec.rb b/spec/controllers/administrateurs/groupe_instructeurs_controller_spec.rb index 86fb07db8..e916435de 100644 --- a/spec/controllers/administrateurs/groupe_instructeurs_controller_spec.rb +++ b/spec/controllers/administrateurs/groupe_instructeurs_controller_spec.rb @@ -359,7 +359,7 @@ describe Administrateurs::GroupeInstructeursController, type: :controller do context 'when the admin wants to assign an instructor who is already assigned on this procedure' do let(:instructeur) { create(:instructeur) } before { procedure_non_routee.groupe_instructeurs.first.add_instructeurs(emails: [instructeur.user.email]) } - let(:emails) { [instructeur.email].to_json } + let(:emails) { [instructeur.email] } it { expect(subject).to render_template(:index) } end