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

Added support for concatenating multiple LDAP attributes in displayName #5295

Merged

Conversation

MatthieuLeboeuf
Copy link
Contributor

Hello

To resolve #1684 and the next of #5288

I've made the requested changes, but I can't seem to fix the test error. Do you have any ideas ?

@ssddanbrown ssddanbrown added this to the Next Feature Release milestone Dec 1, 2024
ssddanbrown added a commit that referenced this pull request Dec 1, 2024
Review of #5295
Added test to cover functionality.
Moved splitting from config to service.
@ssddanbrown ssddanbrown merged commit 87242ce into BookStackApp:development Dec 1, 2024
1 check passed
@ssddanbrown
Copy link
Member

ssddanbrown commented Dec 1, 2024

Thanks again for providing this @MatthieuLeboeuf!
I followed this up with 90341e0 just to mainly add test coverage for this feature.

Not certain what test was originally failing for you, but it might have been due to the ordering being changed via the merge of attributes when pulling back user data, which would have meant (after filtering away of a null thumbnail attribute) that the display name attribution in the test would be at a different index to what's expected.
In my commit I've altered this to be array expansion at the original array location instead.

This will be part of the next feature release.

Docs Updates

  • Document ability to use mulitple attributes via LDAP_DISPLAY_NAME_ATTRIBUTE="firstname|lastname" syntax.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

LDAP_DISPLAY_NAME_ATTRIBUTE merge multiple values
2 participants