Skip to content

fix(group): Deletion of groups handle correctly#2256

Merged
javirln merged 1 commit into
chainloop-dev:mainfrom
javirln:fix/pfm-3339
Jul 15, 2025
Merged

fix(group): Deletion of groups handle correctly#2256
javirln merged 1 commit into
chainloop-dev:mainfrom
javirln:fix/pfm-3339

Conversation

@javirln

@javirln javirln commented Jul 15, 2025

Copy link
Copy Markdown
Member

This patch fixes issues with group removal, where related resources were being left behind. Deleting a group now performs the following actions:

  • Marks the group as deleted
  • Marks all user group memberships as deleted
  • Removes all group-related entries from the membership table, including users with the maintainer role and group memberships in projects
  • Marks all pending invitations associated with the group as deleted, if any

It also creates a migration that fixes all current dangling groups if any in the system by performing the same steps as above.

Signed-off-by: Javier Rodriguez <javier@chainloop.dev>
@javirln javirln requested review from jiparis and migmartri July 15, 2025 06:08
@javirln javirln self-assigned this Jul 15, 2025

@migmartri migmartri left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks

membership.MemberID(groupID),
membership.And(
membership.ResourceID(groupID),
membership.ResourceTypeEQ(authz.ResourceTypeGroup),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

is this possible? a membership with type = group? Group memberships have their own table, right?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

replying myself, the maintainer role is stored here, thanks!

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes the maintainer one

@jiparis jiparis left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks.
As a side note, not for now, I would remove completely the deleted_at from group_membership table, as we don't have it in other "membership" tables.

@javirln javirln merged commit 174a8c5 into chainloop-dev:main Jul 15, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants