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

Populate User name field during social auth #3968

Merged
merged 6 commits into from
Jun 27, 2023

Conversation

Andrew-Chen-Wang
Copy link
Contributor

Description

The "name" field is not filled with social authentication using django allauth due to removing the first and last name field in the User model. This PR fills in the name field.

Checklist:

  • I've made sure that tests are updated accordingly (especially if adding or updating a template option)
  • I've updated the documentation or confirm that my change doesn't require any updates

Rationale

To fill in the name field during social auth

Copy link
Member

@browniebroke browniebroke left a comment

Choose a reason for hiding this comment

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

Do you have an exemple of a social provider I could use to test this without too much setup? Also, it would be nice if we could write an automated test for it, this is

Comment on lines +20 to +25
if name := data.get("name"):
user.name = name
elif first_name := data.get("first_name"):
user.name = first_name
if last_name := data.get("last_name"):
user.name += f" {last_name}"
Copy link
Member

Choose a reason for hiding this comment

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

I seems like it would be more consistent with django-allauth if we were to do it in the save_user() method: https://github.com/pennersr/django-allauth/blob/7060e3fb6e00f01bd881890546244670929b5cf3/allauth/account/adapter.py#L224-L251

Unless there a specific reason not to?

Copy link
Contributor Author

@Andrew-Chen-Wang Andrew-Chen-Wang Dec 24, 2022

Choose a reason for hiding this comment

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

It's been awhile, but I believe save_user isn't actually used for social registration (i.e. it's not called if you use auto signup):

save_user(self, request, sociallogin, form=None): Populates and saves the User instance (and related social login data). The signup form is not available in case of auto signup.

https://django-allauth.readthedocs.io/en/latest/advanced.html?highlight=populate_user#creating-and-populating-user-instances

@Andrew-Chen-Wang
Copy link
Contributor Author

Andrew-Chen-Wang commented Dec 24, 2022

Thanks for the review

Do you have an exemple of a social provider I could use to test this without too much setup?

I find Google the easiest and Twitter the second easiest to set up.

Also, it would be nice if we could write an automated test for it

We could add mock requests? Otherwise, not sure whether adding these OAuth providers and a bot for automated testing will trigger a captcha warning.

@browniebroke browniebroke merged commit 5a7de40 into cookiecutter:master Jun 27, 2023
@browniebroke browniebroke changed the title Populate user name field in social auth Populate User name field during social auth Jun 27, 2023
@Andrew-Chen-Wang Andrew-Chen-Wang deleted the patch-6 branch June 28, 2023 02:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants