ETQ Administrateur - ajouter un lien ou un email à un service #11207
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #11207 +/- ##
==========================================
- Coverage 84.56% 84.41% -0.16%
==========================================
Files 1196 1199 +3
Lines 26296 26391 +95
Branches 4962 4968 +6
==========================================
+ Hits 22238 22278 +40
- Misses 4058 4113 +55 ☔ View full report in Codecov by Sentry. |
- Encode URL with Addressable::URI.encode before parsing - check value before encoding
app/validators/url_validator.rb
Outdated
| # First check if it's an email when accept_email is true | ||
| if options.fetch(:accept_email) && value.to_s.include?('@') | ||
| validator = StrictEmailValidator.new(attributes: attribute) | ||
| return true if validator.validate_each(record, attribute, value) | ||
| # If email validation failed, return false to prevent URL validation attempt | ||
| return false | ||
| end | ||
|
|
||
| # If not an email, validate as URL | ||
| begin | ||
| uri = if value.present? | ||
| encoded_value = Addressable::URI.encode(value.to_s) | ||
| Addressable::URI.parse(encoded_value) | ||
| end | ||
|
|
There was a problem hiding this comment.
Je réécris ici ce qu'on s'est dit à l'oral pour éviter un nouveau travail par un collegue.
Le changement d'implem de l'url_validator, ne semble pas nécessaire car il présente déjà une option pour valider des emails.
Cela étant dit, on voit que l'implem historique est légère : le mail doit uniquement contenir un @. On pourrait réutiliser ici effectivement le StrictEmailValidator pour durcir la compat.
Finalement, si on le fait. Ca serait chouette de le faire dans un commit séparé avec un test qui montre pourquoi on le fait.
app/models/service.rb
Outdated
| @@ -27,6 +27,7 @@ class Service < ApplicationRecord | |||
| validates :siret, comparison: { other_than: SIRET_TEST, message: "n'est pas valide" }, on: :update | |||
| validates :type_organisme, presence: { message: 'doit être renseigné' }, allow_nil: false | |||
| validates :email, presence: { message: 'doit être renseigné' }, allow_nil: false | |||
| validates :email, url: { no_local: true, allow_blank: false, accept_email: true } | |||
There was a problem hiding this comment.
comme vu ensemble, ca serait top de le renommer en email_or_contact_page.
Par ailleurs, y a aurait pas moyen de fusionner cette ligne avec celle du dessus ?
There was a problem hiding this comment.
merci @LeSim pour tes retours.
Effectivement la validation des emails actuellement laisse passer des mails en "nom@mail" ce qui n'est pas conforme.
j'ai fait de nouveaux commit sur la base de tes commentaires mais pas créé la nouvelle colonne et renommé l'élément en base de données.
Tous les tests unitaires sont désormais ok.
There was a problem hiding this comment.
du coup je creuse pour améliorer la validation du mail pour procedure/lien_dpo et service/mail pour l'instant avant de travailler la migration et renommage en base.
mais le contrôle des emails n'est pas assez abouti, un mail de type mail@site par exemple passe alors qu'il ne devrait pas.
|
Pour info on en a discuté au point UX avec @marleneklok entre autres car dans l'objectif de soulager le support ça nous semblait plus pertinent d'avoir 2 champs séparés : car le lien aurait plus role de "FAQ" ou d'accompagnement dans la démarche, et que certains services voudraient mettre un email ET ce lien. On en reparle à une prochaine réunion si vous voulez ! |
|
merci @colinux & @marleneklok . Je m'étais calé sur ce qui se faisait avec le DPO mais je vous rejoins, de mettre les deux est plus pertinent. Il faut qu'on discute ensuite du côté de l'UX pour le choix de l'usager dans le cas où les deux sont précisés pour que les admins puissent orienter vers l'un ou l'autre au choix. |
…ches-simplifiees/demarches-simplifiees.fr into 11205-link-or-mail-service
Sur le même modèle que le champ relatif au contact DPO, possibilité d'ouvrir aux services l'ajout d'un lien, en plus de l'email :
Les modifications apportées concernent l'écran administrateur pour la création ou la modification du service et rendent visible les éléments côté instructeurs avec une adaptation de la traduction selon qu'il s'agit d'un mail ou d'un lien.
Le format de l'input est également contrôlé pour assurer le format.