Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Respect user enumeration settings in user status lists #27879
Respect user enumeration settings in user status lists #27879
Changes from all commits
80d472d
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 think this does not scale. We need to do it in another way. In worst case someone is in hundreds of (automated) groups, you run hundreds of queries to generate the user list to then later know that they don't even have a status set or something. Can't we do it similar to
lib/private/Collaboration/Collaborators/UserPlugin.php
and first get the recent user statuses and then filter out the results?Basically get the status of 100 people, then filter out, if less than the requested users remain retry
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.
Mhhh, you certainly have an argument here 😬
But I'm unsure whether the other way around scales better: It's rather unlikely that the desired 8 users from shared groups with recent status changes are amongh the first 100. So in many cases we'll end up iterating over all users with recent status changes. And for each user we have to check whether they're in a shared group.
So I see two approaches and I'm not sure which one scales better:
@nickvergessen you're way more experienced, so I'd trust you on what scales better on common setups/instances.
Besides: I'm not sure what you mean with your reference to
lib/private/Collaboration/Collaborators/UserPlugin.php
. I mostly copied its implementation to search for users from shared groups. Looking atlib/private/Collaboration/Collaborators/UserPlugin.php#L98-L115
, it iterates over all members of all groups that the user is member of just like the implementation in this PR, no? Maybe I miss something :grimaced: