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

diff --git a/models/fixtures/comment.yml b/models/fixtures/comment.yml
index e38952ac558dd972be772dad6d308867bc30897c..3292bb4848e857c268443b54fbab558dce9b1c18 100644 (file)
--- 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 c2d21089ce25c085da47cc26ab57924820524efb..7bbbab26febb8891c280eb50430dc7eb17510ed6 100644 (file)
--- 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 347598300e5fafd1dc3822e35407192539dad13d..8de6ea9cb99baa19bb42296e96fbb3cb1268bcdc 100644 (file)
--- 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 4b076606bf519c61bc4df6e75fc835179ca13aed..4b6542ddb4c158112f8ce42c2a366ae2867caf2b 100644 (file)
--- 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 7c80258c2b8d9ddaca1c31cc6e080463d44ba841..52724d07d4d3325dddc9820d2b18f51cc93f68b5 100644 (file)
--- 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})
+
+}