[BO - Signalement] Affichage champs obligatoires / facultatifs et des erreurs à l'édition #2303
Conversation
| if (null !== $this->createdFrom) { | ||
| return $this->profileDeclarant; | ||
| } | ||
|
|
||
| if (false === $this->isNotOccupant) { | ||
| return ProfileDeclarant::LOCATAIRE; | ||
| } | ||
|
|
||
| if (\in_array($this->lienDeclarantOccupant, ['PROFESSIONNEL', 'pro', 'assistante sociale', 'curatrice'])) { | ||
| return ProfileDeclarant::TIERS_PRO; | ||
| } | ||
|
|
||
| return ProfileDeclarant::TIERS_PARTICULIER; |
There was a problem hiding this comment.
Compliqué à gérer les erreurs sur les anciens signalements sans profileDeclarant, du coup détermine un profileDeclarant à l'aide de isNotOccupant et lienDeclarantOccupant
['PROFESSIONNEL', 'pro', 'assistante sociale', 'curatrice'] trouvé dans la base de données
Pour les bailleurs et tiers de secours c'était pas géré avant
Voyez vous des effets de bord ?
| 'App\\Dto\\Request\\Signalement\\CoordonneesFoyerRequest', | ||
| 'nom', | ||
| signalement.profileDeclarant) }}</label> | ||
| <input class="fr-input" type="text" id="nom" name="nom" value="{{ signalement.nomOccupant }}" maxlength="50"> |
There was a problem hiding this comment.
ça fait des doublons d'id (avec les autres modales), ce n'est pas valide en html
et du coup l'attribut for ne va pas forcément pointer vers le bon
il faut des id vraiment uniques
There was a problem hiding this comment.
Effectivement :-) dire que j'avais hésité à me lancer la dedans :-D
There was a problem hiding this comment.
Ils ont tous des id différents maintenant
| private EntityManagerInterface $entityManager, | ||
| ) { | ||
| } | ||
| private const ERROR_MSG = 'Une erreur s\'est produite. Veuillez actualiser la page.'; |
There was a problem hiding this comment.
On ne précise pas qu'il s'agit d'une erreur Token CSRF ?
There was a problem hiding this comment.
Pas besoin, c'est un message pour les utilisateurs, ils ne savent pas ce qu'est un token CSRF.
There was a problem hiding this comment.
Je suis pas d'accord avec ça : nous on sait ce que ca signifie, et le jour ou on reçoit une copie d'écran avec ce message on comprends mieux que "Une erreur s'est produite. Veuillez actualiser la page"
There was a problem hiding this comment.
A voir à l'usage pour l'instant ça va :-)
| divErrorElement.classList.remove('fr-input-group--error'); | ||
| let pErrorElement = divErrorElement.querySelector('.fr-error-text'); | ||
| if (pErrorElement) { | ||
| pErrorElement.remove(); |
There was a problem hiding this comment.
Il faudrait appliquer le meme fonctionnement au submit car si je corrige une erreur mais pas toutes et que je renvoi le formulaire ca ne se met pas à jour correctement
| private readonly ?string $typeLogementNatureAutrePrecision = null, | ||
| #[Assert\NotBlank(message: 'Merci de selectioner pièce unique ou plusieurs pièces.')] |
There was a problem hiding this comment.
Si je dis pas de bêtises on doit pouvoir laisser des valeur vides (permet de différencier sir l'user n'a pas répondu ou a mis je ne sais pas, ou si ca vient des ancien formulaire) donc dans quasiment tous les cas il faut retirer les NotBlank
There was a problem hiding this comment.
il est prévu de faire évoluer le formulaire de dépôt et ajouter des je ne sais pas sur certaines questions
#2275
il faudrait progressivement que les anciens formulaires puisse migrer sur les nouvelles règles de saisie avec des règles équivalentes sur le formulaire front et les éditions.
C'est vers la ou on doit tendre que de rester dans un état hybride (ancien/nouveau)
C'est ce qui m'a motivé à ajouter un profileDeclarant pour les anciens formulaires
src/Utils/AttributeParser.php
Outdated
| ): string { | ||
| $reflector = new \ReflectionClass($class); | ||
| /** @var \ReflectionAttribute[] $attributes */ | ||
| $attributes = $reflector->getProperty($field)->getAttributes(NotBlank::class); |
There was a problem hiding this comment.
J'aurais préféré qu'on se base sur l'attribut required de l'input, ce qui nous aurait forcé à passer les champ obligatoire en required ce qui n'est pas le cas actuellement (ex : nom bailleur) pour éviter des validation serveur de ce type.
Ce qui serait automatique si on avait utilisé les formType, c'est un travail plus conséquent mais on aurait gagné en simplicité et maintenance à mon sens.
There was a problem hiding this comment.
Les jours de la fiche de signalement tel qu'on l'a aujourd'hui sont comptés :-D
Même si sur cette PR ça aurait facilité de tout gérér via des formulaires symfony mais c’était peu lourd de tout revoir alors qu'on se dirige vers une refonte très bientôt avec l'abandon du twig sur la fiche.
L'absence de vérification coté client c'était pour pas alourdir les templates html et ça reste des éditions qui se feront à la marge.
Bien entendu le mieux aurait été de gérer les deux mais ça faisait lourd.
There was a problem hiding this comment.
Je continue de trouver dommage de pas avoir l'attribut required HTML pour les quelque champs toujours obligatoire mais ok
templates/back/signalement/view/edit-modals/edit-informations-logement.html.twig
Outdated
Show resolved
Hide resolved
sfinx13
force-pushed
the
feature/2168-edit-modale-form-ajax
branch
from
6a57fa5
to
5952f9d
Compare
| message: 'Merci de sélectioner pièce unique ou plusieurs pièces.', | ||
| groups: ['LOCATAIRE', 'BAILLEUR_OCCUPANT', 'BAILLEUR', 'TIERS_PARTICULIER', 'TIERS_PRO', 'SERVICE_SECOURS'] | ||
| )] | ||
| #[Assert\Choice( |
There was a problem hiding this comment.
A terme il faudrait qu'on ai la contrainte sur les tous les champs fermé, mais on va oublier dans l'immédiat
| private readonly ?string $nomStructure = null, | ||
| #[Assert\NotBlank(message: 'Merci de sélectionner une civilité.', groups: ['LOCATAIRE'])] |
There was a problem hiding this comment.
Je pense que c'est un champ que l'on avait pas dans l'ancien formulaire et qu'il faudrait pouvoir laisser vide
There was a problem hiding this comment.
oui celui ci je l'ai laissé volontairement. ça reste raisonnable de forcer la civilité pour les anciennes fiches. On laisse venir si c'est bloquant
| { | ||
| public function __construct( | ||
| private readonly ?string $isLogementSocial = null, | ||
| #[Assert\NotBlank(['message' => 'Veuillez définir le champ demande relogement', 'groups' => ['LOCATAIRE', 'BAILLEUR_OCCUPANT']])] | ||
| #[Assert\NotBlank( | ||
| message: 'Veuillez définir le champ demande relogement.', |
There was a problem hiding this comment.
Veuillez préciser si une demande de relogement a été faite
There was a problem hiding this comment.
Retours de test
Possible de désactiver le bouton Valider en attendant le résultat de la requête Ajax ?
Quelques messages d'erreur à ajuster.
Sinon en tant que bailleur, il n'est pas inscrit que le nombre d'étages (d'ailleurs, le nom du champ pourrait être changé, non ?) est facultatif. Pourtant, si je supprime, je ne suis pas bloqué.
|
C'est fait
Vieux champs désormais facultatif |
Ticket
#2168
Description
Améliorer l'UX en affichant les erreurs dans la modale et mentionner les champs facultatifs
Changements apportés
data-ajax-form)SignalementEditControllerPré-requis
make npm-buildPOST puis PUTTests
S'appuyer sur le fichiers pour vérifier les contraintes https://docs.google.com/spreadsheets/d/1hvhNHY5ZS-MtgbEBo8WCUrj4aKExJqMxGANP3Z1zDEg/edit?usp=sharing