Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 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) {
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,
Expand Down
2 changes: 1 addition & 1 deletion modules/notification/base/notifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion modules/notification/base/null.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 6 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); err != nil {
if err := mailer.MailParticipants(issue, issue.Poster, activities_model.ActionCreateIssue, mentions, false); 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); err != nil {
if err := mailer.MailParticipants(issue, doer, actionType, nil, false); 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); 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)
}
}
Expand Down Expand Up @@ -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)
}
}
Expand Down
4 changes: 2 additions & 2 deletions modules/notification/notification.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

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

Expand Down
2 changes: 1 addition & 1 deletion routers/api/v1/repo/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion routers/web/repo/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
2 changes: 1 addition & 1 deletion services/automerge/automerge.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
28 changes: 15 additions & 13 deletions services/mailer/mail_issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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
Expand All @@ -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)
}
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)
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
Expand Down
6 changes: 3 additions & 3 deletions services/pull/merge.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -874,7 +874,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
}
6 changes: 3 additions & 3 deletions tests/integration/pull_merge_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down