Skip to content

perf: Replace more calls from getUserGroups to getUserGroupIds#61904

Open
CarlSchwan wants to merge 2 commits into
masterfrom
carl/groups-optimization
Open

perf: Replace more calls from getUserGroups to getUserGroupIds#61904
CarlSchwan wants to merge 2 commits into
masterfrom
carl/groups-optimization

Conversation

@CarlSchwan

Copy link
Copy Markdown
Member

Summary

Faster as we only need the gid and not the full group objects

TODO

  • ...

Checklist

AI (if applicable)

  • The content of this PR was partly or fully generated using AI

@CarlSchwan CarlSchwan added this to the Nextcloud 35 milestone Jul 8, 2026
@CarlSchwan CarlSchwan self-assigned this Jul 8, 2026
@CarlSchwan CarlSchwan requested a review from a team as a code owner July 8, 2026 08:31
@CarlSchwan CarlSchwan added the 3. to review Waiting for reviews label Jul 8, 2026
@CarlSchwan CarlSchwan requested review from Altahrim, ArtificialOwl, leftybournes and salmart-dev and removed request for a team July 8, 2026 08:31
@CarlSchwan CarlSchwan force-pushed the carl/groups-optimization branch 2 times, most recently from 7b3fa5b to 986ce7e Compare July 10, 2026 08:40
Faster as we only need the gid and not the full group objects

Signed-off-by: Carl Schwan <carlschwan@kde.org>
Signed-off-by: Carl Schwan <carlschwan@kde.org>
@CarlSchwan CarlSchwan force-pushed the carl/groups-optimization branch from 986ce7e to 8dd4c83 Compare July 10, 2026 09:45
Comment on lines +280 to +282
if ($displayName !== null) {
$info[] = ['id' => $groupId, 'displayname' => $displayName];
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
if ($displayName !== null) {
$info[] = ['id' => $groupId, 'displayname' => $displayName];
}
$info[] = ['id' => $groupId, 'displayname' => $displayName ?? $groupId];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What could be a reason for getGroupsObjects to return null as displayname?

The reason for #56533 was that a missing group object (in the store) makes the group assignment invisible. If you omit groups without a display, they are hidden again.

Edge case, but easy to avoid with a fallback to groupId as displayname.

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

Labels

3. to review Waiting for reviews

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants