fix(grouping): Fix project-deletion-triggered `GroupHash` deletion
This is a follow-up to https://github.com/getsentry/sentry/pull/76312, fixing another bug with grouphash deletion. Currently, when a project is deleted, its associated GroupHash records are deleted using the BulkModelDeletionTask. Unfortunately, that task doesn't take into account any dependent tables, meaning that the corresponding GroupHashMetadata records aren't deleted as they should be.
This fixes that by no longer specifying which deletion task project deletion should use for GroupHash records, which then allows use of the GroupHash-specific deletion task registered in the deletion module's __init__.py.
Note that while the results of this change aren't tested in this PR, they are implicitly tested in https://github.com/getsentry/sentry/pull/76245, which will directly follow this one and which for the first time will actually create GroupHashMetadata records. Without the change in this PR, the presence of that PR's new GroupHashMetadata records causes the project deletion test to fail, but with this change, they pass.
is the deletion of GroupHash still being done in bulk, and if not, is this a concern? I see that GroupHashDeletionTask extends ModelDeletionTask
is the deletion of
GroupHashstill being done in bulk, and if not, is this a concern? I see thatGroupHashDeletionTaskextendsModelDeletionTask
So there are two ways a GroupHash deletion can be triggered - by deleting a group or by deleting a project. When it's a group that's deleted, it's already using the grouphash-specific deletion task (which, as you say, isn't bulk). And deleting a project deletes all the groups, so another option here would be to just remove GroupHash from the project deletion task, and let the GroupHash and GroupHashMetadata deletes cascade from the Group deletion.
In any case, no, I don't think it's an issue, because if you look at that __init__ file where all the deletion tasks are registered (that I linked in the PR description), lots of models have bespoke deletion tasks, and they all inherit from ModelDeletionTask. Also, we have no choice here, because (as is the case with many of the models which have custom tasks), the delete has to cascade, and the way that happens is by subclassing and overriding the get_child_relations method (which is called by ModelDeletionTask but not by BulkModelDeletionClass).
UPDATE: I decided it actually is cleaner to just let the groups handle the deletion, so I've switched to that approach and updated the PR description.
are there appropriate tests to ensure that groups still get deleted when a project is?
are there appropriate tests to ensure that groups still get deleted when a project is?
That groups get deleted? Yeah, we test that here. But why would changing what triggers GroupHash deletion change the deletion of groups?
are there appropriate tests to ensure that groups still get deleted when a project is?
That groups get deleted? Yeah, we test that here. But why would changing what triggers
GroupHashdeletion change the deletion of groups?
sorry, meant Group
sorry, meant
Group
Yeah, no, I know. That's what you said originally. And we do test for it, in the spot I linked above. I'm just confused why that's a question here, since we're not changing anything about group deletion.