From 7928f404b4821b14bd581d35b7305331b7f69122 Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Sun, 16 Oct 2022 18:22:34 +0200 Subject: [PATCH] Display total commit count in hook message (#21400) Fixes #21379 The commits are capped by `setting.UI.FeedMaxCommitNum` so `len(commits)` is not the correct number. So this PR adds a new `TotalCommits` field. Co-authored-by: wxiaoguang --- modules/notification/webhook/webhook.go | 38 +++++++++++++------------ modules/structs/hook.go | 19 +++++++------ routers/api/v1/repo/hook.go | 19 +++++++------ routers/web/repo/webhook.go | 19 +++++++------ services/webhook/dingtalk.go | 8 +++--- services/webhook/dingtalk_test.go | 2 +- services/webhook/discord.go | 4 +-- services/webhook/general_test.go | 13 +++++---- services/webhook/matrix.go | 4 +-- services/webhook/msteams.go | 6 ++-- services/webhook/slack.go | 4 +-- services/webhook/telegram.go | 4 +-- services/webhook/wechatwork.go | 2 +- 13 files changed, 74 insertions(+), 68 deletions(-) diff --git a/modules/notification/webhook/webhook.go b/modules/notification/webhook/webhook.go index be71d18fdad3d..e61929b4cf277 100644 --- a/modules/notification/webhook/webhook.go +++ b/modules/notification/webhook/webhook.go @@ -608,15 +608,16 @@ func (m *webhookNotifier) NotifyPushCommits(pusher *user_model.User, repo *repo_ } if err := webhook_services.PrepareWebhooks(repo, webhook.HookEventPush, &api.PushPayload{ - Ref: opts.RefFullName, - Before: opts.OldCommitID, - After: opts.NewCommitID, - CompareURL: setting.AppURL + commits.CompareURL, - Commits: apiCommits, - HeadCommit: apiHeadCommit, - Repo: convert.ToRepo(repo, perm.AccessModeOwner), - Pusher: apiPusher, - Sender: apiPusher, + Ref: opts.RefFullName, + Before: opts.OldCommitID, + After: opts.NewCommitID, + CompareURL: setting.AppURL + commits.CompareURL, + Commits: apiCommits, + TotalCommits: commits.Len, + HeadCommit: apiHeadCommit, + Repo: convert.ToRepo(repo, perm.AccessModeOwner), + Pusher: apiPusher, + Sender: apiPusher, }); err != nil { log.Error("PrepareWebhooks: %v", err) } @@ -838,15 +839,16 @@ func (m *webhookNotifier) NotifySyncPushCommits(pusher *user_model.User, repo *r } if err := webhook_services.PrepareWebhooks(repo, webhook.HookEventPush, &api.PushPayload{ - Ref: opts.RefFullName, - Before: opts.OldCommitID, - After: opts.NewCommitID, - CompareURL: setting.AppURL + commits.CompareURL, - Commits: apiCommits, - HeadCommit: apiHeadCommit, - Repo: convert.ToRepo(repo, perm.AccessModeOwner), - Pusher: apiPusher, - Sender: apiPusher, + Ref: opts.RefFullName, + Before: opts.OldCommitID, + After: opts.NewCommitID, + CompareURL: setting.AppURL + commits.CompareURL, + Commits: apiCommits, + TotalCommits: commits.Len, + HeadCommit: apiHeadCommit, + Repo: convert.ToRepo(repo, perm.AccessModeOwner), + Pusher: apiPusher, + Sender: apiPusher, }); err != nil { log.Error("PrepareWebhooks: %v", err) } diff --git a/modules/structs/hook.go b/modules/structs/hook.go index 07d51915dea9f..4d186c2ca2ca0 100644 --- a/modules/structs/hook.go +++ b/modules/structs/hook.go @@ -267,15 +267,16 @@ func (p *ReleasePayload) JSONPayload() ([]byte, error) { // PushPayload represents a payload information of push event. type PushPayload struct { - Ref string `json:"ref"` - Before string `json:"before"` - After string `json:"after"` - CompareURL string `json:"compare_url"` - Commits []*PayloadCommit `json:"commits"` - HeadCommit *PayloadCommit `json:"head_commit"` - Repo *Repository `json:"repository"` - Pusher *User `json:"pusher"` - Sender *User `json:"sender"` + Ref string `json:"ref"` + Before string `json:"before"` + After string `json:"after"` + CompareURL string `json:"compare_url"` + Commits []*PayloadCommit `json:"commits"` + TotalCommits int `json:"total_commits"` + HeadCommit *PayloadCommit `json:"head_commit"` + Repo *Repository `json:"repository"` + Pusher *User `json:"pusher"` + Sender *User `json:"sender"` } // JSONPayload FIXME diff --git a/routers/api/v1/repo/hook.go b/routers/api/v1/repo/hook.go index b17142c0f647f..86361817cb464 100644 --- a/routers/api/v1/repo/hook.go +++ b/routers/api/v1/repo/hook.go @@ -169,15 +169,16 @@ func TestHook(ctx *context.APIContext) { commitID := ctx.Repo.Commit.ID.String() if err := webhook_service.PrepareWebhook(hook, ctx.Repo.Repository, webhook.HookEventPush, &api.PushPayload{ - Ref: ref, - Before: commitID, - After: commitID, - CompareURL: setting.AppURL + ctx.Repo.Repository.ComposeCompareURL(commitID, commitID), - Commits: []*api.PayloadCommit{commit}, - HeadCommit: commit, - Repo: convert.ToRepo(ctx.Repo.Repository, perm.AccessModeNone), - Pusher: convert.ToUserWithAccessMode(ctx.Doer, perm.AccessModeNone), - Sender: convert.ToUserWithAccessMode(ctx.Doer, perm.AccessModeNone), + Ref: ref, + Before: commitID, + After: commitID, + CompareURL: setting.AppURL + ctx.Repo.Repository.ComposeCompareURL(commitID, commitID), + Commits: []*api.PayloadCommit{commit}, + TotalCommits: 1, + HeadCommit: commit, + Repo: convert.ToRepo(ctx.Repo.Repository, perm.AccessModeNone), + Pusher: convert.ToUserWithAccessMode(ctx.Doer, perm.AccessModeNone), + Sender: convert.ToUserWithAccessMode(ctx.Doer, perm.AccessModeNone), }); err != nil { ctx.Error(http.StatusInternalServerError, "PrepareWebhook: ", err) return diff --git a/routers/web/repo/webhook.go b/routers/web/repo/webhook.go index 2b9f78ab018e0..78e2b65a96de9 100644 --- a/routers/web/repo/webhook.go +++ b/routers/web/repo/webhook.go @@ -1273,15 +1273,16 @@ func TestWebhook(ctx *context.Context) { commitID := commit.ID.String() p := &api.PushPayload{ - Ref: git.BranchPrefix + ctx.Repo.Repository.DefaultBranch, - Before: commitID, - After: commitID, - CompareURL: setting.AppURL + ctx.Repo.Repository.ComposeCompareURL(commitID, commitID), - Commits: []*api.PayloadCommit{apiCommit}, - HeadCommit: apiCommit, - Repo: convert.ToRepo(ctx.Repo.Repository, perm.AccessModeNone), - Pusher: apiUser, - Sender: apiUser, + Ref: git.BranchPrefix + ctx.Repo.Repository.DefaultBranch, + Before: commitID, + After: commitID, + CompareURL: setting.AppURL + ctx.Repo.Repository.ComposeCompareURL(commitID, commitID), + Commits: []*api.PayloadCommit{apiCommit}, + TotalCommits: 1, + HeadCommit: apiCommit, + Repo: convert.ToRepo(ctx.Repo.Repository, perm.AccessModeNone), + Pusher: apiUser, + Sender: apiUser, } if err := webhook_service.PrepareWebhook(w, ctx.Repo.Repository, webhook.HookEventPush, p); err != nil { ctx.Flash.Error("PrepareWebhook: " + err.Error()) diff --git a/services/webhook/dingtalk.go b/services/webhook/dingtalk.go index 642cf6f2fda16..0fc61ce8c1394 100644 --- a/services/webhook/dingtalk.go +++ b/services/webhook/dingtalk.go @@ -67,14 +67,14 @@ func (d *DingtalkPayload) Push(p *api.PushPayload) (api.Payloader, error) { ) var titleLink, linkText string - if len(p.Commits) == 1 { + if p.TotalCommits == 1 { commitDesc = "1 new commit" titleLink = p.Commits[0].URL - linkText = fmt.Sprintf("view commit %s", p.Commits[0].ID[:7]) + linkText = "view commit" } else { - commitDesc = fmt.Sprintf("%d new commits", len(p.Commits)) + commitDesc = fmt.Sprintf("%d new commits", p.TotalCommits) titleLink = p.CompareURL - linkText = fmt.Sprintf("view commit %s...%s", p.Commits[0].ID[:7], p.Commits[len(p.Commits)-1].ID[:7]) + linkText = "view commits" } if titleLink == "" { titleLink = p.Repo.HTMLURL + "/src/" + util.PathEscapeSegments(branchName) diff --git a/services/webhook/dingtalk_test.go b/services/webhook/dingtalk_test.go index b66b5e43a824d..e411043364ede 100644 --- a/services/webhook/dingtalk_test.go +++ b/services/webhook/dingtalk_test.go @@ -82,7 +82,7 @@ func TestDingTalkPayload(t *testing.T) { assert.Equal(t, "[2020558](http://localhost:3000/test/repo/commit/2020558fe2e34debb818a514715839cabd25e778) commit message - user1\r\n[2020558](http://localhost:3000/test/repo/commit/2020558fe2e34debb818a514715839cabd25e778) commit message - user1", pl.(*DingtalkPayload).ActionCard.Text) assert.Equal(t, "[test/repo:test] 2 new commits", pl.(*DingtalkPayload).ActionCard.Title) - assert.Equal(t, "view commit 2020558...2020558", pl.(*DingtalkPayload).ActionCard.SingleTitle) + assert.Equal(t, "view commits", pl.(*DingtalkPayload).ActionCard.SingleTitle) assert.Equal(t, "http://localhost:3000/test/repo/src/test", parseRealSingleURL(pl.(*DingtalkPayload).ActionCard.SingleURL)) }) diff --git a/services/webhook/discord.go b/services/webhook/discord.go index ae5460b9a7a1b..f65a8648c435a 100644 --- a/services/webhook/discord.go +++ b/services/webhook/discord.go @@ -141,11 +141,11 @@ func (d *DiscordPayload) Push(p *api.PushPayload) (api.Payloader, error) { ) var titleLink string - if len(p.Commits) == 1 { + if p.TotalCommits == 1 { commitDesc = "1 new commit" titleLink = p.Commits[0].URL } else { - commitDesc = fmt.Sprintf("%d new commits", len(p.Commits)) + commitDesc = fmt.Sprintf("%d new commits", p.TotalCommits) titleLink = p.CompareURL } if titleLink == "" { diff --git a/services/webhook/general_test.go b/services/webhook/general_test.go index 4d73afe060a68..a2606e4aca734 100644 --- a/services/webhook/general_test.go +++ b/services/webhook/general_test.go @@ -82,12 +82,13 @@ func pushTestPayload() *api.PushPayload { } return &api.PushPayload{ - Ref: "refs/heads/test", - Before: "2020558fe2e34debb818a514715839cabd25e777", - After: "2020558fe2e34debb818a514715839cabd25e778", - CompareURL: "", - HeadCommit: commit, - Commits: []*api.PayloadCommit{commit, commit}, + Ref: "refs/heads/test", + Before: "2020558fe2e34debb818a514715839cabd25e777", + After: "2020558fe2e34debb818a514715839cabd25e778", + CompareURL: "", + HeadCommit: commit, + Commits: []*api.PayloadCommit{commit, commit}, + TotalCommits: 2, Repo: &api.Repository{ HTMLURL: "http://localhost:3000/test/repo", Name: "repo", diff --git a/services/webhook/matrix.go b/services/webhook/matrix.go index a42ab2a93e063..37608f22bef2d 100644 --- a/services/webhook/matrix.go +++ b/services/webhook/matrix.go @@ -154,10 +154,10 @@ func (m *MatrixPayloadUnsafe) Release(p *api.ReleasePayload) (api.Payloader, err func (m *MatrixPayloadUnsafe) Push(p *api.PushPayload) (api.Payloader, error) { var commitDesc string - if len(p.Commits) == 1 { + if p.TotalCommits == 1 { commitDesc = "1 commit" } else { - commitDesc = fmt.Sprintf("%d commits", len(p.Commits)) + commitDesc = fmt.Sprintf("%d commits", p.TotalCommits) } repoLink := MatrixLinkFormatter(p.Repo.HTMLURL, p.Repo.FullName) diff --git a/services/webhook/msteams.go b/services/webhook/msteams.go index 59e2e93493982..856e31b4195bb 100644 --- a/services/webhook/msteams.go +++ b/services/webhook/msteams.go @@ -124,11 +124,11 @@ func (m *MSTeamsPayload) Push(p *api.PushPayload) (api.Payloader, error) { ) var titleLink string - if len(p.Commits) == 1 { + if p.TotalCommits == 1 { commitDesc = "1 new commit" titleLink = p.Commits[0].URL } else { - commitDesc = fmt.Sprintf("%d new commits", len(p.Commits)) + commitDesc = fmt.Sprintf("%d new commits", p.TotalCommits) titleLink = p.CompareURL } if titleLink == "" { @@ -155,7 +155,7 @@ func (m *MSTeamsPayload) Push(p *api.PushPayload) (api.Payloader, error) { text, titleLink, greenColor, - &MSTeamsFact{"Commit count:", fmt.Sprintf("%d", len(p.Commits))}, + &MSTeamsFact{"Commit count:", fmt.Sprintf("%d", p.TotalCommits)}, ), nil } diff --git a/services/webhook/slack.go b/services/webhook/slack.go index 11e1d3c081c28..0975bba1edcc8 100644 --- a/services/webhook/slack.go +++ b/services/webhook/slack.go @@ -171,10 +171,10 @@ func (s *SlackPayload) Push(p *api.PushPayload) (api.Payloader, error) { commitString string ) - if len(p.Commits) == 1 { + if p.TotalCommits == 1 { commitDesc = "1 new commit" } else { - commitDesc = fmt.Sprintf("%d new commits", len(p.Commits)) + commitDesc = fmt.Sprintf("%d new commits", p.TotalCommits) } if len(p.CompareURL) > 0 { commitString = SlackLinkFormatter(p.CompareURL, commitDesc) diff --git a/services/webhook/telegram.go b/services/webhook/telegram.go index 64211493ec62c..97b8a4636e56e 100644 --- a/services/webhook/telegram.go +++ b/services/webhook/telegram.go @@ -89,11 +89,11 @@ func (t *TelegramPayload) Push(p *api.PushPayload) (api.Payloader, error) { ) var titleLink string - if len(p.Commits) == 1 { + if p.TotalCommits == 1 { commitDesc = "1 new commit" titleLink = p.Commits[0].URL } else { - commitDesc = fmt.Sprintf("%d new commits", len(p.Commits)) + commitDesc = fmt.Sprintf("%d new commits", p.TotalCommits) titleLink = p.CompareURL } if titleLink == "" { diff --git a/services/webhook/wechatwork.go b/services/webhook/wechatwork.go index de8b777066576..29d866695a4fb 100644 --- a/services/webhook/wechatwork.go +++ b/services/webhook/wechatwork.go @@ -93,7 +93,7 @@ func (f *WechatworkPayload) Push(p *api.PushPayload) (api.Payloader, error) { for i, commit := range p.Commits { var authorName string if commit.Author != nil { - authorName = "Author:" + commit.Author.Name + authorName = "Author: " + commit.Author.Name } message := strings.ReplaceAll(commit.Message, "\n\n", "\r\n")