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>
This commit is contained in:
Nanguan Lin 2023-09-21 19:59:50 +08:00 committed by GitHub
parent 218e719ea8
commit 2f8e1604f8
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
12 changed files with 115 additions and 23 deletions

View file

@ -321,3 +321,20 @@
created_unix: 946684830 created_unix: 946684830
updated_unix: 978307200 updated_unix: 978307200
is_locked: false 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

View file

@ -89,3 +89,12 @@
base_branch: main base_branch: main
merge_base: cbff181af4c9c7fee3cf6c106699e07d9a3f54e6 merge_base: cbff181af4c9c7fee3cf6c106699e07d9a3f54e6
has_merged: false 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

View file

@ -679,7 +679,7 @@
num_forks: 0 num_forks: 0
num_issues: 0 num_issues: 0
num_closed_issues: 0 num_closed_issues: 0
num_pulls: 0 num_pulls: 1
num_closed_pulls: 0 num_closed_pulls: 0
num_milestones: 0 num_milestones: 0
num_closed_milestones: 0 num_closed_milestones: 0

View file

@ -132,3 +132,41 @@
content: "singular review from org6 and final review for this pr" content: "singular review from org6 and final review for this pr"
updated_unix: 946684831 updated_unix: 946684831
created_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

View file

@ -93,7 +93,7 @@
name: review_team name: review_team
authorize: 1 # read authorize: 1 # read
num_repos: 1 num_repos: 1
num_members: 2 num_members: 3
includes_all_repositories: false includes_all_repositories: false
can_create_org_repo: false can_create_org_repo: false

View file

@ -123,3 +123,9 @@
org_id: 36 org_id: 36
team_id: 20 team_id: 20
uid: 5 uid: 5
-
id: 22
org_id: 17
team_id: 9
uid: 15

View file

@ -362,14 +362,21 @@ func applyReviewRequestedCondition(sess *xorm.Session, reviewRequestedID int64)
From("team_user"). From("team_user").
Where(builder.Eq{"team_user.uid": reviewRequestedID}) 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"). subQuery := builder.Select("review.issue_id").
From("review"). From("review").
Where(builder.And( Where(builder.And(
builder.In("review.type", []ReviewType{ReviewTypeRequest, ReviewTypeReject, ReviewTypeApprove}), builder.Eq{"review.type": ReviewTypeRequest},
builder.Or( builder.Or(
builder.Eq{"review.reviewer_id": reviewRequestedID}, builder.Eq{"review.reviewer_id": reviewRequestedID},
builder.In("review.reviewer_team_id", existInTeamQuery), builder.In("review.reviewer_team_id", existInTeamQuery),
), ),
builder.In("review.id", maxReview),
)) ))
return sess.Where("issue.poster_id <> ?", reviewRequestedID). return sess.Where("issue.poster_id <> ?", reviewRequestedID).
And(builder.In("issue.id", subQuery)) And(builder.In("issue.id", subQuery))

View file

@ -403,7 +403,7 @@ func TestCountIssues(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase()) assert.NoError(t, unittest.PrepareTestDatabase())
count, err := issues_model.CountIssues(db.DefaultContext, &issues_model.IssuesOptions{}) count, err := issues_model.CountIssues(db.DefaultContext, &issues_model.IssuesOptions{})
assert.NoError(t, err) assert.NoError(t, err)
assert.EqualValues(t, 19, count) assert.EqualValues(t, 20, count)
} }
func TestIssueLoadAttributes(t *testing.T) { func TestIssueLoadAttributes(t *testing.T) {

View file

@ -180,6 +180,21 @@ func searchIssueByID(t *testing.T) {
}, },
[]int64{11, 6, 5, 3, 2, 1}, []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 { for _, test := range tests {
@ -206,7 +221,7 @@ func searchIssueIsPull(t *testing.T) {
SearchOptions{ SearchOptions{
IsPull: util.OptionalBoolTrue, IsPull: util.OptionalBoolTrue,
}, },
[]int64{12, 11, 19, 9, 8, 3, 2}, []int64{12, 11, 20, 19, 9, 8, 3, 2},
}, },
} }
for _, test := range tests { for _, test := range tests {
@ -227,7 +242,7 @@ func searchIssueIsClosed(t *testing.T) {
SearchOptions{ SearchOptions{
IsClosed: util.OptionalBoolFalse, 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{ SearchOptions{
@ -293,7 +308,7 @@ func searchIssueByLabelID(t *testing.T) {
SearchOptions{ SearchOptions{
ExcludedLabelIDs: []int64{1}, 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 { for _, test := range tests {
@ -317,7 +332,7 @@ func searchIssueByTime(t *testing.T) {
SearchOptions{ SearchOptions{
UpdatedAfterUnix: int64Pointer(0), 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 { for _, test := range tests {
@ -338,7 +353,7 @@ func searchIssueWithOrder(t *testing.T) {
SearchOptions{ SearchOptions{
SortBy: internal.SortByCreatedAsc, 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 { for _, test := range tests {
@ -393,7 +408,7 @@ func searchIssueWithPaginator(t *testing.T) {
}, },
}, },
[]int64{17, 16, 15, 14, 13}, []int64{17, 16, 15, 14, 13},
19, 20,
}, },
} }
for _, test := range tests { for _, test := range tests {

View file

@ -219,7 +219,7 @@ func TestAPISearchIssues(t *testing.T) {
token := getUserToken(t, "user2", auth_model.AccessTokenScopeReadIssue) token := getUserToken(t, "user2", auth_model.AccessTokenScopeReadIssue)
// as this API was used in the frontend, it uses UI page size // 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 { if expectedIssueCount > setting.UI.IssuePagingNum {
expectedIssueCount = setting.UI.IssuePagingNum expectedIssueCount = setting.UI.IssuePagingNum
} }
@ -243,7 +243,7 @@ func TestAPISearchIssues(t *testing.T) {
req = NewRequest(t, "GET", link.String()) req = NewRequest(t, "GET", link.String())
resp = MakeRequest(t, req, http.StatusOK) resp = MakeRequest(t, req, http.StatusOK)
DecodeJSON(t, resp, &apiIssues) DecodeJSON(t, resp, &apiIssues)
assert.Len(t, apiIssues, 10) assert.Len(t, apiIssues, 11)
query.Del("since") query.Del("since")
query.Del("before") query.Del("before")
@ -259,15 +259,15 @@ func TestAPISearchIssues(t *testing.T) {
req = NewRequest(t, "GET", link.String()) req = NewRequest(t, "GET", link.String())
resp = MakeRequest(t, req, http.StatusOK) resp = MakeRequest(t, req, http.StatusOK)
DecodeJSON(t, resp, &apiIssues) 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, 19) assert.Len(t, apiIssues, 20)
query.Add("limit", "10") query.Add("limit", "10")
link.RawQuery = query.Encode() link.RawQuery = query.Encode()
req = NewRequest(t, "GET", link.String()) req = NewRequest(t, "GET", link.String())
resp = MakeRequest(t, req, http.StatusOK) resp = MakeRequest(t, req, http.StatusOK)
DecodeJSON(t, resp, &apiIssues) 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) assert.Len(t, apiIssues, 10)
query = url.Values{"assigned": {"true"}, "state": {"all"}, "token": {token}} query = url.Values{"assigned": {"true"}, "state": {"all"}, "token": {token}}
@ -317,7 +317,7 @@ func TestAPISearchIssuesWithLabels(t *testing.T) {
defer tests.PrepareTestEnv(t)() defer tests.PrepareTestEnv(t)()
// as this API was used in the frontend, it uses UI page size // 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 { if expectedIssueCount > setting.UI.IssuePagingNum {
expectedIssueCount = setting.UI.IssuePagingNum expectedIssueCount = setting.UI.IssuePagingNum
} }

View file

@ -33,7 +33,7 @@ func TestNodeinfo(t *testing.T) {
assert.True(t, nodeinfo.OpenRegistrations) assert.True(t, nodeinfo.OpenRegistrations)
assert.Equal(t, "gitea", nodeinfo.Software.Name) assert.Equal(t, "gitea", nodeinfo.Software.Name)
assert.Equal(t, 25, nodeinfo.Usage.Users.Total) 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) assert.Equal(t, 2, nodeinfo.Usage.LocalComments)
}) })
} }

View file

@ -356,7 +356,7 @@ func TestSearchIssues(t *testing.T) {
session := loginUser(t, "user2") session := loginUser(t, "user2")
expectedIssueCount := 17 // from the fixtures expectedIssueCount := 18 // from the fixtures
if expectedIssueCount > setting.UI.IssuePagingNum { if expectedIssueCount > setting.UI.IssuePagingNum {
expectedIssueCount = setting.UI.IssuePagingNum expectedIssueCount = setting.UI.IssuePagingNum
} }
@ -377,7 +377,7 @@ func TestSearchIssues(t *testing.T) {
req = NewRequest(t, "GET", link.String()) req = NewRequest(t, "GET", link.String())
resp = session.MakeRequest(t, req, http.StatusOK) resp = session.MakeRequest(t, req, http.StatusOK)
DecodeJSON(t, resp, &apiIssues) DecodeJSON(t, resp, &apiIssues)
assert.Len(t, apiIssues, 10) assert.Len(t, apiIssues, 11)
query.Del("since") query.Del("since")
query.Del("before") query.Del("before")
@ -393,15 +393,15 @@ func TestSearchIssues(t *testing.T) {
req = NewRequest(t, "GET", link.String()) req = NewRequest(t, "GET", link.String())
resp = session.MakeRequest(t, req, http.StatusOK) resp = session.MakeRequest(t, req, http.StatusOK)
DecodeJSON(t, resp, &apiIssues) 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, 19) assert.Len(t, apiIssues, 20)
query.Add("limit", "5") query.Add("limit", "5")
link.RawQuery = query.Encode() link.RawQuery = query.Encode()
req = NewRequest(t, "GET", link.String()) req = NewRequest(t, "GET", link.String())
resp = session.MakeRequest(t, req, http.StatusOK) resp = session.MakeRequest(t, req, http.StatusOK)
DecodeJSON(t, resp, &apiIssues) 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) assert.Len(t, apiIssues, 5)
query = url.Values{"assigned": {"true"}, "state": {"all"}} query = url.Values{"assigned": {"true"}, "state": {"all"}}
@ -450,7 +450,7 @@ func TestSearchIssues(t *testing.T) {
func TestSearchIssuesWithLabels(t *testing.T) { func TestSearchIssuesWithLabels(t *testing.T) {
defer tests.PrepareTestEnv(t)() defer tests.PrepareTestEnv(t)()
expectedIssueCount := 17 // from the fixtures expectedIssueCount := 18 // from the fixtures
if expectedIssueCount > setting.UI.IssuePagingNum { if expectedIssueCount > setting.UI.IssuePagingNum {
expectedIssueCount = setting.UI.IssuePagingNum expectedIssueCount = setting.UI.IssuePagingNum
} }