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

[8450] update django-allauth #5779

Merged
merged 2 commits into from
Dec 3, 2024
Merged

[8450] update django-allauth #5779

merged 2 commits into from
Dec 3, 2024

Conversation

goapunk
Copy link
Contributor

@goapunk goapunk commented Dec 3, 2024

Describe your changes
Update django-allauth to latest version and adapt to changes. Remove the unused social settings and forms.

Tasks

  • PR name contains story or task reference
  • Steps to recreate and test the changes
  • Documentation (docs and inline)
  • Tests (including n+1 and django_assert_num_queries where applicable)
  • Changelog

@goapunk goapunk changed the title update django-allauth [8450] update django-allauth Dec 3, 2024
@goapunk goapunk requested a review from m4ra December 3, 2024 10:30
Copy link
Contributor

@m4ra m4ra left a comment

Choose a reason for hiding this comment

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

if socialaccount is not used, then lets remove it also from the settings INSTALLED_APPS.
There is also a change on how the user is logged in after clicking the link
Verifying the email address by clicking on the link would no longer log you in, even in case of ACCOUNT_LOGIN_ON_EMAIL_CONFIRMATION = True. from changelog. Good to add this in the changelog maybe? We need to inform also PM?

@goapunk
Copy link
Contributor Author

goapunk commented Dec 3, 2024

if socialaccount is not used, then lets remove it also from the settings INSTALLED_APPS.

yes, good point!

There is also a change on how the user is logged in after clicking the link Verifying the email address by clicking on the link would no longer log you in, even in case of ACCOUNT_LOGIN_ON_EMAIL_CONFIRMATION = True. from changelog. Good to add this in the changelog maybe? We need to inform also PM?

To me this reads like a bugfix, they fixed that ACCOUNT_LOGIN_ON_EMAIL_CONFIRMATION=True is now working again as intended after it broke in a previous release, so the intended behaviour still the same. I will test if the bug is on stage and if it behaves different to this PR. If it's the same I guess we don't need to tell PMs

@goapunk goapunk force-pushed the jd-2024-12-update-allauth branch from cb724b0 to 427e39f Compare December 3, 2024 15:13
@goapunk goapunk requested a review from m4ra December 3, 2024 15:14
@goapunk
Copy link
Contributor Author

goapunk commented Dec 3, 2024

@m4ra I tested the behaviour on stage against this PR and for both you get automatically logged in when verifying your email, so I think we missed the version of allauth where it broke :)

@m4ra
Copy link
Contributor

m4ra commented Dec 3, 2024

@goapunk I tested already on stage, and logs you in after verifying email. So should be fine

@goapunk
Copy link
Contributor Author

goapunk commented Dec 3, 2024

@goapunk I tested already on stage, and logs you in after verifying email. So should be fine

ah, thanks!

Copy link
Contributor

@m4ra m4ra left a comment

Choose a reason for hiding this comment

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

thanks!

@goapunk
Copy link
Contributor Author

goapunk commented Dec 3, 2024

@m4ra hmm, now the tests are failing :/ maybe I leave the social account stuff in for now ? We usually have the policy to remove unused code but also this might be out of scope for the release story

@goapunk goapunk force-pushed the jd-2024-12-update-allauth branch from 427e39f to f184f71 Compare December 3, 2024 15:40
@goapunk goapunk requested a review from m4ra December 3, 2024 15:40
@m4ra
Copy link
Contributor

m4ra commented Dec 3, 2024

Leave it out yes. Maybe create an gh issue

@goapunk
Copy link
Contributor Author

goapunk commented Dec 3, 2024

created #5828

@m4ra m4ra merged commit 4cd24c1 into main Dec 3, 2024
2 of 3 checks passed
@m4ra m4ra deleted the jd-2024-12-update-allauth branch December 3, 2024 15:52
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.

2 participants