Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: bug sur les gestionnaires sur Admin #1638

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

Guilouf
Copy link
Collaborator

@Guilouf Guilouf commented Jan 16, 2025

Quoi ?

Bug rapporté par Pauline, trace ici

Pourquoi ?

Problème de contrainte d'intégrité sur les gestionnaires, alors qu'aucune n'est définie dans le modèle. Le problème se situe ici: https://github.com/gip-inclusion/le-marche/pull/109/files#diff-5e753eeddbaaab6c4cf67e55983d1712f6442f0d7c910907e9bd86d1eeb61f71R58

Du SQL a été exécuté, sans que le modèle de données ne reflète ces changements.

Comment ?

Une nouvelle migration à été créée pour remettre à plat le modèle sans contraintes fantômes.

Commentaire

Plus de contraintes fantômes, mais plus de contrainte tout court, un même utilisateur peut être gestionnaire 2 fois pour une même structure. Faut il ajouter une contrainte sur le modèle intermédiaire SiaeUser ?

Ticket sur django ouvert

@Guilouf Guilouf added the bug Something isn't working label Jan 16, 2025
@Guilouf Guilouf self-assigned this Jan 16, 2025
@Guilouf Guilouf marked this pull request as ready for review January 17, 2025 10:17
Copy link
Contributor

@SebastienReuiller SebastienReuiller left a comment

Choose a reason for hiding this comment

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

Whaou ! Bravo d'avoir trouvé ce bug bien vicieux.

Ta solution est peut-être un peu radicale/risquée de tout copier et renommer.

Est-ce que tu peux ajouter la contrainte d'unicité sur user / siae ? Logiquement, on ne peut pas avoir un utilisateur plusieurs fois liés à la même structure, mais sans cette contrainte, c'est possible de le faire dans l'admin.

@Guilouf Guilouf force-pushed the guilouf/integrity-users branch from 41a0983 to 84c3700 Compare January 21, 2025 17:49
@Guilouf
Copy link
Collaborator Author

Guilouf commented Jan 23, 2025

Whaou ! Bravo d'avoir trouvé ce bug bien vicieux.

Ta solution est peut-être un peu radicale/risquée de tout copier et renommer.

Est-ce que tu peux ajouter la contrainte d'unicité sur user / siae ? Logiquement, on ne peut pas avoir un utilisateur plusieurs fois liés à la même structure, mais sans cette contrainte, c'est possible de le faire dans l'admin.

Merci pour la review, j'ai ajouté la contrainte d'intégrité, et la validation qui va avec. Pour ce qui est de la radicalité de la solution, je pense que c'est la manière la plus sure d'effacer les conséquences de la migration SQL sauvage, et d'avoir un SQL qui correspond exactement à ce qui a été fait par l'ORM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants