-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
feat(ldap): sync additional properties to profile and SAB #45512
Conversation
Pending documentation at nextcloud/documentation#11323 (although it has to be adjusted to the removed anniversary field). |
5463fdc
to
8855b2b
Compare
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.
What kind of LDAP attribute was this tested with?
I’m afraid it won’t work with LDAP date format which is quite specific and not supported by PHP out-of-the-box.
Côme made a good point. There is no standardized birthdate field on LDAP. However, some implementations use the standardized generalized date format while others user the So we need a parser for the LDAP format and fallback to the date constructor. |
@come-nc I tested it with an attribute using the same Generalized Time syntax that you linked and it worked fine for both showing the date in the frontend and generating it on contact cards for the cardDAV server. Here is the full attribute type I use:
|
Using the two examples from the RFC the second one crashes DateTime constructor: Try setting your attribute to "199412160532-0500" which is a valid value from the RFC. |
I added a rudimentary parser, that just uses the first 8 characters from the LDAP generalized syntax and tries to parse YYYYMMDD or YYYY-MM-DD otherwise. Is that sufficient? I also tested it with a real LDAP generalized time attribute on my local dev instance. Of course, I also added some unit test for the parser method ... |
7d8a467
to
e1b5855
Compare
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.
Looks good
some bundle updates are suspicious. e.g. dist/weather_status-weather-status.js changed
Synced from LDAP to profile: - Date of birth Synced from LDAP to SAB (via the profile): - Biography - Date of birth Original code by Jake Nabasny (GitHub: @slapcat) Co-authored-by: Jake Nabasny <jake@nabasny.com> Co-authored-by: Richard Steinmetz <richard@steinmetz.cloud> Signed-off-by: Richard Steinmetz <richard@steinmetz.cloud>
abef449
to
f863290
Compare
🎉🎉🎉🎉🎉 |
/** | ||
* @since 30.0.0 | ||
*/ | ||
public const PROPERTY_BIRTHDATE = 'birthdate'; |
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.
@st3iny Please document in Please document in https://docs.nextcloud.com/server/latest/developer_manual/app_publishing_maintenance/app_upgrade_guide/upgrade_to_30.html#id1
Will this change allow people to add their birthday to their profile even if the nextcloud server uses not LDAP but the default database? |
@aurisnoctis Yes, it does work without LDAP. |
Summary
This PR merges #41793 and #39592. I added neccessary frontend code so that users can change their date of births and adjust the scope in case they don't want it to be synced to the SAB.
I also removed the anniversary date after having a discussion with our designers. We are required to take the detour via the profile page to allow users to set the scope. Considering the broad spectrum that Nextcloud is used in, it doesn't make sense to have an anniversary date on the profile.
Synced from LDAP to profile:
Synced from LDAP to SAB (via the profile):
Original code by Jake Nabasny (GitHub: @slapcat). Your contribution is highly valued ❤️
Screenshots
Synced contact in SAB
Date of birth in personal settings
LDAP wizard (advanced tab)
TODO
Checklist