Skip to content

delete role when ownership delete#2154

Closed
grubolsch wants to merge 1 commit intoIII-6975/fix-replay-role-addedfrom
III-6975/remove-role-when-ownership-deleted
Closed

delete role when ownership delete#2154
grubolsch wants to merge 1 commit intoIII-6975/fix-replay-role-addedfrom
III-6975/remove-role-when-ownership-deleted

Conversation

@grubolsch
Copy link
Contributor

Added

Changed

Removed

Fixed


Ticket: https://jira.uitdatabank.be/browse/...

$this->{$handleMethod}($event, $domainMessage);
}

public function deleteRoleIfNoOwnerships(?Uuid $roleId): void
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this a public method?

{
// Search for any ownerships (approved state only) that are linked to this role
// We look for approved ownerships since only approved ownerships have members in the role
$ownerships = $this->ownershipSearchRepository->search(
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure this will work because you only search for 100 ownerships. It also seems overkill to iterate over all these ownerships.

Should we not just get the remaining users on the role after the RemoveUser? If the last user on a role is removed the role can also be deleted? It doesn't matter that ownerships still point to this deleted role because it gets a soft delete.

Copy link
Contributor

Choose a reason for hiding this comment

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

Another approach would be to add the roleId as a SearchParameter so you can verify if no other Ownership references this Role

@lucwollants
Copy link
Contributor

I think we can close this since we have some nice tickets https://jira.publiq.be/browse/III-7029 and https://jira.publiq.be/browse/III-7030

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.

2 participants