Candidature: correction d'une erreur d'acceptation dans de rares conditions#4522
Candidature: correction d'une erreur d'acceptation dans de rares conditions#4522calummackervoy merged 1 commit intomasterfrom
Conversation
tonial
left a comment
tonial
left a comment
There was a problem hiding this comment.
Je pense que tu peux améliorer la façon dont on passe birthdate à CertifiedCriteriaInfoRequiredForm.
J'ai fait 2 propositions mais je ne suis pas encore sur de quelle est la meilleurs 🤔 cela peut valoir le coup d'en discuter rapidement en SU, ou de demander l'avis à d'autres dev.
Il faudrait également que tu ajoute un test dans tests/www/apply/test_process.py::ProcessAcceptViewsTest qui plante comme dans sentry avant ta correction car pour l'instant on ne garanti pas que le problème ne se reproduira pas : typiquement si quelqu'un rechange ton birthdate = parse_date(birthdate) aucun test ne casse.
| expected_msg = ( | ||
| f"Le code INSEE {birth_place.code} n'est pas référencé par l'ASP en date du {early_date:%d/%m/%Y}" | ||
| ) | ||
| assert form.errors["birth_place"] == [expected_msg] |
There was a problem hiding this comment.
Merci pour les tests sur le formulaire !
…uiredForm fix accept candidate: clean birthdate when passing to CertifiedCriteriaInfoRequiredForm
calummackervoy
force-pushed
the
calum/fix-date-format
branch
from
13d6e3f to
a66a85d
Compare
| try: | ||
| birthdate = JobSeekerPersonalDataForm.base_fields["birthdate"].clean( | ||
| form_personal_data.data.get("birthdate") | ||
| ) | ||
| except ValidationError: | ||
| pass # will be presented to user later |
There was a problem hiding this comment.
Pourquoi ne pas faire if form_personal_data.is_valid(): ?
There was a problem hiding this comment.
Pour éviter qu'un autre champ invalide va empêcher la validation du birth_place sur CertifiedCriteriaInfoRequiredForm
There was a problem hiding this comment.
Ok. C’est plus compliqué que nécessaire pour un cas très à la marge, mais comme tu préfères.
|
Attention, le titre des PRs est utilisé pour communiquer les changements au utilisateurs. Mieux vaut le reformuler pour expliquer un peu plus ce qui a été changé (notamment erreur 500 qui n’est pas très parlant) |
francoisfreitag
left a comment
francoisfreitag
left a comment
There was a problem hiding this comment.
Autrement LGTM, merci d’avoir fixé le problème 💯
calummackervoy
changed the title
calummackervoy
changed the title
francoisfreitag
changed the title
4 participants
🤔 Pourquoi ?
CertifiedCriteriaInfoRequiredFormutilise un champbirthdatequi est anticipé d'être unedate, mais c'est dans l'utilisation c'est possible que c'est unstr, qui cause unValueError🍰 Comment ?
birthdatedansCertifiedCriteriaInfoRequiredFormest une dateCertifiedCriteriaInfoRequiredForm🚨 À vérifier