diff --git a/app/components/procedure/invitation_with_typo_component.rb b/app/components/procedure/invitation_with_typo_component.rb new file mode 100644 index 000000000..efd6cd15a --- /dev/null +++ b/app/components/procedure/invitation_with_typo_component.rb @@ -0,0 +1,11 @@ +class Procedure::InvitationWithTypoComponent < ApplicationComponent + def initialize(maybe_typos:, url:, title:) + @maybe_typos = maybe_typos + @url = url + @title = title + end + + def render? + @maybe_typos.present? + end +end diff --git a/app/components/procedure/invitation_with_typo_component/invitation_with_typo_component.html.haml b/app/components/procedure/invitation_with_typo_component/invitation_with_typo_component.html.haml new file mode 100644 index 000000000..63c9be415 --- /dev/null +++ b/app/components/procedure/invitation_with_typo_component/invitation_with_typo_component.html.haml @@ -0,0 +1,10 @@ += render Dsfr::AlertComponent.new(title: "nous pensons avoir identifié une faute de frappe : ", state: :warning, extra_class_names: 'fr-mb-3w') do |c| + - c.with_body do + %p= @title + %ul#maybe_typos_errors + - @maybe_typos.each do |(actual_email, suggested_email)| + %li + = "Je confirme " + = button_to "#{actual_email}", @url, method: :POST, params: { final_email: actual_email }, class: 'fr-btn fr-btn--tertiary fr-btn--sm', form: {class: 'inline'} + = " ou " + = button_to "#{suggested_email}", @url, method: :POST, params: { final_email: suggested_email }, class: 'fr-btn fr-btn--tertiary fr-btn--sm', form: {class: 'inline'} diff --git a/app/controllers/administrateurs/experts_procedures_controller.rb b/app/controllers/administrateurs/experts_procedures_controller.rb index 1a59bcba5..454b72730 100644 --- a/app/controllers/administrateurs/experts_procedures_controller.rb +++ b/app/controllers/administrateurs/experts_procedures_controller.rb @@ -1,25 +1,32 @@ module Administrateurs class ExpertsProceduresController < AdministrateurController + include EmailSanitizableConcern before_action :retrieve_procedure + before_action :retrieve_experts_procedure, only: [:index] + before_action :retrieve_experts_emails, only: [:index] def index - @experts_procedure = @procedure - .experts_procedures - .where(revoked_at: nil) - .sort_by { |expert_procedure| expert_procedure.expert.email } - @experts_emails = experts_procedure_emails end def create emails = params['emails'].presence || [].to_json - emails = JSON.parse(emails).map(&:strip).map(&:downcase) + emails = JSON.parse(emails).map { EmailSanitizer.sanitize(_1) } + @maybe_typos, emails = emails + .map { |email| [email, EmailChecker.check(email:)[:suggestions]&.first] } + .partition { _1[1].present? } + errors = if !@maybe_typos.empty? + ["Attention, nous pensons avoir identifié une faute de frappe dans les invitations : #{@maybe_typos.map(&:first).join(', ')}. Veuillez, #{view_context.link_to(" verifier l'orthographe", "#maybe_typos_errors")} des invitations."] + else + [] + end + 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?) if invalid_users.any? - flash[:alert] = invalid_users + errors += invalid_users .filter { |user| user.errors.present? } .map { |user| "#{user.email} : #{user.errors.full_messages_for(:email).join(', ')}" } end @@ -32,12 +39,16 @@ module Administrateurs end end - flash[:notice] = t('.experts_assignment', + flash.now[:notice] = t('.experts_assignment', count: valid_users.count, value: valid_users.map(&:email).join(', '), procedure: @procedure.id) end - redirect_to admin_procedure_experts_path(@procedure) + + flash.now[:alert] = errors.join(". ") if !errors.empty? + retrieve_experts_procedure + retrieve_experts_emails + render :index end def update @@ -57,8 +68,12 @@ module Administrateurs private - def experts_procedure_emails - @procedure.experts.map(&:email).sort + def retrieve_experts_procedure + @experts_procedure ||= @procedure.experts_procedures.where(revoked_at: nil).sort_by { _1.expert.email } + end + + def retrieve_experts_emails + @experts_emails ||= @experts_procedure.map { _1.expert.email } end def expert_procedure_params diff --git a/app/controllers/email_checker_controller.rb b/app/controllers/email_checker_controller.rb index 19cd0493b..b794b4d7a 100644 --- a/app/controllers/email_checker_controller.rb +++ b/app/controllers/email_checker_controller.rb @@ -1,5 +1,5 @@ class EmailCheckerController < ApplicationController def show - render json: EmailChecker.new.check(email: params[:email]) + render json: EmailChecker.check(email: params[:email]) end end diff --git a/app/javascript/controllers/email_input_controller.ts b/app/javascript/controllers/email_input_controller.ts index f8442e1d3..d0afb3168 100644 --- a/app/javascript/controllers/email_input_controller.ts +++ b/app/javascript/controllers/email_input_controller.ts @@ -4,7 +4,7 @@ import { ApplicationController } from './application_controller'; type checkEmailResponse = { success: boolean; - email_suggestions: string[]; + suggestions: string[]; }; export class EmailInputController extends ApplicationController { @@ -36,8 +36,8 @@ export class EmailInputController extends ApplicationController { url.toString() ).json(); - if (data && data.email_suggestions && data.email_suggestions.length > 0) { - this.suggestionTarget.innerHTML = data.email_suggestions[0]; + if (data && data.suggestions && data.suggestions.length > 0) { + this.suggestionTarget.innerHTML = data.suggestions[0]; show(this.ariaRegionTarget); this.ariaRegionTarget.setAttribute('aria-live', 'assertive'); } diff --git a/app/lib/email_checker.rb b/app/lib/email_checker.rb index e2d2d5222..bf1a2d871 100644 --- a/app/lib/email_checker.rb +++ b/app/lib/email_checker.rb @@ -615,7 +615,7 @@ class EmailChecker 'ac-toulous.fr' ].freeze - def check(email:) + def self.check(email:) return { success: false } if email.blank? parsed_email = Mail::Address.new(EmailSanitizableConcern::EmailSanitizer.sanitize(email)) @@ -626,29 +626,29 @@ class EmailChecker similar_domains = closest_domains(domain: parsed_email.domain) return { success: true } if similar_domains.empty? - { success: true, email_suggestions: email_suggestions(parsed_email:, similar_domains:) } + { success: true, suggestions: suggestions(parsed_email:, similar_domains:) } rescue Mail::Field::IncompleteParseError return { success: false } end private - def closest_domains(domain:) + def self.closest_domains(domain:) KNOWN_DOMAINS.filter do |known_domain| close_by_distance_of(domain, known_domain, distance: 1) || with_same_chars_and_close_by_distance_of(domain, known_domain, distance: 2) end end - def close_by_distance_of(a, b, distance:) + def self.close_by_distance_of(a, b, distance:) String::Similarity.levenshtein_distance(a, b) == distance end - def with_same_chars_and_close_by_distance_of(a, b, distance:) + def self.with_same_chars_and_close_by_distance_of(a, b, distance:) close_by_distance_of(a, b, distance: 2) && a.chars.sort == b.chars.sort end - def email_suggestions(parsed_email:, similar_domains:) + def self.suggestions(parsed_email:, similar_domains:) similar_domains.map { Mail::Address.new("#{parsed_email.local}@#{_1}").to_s } end end diff --git a/app/views/administrateurs/experts_procedures/index.html.haml b/app/views/administrateurs/experts_procedures/index.html.haml index dc8909f82..bffd9f792 100644 --- a/app/views/administrateurs/experts_procedures/index.html.haml +++ b/app/views/administrateurs/experts_procedures/index.html.haml @@ -58,6 +58,7 @@ - if @procedure.experts_require_administrateur_invitation? .card + = render Procedure::InvitationWithTypoComponent.new(maybe_typos: @maybe_typos, url: admin_procedure_experts_path(@procedure), title: "Avant d'ajouter l'email à la liste d'expert prédéfinie, veuillez confirmer" ) = form_for :experts_procedure, url: admin_procedure_experts_path(@procedure), html: { class: 'form' } do |f| diff --git a/spec/controllers/administrateurs/experts_procedures_controller_spec.rb b/spec/controllers/administrateurs/experts_procedures_controller_spec.rb index 6a9ca33d5..92f84d7e6 100644 --- a/spec/controllers/administrateurs/experts_procedures_controller_spec.rb +++ b/spec/controllers/administrateurs/experts_procedures_controller_spec.rb @@ -21,23 +21,47 @@ describe Administrateurs::ExpertsProceduresController, type: :controller do describe '#create' do let(:expert) { create(:expert) } let(:expert2) { create(:expert) } + let(:procedure) { create :procedure, administrateur: admin, experts_require_administrateur_invitation: true } - subject do - post :create, params: { - procedure_id: procedure.id, - emails: "[\"#{expert.email}\",\"#{expert2.email}\"]" - } + subject { post :create, params: params } + before { subject } + + context 'when inviting multiple valid experts' do + let(:params) { { procedure_id: procedure.id, emails: [expert.email, expert2.email].to_json } } + + it 'creates experts' do + expect(procedure.experts.include?(expert)).to be_truthy + expect(procedure.experts.include?(expert2)).to be_truthy + expect(flash.notice).to be_present + expect(assigns(:maybe_typos)).to eq([]) + expect(response).to have_http_status(:success) + end end - before do - subject + context 'when inviting expert using an email with typos' do + let(:params) { { procedure_id: procedure.id, emails: ['martin@oraneg.fr'].to_json } } + render_views + it 'warns' do + expect(flash.alert).to be_present + expect(assigns(:maybe_typos)).to eq([['martin@oraneg.fr', 'martin@orange.fr']]) + expect(response).to have_http_status(:success) + end end - context 'of multiple experts' do - it { expect(procedure.experts.include?(expert)).to be_truthy } - it { expect(procedure.experts.include?(expert2)).to be_truthy } - it { expect(flash.notice).to be_present } - it { expect(response).to redirect_to(admin_procedure_experts_path(procedure)) } + context 'when forcing email with typos' do + render_views + let(:final_email) { 'martin@oraneg.fr' } + let(:params) { { procedure_id: procedure.id, final_email: } } + + it 'works' do + created_user = User.where(email: final_email).first + expect(created_user).to be_an_instance_of(User) + expect(created_user.expert).to be_an_instance_of(Expert) + expect(procedure.experts.include?(created_user.expert)).to be_truthy + expect(flash.notice).to be_present + expect(response).to have_http_status(:success) + expect(response.body).to have_content(final_email) + end end end diff --git a/spec/controllers/email_checker_controller_spec.rb b/spec/controllers/email_checker_controller_spec.rb index 685bd5de9..bcb232301 100644 --- a/spec/controllers/email_checker_controller_spec.rb +++ b/spec/controllers/email_checker_controller_spec.rb @@ -16,7 +16,7 @@ describe EmailCheckerController, type: :controller do let(:params) { { email: 'martin@orane.fr' } } it do expect(response).to have_http_status(:success) - expect(body).to eq({ success: true, email_suggestions: ['martin@orange.fr'] }) + expect(body).to eq({ success: true, suggestions: ['martin@orange.fr'] }) end end diff --git a/spec/lib/email_checker_spec.rb b/spec/lib/email_checker_spec.rb index f9c35ea91..fd650ff31 100644 --- a/spec/lib/email_checker_spec.rb +++ b/spec/lib/email_checker_spec.rb @@ -1,6 +1,6 @@ describe EmailChecker do describe 'check' do - subject { described_class.new } + subject { described_class } it 'works with identified use cases' do expect(subject.check(email: nil)).to eq({ success: false }) @@ -10,22 +10,22 @@ describe EmailChecker do # allow same domain expect(subject.check(email: "martin@orange.fr")).to eq({ success: true }) # find difference of 1 lev distance - expect(subject.check(email: "martin@orane.fr")).to eq({ success: true, email_suggestions: ['martin@orange.fr'] }) + expect(subject.check(email: "martin@orane.fr")).to eq({ success: true, suggestions: ['martin@orange.fr'] }) # find difference of 2 lev distance, only with same chars - expect(subject.check(email: "martin@oragne.fr")).to eq({ success: true, email_suggestions: ['martin@orange.fr'] }) + expect(subject.check(email: "martin@oragne.fr")).to eq({ success: true, suggestions: ['martin@orange.fr'] }) # ignore unknown domain expect(subject.check(email: "martin@ore.fr")).to eq({ success: true }) end it 'passes through real use cases, with levenshtein_distance 1' do - expect(subject.check(email: "martin@asn.com")).to eq({ success: true, email_suggestions: ['martin@msn.com'] }) - expect(subject.check(email: "martin@gamail.com")).to eq({ success: true, email_suggestions: ['martin@gmail.com'] }) - expect(subject.check(email: "martin@glail.com")).to eq({ success: true, email_suggestions: ['martin@gmail.com'] }) - expect(subject.check(email: "martin@gmail.coml")).to eq({ success: true, email_suggestions: ['martin@gmail.com'] }) - expect(subject.check(email: "martin@gmail.con")).to eq({ success: true, email_suggestions: ['martin@gmail.com'] }) - expect(subject.check(email: "martin@hotmil.fr")).to eq({ success: true, email_suggestions: ['martin@hotmail.fr'] }) - expect(subject.check(email: "martin@mail.com")).to eq({ success: true, email_suggestions: ["martin@gmail.com", "martin@ymail.com", "martin@mailo.com"] }) - expect(subject.check(email: "martin@msc.com")).to eq({ success: true, email_suggestions: ["martin@msn.com", "martin@mac.com"] }) + expect(subject.check(email: "martin@asn.com")).to eq({ success: true, suggestions: ['martin@msn.com'] }) + expect(subject.check(email: "martin@gamail.com")).to eq({ success: true, suggestions: ['martin@gmail.com'] }) + expect(subject.check(email: "martin@glail.com")).to eq({ success: true, suggestions: ['martin@gmail.com'] }) + expect(subject.check(email: "martin@gmail.coml")).to eq({ success: true, suggestions: ['martin@gmail.com'] }) + expect(subject.check(email: "martin@gmail.con")).to eq({ success: true, suggestions: ['martin@gmail.com'] }) + expect(subject.check(email: "martin@hotmil.fr")).to eq({ success: true, suggestions: ['martin@hotmail.fr'] }) + expect(subject.check(email: "martin@mail.com")).to eq({ success: true, suggestions: ["martin@gmail.com", "martin@ymail.com", "martin@mailo.com"] }) + expect(subject.check(email: "martin@msc.com")).to eq({ success: true, suggestions: ["martin@msn.com", "martin@mac.com"] }) expect(subject.check(email: "martin@ymail.com")).to eq({ success: true }) end