From 447c9b428f4cf50174ef7f3fbea56b5405f6bbf8 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Thu, 16 Mar 2017 02:34:24 +0100 Subject: [PATCH] Send notifications to partecipants in issue comments (#1217) * Send notifications to partecipants in issue comments Closes #1216 Includes test (still failing) * Do not include "labelers" to participants Fix test to expect what GetParticipants return --- models/fixtures/comment.yml | 14 +++++++++++++- models/fixtures/issue.yml | 2 +- models/issue.go | 18 ++++++++++++++++++ models/issue_mail.go | 26 ++++++++++++++++++++++++-- models/issue_test.go | 24 ++++++++++++++++++++++++ 5 files changed, 80 insertions(+), 4 deletions(-) diff --git a/models/fixtures/comment.yml b/models/fixtures/comment.yml index e38952ac5..3292bb484 100644 --- a/models/fixtures/comment.yml +++ b/models/fixtures/comment.yml @@ -2,6 +2,18 @@ id: 1 type: 7 # label poster_id: 2 - issue_id: 1 + issue_id: 1 # in repo_id 1 label_id: 1 content: "1" +- + id: 2 + type: 0 # comment + poster_id: 3 # user not watching (see watch.yml) + issue_id: 1 # in repo_id 1 + content: "good work!" +- + id: 3 + type: 0 # comment + poster_id: 5 # user not watching (see watch.yml) + issue_id: 1 # in repo_id 1 + content: "meh..." diff --git a/models/fixtures/issue.yml b/models/fixtures/issue.yml index c2d21089c..7bbbab26f 100644 --- a/models/fixtures/issue.yml +++ b/models/fixtures/issue.yml @@ -8,7 +8,7 @@ content: content1 is_closed: false is_pull: false - num_comments: 0 + num_comments: 2 created_unix: 946684800 updated_unix: 978307200 diff --git a/models/issue.go b/models/issue.go index 347598300..8de6ea9cb 100644 --- a/models/issue.go +++ b/models/issue.go @@ -1134,6 +1134,24 @@ func Issues(opts *IssuesOptions) ([]*Issue, error) { return issues, nil } +// GetParticipantsByIssueID returns all users who are participated in comments of an issue. +func GetParticipantsByIssueID(issueID int64) ([]*User, error) { + userIDs := make([]int64, 0, 5) + if err := x.Table("comment").Cols("poster_id"). + Where("issue_id = ?", issueID). + And("type = ?", CommentTypeComment). + Distinct("poster_id"). + Find(&userIDs); err != nil { + return nil, fmt.Errorf("get poster IDs: %v", err) + } + if len(userIDs) == 0 { + return nil, nil + } + + users := make([]*User, 0, len(userIDs)) + return users, x.In("id", userIDs).Find(&users) +} + // UpdateIssueMentions extracts mentioned people from content and // updates issue-user relations for them. func UpdateIssueMentions(e Engine, issueID int64, mentions []string) error { diff --git a/models/issue_mail.go b/models/issue_mail.go index 4b076606b..4b6542ddb 100644 --- a/models/issue_mail.go +++ b/models/issue_mail.go @@ -19,15 +19,27 @@ func (issue *Issue) mailSubject() string { } // mailIssueCommentToParticipants can be used for both new issue creation and comment. +// This function sends two list of emails: +// 1. Repository watchers and users who are participated in comments. +// 2. Users who are not in 1. but get mentioned in current issue/comment. func mailIssueCommentToParticipants(issue *Issue, doer *User, mentions []string) error { if !setting.Service.EnableNotifyMail { return nil } - // Mail watchers. watchers, err := GetWatchers(issue.RepoID) if err != nil { - return fmt.Errorf("GetWatchers [%d]: %v", issue.RepoID, err) + return fmt.Errorf("GetWatchers [repo_id: %d]: %v", issue.RepoID, err) + } + participants, err := GetParticipantsByIssueID(issue.ID) + if err != nil { + return fmt.Errorf("GetParticipantsByIssueID [issue_id: %d]: %v", issue.ID, err) + } + + // In case the issue poster is not watching the repository, + // even if we have duplicated in watchers, can be safely filtered out. + if issue.PosterID != doer.ID { + participants = append(participants, issue.Poster) } tos := make([]string, 0, len(watchers)) // List of email addresses. @@ -48,6 +60,16 @@ func mailIssueCommentToParticipants(issue *Issue, doer *User, mentions []string) tos = append(tos, to.Email) names = append(names, to.Name) } + for i := range participants { + if participants[i].ID == doer.ID { + continue + } else if com.IsSliceContainsStr(names, participants[i].Name) { + continue + } + + tos = append(tos, participants[i].Email) + names = append(names, participants[i].Name) + } SendIssueCommentMail(issue, doer, tos) // Mail mentioned people and exclude watchers. diff --git a/models/issue_test.go b/models/issue_test.go index 7c80258c2..52724d07d 100644 --- a/models/issue_test.go +++ b/models/issue_test.go @@ -5,6 +5,7 @@ package models import ( + "sort" "testing" "github.com/stretchr/testify/assert" @@ -58,3 +59,26 @@ func TestGetIssuesByIDs(t *testing.T) { testSuccess([]int64{1, 2, 3}, []int64{}) testSuccess([]int64{1, 2, 3}, []int64{NonexistentID}) } + +func TestGetParticipantsByIssueID(t *testing.T) { + + assert.NoError(t, PrepareTestDatabase()) + + checkPartecipants := func(issueID int64, userIDs []int) { + partecipants, err := GetParticipantsByIssueID(issueID) + if assert.NoError(t, err) { + partecipantsIDs := make([]int,len(partecipants)) + for i,u := range partecipants { partecipantsIDs[i] = int(u.ID) } + sort.Ints(partecipantsIDs) + sort.Ints(userIDs) + assert.Equal(t, userIDs, partecipantsIDs) + } + + } + + // User 1 is issue1 poster (see fixtures/issue.yml) + // User 2 only labeled issue1 (see fixtures/comment.yml) + // Users 3 and 5 made actual comments (see fixtures/comment.yml) + checkPartecipants(1, []int{3,5}) + +}