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

Remove memberships when deleting tenant or project #94

Merged
merged 15 commits into from
Apr 24, 2024

Conversation

iljarotar
Copy link
Contributor

No description provided.

@iljarotar iljarotar requested a review from majst01 April 18, 2024 08:20
@iljarotar iljarotar requested a review from a team as a code owner April 18, 2024 08:20
pkg/service/project.go Show resolved Hide resolved
return nil, err
}
for _, m := range memberships {
err := s.projectMemberStore.Delete(ctx, m.Meta.Id)
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be possible to just delete these member through a single SQL query? I guess it's more robust than a loop.

Copy link
Contributor

Choose a reason for hiding this comment

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

This requires a additional Store Interface DeleteAll or so ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Don't know how difficult this is. Maybe we can return "Unimplemented" for project and tenant and just implement for the member stores?

Copy link
Contributor

Choose a reason for hiding this comment

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

i have i implementation, unsure if this should be pushed to this PR or create a new one

Copy link
Contributor

Choose a reason for hiding this comment

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

pushed here

Copy link
Contributor

Choose a reason for hiding this comment

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

done

@iljarotar
Copy link
Contributor Author

Is it necessary to delete project members when a tenant is deleted? I thought such cases should never occur, since it's not permitted to delete a tenant who still has projects.

@majst01
Copy link
Contributor

majst01 commented Apr 22, 2024

Is it necessary to delete project members when a tenant is deleted? I thought such cases should never occur, since it's not permitted to delete a tenant who still has projects.

This is correct, my bad, but should not hurt

@Gerrit91 Gerrit91 merged commit 45b55bd into master Apr 24, 2024
1 check passed
@Gerrit91 Gerrit91 deleted the cleanup-memberships branch April 24, 2024 08:50
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.

None yet

3 participants