Merge pull request #1506 from betagouv/fix-1479

Gestionnaire emails with nbsp 😩
This commit is contained in:
gregoirenovel 2018-03-14 17:46:15 +01:00 committed by GitHub
commit 939c7bc970
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
18 changed files with 183 additions and 38 deletions

View file

@ -3,7 +3,7 @@ module NewGestionnaire
before_action :authenticate_gestionnaire!, except: [:sign_up, :create_gestionnaire]
before_action :redirect_if_no_sign_up_needed, only: [:sign_up]
before_action :check_avis_exists_and_email_belongs_to_avis, only: [:sign_up, :create_gestionnaire]
before_action :set_avis_and_dossier, only: [:show, :instruction, :messagerie, :create_commentaire]
before_action :set_avis_and_dossier, only: [:show, :instruction, :messagerie, :create_commentaire, :update]
A_DONNER_STATUS = 'a-donner'
DONNES_STATUS = 'donnes'
@ -29,12 +29,18 @@ module NewGestionnaire
end
def instruction
@new_avis = Avis.new
end
def update
avis.update(avis_params)
flash.notice = 'Votre réponse est enregistrée.'
redirect_to instruction_gestionnaire_avis_path(avis)
if @avis.update(avis_params)
flash.notice = 'Votre réponse est enregistrée.'
redirect_to instruction_gestionnaire_avis_path(@avis)
else
flash.now.alert = @avis.errors.full_messages
@new_avis = Avis.new
render :instruction
end
end
def messagerie
@ -55,8 +61,16 @@ module NewGestionnaire
def create_avis
confidentiel = avis.confidentiel || params[:avis][:confidentiel]
Avis.create(create_avis_params.merge(claimant: current_gestionnaire, dossier: avis.dossier, confidentiel: confidentiel))
redirect_to instruction_gestionnaire_avis_path(avis)
@new_avis = Avis.new(create_avis_params.merge(claimant: current_gestionnaire, dossier: avis.dossier, confidentiel: confidentiel))
if @new_avis.save
flash.notice = "Une demande d'avis a été envoyée à #{@new_avis.email_to_display}"
redirect_to instruction_gestionnaire_avis_path(avis)
else
flash.now.alert = @new_avis.errors.full_messages
set_avis_and_dossier
render :instruction
end
end
def sign_up

View file

@ -27,6 +27,7 @@ module NewGestionnaire
def avis
@avis_seen_at = current_gestionnaire.follows.find_by(dossier: dossier)&.avis_seen_at
@avis = Avis.new
end
def personnes_impliquees
@ -159,8 +160,15 @@ module NewGestionnaire
end
def create_avis
Avis.create(avis_params.merge(claimant: current_gestionnaire, dossier: dossier))
redirect_to avis_gestionnaire_dossier_path(procedure, dossier)
@avis = Avis.new(avis_params.merge(claimant: current_gestionnaire, dossier: dossier))
if @avis.save
flash.notice = "Une demande d'avis a été envoyée à #{@avis.email_to_display}"
redirect_to avis_gestionnaire_dossier_path(procedure, dossier)
else
flash.now.alert = @avis.errors.full_messages
@avis_seen_at = current_gestionnaire.follows.find_by(dossier: dossier)&.avis_seen_at
render :avis
end
end
def update_annotations

View file

@ -1,14 +1,16 @@
class Administrateur < ApplicationRecord
include CredentialsSyncableConcern
include EmailSanitizableConcern
devise :database_authenticatable, :registerable,
:recoverable, :rememberable, :trackable, :validatable
has_and_belongs_to_many :gestionnaires
has_many :procedures
before_validation -> { sanitize_email(:email) }
before_save :ensure_api_token
include CredentialsSyncableConcern
scope :inactive, -> { where(active: false) }
def self.find_inactive_by_token(reset_password_token)

View file

@ -1,9 +1,13 @@
class Avis < ApplicationRecord
include EmailSanitizableConcern
belongs_to :dossier, touch: true
belongs_to :gestionnaire
belongs_to :claimant, class_name: 'Gestionnaire'
before_save :clean_email
validates :email, format: { with: Devise.email_regexp, message: "n'est pas valide" }, allow_nil: true
before_validation -> { sanitize_email(:email) }
before_create :try_to_assign_gestionnaire
after_create :notify_gestionnaire
@ -29,12 +33,6 @@ class Avis < ApplicationRecord
private
def clean_email
if email.present?
self.email = email.downcase.strip
end
end
def notify_gestionnaire
AvisMailer.avis_invitation(self).deliver_now
end

View file

@ -0,0 +1,10 @@
module EmailSanitizableConcern
extend ActiveSupport::Concern
def sanitize_email(attribute)
value_to_sanitize = self.send(attribute)
if value_to_sanitize.present?
self[attribute] = value_to_sanitize.gsub(/[[:space:]]/, ' ').strip.downcase
end
end
end

View file

@ -1,9 +1,14 @@
class Gestionnaire < ApplicationRecord
include CredentialsSyncableConcern
include EmailSanitizableConcern
devise :database_authenticatable, :registerable,
:recoverable, :rememberable, :trackable, :validatable
has_and_belongs_to_many :administrateurs
before_validation -> { sanitize_email(:email) }
has_many :assign_to, dependent: :destroy
has_many :procedures, through: :assign_to
has_many :dossiers, -> { state_not_brouillon }, through: :procedures
@ -12,8 +17,6 @@ class Gestionnaire < ApplicationRecord
has_many :avis
has_many :dossiers_from_avis, through: :avis, source: :dossier
include CredentialsSyncableConcern
def visible_procedures
procedures.publiees_ou_archivees
end

View file

@ -1,9 +1,13 @@
class Invite < ApplicationRecord
include EmailSanitizableConcern
belongs_to :dossier
belongs_to :user
before_validation -> { sanitize_email(:email) }
validates :email, presence: true
validates :email, uniqueness: { scope: :dossier_id }
validates :email, email_format: true
validates :email, format: { with: Devise.email_regexp, message: "n'est pas valide" }, allow_nil: true
end

View file

@ -1,4 +1,7 @@
class User < ApplicationRecord
include CredentialsSyncableConcern
include EmailSanitizableConcern
enum loged_in_with_france_connect: {
particulier: 'particulier',
entreprise: 'entreprise'
@ -18,7 +21,7 @@ class User < ApplicationRecord
delegate :given_name, :family_name, :email_france_connect, :gender, :birthdate, :birthplace, :france_connect_particulier_id, to: :france_connect_information
accepts_nested_attributes_for :france_connect_information
include CredentialsSyncableConcern
before_validation -> { sanitize_email(:email) }
def self.find_for_france_connect email, siret
user = User.find_by(email: email)

View file

@ -1,10 +0,0 @@
class EmailFormatValidator < ActiveModel::Validator
def email_regex
/\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\z/i
end
def validate(record)
return if record.email.blank?
record.errors[:base] << "Email invalide" if !email_regex.match(record.email)
end
end

View file

@ -21,6 +21,6 @@
.send-wrapper
= f.submit 'Envoyer votre avis', class: 'button send'
= render partial: "new_gestionnaire/shared/avis/form", locals: { url: avis_gestionnaire_avis_path(@avis), must_be_confidentiel: @avis.confidentiel? }
= render partial: "new_gestionnaire/shared/avis/form", locals: { url: avis_gestionnaire_avis_path(@avis), must_be_confidentiel: @avis.confidentiel?, avis: @new_avis }
= render partial: 'new_gestionnaire/shared/avis/list', locals: { avis: @dossier.avis_for(current_gestionnaire), avis_seen_at: nil }

View file

@ -3,6 +3,6 @@
= render partial: "header", locals: { dossier: @dossier }
.container
= render partial: "new_gestionnaire/shared/avis/form", locals: { url: avis_gestionnaire_dossier_path(@dossier.procedure, @dossier), must_be_confidentiel: false }
= render partial: "new_gestionnaire/shared/avis/form", locals: { url: avis_gestionnaire_dossier_path(@dossier.procedure, @dossier), must_be_confidentiel: false, avis: @avis }
= render partial: 'new_gestionnaire/shared/avis/list', locals: { avis: @dossier.avis, avis_seen_at: @avis_seen_at }

View file

@ -2,9 +2,9 @@
%h1.tab-title Inviter une personne à donner son avis
%p.avis-notice L'invité pourra consulter, donner un avis sur le dossier et contribuer au fil de messagerie, mais il ne pourra le modifier.
= form_for Avis.new, url: url, html: { class: 'form' } do |f|
= form_for avis, url: url, html: { class: 'form' } do |f|
= f.email_field :email, placeholder: 'Adresse email', required: true
= f.text_area :introduction, rows: 3, value: 'Bonjour, merci de me donner votre avis sur ce dossier.', required: true
= f.text_area :introduction, rows: 3, value: avis.introduction || 'Bonjour, merci de me donner votre avis sur ce dossier.', required: true
.flex.justify-between.align-baseline
- if must_be_confidentiel
%p.confidentiel.flex

View file

@ -0,0 +1,7 @@
fr:
activerecord:
models:
avis: 'Avis'
attributes:
avis:
answer: "Réponse"

View file

@ -108,6 +108,16 @@ describe NewGestionnaire::AvisController, type: :controller do
post :create_avis, params: { id: previous_avis.id, avis: { email: email, introduction: intro, confidentiel: asked_confidentiel } }
end
context 'when an invalid email' do
let(:previous_avis_confidentiel) { false }
let(:asked_confidentiel) { false }
let(:email) { "toto.fr" }
it { expect(response).to render_template :instruction }
it { expect(flash.alert).to eq(["Email n'est pas valide"]) }
it { expect(Avis.last).to eq(previous_avis) }
end
context 'when the previous avis is public' do
let(:previous_avis_confidentiel) { false }

View file

@ -337,20 +337,34 @@ describe NewGestionnaire::DossiersController, type: :controller do
describe "#create_avis" do
let(:saved_avis) { dossier.avis.first }
before do
subject do
post :create_avis, params: {
procedure_id: procedure.id,
dossier_id: dossier.id,
avis: { email: 'email@a.com', introduction: 'intro', confidentiel: true }
avis: { email: email, introduction: 'intro', confidentiel: true }
}
end
before do
subject
end
let(:email) { 'email@a.com' }
it { expect(saved_avis.email).to eq('email@a.com') }
it { expect(saved_avis.introduction).to eq('intro') }
it { expect(saved_avis.confidentiel).to eq(true) }
it { expect(saved_avis.dossier).to eq(dossier) }
it { expect(saved_avis.claimant).to eq(gestionnaire) }
it { expect(response).to redirect_to(avis_gestionnaire_dossier_path(dossier.procedure, dossier)) }
context "with an invalid email" do
let(:email) { 'emaila.com' }
it { expect(response).to render_template :avis }
it { expect(flash.alert).to eq(["Email n'est pas valide"]) }
it { expect { subject }.not_to change(Avis, :count) }
end
end
describe "#update_annotations" do

View file

@ -118,7 +118,7 @@ RSpec.describe Avis, type: :model do
end
end
describe "#clean_email" do
describe "email sanitization" do
subject { Avis.create(claimant: claimant, email: email, dossier: create(:dossier), gestionnaire: create(:gestionnaire)) }
context "when there is no email" do

View file

@ -0,0 +1,51 @@
describe EmailSanitizableConcern, type: :model do
describe 'sanitize_email' do
let(:email_concern) do
(Class.new do
include EmailSanitizableConcern
attr_accessor :email
def initialize(email)
self.email = email
end
def [](key)
self.send(key)
end
def []=(key, value)
self.send("#{key}=", value)
end
end).new(email)
end
before do
email_concern.sanitize_email(:email)
end
context 'on an empty email' do
let(:email) { '' }
it { expect(email_concern.email).to eq('') }
end
context 'on a valid email' do
let(:email) { 'michel@toto.fr' }
it { expect(email_concern.email).to eq('michel@toto.fr') }
end
context 'on an email with trailing spaces' do
let(:email) { ' michel@toto.fr ' }
it { expect(email_concern.email).to eq('michel@toto.fr') }
end
context 'on an email with trailing nbsp' do
let(:email) { ' michel@toto.fr  ' }
it { expect(email_concern.email).to eq('michel@toto.fr') }
end
context 'on an invalid email' do
let(:email) { 'mich el@toto.fr' }
it { expect(email_concern.email).to eq('mich el@toto.fr') }
end
end
end

View file

@ -24,5 +24,36 @@ describe Invite do
it { expect{ subject }.to raise_error ActiveRecord::RecordInvalid }
end
context "email validation" do
let(:invite) { build(:invite, email: email, dossier: dossier1) }
context 'when an email is invalid' do
let(:email) { 'toto.fr' }
it do
expect(invite.save).to be false
expect(invite.errors.full_messages).to eq(["Email n'est pas valide"])
end
context 'when an email is empty' do
let(:email) { nil }
it do
expect(invite.save).to be false
expect(invite.errors.full_messages).to eq(["Email est vide"])
end
end
end
context 'when an email is valid' do
let(:email) { 'toto@toto.fr' }
it do
expect(invite.save).to be true
expect(invite.errors.full_messages).to eq([])
end
end
end
end
end