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

User model #929

Merged
merged 4 commits into from
Dec 19, 2024
Merged

User model #929

merged 4 commits into from
Dec 19, 2024

Conversation

imperosol
Copy link
Contributor

Habituellement, le changement de modèle User par défaut se fait en héritant d'AbstractUser (qui hérite lui-même d'AbstractBaseUser et de PermissionMixin), mais notre modèle hérite d'AbstractBaseUser. On perd donc le PermissionMixin. Ca nous place en grosse partie hors des clous du système d'auth de django. Et c'est vraiment chiant à gérer.

L'idée initiale était de remplacer le système de permissions de Django, plutôt que de construire quelque chose en plus. Ca se voit, d'ailleurs. Notre modèle User est une grosse duplication d'AbstractUser ; il hérite juste pas de PermissionMixin.

Eh bah bilan, l'idée était pas terrible. Notre propre interface est pas toujours respectée (sérieusement, regardez-moi le foutoir des permissions de la table des clubs). Et la vérification des droits n'est pas très efficace, voire est carrément inefficace (O(n) sur des ListView, tmtc).

Bref, on va tenter de se remettre sur les rails. Et pour commencer ça, on va revenir sur AbstractUser.
C'est juste la toute première étape, mais c'est déjà un gros truc. D'autres PRs suivront pour la refonte du système de perms.

Copy link
Contributor

@klmp200 klmp200 left a comment

Choose a reason for hiding this comment

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

C'est beaucoup de travail que je vois là et ça fait assez peur. Je pose quelques questions pour essayer de suivre un peu ce qui a été fait et voir si je comprend bien la review que j'ai fait

club/migrations/0010_auto_20170912_2028.py Show resolved Hide resolved
core/models.py Show resolved Hide resolved
core/models.py Show resolved Hide resolved
@klmp200 klmp200 requested a review from Hyask November 21, 2024 23:37
@imperosol imperosol requested review from klmp200 and removed request for Hyask November 23, 2024 12:14
Copy link
Contributor

@Hyask Hyask left a comment

Choose a reason for hiding this comment

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

Cool que tu démarres ce taf, ça va effectivement simplifier pas mal de trucs!

@imperosol imperosol force-pushed the user-model branch 2 times, most recently from 3fcc460 to aaaa225 Compare December 8, 2024 13:03
@imperosol imperosol force-pushed the user-model branch 3 times, most recently from 8d8ba1a to 5b361c9 Compare December 17, 2024 12:27
Copy link
Contributor

@klmp200 klmp200 left a comment

Choose a reason for hiding this comment

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

Ça fait peur quand même

@klmp200 klmp200 merged commit ddeb12f into taiste Dec 19, 2024
3 checks passed
@klmp200 klmp200 deleted the user-model branch December 19, 2024 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants