-
-
Notifications
You must be signed in to change notification settings - Fork 546
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
By default, set the fullname from first+last and vice-versa #683
base: master
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## master #683 +/- ##
==========================================
- Coverage 77.01% 76.94% -0.07%
==========================================
Files 319 319
Lines 9677 9687 +10
Branches 1036 1040 +4
==========================================
+ Hits 7453 7454 +1
- Misses 2072 2081 +9
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.
|
first, _space, last = details['fullname'].rpartition(' ') | ||
details['first_name'] = first.strip() | ||
details['last_name'] = last.strip() | ||
print(f"end social_names {details=}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(I'll need to remove this, but I'd like to confirm that the general direction seems good before I do the cleanup.)
Any thoughts on this general feature? |
for more information, see https://pre-commit.ci
Any thoughts? |
I think it makes sense. We do something similar in Weblate: https://github.com/WeblateOrg/weblate/blob/bc06af07cd83e00e0c1e8a914638b38f9952e375/weblate/accounts/pipeline.py#L465-L474 |
Fixes #678.
Proposed changes
As described in #678 , when the SAML backend supplies a
fullname
, it's desirable for my Django user to havefirst_name
andlast_name
set. This changes PSA so iffullname
is set (in, eg, a SAML response) but notfirst_name
/last_name
, by default it generates afirst_name
andlast_name
automatically. Similarly, it does the reverse transformation whenfullname
is missing. IMO this is a net improvement, but obviously there's questions like "should this be enabled by default", "should it be in the default pipeline", etc. -- I'm happy to tweak this to make it fit the team's design goals better. Assuming the concept makes sense, I can write docs.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.