From 733e36e1c054bf4b4bc143a8f7d1e4fce4553ed3 Mon Sep 17 00:00:00 2001 From: xiaodongzhang Date: Mon, 29 May 2023 01:55:13 +0800 Subject: [PATCH 1/8] fix: rename NotifyPullReviewRequest to NotifyPullRequestReviewRequest --- modules/notification/base/notifier.go | 2 +- modules/notification/base/null.go | 4 ++-- modules/notification/mail/mail.go | 2 +- modules/notification/notification.go | 6 +++--- modules/notification/ui/ui.go | 2 +- services/issue/assignee.go | 4 ++-- services/webhook/notifier.go | 4 ++-- 7 files changed, 12 insertions(+), 12 deletions(-) diff --git a/modules/notification/base/notifier.go b/modules/notification/base/notifier.go index 87eae3f414a0f..e1762bb1ee9a2 100644 --- a/modules/notification/base/notifier.go +++ b/modules/notification/base/notifier.go @@ -28,7 +28,7 @@ type Notifier interface { NotifyDeleteIssue(ctx context.Context, doer *user_model.User, issue *issues_model.Issue) NotifyIssueChangeMilestone(ctx context.Context, doer *user_model.User, issue *issues_model.Issue, oldMilestoneID int64) NotifyIssueChangeAssignee(ctx context.Context, doer *user_model.User, issue *issues_model.Issue, assignee *user_model.User, removed bool, comment *issues_model.Comment) - NotifyPullReviewRequest(ctx context.Context, doer *user_model.User, issue *issues_model.Issue, reviewer *user_model.User, isRequest bool, comment *issues_model.Comment) + NotifyPullRequestReviewRequest(ctx context.Context, doer *user_model.User, issue *issues_model.Issue, reviewer *user_model.User, isRequest bool, comment *issues_model.Comment) NotifyIssueChangeContent(ctx context.Context, doer *user_model.User, issue *issues_model.Issue, oldContent string) NotifyIssueClearLabels(ctx context.Context, doer *user_model.User, issue *issues_model.Issue) NotifyIssueChangeTitle(ctx context.Context, doer *user_model.User, issue *issues_model.Issue, oldTitle string) diff --git a/modules/notification/base/null.go b/modules/notification/base/null.go index 187c36beeb4f8..338790b356426 100644 --- a/modules/notification/base/null.go +++ b/modules/notification/base/null.go @@ -120,8 +120,8 @@ func (*NullNotifier) NotifyIssueChangeContent(ctx context.Context, doer *user_mo func (*NullNotifier) NotifyIssueChangeAssignee(ctx context.Context, doer *user_model.User, issue *issues_model.Issue, assignee *user_model.User, removed bool, comment *issues_model.Comment) { } -// NotifyPullReviewRequest places a place holder function -func (*NullNotifier) NotifyPullReviewRequest(ctx context.Context, doer *user_model.User, issue *issues_model.Issue, reviewer *user_model.User, isRequest bool, comment *issues_model.Comment) { +// NotifyPullRequestReviewRequest places a place holder function +func (*NullNotifier) NotifyPullRequestReviewRequest(ctx context.Context, doer *user_model.User, issue *issues_model.Issue, reviewer *user_model.User, isRequest bool, comment *issues_model.Comment) { } // NotifyIssueClearLabels places a place holder function diff --git a/modules/notification/mail/mail.go b/modules/notification/mail/mail.go index 7e54df44c4032..a5fed51ebd0eb 100644 --- a/modules/notification/mail/mail.go +++ b/modules/notification/mail/mail.go @@ -123,7 +123,7 @@ func (m *mailNotifier) NotifyIssueChangeAssignee(ctx context.Context, doer *user } } -func (m *mailNotifier) NotifyPullReviewRequest(ctx context.Context, doer *user_model.User, issue *issues_model.Issue, reviewer *user_model.User, isRequest bool, comment *issues_model.Comment) { +func (m *mailNotifier) NotifyPullRequestReviewRequest(ctx context.Context, doer *user_model.User, issue *issues_model.Issue, reviewer *user_model.User, isRequest bool, comment *issues_model.Comment) { if isRequest && doer.ID != reviewer.ID && reviewer.EmailNotifications() != user_model.EmailNotificationsDisabled { ct := fmt.Sprintf("Requested to review %s.", issue.HTMLURL()) if err := mailer.SendIssueAssignedMail(ctx, issue, doer, ct, comment, []*user_model.User{reviewer}); err != nil { diff --git a/modules/notification/notification.go b/modules/notification/notification.go index 72dd1c99e53dc..6153c9e3d6870 100644 --- a/modules/notification/notification.go +++ b/modules/notification/notification.go @@ -230,10 +230,10 @@ func NotifyIssueChangeAssignee(ctx context.Context, doer *user_model.User, issue } } -// NotifyPullReviewRequest notifies Request Review change -func NotifyPullReviewRequest(ctx context.Context, doer *user_model.User, issue *issues_model.Issue, reviewer *user_model.User, isRequest bool, comment *issues_model.Comment) { +// NotifyPullRequestReviewRequest notifies Request Review change +func NotifyPullRequestReviewRequest(ctx context.Context, doer *user_model.User, issue *issues_model.Issue, reviewer *user_model.User, isRequest bool, comment *issues_model.Comment) { for _, notifier := range notifiers { - notifier.NotifyPullReviewRequest(ctx, doer, issue, reviewer, isRequest, comment) + notifier.NotifyPullRequestReviewRequest(ctx, doer, issue, reviewer, isRequest, comment) } } diff --git a/modules/notification/ui/ui.go b/modules/notification/ui/ui.go index 6c5f22c122926..2ca1a7700f2cf 100644 --- a/modules/notification/ui/ui.go +++ b/modules/notification/ui/ui.go @@ -229,7 +229,7 @@ func (ns *notificationService) NotifyIssueChangeAssignee(ctx context.Context, do } } -func (ns *notificationService) NotifyPullReviewRequest(ctx context.Context, doer *user_model.User, issue *issues_model.Issue, reviewer *user_model.User, isRequest bool, comment *issues_model.Comment) { +func (ns *notificationService) NotifyPullRequestReviewRequest(ctx context.Context, doer *user_model.User, issue *issues_model.Issue, reviewer *user_model.User, isRequest bool, comment *issues_model.Comment) { if isRequest { opts := issueNotificationOpts{ IssueID: issue.ID, diff --git a/services/issue/assignee.go b/services/issue/assignee.go index 4d0224d4bff10..943a761e28290 100644 --- a/services/issue/assignee.go +++ b/services/issue/assignee.go @@ -73,7 +73,7 @@ func ReviewRequest(ctx context.Context, issue *issues_model.Issue, doer, reviewe } if comment != nil { - notification.NotifyPullReviewRequest(ctx, doer, issue, reviewer, isAdd, comment) + notification.NotifyPullRequestReviewRequest(ctx, doer, issue, reviewer, isAdd, comment) } return comment, err @@ -260,7 +260,7 @@ func TeamReviewRequest(ctx context.Context, issue *issues_model.Issue, doer *use continue } comment.AssigneeID = member.ID - notification.NotifyPullReviewRequest(ctx, doer, issue, member, isAdd, comment) + notification.NotifyPullRequestReviewRequest(ctx, doer, issue, member, isAdd, comment) } return comment, err diff --git a/services/webhook/notifier.go b/services/webhook/notifier.go index e10c83e02f807..e6d1875b2996f 100644 --- a/services/webhook/notifier.go +++ b/services/webhook/notifier.go @@ -719,9 +719,9 @@ func (m *webhookNotifier) NotifyPullRequestReview(ctx context.Context, pr *issue } } -func (m *webhookNotifier) NotifyPullReviewRequest(ctx context.Context, doer *user_model.User, issue *issues_model.Issue, reviewer *user_model.User, isRequest bool, comment *issues_model.Comment) { +func (m *webhookNotifier) NotifyPullRequestReviewRequest(ctx context.Context, doer *user_model.User, issue *issues_model.Issue, reviewer *user_model.User, isRequest bool, comment *issues_model.Comment) { if !issue.IsPull { - log.Warn("NotifyPullReviewRequest: issue is not a pull request: %v", issue.ID) + log.Warn("NotifyPullRequestReviewRequest: issue is not a pull request: %v", issue.ID) return } mode, _ := access_model.AccessLevelUnit(ctx, doer, issue.Repo, unit.TypePullRequests) From 0209e96c2fe84f89396ecfecce8fc9999a370a22 Mon Sep 17 00:00:00 2001 From: xiaodongzhang Date: Tue, 6 Jun 2023 23:19:07 +0800 Subject: [PATCH 2/8] fix: Feishu webhook of issue update not linking to full issue url --- services/webhook/feishu.go | 35 +++++++++++++++----- services/webhook/general.go | 65 +++++++++++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 9 deletions(-) diff --git a/services/webhook/feishu.go b/services/webhook/feishu.go index 885cde3bc5195..51fc4418aeab5 100644 --- a/services/webhook/feishu.go +++ b/services/webhook/feishu.go @@ -97,23 +97,40 @@ func (f *FeishuPayload) Push(p *api.PushPayload) (api.Payloader, error) { // Issue implements PayloadConvertor Issue method func (f *FeishuPayload) Issue(p *api.IssuePayload) (api.Payloader, error) { - text, issueTitle, attachmentText, _ := getIssuesPayloadInfo(p, noneLinkFormatter, true) - - return newFeishuTextPayload(issueTitle + "\r\n" + text + "\r\n\r\n" + attachmentText), nil + title, link, by, operator, result, assignees := getIssuesInfo(p) + var res api.Payloader + if assignees != "" { + if p.Action == api.HookIssueAssigned || p.Action == api.HookIssueUnassigned || p.Action == api.HookIssueMilestoned { + res = newFeishuTextPayload(title + "\r\n" + link + "\r\n" + by + "\r\n" + operator + "\r\n" + result + "\r\n" + assignees + "\r\n\r\n" + p.Issue.Body) + } else { + res = newFeishuTextPayload(title + "\r\n" + link + "\r\n" + by + "\r\n" + operator + "\r\n" + assignees + "\r\n\r\n" + p.Issue.Body) + } + } else { + res = newFeishuTextPayload(title + "\r\n" + link + "\r\n" + by + "\r\n" + operator + "\r\n\r\n" + p.Issue.Body) + } + return res, nil } // IssueComment implements PayloadConvertor IssueComment method func (f *FeishuPayload) IssueComment(p *api.IssueCommentPayload) (api.Payloader, error) { - text, issueTitle, _ := getIssueCommentPayloadInfo(p, noneLinkFormatter, true) - - return newFeishuTextPayload(issueTitle + "\r\n" + text + "\r\n\r\n" + p.Comment.Body), nil + title, link, by, operator := getIssuesCommentInfo(p) + return newFeishuTextPayload(title + "\r\n" + link + "\r\n" + by + "\r\n" + operator + "\r\n\r\n" + p.Comment.Body), nil } // PullRequest implements PayloadConvertor PullRequest method func (f *FeishuPayload) PullRequest(p *api.PullRequestPayload) (api.Payloader, error) { - text, issueTitle, attachmentText, _ := getPullRequestPayloadInfo(p, noneLinkFormatter, true) - - return newFeishuTextPayload(issueTitle + "\r\n" + text + "\r\n\r\n" + attachmentText), nil + title, link, by, operator, result, assignees := getPullRequestInfo(p) + var res api.Payloader + if assignees != "" { + if p.Action == api.HookIssueAssigned || p.Action == api.HookIssueUnassigned || p.Action == api.HookIssueMilestoned { + res = newFeishuTextPayload(title + "\r\n" + link + "\r\n" + by + "\r\n" + operator + "\r\n" + result + "\r\n" + assignees + "\r\n\r\n" + p.PullRequest.Body) + } else { + res = newFeishuTextPayload(title + "\r\n" + link + "\r\n" + by + "\r\n" + operator + "\r\n" + assignees + "\r\n\r\n" + p.PullRequest.Body) + } + } else { + res = newFeishuTextPayload(title + "\r\n" + link + "\r\n" + by + "\r\n" + operator + "\r\n\r\n" + p.PullRequest.Body) + } + return res, nil } // Review implements PayloadConvertor Review method diff --git a/services/webhook/general.go b/services/webhook/general.go index 1f7d204d1f74c..62256f90195d4 100644 --- a/services/webhook/general.go +++ b/services/webhook/general.go @@ -28,6 +28,71 @@ func htmlLinkFormatter(url, text string) string { return fmt.Sprintf(`%s`, html.EscapeString(url), html.EscapeString(text)) } +// getPullRequestInfo gets the information for a pull request +func getPullRequestInfo(p *api.PullRequestPayload) (string, string, string, string, string, string) { + if p.PullRequest.HasMerged { + p.Action = "merged" + } + title := fmt.Sprintf("[PullRequest-%s #%d]: %s", p.Repository.FullName, p.PullRequest.Index, p.Action) + assignList := p.PullRequest.Assignees + list := make([]string, len(assignList)) + var assignees string + var operateResult string + for i, user := range assignList { + list[i] = user.UserName + } + if p.Action == api.HookIssueAssigned { + operateResult = fmt.Sprintf("%s assign this to %s", p.Sender.UserName, assignList[len(assignList)-1].UserName) + } else if p.Action == api.HookIssueUnassigned { + operateResult = fmt.Sprintf("%s unassigned this for someone", p.Sender.UserName) + } else if p.Action == api.HookIssueMilestoned { + operateResult = fmt.Sprintf("%s/milestone/%d", p.Repository.HTMLURL, p.PullRequest.Milestone.ID) + } + link := p.PullRequest.HTMLURL + by := fmt.Sprintf("PullRequest by %s", p.PullRequest.Poster.UserName) + if len(list) > 0 { + assignees = fmt.Sprintf("Assignees: %s", strings.Join(list, ", ")) + } + operator := fmt.Sprintf("Operator: %s", p.Sender.UserName) + return title, link, by, operator, operateResult, assignees +} + +// getIssuesInfo gets the information for an issue +func getIssuesInfo(p *api.IssuePayload) (string, string, string, string, string, string) { + issueTitle := fmt.Sprintf("[Issue-%s #%d]: %s", p.Repository.FullName, p.Issue.Index, p.Action) + assignList := p.Issue.Assignees + list := make([]string, len(assignList)) + var assignees string + var operateResult string + + for i, user := range assignList { + list[i] = user.UserName + } + if p.Action == api.HookIssueAssigned { + operateResult = fmt.Sprintf("%s assign this to %s", p.Sender.UserName, assignList[len(assignList)-1].UserName) + } else if p.Action == api.HookIssueUnassigned { + operateResult = fmt.Sprintf("%s unassigned this for someone", p.Sender.UserName) + } else if p.Action == api.HookIssueMilestoned { + operateResult = fmt.Sprintf("%s/milestone/%d", p.Repository.HTMLURL, p.Issue.Milestone.ID) + } + link := p.Issue.HTMLURL + by := fmt.Sprintf("Issue by %s", p.Issue.Poster.UserName) + if len(list) > 0 { + assignees = fmt.Sprintf("Assignees: %s", strings.Join(list, ", ")) + } + operator := fmt.Sprintf("Operator: %s", p.Sender.UserName) + return issueTitle, link, by, operator, operateResult, assignees +} + +// getIssuesCommentInfo gets the information for an issue comment +func getIssuesCommentInfo(p *api.IssueCommentPayload) (string, string, string, string) { + issueTitle := fmt.Sprintf("[Issue Comment-%s #%d]: %s", p.Repository.FullName, p.Issue.Index, p.Action) + issueLink := p.Issue.HTMLURL + issueBy := fmt.Sprintf("Issue by %s", p.Issue.Poster.UserName) + operator := fmt.Sprintf("Operator: %s", p.Sender.UserName) + return issueTitle, issueLink, issueBy, operator +} + 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) From a4ef52e2a8787976aa8e1485763e061712114cbd Mon Sep 17 00:00:00 2001 From: xiaodongzhang Date: Wed, 7 Jun 2023 00:54:36 +0800 Subject: [PATCH 3/8] fix: ci test --- services/webhook/feishu_test.go | 10 +++++----- services/webhook/general_test.go | 18 +++++++++++++++++- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/services/webhook/feishu_test.go b/services/webhook/feishu_test.go index 84549c1fa5764..64f4c0bab6ce3 100644 --- a/services/webhook/feishu_test.go +++ b/services/webhook/feishu_test.go @@ -72,7 +72,7 @@ func TestFeishuPayload(t *testing.T) { require.NotNil(t, pl) require.IsType(t, &FeishuPayload{}, pl) - assert.Equal(t, "#2 crash\r\n[test/repo] Issue opened: #2 crash by user1\r\n\r\nissue body", pl.(*FeishuPayload).Content.Text) + assert.Equal(t, "[Issue-test/repo #2]: opened\r\nhttp://localhost:3000/test/repo/issues/2\r\nIssue by user1\r\nOperator: user1\r\nAssignees: user1\r\n\r\nissue body", pl.(*FeishuPayload).Content.Text) p.Action = api.HookIssueClosed pl, err = d.Issue(p) @@ -80,7 +80,7 @@ func TestFeishuPayload(t *testing.T) { require.NotNil(t, pl) require.IsType(t, &FeishuPayload{}, pl) - assert.Equal(t, "#2 crash\r\n[test/repo] Issue closed: #2 crash by user1", pl.(*FeishuPayload).Content.Text) + assert.Equal(t, "[Issue-test/repo #2]: closed\r\nhttp://localhost:3000/test/repo/issues/2\r\nIssue by user1\r\nOperator: user1\r\nAssignees: user1\r\n\r\nissue body", pl.(*FeishuPayload).Content.Text) }) t.Run("IssueComment", func(t *testing.T) { @@ -92,7 +92,7 @@ func TestFeishuPayload(t *testing.T) { require.NotNil(t, pl) require.IsType(t, &FeishuPayload{}, pl) - assert.Equal(t, "#2 crash\r\n[test/repo] New comment on issue #2 crash by user1\r\n\r\nmore info needed", pl.(*FeishuPayload).Content.Text) + assert.Equal(t, "[Issue Comment-test/repo #2]: created\r\nhttp://localhost:3000/test/repo/issues/2\r\nIssue by user1\r\nOperator: user1\r\n\r\nmore info needed", pl.(*FeishuPayload).Content.Text) }) t.Run("PullRequest", func(t *testing.T) { @@ -104,7 +104,7 @@ func TestFeishuPayload(t *testing.T) { require.NotNil(t, pl) require.IsType(t, &FeishuPayload{}, pl) - assert.Equal(t, "#12 Fix bug\r\n[test/repo] Pull request opened: #12 Fix bug by user1\r\n\r\nfixes bug #2", pl.(*FeishuPayload).Content.Text) + assert.Equal(t, "[PullRequest-test/repo #12]: opened\r\nhttp://localhost:3000/test/repo/pulls/12\r\nPullRequest by user1\r\nOperator: user1\r\nAssignees: user1\r\n\r\nfixes bug #2", pl.(*FeishuPayload).Content.Text) }) t.Run("PullRequestComment", func(t *testing.T) { @@ -116,7 +116,7 @@ func TestFeishuPayload(t *testing.T) { require.NotNil(t, pl) require.IsType(t, &FeishuPayload{}, pl) - assert.Equal(t, "#12 Fix bug\r\n[test/repo] New comment on pull request #12 Fix bug by user1\r\n\r\nchanges requested", pl.(*FeishuPayload).Content.Text) + assert.Equal(t, "[Issue Comment-test/repo #12]: created\r\nhttp://localhost:3000/test/repo/pulls/12\r\nIssue by user1\r\nOperator: user1\r\n\r\nchanges requested", pl.(*FeishuPayload).Content.Text) }) t.Run("Review", func(t *testing.T) { diff --git a/services/webhook/general_test.go b/services/webhook/general_test.go index ba58ca4f90d6e..64bd72f5a05a4 100644 --- a/services/webhook/general_test.go +++ b/services/webhook/general_test.go @@ -123,6 +123,10 @@ func issueTestPayload() *api.IssuePayload { HTMLURL: "http://localhost:3000/test/repo/issues/2", Title: "crash", Body: "issue body", + Poster: &api.User{ + UserName: "user1", + AvatarURL: "http://localhost:3000/user1/avatar", + }, Assignees: []*api.User{ { UserName: "user1", @@ -161,7 +165,11 @@ func issueCommentTestPayload() *api.IssueCommentPayload { URL: "http://localhost:3000/api/v1/repos/test/repo/issues/2", HTMLURL: "http://localhost:3000/test/repo/issues/2", Title: "crash", - Body: "this happened", + Poster: &api.User{ + UserName: "user1", + AvatarURL: "http://localhost:3000/user1/avatar", + }, + Body: "this happened", }, } } @@ -190,6 +198,10 @@ func pullRequestCommentTestPayload() *api.IssueCommentPayload { HTMLURL: "http://localhost:3000/test/repo/pulls/12", Title: "Fix bug", Body: "fixes bug #2", + Poster: &api.User{ + UserName: "user1", + AvatarURL: "http://localhost:3000/user1/avatar", + }, }, IsPull: true, } @@ -254,6 +266,10 @@ func pullRequestTestPayload() *api.PullRequestPayload { Title: "Fix bug", Body: "fixes bug #2", Mergeable: true, + Poster: &api.User{ + UserName: "user1", + AvatarURL: "http://localhost:3000/user1/avatar", + }, Assignees: []*api.User{ { UserName: "user1", From 1bff038fb2f0862fdae3a6d549c53afa755fb41e Mon Sep 17 00:00:00 2001 From: Makonike Date: Sat, 29 Jul 2023 14:37:06 +0800 Subject: [PATCH 4/8] fix --- services/webhook/feishu.go | 14 ++++----- services/webhook/general.go | 59 +++++++++++++++++++------------------ 2 files changed, 37 insertions(+), 36 deletions(-) diff --git a/services/webhook/feishu.go b/services/webhook/feishu.go index 51fc4418aeab5..089f51952fd78 100644 --- a/services/webhook/feishu.go +++ b/services/webhook/feishu.go @@ -101,12 +101,12 @@ func (f *FeishuPayload) Issue(p *api.IssuePayload) (api.Payloader, error) { var res api.Payloader if assignees != "" { if p.Action == api.HookIssueAssigned || p.Action == api.HookIssueUnassigned || p.Action == api.HookIssueMilestoned { - res = newFeishuTextPayload(title + "\r\n" + link + "\r\n" + by + "\r\n" + operator + "\r\n" + result + "\r\n" + assignees + "\r\n\r\n" + p.Issue.Body) + res = newFeishuTextPayload(fmt.Sprintf("%s\n%s\n%s\n%s\n%s\n%s\n\n%s", title, link, by, operator, result, assignees, p.Issue.Body)) } else { - res = newFeishuTextPayload(title + "\r\n" + link + "\r\n" + by + "\r\n" + operator + "\r\n" + assignees + "\r\n\r\n" + p.Issue.Body) + res = newFeishuTextPayload(fmt.Sprintf("%s\n%s\n%s\n%s\n%s\n\n%s", title, link, by, operator, assignees, p.Issue.Body)) } } else { - res = newFeishuTextPayload(title + "\r\n" + link + "\r\n" + by + "\r\n" + operator + "\r\n\r\n" + p.Issue.Body) + res = newFeishuTextPayload(fmt.Sprintf("%s\n%s\n%s\n%s\n\n%s", title, link, by, operator, p.Issue.Body)) } return res, nil } @@ -114,7 +114,7 @@ func (f *FeishuPayload) Issue(p *api.IssuePayload) (api.Payloader, error) { // IssueComment implements PayloadConvertor IssueComment method func (f *FeishuPayload) IssueComment(p *api.IssueCommentPayload) (api.Payloader, error) { title, link, by, operator := getIssuesCommentInfo(p) - return newFeishuTextPayload(title + "\r\n" + link + "\r\n" + by + "\r\n" + operator + "\r\n\r\n" + p.Comment.Body), nil + return newFeishuTextPayload(fmt.Sprintf("%s\n%s\n%s\n%s\n\n%s", title, link, by, operator, p.Comment.Body)), nil } // PullRequest implements PayloadConvertor PullRequest method @@ -123,12 +123,12 @@ func (f *FeishuPayload) PullRequest(p *api.PullRequestPayload) (api.Payloader, e var res api.Payloader if assignees != "" { if p.Action == api.HookIssueAssigned || p.Action == api.HookIssueUnassigned || p.Action == api.HookIssueMilestoned { - res = newFeishuTextPayload(title + "\r\n" + link + "\r\n" + by + "\r\n" + operator + "\r\n" + result + "\r\n" + assignees + "\r\n\r\n" + p.PullRequest.Body) + res = newFeishuTextPayload(fmt.Sprintf("%s\n%s\n%s\n%s\n%s\n%s\n\n%s", title, link, by, operator, result, assignees, p.PullRequest.Body)) } else { - res = newFeishuTextPayload(title + "\r\n" + link + "\r\n" + by + "\r\n" + operator + "\r\n" + assignees + "\r\n\r\n" + p.PullRequest.Body) + res = newFeishuTextPayload(fmt.Sprintf("%s\n%s\n%s\n%s\n%s\n\n%s", title, link, by, operator, assignees, p.PullRequest.Body)) } } else { - res = newFeishuTextPayload(title + "\r\n" + link + "\r\n" + by + "\r\n" + operator + "\r\n\r\n" + p.PullRequest.Body) + res = newFeishuTextPayload(fmt.Sprintf("%s\n%s\n%s\n%s\n\n%s", title, link, by, operator, p.PullRequest.Body)) } return res, nil } diff --git a/services/webhook/general.go b/services/webhook/general.go index 62256f90195d4..4c04c8049e59a 100644 --- a/services/webhook/general.go +++ b/services/webhook/general.go @@ -29,17 +29,16 @@ func htmlLinkFormatter(url, text string) string { } // getPullRequestInfo gets the information for a pull request -func getPullRequestInfo(p *api.PullRequestPayload) (string, string, string, string, string, string) { +func getPullRequestInfo(p *api.PullRequestPayload) (title, link, by, operator, operateResult, assignees string) { if p.PullRequest.HasMerged { p.Action = "merged" } - title := fmt.Sprintf("[PullRequest-%s #%d]: %s", p.Repository.FullName, p.PullRequest.Index, p.Action) + title = fmt.Sprintf("[PullRequest-%s #%d]: %s\n%s", p.Repository.FullName, p.PullRequest.Index, p.Action, p.PullRequest.Title) assignList := p.PullRequest.Assignees - list := make([]string, len(assignList)) - var assignees string - var operateResult string + assignStringList := make([]string, len(assignList)) + for i, user := range assignList { - list[i] = user.UserName + assignStringList[i] = user.UserName } if p.Action == api.HookIssueAssigned { operateResult = fmt.Sprintf("%s assign this to %s", p.Sender.UserName, assignList[len(assignList)-1].UserName) @@ -48,25 +47,23 @@ func getPullRequestInfo(p *api.PullRequestPayload) (string, string, string, stri } else if p.Action == api.HookIssueMilestoned { operateResult = fmt.Sprintf("%s/milestone/%d", p.Repository.HTMLURL, p.PullRequest.Milestone.ID) } - link := p.PullRequest.HTMLURL - by := fmt.Sprintf("PullRequest by %s", p.PullRequest.Poster.UserName) - if len(list) > 0 { - assignees = fmt.Sprintf("Assignees: %s", strings.Join(list, ", ")) + link = p.PullRequest.HTMLURL + by = fmt.Sprintf("PullRequest by %s", p.PullRequest.Poster.UserName) + if len(assignStringList) > 0 { + assignees = fmt.Sprintf("Assignees: %s", strings.Join(assignStringList, ", ")) } - operator := fmt.Sprintf("Operator: %s", p.Sender.UserName) + operator = fmt.Sprintf("Operator: %s", p.Sender.UserName) return title, link, by, operator, operateResult, assignees } // getIssuesInfo gets the information for an issue -func getIssuesInfo(p *api.IssuePayload) (string, string, string, string, string, string) { - issueTitle := fmt.Sprintf("[Issue-%s #%d]: %s", p.Repository.FullName, p.Issue.Index, p.Action) +func getIssuesInfo(p *api.IssuePayload) (issueTitle, link, by, operator, operateResult, assignees string) { + issueTitle = fmt.Sprintf("[Issue-%s #%d]: %s\n%s", p.Repository.FullName, p.Issue.Index, p.Action, p.Issue.Title) assignList := p.Issue.Assignees - list := make([]string, len(assignList)) - var assignees string - var operateResult string + assignStringList := make([]string, len(assignList)) for i, user := range assignList { - list[i] = user.UserName + assignStringList[i] = user.UserName } if p.Action == api.HookIssueAssigned { operateResult = fmt.Sprintf("%s assign this to %s", p.Sender.UserName, assignList[len(assignList)-1].UserName) @@ -75,22 +72,26 @@ func getIssuesInfo(p *api.IssuePayload) (string, string, string, string, string, } else if p.Action == api.HookIssueMilestoned { operateResult = fmt.Sprintf("%s/milestone/%d", p.Repository.HTMLURL, p.Issue.Milestone.ID) } - link := p.Issue.HTMLURL - by := fmt.Sprintf("Issue by %s", p.Issue.Poster.UserName) - if len(list) > 0 { - assignees = fmt.Sprintf("Assignees: %s", strings.Join(list, ", ")) + link = p.Issue.HTMLURL + by = fmt.Sprintf("Issue by %s", p.Issue.Poster.UserName) + if len(assignStringList) > 0 { + assignees = fmt.Sprintf("Assignees: %s", strings.Join(assignStringList, ", ")) } - operator := fmt.Sprintf("Operator: %s", p.Sender.UserName) + operator = fmt.Sprintf("Operator: %s", p.Sender.UserName) return issueTitle, link, by, operator, operateResult, assignees } -// getIssuesCommentInfo gets the information for an issue comment -func getIssuesCommentInfo(p *api.IssueCommentPayload) (string, string, string, string) { - issueTitle := fmt.Sprintf("[Issue Comment-%s #%d]: %s", p.Repository.FullName, p.Issue.Index, p.Action) - issueLink := p.Issue.HTMLURL - issueBy := fmt.Sprintf("Issue by %s", p.Issue.Poster.UserName) - operator := fmt.Sprintf("Operator: %s", p.Sender.UserName) - return issueTitle, issueLink, issueBy, operator +// getIssuesCommentInfo gets the information for a comment +func getIssuesCommentInfo(p *api.IssueCommentPayload) (title, link, by, operator string) { + title = fmt.Sprintf("[Comment-%s #%d]: %s\n%s", p.Repository.FullName, p.Issue.Index, p.Action, p.Issue.Title) + link = p.Issue.HTMLURL + if p.IsPull { + by = fmt.Sprintf("PullRequest by %s", p.Issue.Poster.UserName) + } else { + by = fmt.Sprintf("Issue by %s", p.Issue.Poster.UserName) + } + operator = fmt.Sprintf("Operator: %s", p.Sender.UserName) + return title, link, by, operator } func getIssuesPayloadInfo(p *api.IssuePayload, linkFormatter linkFormatter, withSender bool) (string, string, string, int) { From 5632b8ee75eeef248e3385c4b44d1737063d7223 Mon Sep 17 00:00:00 2001 From: Makonike Date: Sat, 29 Jul 2023 15:10:19 +0800 Subject: [PATCH 5/8] fix --- services/webhook/feishu_test.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/services/webhook/feishu_test.go b/services/webhook/feishu_test.go index 64f4c0bab6ce3..4365cecfc495f 100644 --- a/services/webhook/feishu_test.go +++ b/services/webhook/feishu_test.go @@ -4,10 +4,10 @@ package webhook import ( - "testing" - api "code.gitea.io/gitea/modules/structs" webhook_module "code.gitea.io/gitea/modules/webhook" + _ "github.com/mattn/go-sqlite3" + "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -72,7 +72,7 @@ func TestFeishuPayload(t *testing.T) { require.NotNil(t, pl) require.IsType(t, &FeishuPayload{}, pl) - assert.Equal(t, "[Issue-test/repo #2]: opened\r\nhttp://localhost:3000/test/repo/issues/2\r\nIssue by user1\r\nOperator: user1\r\nAssignees: user1\r\n\r\nissue body", pl.(*FeishuPayload).Content.Text) + assert.Equal(t, "[Issue-test/repo #2]: opened\ncrash\nhttp://localhost:3000/test/repo/issues/2\nIssue by user1\nOperator: user1\nAssignees: user1\n\nissue body", pl.(*FeishuPayload).Content.Text) p.Action = api.HookIssueClosed pl, err = d.Issue(p) @@ -80,7 +80,7 @@ func TestFeishuPayload(t *testing.T) { require.NotNil(t, pl) require.IsType(t, &FeishuPayload{}, pl) - assert.Equal(t, "[Issue-test/repo #2]: closed\r\nhttp://localhost:3000/test/repo/issues/2\r\nIssue by user1\r\nOperator: user1\r\nAssignees: user1\r\n\r\nissue body", pl.(*FeishuPayload).Content.Text) + assert.Equal(t, "[Issue-test/repo #2]: closed\ncrash\nhttp://localhost:3000/test/repo/issues/2\nIssue by user1\nOperator: user1\nAssignees: user1\n\nissue body", pl.(*FeishuPayload).Content.Text) }) t.Run("IssueComment", func(t *testing.T) { @@ -92,7 +92,7 @@ func TestFeishuPayload(t *testing.T) { require.NotNil(t, pl) require.IsType(t, &FeishuPayload{}, pl) - assert.Equal(t, "[Issue Comment-test/repo #2]: created\r\nhttp://localhost:3000/test/repo/issues/2\r\nIssue by user1\r\nOperator: user1\r\n\r\nmore info needed", pl.(*FeishuPayload).Content.Text) + assert.Equal(t, "[Comment-test/repo #2]: created\ncrash\nhttp://localhost:3000/test/repo/issues/2\nIssue by user1\nOperator: user1\n\nmore info needed", pl.(*FeishuPayload).Content.Text) }) t.Run("PullRequest", func(t *testing.T) { @@ -104,7 +104,7 @@ func TestFeishuPayload(t *testing.T) { require.NotNil(t, pl) require.IsType(t, &FeishuPayload{}, pl) - assert.Equal(t, "[PullRequest-test/repo #12]: opened\r\nhttp://localhost:3000/test/repo/pulls/12\r\nPullRequest by user1\r\nOperator: user1\r\nAssignees: user1\r\n\r\nfixes bug #2", pl.(*FeishuPayload).Content.Text) + assert.Equal(t, "[PullRequest-test/repo #12]: opened\nFix bug\nhttp://localhost:3000/test/repo/pulls/12\nPullRequest by user1\nOperator: user1\nAssignees: user1\n\nfixes bug #2", pl.(*FeishuPayload).Content.Text) }) t.Run("PullRequestComment", func(t *testing.T) { @@ -116,7 +116,7 @@ func TestFeishuPayload(t *testing.T) { require.NotNil(t, pl) require.IsType(t, &FeishuPayload{}, pl) - assert.Equal(t, "[Issue Comment-test/repo #12]: created\r\nhttp://localhost:3000/test/repo/pulls/12\r\nIssue by user1\r\nOperator: user1\r\n\r\nchanges requested", pl.(*FeishuPayload).Content.Text) + assert.Equal(t, "[Comment-test/repo #12]: created\nFix bug\nhttp://localhost:3000/test/repo/pulls/12\nPullRequest by user1\nOperator: user1\n\nchanges requested", pl.(*FeishuPayload).Content.Text) }) t.Run("Review", func(t *testing.T) { From 3cdd931890b6268da37c55ff87cc38a4fcb2ab97 Mon Sep 17 00:00:00 2001 From: Makonike Date: Sat, 29 Jul 2023 15:10:41 +0800 Subject: [PATCH 6/8] fix --- services/webhook/feishu_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/services/webhook/feishu_test.go b/services/webhook/feishu_test.go index 4365cecfc495f..8b671eac7d1bd 100644 --- a/services/webhook/feishu_test.go +++ b/services/webhook/feishu_test.go @@ -6,7 +6,6 @@ package webhook import ( api "code.gitea.io/gitea/modules/structs" webhook_module "code.gitea.io/gitea/modules/webhook" - _ "github.com/mattn/go-sqlite3" "testing" "github.com/stretchr/testify/assert" From c1ce86fe98e167f3bb06737c3d45f7c939f25caa Mon Sep 17 00:00:00 2001 From: Makonike Date: Sat, 29 Jul 2023 15:11:05 +0800 Subject: [PATCH 7/8] fix --- services/webhook/feishu_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/services/webhook/feishu_test.go b/services/webhook/feishu_test.go index 8b671eac7d1bd..a3182e82b09af 100644 --- a/services/webhook/feishu_test.go +++ b/services/webhook/feishu_test.go @@ -4,9 +4,10 @@ package webhook import ( + "testing" + api "code.gitea.io/gitea/modules/structs" webhook_module "code.gitea.io/gitea/modules/webhook" - "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" From 62a6a10fc4c9531f8becbabfa98569a75a51e4f9 Mon Sep 17 00:00:00 2001 From: Makonike Date: Wed, 23 Aug 2023 17:10:44 +0800 Subject: [PATCH 8/8] fix: remove merged --- services/webhook/general.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/services/webhook/general.go b/services/webhook/general.go index 4c04c8049e59a..20085976cc26f 100644 --- a/services/webhook/general.go +++ b/services/webhook/general.go @@ -30,9 +30,6 @@ func htmlLinkFormatter(url, text string) string { // getPullRequestInfo gets the information for a pull request func getPullRequestInfo(p *api.PullRequestPayload) (title, link, by, operator, operateResult, assignees string) { - if p.PullRequest.HasMerged { - p.Action = "merged" - } title = fmt.Sprintf("[PullRequest-%s #%d]: %s\n%s", p.Repository.FullName, p.PullRequest.Index, p.Action, p.PullRequest.Title) assignList := p.PullRequest.Assignees assignStringList := make([]string, len(assignList))