From 7af2ccd5115caf8889ba20b61881c44694c82cb1 Mon Sep 17 00:00:00 2001 From: S7evinK Date: Thu, 3 Sep 2020 04:46:02 +0200 Subject: [PATCH] Avoid sending "0 new commits" webhooks (#12212) * Avoid sending "0 new commits" webhook Signed-off-by: Till Faelligen * Revert "Avoid sending "0 new commits" webhook" This reverts commit 1f47ccfacd81470e2c33a02bb8d255172aa4ec08. * Move commit count check to more central place * Make tests pass * Update modules/webhook/webhook.go Co-authored-by: Lauris BH Co-authored-by: Lauris BH Co-authored-by: zeripath Co-authored-by: techknowlogick --- modules/webhook/webhook.go | 8 ++++++++ modules/webhook/webhook_test.go | 4 ++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/modules/webhook/webhook.go b/modules/webhook/webhook.go index 97487214503..2ef150210e7 100644 --- 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 index e88e67e9bfe..10c32a9485b 100644 --- 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) }