Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mail assignee when issue/pull request is assigned #8546

Merged
merged 23 commits into from
Oct 25, 2019
+333 −216
Merged

Mail assignee when issue/pull request is assigned #8546

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
fb0bb14
Send email to assigned user
davidsvantesson Oct 12, 2019
385c14d
Only send mail if enabled
davidsvantesson Oct 16, 2019
27bbc9d
Mail also when assigned through API
davidsvantesson Oct 16, 2019
b063676
Need to refactor functions from models to issue service
davidsvantesson Oct 16, 2019
1374db5
Refer to issue index rather than ID
davidsvantesson Oct 16, 2019
a3556c9
Merge branch 'master' into mail-assigned
davidsvantesson Oct 16, 2019
630dceb
Disable email notifications completly at initalization if global disable
davidsvantesson Oct 17, 2019
18187fa
Check of user enbled mail shall be in mail notification function only
davidsvantesson Oct 17, 2019
99f8db4
Initialize notifications from routers init function.
davidsvantesson Oct 18, 2019
ba31fed
Use the assigned comment when sending assigned mail
davidsvantesson Oct 18, 2019
04450f9
Refactor so that assignees always added as separate step when new iss…
davidsvantesson Oct 18, 2019
fea106d
Check error from AddAssignees
davidsvantesson Oct 18, 2019
5f6f77f
Check if user can be assiged to issue or pull request
davidsvantesson Oct 18, 2019
15b84ed
Missing return
davidsvantesson Oct 18, 2019
a7562d1
Refactor of CanBeAssigned check.
davidsvantesson Oct 19, 2019
075c7c6
Clarify function names (toggle rather than update/change), and clean up.
davidsvantesson Oct 19, 2019
e0b2648
Merge remote-tracking branch 'upstream/master' into mail-assigned
davidsvantesson Oct 19, 2019
0d5202c
Fix review comments.
davidsvantesson Oct 20, 2019
e761790
Merge branch 'master' into mail-assigned
lunny Oct 20, 2019
abc21a5
Flash error if assignees was not added when creating issue/pr
davidsvantesson Oct 21, 2019
430a3c8
Generate error if assignee users doesn't exist
davidsvantesson Oct 23, 2019
4c10b70
Merge branch 'master' into mail-assigned
lunny Oct 25, 2019
152fef9
Merge branch 'master' into mail-assigned
davidsvantesson Oct 25, 2019
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
49 changes: 3 additions & 46 deletions models/issue.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -984,7 +984,6 @@ type NewIssueOptions struct {
Repo *Repository
Issue *Issue
LabelIDs []int64
AssigneeIDs []int64
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why removed this?

Copy link
Contributor Author

@davidsvantesson davidsvantesson Oct 19, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved adding assignees outside the creation of new issues. Reason is that each added assignee creates a new comment (issue event) which is used in the notification.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think adding assignee comment should be in the transaction but not in the notifications?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure I understand now. The added assignee comment is used when sending the notification (the comment hash is included in the mail).

Copy link
Member

@guillep2k guillep2k Oct 20, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lunny If I've read that right, it's not a comment in the sense of a row in the comment table but a message for the contents of the e-mail. @davidsvantesson, perhaps it should be renamed to message to avoid confusion with the existing model/structure?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if I still misunderstand you. Adding an assignee creates a Comment in the database of type CommentTypeAssignees. CommentTypeComment is another type. Maybe IssueEvent would have been a better name to reflect it is not only comments. Anyhow the Comment (of type Assignees) is used when constructing the e-mail.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@davidsvantesson OK, sorry, I got confused about that. I understand @lunny's concern then. However, I'm not sure I'd block the creation of the issue because of an error while doing the assignments. That's why I think the error Flash is useful.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The input data is validated before issue creation, but it is of course possible it can still fail because of unconsidered reasons / bugs. I think it is a good idea to use error flash.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added an error flash now specifically for the case if add assignees fail when creating new issues/pr. Other error flashes I think can be for another PR.
I am not sure how to handle it for API creation. Right now it can potentially give error although the issue itself is created.

Attachments []string // In UUID format.
IsPull bool
}
Expand All @@ -1006,39 +1005,6 @@ func newIssue(e *xorm.Session, doer *User, opts NewIssueOptions) (err error) {
}
}

// Keep the old assignee id thingy for compatibility reasons
if opts.Issue.AssigneeID > 0 {
isAdded := false
// Check if the user has already been passed to issue.AssigneeIDs, if not, add it
for _, aID := range opts.AssigneeIDs {
if aID == opts.Issue.AssigneeID {
isAdded = true
break
}
}

if !isAdded {
opts.AssigneeIDs = append(opts.AssigneeIDs, opts.Issue.AssigneeID)
}
}

// Check for and validate assignees
if len(opts.AssigneeIDs) > 0 {
for _, assigneeID := range opts.AssigneeIDs {
user, err := getUserByID(e, assigneeID)
if err != nil {
return fmt.Errorf("getUserByID [user_id: %d, repo_id: %d]: %v", assigneeID, opts.Repo.ID, err)
}
valid, err := canBeAssigned(e, user, opts.Repo)
if err != nil {
return fmt.Errorf("canBeAssigned [user_id: %d, repo_id: %d]: %v", assigneeID, opts.Repo.ID, err)
}
if !valid {
return ErrUserDoesNotHaveAccessToRepo{UserID: assigneeID, RepoName: opts.Repo.Name}
}
}
}

// Milestone and assignee validation should happen before insert actual object.
davidsvantesson marked this conversation as resolved.
Show resolved Hide resolved
if _, err := e.SetExpr("`index`", "coalesce(MAX(`index`),0)+1").
Where("repo_id=?", opts.Issue.RepoID).
Expand All @@ -1060,14 +1026,6 @@ func newIssue(e *xorm.Session, doer *User, opts NewIssueOptions) (err error) {
}
}

// Insert the assignees
for _, assigneeID := range opts.AssigneeIDs {
err = opts.Issue.changeAssignee(e, doer, assigneeID, true)
if err != nil {
return err
}
}

if opts.IsPull {
_, err = e.Exec("UPDATE `repository` SET num_pulls = num_pulls + 1 WHERE id = ?", opts.Issue.RepoID)
} else {
Expand Down Expand Up @@ -1125,11 +1083,11 @@ func newIssue(e *xorm.Session, doer *User, opts NewIssueOptions) (err error) {
}

// NewIssue creates new issue with labels for repository.
func NewIssue(repo *Repository, issue *Issue, labelIDs []int64, assigneeIDs []int64, uuids []string) (err error) {
func NewIssue(repo *Repository, issue *Issue, labelIDs []int64, uuids []string) (err error) {
// Retry several times in case INSERT fails due to duplicate key for (repo_id, index); see #7887
i := 0
for {
if err = newIssueAttempt(repo, issue, labelIDs, assigneeIDs, uuids); err == nil {
if err = newIssueAttempt(repo, issue, labelIDs, uuids); err == nil {
return nil
}
if !IsErrNewIssueInsert(err) {
Expand All @@ -1143,7 +1101,7 @@ func NewIssue(repo *Repository, issue *Issue, labelIDs []int64, assigneeIDs []in
return fmt.Errorf("NewIssue: too many errors attempting to insert the new issue. Last error was: %v", err)
}

func newIssueAttempt(repo *Repository, issue *Issue, labelIDs []int64, assigneeIDs []int64, uuids []string) (err error) {
func newIssueAttempt(repo *Repository, issue *Issue, labelIDs []int64, uuids []string) (err error) {
sess := x.NewSession()
defer sess.Close()
if err = sess.Begin(); err != nil {
Expand All @@ -1155,7 +1113,6 @@ func newIssueAttempt(repo *Repository, issue *Issue, labelIDs []int64, assigneeI
Issue: issue,
LabelIDs: labelIDs,
Attachments: uuids,
AssigneeIDs: assigneeIDs,
}); err != nil {
if IsErrUserDoesNotHaveAccessToRepo(err) || IsErrNewIssueInsert(err) {
return err
Expand Down
109 changes: 22 additions & 87 deletions models/issue_assignees.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,73 +110,63 @@ func clearAssigneeByUserID(sess *xorm.Session, userID int64) (err error) {
return
}

// AddAssigneeIfNotAssigned adds an assignee only if he isn't aleady assigned to the issue
func AddAssigneeIfNotAssigned(issue *Issue, doer *User, assigneeID int64) (err error) {
// Check if the user is already assigned
isAssigned, err := IsUserAssignedToIssue(issue, &User{ID: assigneeID})
if err != nil {
return err
}

if !isAssigned {
return issue.ChangeAssignee(doer, assigneeID)
}
return nil
}

// UpdateAssignee deletes or adds an assignee to an issue
func UpdateAssignee(issue *Issue, doer *User, assigneeID int64) (err error) {
return issue.ChangeAssignee(doer, assigneeID)
_, _, err = issue.ChangeAssignee(doer, assigneeID)
return err
}

// ChangeAssignee changes the Assignee of this issue.
func (issue *Issue) ChangeAssignee(doer *User, assigneeID int64) (err error) {
func (issue *Issue) ChangeAssignee(doer *User, assigneeID int64) (removed bool, comment *Comment, err error) {
sess := x.NewSession()
davidsvantesson marked this conversation as resolved.
Show resolved Hide resolved
defer sess.Close()

if err := sess.Begin(); err != nil {
return err
return false, nil, err
}

if err := issue.changeAssignee(sess, doer, assigneeID, false); err != nil {
return err
removed, comment, err = issue.changeAssignee(sess, doer, assigneeID, false)
if err != nil {
return false, nil, err
}

if err := sess.Commit(); err != nil {
return err
return false, nil, err
}

go HookQueue.Add(issue.RepoID)
return nil

return removed, comment, nil
}

func (issue *Issue) changeAssignee(sess *xorm.Session, doer *User, assigneeID int64, isCreate bool) (err error) {
func (issue *Issue) changeAssignee(sess *xorm.Session, doer *User, assigneeID int64, isCreate bool) (removed bool, comment *Comment, err error) {
// Update the assignee
removed, err := updateIssueAssignee(sess, issue, assigneeID)
removed, err = updateIssueAssignee(sess, issue, assigneeID)
davidsvantesson marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return fmt.Errorf("UpdateIssueUserByAssignee: %v", err)
return false, nil, fmt.Errorf("UpdateIssueUserByAssignee: %v", err)
}

// Repo infos
if err = issue.loadRepo(sess); err != nil {
return fmt.Errorf("loadRepo: %v", err)
return false, nil, fmt.Errorf("loadRepo: %v", err)
}

// Comment
if _, err = createAssigneeComment(sess, doer, issue.Repo, issue, assigneeID, removed); err != nil {
return fmt.Errorf("createAssigneeComment: %v", err)
comment, err = createAssigneeComment(sess, doer, issue.Repo, issue, assigneeID, removed)
if err != nil {
return false, nil, fmt.Errorf("createAssigneeComment: %v", err)
}

// if pull request is in the middle of creation - don't call webhook
if isCreate {
return nil
return removed, comment, err
}

if issue.IsPull {
mode, _ := accessLevelUnit(sess, doer, issue.Repo, UnitTypePullRequests)

if err = issue.loadPullRequest(sess); err != nil {
return fmt.Errorf("loadPullRequest: %v", err)
return false, nil, fmt.Errorf("loadPullRequest: %v", err)
}
issue.PullRequest.Issue = issue
apiPullRequest := &api.PullRequestPayload{
Expand All @@ -192,7 +182,7 @@ func (issue *Issue) changeAssignee(sess *xorm.Session, doer *User, assigneeID in
}
if err := prepareWebhooks(sess, issue.Repo, HookEventPullRequest, apiPullRequest); err != nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The webhooks part doesn't seem related with changing assignees. I thihk it should be moved to another function.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually it is, below is help text for PR trigger event. 😉

Pull request opened, closed, reopened, edited, approved, rejected, review comment, assigned, unassigned, label updated, label cleared or synchronized.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually it is, below is help text for PR trigger event. 😉

Pull request opened, closed, reopened, edited, approved, rejected, review comment, assigned, unassigned, label updated, label cleared or synchronized.

Oh, that's not what I meant. 😄 What I meant is that this code seems to belong to services rather than models. Anyway, it was here before, so refactoring should be for another PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aha, yes you are probably right, but I will leave out that refactoring from this PR. I think we should have some design guidelines what type of code should be where. I think I've started to understand Gitea's code structure now....

log.Error("PrepareWebhooks [is_pull: %v, remove_assignee: %v]: %v", issue.IsPull, removed, err)
return nil
return false, nil, err
}
} else {
mode, _ := accessLevelUnit(sess, doer, issue.Repo, UnitTypeIssues)
Expand All @@ -210,65 +200,10 @@ func (issue *Issue) changeAssignee(sess *xorm.Session, doer *User, assigneeID in
}
if err := prepareWebhooks(sess, issue.Repo, HookEventIssues, apiIssue); err != nil {
log.Error("PrepareWebhooks [is_pull: %v, remove_assignee: %v]: %v", issue.IsPull, removed, err)
return nil
return false, nil, err
}
}
return nil
}

// UpdateAPIAssignee is a helper function to add or delete one or multiple issue assignee(s)
// Deleting is done the GitHub way (quote from their api documentation):
// https://developer.github.com/v3/issues/#edit-an-issue
// "assignees" (array): Logins for Users to assign to this issue.
// Pass one or more user logins to replace the set of assignees on this Issue.
// Send an empty array ([]) to clear all assignees from the Issue.
func UpdateAPIAssignee(issue *Issue, oneAssignee string, multipleAssignees []string, doer *User) (err error) {
var allNewAssignees []*User

// Keep the old assignee thingy for compatibility reasons
if oneAssignee != "" {
// Prevent double adding assignees
var isDouble bool
for _, assignee := range multipleAssignees {
if assignee == oneAssignee {
isDouble = true
break
}
}

if !isDouble {
multipleAssignees = append(multipleAssignees, oneAssignee)
}
}

// Loop through all assignees to add them
for _, assigneeName := range multipleAssignees {
assignee, err := GetUserByName(assigneeName)
if err != nil {
return err
}

allNewAssignees = append(allNewAssignees, assignee)
}

// Delete all old assignees not passed
if err = DeleteNotPassedAssignee(issue, doer, allNewAssignees); err != nil {
return err
}

// Add all new assignees
// Update the assignee. The function will check if the user exists, is already
// assigned (which he shouldn't as we deleted all assignees before) and
// has access to the repo.
for _, assignee := range allNewAssignees {
// Extra method to prevent double adding (which would result in removing)
err = AddAssigneeIfNotAssigned(issue, doer, assignee.ID)
if err != nil {
return err
}
}

return
return removed, comment, nil
}

// MakeIDsFromAPIAssigneesToAdd returns an array with all assignee IDs
Expand Down
2 changes: 1 addition & 1 deletion models/issue_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ func testInsertIssue(t *testing.T, title, content string) {
Title: title,
Content: content,
}
err := NewIssue(repo, &issue, nil, nil, nil)
err := NewIssue(repo, &issue, nil, nil)
assert.NoError(t, err)

var newIssue Issue
Expand Down
7 changes: 3 additions & 4 deletions models/pull.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -656,11 +656,11 @@ func (pr *PullRequest) testPatch(e Engine) (err error) {
}

// NewPullRequest creates new pull request with labels for repository.
func NewPullRequest(repo *Repository, pull *Issue, labelIDs []int64, uuids []string, pr *PullRequest, patch []byte, assigneeIDs []int64) (err error) {
func NewPullRequest(repo *Repository, pull *Issue, labelIDs []int64, uuids []string, pr *PullRequest, patch []byte) (err error) {
// Retry several times in case INSERT fails due to duplicate key for (repo_id, index); see #7887
i := 0
for {
if err = newPullRequestAttempt(repo, pull, labelIDs, uuids, pr, patch, assigneeIDs); err == nil {
if err = newPullRequestAttempt(repo, pull, labelIDs, uuids, pr, patch); err == nil {
return nil
}
if !IsErrNewIssueInsert(err) {
Expand All @@ -674,7 +674,7 @@ func NewPullRequest(repo *Repository, pull *Issue, labelIDs []int64, uuids []str
return fmt.Errorf("NewPullRequest: too many errors attempting to insert the new issue. Last error was: %v", err)
}

func newPullRequestAttempt(repo *Repository, pull *Issue, labelIDs []int64, uuids []string, pr *PullRequest, patch []byte, assigneeIDs []int64) (err error) {
func newPullRequestAttempt(repo *Repository, pull *Issue, labelIDs []int64, uuids []string, pr *PullRequest, patch []byte) (err error) {
sess := x.NewSession()
defer sess.Close()
if err = sess.Begin(); err != nil {
Expand All @@ -687,7 +687,6 @@ func newPullRequestAttempt(repo *Repository, pull *Issue, labelIDs []int64, uuid
LabelIDs: labelIDs,
Attachments: uuids,
IsPull: true,
AssigneeIDs: assigneeIDs,
}); err != nil {
if IsErrUserDoesNotHaveAccessToRepo(err) || IsErrNewIssueInsert(err) {
return err
Expand Down
19 changes: 16 additions & 3 deletions models/repo_permission.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -329,10 +329,23 @@ func HasAccessUnit(user *User, repo *Repository, unitType UnitType, testMode Acc
return hasAccessUnit(x, user, repo, unitType, testMode)
}

// canBeAssigned return true if user could be assigned to a repo
func canBeAssignedIssues(e Engine, user *User, repo *Repository) (bool, error) {
return hasAccessUnit(e, user, repo, UnitTypeIssues, AccessModeWrite)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this change is right. Who can be assigned for an issue? We should consider it carefully.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lunny What are your thoughts? I didn't think about it before, but perhaps an Issue can be assigned to someone from QA or customer service that not necessarily will need to create PRs (e.g just gather information). They will probably not even need access to the code.
A more important consideration is who can assign other people to an issue, as they need to have the authority to do so. I believe we're still not making that distinction .

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Private repositories: At least read access to issues (or pull requests) should be needed so you can at least see the issue/pr. You can have access to code but not read access to issues. I don't know if it makes sense to be able to assign someone which does not have write permission in the issue/pr (can you still make comments?)
Public repositories: Anyone has at least read access so the same applies.
For me it is OK to be less restrictive and allow assigning anyone with read access, but maybe it should be a separate PR. Should I change back to UnitTypeCode to keep existing behavior (just refactor now)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current code is very complex. It is already required that assignee has write access to issue / pr. It is checked by ValidateRepoMetas in routers/repo/issue.go. So I think this code shall be removed and use existing function instead.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps the criteria for "can be assigned" should be "write on this issue OR (read this issue AND create a PR)". Because a PR can close an issue by means of commit messages when it's merged; write access to the issue is not really required. In any case, if you're assigned you should have some way of closing it or comment on it for others to close.
Just for the sake of discussion, anyone assigned should be allowed to comment on it, even if they wouldn't be able to comment on other issues (but that's not for this PR).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the check of "is assignable" should match users shown in the UI? The UI list is currently only based on any write access to the repository.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree they should match, either way we want that criteria to be.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have relaxed the required for whom is assignable to anyone with write access to code, issues or pull requests. The UI only check for any write access to repo so this is to get as close as possible to that. (I know it is still not identical as I have not considered other units than these three).
I think we shall leave additional changes of assignable out of this PR, and continue discussing this in #4329.

}

// CanBeAssignedIssues return true if user can be assigned to issues in repo
func CanBeAssignedIssues(user *User, repo *Repository) (bool, error) {
return canBeAssignedIssues(x, user, repo)
}

// FIXME: user could send PullRequest also could be assigned???
func canBeAssigned(e Engine, user *User, repo *Repository) (bool, error) {
return hasAccessUnit(e, user, repo, UnitTypeCode, AccessModeWrite)
func canBeAssignedPullRequests(e Engine, user *User, repo *Repository) (bool, error) {
return hasAccessUnit(e, user, repo, UnitTypePullRequests, AccessModeWrite)
}

// CanBeAssignedPullRequests return true if user can be assigned to pull requests in repo
func CanBeAssignedPullRequests(user *User, repo *Repository) (bool, error) {
return canBeAssignedPullRequests(x, user, repo)
}

func hasAccess(e Engine, userID int64, repo *Repository) (bool, error) {
Expand Down
2 changes: 1 addition & 1 deletion modules/notification/base/notifier.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ type Notifier interface {
NotifyNewIssue(*models.Issue)
NotifyIssueChangeStatus(*models.User, *models.Issue, bool)
NotifyIssueChangeMilestone(doer *models.User, issue *models.Issue)
NotifyIssueChangeAssignee(doer *models.User, issue *models.Issue, removed bool)
NotifyIssueChangeAssignee(doer *models.User, issue *models.Issue, assignee *models.User, removed bool, comment *models.Comment)
NotifyIssueChangeContent(doer *models.User, issue *models.Issue, oldContent string)
NotifyIssueClearLabels(doer *models.User, issue *models.Issue)
NotifyIssueChangeTitle(doer *models.User, issue *models.Issue, oldTitle string)
Expand Down
2 changes: 1 addition & 1 deletion modules/notification/base/null.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ func (*NullNotifier) NotifyIssueChangeContent(doer *models.User, issue *models.I
}

// NotifyIssueChangeAssignee places a place holder function
func (*NullNotifier) NotifyIssueChangeAssignee(doer *models.User, issue *models.Issue, removed bool) {
func (*NullNotifier) NotifyIssueChangeAssignee(doer *models.User, issue *models.Issue, assignee *models.User, removed bool, comment *models.Comment) {
}

// NotifyIssueClearLabels places a place holder function
Expand Down
10 changes: 10 additions & 0 deletions modules/notification/mail/mail.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
package mail

import (
"fmt"

"code.gitea.io/gitea/models"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/notification/base"
Expand Down Expand Up @@ -88,3 +90,11 @@ func (m *mailNotifier) NotifyPullRequestReview(pr *models.PullRequest, r *models
log.Error("MailParticipants: %v", err)
}
}

func (m *mailNotifier) NotifyIssueChangeAssignee(doer *models.User, issue *models.Issue, assignee *models.User, removed bool, comment *models.Comment) {
// mail only sent to added assignees and not self-assignee
if !removed && doer.ID != assignee.ID && assignee.EmailNotifications() == models.EmailNotificationsEnabled {
ct := fmt.Sprintf("Assigned #%d.", issue.Index)
mailer.SendIssueAssignedMail(issue, doer, ct, comment, []string{assignee.Email})
}
}
Loading