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 UserMetadata not being created if createsuperuser script is used #1113

Merged
merged 1 commit into from
Feb 26, 2024

Conversation

mvandenburgh
Copy link
Member

@mvandenburgh mvandenburgh commented Jun 7, 2022

Before, we were listening to the allauth.user_signed_up signed up signal to create the UserMetadata. Howver, this doesn't work if the User is created outside of django-allauth (like when manage.py createsuperuser is used), so this changes it to use the more general User post_save signal.

Updated approach: Before, we were listening to the allauth.user_signed_up signed up signal to create the UserMetadata. This doesn't work if the User is created with the createsuperuser management command, as composed-configuration overrides that to create a user of type EmailAsUsernameProxyUser (which is a subclass of User). See kitware-resonant/django-composed-configuration#187 for more info.

(Another) updated approach: the previous approach didn't work either, so I just overrode the createsuperuser script to manually create the UserMetadata record after it runs. I used Django signals to do this, as there's no way to get the new user's info from the Command object.

Fixes #1085

@mvandenburgh
Copy link
Member Author

There seems to be a better way to do this without using signals - I'll close this and open a new PR

@mvandenburgh mvandenburgh deleted the fix-user-metadata-creation branch August 5, 2022 18:29
@mvandenburgh mvandenburgh restored the fix-user-metadata-creation branch December 1, 2023 18:52
@mvandenburgh mvandenburgh reopened this Dec 1, 2023
@mvandenburgh
Copy link
Member Author

There seems to be a better way to do this without using signals - I'll close this and open a new PR

I'm not sure what "better way" I was thinking about here, but this issue still occurs in dev environments. Unless I'm missing something, signals seems like the way to do this?

@mvandenburgh mvandenburgh requested review from danlamanna and jjnesbitt and removed request for jjnesbitt December 1, 2023 18:54
@mvandenburgh
Copy link
Member Author

I'm working on fixing the failing tests.

@mvandenburgh mvandenburgh force-pushed the fix-user-metadata-creation branch 2 times, most recently from 70ac7bf to a3cfb11 Compare December 1, 2023 19:15
@jjnesbitt
Copy link
Member

After testing this locally, it doesn't seem the post_save signal is actually dispatched when the createsuperuser command is used.

@mvandenburgh
Copy link
Member Author

After testing this locally, it doesn't seem the post_save signal is actually dispatched when the createsuperuser command is used.

Oh, maybe that was why I closed this before 😄
I was testing with the UI, I didn't notice that one of the motivations was to make it work with createsuperuser as well. Sorry for the confusion

@waxlamp
Copy link
Member

waxlamp commented Dec 11, 2023

Can we do something super dumb, like include another management command to run after createsuperuser that patches up the new superuser with the required stuff?

@mvandenburgh mvandenburgh force-pushed the fix-user-metadata-creation branch 2 times, most recently from 6bf3886 to d4542b6 Compare January 1, 2024 18:02
@mvandenburgh
Copy link
Member Author

I attempted to fix this with 6b5b6e4, but it's throwing some error during tests. I think ultimately this is something we need to fix upstream in composed-configuration.

@mvandenburgh mvandenburgh merged commit 3cc675d into master Feb 26, 2024
10 checks passed
@mvandenburgh mvandenburgh deleted the fix-user-metadata-creation branch February 26, 2024 21:48
@dandibot
Copy link
Member

🚀 PR was released in v0.3.77 🚀

@dandibot dandibot added the released This issue/pull request has been released. label Feb 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released This issue/pull request has been released.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Should guard better for user metadata being malformed/missing?
4 participants