Skip to content

Commit 31e7aa6

Browse files
patka-123Earl Warren
patka-123
authored and
Earl Warren
committed
Reduce links in chat notifications to avoid multiple previews (go-gitea#6908)
Some messages had multiple links resulting in multiple previews per message. The superfluous links have been removed leaving only the most important link. All info the other links provided can be immediately accessed through the main link that is left over. The fork and push messages still have multiple links because from the former you want to see where the fork originates from, the latter has a link per commit. Resolves go-gitea#162 ## Checklist The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org). ### Tests - I added test coverage for Go changes... - [x] in their respective `*_test.go` for unit tests. - [ ] in the `tests/integration` directory if it involves interactions with a live Forgejo server. - I added test coverage for JavaScript changes... - [ ] in `web_src/js/*.test.js` if it can be unit tested. - [ ] in `tests/e2e/*.test.e2e.js` if it requires interactions with a live Forgejo server (see also the [developer guide for JavaScript testing](https://codeberg.org/forgejo/forgejo/src/branch/forgejo/tests/e2e/README.md#end-to-end-tests)). ### Documentation - [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change. - [x] I did not document these changes and I do not expect someone else to do it. ### Release notes - [ ] I do not want this change to show in the release notes. - [x] I want the title to show in the release notes with a link to this pull request. - [ ] I want the content of the `release-notes/<pull request number>.md` to be be used for the release notes instead of the title. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/6908 Reviewed-by: Gusted <[email protected]> Co-authored-by: patka <[email protected]> Co-committed-by: patka <[email protected]>
1 parent 06556ab commit 31e7aa6

File tree

8 files changed

+120
-147
lines changed

8 files changed

+120
-147
lines changed

‎services/webhook/deliver_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -150,12 +150,12 @@ func TestWebhookDeliverHookTask(t *testing.T) {
150150
require.NoError(t, err)
151151
assert.Equal(t, `{"data": 42}`, string(body))
152152

153-
case "/webhook/6db5dc1e282529a8c162c7fe93dd2667494eeb51":
153+
case "/webhook/86aaa4d69df5aa487cb0148af4ae7e546933057b":
154154
// Version 2
155155
assert.Equal(t, "application/json", r.Header.Get("Content-Type"))
156156
body, err := io.ReadAll(r.Body)
157157
require.NoError(t, err)
158-
assert.Len(t, body, 2147)
158+
assert.Len(t, body, 1909)
159159

160160
default:
161161
w.WriteHeader(404)

‎services/webhook/general.go‎

Lines changed: 41 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -92,47 +92,44 @@ func getIssuesCommentInfo(p *api.IssueCommentPayload) (title, link, by, operator
9292
}
9393

9494
func getIssuesPayloadInfo(p *api.IssuePayload, linkFormatter linkFormatter, withSender bool) (string, string, string, int) {
95-
repoLink := linkFormatter(p.Repository.HTMLURL, p.Repository.FullName)
9695
issueTitle := fmt.Sprintf("#%d %s", p.Index, p.Issue.Title)
9796
titleLink := linkFormatter(fmt.Sprintf("%s/issues/%d", p.Repository.HTMLURL, p.Index), issueTitle)
9897
var text string
9998
color := yellowColor
10099

101100
switch p.Action {
102101
case api.HookIssueOpened:
103-
text = fmt.Sprintf("[%s] Issue opened: %s", repoLink, titleLink)
102+
text = fmt.Sprintf("[%s] Issue opened: %s", p.Repository.FullName, titleLink)
104103
color = orangeColor
105104
case api.HookIssueClosed:
106-
text = fmt.Sprintf("[%s] Issue closed: %s", repoLink, titleLink)
105+
text = fmt.Sprintf("[%s] Issue closed: %s", p.Repository.FullName, titleLink)
107106
color = redColor
108107
case api.HookIssueReOpened:
109-
text = fmt.Sprintf("[%s] Issue re-opened: %s", repoLink, titleLink)
108+
text = fmt.Sprintf("[%s] Issue re-opened: %s", p.Repository.FullName, titleLink)
110109
case api.HookIssueEdited:
111-
text = fmt.Sprintf("[%s] Issue edited: %s", repoLink, titleLink)
110+
text = fmt.Sprintf("[%s] Issue edited: %s", p.Repository.FullName, titleLink)
112111
case api.HookIssueAssigned:
113112
list := make([]string, len(p.Issue.Assignees))
114113
for i, user := range p.Issue.Assignees {
115114
list[i] = linkFormatter(setting.AppURL+url.PathEscape(user.UserName), user.UserName)
116115
}
117-
text = fmt.Sprintf("[%s] Issue assigned to %s: %s", repoLink, strings.Join(list, ", "), titleLink)
116+
text = fmt.Sprintf("[%s] Issue assigned to %s: %s", p.Repository.FullName, strings.Join(list, ", "), titleLink)
118117
color = greenColor
119118
case api.HookIssueUnassigned:
120-
text = fmt.Sprintf("[%s] Issue unassigned: %s", repoLink, titleLink)
119+
text = fmt.Sprintf("[%s] Issue unassigned: %s", p.Repository.FullName, titleLink)
121120
case api.HookIssueLabelUpdated:
122-
text = fmt.Sprintf("[%s] Issue labels updated: %s", repoLink, titleLink)
121+
text = fmt.Sprintf("[%s] Issue labels updated: %s", p.Repository.FullName, titleLink)
123122
case api.HookIssueLabelCleared:
124-
text = fmt.Sprintf("[%s] Issue labels cleared: %s", repoLink, titleLink)
123+
text = fmt.Sprintf("[%s] Issue labels cleared: %s", p.Repository.FullName, titleLink)
125124
case api.HookIssueSynchronized:
126-
text = fmt.Sprintf("[%s] Issue synchronized: %s", repoLink, titleLink)
125+
text = fmt.Sprintf("[%s] Issue synchronized: %s", p.Repository.FullName, titleLink)
127126
case api.HookIssueMilestoned:
128-
mileStoneLink := fmt.Sprintf("%s/milestone/%d", p.Repository.HTMLURL, p.Issue.Milestone.ID)
129-
text = fmt.Sprintf("[%s] Issue milestoned to %s: %s", repoLink,
130-
linkFormatter(mileStoneLink, p.Issue.Milestone.Title), titleLink)
127+
text = fmt.Sprintf("[%s] Issue milestoned to %s: %s", p.Repository.FullName, p.Issue.Milestone.Title, titleLink)
131128
case api.HookIssueDemilestoned:
132-
text = fmt.Sprintf("[%s] Issue milestone cleared: %s", repoLink, titleLink)
129+
text = fmt.Sprintf("[%s] Issue milestone cleared: %s", p.Repository.FullName, titleLink)
133130
}
134131
if withSender {
135-
text += fmt.Sprintf(" by %s", linkFormatter(setting.AppURL+url.PathEscape(p.Sender.UserName), p.Sender.UserName))
132+
text += fmt.Sprintf(" by %s", p.Sender.UserName)
136133
}
137134

138135
var attachmentText string
@@ -144,7 +141,6 @@ func getIssuesPayloadInfo(p *api.IssuePayload, linkFormatter linkFormatter, with
144141
}
145142

146143
func getPullRequestPayloadInfo(p *api.PullRequestPayload, linkFormatter linkFormatter, withSender bool) (string, string, string, int) {
147-
repoLink := linkFormatter(p.Repository.HTMLURL, p.Repository.FullName)
148144
issueTitle := fmt.Sprintf("#%d %s", p.Index, p.PullRequest.Title)
149145
titleLink := linkFormatter(p.PullRequest.URL, issueTitle)
150146
var text string
@@ -153,96 +149,92 @@ func getPullRequestPayloadInfo(p *api.PullRequestPayload, linkFormatter linkForm
153149

154150
switch p.Action {
155151
case api.HookIssueOpened:
156-
text = fmt.Sprintf("[%s] Pull request opened: %s", repoLink, titleLink)
152+
text = fmt.Sprintf("[%s] Pull request opened: %s", p.Repository.FullName, titleLink)
157153
attachmentText = p.PullRequest.Body
158154
color = greenColor
159155
case api.HookIssueClosed:
160156
if p.PullRequest.HasMerged {
161-
text = fmt.Sprintf("[%s] Pull request merged: %s", repoLink, titleLink)
157+
text = fmt.Sprintf("[%s] Pull request merged: %s", p.Repository.FullName, titleLink)
162158
color = purpleColor
163159
} else {
164-
text = fmt.Sprintf("[%s] Pull request closed: %s", repoLink, titleLink)
160+
text = fmt.Sprintf("[%s] Pull request closed: %s", p.Repository.FullName, titleLink)
165161
color = redColor
166162
}
167163
case api.HookIssueReOpened:
168-
text = fmt.Sprintf("[%s] Pull request re-opened: %s", repoLink, titleLink)
164+
text = fmt.Sprintf("[%s] Pull request re-opened: %s", p.Repository.FullName, titleLink)
169165
case api.HookIssueEdited:
170-
text = fmt.Sprintf("[%s] Pull request edited: %s", repoLink, titleLink)
166+
text = fmt.Sprintf("[%s] Pull request edited: %s", p.Repository.FullName, titleLink)
171167
attachmentText = p.PullRequest.Body
172168
case api.HookIssueAssigned:
173169
list := make([]string, len(p.PullRequest.Assignees))
174170
for i, user := range p.PullRequest.Assignees {
175171
list[i] = linkFormatter(setting.AppURL+user.UserName, user.UserName)
176172
}
177-
text = fmt.Sprintf("[%s] Pull request assigned to %s: %s", repoLink,
173+
text = fmt.Sprintf("[%s] Pull request assigned to %s: %s", p.Repository.FullName,
178174
strings.Join(list, ", "), titleLink)
179175
color = greenColor
180176
case api.HookIssueUnassigned:
181-
text = fmt.Sprintf("[%s] Pull request unassigned: %s", repoLink, titleLink)
177+
text = fmt.Sprintf("[%s] Pull request unassigned: %s", p.Repository.FullName, titleLink)
182178
case api.HookIssueLabelUpdated:
183-
text = fmt.Sprintf("[%s] Pull request labels updated: %s", repoLink, titleLink)
179+
text = fmt.Sprintf("[%s] Pull request labels updated: %s", p.Repository.FullName, titleLink)
184180
case api.HookIssueLabelCleared:
185-
text = fmt.Sprintf("[%s] Pull request labels cleared: %s", repoLink, titleLink)
181+
text = fmt.Sprintf("[%s] Pull request labels cleared: %s", p.Repository.FullName, titleLink)
186182
case api.HookIssueSynchronized:
187-
text = fmt.Sprintf("[%s] Pull request synchronized: %s", repoLink, titleLink)
183+
text = fmt.Sprintf("[%s] Pull request synchronized: %s", p.Repository.FullName, titleLink)
188184
case api.HookIssueMilestoned:
189-
mileStoneLink := fmt.Sprintf("%s/milestone/%d", p.Repository.HTMLURL, p.PullRequest.Milestone.ID)
190-
text = fmt.Sprintf("[%s] Pull request milestoned to %s: %s", repoLink,
191-
linkFormatter(mileStoneLink, p.PullRequest.Milestone.Title), titleLink)
185+
text = fmt.Sprintf("[%s] Pull request milestoned to %s: %s", p.Repository.FullName, p.PullRequest.Milestone.Title, titleLink)
192186
case api.HookIssueDemilestoned:
193-
text = fmt.Sprintf("[%s] Pull request milestone cleared: %s", repoLink, titleLink)
187+
text = fmt.Sprintf("[%s] Pull request milestone cleared: %s", p.Repository.FullName, titleLink)
194188
case api.HookIssueReviewed:
195-
text = fmt.Sprintf("[%s] Pull request reviewed: %s", repoLink, titleLink)
189+
text = fmt.Sprintf("[%s] Pull request reviewed: %s", p.Repository.FullName, titleLink)
196190
attachmentText = p.Review.Content
197191
case api.HookIssueReviewRequested:
198-
text = fmt.Sprintf("[%s] Pull request review requested: %s", repoLink, titleLink)
192+
text = fmt.Sprintf("[%s] Pull request review requested: %s", p.Repository.FullName, titleLink)
199193
case api.HookIssueReviewRequestRemoved:
200-
text = fmt.Sprintf("[%s] Pull request review request removed: %s", repoLink, titleLink)
194+
text = fmt.Sprintf("[%s] Pull request review request removed: %s", p.Repository.FullName, titleLink)
201195
}
202196
if withSender {
203-
text += fmt.Sprintf(" by %s", linkFormatter(setting.AppURL+p.Sender.UserName, p.Sender.UserName))
197+
text += fmt.Sprintf(" by %s", p.Sender.UserName)
204198
}
205199

206200
return text, issueTitle, attachmentText, color
207201
}
208202

209203
func getReleasePayloadInfo(p *api.ReleasePayload, linkFormatter linkFormatter, withSender bool) (text string, color int) {
210-
repoLink := linkFormatter(p.Repository.HTMLURL, p.Repository.FullName)
211204
refLink := linkFormatter(p.Repository.HTMLURL+"/releases/tag/"+util.PathEscapeSegments(p.Release.TagName), p.Release.TagName)
212205

213206
switch p.Action {
214207
case api.HookReleasePublished:
215-
text = fmt.Sprintf("[%s] Release created: %s", repoLink, refLink)
208+
text = fmt.Sprintf("[%s] Release created: %s", p.Repository.FullName, refLink)
216209
color = greenColor
217210
case api.HookReleaseUpdated:
218-
text = fmt.Sprintf("[%s] Release updated: %s", repoLink, refLink)
211+
text = fmt.Sprintf("[%s] Release updated: %s", p.Repository.FullName, refLink)
219212
color = yellowColor
220213
case api.HookReleaseDeleted:
221-
text = fmt.Sprintf("[%s] Release deleted: %s", repoLink, refLink)
214+
text = fmt.Sprintf("[%s] Release deleted: %s", p.Repository.FullName, refLink)
222215
color = redColor
223216
}
224217
if withSender {
225-
text += fmt.Sprintf(" by %s", linkFormatter(setting.AppURL+url.PathEscape(p.Sender.UserName), p.Sender.UserName))
218+
text += fmt.Sprintf(" by %s", p.Sender.UserName)
226219
}
227220

228221
return text, color
229222
}
230223

231224
func getWikiPayloadInfo(p *api.WikiPayload, linkFormatter linkFormatter, withSender bool) (string, int, string) {
232-
repoLink := linkFormatter(p.Repository.HTMLURL, p.Repository.FullName)
233225
pageLink := linkFormatter(p.Repository.HTMLURL+"/wiki/"+url.PathEscape(p.Page), p.Page)
234226

235227
var text string
236228
color := greenColor
237229

238230
switch p.Action {
239231
case api.HookWikiCreated:
240-
text = fmt.Sprintf("[%s] New wiki page '%s'", repoLink, pageLink)
232+
text = fmt.Sprintf("[%s] New wiki page '%s'", p.Repository.FullName, pageLink)
241233
case api.HookWikiEdited:
242-
text = fmt.Sprintf("[%s] Wiki page '%s' edited", repoLink, pageLink)
234+
text = fmt.Sprintf("[%s] Wiki page '%s' edited", p.Repository.FullName, pageLink)
243235
color = yellowColor
244236
case api.HookWikiDeleted:
245-
text = fmt.Sprintf("[%s] Wiki page '%s' deleted", repoLink, pageLink)
237+
text = fmt.Sprintf("[%s] Wiki page '%s' deleted", p.Repository.FullName, pageLink)
246238
color = redColor
247239
}
248240

@@ -251,14 +243,13 @@ func getWikiPayloadInfo(p *api.WikiPayload, linkFormatter linkFormatter, withSen
251243
}
252244

253245
if withSender {
254-
text += fmt.Sprintf(" by %s", linkFormatter(setting.AppURL+url.PathEscape(p.Sender.UserName), p.Sender.UserName))
246+
text += fmt.Sprintf(" by %s", p.Sender.UserName)
255247
}
256248

257249
return text, color, pageLink
258250
}
259251

260252
func getIssueCommentPayloadInfo(p *api.IssueCommentPayload, linkFormatter linkFormatter, withSender bool) (string, string, int) {
261-
repoLink := linkFormatter(p.Repository.HTMLURL, p.Repository.FullName)
262253
issueTitle := fmt.Sprintf("#%d %s", p.Issue.Index, p.Issue.Title)
263254

264255
var text, typ, titleLink string
@@ -274,20 +265,20 @@ func getIssueCommentPayloadInfo(p *api.IssueCommentPayload, linkFormatter linkFo
274265

275266
switch p.Action {
276267
case api.HookIssueCommentCreated:
277-
text = fmt.Sprintf("[%s] New comment on %s %s", repoLink, typ, titleLink)
268+
text = fmt.Sprintf("[%s] New comment on %s %s", p.Repository.FullName, typ, titleLink)
278269
if p.IsPull {
279270
color = greenColorLight
280271
} else {
281272
color = orangeColorLight
282273
}
283274
case api.HookIssueCommentEdited:
284-
text = fmt.Sprintf("[%s] Comment edited on %s %s", repoLink, typ, titleLink)
275+
text = fmt.Sprintf("[%s] Comment edited on %s %s", p.Repository.FullName, typ, titleLink)
285276
case api.HookIssueCommentDeleted:
286-
text = fmt.Sprintf("[%s] Comment deleted on %s %s", repoLink, typ, titleLink)
277+
text = fmt.Sprintf("[%s] Comment deleted on %s %s", p.Repository.FullName, typ, titleLink)
287278
color = redColor
288279
}
289280
if withSender {
290-
text += fmt.Sprintf(" by %s", linkFormatter(setting.AppURL+url.PathEscape(p.Sender.UserName), p.Sender.UserName))
281+
text += fmt.Sprintf(" by %s", p.Sender.UserName)
291282
}
292283

293284
return text, issueTitle, color
@@ -305,7 +296,7 @@ func getPackagePayloadInfo(p *api.PackagePayload, linkFormatter linkFormatter, w
305296
color = redColor
306297
}
307298
if withSender {
308-
text += fmt.Sprintf(" by %s", linkFormatter(setting.AppURL+url.PathEscape(p.Sender.UserName), p.Sender.UserName))
299+
text += fmt.Sprintf(" by %s", p.Sender.UserName)
309300
}
310301

311302
return text, color

‎services/webhook/matrix.go‎

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"crypto/sha1"
1010
"encoding/hex"
1111
"fmt"
12+
"html"
1213
"html/template"
1314
"net/http"
1415
"net/url"
@@ -19,7 +20,6 @@ import (
1920
"code.gitea.io/gitea/modules/git"
2021
"code.gitea.io/gitea/modules/json"
2122
"code.gitea.io/gitea/modules/log"
22-
"code.gitea.io/gitea/modules/setting"
2323
api "code.gitea.io/gitea/modules/structs"
2424
"code.gitea.io/gitea/modules/svg"
2525
"code.gitea.io/gitea/modules/util"
@@ -143,9 +143,8 @@ func (m matrixConvertor) newPayload(text string, commits ...*api.PayloadCommit)
143143

144144
// Create implements payloadConvertor Create method
145145
func (m matrixConvertor) Create(p *api.CreatePayload) (MatrixPayload, error) {
146-
repoLink := htmlLinkFormatter(p.Repo.HTMLURL, p.Repo.FullName)
147146
refLink := MatrixLinkToRef(p.Repo.HTMLURL, p.Ref)
148-
text := fmt.Sprintf("[%s:%s] %s created by %s", repoLink, refLink, p.RefType, p.Sender.UserName)
147+
text := fmt.Sprintf("[%s:%s] %s created by %s", p.Repo.FullName, refLink, p.RefType, p.Sender.UserName)
149148

150149
return m.newPayload(text)
151150
}
@@ -206,9 +205,8 @@ func (m matrixConvertor) Push(p *api.PushPayload) (MatrixPayload, error) {
206205
commitDesc = fmt.Sprintf("%d commits", p.TotalCommits)
207206
}
208207

209-
repoLink := htmlLinkFormatter(p.Repo.HTMLURL, p.Repo.FullName)
210-
branchLink := MatrixLinkToRef(p.Repo.HTMLURL, p.Ref)
211-
text := fmt.Sprintf("[%s] %s pushed %s to %s:<br>", repoLink, p.Pusher.UserName, commitDesc, branchLink)
208+
refName := html.EscapeString(git.RefName(p.Ref).ShortName())
209+
text := fmt.Sprintf("[%s] %s pushed %s to %s:<br>", p.Repo.FullName, p.Pusher.UserName, commitDesc, refName)
212210

213211
// for each commit, generate a new line text
214212
for i, commit := range p.Commits {
@@ -231,10 +229,8 @@ func (m matrixConvertor) PullRequest(p *api.PullRequestPayload) (MatrixPayload,
231229

232230
// Review implements payloadConvertor Review method
233231
func (m matrixConvertor) Review(p *api.PullRequestPayload, event webhook_module.HookEventType) (MatrixPayload, error) {
234-
senderLink := htmlLinkFormatter(setting.AppURL+url.PathEscape(p.Sender.UserName), p.Sender.UserName)
235232
title := fmt.Sprintf("#%d %s", p.Index, p.PullRequest.Title)
236233
titleLink := htmlLinkFormatter(p.PullRequest.HTMLURL, title)
237-
repoLink := htmlLinkFormatter(p.Repository.HTMLURL, p.Repository.FullName)
238234
var text string
239235

240236
if p.Action == api.HookIssueReviewed {
@@ -243,37 +239,35 @@ func (m matrixConvertor) Review(p *api.PullRequestPayload, event webhook_module.
243239
return MatrixPayload{}, err
244240
}
245241

246-
text = fmt.Sprintf("[%s] Pull request review %s: %s by %s", repoLink, action, titleLink, senderLink)
242+
text = fmt.Sprintf("[%s] Pull request review %s: %s by %s", p.Repository.FullName, action, titleLink, p.Sender.UserName)
247243
}
248244

249245
return m.newPayload(text)
250246
}
251247

252248
// Repository implements payloadConvertor Repository method
253249
func (m matrixConvertor) Repository(p *api.RepositoryPayload) (MatrixPayload, error) {
254-
senderLink := htmlLinkFormatter(setting.AppURL+p.Sender.UserName, p.Sender.UserName)
255250
repoLink := htmlLinkFormatter(p.Repository.HTMLURL, p.Repository.FullName)
256251
var text string
257252

258253
switch p.Action {
259254
case api.HookRepoCreated:
260-
text = fmt.Sprintf("[%s] Repository created by %s", repoLink, senderLink)
255+
text = fmt.Sprintf("[%s] Repository created by %s", repoLink, p.Sender.UserName)
261256
case api.HookRepoDeleted:
262-
text = fmt.Sprintf("[%s] Repository deleted by %s", repoLink, senderLink)
257+
text = fmt.Sprintf("[%s] Repository deleted by %s", repoLink, p.Sender.UserName)
263258
}
264259
return m.newPayload(text)
265260
}
266261

267262
func (m matrixConvertor) Package(p *api.PackagePayload) (MatrixPayload, error) {
268-
senderLink := htmlLinkFormatter(setting.AppURL+p.Sender.UserName, p.Sender.UserName)
269263
packageLink := htmlLinkFormatter(p.Package.HTMLURL, p.Package.Name)
270264
var text string
271265

272266
switch p.Action {
273267
case api.HookPackageCreated:
274-
text = fmt.Sprintf("[%s] Package published by %s", packageLink, senderLink)
268+
text = fmt.Sprintf("[%s] Package published by %s", packageLink, p.Sender.UserName)
275269
case api.HookPackageDeleted:
276-
text = fmt.Sprintf("[%s] Package deleted by %s", packageLink, senderLink)
270+
text = fmt.Sprintf("[%s] Package deleted by %s", packageLink, p.Sender.UserName)
277271
}
278272

279273
return m.newPayload(text)

0 commit comments

Comments
 (0)