Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BO - Signalement] Mise en évidence des adresses e-mails mal formattées #2566

Merged
merged 10 commits into from
Jun 4, 2024

Conversation

emilschn
Copy link
Collaborator

@emilschn emilschn commented May 14, 2024

Ticket

#1893

Description

Mise en évidence des adresses e-mail mal formattées dans la fiche signalement et dans les infos de la nouvelle page de suivi usager.
Ajout d'une commande permettant de corriger un bon lot d'adresses e-mails.

Règles de remplacement

Champs concernés sur la table Signalement : mailOccupant, mailDeclarant, mailProprio.

Chaines existantes Remplacement
Non communiqué, ?, ??, inconnu@inconnu.com, inconnu@inconnu, email@inconnu, test@test, x@x.com, test@fr, x@x.xx inconnu@histologe.fr
,fr, ,com, ,net, ?fr, ?com, ?net .fr, .com, .net

Tests

  • Modifier des signalements via la BDD et vérifier si le badge d'erreur s'affiche pour adresse occupant
  • Idem adresse déclarant
  • Idem adresse proprio
  • Utiliser des adresses non-autorisées sur les 3 champs d'adresse e-mail et jouer la commande make console app="fix-email-addresses" pour vérifier qu'elle est bien corrigée

src/Twig/AppExtension.php Outdated Show resolved Hide resolved
src/Twig/AppExtension.php Outdated Show resolved Hide resolved
templates/back/signalement/view/information.html.twig Outdated Show resolved Hide resolved
src/Twig/AppExtension.php Outdated Show resolved Hide resolved
Copy link
Collaborator

@sfinx13 sfinx13 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ajouter des tests (valide et non valide), d'ailleurs pour faciliter les tests, cette fonction pourrait être une classe helper

@emilschn emilschn marked this pull request as draft May 16, 2024 09:48
@emilschn emilschn marked this pull request as draft May 16, 2024 09:48
@emilschn
Copy link
Collaborator Author

Je passe en draft.
En plus de vos retours, il faut ajouter une commande pour corriger les e-mails existants.

@emilschn emilschn force-pushed the feature/1893-not-valid-emails branch 2 times, most recently from 5fd1d7c to dba366f Compare May 21, 2024 14:02
@emilschn
Copy link
Collaborator Author

@hmeneuvrier @sfinx13 en utilisant le validator de Symfony et faisant des tests unitaires, vous pouvez voir que ça laisse passer certains trucs ; mais l'autre méthode que j'utilisais était peut-être trop restrictive.
Je ne sais pas trop quoi en penser :)

@emilschn emilschn marked this pull request as ready for review May 23, 2024 13:59
Copy link
Collaborator

@hmeneuvrier hmeneuvrier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

C'est super bien, effectivement ça laisse passer quelques cas, même avec le mode strict, mais je préfère que ce soit cohérent partout

src/Service/EmailValidator.php Outdated Show resolved Hide resolved
src/Twig/AppExtension.php Outdated Show resolved Hide resolved
@@ -0,0 +1,22 @@
<?php

namespace App\Service;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Y'a un dossier Validator dans le projet, plutôt le mettre la bas

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

J'ai fait ça
Par contre, je n'ai pas suivi le même format que les autres Validator. Je suis resté sur une classe statique.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oui oui c'est différent

Copy link
Collaborator

@numew numew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quelques retours tech est une exception à corriger

src/Command/FixEmailAddressesCommand.php Outdated Show resolved Hide resolved
src/Command/FixEmailAddressesCommand.php Outdated Show resolved Hide resolved
src/Command/FixEmailAddressesCommand.php Outdated Show resolved Hide resolved
src/Repository/SignalementRepository.php Outdated Show resolved Hide resolved
src/Service/EmailValidator.php Outdated Show resolved Hide resolved
@emilschn emilschn force-pushed the feature/1893-not-valid-emails branch from 3222b5d to d4c6f0d Compare May 24, 2024 12:48
@emilschn
Copy link
Collaborator Author

@sfinx13 @hmeneuvrier @numew J'ai pris en compte vos retours.
J'ai un petit doute sur ce que vous attendiez pour le Validator, je vous laisse me redire.

Copy link
Collaborator

@hmeneuvrier hmeneuvrier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests et relecture OK pour moi

Copy link
Collaborator

@sfinx13 sfinx13 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lecture et test OK
Proposition d'amélioration rendre inactif la checkbox suivi usager si aucun des mails usager n'est valide
image

@emilschn emilschn force-pushed the feature/1893-not-valid-emails branch from 866a70e to 57e474b Compare May 31, 2024 14:23
@emilschn
Copy link
Collaborator Author

Les corrections sont faites.
Le nouveau dashboard metabase est ici : https://histologe-metabase.osc-fr1.scalingo.io/question/1179-e-mails-non-valides

Copy link
Collaborator

@hmeneuvrier hmeneuvrier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tout ok pour moi avec les dernières modifs

Copy link
Collaborator

@sfinx13 sfinx13 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lecture et test OK

src/Twig/AppExtension.php Outdated Show resolved Hide resolved
Copy link

sonarcloud bot commented Jun 3, 2024

Quality Gate Passed Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@hmeneuvrier
Copy link
Collaborator

Ok, j'ai vu le dernier retour de Saidi et la modif d'Emilien, je merge !

@hmeneuvrier hmeneuvrier merged commit 239a8a6 into develop Jun 4, 2024
3 checks passed
@hmeneuvrier hmeneuvrier deleted the feature/1893-not-valid-emails branch June 11, 2024 10:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants