diff --git a/models/issues/TestGetParticipantIDsByIssue/comment.yml b/models/issues/TestGetParticipantIDsByIssue/comment.yml new file mode 100644 index 0000000000..7ab1cf88a6 --- /dev/null +++ b/models/issues/TestGetParticipantIDsByIssue/comment.yml @@ -0,0 +1,10 @@ +- id: 1001 + type: 21 # code comment + poster_id: 10 + issue_id: 1 + review_id: 1001 + content: "Some code comment that is pending to be published" + line: -4 + tree_path: "README.md" + created_unix: 946684812 + invalidated: false diff --git a/models/issues/TestGetParticipantIDsByIssue/review.yml b/models/issues/TestGetParticipantIDsByIssue/review.yml new file mode 100644 index 0000000000..2b6fe5706e --- /dev/null +++ b/models/issues/TestGetParticipantIDsByIssue/review.yml @@ -0,0 +1,7 @@ +- id: 1001 + type: 0 # Pending review + reviewer_id: 10 + issue_id: 1 + content: "Pending review for issue 1" + updated_unix: 946684810 + created_unix: 946684810 diff --git a/models/issues/comment_list.go b/models/issues/comment_list.go index 9b502d1c91..3996dcb29a 100644 --- a/models/issues/comment_list.go +++ b/models/issues/comment_list.go @@ -417,7 +417,7 @@ func (comments CommentList) getReviewIDs() []int64 { }) } -func (comments CommentList) loadReviews(ctx context.Context) error { +func (comments CommentList) LoadReviews(ctx context.Context) error { if len(comments) == 0 { return nil } @@ -476,7 +476,7 @@ func (comments CommentList) LoadAttributes(ctx context.Context) (err error) { return err } - if err = comments.loadReviews(ctx); err != nil { + if err = comments.LoadReviews(ctx); err != nil { return err } diff --git a/models/issues/issue.go b/models/issues/issue.go index fc8794ad50..a90686eb50 100644 --- a/models/issues/issue.go +++ b/models/issues/issue.go @@ -591,10 +591,12 @@ func GetParticipantsIDsByIssueID(ctx context.Context, issueID int64) ([]int64, e userIDs := make([]int64, 0, 5) return userIDs, db.GetEngine(ctx). Table("comment"). - Cols("poster_id"). - Where("issue_id = ?", issueID). - And("type in (?,?,?)", CommentTypeComment, CommentTypeCode, CommentTypeReview). - Distinct("poster_id"). + Cols("`comment`.poster_id"). + Where("`comment`.issue_id = ?", issueID). + And("`comment`.type in (?,?,?)", CommentTypeComment, CommentTypeCode, CommentTypeReview). + And("`review`.type is null or `review`.type != ?", ReviewTypePending). + Join("LEFT", "`review`", "`review`.id = `comment`.review_id"). + Distinct("`comment`.poster_id"). Find(&userIDs) } @@ -623,9 +625,11 @@ func (issue *Issue) GetParticipantIDsByIssue(ctx context.Context) ([]int64, erro if err := db.GetEngine(ctx).Table("comment").Cols("poster_id"). Where("`comment`.issue_id = ?", issue.ID). And("`comment`.type in (?,?,?)", CommentTypeComment, CommentTypeCode, CommentTypeReview). + And("`review`.type != ?", ReviewTypePending). And("`user`.is_active = ?", true). And("`user`.prohibit_login = ?", false). Join("INNER", "`user`", "`user`.id = `comment`.poster_id"). + Join("INNER", "`review`", "`review`.reviewer_id = `user`.id"). Distinct("poster_id"). Find(&userIDs); err != nil { return nil, fmt.Errorf("get poster IDs: %w", err) diff --git a/models/issues/issue_test.go b/models/issues/issue_test.go index 2059c5013a..e9617548e9 100644 --- a/models/issues/issue_test.go +++ b/models/issues/issue_test.go @@ -85,6 +85,7 @@ func TestGetIssuesByIDs(t *testing.T) { } func TestGetParticipantIDsByIssue(t *testing.T) { + defer unittest.OverrideFixtures("models/issues/TestGetParticipantIDsByIssue")() require.NoError(t, unittest.PrepareTestDatabase()) checkParticipants := func(issueID int64, userIDs []int) { @@ -107,6 +108,7 @@ func TestGetParticipantIDsByIssue(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 + // User 10 has a pending review, thus not an active participant, yet (see TestGetParticipantIDsByIssue/comment.yml) checkParticipants(1, []int{1, 5}) } diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index d46ea7f133..9df340378d 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -1654,17 +1654,27 @@ func ViewIssue(ctx *context.Context) { ctx.ServerError("LoadAttachmentsByIssue", err) return } + if err := issue.Comments.LoadPosters(ctx); err != nil { ctx.ServerError("LoadPosters", err) return } + if err := issue.Comments.LoadReviews(ctx); err != nil { + ctx.ServerError("LoadReviews", err) + return + } + for commentIdx, comment = range issue.Comments { comment.Issue = issue metas := ctx.Repo.Repository.ComposeMetas(ctx) metas["scope"] = fmt.Sprintf("comment-%d", commentIdx) - if comment.Type == issues_model.CommentTypeComment || comment.Type == issues_model.CommentTypeReview { + if comment.Review != nil && comment.Review.Type == issues_model.ReviewTypePending { + continue + } + + if comment.Type == issues_model.CommentTypeComment { comment.RenderedContent, err = markdown.RenderString(&markup.RenderContext{ Links: markup.Links{ Base: ctx.Repo.RepoLink, diff --git a/services/mailer/fixtures/TestCloseIssue/comment.yml b/services/mailer/fixtures/TestCloseIssue/comment.yml new file mode 100644 index 0000000000..7ab1cf88a6 --- /dev/null +++ b/services/mailer/fixtures/TestCloseIssue/comment.yml @@ -0,0 +1,10 @@ +- id: 1001 + type: 21 # code comment + poster_id: 10 + issue_id: 1 + review_id: 1001 + content: "Some code comment that is pending to be published" + line: -4 + tree_path: "README.md" + created_unix: 946684812 + invalidated: false diff --git a/services/mailer/fixtures/TestCloseIssue/review.yml b/services/mailer/fixtures/TestCloseIssue/review.yml new file mode 100644 index 0000000000..2b6fe5706e --- /dev/null +++ b/services/mailer/fixtures/TestCloseIssue/review.yml @@ -0,0 +1,7 @@ +- id: 1001 + type: 0 # Pending review + reviewer_id: 10 + issue_id: 1 + content: "Pending review for issue 1" + updated_unix: 946684810 + created_unix: 946684810 diff --git a/services/mailer/mail_issue_test.go b/services/mailer/mail_issue_test.go index ad9e0aa525..3ed83fd03a 100644 --- a/services/mailer/mail_issue_test.go +++ b/services/mailer/mail_issue_test.go @@ -18,6 +18,7 @@ import ( ) func TestCloseIssue(t *testing.T) { + defer unittest.OverrideFixtures("services/mailer/fixtures/TestCloseIssue")() defer require.NoError(t, unittest.PrepareTestDatabase()) called := false diff --git a/tests/integration/fixtures/TestPullRequestParticipants/comment.yml b/tests/integration/fixtures/TestPullRequestParticipants/comment.yml new file mode 100644 index 0000000000..85f915a2f3 --- /dev/null +++ b/tests/integration/fixtures/TestPullRequestParticipants/comment.yml @@ -0,0 +1,10 @@ +- id: 1001 + type: 21 # code comment + poster_id: 10 + issue_id: 2 + review_id: 1001 + content: "Some code comment that is pending to be published" + line: -4 + tree_path: "README.md" + created_unix: 946684812 + invalidated: false diff --git a/tests/integration/fixtures/TestPullRequestParticipants/review.yml b/tests/integration/fixtures/TestPullRequestParticipants/review.yml new file mode 100644 index 0000000000..d75122799d --- /dev/null +++ b/tests/integration/fixtures/TestPullRequestParticipants/review.yml @@ -0,0 +1,7 @@ +- id: 1001 + type: 0 # Pending review + reviewer_id: 10 + issue_id: 2 + content: "Pending review for issue 2" + updated_unix: 946684810 + created_unix: 946684810 diff --git a/tests/integration/pull_review_test.go b/tests/integration/pull_review_test.go index cd0c75eff2..69628e08b5 100644 --- a/tests/integration/pull_review_test.go +++ b/tests/integration/pull_review_test.go @@ -64,6 +64,20 @@ func TestPullView_ReviewerMissed(t *testing.T) { assert.True(t, test.IsNormalPageCompleted(resp.Body.String())) } +func TestPullRequestParticipants(t *testing.T) { + defer unittest.OverrideFixtures("tests/integration/fixtures/TestPullRequestParticipants")() + defer tests.PrepareTestEnv(t)() + session := loginUser(t, "user1") + + req := NewRequest(t, "GET", "/user2/repo1/pulls/2") + resp := session.MakeRequest(t, req, http.StatusOK) + assert.Contains(t, resp.Body.String(), "2 participants") + assert.Contains(t, resp.Body.String(), ``) + assert.Contains(t, resp.Body.String(), ``) + // does not contain user10 which has a pending review for this issue + assert.NotContains(t, resp.Body.String(), ``) +} + func loadComment(t *testing.T, commentID string) *issues_model.Comment { t.Helper() id, err := strconv.ParseInt(commentID, 10, 64)