From fb030624780f426472b8464aac0d50228f93017a Mon Sep 17 00:00:00 2001 From: Kemal Zebari <60799661+kemzeb@users.noreply.github.com> Date: Thu, 7 Nov 2024 11:56:53 -0800 Subject: [PATCH] Only provide the commit summary for Discord webhook push events (#32432) Resolves #32371. #31970 should have just showed the commit summary, but `strings.SplitN()` was misused such that we did not perform any splitting at all and just used the message. This was not caught in the unit test made in that PR since the test commit summary was > 50 (which truncated away the commit description). This snapshot resolves this and adds another unit test to ensure that we only show the commit summary. --- services/webhook/discord.go | 2 +- services/webhook/discord_test.go | 16 +++++++++++++++- services/webhook/general_test.go | 2 +- 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/services/webhook/discord.go b/services/webhook/discord.go index 59e87a7e1f..c562d98168 100644 --- a/services/webhook/discord.go +++ b/services/webhook/discord.go @@ -156,7 +156,7 @@ func (d discordConvertor) Push(p *api.PushPayload) (DiscordPayload, error) { // for each commit, generate attachment text for i, commit := range p.Commits { // limit the commit message display to just the summary, otherwise it would be hard to read - message := strings.TrimRight(strings.SplitN(commit.Message, "\n", 1)[0], "\r") + message := strings.TrimRight(strings.SplitN(commit.Message, "\n", 2)[0], "\r") // a limit of 50 is set because GitHub does the same if utf8.RuneCountInString(message) > 50 { diff --git a/services/webhook/discord_test.go b/services/webhook/discord_test.go index fbb4b24ef1..36b99d452e 100644 --- a/services/webhook/discord_test.go +++ b/services/webhook/discord_test.go @@ -80,12 +80,26 @@ func TestDiscordPayload(t *testing.T) { assert.Equal(t, p.Sender.AvatarURL, pl.Embeds[0].Author.IconURL) }) - t.Run("PushWithLongCommitMessage", func(t *testing.T) { + t.Run("PushWithMultilineCommitMessage", func(t *testing.T) { p := pushTestMultilineCommitMessagePayload() pl, err := dc.Push(p) require.NoError(t, err) + assert.Len(t, pl.Embeds, 1) + assert.Equal(t, "[test/repo:test] 2 new commits", pl.Embeds[0].Title) + assert.Equal(t, "[2020558](http://localhost:3000/test/repo/commit/2020558fe2e34debb818a514715839cabd25e778) chore: This is a commit summary - user1\n[2020558](http://localhost:3000/test/repo/commit/2020558fe2e34debb818a514715839cabd25e778) chore: This is a commit summary - user1", pl.Embeds[0].Description) + assert.Equal(t, p.Sender.UserName, pl.Embeds[0].Author.Name) + assert.Equal(t, setting.AppURL+p.Sender.UserName, pl.Embeds[0].Author.URL) + assert.Equal(t, p.Sender.AvatarURL, pl.Embeds[0].Author.IconURL) + }) + + t.Run("PushWithLongCommitSummary", func(t *testing.T) { + p := pushTestPayloadWithCommitMessage("This is a commit summary ⚠️⚠️⚠️⚠️ containing 你好 ⚠️⚠️️\n\nThis is the message body") + + pl, err := dc.Push(p) + require.NoError(t, err) + assert.Len(t, pl.Embeds, 1) assert.Equal(t, "[test/repo:test] 2 new commits", pl.Embeds[0].Title) assert.Equal(t, "[2020558](http://localhost:3000/test/repo/commit/2020558fe2e34debb818a514715839cabd25e778) This is a commit summary ⚠️⚠️⚠️⚠️ containing 你好... - user1\n[2020558](http://localhost:3000/test/repo/commit/2020558fe2e34debb818a514715839cabd25e778) This is a commit summary ⚠️⚠️⚠️⚠️ containing 你好... - user1", pl.Embeds[0].Description) diff --git a/services/webhook/general_test.go b/services/webhook/general_test.go index d6a77c9442..ef1ec7f324 100644 --- a/services/webhook/general_test.go +++ b/services/webhook/general_test.go @@ -68,7 +68,7 @@ func pushTestPayload() *api.PushPayload { } func pushTestMultilineCommitMessagePayload() *api.PushPayload { - return pushTestPayloadWithCommitMessage("This is a commit summary ⚠️⚠️⚠️⚠️ containing 你好 ⚠️⚠️️\n\nThis is the message body.") + return pushTestPayloadWithCommitMessage("chore: This is a commit summary\n\nThis is a commit description.") } func pushTestPayloadWithCommitMessage(message string) *api.PushPayload {