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

MPP-3390: Update django-allauth to 0.60.0 #4246

Merged
merged 7 commits into from
Jan 9, 2024

Conversation

jwhitlock
Copy link
Member

@jwhitlock jwhitlock commented Dec 18, 2023

This updates django-allauth from 0.54.0 to 0.59.0 for MPP-3390.

Changes needed for upgrades:

  • Set older emails as verified. New emails are auto-verified with PR MPP-3390: Auto-verify emails #4111
  • 0.55 - This update made changes to allow adding OpenID providers in the Django admin rather than only in the settings.
    • Two fields, provider_id and settings, were added to socialaccount_socialapp to support OpenID providers in the Django admin. These are both non-nullable with Django-level defaults. A new migration adds database-level defaults for our migration tests.
    • There was a change in how a Socialauth Provider is created. This required changes in the terms_accepted_view, which was manually creating a FirefoxAccountsProvider in order to create a new account.
    • A working SocialApp is now needed for some tests which previously did not need them.
  • 0.56 - This update captures the request in a global context, so the code doesn't need to pass it around as much.
    • Add a required middleware to load the request into a global context
    • This update added a case-insensitive index on email, so we can drop the one we added.
  • 0.57 - No Relay changes required
  • 0.58 - No Relay changes required
  • 0.59 - No Relay changes required
  • 0.60 - No Relay changes required

@jwhitlock jwhitlock force-pushed the update-django-allauth-mpp-3390 branch from c5f6e42 to 17fed21 Compare December 18, 2023 20:40
@jwhitlock jwhitlock force-pushed the update-django-allauth-mpp-3390 branch from 17fed21 to f9306da Compare January 8, 2024 17:29
@jwhitlock jwhitlock changed the title MPP-3390: Update django-allauth to 0.59.0 MPP-3390: Update django-allauth to 0.60.0 Jan 8, 2024
@jwhitlock
Copy link
Member Author

Force-push to rebase and add 0.60.0 update.

@jwhitlock jwhitlock force-pushed the update-django-allauth-mpp-3390 branch from f9306da to ac63fa0 Compare January 9, 2024 19:15
All historical accounts that arrived from FxA have emails that were
verified before the Relay account was created. Before django-allauth
0.55.0, Relay ignored email verification.
With this update, a provider that has a database-backed SocialApp (for
example, for OpenID client credentials) needs to be initialized with
that SocialApp. Switch to using the socialaccount app adapter to load
the FirefoxAccountProvider, and setup the SocialApp in tests.

With this update, non-null provider_id and settings fields are added to
the socialaccount_socialapp table. This is OK as long as a new social
app is not added during a database migration. For Relay testing, add a
migration with default values.
This release requires a new allauth middleware
We added indexes on upper(email) for the account_emailaddress and
auth_user tables. The upper(email) index for account_emailaddress
was also added by django-allauth in 0.56.0. This migration removes
Relay's copy of that index.
@jwhitlock jwhitlock force-pushed the update-django-allauth-mpp-3390 branch from ac63fa0 to e496846 Compare January 9, 2024 19:21
@jwhitlock
Copy link
Member Author

With PR #4177 merged, this is ready for review.

@jwhitlock jwhitlock requested a review from groovecoder January 9, 2024 19:41
Copy link
Member

@groovecoder groovecoder left a comment

Choose a reason for hiding this comment

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

Code looks almost 100% good but I have a blocking concern/question on the migration that removes the index.

Spot-checks: sign-up with a new account works, and sign into existing account works too.

api/tests/views_tests.py Show resolved Hide resolved
api/tests/views_tests.py Outdated Show resolved Hide resolved
api/views/__init__.py Show resolved Hide resolved
Copy link
Member Author

@jwhitlock jwhitlock left a comment

Choose a reason for hiding this comment

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

Thanks @groovecoder. I've made the suggested test changes and answered some questions about the migrations.

@jwhitlock jwhitlock requested a review from groovecoder January 9, 2024 22:42
@groovecoder groovecoder added this pull request to the merge queue Jan 9, 2024
Merged via the queue into main with commit 0229b7c Jan 9, 2024
25 checks passed
@groovecoder groovecoder deleted the update-django-allauth-mpp-3390 branch January 9, 2024 23:33
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