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

Fix sscpac/chat-locker#14. user search not returning users. #16

Merged
merged 2 commits into from
Jul 29, 2015

Conversation

rwakida
Copy link

@rwakida rwakida commented Jul 25, 2015

RocketChat searches the users collection on the 'name' field. however, we
didn't set the value when importing ldap. Updated directoryservice to
set the name field using ldap username.

rwakida added 2 commits July 24, 2015 21:08
RocketChat searches the users collection on the 'name' field.  however, we
didn't set the value when importing ldap.  Updated directoryservice to
set the name field using ldap username.
ldap's givenName and sn fields.

- Replaced confusing field mapping with functions that directly convert an
  LDAP entry to an Rocket.Chat user object.
- Set name field to use ldap's given name and sn fields, and also set in
  profile.first_name and profile.last_name to aid in search by first or
  last name.
@rwakida rwakida force-pushed the bugfix/chat-locker-14 branch from a43483c to ae3df35 Compare July 28, 2015 01:49
@bbrockman bbrockman modified the milestone: Product Backlog Jul 28, 2015
@douglasrapp
Copy link

Looks good to me. I like the refactor and commenting for the LDAP attribute mapping code - much cleaner and easier to read. After deploying, I see the "name" field populated, and it gets appropriately pulled when I perform a user search inside the app.

rwakida added a commit that referenced this pull request Jul 29, 2015
Fix #14.  user search not returning users.
@rwakida rwakida merged commit 8f87101 into master Jul 29, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants