Skip to content

Commit 7770335

Browse files
committed
Send notifications to partecipants in issue comments
Closes #1216 Includes test (still failing)
1 parent 09fe4a2 commit 7770335

File tree

5 files changed

+80
-4
lines changed

5 files changed

+80
-4
lines changed

models/fixtures/comment.yml

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,18 @@
22
id: 1
33
type: 7 # label
44
poster_id: 2
5-
issue_id: 1
5+
issue_id: 1 # in repo_id 1
66
label_id: 1
77
content: "1"
8+
-
9+
id: 2
10+
type: 0 # comment
11+
poster_id: 3 # user not watching (see watch.yml)
12+
issue_id: 1 # in repo_id 1
13+
content: "good work!"
14+
-
15+
id: 3
16+
type: 0 # comment
17+
poster_id: 5 # user not watching (see watch.yml)
18+
issue_id: 1 # in repo_id 1
19+
content: "meh..."

models/fixtures/issue.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
content: content1
99
is_closed: false
1010
is_pull: false
11-
num_comments: 0
11+
num_comments: 2
1212
created_unix: 946684800
1313
updated_unix: 978307200
1414

models/issue.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1134,6 +1134,23 @@ func Issues(opts *IssuesOptions) ([]*Issue, error) {
11341134
return issues, nil
11351135
}
11361136

1137+
// GetParticipantsByIssueID returns all users who are participated in comments of an issue.
1138+
func GetParticipantsByIssueID(issueID int64) ([]*User, error) {
1139+
userIDs := make([]int64, 0, 5)
1140+
if err := x.Table("comment").Cols("poster_id").
1141+
Where("issue_id = ?", issueID).
1142+
Distinct("poster_id").
1143+
Find(&userIDs); err != nil {
1144+
return nil, fmt.Errorf("get poster IDs: %v", err)
1145+
}
1146+
if len(userIDs) == 0 {
1147+
return nil, nil
1148+
}
1149+
1150+
users := make([]*User, 0, len(userIDs))
1151+
return users, x.In("id", userIDs).Find(&users)
1152+
}
1153+
11371154
// UpdateIssueMentions extracts mentioned people from content and
11381155
// updates issue-user relations for them.
11391156
func UpdateIssueMentions(e Engine, issueID int64, mentions []string) error {

models/issue_mail.go

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,27 @@ func (issue *Issue) mailSubject() string {
1919
}
2020

2121
// mailIssueCommentToParticipants can be used for both new issue creation and comment.
22+
// This function sends two list of emails:
23+
// 1. Repository watchers and users who are participated in comments.
24+
// 2. Users who are not in 1. but get mentioned in current issue/comment.
2225
func mailIssueCommentToParticipants(issue *Issue, doer *User, mentions []string) error {
2326
if !setting.Service.EnableNotifyMail {
2427
return nil
2528
}
2629

27-
// Mail watchers.
2830
watchers, err := GetWatchers(issue.RepoID)
2931
if err != nil {
30-
return fmt.Errorf("GetWatchers [%d]: %v", issue.RepoID, err)
32+
return fmt.Errorf("GetWatchers [repo_id: %d]: %v", issue.RepoID, err)
33+
}
34+
participants, err := GetParticipantsByIssueID(issue.ID)
35+
if err != nil {
36+
return fmt.Errorf("GetParticipantsByIssueID [issue_id: %d]: %v", issue.ID, err)
37+
}
38+
39+
// In case the issue poster is not watching the repository,
40+
// even if we have duplicated in watchers, can be safely filtered out.
41+
if issue.PosterID != doer.ID {
42+
participants = append(participants, issue.Poster)
3143
}
3244

3345
tos := make([]string, 0, len(watchers)) // List of email addresses.
@@ -48,6 +60,16 @@ func mailIssueCommentToParticipants(issue *Issue, doer *User, mentions []string)
4860
tos = append(tos, to.Email)
4961
names = append(names, to.Name)
5062
}
63+
for i := range participants {
64+
if participants[i].ID == doer.ID {
65+
continue
66+
} else if com.IsSliceContainsStr(names, participants[i].Name) {
67+
continue
68+
}
69+
70+
tos = append(tos, participants[i].Email)
71+
names = append(names, participants[i].Name)
72+
}
5173
SendIssueCommentMail(issue, doer, tos)
5274

5375
// Mail mentioned people and exclude watchers.

models/issue_test.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
package models
66

77
import (
8+
"fmt"
89
"testing"
910

1011
"github.com/stretchr/testify/assert"
@@ -58,3 +59,27 @@ func TestGetIssuesByIDs(t *testing.T) {
5859
testSuccess([]int64{1, 2, 3}, []int64{})
5960
testSuccess([]int64{1, 2, 3}, []int64{NonexistentID})
6061
}
62+
63+
func TestGetParticipantsByIssueID(t *testing.T) {
64+
65+
assert.NoError(t, PrepareTestDatabase())
66+
67+
checkPartecipants := func(issueID int64, userIDs []int64) {
68+
people, err := GetParticipantsByIssueID(issueID)
69+
if assert.NoError(t, err) {
70+
peopleIDs := make([]int64,len(people))
71+
for i,u := range people { peopleIDs[i] = u.ID }
72+
73+
for _, userID := range userIDs {
74+
user := AssertExistsAndLoadBean(t, &User{ID: userID}).(*User)
75+
assert.Contains(t, people, user, fmt.Sprintf("User %d is not found among participants for issue %d: %v", userID, issueID, peopleIDs))
76+
}
77+
}
78+
79+
}
80+
81+
// User 1 is issue1 poster (see fixtures/issue.yml)
82+
// User 2 only labeled issue1 (see fixtures/comment.yml)
83+
checkPartecipants(1, []int64{1,2})
84+
85+
}

0 commit comments

Comments
 (0)