Delete only HookTasks of the repository
#17981
Conversation
| @@ -766,7 +766,6 @@ func DeleteRepository(doer *user_model.User, uid, repoID int64) error { | |||
| &Comment{RefRepoID: repoID}, | |||
| &CommitStatus{RepoID: repoID}, | |||
| &DeletedBranch{RepoID: repoID}, | |||
| &webhook.HookTask{RepoID: repoID}, | |||
There was a problem hiding this comment.
But a HookTask belongs to this repository which produced because of an org webhook should also be deleted? Otherwise we will have dirty records?
There was a problem hiding this comment.
A HookTask belongs to a Webhook. If a system or organization webhook generates a HookTask I would be surprised if this history entry of the webhook gets deleted just because the repository was deleted. The entry should only be deleted if the system/org webhook gets deleted.
There was a problem hiding this comment.
The hook_task should belong to webhook but also repository. That means, deleting webhook will delete all webhook generated hook tasks. Deleting repository will delete all hook tasks for the repository.
For an individual repository, a webhook will generate hooktask for only one repository. So there is no problem whatever delete repository or delete webhook, we could delete all hooktask according webhook id. But for organization repository, the webhook maybe created and stay a long time(maybe forever? :)). But some repositories created and some repositories deleted. (btw: a possible bug maybe when create a new repository, the system webhook and organization webhook didn't apply into them, I have to check.) So maybe you have never chance to delete all these hook tasks.
There was a problem hiding this comment.
The hook_task should belong to webhook but also repository. That means, deleting webhook will delete all webhook generated hook tasks. Deleting repository will delete all hook tasks for the repository.
Same is true for an issue comment but you do not delete the comment just because you delete the user. The comment is related to the issue like the hooktask to the webhook. The history entry of a webhook should not get deleted just because an unused reference is deleted. You don't remove the system notices of a repository just because the repository isn't there anymore.
But for organization repository, the webhook maybe created and stay a long time(maybe forever? :)).
Webhooks could stay a long time. The hook tasks of a webhook live up to 7 days. There is a cronjob which deletes them.
GiteaBot
added
the
lgtm/need 2
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 months. Thank you for your contributions. |
|
go away bot |
lunny
added
the
issue/confirmed
|
I still think add a |
|
Since #17940 merged, I think maybe this could be closed? |
|
ping ~~~ |
wxiaoguang
added
the
issue/needs-feedback
|
second ping ~~~ |
Extracted from #17940.
The old code deletes every
HookTaskassociated with the repo but that's wrong because it deletes tasks from system and organziation webhooks too.Generally I think it's better to delegate the management of dependencies to the correct struct (
WebHookin this case) which knows what to delete instead of trying to do this "extern" withDeleteBeans.