From fb3a6aba7de2a22e102636c010b7c1faf740f1d7 Mon Sep 17 00:00:00 2001 From: John Olheiser <42128690+jolheiser@users.noreply.github.com> Date: Sat, 4 Jan 2020 16:20:15 -0600 Subject: [PATCH] Only show sender if it makes sense (#9601) Signed-off-by: jolheiser --- modules/webhook/dingtalk.go | 8 +-- modules/webhook/discord.go | 8 +-- modules/webhook/general.go | 92 +++++++++++++++++++---------------- modules/webhook/msteams.go | 8 +-- modules/webhook/slack.go | 8 +-- modules/webhook/slack_test.go | 4 +- modules/webhook/telegram.go | 8 +-- 7 files changed, 71 insertions(+), 65 deletions(-) diff --git a/modules/webhook/dingtalk.go b/modules/webhook/dingtalk.go index 4869c1a37c..0d569f4120 100644 --- a/modules/webhook/dingtalk.go +++ b/modules/webhook/dingtalk.go @@ -132,7 +132,7 @@ func getDingtalkPushPayload(p *api.PushPayload) (*DingtalkPayload, error) { } func getDingtalkIssuesPayload(p *api.IssuePayload) (*DingtalkPayload, error) { - text, issueTitle, attachmentText, _ := getIssuesPayloadInfo(p, noneLinkFormatter) + text, issueTitle, attachmentText, _ := getIssuesPayloadInfo(p, noneLinkFormatter, true) return &DingtalkPayload{ MsgType: "actionCard", @@ -148,7 +148,7 @@ func getDingtalkIssuesPayload(p *api.IssuePayload) (*DingtalkPayload, error) { } func getDingtalkIssueCommentPayload(p *api.IssueCommentPayload) (*DingtalkPayload, error) { - text, issueTitle, _ := getIssueCommentPayloadInfo(p, noneLinkFormatter) + text, issueTitle, _ := getIssueCommentPayloadInfo(p, noneLinkFormatter, true) return &DingtalkPayload{ MsgType: "actionCard", @@ -163,7 +163,7 @@ func getDingtalkIssueCommentPayload(p *api.IssueCommentPayload) (*DingtalkPayloa } func getDingtalkPullRequestPayload(p *api.PullRequestPayload) (*DingtalkPayload, error) { - text, issueTitle, attachmentText, _ := getPullRequestPayloadInfo(p, noneLinkFormatter) + text, issueTitle, attachmentText, _ := getPullRequestPayloadInfo(p, noneLinkFormatter, true) return &DingtalkPayload{ MsgType: "actionCard", @@ -236,7 +236,7 @@ func getDingtalkRepositoryPayload(p *api.RepositoryPayload) (*DingtalkPayload, e } func getDingtalkReleasePayload(p *api.ReleasePayload) (*DingtalkPayload, error) { - text, _ := getReleasePayloadInfo(p, noneLinkFormatter) + text, _ := getReleasePayloadInfo(p, noneLinkFormatter, true) return &DingtalkPayload{ MsgType: "actionCard", diff --git a/modules/webhook/discord.go b/modules/webhook/discord.go index ea69f36fe7..c1e8421228 100644 --- a/modules/webhook/discord.go +++ b/modules/webhook/discord.go @@ -227,7 +227,7 @@ func getDiscordPushPayload(p *api.PushPayload, meta *DiscordMeta) (*DiscordPaylo } func getDiscordIssuesPayload(p *api.IssuePayload, meta *DiscordMeta) (*DiscordPayload, error) { - text, _, attachmentText, color := getIssuesPayloadInfo(p, noneLinkFormatter) + text, _, attachmentText, color := getIssuesPayloadInfo(p, noneLinkFormatter, false) return &DiscordPayload{ Username: meta.Username, @@ -249,7 +249,7 @@ func getDiscordIssuesPayload(p *api.IssuePayload, meta *DiscordMeta) (*DiscordPa } func getDiscordIssueCommentPayload(p *api.IssueCommentPayload, discord *DiscordMeta) (*DiscordPayload, error) { - text, _, color := getIssueCommentPayloadInfo(p, noneLinkFormatter) + text, _, color := getIssueCommentPayloadInfo(p, noneLinkFormatter, false) return &DiscordPayload{ Username: discord.Username, @@ -271,7 +271,7 @@ func getDiscordIssueCommentPayload(p *api.IssueCommentPayload, discord *DiscordM } func getDiscordPullRequestPayload(p *api.PullRequestPayload, meta *DiscordMeta) (*DiscordPayload, error) { - text, _, attachmentText, color := getPullRequestPayloadInfo(p, noneLinkFormatter) + text, _, attachmentText, color := getPullRequestPayloadInfo(p, noneLinkFormatter, false) return &DiscordPayload{ Username: meta.Username, @@ -368,7 +368,7 @@ func getDiscordRepositoryPayload(p *api.RepositoryPayload, meta *DiscordMeta) (* } func getDiscordReleasePayload(p *api.ReleasePayload, meta *DiscordMeta) (*DiscordPayload, error) { - text, color := getReleasePayloadInfo(p, noneLinkFormatter) + text, color := getReleasePayloadInfo(p, noneLinkFormatter, false) return &DiscordPayload{ Username: meta.Username, diff --git a/modules/webhook/general.go b/modules/webhook/general.go index 28c3b2730d..bc9a10b529 100644 --- a/modules/webhook/general.go +++ b/modules/webhook/general.go @@ -25,8 +25,7 @@ func htmlLinkFormatter(url string, text string) string { return fmt.Sprintf(`%s`, url, html.EscapeString(text)) } -func getIssuesPayloadInfo(p *api.IssuePayload, linkFormatter linkFormatter) (string, string, string, int) { - senderLink := linkFormatter(setting.AppURL+p.Sender.UserName, p.Sender.UserName) +func getIssuesPayloadInfo(p *api.IssuePayload, linkFormatter linkFormatter, withSender bool) (string, string, string, int) { repoLink := linkFormatter(p.Repository.HTMLURL, p.Repository.FullName) issueTitle := fmt.Sprintf("#%d %s", p.Index, p.Issue.Title) titleLink := linkFormatter(fmt.Sprintf("%s/issues/%d", p.Repository.HTMLURL, p.Index), issueTitle) @@ -35,34 +34,36 @@ func getIssuesPayloadInfo(p *api.IssuePayload, linkFormatter linkFormatter) (str switch p.Action { case api.HookIssueOpened: - text = fmt.Sprintf("[%s] Issue opened: %s by %s", repoLink, titleLink, senderLink) + text = fmt.Sprintf("[%s] Issue opened: %s", repoLink, titleLink) color = orangeColor case api.HookIssueClosed: - text = fmt.Sprintf("[%s] Issue closed: %s by %s", repoLink, titleLink, senderLink) + text = fmt.Sprintf("[%s] Issue closed: %s", repoLink, titleLink) color = redColor case api.HookIssueReOpened: - text = fmt.Sprintf("[%s] Issue re-opened: %s by %s", repoLink, titleLink, senderLink) + text = fmt.Sprintf("[%s] Issue re-opened: %s", repoLink, titleLink) case api.HookIssueEdited: - text = fmt.Sprintf("[%s] Issue edited: %s by %s", repoLink, titleLink, senderLink) + text = fmt.Sprintf("[%s] Issue edited: %s", repoLink, titleLink) case api.HookIssueAssigned: - text = fmt.Sprintf("[%s] Issue assigned to %s: %s by %s", repoLink, - linkFormatter(setting.AppURL+p.Issue.Assignee.UserName, p.Issue.Assignee.UserName), - titleLink, senderLink) + text = fmt.Sprintf("[%s] Issue assigned to %s: %s", repoLink, + linkFormatter(setting.AppURL+p.Issue.Assignee.UserName, p.Issue.Assignee.UserName), titleLink) color = greenColor case api.HookIssueUnassigned: - text = fmt.Sprintf("[%s] Issue unassigned: %s by %s", repoLink, titleLink, senderLink) + text = fmt.Sprintf("[%s] Issue unassigned: %s", repoLink, titleLink) case api.HookIssueLabelUpdated: - text = fmt.Sprintf("[%s] Issue labels updated: %s by %s", repoLink, titleLink, senderLink) + text = fmt.Sprintf("[%s] Issue labels updated: %s", repoLink, titleLink) case api.HookIssueLabelCleared: - text = fmt.Sprintf("[%s] Issue labels cleared: %s by %s", repoLink, titleLink, senderLink) + text = fmt.Sprintf("[%s] Issue labels cleared: %s", repoLink, titleLink) case api.HookIssueSynchronized: - text = fmt.Sprintf("[%s] Issue synchronized: %s by %s", repoLink, titleLink, senderLink) + text = fmt.Sprintf("[%s] Issue synchronized: %s", repoLink, titleLink) case api.HookIssueMilestoned: mileStoneLink := fmt.Sprintf("%s/milestone/%d", p.Repository.HTMLURL, p.Issue.Milestone.ID) - text = fmt.Sprintf("[%s] Issue milestoned to %s: %s by %s", repoLink, - linkFormatter(mileStoneLink, p.Issue.Milestone.Title), titleLink, senderLink) + text = fmt.Sprintf("[%s] Issue milestoned to %s: %s", repoLink, + linkFormatter(mileStoneLink, p.Issue.Milestone.Title), titleLink) case api.HookIssueDemilestoned: - text = fmt.Sprintf("[%s] Issue milestone cleared: %s by %s", repoLink, titleLink, senderLink) + text = fmt.Sprintf("[%s] Issue milestone cleared: %s", repoLink, titleLink) + } + if withSender { + text += fmt.Sprintf(" by %s", linkFormatter(setting.AppURL+p.Sender.UserName, p.Sender.UserName)) } var attachmentText string @@ -73,8 +74,7 @@ func getIssuesPayloadInfo(p *api.IssuePayload, linkFormatter linkFormatter) (str return text, issueTitle, attachmentText, color } -func getPullRequestPayloadInfo(p *api.PullRequestPayload, linkFormatter linkFormatter) (string, string, string, int) { - senderLink := linkFormatter(setting.AppURL+p.Sender.UserName, p.Sender.UserName) +func getPullRequestPayloadInfo(p *api.PullRequestPayload, linkFormatter linkFormatter, withSender bool) (string, string, string, int) { repoLink := linkFormatter(p.Repository.HTMLURL, p.Repository.FullName) issueTitle := fmt.Sprintf("#%d %s", p.Index, p.PullRequest.Title) titleLink := linkFormatter(p.PullRequest.URL, issueTitle) @@ -83,43 +83,45 @@ func getPullRequestPayloadInfo(p *api.PullRequestPayload, linkFormatter linkForm switch p.Action { case api.HookIssueOpened: - text = fmt.Sprintf("[%s] Pull request %s opened by %s", repoLink, titleLink, senderLink) + text = fmt.Sprintf("[%s] Pull request opened: %s", repoLink, titleLink) color = greenColor case api.HookIssueClosed: if p.PullRequest.HasMerged { - text = fmt.Sprintf("[%s] Pull request %s merged by %s", repoLink, titleLink, senderLink) + text = fmt.Sprintf("[%s] Pull request merged: %s", repoLink, titleLink) color = purpleColor } else { - text = fmt.Sprintf("[%s] Pull request %s closed by %s", repoLink, titleLink, senderLink) + text = fmt.Sprintf("[%s] Pull request closed: %s", repoLink, titleLink) color = redColor } case api.HookIssueReOpened: - text = fmt.Sprintf("[%s] Pull request %s re-opened by %s", repoLink, titleLink, senderLink) + text = fmt.Sprintf("[%s] Pull request re-opened: %s", repoLink, titleLink) case api.HookIssueEdited: - text = fmt.Sprintf("[%s] Pull request %s edited by %s", repoLink, titleLink, senderLink) + text = fmt.Sprintf("[%s] Pull request edited: %s", repoLink, titleLink) case api.HookIssueAssigned: list := make([]string, len(p.PullRequest.Assignees)) for i, user := range p.PullRequest.Assignees { list[i] = linkFormatter(setting.AppURL+user.UserName, user.UserName) } - text = fmt.Sprintf("[%s] Pull request %s assigned to %s by %s", repoLink, - strings.Join(list, ", "), - titleLink, senderLink) + text = fmt.Sprintf("[%s] Pull request assigned: %s to %s", repoLink, + strings.Join(list, ", "), titleLink) color = greenColor case api.HookIssueUnassigned: - text = fmt.Sprintf("[%s] Pull request %s unassigned by %s", repoLink, titleLink, senderLink) + text = fmt.Sprintf("[%s] Pull request unassigned: %s", repoLink, titleLink) case api.HookIssueLabelUpdated: - text = fmt.Sprintf("[%s] Pull request %s labels updated by %s", repoLink, titleLink, senderLink) + text = fmt.Sprintf("[%s] Pull request labels updated: %s", repoLink, titleLink) case api.HookIssueLabelCleared: - text = fmt.Sprintf("[%s] Pull request %s labels cleared by %s", repoLink, titleLink, senderLink) + text = fmt.Sprintf("[%s] Pull request labels cleared: %s", repoLink, titleLink) case api.HookIssueSynchronized: - text = fmt.Sprintf("[%s] Pull request %s synchronized by %s", repoLink, titleLink, senderLink) + text = fmt.Sprintf("[%s] Pull request synchronized: %s", repoLink, titleLink) case api.HookIssueMilestoned: mileStoneLink := fmt.Sprintf("%s/milestone/%d", p.Repository.HTMLURL, p.PullRequest.Milestone.ID) - text = fmt.Sprintf("[%s] Pull request %s milestoned to %s by %s", repoLink, - linkFormatter(mileStoneLink, p.PullRequest.Milestone.Title), titleLink, senderLink) + text = fmt.Sprintf("[%s] Pull request milestoned: %s to %s", repoLink, + linkFormatter(mileStoneLink, p.PullRequest.Milestone.Title), titleLink) case api.HookIssueDemilestoned: - text = fmt.Sprintf("[%s] Pull request %s milestone cleared by %s", repoLink, titleLink, senderLink) + text = fmt.Sprintf("[%s] Pull request milestone cleared: %s", repoLink, titleLink) + } + if withSender { + text += fmt.Sprintf(" by %s", linkFormatter(setting.AppURL+p.Sender.UserName, p.Sender.UserName)) } var attachmentText string @@ -130,28 +132,29 @@ func getPullRequestPayloadInfo(p *api.PullRequestPayload, linkFormatter linkForm return text, issueTitle, attachmentText, color } -func getReleasePayloadInfo(p *api.ReleasePayload, linkFormatter linkFormatter) (text string, color int) { - senderLink := linkFormatter(setting.AppURL+p.Sender.UserName, p.Sender.UserName) +func getReleasePayloadInfo(p *api.ReleasePayload, linkFormatter linkFormatter, withSender bool) (text string, color int) { repoLink := linkFormatter(p.Repository.HTMLURL, p.Repository.FullName) refLink := linkFormatter(p.Repository.HTMLURL+"/src/"+p.Release.TagName, p.Release.TagName) switch p.Action { case api.HookReleasePublished: - text = fmt.Sprintf("[%s] Release %s created by %s", repoLink, refLink, senderLink) + text = fmt.Sprintf("[%s] Release created: %s", repoLink, refLink) color = greenColor case api.HookReleaseUpdated: - text = fmt.Sprintf("[%s] Release %s updated by %s", repoLink, refLink, senderLink) + text = fmt.Sprintf("[%s] Release updated: %s", repoLink, refLink) color = yellowColor case api.HookReleaseDeleted: - text = fmt.Sprintf("[%s] Release %s deleted by %s", repoLink, refLink, senderLink) + text = fmt.Sprintf("[%s] Release deleted: %s", repoLink, refLink) color = redColor } + if withSender { + text += fmt.Sprintf(" by %s", linkFormatter(setting.AppURL+p.Sender.UserName, p.Sender.UserName)) + } return text, color } -func getIssueCommentPayloadInfo(p *api.IssueCommentPayload, linkFormatter linkFormatter) (string, string, int) { - senderLink := linkFormatter(setting.AppURL+p.Sender.UserName, p.Sender.UserName) +func getIssueCommentPayloadInfo(p *api.IssueCommentPayload, linkFormatter linkFormatter, withSender bool) (string, string, int) { repoLink := linkFormatter(p.Repository.HTMLURL, p.Repository.FullName) issueTitle := fmt.Sprintf("#%d %s", p.Issue.Index, p.Issue.Title) @@ -168,18 +171,21 @@ func getIssueCommentPayloadInfo(p *api.IssueCommentPayload, linkFormatter linkFo switch p.Action { case api.HookIssueCommentCreated: - text = fmt.Sprintf("[%s] New comment on %s %s by %s", repoLink, typ, titleLink, senderLink) + text = fmt.Sprintf("[%s] New comment on %s %s", repoLink, typ, titleLink) if p.IsPull { color = greenColorLight } else { color = orangeColorLight } case api.HookIssueCommentEdited: - text = fmt.Sprintf("[%s] Comment on %s %s edited by %s", repoLink, typ, titleLink, senderLink) + text = fmt.Sprintf("[%s] Comment edited on %s %s", repoLink, typ, titleLink) case api.HookIssueCommentDeleted: - text = fmt.Sprintf("[%s] Comment on %s %s deleted by %s", repoLink, typ, titleLink, senderLink) + text = fmt.Sprintf("[%s] Comment deleted on %s %s", repoLink, typ, titleLink) color = redColor } + if withSender { + text += fmt.Sprintf(" by %s", linkFormatter(setting.AppURL+p.Sender.UserName, p.Sender.UserName)) + } return text, issueTitle, color } diff --git a/modules/webhook/msteams.go b/modules/webhook/msteams.go index 4c148421fe..b47725209d 100644 --- a/modules/webhook/msteams.go +++ b/modules/webhook/msteams.go @@ -266,7 +266,7 @@ func getMSTeamsPushPayload(p *api.PushPayload) (*MSTeamsPayload, error) { } func getMSTeamsIssuesPayload(p *api.IssuePayload) (*MSTeamsPayload, error) { - text, _, attachmentText, color := getIssuesPayloadInfo(p, noneLinkFormatter) + text, _, attachmentText, color := getIssuesPayloadInfo(p, noneLinkFormatter, false) return &MSTeamsPayload{ Type: "MessageCard", @@ -308,7 +308,7 @@ func getMSTeamsIssuesPayload(p *api.IssuePayload) (*MSTeamsPayload, error) { } func getMSTeamsIssueCommentPayload(p *api.IssueCommentPayload) (*MSTeamsPayload, error) { - text, _, color := getIssueCommentPayloadInfo(p, noneLinkFormatter) + text, _, color := getIssueCommentPayloadInfo(p, noneLinkFormatter, false) return &MSTeamsPayload{ Type: "MessageCard", @@ -350,7 +350,7 @@ func getMSTeamsIssueCommentPayload(p *api.IssueCommentPayload) (*MSTeamsPayload, } func getMSTeamsPullRequestPayload(p *api.PullRequestPayload) (*MSTeamsPayload, error) { - text, _, attachmentText, color := getPullRequestPayloadInfo(p, noneLinkFormatter) + text, _, attachmentText, color := getPullRequestPayloadInfo(p, noneLinkFormatter, false) return &MSTeamsPayload{ Type: "MessageCard", @@ -503,7 +503,7 @@ func getMSTeamsRepositoryPayload(p *api.RepositoryPayload) (*MSTeamsPayload, err } func getMSTeamsReleasePayload(p *api.ReleasePayload) (*MSTeamsPayload, error) { - text, color := getReleasePayloadInfo(p, noneLinkFormatter) + text, color := getReleasePayloadInfo(p, noneLinkFormatter, false) return &MSTeamsPayload{ Type: "MessageCard", diff --git a/modules/webhook/slack.go b/modules/webhook/slack.go index 508cb13b8f..11ad4c1b8b 100644 --- a/modules/webhook/slack.go +++ b/modules/webhook/slack.go @@ -144,7 +144,7 @@ func getSlackForkPayload(p *api.ForkPayload, slack *SlackMeta) (*SlackPayload, e } func getSlackIssuesPayload(p *api.IssuePayload, slack *SlackMeta) (*SlackPayload, error) { - text, issueTitle, attachmentText, color := getIssuesPayloadInfo(p, SlackLinkFormatter) + text, issueTitle, attachmentText, color := getIssuesPayloadInfo(p, SlackLinkFormatter, true) pl := &SlackPayload{ Channel: slack.Channel, @@ -167,7 +167,7 @@ func getSlackIssuesPayload(p *api.IssuePayload, slack *SlackMeta) (*SlackPayload } func getSlackIssueCommentPayload(p *api.IssueCommentPayload, slack *SlackMeta) (*SlackPayload, error) { - text, issueTitle, color := getIssueCommentPayloadInfo(p, SlackLinkFormatter) + text, issueTitle, color := getIssueCommentPayloadInfo(p, SlackLinkFormatter, true) return &SlackPayload{ Channel: slack.Channel, @@ -184,7 +184,7 @@ func getSlackIssueCommentPayload(p *api.IssueCommentPayload, slack *SlackMeta) ( } func getSlackReleasePayload(p *api.ReleasePayload, slack *SlackMeta) (*SlackPayload, error) { - text, _ := getReleasePayloadInfo(p, SlackLinkFormatter) + text, _ := getReleasePayloadInfo(p, SlackLinkFormatter, true) return &SlackPayload{ Channel: slack.Channel, @@ -239,7 +239,7 @@ func getSlackPushPayload(p *api.PushPayload, slack *SlackMeta) (*SlackPayload, e } func getSlackPullRequestPayload(p *api.PullRequestPayload, slack *SlackMeta) (*SlackPayload, error) { - text, issueTitle, attachmentText, color := getPullRequestPayloadInfo(p, SlackLinkFormatter) + text, issueTitle, attachmentText, color := getPullRequestPayloadInfo(p, SlackLinkFormatter, true) pl := &SlackPayload{ Channel: slack.Channel, diff --git a/modules/webhook/slack_test.go b/modules/webhook/slack_test.go index fe4f1384fc..a418c8e2e2 100644 --- a/modules/webhook/slack_test.go +++ b/modules/webhook/slack_test.go @@ -70,7 +70,7 @@ func TestSlackReleasePayload(t *testing.T) { require.Nil(t, err) require.NotNil(t, pl) - assert.Equal(t, "[] Release created by ", pl.Text) + assert.Equal(t, "[] Release created: by ", pl.Text) } func TestSlackPullRequestPayload(t *testing.T) { @@ -84,5 +84,5 @@ func TestSlackPullRequestPayload(t *testing.T) { require.Nil(t, err) require.NotNil(t, pl) - assert.Equal(t, "[] Pull request opened by ", pl.Text) + assert.Equal(t, "[] Pull request opened: by ", pl.Text) } diff --git a/modules/webhook/telegram.go b/modules/webhook/telegram.go index a98d47d55c..42adb40be2 100644 --- a/modules/webhook/telegram.go +++ b/modules/webhook/telegram.go @@ -125,7 +125,7 @@ func getTelegramPushPayload(p *api.PushPayload) (*TelegramPayload, error) { } func getTelegramIssuesPayload(p *api.IssuePayload) (*TelegramPayload, error) { - text, _, attachmentText, _ := getIssuesPayloadInfo(p, htmlLinkFormatter) + text, _, attachmentText, _ := getIssuesPayloadInfo(p, htmlLinkFormatter, true) return &TelegramPayload{ Message: text + "\n\n" + attachmentText, @@ -133,7 +133,7 @@ func getTelegramIssuesPayload(p *api.IssuePayload) (*TelegramPayload, error) { } func getTelegramIssueCommentPayload(p *api.IssueCommentPayload) (*TelegramPayload, error) { - text, _, _ := getIssueCommentPayloadInfo(p, htmlLinkFormatter) + text, _, _ := getIssueCommentPayloadInfo(p, htmlLinkFormatter, true) return &TelegramPayload{ Message: text + "\n" + p.Comment.Body, @@ -141,7 +141,7 @@ func getTelegramIssueCommentPayload(p *api.IssueCommentPayload) (*TelegramPayloa } func getTelegramPullRequestPayload(p *api.PullRequestPayload) (*TelegramPayload, error) { - text, _, attachmentText, _ := getPullRequestPayloadInfo(p, htmlLinkFormatter) + text, _, attachmentText, _ := getPullRequestPayloadInfo(p, htmlLinkFormatter, true) return &TelegramPayload{ Message: text + "\n" + attachmentText, @@ -166,7 +166,7 @@ func getTelegramRepositoryPayload(p *api.RepositoryPayload) (*TelegramPayload, e } func getTelegramReleasePayload(p *api.ReleasePayload) (*TelegramPayload, error) { - text, _ := getReleasePayloadInfo(p, htmlLinkFormatter) + text, _ := getReleasePayloadInfo(p, htmlLinkFormatter, true) return &TelegramPayload{ Message: text + "\n",