Skip to content

Commit

Permalink
Send mail to issue/pr assignee/reviewer also when OnMention is set (g…
Browse files Browse the repository at this point in the history
…o-gitea#18707)

I want to address go-gitea#17892, where emails notifications are not sent to assignees (issue and PR) and reviewers (PR) when they have the email setting Only email on mention enabled.

From the user experience perspective, when a user gets a issue/PR assigned or a PR review request, he/she would expect to be implicitly mentioned since the assignment or request is personal and targeting a single person only. Thus I see go-gitea#17892 as a bug. Could we therefore mark this ticket as such?

The changed code just explicitly checks for the EmailNotificationsOnMention setting beside the existing EmailNotificationsEnabled check. Too rude?

@lunny mentioned a mock mail server for tests, is there something ready. How could I make use of it?

go-gitea#12774 (comment)

Fix go-gitea#17892
  • Loading branch information
flozzone authored Feb 12, 2022
1 parent 2ddff74 commit a39287c
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions modules/notification/mail/mail.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ func (m *mailNotifier) NotifyPullRequestCodeComment(pr *models.PullRequest, comm

func (m *mailNotifier) NotifyIssueChangeAssignee(doer *user_model.User, issue *models.Issue, assignee *user_model.User, removed bool, comment *models.Comment) {
// mail only sent to added assignees and not self-assignee
if !removed && doer.ID != assignee.ID && assignee.EmailNotifications() == user_model.EmailNotificationsEnabled {
if !removed && doer.ID != assignee.ID && (assignee.EmailNotifications() == user_model.EmailNotificationsEnabled || assignee.EmailNotifications() == user_model.EmailNotificationsOnMention) {
ct := fmt.Sprintf("Assigned #%d.", issue.Index)
if err := mailer.SendIssueAssignedMail(issue, doer, ct, comment, []*user_model.User{assignee}); err != nil {
log.Error("Error in SendIssueAssignedMail for issue[%d] to assignee[%d]: %v", issue.ID, assignee.ID, err)
Expand All @@ -133,7 +133,7 @@ func (m *mailNotifier) NotifyIssueChangeAssignee(doer *user_model.User, issue *m
}

func (m *mailNotifier) NotifyPullReviewRequest(doer *user_model.User, issue *models.Issue, reviewer *user_model.User, isRequest bool, comment *models.Comment) {
if isRequest && doer.ID != reviewer.ID && reviewer.EmailNotifications() == user_model.EmailNotificationsEnabled {
if isRequest && doer.ID != reviewer.ID && (reviewer.EmailNotifications() == user_model.EmailNotificationsEnabled || reviewer.EmailNotifications() == user_model.EmailNotificationsOnMention) {
ct := fmt.Sprintf("Requested to review %s.", issue.HTMLURL())
if err := mailer.SendIssueAssignedMail(issue, doer, ct, comment, []*user_model.User{reviewer}); err != nil {
log.Error("Error in SendIssueAssignedMail for issue[%d] to reviewer[%d]: %v", issue.ID, reviewer.ID, err)
Expand Down

0 comments on commit a39287c

Please sign in to comment.