forks/forgejo
1
0
Fork 0
Code Releases Activity

Fix repository create/delete event webhooks (#13008)

Browse source 
This small PR changes the webhook trigger behaviour to be more in line with what's expected. (When 'repository' events are enabled, of course)

In other words:

For system-wide or default webhooks, repository events will now trigger said webhook. Previously it had to be under an organization for create events to be visible - a tad unexpected!
Deleting a repository will now fire its own defined webhooks, not just organisational and system ones.
In order to enable the latter the webhook has to now be triggered before the actual repo undergoes deletion. I'm willing to tweak this to try and 'grab' the webhook model beforehand and trigger the webhook notifier directly afterwards, but this may make the code more complex for little benefit.

Closes #11766, #9180.
This commit is contained in:
James Lakin 2020-10-02 09:37:46 +00:00 committed by GitHub
parent 77f3dbed6d
commit 6fc129fe62
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 17 additions and 23 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
modules/notification/webhook/webhook.go
View file

@ -99,7 +99,6 @@ func (m *webhookNotifier) NotifyForkRepository(doer *models.User, oldRepo, repo
func (m *webhookNotifier) NotifyCreateRepository(doer *models.User, u *models.User, repo *models.Repository) { func (m *webhookNotifier) NotifyCreateRepository(doer *models.User, u *models.User, repo *models.Repository) {
// Add to hook queue for created repo after session commit. // Add to hook queue for created repo after session commit.
if u.IsOrganization() {
if err := webhook_module.PrepareWebhooks(repo, models.HookEventRepository, &api.RepositoryPayload{ if err := webhook_module.PrepareWebhooks(repo, models.HookEventRepository, &api.RepositoryPayload{
Action: api.HookRepoCreated, Action: api.HookRepoCreated,
Repository: repo.APIFormat(models.AccessModeOwner), Repository: repo.APIFormat(models.AccessModeOwner),
@ -108,13 +107,11 @@ func (m *webhookNotifier) NotifyCreateRepository(doer *models.User, u *models.Us
}); err != nil { }); err != nil {
log.Error("PrepareWebhooks [repo_id: %d]: %v", repo.ID, err) log.Error("PrepareWebhooks [repo_id: %d]: %v", repo.ID, err)
} }
}
} }
func (m *webhookNotifier) NotifyDeleteRepository(doer *models.User, repo *models.Repository) { func (m *webhookNotifier) NotifyDeleteRepository(doer *models.User, repo *models.Repository) {
u := repo.MustOwner() u := repo.MustOwner()
if u.IsOrganization() {
if err := webhook_module.PrepareWebhooks(repo, models.HookEventRepository, &api.RepositoryPayload{ if err := webhook_module.PrepareWebhooks(repo, models.HookEventRepository, &api.RepositoryPayload{
Action: api.HookRepoDeleted, Action: api.HookRepoDeleted,
Repository: repo.APIFormat(models.AccessModeOwner), Repository: repo.APIFormat(models.AccessModeOwner),
@ -123,7 +120,6 @@ func (m *webhookNotifier) NotifyDeleteRepository(doer *models.User, repo *models
}); err != nil { }); err != nil {
log.Error("PrepareWebhooks [repo_id: %d]: %v", repo.ID, err) log.Error("PrepareWebhooks [repo_id: %d]: %v", repo.ID, err)
} }
}
} }
func (m *webhookNotifier) NotifyIssueChangeAssignee(doer *models.User, issue *models.Issue, assignee *models.User, removed bool, comment *models.Comment) { func (m *webhookNotifier) NotifyIssueChangeAssignee(doer *models.User, issue *models.Issue, assignee *models.User, removed bool, comment *models.Comment) {

8
services/repository/repository.go
View file

@ -64,13 +64,11 @@ func DeleteRepository(doer *models.User, repo *models.Repository) error {
log.Error("CloseRepoBranchesPulls failed: %v", err) log.Error("CloseRepoBranchesPulls failed: %v", err)
} }
if err := models.DeleteRepository(doer, repo.OwnerID, repo.ID); err != nil { // If the repo itself has webhooks, we need to trigger them before deleting it...
return err
}
notification.NotifyDeleteRepository(doer, repo) notification.NotifyDeleteRepository(doer, repo)
return nil err := models.DeleteRepository(doer, repo.OwnerID, repo.ID)
return err
} }
// PushCreateRepo creates a repository when a new repository is pushed to an appropriate namespace // PushCreateRepo creates a repository when a new repository is pushed to an appropriate namespace