diff options
author | kolaente <k@knt.li> | 2022-11-03 16:49:00 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-11-03 23:49:00 +0800 |
commit | 085f717529008c31b147f76ea7eeaf06ca8801bd (patch) | |
tree | 673e6904f488d1c2cd0def082d4ff979c72a3de2 /services | |
parent | f17edfaf5a31ea3f4e9152424b75c2c4986acbe3 (diff) | |
download | gitea-085f717529008c31b147f76ea7eeaf06ca8801bd.tar.gz gitea-085f717529008c31b147f76ea7eeaf06ca8801bd.zip |
feat: notify doers of a merge when automerging (#21553)
I found myself wondering whether a PR I scheduled for automerge was
actually merged. It was, but I didn't receive a mail notification for it
- that makes sense considering I am the doer and usually don't want to
receive such notifications. But ideally I want to receive a notification
when a PR was merged because I scheduled it for automerge.
This PR implements exactly that.
The implementation works, but I wonder if there's a way to avoid passing
the "This PR was automerged" state down so much. I tried solving this
via the database (checking if there's an automerge scheduled for this PR
when sending the notification) but that did not work reliably, probably
because sending the notification happens async and the entry might have
already been deleted. My implementation might be the most
straightforward but maybe not the most elegant.
Signed-off-by: Andrew Thornton <art27@cantab.net>
Co-authored-by: Lauris BH <lauris@nix.lv>
Co-authored-by: Andrew Thornton <art27@cantab.net>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Diffstat (limited to 'services')
-rw-r--r-- | services/automerge/automerge.go | 2 | ||||
-rw-r--r-- | services/mailer/mail.go | 4 | ||||
-rw-r--r-- | services/mailer/mail_issue.go | 29 | ||||
-rw-r--r-- | services/pull/merge.go | 8 |
4 files changed, 25 insertions, 18 deletions
diff --git a/services/automerge/automerge.go b/services/automerge/automerge.go index ca008ebfe6..3ee8af2344 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.go b/services/mailer/mail.go index a5bfa496f9..85a7d107e5 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 33a20694e8..61e276805d 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) } @@ -181,17 +182,19 @@ 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 - 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/merge.go b/services/pull/merge.go index 0ca3730183..56ee9c9a73 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: %w", err) @@ -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) + if wasAutoMerged { + notification.NotifyAutoMergePullRequest(pr, doer) + } else { + notification.NotifyMergePullRequest(pr, doer) + } // Reset cached commit count cache.Remove(pr.Issue.Repo.GetCommitsCountCacheKey(pr.BaseBranch, true)) |