Éligibilité : Correction de la durée de validité de la certification des critères - #6126
Conversation
celine-m-s
left a comment
celine-m-s
left a comment
There was a problem hiding this comment.
C'est tout bon pour moi mais j'aurais bien attendu le retour de Zohra pour avoir son avis.
|
On peut attendre lundi prochain, pas d’urgence, le bug est là depuis plusieurs mois et n’affectera pas beaucoup plus d’utilisateurs. |
67c20ae to
a270a02
Compare
| if hiring_start_at and criterion.certified: | ||
| validity_period = InclusiveDateRange( | ||
| hiring_start_at - datetime.timedelta(days=criterion.CERTIFICATION_GRACE_PERIOD_DAYS), | ||
| hiring_start_at, | ||
| ) | ||
| criterion.is_considered_certified = _inclusive_overlap(validity_period, criterion.certification_period) |
There was a problem hiding this comment.
Pourquoi créer une InclusiveDateRange et pas juste tester que hiring_start_at est dans le range de certification_period ? Je crois que ça fait pareil et que c'est plus simple, non ? 🤔
Ce qui d'ailleurs me fait me demander si il faudrait pas que ça soit une méthode sur le modèle ? Et éventuellement faire un template filter pour les templates plutôt qu'ajouter un attribut.
There was a problem hiding this comment.
Pourquoi créer une InclusiveDateRange et pas juste tester que hiring_start_at est dans le range de certification_period ? Je crois que ça fait pareil et que c'est plus simple, non ? 🤔
L’idée était de porter à parité dans le premier commit en virant la requête SQL en trop (et donc en gardant le bug de double durée de certification de is_considered_certified), puis de dégager le bug en utilisant contains dans le second commit.
Ce qui d'ailleurs me fait me demander si il faudrait pas que ça soit une méthode sur le modèle ? Et éventuellement faire un template filter pour les templates plutôt qu'ajouter un attribut.
Le problème est qu’il faut passer la hiring_start_at, et que c’est pénible à faire dans les templates. Je trouve un peu dommage d’injecter les données incomplètes (hiring_start_at, eligibility_diagnosis) pour finir par appeler du Python via un template filter custom qui ne sert que dans ce cas. Il me semble plus lisible, facile à maintenir, etc de formatter les données en Python dans les vues, pour que le template puisse simplement itérer et présenter.
The computation of certification expiry was incorrect, a criteria was considered valid much longer than intended. This bug hasn’t surfaced to employers though, because the eligibility diagnosis validity period has been adjusted to match the certification period, so the diagnosis expired even though the certification was still valid. This commit adds a test to confirm employers did not see the certified badge. Refs #6126
The computation of certification expiry was incorrect, a criteria was considered valid much longer than intended. This bug hasn’t surfaced to employers though, because the eligibility diagnosis validity period has been adjusted to match the certification period, so the diagnosis expired even though the certification was still valid. This commit adds a test to confirm employers did not see the certified badge. Refs #6126
The computation of certification expiry was incorrect, a criteria was considered valid much longer than intended. This bug hasn’t surfaced to employers though, because the eligibility diagnosis validity period has been adjusted to match the certification period, so the diagnosis expired even though the certification was still valid. This commit adds a test to confirm employers did not see the certified badge. Refs #6126
Instead of instructing the database to annotate whether the diagnosis is considered valid, compute that in Python. The database does not provide additional value, and calling `.annotate(is_considered_certified=…)` invalidates the prefetched administrative criteria.
Originally, the API particulier provided a start date and an end date (computed as the current date + 3 months). It was updated to always return None for the end date field, to better indicate that the end date is not known without breaking existing integrations. The certification period was updated to follow the API changes with 9d44cd1, where the end date is now computed to 92 days after the certification date, as the API only answers the question “is that person a beneficiary today?”. The 92 days come from our purpose for certification: it’s the 3-month limit to justify access to the RSA. However, we did not go back to `is_considered_certified`, which also grants the 92 days grace period, resulting in an 184 days grace period.
a270a02 to
b7057f1
Compare
Suspect IssuesThis pull request was deployed and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
The computation of certification expiry was incorrect, a criteria was considered valid much longer than intended. This bug hasn’t surfaced to employers though, because the eligibility diagnosis validity period has been adjusted to match the certification period, so the diagnosis expired even though the certification was still valid. This commit adds a test to confirm employers did not see the certified badge. Refs #6126
3 participants
🤔 Pourquoi ?
Au départ, l’API particulier donnait une période d’éligibilité pour la certification des critères. Il s’est avéré que la réponse de l’API correspond à la question « est-ce que cette personne est bénéficiaire aujourd’hui ? », et que la date de début correspond à l’ouverture des droits actuels. La date de fin s’est révélée fausse et a été changée pour ne jamais être spécifiée par l’API (voir 9d44cd1).
Nous avons décidé de fixer la date de fin à 92 jours, qui correspond à la fin d’acceptabilité d’un justificatif daté (justificatif de moins de trois mois). La période de certification inclut donc maintenant le délai de trois mois (
is_considered_valid), mais nous ne sommes pas revenus surwith_is_considered_valid, qui octroie également ce délai. Ainsi, le délai de 92 jours est octroyé deux fois, soit un total de 184 jours au lieu des 92 jours attendus.🍰 Comment ?
Retrait de l’annotation en base de données qui empêchait de profiter des données préchargées (
prefetch_related).🏝️ Comment tester ?
SelectedAdministrativeCriteria(certified=True, …)avec une période de certification se terminant il y a 150 jours (~5 mois).