Skip to content

Commit

Permalink
Merge pull request #2 from zeripath/patch-feature/notify-pr-doer-on-a…
Browse files Browse the repository at this point in the history
…utomerge

New AutoMergePullRequest action
  • Loading branch information
kolaente authored Oct 31, 2022
2 parents 5e1e1ff + af027f5 commit ab83966
Show file tree
Hide file tree
Showing 15 changed files with 81 additions and 23 deletions.
3 changes: 2 additions & 1 deletion models/activities/action.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ const (
ActionPublishRelease // 24
ActionPullReviewDismissed // 25
ActionPullRequestReadyForReview // 26
ActionAutoMergePullRequest // 27
)

// Action represents user operation type and other information to
Expand Down Expand Up @@ -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
}
Expand Down
16 changes: 15 additions & 1 deletion modules/notification/action/action.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 {
Expand Down
3 changes: 2 additions & 1 deletion modules/notification/base/notifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 5 additions & 1 deletion modules/notification/base/null.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
22 changes: 16 additions & 6 deletions modules/notification/mail/mail.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand All @@ -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)
}
}
Expand All @@ -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)
}
}
Expand Down Expand Up @@ -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)
}
}
Expand Down
11 changes: 9 additions & 2 deletions modules/notification/notification.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down
6 changes: 5 additions & 1 deletion modules/notification/ui/ui.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
7 changes: 6 additions & 1 deletion modules/notification/webhook/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down
2 changes: 1 addition & 1 deletion modules/templates/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
1 change: 1 addition & 0 deletions options/locale/locale_en-US.ini
Original file line number Diff line number Diff line change
Expand Up @@ -3001,6 +3001,7 @@ reopen_pull_request = `reopened pull request <a href="%[1]s">%[3]s#%[2]s</a>`
comment_issue = `commented on issue <a href="%[1]s">%[3]s#%[2]s</a>`
comment_pull = `commented on pull request <a href="%[1]s">%[3]s#%[2]s</a>`
merge_pull_request = `merged pull request <a href="%[1]s">%[3]s#%[2]s</a>`
auto_merge_pull_request = `automatically merged pull request <a href="%[1]s">%[3]s#%[2]s</a>`
transfer_repo = transferred repository <code>%s</code> to <a href="%s">%s</a>
push_tag = pushed tag <a href="%[2]s">%[3]s</a> to <a href="%[1]s">%[4]s</a>
delete_tag = deleted tag %[2]s from <a href="%[1]s">%[3]s</a>
Expand Down
8 changes: 7 additions & 1 deletion routers/web/feed/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 == "#" {
Expand Down Expand Up @@ -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()
Expand Down
4 changes: 2 additions & 2 deletions services/mailer/mail.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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"
Expand Down
5 changes: 3 additions & 2 deletions services/mailer/mail_issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion services/pull/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 6 additions & 2 deletions services/pull/merge.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -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
}

0 comments on commit ab83966

Please sign in to comment.