Fix review request number and add more tests (#27104)

fix #27019
## testfixture yml
1. add issue20(a pr issue) in repo 23, org 17
2. add user15 to team 9
3. add four reviews about issue20
## test case
add two tests that are described with code comments
the code before pr #26784 failed the first test
<img width="479" alt="image"
src="https://github.com/go-gitea/gitea/assets/70063547/1d9b5787-11b4-4c4d-931f-6a9869547f35">
current code failed the second test(as mentioned in #27019)
<img width="484" alt="image"
src="https://github.com/go-gitea/gitea/assets/70063547/05608055-7587-43d1-bae1-92c688270819">
Any advice is appreciated.

---------

Co-authored-by: CaiCandong <50507092+CaiCandong@users.noreply.github.com>
Co-authored-by: Giteabot <teabot@gitea.io>

diff --git a/models/fixtures/issue.yml b/models/fixtures/issue.yml
index fa72f9b647da09a571074db7166636a5cfbfc511..ccc1fe41fbd9761715c9599b3c15b050060ae77f 100644 (file)
--- a/models/fixtures/issue.yml
+++ b/models/fixtures/issue.yml
@@ -321,3 +321,20 @@
   created_unix: 946684830
   updated_unix: 978307200
   is_locked: false
+
+-
+  id: 20
+  repo_id: 23
+  index: 1
+  poster_id: 2
+  original_author_id: 0
+  name: issue for pr
+  content: content
+  milestone_id: 0
+  priority: 0
+  is_closed: false
+  is_pull: true
+  num_comments: 0
+  created_unix: 978307210
+  updated_unix: 978307210
+  is_locked: false

diff --git a/models/fixtures/pull_request.yml b/models/fixtures/pull_request.yml
index e5589ac703d7794c1fb4676634273b18444746e4..396bdba88cbc512b211f340d06667994db9776f1 100644 (file)
--- a/models/fixtures/pull_request.yml
+++ b/models/fixtures/pull_request.yml
@@ -89,3 +89,12 @@
   base_branch: main
   merge_base: cbff181af4c9c7fee3cf6c106699e07d9a3f54e6
   has_merged: false
+
+-
+  id: 8
+  type: 0 # gitea pull request
+  status: 2 # mergable
+  issue_id: 20
+  index: 1
+  head_repo_id: 23
+  base_repo_id: 23
\ No newline at end of file

diff --git a/models/fixtures/repository.yml b/models/fixtures/repository.yml
index c63b7ebd481702595ea62b2dc5c620b44edd2a4f..373c1caa6257af0911d2804d5fc915ca81a5e7a1 100644 (file)
--- a/models/fixtures/repository.yml
+++ b/models/fixtures/repository.yml
@@ -679,7 +679,7 @@
   num_forks: 0
   num_issues: 0
   num_closed_issues: 0
-  num_pulls: 0
+  num_pulls: 1
   num_closed_pulls: 0
   num_milestones: 0
   num_closed_milestones: 0

diff --git a/models/fixtures/review.yml b/models/fixtures/review.yml
index dda13dc468e47f3bf2aa91335a2ba272ce1e3914..f964c6ac06412896813ed0de0567f3ef1446efcb 100644 (file)
--- a/models/fixtures/review.yml
+++ b/models/fixtures/review.yml
@@ -132,3 +132,41 @@
   content: "singular review from org6 and final review for this pr"
   updated_unix: 946684831
   created_unix: 946684831
+
+- 
+  id: 16
+  type: 4
+  reviewer_id: 20
+  issue_id: 20
+  content: "review request for user20"
+  updated_unix: 946684832
+  created_unix: 946684832
+
+- 
+  id: 17
+  type: 1
+  reviewer_id: 20
+  issue_id: 20
+  content: "review approved by user20"
+  updated_unix: 946684833
+  created_unix: 946684833
+- 
+  id: 18
+  type: 4
+  reviewer_id: 0
+  reviewer_team_id: 5 
+  issue_id: 20
+  content: "review request for team5"
+  updated_unix: 946684834
+  created_unix: 946684834
+
+-
+  id: 19
+  type: 4
+  reviewer_id: 15
+  reviewer_team_id: 0
+  issue_id: 20
+  content: "review request for user15"
+  updated_unix: 946684835
+  created_unix: 946684835
+

diff --git a/models/fixtures/team.yml b/models/fixtures/team.yml
index 65326eedbf47634e621177544831768ada01c1fc..295e51e39ce945bbe972c1c66ad74812c863f003 100644 (file)
--- a/models/fixtures/team.yml
+++ b/models/fixtures/team.yml
@@ -93,7 +93,7 @@
   name: review_team
   authorize: 1 # read
   num_repos: 1
-  num_members: 2
+  num_members: 3
   includes_all_repositories: false
   can_create_org_repo: false
 

diff --git a/models/fixtures/team_user.yml b/models/fixtures/team_user.yml
index feace5f2a531d04ce1c986d96fc5aaf97e26cf3a..a5f1e9fd92aaf56b637daa02dabf6a8d07b6f1ed 100644 (file)
--- a/models/fixtures/team_user.yml
+++ b/models/fixtures/team_user.yml
@@ -123,3 +123,9 @@
   org_id: 36
   team_id: 20
   uid: 5
+
+- 
+  id: 22
+  org_id: 17
+  team_id: 9
+  uid: 15

diff --git a/models/issues/issue_search.go b/models/issues/issue_search.go
index 5d40b447042c3fe2cda42d3df79fa38c1b67896a..5c05ead6879e737d967fdd044abab84bc11ca404 100644 (file)
--- a/models/issues/issue_search.go
+++ b/models/issues/issue_search.go
@@ -362,14 +362,21 @@ func applyReviewRequestedCondition(sess *xorm.Session, reviewRequestedID int64)
                From("team_user").
                Where(builder.Eq{"team_user.uid": reviewRequestedID})
 
+       // if the review is approved or rejected, it should not be shown in the review requested list
+       maxReview := builder.Select("MAX(r.id)").
+               From("review as r").
+               Where(builder.In("r.type", []ReviewType{ReviewTypeApprove, ReviewTypeReject, ReviewTypeRequest})).
+               GroupBy("r.issue_id, r.reviewer_id, r.reviewer_team_id")
+
        subQuery := builder.Select("review.issue_id").
                From("review").
                Where(builder.And(
-                       builder.In("review.type", []ReviewType{ReviewTypeRequest, ReviewTypeReject, ReviewTypeApprove}),
+                       builder.Eq{"review.type": ReviewTypeRequest},
                        builder.Or(
                                builder.Eq{"review.reviewer_id": reviewRequestedID},
                                builder.In("review.reviewer_team_id", existInTeamQuery),
                        ),
+                       builder.In("review.id", maxReview),
                ))
        return sess.Where("issue.poster_id <> ?", reviewRequestedID).
                And(builder.In("issue.id", subQuery))

diff --git a/models/issues/issue_test.go b/models/issues/issue_test.go
index b7fa7eff1c1509494c11a21dcb31e7a7d8a04ab6..513ae241bc9cc9b4deb7d46dc61bce1693187413 100644 (file)
--- a/models/issues/issue_test.go
+++ b/models/issues/issue_test.go
@@ -403,7 +403,7 @@ func TestCountIssues(t *testing.T) {
        assert.NoError(t, unittest.PrepareTestDatabase())
        count, err := issues_model.CountIssues(db.DefaultContext, &issues_model.IssuesOptions{})
        assert.NoError(t, err)
-       assert.EqualValues(t, 19, count)
+       assert.EqualValues(t, 20, count)
 }
 
 func TestIssueLoadAttributes(t *testing.T) {

diff --git a/modules/indexer/issues/indexer_test.go b/modules/indexer/issues/indexer_test.go
index 0e36d213137374ca7ee16650af3d1afc760ff839..7241f6313c891026f0dd5d6e2d506a67ef99b868 100644 (file)
--- a/modules/indexer/issues/indexer_test.go
+++ b/modules/indexer/issues/indexer_test.go
@@ -180,6 +180,21 @@ func searchIssueByID(t *testing.T) {
                        },
                        []int64{11, 6, 5, 3, 2, 1},
                },
+               {
+                       // issue 20 request user 15 and team 5 which user 15 belongs to
+                       // the review request number of issue 20 should be 1
+                       SearchOptions{
+                               ReviewRequestedID: int64Pointer(15),
+                       },
+                       []int64{12, 20},
+               },
+               {
+                       // user 20 approved the issue 20, so return nothing
+                       SearchOptions{
+                               ReviewRequestedID: int64Pointer(20),
+                       },
+                       []int64{},
+               },
        }
 
        for _, test := range tests {
@@ -206,7 +221,7 @@ func searchIssueIsPull(t *testing.T) {
                        SearchOptions{
                                IsPull: util.OptionalBoolTrue,
                        },
-                       []int64{12, 11, 19, 9, 8, 3, 2},
+                       []int64{12, 11, 20, 19, 9, 8, 3, 2},
                },
        }
        for _, test := range tests {
@@ -227,7 +242,7 @@ func searchIssueIsClosed(t *testing.T) {
                        SearchOptions{
                                IsClosed: util.OptionalBoolFalse,
                        },
-                       []int64{17, 16, 15, 14, 13, 12, 11, 6, 19, 18, 10, 7, 9, 8, 3, 2, 1},
+                       []int64{17, 16, 15, 14, 13, 12, 11, 20, 6, 19, 18, 10, 7, 9, 8, 3, 2, 1},
                },
                {
                        SearchOptions{
@@ -293,7 +308,7 @@ func searchIssueByLabelID(t *testing.T) {
                        SearchOptions{
                                ExcludedLabelIDs: []int64{1},
                        },
-                       []int64{17, 16, 15, 14, 13, 12, 11, 6, 5, 19, 18, 10, 7, 4, 9, 8, 3},
+                       []int64{17, 16, 15, 14, 13, 12, 11, 20, 6, 5, 19, 18, 10, 7, 4, 9, 8, 3},
                },
        }
        for _, test := range tests {
@@ -317,7 +332,7 @@ func searchIssueByTime(t *testing.T) {
                        SearchOptions{
                                UpdatedAfterUnix: int64Pointer(0),
                        },
-                       []int64{17, 16, 15, 14, 13, 12, 11, 6, 5, 19, 18, 10, 7, 4, 9, 8, 3, 2, 1},
+                       []int64{17, 16, 15, 14, 13, 12, 11, 20, 6, 5, 19, 18, 10, 7, 4, 9, 8, 3, 2, 1},
                },
        }
        for _, test := range tests {
@@ -338,7 +353,7 @@ func searchIssueWithOrder(t *testing.T) {
                        SearchOptions{
                                SortBy: internal.SortByCreatedAsc,
                        },
-                       []int64{1, 2, 3, 8, 9, 4, 7, 10, 18, 19, 5, 6, 11, 12, 13, 14, 15, 16, 17},
+                       []int64{1, 2, 3, 8, 9, 4, 7, 10, 18, 19, 5, 6, 20, 11, 12, 13, 14, 15, 16, 17},
                },
        }
        for _, test := range tests {
@@ -393,7 +408,7 @@ func searchIssueWithPaginator(t *testing.T) {
                                },
                        },
                        []int64{17, 16, 15, 14, 13},
-                       19,
+                       20,
                },
        }
        for _, test := range tests {

diff --git a/tests/integration/api_issue_test.go b/tests/integration/api_issue_test.go
index 808d288356f60b94f377481deeda8ed9bb95110d..29f09fa09e747f7fdab55278856221bdf98c9fe1 100644 (file)
--- a/tests/integration/api_issue_test.go
+++ b/tests/integration/api_issue_test.go
@@ -219,7 +219,7 @@ func TestAPISearchIssues(t *testing.T) {
        token := getUserToken(t, "user2", auth_model.AccessTokenScopeReadIssue)
 
        // as this API was used in the frontend, it uses UI page size
-       expectedIssueCount := 17 // from the fixtures
+       expectedIssueCount := 18 // from the fixtures
        if expectedIssueCount > setting.UI.IssuePagingNum {
                expectedIssueCount = setting.UI.IssuePagingNum
        }
@@ -243,7 +243,7 @@ func TestAPISearchIssues(t *testing.T) {
        req = NewRequest(t, "GET", link.String())
        resp = MakeRequest(t, req, http.StatusOK)
        DecodeJSON(t, resp, &apiIssues)
-       assert.Len(t, apiIssues, 10)
+       assert.Len(t, apiIssues, 11)
        query.Del("since")
        query.Del("before")
 
@@ -259,15 +259,15 @@ func TestAPISearchIssues(t *testing.T) {
        req = NewRequest(t, "GET", link.String())
        resp = MakeRequest(t, req, http.StatusOK)
        DecodeJSON(t, resp, &apiIssues)
-       assert.EqualValues(t, "19", resp.Header().Get("X-Total-Count"))
-       assert.Len(t, apiIssues, 19)
+       assert.EqualValues(t, "20", resp.Header().Get("X-Total-Count"))
+       assert.Len(t, apiIssues, 20)
 
        query.Add("limit", "10")
        link.RawQuery = query.Encode()
        req = NewRequest(t, "GET", link.String())
        resp = MakeRequest(t, req, http.StatusOK)
        DecodeJSON(t, resp, &apiIssues)
-       assert.EqualValues(t, "19", resp.Header().Get("X-Total-Count"))
+       assert.EqualValues(t, "20", resp.Header().Get("X-Total-Count"))
        assert.Len(t, apiIssues, 10)
 
        query = url.Values{"assigned": {"true"}, "state": {"all"}, "token": {token}}
@@ -317,7 +317,7 @@ func TestAPISearchIssuesWithLabels(t *testing.T) {
        defer tests.PrepareTestEnv(t)()
 
        // as this API was used in the frontend, it uses UI page size
-       expectedIssueCount := 17 // from the fixtures
+       expectedIssueCount := 18 // from the fixtures
        if expectedIssueCount > setting.UI.IssuePagingNum {
                expectedIssueCount = setting.UI.IssuePagingNum
        }

diff --git a/tests/integration/api_nodeinfo_test.go b/tests/integration/api_nodeinfo_test.go
index a347ec5b3b1f980653779478a973b912bb268a86..4cbd25f5deab1db812e0761f56f880afb024e693 100644 (file)
--- a/tests/integration/api_nodeinfo_test.go
+++ b/tests/integration/api_nodeinfo_test.go
@@ -33,7 +33,7 @@ func TestNodeinfo(t *testing.T) {
                assert.True(t, nodeinfo.OpenRegistrations)
                assert.Equal(t, "gitea", nodeinfo.Software.Name)
                assert.Equal(t, 25, nodeinfo.Usage.Users.Total)
-               assert.Equal(t, 19, nodeinfo.Usage.LocalPosts)
+               assert.Equal(t, 20, nodeinfo.Usage.LocalPosts)
                assert.Equal(t, 2, nodeinfo.Usage.LocalComments)
        })
 }

diff --git a/tests/integration/issue_test.go b/tests/integration/issue_test.go
index 4cae00f0bcb07d03d6f11ba25423e58291c314ce..853e565b0fac85177792e70c65e69ab8209b7991 100644 (file)
--- a/tests/integration/issue_test.go
+++ b/tests/integration/issue_test.go
@@ -356,7 +356,7 @@ func TestSearchIssues(t *testing.T) {
 
        session := loginUser(t, "user2")
 
-       expectedIssueCount := 17 // from the fixtures
+       expectedIssueCount := 18 // from the fixtures
        if expectedIssueCount > setting.UI.IssuePagingNum {
                expectedIssueCount = setting.UI.IssuePagingNum
        }
@@ -377,7 +377,7 @@ func TestSearchIssues(t *testing.T) {
        req = NewRequest(t, "GET", link.String())
        resp = session.MakeRequest(t, req, http.StatusOK)
        DecodeJSON(t, resp, &apiIssues)
-       assert.Len(t, apiIssues, 10)
+       assert.Len(t, apiIssues, 11)
        query.Del("since")
        query.Del("before")
 
@@ -393,15 +393,15 @@ func TestSearchIssues(t *testing.T) {
        req = NewRequest(t, "GET", link.String())
        resp = session.MakeRequest(t, req, http.StatusOK)
        DecodeJSON(t, resp, &apiIssues)
-       assert.EqualValues(t, "19", resp.Header().Get("X-Total-Count"))
-       assert.Len(t, apiIssues, 19)
+       assert.EqualValues(t, "20", resp.Header().Get("X-Total-Count"))
+       assert.Len(t, apiIssues, 20)
 
        query.Add("limit", "5")
        link.RawQuery = query.Encode()
        req = NewRequest(t, "GET", link.String())
        resp = session.MakeRequest(t, req, http.StatusOK)
        DecodeJSON(t, resp, &apiIssues)
-       assert.EqualValues(t, "19", resp.Header().Get("X-Total-Count"))
+       assert.EqualValues(t, "20", resp.Header().Get("X-Total-Count"))
        assert.Len(t, apiIssues, 5)
 
        query = url.Values{"assigned": {"true"}, "state": {"all"}}
@@ -450,7 +450,7 @@ func TestSearchIssues(t *testing.T) {
 func TestSearchIssuesWithLabels(t *testing.T) {
        defer tests.PrepareTestEnv(t)()
 
-       expectedIssueCount := 17 // from the fixtures
+       expectedIssueCount := 18 // from the fixtures
        if expectedIssueCount > setting.UI.IssuePagingNum {
                expectedIssueCount = setting.UI.IssuePagingNum
        }