Skip to content

Conversation

@benjaminfrueh
Copy link
Contributor

@benjaminfrueh benjaminfrueh commented Nov 19, 2025

Description

Fixes an issue where group IDs were displayed instead of display names when editing a user.

Problem

When a group is renamed (so id and displayName are different), the NcSelect component in edit mode shows the group ID instead of the display name. This happens because userGroups and userSubAdminGroups are initialised with name: id in the UserRowMixin.js

Solution

Convert userGroups and userSubAdminGroups to computed properties that look up the current groups from the store, ensuring they always have up-to-date display names.

Also removed selectedGroups and selectedSubAdminGroupsas they were not used.

@benjaminfrueh benjaminfrueh requested a review from a team as a code owner November 19, 2025 12:42
@benjaminfrueh benjaminfrueh requested review from juliusknorr, nfebe, sorbaugh and szaimen and removed request for a team November 19, 2025 12:42
@kesselb
Copy link
Contributor

kesselb commented Nov 19, 2025

Thanks for your pr 👍

I'm currently also working on a ticket related to user management and groups, and I took a brief look at your PR to avoid conflicts.

The groups on the left are lazy-loaded. On your branch, I'm only seeing the groups that are already loaded. However, users might be assigned to groups that are not lazy-loaded yet, and thus those are missing now.

Main

Screenshot From 2025-11-19 14-54-07

This branch

Screenshot From 2025-11-19 14-58-58

(the groups are shown as soon as one scrolls down in the groups list)

Helper script to create some test groups

for i in {1..30}; do
  occ group:add --display-name "Test Group $i" group$i
done

for i in {31..35}; do
  occ group:add --display-name "Test Group $i" group$i
  occ group:adduser group$i alice
  occ group:adduser group$i bob
  occ group:adduser group$i fry
  occ group:adduser group$i hermes
  occ group:adduser group$i bender
  occ group:adduser group$i professor
done

Copy link
Contributor

@susnux susnux left a comment

Choose a reason for hiding this comment

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

this is quite clean :)

@susnux
Copy link
Contributor

susnux commented Nov 19, 2025

For the lazy loaded groups it seems like there is a reactivity issue in the store - not caused by this PR but its visible now.

@NikolausDemmel
Copy link

I made my own attempt to fix the group display name issue in the admin page a few weeks ago (#55871), but it seems this PR is considered a cleaner solution (#56509 (comment)). That's great, I don't mind closing my PR.

This PR could reference issue #55785, which reported the discrepancy in displayed group names.

In my PR, I attempted to adapt one of the existing cypress test to create a regression test for the display name issue. Feel free to include that in your PR. But your call, of course. https://github.com/nextcloud/server/pull/56509/files#diff-434017d314dcaf501e8d0651a9ebd294c1d1c51864047867a3ef9abf06be6ffd

@NikolausDemmel
Copy link

If this is preferred over the other PR, could we also consider this for backporting to 32?

@github-actions
Copy link
Contributor

github-actions bot commented Dec 4, 2025

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

(If you believe you should not receive this message, you can add yourself to the blocklist.)

@NikolausDemmel
Copy link

Happy new year everyone! Could we please get this PR approved / merged and backported? 🙏

@nextcloud-bot nextcloud-bot mentioned this pull request Jan 7, 2026
@benjaminfrueh
Copy link
Contributor Author

/backport to stable32

@nextcloud-bot nextcloud-bot mentioned this pull request Jan 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants