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

check user state when fetching to avoid dealing with offline objects, fixes #9502 #9640

Merged
merged 1 commit into from
May 29, 2018

Conversation

blizzz
Copy link
Member

@blizzz blizzz commented May 28, 2018

The problem stems from a refactor done for Nextcloud 13 to fasten up retrieving the users from LDAP. Instead of applying data every time a user is fetched, this is now done in a background job. However in that process, if retrieved users were flagged as deleted, the flag was removed. This does not happen anymore, leading this into this bug.

Reproduction steps:

  1. Have LDAP configured
  2. visit users page
  3. Manually set a user as deleted by: occ user:setting $USERID user_ldap isDeleted 1 (you can get the $USERID from ldap_user_mappings table)
  4. (Optional): check the user is listed in occ ldap:show-remnants
  5. go to users page

before this patch it just stays empty, Call to undefined method OCA\\User_LDAP\\User\\OfflineUser::composeAndStoreDisplayName() is logged. Now, the users are all shown, and the flag was removed which is indicated by the missing the user not listed anymore when you call occ ldap:show-remnants again.

fixes #9502

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good 👍

Copy link
Member

@rullzer rullzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@blizzz blizzz merged commit c0ed6f9 into master May 29, 2018
@blizzz blizzz deleted the fix/9502/missedstatecheck branch May 29, 2018 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants