From dbe496c7d69043902bf340141c24842f07e0f8bb Mon Sep 17 00:00:00 2001 From: kolaente Date: Sat, 22 Oct 2022 19:44:41 +0200 Subject: [PATCH 1/2] feat: notify doers of a merge when automerging --- modules/notification/action/action.go | 2 +- modules/notification/base/notifier.go | 2 +- modules/notification/base/null.go | 2 +- modules/notification/mail/mail.go | 12 +++++------ modules/notification/notification.go | 4 ++-- modules/notification/ui/ui.go | 2 +- modules/notification/webhook/webhook.go | 2 +- routers/api/v1/repo/pull.go | 2 +- routers/web/repo/pull.go | 2 +- services/automerge/automerge.go | 2 +- services/mailer/mail_issue.go | 28 +++++++++++++------------ services/pull/check.go | 2 +- services/pull/merge.go | 6 +++--- tests/integration/pull_merge_test.go | 6 +++--- 14 files changed, 38 insertions(+), 36 deletions(-) diff --git a/modules/notification/action/action.go b/modules/notification/action/action.go index d3ff8b156e1c7..f49aa76a13d6b 100644 --- a/modules/notification/action/action.go +++ b/modules/notification/action/action.go @@ -269,7 +269,7 @@ func (a *actionNotifier) NotifyPullRequestReview(pr *issues_model.PullRequest, r } } -func (*actionNotifier) NotifyMergePullRequest(pr *issues_model.PullRequest, doer *user_model.User) { +func (*actionNotifier) NotifyMergePullRequest(pr *issues_model.PullRequest, doer *user_model.User, wasAutoMerged bool) { if err := activities_model.NotifyWatchers(&activities_model.Action{ ActUserID: doer.ID, ActUser: doer, diff --git a/modules/notification/base/notifier.go b/modules/notification/base/notifier.go index d6cec92e19ba7..c01fc3702e328 100644 --- a/modules/notification/base/notifier.go +++ b/modules/notification/base/notifier.go @@ -34,7 +34,7 @@ type Notifier interface { NotifyIssueChangeLabels(doer *user_model.User, issue *issues_model.Issue, addedLabels, removedLabels []*issues_model.Label) NotifyNewPullRequest(pr *issues_model.PullRequest, mentions []*user_model.User) - NotifyMergePullRequest(*issues_model.PullRequest, *user_model.User) + NotifyMergePullRequest(pr *issues_model.PullRequest, doer *user_model.User, wasAutoMerged bool) NotifyPullRequestSynchronized(doer *user_model.User, pr *issues_model.PullRequest) NotifyPullRequestReview(pr *issues_model.PullRequest, review *issues_model.Review, comment *issues_model.Comment, mentions []*user_model.User) NotifyPullRequestCodeComment(pr *issues_model.PullRequest, comment *issues_model.Comment, mentions []*user_model.User) diff --git a/modules/notification/base/null.go b/modules/notification/base/null.go index b113ae79e1e54..135ded4ded74c 100644 --- a/modules/notification/base/null.go +++ b/modules/notification/base/null.go @@ -51,7 +51,7 @@ func (*NullNotifier) NotifyPullRequestCodeComment(pr *issues_model.PullRequest, } // NotifyMergePullRequest places a place holder function -func (*NullNotifier) NotifyMergePullRequest(pr *issues_model.PullRequest, doer *user_model.User) { +func (*NullNotifier) NotifyMergePullRequest(pr *issues_model.PullRequest, doer *user_model.User, b bool) { } // NotifyPullRequestSynchronized places a place holder function diff --git a/modules/notification/mail/mail.go b/modules/notification/mail/mail.go index 100b4eb36f86c..b0db48ac43f9f 100644 --- a/modules/notification/mail/mail.go +++ b/modules/notification/mail/mail.go @@ -54,7 +54,7 @@ func (m *mailNotifier) NotifyCreateIssueComment(doer *user_model.User, repo *rep } func (m *mailNotifier) NotifyNewIssue(issue *issues_model.Issue, mentions []*user_model.User) { - if err := mailer.MailParticipants(issue, issue.Poster, activities_model.ActionCreateIssue, mentions); err != nil { + if err := mailer.MailParticipants(issue, issue.Poster, activities_model.ActionCreateIssue, mentions, false); err != nil { log.Error("MailParticipants: %v", err) } } @@ -75,7 +75,7 @@ func (m *mailNotifier) NotifyIssueChangeStatus(doer *user_model.User, issue *iss } } - if err := mailer.MailParticipants(issue, doer, actionType, nil); err != nil { + if err := mailer.MailParticipants(issue, doer, actionType, nil, false); err != nil { log.Error("MailParticipants: %v", err) } } @@ -86,14 +86,14 @@ func (m *mailNotifier) NotifyIssueChangeTitle(doer *user_model.User, issue *issu return } if issue.IsPull && issues_model.HasWorkInProgressPrefix(oldTitle) && !issue.PullRequest.IsWorkInProgress() { - if err := mailer.MailParticipants(issue, doer, activities_model.ActionPullRequestReadyForReview, nil); err != nil { + if err := mailer.MailParticipants(issue, doer, activities_model.ActionPullRequestReadyForReview, nil, false); err != nil { log.Error("MailParticipants: %v", err) } } } func (m *mailNotifier) NotifyNewPullRequest(pr *issues_model.PullRequest, mentions []*user_model.User) { - if err := mailer.MailParticipants(pr.Issue, pr.Issue.Poster, activities_model.ActionCreatePullRequest, mentions); err != nil { + if err := mailer.MailParticipants(pr.Issue, pr.Issue.Poster, activities_model.ActionCreatePullRequest, mentions, false); err != nil { log.Error("MailParticipants: %v", err) } } @@ -143,12 +143,12 @@ func (m *mailNotifier) NotifyPullReviewRequest(doer *user_model.User, issue *iss } } -func (m *mailNotifier) NotifyMergePullRequest(pr *issues_model.PullRequest, doer *user_model.User) { +func (m *mailNotifier) NotifyMergePullRequest(pr *issues_model.PullRequest, doer *user_model.User, wasAutoMerged bool) { if err := pr.LoadIssue(); err != nil { log.Error("pr.LoadIssue: %v", err) return } - if err := mailer.MailParticipants(pr.Issue, doer, activities_model.ActionMergePullRequest, nil); err != nil { + if err := mailer.MailParticipants(pr.Issue, doer, activities_model.ActionMergePullRequest, nil, wasAutoMerged); err != nil { log.Error("MailParticipants: %v", err) } } diff --git a/modules/notification/notification.go b/modules/notification/notification.go index 693c7f5c22f80..814f2f1bebfd5 100644 --- a/modules/notification/notification.go +++ b/modules/notification/notification.go @@ -92,9 +92,9 @@ func NotifyDeleteIssue(doer *user_model.User, issue *issues_model.Issue) { } // NotifyMergePullRequest notifies merge pull request to notifiers -func NotifyMergePullRequest(pr *issues_model.PullRequest, doer *user_model.User) { +func NotifyMergePullRequest(pr *issues_model.PullRequest, doer *user_model.User, wasAutoMerged bool) { for _, notifier := range notifiers { - notifier.NotifyMergePullRequest(pr, doer) + notifier.NotifyMergePullRequest(pr, doer, wasAutoMerged) } } diff --git a/modules/notification/ui/ui.go b/modules/notification/ui/ui.go index 4d96a6b0edd46..29d941530bb37 100644 --- a/modules/notification/ui/ui.go +++ b/modules/notification/ui/ui.go @@ -112,7 +112,7 @@ func (ns *notificationService) NotifyIssueChangeTitle(doer *user_model.User, iss } } -func (ns *notificationService) NotifyMergePullRequest(pr *issues_model.PullRequest, doer *user_model.User) { +func (ns *notificationService) NotifyMergePullRequest(pr *issues_model.PullRequest, doer *user_model.User, wasAutoMerged bool) { _ = ns.issueQueue.Push(issueNotificationOpts{ IssueID: pr.Issue.ID, NotificationAuthorID: doer.ID, diff --git a/modules/notification/webhook/webhook.go b/modules/notification/webhook/webhook.go index 630b56598464d..c8de91b15d762 100644 --- a/modules/notification/webhook/webhook.go +++ b/modules/notification/webhook/webhook.go @@ -632,7 +632,7 @@ func (m *webhookNotifier) NotifyPushCommits(pusher *user_model.User, repo *repo_ } } -func (*webhookNotifier) NotifyMergePullRequest(pr *issues_model.PullRequest, doer *user_model.User) { +func (*webhookNotifier) NotifyMergePullRequest(pr *issues_model.PullRequest, doer *user_model.User, wasAutoMerged bool) { ctx, _, finished := process.GetManager().AddContext(graceful.GetManager().HammerContext(), fmt.Sprintf("webhook.NotifyMergePullRequest Pull[%d] #%d in [%d]", pr.ID, pr.Index, pr.BaseRepoID)) defer finished() diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index f6507dceba89a..754d4bbd02794 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -839,7 +839,7 @@ func MergePullRequest(ctx *context.APIContext) { } } - if err := pull_service.Merge(ctx, pr, ctx.Doer, ctx.Repo.GitRepo, repo_model.MergeStyle(form.Do), form.HeadCommitID, message); err != nil { + if err := pull_service.Merge(ctx, pr, ctx.Doer, ctx.Repo.GitRepo, repo_model.MergeStyle(form.Do), form.HeadCommitID, message, false); err != nil { if models.IsErrInvalidMergeStyle(err) { ctx.Error(http.StatusMethodNotAllowed, "Invalid merge style", fmt.Errorf("%s is not allowed an allowed merge style for this repository", repo_model.MergeStyle(form.Do))) } else if models.IsErrMergeConflicts(err) { diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index aa2c4cdb53b2d..fe2338b8a02ef 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -1002,7 +1002,7 @@ func MergePullRequest(ctx *context.Context) { } } - if err := pull_service.Merge(ctx, pr, ctx.Doer, ctx.Repo.GitRepo, repo_model.MergeStyle(form.Do), form.HeadCommitID, message); err != nil { + if err := pull_service.Merge(ctx, pr, ctx.Doer, ctx.Repo.GitRepo, repo_model.MergeStyle(form.Do), form.HeadCommitID, message, false); err != nil { if models.IsErrInvalidMergeStyle(err) { ctx.Flash.Error(ctx.Tr("repo.pulls.invalid_merge_option")) ctx.Redirect(issue.Link()) diff --git a/services/automerge/automerge.go b/services/automerge/automerge.go index ca008ebfe6c82..3ee8af2344d3f 100644 --- a/services/automerge/automerge.go +++ b/services/automerge/automerge.go @@ -257,7 +257,7 @@ func handlePull(pullID int64, sha string) { defer baseGitRepo.Close() } - if err := pull_service.Merge(ctx, pr, doer, baseGitRepo, scheduledPRM.MergeStyle, "", scheduledPRM.Message); err != nil { + if err := pull_service.Merge(ctx, pr, doer, baseGitRepo, scheduledPRM.MergeStyle, "", scheduledPRM.Message, true); err != nil { log.Error("pull_service.Merge: %v", err) return } diff --git a/services/mailer/mail_issue.go b/services/mailer/mail_issue.go index 15bfa4af41625..79b37f1150fe4 100644 --- a/services/mailer/mail_issue.go +++ b/services/mailer/mail_issue.go @@ -25,11 +25,12 @@ func fallbackMailSubject(issue *issues_model.Issue) string { type mailCommentContext struct { context.Context - Issue *issues_model.Issue - Doer *user_model.User - ActionType activities_model.ActionType - Content string - Comment *issues_model.Comment + Issue *issues_model.Issue + Doer *user_model.User + ActionType activities_model.ActionType + Content string + Comment *issues_model.Comment + ForceDoerNotification bool } const ( @@ -93,7 +94,7 @@ func mailIssueCommentToParticipants(ctx *mailCommentContext, mentions []*user_mo visited := make(container.Set[int64], len(unfiltered)+len(mentions)+1) // Avoid mailing the doer - if ctx.Doer.EmailNotificationsPreference != user_model.EmailNotificationsAndYourOwn { + if ctx.Doer.EmailNotificationsPreference != user_model.EmailNotificationsAndYourOwn && !ctx.ForceDoerNotification { visited.Add(ctx.Doer.ID) } @@ -172,7 +173,7 @@ func mailIssueCommentBatch(ctx *mailCommentContext, users []*user_model.User, vi // MailParticipants sends new issue thread created emails to repository watchers // and mentioned people. -func MailParticipants(issue *issues_model.Issue, doer *user_model.User, opType activities_model.ActionType, mentions []*user_model.User) error { +func MailParticipants(issue *issues_model.Issue, doer *user_model.User, opType activities_model.ActionType, mentions []*user_model.User, forceDoerNotification bool) error { if setting.MailService == nil { // No mail service configured return nil @@ -186,12 +187,13 @@ func MailParticipants(issue *issues_model.Issue, doer *user_model.User, opType a } if err := mailIssueCommentToParticipants( &mailCommentContext{ - Context: context.TODO(), // TODO: use a correct context - Issue: issue, - Doer: doer, - ActionType: opType, - Content: content, - Comment: nil, + Context: context.TODO(), // TODO: use a correct context + Issue: issue, + Doer: doer, + ActionType: opType, + Content: content, + Comment: nil, + ForceDoerNotification: forceDoerNotification, }, mentions); err != nil { log.Error("mailIssueCommentToParticipants: %v", err) } diff --git a/services/pull/check.go b/services/pull/check.go index 288f4dc0b73b7..485c138b00ad3 100644 --- a/services/pull/check.go +++ b/services/pull/check.go @@ -278,7 +278,7 @@ func manuallyMerged(ctx context.Context, pr *issues_model.PullRequest) bool { return false } - notification.NotifyMergePullRequest(pr, merger) + notification.NotifyMergePullRequest(pr, merger, false) log.Info("manuallyMerged[%d]: Marked as manually merged into %s/%s by commit id: %s", pr.ID, pr.BaseRepo.Name, pr.BaseBranch, commit.ID.String()) return true diff --git a/services/pull/merge.go b/services/pull/merge.go index 6f3df6ab2a5a3..2325f3b5757e2 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -133,7 +133,7 @@ func GetDefaultMergeMessage(baseGitRepo *git.Repository, pr *issues_model.PullRe // Merge merges pull request to base repository. // Caller should check PR is ready to be merged (review and status checks) -func Merge(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.User, baseGitRepo *git.Repository, mergeStyle repo_model.MergeStyle, expectedHeadCommitID, message string) error { +func Merge(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.User, baseGitRepo *git.Repository, mergeStyle repo_model.MergeStyle, expectedHeadCommitID, message string, wasAutoMerged bool) error { if err := pr.LoadHeadRepo(); err != nil { log.Error("LoadHeadRepo: %v", err) return fmt.Errorf("LoadHeadRepo: %v", err) @@ -193,7 +193,7 @@ func Merge(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.U log.Error("GetOwner for issue repo [%d]: %v", pr.ID, err) } - notification.NotifyMergePullRequest(pr, doer) + notification.NotifyMergePullRequest(pr, doer, wasAutoMerged) // Reset cached commit count cache.Remove(pr.Issue.Repo.GetCommitsCountCacheKey(pr.BaseBranch, true)) @@ -871,7 +871,7 @@ func MergedManually(pr *issues_model.PullRequest, doer *user_model.User, baseGit return err } - notification.NotifyMergePullRequest(pr, doer) + notification.NotifyMergePullRequest(pr, doer, false) log.Info("manuallyMerged[%d]: Marked as manually merged into %s/%s by commit id: %s", pr.ID, pr.BaseRepo.Name, pr.BaseBranch, commitID) return nil } diff --git a/tests/integration/pull_merge_test.go b/tests/integration/pull_merge_test.go index 335dae4b38adf..3968d09ed4c7f 100644 --- a/tests/integration/pull_merge_test.go +++ b/tests/integration/pull_merge_test.go @@ -245,11 +245,11 @@ func TestCantMergeConflict(t *testing.T) { gitRepo, err := git.OpenRepository(git.DefaultContext, repo_model.RepoPath(user1.Name, repo1.Name)) assert.NoError(t, err) - err = pull.Merge(context.Background(), pr, user1, gitRepo, repo_model.MergeStyleMerge, "", "CONFLICT") + err = pull.Merge(context.Background(), pr, user1, gitRepo, repo_model.MergeStyleMerge, "", "CONFLICT", false) assert.Error(t, err, "Merge should return an error due to conflict") assert.True(t, models.IsErrMergeConflicts(err), "Merge error is not a conflict error") - err = pull.Merge(context.Background(), pr, user1, gitRepo, repo_model.MergeStyleRebase, "", "CONFLICT") + err = pull.Merge(context.Background(), pr, user1, gitRepo, repo_model.MergeStyleRebase, "", "CONFLICT", false) assert.Error(t, err, "Merge should return an error due to conflict") assert.True(t, models.IsErrRebaseConflicts(err), "Merge error is not a conflict error") gitRepo.Close() @@ -344,7 +344,7 @@ func TestCantMergeUnrelated(t *testing.T) { BaseBranch: "base", }) - err = pull.Merge(context.Background(), pr, user1, gitRepo, repo_model.MergeStyleMerge, "", "UNRELATED") + err = pull.Merge(context.Background(), pr, user1, gitRepo, repo_model.MergeStyleMerge, "", "UNRELATED", false) assert.Error(t, err, "Merge should return an error due to unrelated") assert.True(t, models.IsErrMergeUnrelatedHistories(err), "Merge error is not a unrelated histories error") gitRepo.Close() From af027f584fc3f61e3b67ffd8b1d411ce423282d9 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sat, 29 Oct 2022 18:39:39 +0100 Subject: [PATCH 2/2] New AutoMergePullRequest action Signed-off-by: Andrew Thornton --- models/activities/action.go | 3 ++- modules/notification/action/action.go | 16 +++++++++++++++- modules/notification/base/notifier.go | 3 ++- modules/notification/base/null.go | 6 +++++- modules/notification/mail/mail.go | 22 ++++++++++++++++------ modules/notification/notification.go | 11 +++++++++-- modules/notification/ui/ui.go | 6 +++++- modules/notification/webhook/webhook.go | 7 ++++++- modules/templates/helper.go | 2 +- options/locale/locale_en-US.ini | 1 + routers/web/feed/convert.go | 8 +++++++- services/mailer/mail.go | 4 ++-- services/mailer/mail_issue.go | 5 +++-- services/pull/check.go | 2 +- services/pull/merge.go | 8 ++++++-- 15 files changed, 81 insertions(+), 23 deletions(-) diff --git a/models/activities/action.go b/models/activities/action.go index 147511edec6f0..cad3263c2d790 100644 --- a/models/activities/action.go +++ b/models/activities/action.go @@ -64,6 +64,7 @@ const ( ActionPublishRelease // 24 ActionPullReviewDismissed // 25 ActionPullRequestReadyForReview // 26 + ActionAutoMergePullRequest // 27 ) // Action represents user operation type and other information to @@ -550,7 +551,7 @@ func notifyWatchers(ctx context.Context, actions ...*Action) error { if !permIssue[i] { continue } - case ActionCreatePullRequest, ActionCommentPull, ActionMergePullRequest, ActionClosePullRequest, ActionReopenPullRequest: + case ActionCreatePullRequest, ActionCommentPull, ActionMergePullRequest, ActionClosePullRequest, ActionReopenPullRequest, ActionAutoMergePullRequest: if !permPR[i] { continue } diff --git a/modules/notification/action/action.go b/modules/notification/action/action.go index f49aa76a13d6b..44d115f3d72a1 100644 --- a/modules/notification/action/action.go +++ b/modules/notification/action/action.go @@ -269,7 +269,7 @@ func (a *actionNotifier) NotifyPullRequestReview(pr *issues_model.PullRequest, r } } -func (*actionNotifier) NotifyMergePullRequest(pr *issues_model.PullRequest, doer *user_model.User, wasAutoMerged bool) { +func (*actionNotifier) NotifyMergePullRequest(pr *issues_model.PullRequest, doer *user_model.User) { if err := activities_model.NotifyWatchers(&activities_model.Action{ ActUserID: doer.ID, ActUser: doer, @@ -283,6 +283,20 @@ func (*actionNotifier) NotifyMergePullRequest(pr *issues_model.PullRequest, doer } } +func (*actionNotifier) NotifyAutoMergePullRequest(pr *issues_model.PullRequest, doer *user_model.User) { + if err := activities_model.NotifyWatchers(&activities_model.Action{ + ActUserID: doer.ID, + ActUser: doer, + OpType: activities_model.ActionAutoMergePullRequest, + Content: fmt.Sprintf("%d|%s", pr.Issue.Index, pr.Issue.Title), + RepoID: pr.Issue.Repo.ID, + Repo: pr.Issue.Repo, + IsPrivate: pr.Issue.Repo.IsPrivate, + }); err != nil { + log.Error("NotifyWatchers [%d]: %v", pr.ID, err) + } +} + func (*actionNotifier) NotifyPullRevieweDismiss(doer *user_model.User, review *issues_model.Review, comment *issues_model.Comment) { reviewerName := review.Reviewer.Name if len(review.OriginalAuthor) > 0 { diff --git a/modules/notification/base/notifier.go b/modules/notification/base/notifier.go index c01fc3702e328..9edab8213fab4 100644 --- a/modules/notification/base/notifier.go +++ b/modules/notification/base/notifier.go @@ -34,7 +34,8 @@ type Notifier interface { NotifyIssueChangeLabels(doer *user_model.User, issue *issues_model.Issue, addedLabels, removedLabels []*issues_model.Label) NotifyNewPullRequest(pr *issues_model.PullRequest, mentions []*user_model.User) - NotifyMergePullRequest(pr *issues_model.PullRequest, doer *user_model.User, wasAutoMerged bool) + NotifyMergePullRequest(pr *issues_model.PullRequest, doer *user_model.User) + NotifyAutoMergePullRequest(pr *issues_model.PullRequest, doer *user_model.User) NotifyPullRequestSynchronized(doer *user_model.User, pr *issues_model.PullRequest) NotifyPullRequestReview(pr *issues_model.PullRequest, review *issues_model.Review, comment *issues_model.Comment, mentions []*user_model.User) NotifyPullRequestCodeComment(pr *issues_model.PullRequest, comment *issues_model.Comment, mentions []*user_model.User) diff --git a/modules/notification/base/null.go b/modules/notification/base/null.go index 135ded4ded74c..f051fbc26f461 100644 --- a/modules/notification/base/null.go +++ b/modules/notification/base/null.go @@ -51,7 +51,11 @@ func (*NullNotifier) NotifyPullRequestCodeComment(pr *issues_model.PullRequest, } // NotifyMergePullRequest places a place holder function -func (*NullNotifier) NotifyMergePullRequest(pr *issues_model.PullRequest, doer *user_model.User, b bool) { +func (*NullNotifier) NotifyMergePullRequest(pr *issues_model.PullRequest, doer *user_model.User) { +} + +// NotifyAutoMergePullRequest places a place holder function +func (*NullNotifier) NotifyAutoMergePullRequest(pr *issues_model.PullRequest, doer *user_model.User) { } // NotifyPullRequestSynchronized places a place holder function diff --git a/modules/notification/mail/mail.go b/modules/notification/mail/mail.go index b0db48ac43f9f..54f561839d0f4 100644 --- a/modules/notification/mail/mail.go +++ b/modules/notification/mail/mail.go @@ -54,7 +54,7 @@ func (m *mailNotifier) NotifyCreateIssueComment(doer *user_model.User, repo *rep } func (m *mailNotifier) NotifyNewIssue(issue *issues_model.Issue, mentions []*user_model.User) { - if err := mailer.MailParticipants(issue, issue.Poster, activities_model.ActionCreateIssue, mentions, false); err != nil { + if err := mailer.MailParticipants(issue, issue.Poster, activities_model.ActionCreateIssue, mentions); err != nil { log.Error("MailParticipants: %v", err) } } @@ -75,7 +75,7 @@ func (m *mailNotifier) NotifyIssueChangeStatus(doer *user_model.User, issue *iss } } - if err := mailer.MailParticipants(issue, doer, actionType, nil, false); err != nil { + if err := mailer.MailParticipants(issue, doer, actionType, nil); err != nil { log.Error("MailParticipants: %v", err) } } @@ -86,14 +86,14 @@ func (m *mailNotifier) NotifyIssueChangeTitle(doer *user_model.User, issue *issu return } if issue.IsPull && issues_model.HasWorkInProgressPrefix(oldTitle) && !issue.PullRequest.IsWorkInProgress() { - if err := mailer.MailParticipants(issue, doer, activities_model.ActionPullRequestReadyForReview, nil, false); err != nil { + if err := mailer.MailParticipants(issue, doer, activities_model.ActionPullRequestReadyForReview, nil); err != nil { log.Error("MailParticipants: %v", err) } } } func (m *mailNotifier) NotifyNewPullRequest(pr *issues_model.PullRequest, mentions []*user_model.User) { - if err := mailer.MailParticipants(pr.Issue, pr.Issue.Poster, activities_model.ActionCreatePullRequest, mentions, false); err != nil { + if err := mailer.MailParticipants(pr.Issue, pr.Issue.Poster, activities_model.ActionCreatePullRequest, mentions); err != nil { log.Error("MailParticipants: %v", err) } } @@ -143,12 +143,22 @@ func (m *mailNotifier) NotifyPullReviewRequest(doer *user_model.User, issue *iss } } -func (m *mailNotifier) NotifyMergePullRequest(pr *issues_model.PullRequest, doer *user_model.User, wasAutoMerged bool) { +func (m *mailNotifier) NotifyMergePullRequest(pr *issues_model.PullRequest, doer *user_model.User) { if err := pr.LoadIssue(); err != nil { log.Error("pr.LoadIssue: %v", err) return } - if err := mailer.MailParticipants(pr.Issue, doer, activities_model.ActionMergePullRequest, nil, wasAutoMerged); err != nil { + if err := mailer.MailParticipants(pr.Issue, doer, activities_model.ActionMergePullRequest, nil); err != nil { + log.Error("MailParticipants: %v", err) + } +} + +func (m *mailNotifier) NotifyAutoMergePullRequest(pr *issues_model.PullRequest, doer *user_model.User) { + if err := pr.LoadIssue(); err != nil { + log.Error("pr.LoadIssue: %v", err) + return + } + if err := mailer.MailParticipants(pr.Issue, doer, activities_model.ActionAutoMergePullRequest, nil); err != nil { log.Error("MailParticipants: %v", err) } } diff --git a/modules/notification/notification.go b/modules/notification/notification.go index 814f2f1bebfd5..7bdc0a04c4969 100644 --- a/modules/notification/notification.go +++ b/modules/notification/notification.go @@ -92,9 +92,16 @@ func NotifyDeleteIssue(doer *user_model.User, issue *issues_model.Issue) { } // NotifyMergePullRequest notifies merge pull request to notifiers -func NotifyMergePullRequest(pr *issues_model.PullRequest, doer *user_model.User, wasAutoMerged bool) { +func NotifyMergePullRequest(pr *issues_model.PullRequest, doer *user_model.User) { for _, notifier := range notifiers { - notifier.NotifyMergePullRequest(pr, doer, wasAutoMerged) + notifier.NotifyMergePullRequest(pr, doer) + } +} + +// NotifyAutoMergePullRequest notifies merge pull request to notifiers +func NotifyAutoMergePullRequest(pr *issues_model.PullRequest, doer *user_model.User) { + for _, notifier := range notifiers { + notifier.NotifyAutoMergePullRequest(pr, doer) } } diff --git a/modules/notification/ui/ui.go b/modules/notification/ui/ui.go index 29d941530bb37..0e2b3e67c7f6e 100644 --- a/modules/notification/ui/ui.go +++ b/modules/notification/ui/ui.go @@ -112,13 +112,17 @@ func (ns *notificationService) NotifyIssueChangeTitle(doer *user_model.User, iss } } -func (ns *notificationService) NotifyMergePullRequest(pr *issues_model.PullRequest, doer *user_model.User, wasAutoMerged bool) { +func (ns *notificationService) NotifyMergePullRequest(pr *issues_model.PullRequest, doer *user_model.User) { _ = ns.issueQueue.Push(issueNotificationOpts{ IssueID: pr.Issue.ID, NotificationAuthorID: doer.ID, }) } +func (ns *notificationService) NotifyAutoMergePullRequest(pr *issues_model.PullRequest, doer *user_model.User) { + ns.NotifyMergePullRequest(pr, doer) +} + func (ns *notificationService) NotifyNewPullRequest(pr *issues_model.PullRequest, mentions []*user_model.User) { if err := pr.LoadIssue(); err != nil { log.Error("Unable to load issue: %d for pr: %d: Error: %v", pr.IssueID, pr.ID, err) diff --git a/modules/notification/webhook/webhook.go b/modules/notification/webhook/webhook.go index c8de91b15d762..c591e1e34dbcd 100644 --- a/modules/notification/webhook/webhook.go +++ b/modules/notification/webhook/webhook.go @@ -632,7 +632,12 @@ func (m *webhookNotifier) NotifyPushCommits(pusher *user_model.User, repo *repo_ } } -func (*webhookNotifier) NotifyMergePullRequest(pr *issues_model.PullRequest, doer *user_model.User, wasAutoMerged bool) { +func (m *webhookNotifier) NotifyAutoMergePullRequest(pr *issues_model.PullRequest, doer *user_model.User) { + // just redirect to the NotifyMergePullRequest + m.NotifyMergePullRequest(pr, doer) +} + +func (*webhookNotifier) NotifyMergePullRequest(pr *issues_model.PullRequest, doer *user_model.User) { ctx, _, finished := process.GetManager().AddContext(graceful.GetManager().HammerContext(), fmt.Sprintf("webhook.NotifyMergePullRequest Pull[%d] #%d in [%d]", pr.ID, pr.Index, pr.BaseRepoID)) defer finished() diff --git a/modules/templates/helper.go b/modules/templates/helper.go index a7232914400ac..a127b98dc2ed9 100644 --- a/modules/templates/helper.go +++ b/modules/templates/helper.go @@ -905,7 +905,7 @@ func ActionIcon(opType activities_model.ActionType) string { return "git-pull-request" case activities_model.ActionCommentIssue, activities_model.ActionCommentPull: return "comment-discussion" - case activities_model.ActionMergePullRequest: + case activities_model.ActionMergePullRequest, activities_model.ActionAutoMergePullRequest: return "git-merge" case activities_model.ActionCloseIssue, activities_model.ActionClosePullRequest: return "issue-closed" diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 1566dfc97d422..d098b31740f36 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -3001,6 +3001,7 @@ reopen_pull_request = `reopened pull request %[3]s#%[2]s` comment_issue = `commented on issue %[3]s#%[2]s` comment_pull = `commented on pull request %[3]s#%[2]s` merge_pull_request = `merged pull request %[3]s#%[2]s` +auto_merge_pull_request = `automatically merged pull request %[3]s#%[2]s` transfer_repo = transferred repository %s to %s push_tag = pushed tag %[3]s to %[4]s delete_tag = deleted tag %[2]s from %[3]s diff --git a/routers/web/feed/convert.go b/routers/web/feed/convert.go index 306ecf7d6a3ff..c6fc352b647f8 100644 --- a/routers/web/feed/convert.go +++ b/routers/web/feed/convert.go @@ -115,6 +115,12 @@ func feedActionsToFeedItems(ctx *context.Context, actions activities_model.Actio link.Href = pullLink } title += ctx.TrHTMLEscapeArgs("action.merge_pull_request", pullLink, act.GetIssueInfos()[0], act.ShortRepoPath()) + case activities_model.ActionAutoMergePullRequest: + pullLink := toPullLink(act) + if link.Href == "#" { + link.Href = pullLink + } + title += ctx.TrHTMLEscapeArgs("action.auto_merge_pull_request", pullLink, act.GetIssueInfos()[0], act.ShortRepoPath()) case activities_model.ActionCloseIssue: issueLink := toIssueLink(act) if link.Href == "#" { @@ -221,7 +227,7 @@ func feedActionsToFeedItems(ctx *context.Context, actions activities_model.Actio if len(comment) != 0 { desc += "\n\n" + renderMarkdown(ctx, act, comment) } - case activities_model.ActionMergePullRequest: + case activities_model.ActionMergePullRequest, activities_model.ActionAutoMergePullRequest: desc = act.GetIssueInfos()[1] case activities_model.ActionCloseIssue, activities_model.ActionReopenIssue, activities_model.ActionClosePullRequest, activities_model.ActionReopenPullRequest: desc = act.GetIssueTitle() diff --git a/services/mailer/mail.go b/services/mailer/mail.go index a5bfa496f9eaf..85a7d107e5275 100644 --- a/services/mailer/mail.go +++ b/services/mailer/mail.go @@ -340,7 +340,7 @@ func createReference(issue *issues_model.Issue, comment *issues_model.Comment, a extra = fmt.Sprintf("/close/%d", time.Now().UnixNano()/1e6) case activities_model.ActionReopenIssue, activities_model.ActionReopenPullRequest: extra = fmt.Sprintf("/reopen/%d", time.Now().UnixNano()/1e6) - case activities_model.ActionMergePullRequest: + case activities_model.ActionMergePullRequest, activities_model.ActionAutoMergePullRequest: extra = fmt.Sprintf("/merge/%d", time.Now().UnixNano()/1e6) case activities_model.ActionPullRequestReadyForReview: extra = fmt.Sprintf("/ready/%d", time.Now().UnixNano()/1e6) @@ -451,7 +451,7 @@ func actionToTemplate(issue *issues_model.Issue, actionType activities_model.Act name = "close" case activities_model.ActionReopenIssue, activities_model.ActionReopenPullRequest: name = "reopen" - case activities_model.ActionMergePullRequest: + case activities_model.ActionMergePullRequest, activities_model.ActionAutoMergePullRequest: name = "merge" case activities_model.ActionPullReviewDismissed: name = "review_dismissed" diff --git a/services/mailer/mail_issue.go b/services/mailer/mail_issue.go index e865c32028ff1..61e276805dbfb 100644 --- a/services/mailer/mail_issue.go +++ b/services/mailer/mail_issue.go @@ -173,7 +173,7 @@ func mailIssueCommentBatch(ctx *mailCommentContext, users []*user_model.User, vi // MailParticipants sends new issue thread created emails to repository watchers // and mentioned people. -func MailParticipants(issue *issues_model.Issue, doer *user_model.User, opType activities_model.ActionType, mentions []*user_model.User, forceDoerNotification bool) error { +func MailParticipants(issue *issues_model.Issue, doer *user_model.User, opType activities_model.ActionType, mentions []*user_model.User) error { if setting.MailService == nil { // No mail service configured return nil @@ -182,9 +182,10 @@ func MailParticipants(issue *issues_model.Issue, doer *user_model.User, opType a content := issue.Content if opType == activities_model.ActionCloseIssue || opType == activities_model.ActionClosePullRequest || opType == activities_model.ActionReopenIssue || opType == activities_model.ActionReopenPullRequest || - opType == activities_model.ActionMergePullRequest { + opType == activities_model.ActionMergePullRequest || opType == activities_model.ActionAutoMergePullRequest { content = "" } + forceDoerNotification := opType == activities_model.ActionAutoMergePullRequest if err := mailIssueCommentToParticipants( &mailCommentContext{ Context: context.TODO(), // TODO: use a correct context diff --git a/services/pull/check.go b/services/pull/check.go index 46d3c8af7ca89..830ff640b52b8 100644 --- a/services/pull/check.go +++ b/services/pull/check.go @@ -278,7 +278,7 @@ func manuallyMerged(ctx context.Context, pr *issues_model.PullRequest) bool { return false } - notification.NotifyMergePullRequest(pr, merger, false) + notification.NotifyMergePullRequest(pr, merger) log.Info("manuallyMerged[%d]: Marked as manually merged into %s/%s by commit id: %s", pr.ID, pr.BaseRepo.Name, pr.BaseBranch, commit.ID.String()) return true diff --git a/services/pull/merge.go b/services/pull/merge.go index 99333654f2965..56ee9c9a734ec 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -193,7 +193,11 @@ func Merge(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.U log.Error("GetOwner for issue repo [%d]: %v", pr.ID, err) } - notification.NotifyMergePullRequest(pr, doer, wasAutoMerged) + if wasAutoMerged { + notification.NotifyAutoMergePullRequest(pr, doer) + } else { + notification.NotifyMergePullRequest(pr, doer) + } // Reset cached commit count cache.Remove(pr.Issue.Repo.GetCommitsCountCacheKey(pr.BaseBranch, true)) @@ -874,7 +878,7 @@ func MergedManually(pr *issues_model.PullRequest, doer *user_model.User, baseGit return err } - notification.NotifyMergePullRequest(pr, doer, false) + notification.NotifyMergePullRequest(pr, doer) log.Info("manuallyMerged[%d]: Marked as manually merged into %s/%s by commit id: %s", pr.ID, pr.BaseRepo.Name, pr.BaseBranch, commitID) return nil }