Avoid sending "0 new commits" webhooks (#12212)

* Avoid sending "0 new commits" webhook Signed-off-by: Till Faelligen <[email protected]> * Revert "Avoid sending "0 new commits" webhook" This reverts commit 1f47ccf. * Move commit count check to more central place * Make tests pass * Update modules/webhook/webhook.go Co-authored-by: Lauris BH <[email protected]> Co-authored-by: Lauris BH <[email protected]> Co-authored-by: zeripath <[email protected]> Co-authored-by: techknowlogick <[email protected]>
go-gitea · Sep 3, 2020 · 7af2ccd · 7af2ccd
1 parent 514201a
commit 7af2ccd
Show file tree

Hide file tree

Showing 2 changed files with 10 additions and 2 deletions.
diff --git a/modules/webhook/webhook.go b/modules/webhook/webhook.go
@@ -76,6 +76,14 @@ func prepareWebhook(w *models.Webhook, repo *models.Repository, event models.Hoo
 		}
 	}
 
+	// Avoid sending "0 new commits" to non-integration relevant webhooks (e.g. slack, discord, etc.).
+	// Integration webhooks (e.g. drone) still receive the required data.
+	if pushEvent, ok := p.(*api.PushPayload); ok &&
+		w.HookTaskType != models.GITEA && w.HookTaskType != models.GOGS &&
+		len(pushEvent.Commits) == 0 {
+		return nil
+	}
+
 	// If payload has no associated branch (e.g. it's a new tag, issue, etc.),
 	// branch filter has no effect.
 	if branch := getPayloadBranch(p); branch != "" {

diff --git a/modules/webhook/webhook_test.go b/modules/webhook/webhook_test.go
@@ -34,7 +34,7 @@ func TestPrepareWebhooks(t *testing.T) {
 	for _, hookTask := range hookTasks {
 		models.AssertNotExistsBean(t, hookTask)
 	}
-	assert.NoError(t, PrepareWebhooks(repo, models.HookEventPush, &api.PushPayload{}))
+	assert.NoError(t, PrepareWebhooks(repo, models.HookEventPush, &api.PushPayload{Commits: []*api.PayloadCommit{{}}}))
 	for _, hookTask := range hookTasks {
 		models.AssertExistsAndLoadBean(t, hookTask)
 	}
@@ -51,7 +51,7 @@ func TestPrepareWebhooksBranchFilterMatch(t *testing.T) {
 		models.AssertNotExistsBean(t, hookTask)
 	}
 	// this test also ensures that * doesn't handle / in any special way (like shell would)
-	assert.NoError(t, PrepareWebhooks(repo, models.HookEventPush, &api.PushPayload{Ref: "refs/heads/feature/7791"}))
+	assert.NoError(t, PrepareWebhooks(repo, models.HookEventPush, &api.PushPayload{Ref: "refs/heads/feature/7791", Commits: []*api.PayloadCommit{{}}}))
 	for _, hookTask := range hookTasks {
 		models.AssertExistsAndLoadBean(t, hookTask)
 	}