Do not enqueue EntityLeaveWorker recursively
What does this MR do and why?
Fixes the problem in https://gitlab.com/gitlab-org/gitlab/-/issues/393444:
Currently, when you remove a member from a group via the API or the UI, you can specify this option:
Also remove direct user membership from subgroups and projects
When this is checked, the member destroy process runs "recursively", in the sense that for all the descendant groups that has Manoj has a direct member, the destroy service (DestroyService) is invoked once for each such occurrence.
When this service runs recursively, from within the Members::DestroyService, the function enqueue_delete_todos(member) is also being run recursively, where member changes in each recursive call. In each call, member is a Member object of subgroups where Manoj is a direct member.
Unfortunately, running enqueue_delete_todos(member) recursively means that we enqueue a TodosDestroyer::EntityLeaveWorker job for every single time this is run, and it cannot be deduplicated by middleware, since the arguments to each of these jobs differ every time. This causes the problem we see in https://gitlab.com/gitlab-org/gitlab/-/issues/393444
The fix
From my understanding basis the code of EntityLeaveService, which is the service that TodosDestroyer::EntityLeaveWorker uses internally, we do not have to really call this service recursively for each subgroup membership that is being removed.
The EntityLeaveService is capable of figuring out the current state of memberships and access of the user down the hierarchy and correcting them and removing un-accessible todos, even if we supply just the top-most group once. Which means that we have an opportunity here to remove enqueuing TodosDestroyer::EntityLeaveWorker recursively, and just enqueue it just once, for the top-most group in the hierarchy (where the member is actually deleted from), and it should fix the entire state of TODOs down it's hierarchy, which is what this MR does.
Screenshots or screen recordings
Screenshots are required for UI changes, and strongly recommended for all other merge requests.
How to set up and validate locally
Numbered steps to set up and validate the change are strongly suggested.
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.