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

fix: propagate group name changes #1829

Merged
merged 2 commits into from
Feb 20, 2025
Merged

Conversation

st3iny
Copy link
Member

@st3iny st3iny commented Jan 6, 2025

My attempt to fix #1828

Does not fully solve the linked issue.

Remaining problem: Existing members are not updated. Only the group circle.

@ArtificialOwl
Copy link
Member

I am not sure I see the the purpose of this change. the name only stores groupid, not the display name

@sorbaugh
Copy link
Contributor

@st3iny @ArtificialOwl can you provide each other a bit more context to move this PR forward, thank you very much 🙏

@st3iny
Copy link
Member Author

st3iny commented Jan 29, 2025

I linked the issue in the PR description. The issue contains concrete steps to reproduce the problem.

My observations: Renaming a group does not trigger renaming the mapped group inside circles. Somehow, it seems that the group rename event is not listened to and handled.

@sorbaugh
Copy link
Contributor

From the ticket:

Steps to reproduce

  1. Create a user group group-test.
  2. Add the user group to a circle.
  3. Rename the user group to group-renamed.
  4. Open the membership modal again.

Observe that the group is still listed with the former name group-test.

Expected behaviour

The new name should be used in Circles.

Actual behaviour

The initial user group name is used everywhere which is confusing.

Server configuration

Nextcloud version: master (as of today)

@ArtificialOwl ArtificialOwl force-pushed the fix/propagate-group-rename branch from dde3a4e to 83d3dbc Compare February 11, 2025 04:19
@sorbaugh sorbaugh requested a review from artonge February 14, 2025 10:54
@ArtificialOwl ArtificialOwl marked this pull request as ready for review February 14, 2025 10:55
Copy link
Member

@provokateurin provokateurin left a comment

Choose a reason for hiding this comment

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

Changes look sane and renaming groups does work correctly with the changes checked out.

@provokateurin
Copy link
Member

@st3iny cs-fixer is unhappy

st3iny and others added 2 commits February 20, 2025 09:20
Signed-off-by: Richard Steinmetz <[email protected]>


Signed-off-by: Maxence Lange <[email protected]>
@provokateurin provokateurin force-pushed the fix/propagate-group-rename branch from 83d3dbc to 528129b Compare February 20, 2025 08:21
@provokateurin provokateurin merged commit 8a3449f into master Feb 20, 2025
31 checks passed
@provokateurin provokateurin deleted the fix/propagate-group-rename branch February 20, 2025 08:26
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.

Renaming a user group is not reflected when the group is a member of a circle
5 participants