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

fix go-gitea#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 go-gitea#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 go-gitea#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>
GiteaBot · Sep 21, 2023 · 996fe3a · 996fe3a
1 parent 0ca2332
commit 996fe3a
Show file tree

Hide file tree

Showing 12 changed files with 115 additions and 23 deletions.
diff --git 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
@@ -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
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
  }