-
-
Notifications
You must be signed in to change notification settings - Fork 544
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
Allow per-backend user pipeline settings #677
Conversation
I'm confused that CI appears to have still not run. |
Codecov Report
@@ Coverage Diff @@
## master #677 +/- ##
=======================================
Coverage 77.01% 77.01%
=======================================
Files 319 319
Lines 9677 9677
Branches 1036 1036
=======================================
Hits 7453 7453
Misses 2072 2072
Partials 152 152
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Can you please prepare a matching pull request for the documentation? https://github.com/python-social-auth/social-docs/ |
I think this change just makes the code match the already-existing documentation -- see https://github.com/python-social-auth/social-docs/blame/master/docs/configuration/settings.rst#L18, "All settings can be defined per-backend". Can you clarify what you're looking for in terms of doc changes? |
I'm still running with this patch on one of my sites -- it looks like this PR has a conflict now, but (glancing at the conflict) it doesn't look hard to resolve, or like the conflict is from this issue having been fixed. I can go back and update this PR, but @nijel , can you elaborate on what you were looking for in terms of added docs, so I can resolve that too? |
I most likely missed your previous reply, it indeed seems that this PR makes it match the documentation. |
f493c68
to
3fb334e
Compare
According to https://python-social-auth.readthedocs.io/en/latest/configuration/settings.html, "All settings can be defined per-backend by adding the backend name to the setting name, like SOCIAL_AUTH_TWITTER_LOGIN_URL". This changes user.py to actually use per-backend settings for options like USERNAME_IS_FULL_EMAIL. Note that get_username is always called with a backend, but user_details isn't, so a different version of `setting' is needed for the two.
6d9b1b5
to
45f2913
Compare
for more information, see https://pre-commit.ci
I've rebased, if you want to take a look and maybe merge it. |
Merged, thanks for your contribution! |
According to https://python-social-auth.readthedocs.io/en/latest/configuration/settings.html, "All settings can be defined per-backend by adding the backend name to the setting name, like SOCIAL_AUTH_TWITTER_LOGIN_URL". This changes user.py to actually use per-backend settings for options like USERNAME_IS_FULL_EMAIL.
Note that get_username is always called with a backend, but user_details isn't, so a different version of `setting' is needed for the two.
Types of changes
Please check the type of change your PR introduces:
Checklist
Put an
x
in the boxes that apply. You can also fill these out after creatingthe PR. If you're unsure about any of them, don't hesitate to ask. We're here to
help! This is simply a reminder of what we are going to look for before merging
your code.
Other information
Any other information that is important to this PR such as screenshots of how
the component looks before and after the change.