Skip to content

Candidat : Ajout d'une contrainte pour la cohérence entre le pays et la ville de naissance - #6765

Merged
rsebille merged 5 commits into
masterfrom
rsebille/birthplace-constraint
Sep 15, 2025
Merged

rsebille merged 5 commits into
masterfrom
rsebille/birthplace-constraint

Conversation

@rsebille

@rsebille rsebille commented Sep 1, 2025

Copy link
Copy Markdown
Contributor

🤔 Pourquoi ?

Cette règle métier est vérifiée dans les formulaires mais pas dans l'admin, et donc parfois on se tombe sur un os suite à une modification manuelle, ce qui bloque ensuite la transmission des FS,

A noter qu'actuellement nous n'avons aucun cas en erreur ;).

🍰 Comment ?

Une contrainte CHECK n'a accès qu'au du tuple inséré donc je fige la PK de birth_country dans la contrainte et ajoute des tests afin de détecter un changement de clé primaire.

J'aime pas spécialement mais vu que c'est un référentiel et que ça vient d'une fixture ça me semble tout de même acceptable si ça nous évite des incohérences de données (que j'aime encore moins 😁).

@rsebille
rsebille requested a review from EwenKorr September 1, 2025 14:01
@rsebille rsebille self-assigned this Sep 1, 2025
@rsebille rsebille added the ajouté Ajouté dans le changelog. label Sep 1, 2025
Comment thread tests/www/apply/test_process.py
@rsebille
rsebille force-pushed the rsebille/birthplace-constraint branch from 3d33880 to 201a3fb Compare September 1, 2025 14:17

@EwenKorr EwenKorr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

J'étais dessus aussi mais je viens d'apprendre par l'expérience qu'on ne peut pas naviguer dans les relations dans les contraintes.
Ta solution de test pour vérifier que la PK de la France ne change pas est bonne !

Vu qu'on s'est rendu compte via une saisie dans l'admin j'aurais peut-être ajouté un test dans tests/users/test_admin.py aussi ?

Comment thread itou/users/models.py Outdated
Comment thread itou/users/models.py Outdated
Comment thread tests/asp/test_countries.py
Comment thread itou/employee_record/serializers.py
Comment thread tests/users/test_models.py
Comment thread tests/www/apply/test_process.py
@rsebille
rsebille force-pushed the rsebille/birthplace-constraint branch 2 times, most recently from 0a4b74a to 5e07691 Compare September 4, 2025 10:07
Comment thread itou/users/models.py Outdated
Comment thread tests/users/factories.py Outdated
@rsebille
rsebille force-pushed the rsebille/birthplace-constraint branch from 5e07691 to f3e6919 Compare September 4, 2025 14:25
Comment thread tests/conftest.py Outdated
Comment thread itou/users/models.py Outdated
@rsebille
rsebille force-pushed the rsebille/birthplace-constraint branch from f3e6919 to 0d582c6 Compare September 8, 2025 13:58
Comment thread tests/users/test_models.py Outdated
@rsebille
rsebille force-pushed the rsebille/birthplace-constraint branch from 0d582c6 to 5eddff7 Compare September 9, 2025 12:40
We are going to use it in a `CHECK` constraint so getting it at runtime
will not be possible anymore.
This will prevent possible nasty bugs if the INSEE code and the PK are
desynchronized, and it's more likely to break something if it does.

> There should be one-- and preferably only one --obvious way to do it.
@rsebille
rsebille force-pushed the rsebille/birthplace-constraint branch from 5eddff7 to 579ffb3 Compare September 15, 2025 07:36
@rsebille
rsebille added this pull request to the merge queue Sep 15, 2025
Merged via the queue into master with commit 921d703 Sep 15, 2025
14 checks passed
@rsebille
rsebille deleted the rsebille/birthplace-constraint branch September 15, 2025 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ajouté Ajouté dans le changelog.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants