Tech : Correction d'un test bagottant #5311
Conversation
Test cases and assertion in this file are setup to be in UTC timezone, but the `expired` trait use the one in the settings (`Europe/Paris`) so around midnight we don't always get the same date.
rsebille
added
the
no-changelog
| @@ -309,7 +309,7 @@ def test_populate_job_seekers(): | |||
| 1, | |||
| 1, | |||
| 1, | |||
| job_application_2.eligibility_diagnosis.created_at.date(), | |||
| timezone.localdate(job_application_2.eligibility_diagnosis.created_at, timezone=datetime.UTC), | |||
There was a problem hiding this comment.
Pourquoi UTC ? Si on veut la date de création du diag, cela me semble plus logique de convertir le timestamp en se basant sur Europe/Paris non ?
There was a problem hiding this comment.
Venant du modèle, created_at est un datetime.datetime avec la TZ "Europe/Paris".
De ce que j'ai compris là où ça part en eau de boudin c'est après, la table ainsi que les données sont créées directement via psycopg ce qui fait qu'a priori la TZ n'est pas configurée au niveau de la connexion (ce qui est le cas pour django.db.connection qu'on utilise pour lire les données) donc on est en UTC mais on envois un timestamptz donc PG fait la conversion :
[email protected]:itou=# SHOW TIMEZONE;
Etc/UTC
[email protected]:itou=# SELECT '2024-11-06 00:30:00+00'::timestamptz::date;
2024-11-06
[email protected]:itou=# SELECT '2024-11-06 00:30:00+01'::timestamptz::date;
2024-11-05
[email protected]:itou=# SHOW TIMEZONE;
Europe/Paris
[email protected]:itou=# SELECT '2024-11-06 00:30:00+00'::timestamptz::date;
2024-11-06
[email protected]:itou=# SELECT '2024-11-06 00:30:00+01'::timestamptz::date;
2024-11-06
On se retrouve donc avec la date UTC et non pas Europoe/Paris stockée dans la DB, vu que beaucoup de tests du fichier mentionne explicitement la TZ UTC au moment des comparaisons c'est (j'espère) le comportement voulu, et quitte à faire une erreur autant qu'elle soit (à peu près) la même partout, mais oui c'est pas ouf...
There was a problem hiding this comment.
Ça ne me choque pas de stocker la date en UTC.
Mais quand on demande à quelle date a été créé un diagnostic, cela me semble plus logique de répondre en utilisant la TZ Europe/Paris. Mais du coup l'erreur originelle vient de
en envoyant une
datetime dans une colonne de type date sans faire de conversion explicite.
There was a problem hiding this comment.
Si la logique était de stocker les dates en Europe/Paris, alors oui.
Par contre cette situation et conversion arrive aussi pour plein d'autres champs donc la logique, par voie de fait, est actuellement de stocker les dates en UTC.
Je doute qu'on se lance dans ce changement/correction à plus ou moins court terme car un certain nombre de ces champs ne devraient pas être tronqué en fait, mais derrière ça veux dire tout adapter dans les modèles DBT et metabase 😓.
Je ne suis pas en désaccord, mais quasiment tout les champs |
|
On peut étendre le fix à d’autres champs, c’est un pénible, mais je trouve ça préférable à avoir un test qui attend une valeur fausse, surtout sans un commentaire expliquant pourquoi on attend cette valeur. À la lecture des changements proposés, je penserais que le code actuel fait « ce qu’il faut » et que le test le vérifie. |
|
Il ne semble y avoir que 21 occurrence de Ça me semble finalement pas si pénible. |
|
C’est sûr qu’utiliser |
Oui, quitte à corriger ça me semble préférable que le production (les emplois) envois la donnée la plus précise sans trop réfléchir et que ça soit le consommateur qui décide (que ça soit la précision ou la timezone) comment il l'utilise, moins de responsabilité, moins de problèmes :). |
|
Je proposerais bien le compromis suivant : Qu’en penses-tu ? |
Si tu veux. |
|
Yep, c’est proposé dans #5338 |
🤔 Pourquoi ?
https://github.com/gip-inclusion/les-emplois/actions/runs/12470779088/job/34806524412?pr=5249
Introduit (vicieusement) dans #5295.