From d75a23dec3e8de61b689f70ad8e1ceb2fe8e945d Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Wed, 26 Feb 2020 11:51:08 +0100 Subject: [PATCH 1/3] inform participants on UI too --- models/issue.go | 33 ++++++++++++++++++++++++++------- models/notification.go | 8 ++++++++ modules/util/compare.go | 10 ++++++++++ routers/repo/issue.go | 2 +- 4 files changed, 45 insertions(+), 8 deletions(-) diff --git a/models/issue.go b/models/issue.go index a3c81b67023ba..62307eaf4977e 100644 --- a/models/issue.go +++ b/models/issue.go @@ -1277,13 +1277,24 @@ func GetParticipantsIDsByIssueID(issueID int64) ([]int64, error) { // GetParticipantsByIssueID returns all users who are participated in comments of an issue. func GetParticipantsByIssueID(issueID int64) ([]*User, error) { - return getParticipantsByIssueID(x, issueID) + issue, err := getIssueByID(x, issueID) + if err != nil { + return nil, err + } + userIDs, err := getParticipantsByIssueID(x, issue) + if err != nil { + return nil, err + } + if len(userIDs) == 0 { + return nil, nil + } + return GetUsersByIDs(userIDs) } -func getParticipantsByIssueID(e Engine, issueID int64) ([]*User, error) { +func getParticipantsByIssueID(e Engine, issue *Issue) ([]int64, error) { userIDs := make([]int64, 0, 5) if err := e.Table("comment").Cols("poster_id"). - Where("`comment`.issue_id = ?", issueID). + Where("`comment`.issue_id = ?", issue.ID). And("`comment`.type in (?,?,?)", CommentTypeComment, CommentTypeCode, CommentTypeReview). And("`user`.is_active = ?", true). And("`user`.prohibit_login = ?", false). @@ -1292,12 +1303,20 @@ func getParticipantsByIssueID(e Engine, issueID int64) ([]*User, error) { Find(&userIDs); err != nil { return nil, fmt.Errorf("get poster IDs: %v", err) } - if len(userIDs) == 0 { - return nil, nil + if !util.IsInt64InSlice(issue.PosterID, userIDs) { + return append(userIDs, issue.PosterID), nil } + return userIDs, nil +} - users := make([]*User, 0, len(userIDs)) - return users, e.In("id", userIDs).Find(&users) +// IsUserParticipantsOfIssue return true if user is participants of an issue +func IsUserParticipantsOfIssue(user *User, issue *Issue) bool { + userIDs, err := getParticipantsByIssueID(x, issue) + if err != nil { + log.Error(err.Error()) + return false + } + return util.IsInt64InSlice(user.ID, userIDs) } // UpdateIssueMentions updates issue-user relations for mentioned users. diff --git a/models/notification.go b/models/notification.go index c52d6c557a5d6..1a9cbb3d201ba 100644 --- a/models/notification.go +++ b/models/notification.go @@ -160,6 +160,14 @@ func createOrUpdateIssueNotifications(e Engine, issueID, commentID int64, notifi toNotify[id] = struct{}{} } + issueParticipants, err := getParticipantsByIssueID(e, issue) + if err != nil { + return err + } + for _, id := range issueParticipants { + toNotify[id] = struct{}{} + } + // dont notify user who cause notification delete(toNotify, notificationAuthorID) // explicit unwatch on issue diff --git a/modules/util/compare.go b/modules/util/compare.go index f1d1e5718e462..d4f77aed3ba08 100644 --- a/modules/util/compare.go +++ b/modules/util/compare.go @@ -45,6 +45,16 @@ func IsStringInSlice(target string, slice []string) bool { return false } +// IsInt64InSlice sequential searches if int64 exists in slice. +func IsInt64InSlice(target int64, slice []int64) bool { + for i := 0; i < len(slice); i++ { + if slice[i] == target { + return true + } + } + return false +} + // IsEqualSlice returns true if slices are equal. func IsEqualSlice(target []string, source []string) bool { if len(target) != len(source) { diff --git a/routers/repo/issue.go b/routers/repo/issue.go index fdade2795d164..ca7be7bd961a8 100644 --- a/routers/repo/issue.go +++ b/routers/repo/issue.go @@ -704,7 +704,7 @@ func ViewIssue(ctx *context.Context) { iw = &models.IssueWatch{ UserID: ctx.User.ID, IssueID: issue.ID, - IsWatching: models.IsWatching(ctx.User.ID, ctx.Repo.Repository.ID), + IsWatching: models.IsWatching(ctx.User.ID, ctx.Repo.Repository.ID) || models.IsUserParticipantsOfIssue(ctx.User, issue), } } } From b3fae7a3e62e4f07a19e7120a8f20854d881ba02 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Thu, 27 Feb 2020 06:24:50 +0100 Subject: [PATCH 2/3] ajust test --- models/issue_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/issue_test.go b/models/issue_test.go index 681ef8441ac16..81afe3d88d8d1 100644 --- a/models/issue_test.go +++ b/models/issue_test.go @@ -81,7 +81,7 @@ func TestGetParticipantsByIssueID(t *testing.T) { // User 2 only labeled issue1 (see fixtures/comment.yml) // Users 3 and 5 made actual comments (see fixtures/comment.yml) // User 3 is inactive, thus not active participant - checkParticipants(1, []int{5}) + checkParticipants(1, []int{1, 5}) } func TestIssue_ClearLabels(t *testing.T) { From f95baa0dd29c61082bf5663685727955beaaf158 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Thu, 27 Feb 2020 19:42:58 +0100 Subject: [PATCH 3/3] refactor getParticipantIDsByIssue --- models/issue.go | 58 +++++++++++++++++------------------------- models/issue_test.go | 10 +++++--- models/notification.go | 3 +-- 3 files changed, 30 insertions(+), 41 deletions(-) diff --git a/models/issue.go b/models/issue.go index 62307eaf4977e..d356682f01597 100644 --- a/models/issue.go +++ b/models/issue.go @@ -1275,43 +1275,9 @@ func GetParticipantsIDsByIssueID(issueID int64) ([]int64, error) { Find(&userIDs) } -// GetParticipantsByIssueID returns all users who are participated in comments of an issue. -func GetParticipantsByIssueID(issueID int64) ([]*User, error) { - issue, err := getIssueByID(x, issueID) - if err != nil { - return nil, err - } - userIDs, err := getParticipantsByIssueID(x, issue) - if err != nil { - return nil, err - } - if len(userIDs) == 0 { - return nil, nil - } - return GetUsersByIDs(userIDs) -} - -func getParticipantsByIssueID(e Engine, issue *Issue) ([]int64, error) { - userIDs := make([]int64, 0, 5) - if err := e.Table("comment").Cols("poster_id"). - Where("`comment`.issue_id = ?", issue.ID). - And("`comment`.type in (?,?,?)", CommentTypeComment, CommentTypeCode, CommentTypeReview). - And("`user`.is_active = ?", true). - And("`user`.prohibit_login = ?", false). - Join("INNER", "`user`", "`user`.id = `comment`.poster_id"). - Distinct("poster_id"). - Find(&userIDs); err != nil { - return nil, fmt.Errorf("get poster IDs: %v", err) - } - if !util.IsInt64InSlice(issue.PosterID, userIDs) { - return append(userIDs, issue.PosterID), nil - } - return userIDs, nil -} - // IsUserParticipantsOfIssue return true if user is participants of an issue func IsUserParticipantsOfIssue(user *User, issue *Issue) bool { - userIDs, err := getParticipantsByIssueID(x, issue) + userIDs, err := issue.getParticipantIDsByIssue(x) if err != nil { log.Error(err.Error()) return false @@ -1710,6 +1676,28 @@ type DependencyInfo struct { Repository `xorm:"extends"` } +// getParticipantIDsByIssue returns all userIDs who are participated in comments of an issue and issue author +func (issue *Issue) getParticipantIDsByIssue(e Engine) ([]int64, error) { + if issue == nil { + return nil, nil + } + userIDs := make([]int64, 0, 5) + if err := e.Table("comment").Cols("poster_id"). + Where("`comment`.issue_id = ?", issue.ID). + And("`comment`.type in (?,?,?)", CommentTypeComment, CommentTypeCode, CommentTypeReview). + And("`user`.is_active = ?", true). + And("`user`.prohibit_login = ?", false). + Join("INNER", "`user`", "`user`.id = `comment`.poster_id"). + Distinct("poster_id"). + Find(&userIDs); err != nil { + return nil, fmt.Errorf("get poster IDs: %v", err) + } + if !util.IsInt64InSlice(issue.PosterID, userIDs) { + return append(userIDs, issue.PosterID), nil + } + return userIDs, nil +} + // Get Blocked By Dependencies, aka all issues this issue is blocked by. func (issue *Issue) getBlockedByDependencies(e Engine) (issueDeps []*DependencyInfo, err error) { return issueDeps, e. diff --git a/models/issue_test.go b/models/issue_test.go index 81afe3d88d8d1..7ba9a396b2386 100644 --- a/models/issue_test.go +++ b/models/issue_test.go @@ -61,15 +61,17 @@ func TestGetIssuesByIDs(t *testing.T) { testSuccess([]int64{1, 2, 3}, []int64{NonexistentID}) } -func TestGetParticipantsByIssueID(t *testing.T) { +func TestGetParticipantIDsByIssue(t *testing.T) { assert.NoError(t, PrepareTestDatabase()) checkParticipants := func(issueID int64, userIDs []int) { - participants, err := GetParticipantsByIssueID(issueID) + issue, err := GetIssueByID(issueID) + assert.NoError(t, err) + participants, err := issue.getParticipantIDsByIssue(x) if assert.NoError(t, err) { participantsIDs := make([]int, len(participants)) - for i, u := range participants { - participantsIDs[i] = int(u.ID) + for i, uid := range participants { + participantsIDs[i] = int(uid) } sort.Ints(participantsIDs) sort.Ints(userIDs) diff --git a/models/notification.go b/models/notification.go index 1a9cbb3d201ba..d2958630ae0b9 100644 --- a/models/notification.go +++ b/models/notification.go @@ -159,8 +159,7 @@ func createOrUpdateIssueNotifications(e Engine, issueID, commentID int64, notifi for _, id := range repoWatches { toNotify[id] = struct{}{} } - - issueParticipants, err := getParticipantsByIssueID(e, issue) + issueParticipants, err := issue.getParticipantIDsByIssue(e) if err != nil { return err }