Skip to content

Conversation

@come-nc
Copy link
Contributor

@come-nc come-nc commented Nov 18, 2025

Local version of #55871 for CI

@come-nc come-nc added this to the Nextcloud 33 milestone Nov 18, 2025
@come-nc come-nc self-assigned this Nov 18, 2025
@come-nc come-nc requested a review from a team as a code owner November 18, 2025 10:41
@come-nc come-nc added the 3. to review Waiting for reviews label Nov 18, 2025
@come-nc come-nc requested review from nfebe, susnux and szaimen and removed request for a team November 18, 2025 10:41
@come-nc
Copy link
Contributor Author

come-nc commented Nov 18, 2025

/compile rebase

NikolausDemmel and others added 5 commits November 18, 2025 11:28
When editing a user, if groups have display names different from their
IDs (e.g., long group names that get hashed), the group selector was
showing the group ID instead of the display name after page reload.

This happened because the `data()` function initialized `userGroups`
with `{ id, name: id }`, setting both fields to the group ID. The fix
adds a `created()` lifecycle hook that looks up the actual group names
from the Vuex store.

Fixes #55785

Signed-off-by: Claude <noreply@anthropic.com>

Co-Authored-By: Nikolaus Demmel <nikolaus@nikolaus-demmel.de>
Signed-off-by: Nikolaus Demmel <nikolaus@nikolaus-demmel.de>
Add Cypress E2E test to verify that group display names are shown
correctly in the user editor, even when the group ID differs from
the display name (e.g., when long group names get hashed).

The test creates a group with a very long name to trigger Nextcloud's
ID hashing behavior, assigns a user to that group, then verifies that
after a page reload the display name is shown instead of the hash ID.

Related to #55785

Signed-off-by: Claude <noreply@anthropic.com>

Co-Authored-By: Nikolaus Demmel <nikolaus@nikolaus-demmel.de>
Signed-off-by: Nikolaus Demmel <nikolaus@nikolaus-demmel.de>
Signed-off-by: Nikolaus Demmel <nikolaus@nikolaus-demmel.de>
Replace crypto-random-string import with Nextcloud's built-in randomString
utility to match the pattern used in other Cypress tests.

Also add missing testIsolation flag and cleanup hook to follow test
conventions used throughout the codebase.

Signed-off-by: Nikolaus Demmel <nikolaus@nikolaus-demmel.de>
Signed-off-by: nextcloud-command <nextcloud-command@users.noreply.github.com>
@nextcloud-command nextcloud-command force-pushed the fix-group-display-names-55785 branch from 469f4e6 to 6b5064e Compare November 18, 2025 11:33
@kesselb
Copy link
Contributor

kesselb commented Nov 19, 2025

I think that #56524 (using the computed properties) is a bit cleaner.

CarlSchwan
CarlSchwan previously approved these changes Nov 21, 2025
@CarlSchwan CarlSchwan dismissed their stale review November 21, 2025 10:10

This is actually the wrong PR to approve

@come-nc
Copy link
Contributor Author

come-nc commented Nov 24, 2025

I think that #56524 (using the computed properties) is a bit cleaner.

Fine for me, I do not know frontend enough to have an opinion here.
Can you approve/merge/close each of the PRs accordingly?

@NikolausDemmel
Copy link

Thanks for creating a local version of the PR to run CI!

As the author of #55871, I don't mind if #56524 is preferred, as long as the issue is fixed 👍 . I don't have an opinion on which version is cleaner, since I don't know the code well enough.

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.

6 participants