Skip to content

Commit 71ca131

Browse files
author
Gusted
authored
Fix issue overview for teams (#19652)
- Don't use hacky solution to limit to the correct RepoID's, instead use current code to handle these limits. The existing code is more correct than the hacky solution. - Resolves #19636 - Add test-case
1 parent d494cc3 commit 71ca131

12 files changed

+87
-36
lines changed

‎integrations/api_issue_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -209,22 +209,22 @@ func TestAPISearchIssues(t *testing.T) {
209209
req = NewRequest(t, "GET", link.String())
210210
resp = MakeRequest(t, req, http.StatusOK)
211211
DecodeJSON(t, resp, &apiIssues)
212-
assert.EqualValues(t, "15", resp.Header().Get("X-Total-Count"))
212+
assert.EqualValues(t, "17", resp.Header().Get("X-Total-Count"))
213213
assert.Len(t, apiIssues, 10) // there are more but 10 is page item limit
214214

215215
query.Add("limit", "20")
216216
link.RawQuery = query.Encode()
217217
req = NewRequest(t, "GET", link.String())
218218
resp = MakeRequest(t, req, http.StatusOK)
219219
DecodeJSON(t, resp, &apiIssues)
220-
assert.Len(t, apiIssues, 15)
220+
assert.Len(t, apiIssues, 17)
221221

222222
query = url.Values{"assigned": {"true"}, "state": {"all"}, "token": {token}}
223223
link.RawQuery = query.Encode()
224224
req = NewRequest(t, "GET", link.String())
225225
resp = MakeRequest(t, req, http.StatusOK)
226226
DecodeJSON(t, resp, &apiIssues)
227-
assert.Len(t, apiIssues, 1)
227+
assert.Len(t, apiIssues, 2)
228228

229229
query = url.Values{"milestones": {"milestone1"}, "state": {"all"}, "token": {token}}
230230
link.RawQuery = query.Encode()
@@ -252,7 +252,7 @@ func TestAPISearchIssues(t *testing.T) {
252252
req = NewRequest(t, "GET", link.String())
253253
resp = MakeRequest(t, req, http.StatusOK)
254254
DecodeJSON(t, resp, &apiIssues)
255-
assert.Len(t, apiIssues, 3)
255+
assert.Len(t, apiIssues, 5)
256256

257257
query = url.Values{"owner": {"user3"}, "team": {"team1"}, "token": {token}} // organization + team
258258
link.RawQuery = query.Encode()

‎integrations/api_nodeinfo_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ func TestNodeinfo(t *testing.T) {
2929
assert.True(t, nodeinfo.OpenRegistrations)
3030
assert.Equal(t, "gitea", nodeinfo.Software.Name)
3131
assert.Equal(t, 23, nodeinfo.Usage.Users.Total)
32-
assert.Equal(t, 15, nodeinfo.Usage.LocalPosts)
32+
assert.Equal(t, 17, nodeinfo.Usage.LocalPosts)
3333
assert.Equal(t, 2, nodeinfo.Usage.LocalComments)
3434
})
3535
}

‎integrations/issue_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -392,22 +392,22 @@ func TestSearchIssues(t *testing.T) {
392392
req = NewRequest(t, "GET", link.String())
393393
resp = session.MakeRequest(t, req, http.StatusOK)
394394
DecodeJSON(t, resp, &apiIssues)
395-
assert.EqualValues(t, "15", resp.Header().Get("X-Total-Count"))
395+
assert.EqualValues(t, "17", resp.Header().Get("X-Total-Count"))
396396
assert.Len(t, apiIssues, 10) // there are more but 10 is page item limit
397397

398398
query.Add("limit", "20")
399399
link.RawQuery = query.Encode()
400400
req = NewRequest(t, "GET", link.String())
401401
resp = session.MakeRequest(t, req, http.StatusOK)
402402
DecodeJSON(t, resp, &apiIssues)
403-
assert.Len(t, apiIssues, 15)
403+
assert.Len(t, apiIssues, 17)
404404

405405
query = url.Values{"assigned": {"true"}, "state": {"all"}}
406406
link.RawQuery = query.Encode()
407407
req = NewRequest(t, "GET", link.String())
408408
resp = session.MakeRequest(t, req, http.StatusOK)
409409
DecodeJSON(t, resp, &apiIssues)
410-
assert.Len(t, apiIssues, 1)
410+
assert.Len(t, apiIssues, 2)
411411

412412
query = url.Values{"milestones": {"milestone1"}, "state": {"all"}}
413413
link.RawQuery = query.Encode()
@@ -435,7 +435,7 @@ func TestSearchIssues(t *testing.T) {
435435
req = NewRequest(t, "GET", link.String())
436436
resp = session.MakeRequest(t, req, http.StatusOK)
437437
DecodeJSON(t, resp, &apiIssues)
438-
assert.Len(t, apiIssues, 3)
438+
assert.Len(t, apiIssues, 5)
439439

440440
query = url.Values{"owner": {"user3"}, "team": {"team1"}} // organization + team
441441
link.RawQuery = query.Encode()

‎models/fixtures/issue.yml

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,3 +184,27 @@
184184
is_pull: false
185185
created_unix: 1602935696
186186
updated_unix: 1602935696
187+
188+
-
189+
id: 16
190+
repo_id: 32
191+
index: 1
192+
poster_id: 2
193+
name: just a normal issue
194+
content: content
195+
is_closed: false
196+
is_pull: false
197+
created_unix: 1602935696
198+
updated_unix: 1602935696
199+
200+
-
201+
id: 17
202+
repo_id: 32
203+
index: 2
204+
poster_id: 15
205+
name: a issue with a assignment
206+
content: content
207+
is_closed: false
208+
is_pull: false
209+
created_unix: 1602935696
210+
updated_unix: 1602935696

‎models/fixtures/issue_assignees.yml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,3 +10,7 @@
1010
id: 3
1111
assignee_id: 2
1212
issue_id: 6
13+
-
14+
id: 4
15+
assignee_id: 2
16+
issue_id: 17

‎models/fixtures/issue_index.yml

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,9 @@
1010
-
1111
group_id: 10
1212
max_index: 1
13+
-
14+
group_id: 32
15+
max_index: 2
1316
-
1417
group_id: 48
1518
max_index: 1
@@ -21,4 +24,4 @@
2124
max_index: 1
2225
-
2326
group_id: 51
24-
max_index: 1
27+
max_index: 1

‎models/fixtures/repository.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -483,7 +483,7 @@
483483
is_private: false
484484
num_stars: 0
485485
num_forks: 0
486-
num_issues: 0
486+
num_issues: 2
487487
is_mirror: false
488488
status: 0
489489

‎models/fixtures/team_unit.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,7 @@
252252

253253
-
254254
id: 43
255+
org_id: 3
255256
team_id: 7
256257
type: 2 # issues
257258
access_mode: 2

‎models/issue.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1349,9 +1349,7 @@ func (opts *IssuesOptions) setupSessionNoLimit(sess *xorm.Session) {
13491349
}
13501350

13511351
if opts.User != nil {
1352-
sess.And(
1353-
issuePullAccessibleRepoCond("issue.repo_id", opts.User.ID, opts.Org, opts.Team, opts.IsPull.IsTrue()),
1354-
)
1352+
sess.And(issuePullAccessibleRepoCond("issue.repo_id", opts.User.ID, opts.Org, opts.Team, opts.IsPull.IsTrue()))
13551353
}
13561354
}
13571355

@@ -1463,6 +1461,7 @@ func Issues(opts *IssuesOptions) ([]*Issue, error) {
14631461

14641462
sess := e.Join("INNER", "repository", "`issue`.repo_id = `repository`.id")
14651463
opts.setupSessionWithLimit(sess)
1464+
14661465
sortIssuesSession(sess, opts.SortType, opts.PriorityRepoID)
14671466

14681467
issues := make([]*Issue, 0, opts.ListOptions.PageSize)
@@ -1484,6 +1483,7 @@ func CountIssues(opts *IssuesOptions) (int64, error) {
14841483
sess := e.Select("COUNT(issue.id) AS count").Table("issue")
14851484
sess.Join("INNER", "repository", "`issue`.repo_id = `repository`.id")
14861485
opts.setupSessionNoLimit(sess)
1486+
14871487
return sess.Count()
14881488
}
14891489

‎models/issue_test.go

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"code.gitea.io/gitea/models/db"
1717
"code.gitea.io/gitea/models/foreignreference"
1818
issues_model "code.gitea.io/gitea/models/issues"
19+
"code.gitea.io/gitea/models/organization"
1920
repo_model "code.gitea.io/gitea/models/repo"
2021
"code.gitea.io/gitea/models/unittest"
2122
user_model "code.gitea.io/gitea/models/user"
@@ -287,6 +288,20 @@ func TestGetUserIssueStats(t *testing.T) {
287288
ClosedCount: 0,
288289
},
289290
},
291+
{
292+
UserIssueStatsOptions{
293+
UserID: 2,
294+
Org: unittest.AssertExistsAndLoadBean(t, &organization.Organization{ID: 3}).(*organization.Organization),
295+
Team: unittest.AssertExistsAndLoadBean(t, &organization.Team{ID: 7}).(*organization.Team),
296+
FilterMode: FilterModeAll,
297+
},
298+
IssueStats{
299+
YourRepositoriesCount: 2,
300+
AssignCount: 1,
301+
CreateCount: 1,
302+
OpenCount: 2,
303+
},
304+
},
290305
} {
291306
t.Run(fmt.Sprintf("%#v", test.Opts), func(t *testing.T) {
292307
stats, err := GetUserIssueStats(test.Opts)
@@ -341,7 +356,7 @@ func TestGetRepoIDsForIssuesOptions(t *testing.T) {
341356
IssuesOptions{
342357
AssigneeID: 2,
343358
},
344-
[]int64{3},
359+
[]int64{3, 32},
345360
},
346361
{
347362
IssuesOptions{
@@ -595,5 +610,5 @@ func TestCountIssues(t *testing.T) {
595610
assert.NoError(t, unittest.PrepareTestDatabase())
596611
count, err := CountIssues(&IssuesOptions{})
597612
assert.NoError(t, err)
598-
assert.EqualValues(t, 15, count)
613+
assert.EqualValues(t, 17, count)
599614
}

‎models/repo_list.go

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"strings"
1010

1111
"code.gitea.io/gitea/models/db"
12+
"code.gitea.io/gitea/models/organization"
1213
"code.gitea.io/gitea/models/perm"
1314
repo_model "code.gitea.io/gitea/models/repo"
1415
"code.gitea.io/gitea/models/unit"
@@ -246,11 +247,26 @@ func teamUnitsRepoCond(id string, userID, orgID, teamID int64, units ...unit.Typ
246247
builder.Eq{
247248
"team_id": teamID,
248249
}.And(
249-
builder.In(
250-
"team_id", builder.Select("team_id").From("team_user").Where(
251-
builder.Eq{
250+
builder.Or(
251+
// Check if the user is member of the team.
252+
builder.In(
253+
"team_id", builder.Select("team_id").From("team_user").Where(
254+
builder.Eq{
255+
"uid": userID,
256+
},
257+
),
258+
),
259+
// Check if the user is in the owner team of the organisation.
260+
builder.Exists(builder.Select("team_id").From("team_user").
261+
Where(builder.Eq{
262+
"org_id": orgID,
263+
"team_id": builder.Select("id").From("team").Where(
264+
builder.Eq{
265+
"org_id": orgID,
266+
"lower_name": strings.ToLower(organization.OwnerTeamName),
267+
}),
252268
"uid": userID,
253-
},
269+
}),
254270
),
255271
)).And(
256272
builder.In(

‎routers/web/user/home.go

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -443,12 +443,13 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) {
443443
AllLimited: false,
444444
}
445445

446-
if ctxUser.IsOrganization() && ctx.Org.Team != nil {
447-
repoOpts.TeamID = ctx.Org.Team.ID
446+
if team != nil {
447+
repoOpts.TeamID = team.ID
448448
}
449449

450450
switch filterMode {
451451
case models.FilterModeAll:
452+
case models.FilterModeYourRepositories:
452453
case models.FilterModeAssign:
453454
opts.AssigneeID = ctx.Doer.ID
454455
case models.FilterModeCreate:
@@ -457,13 +458,6 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) {
457458
opts.MentionedID = ctx.Doer.ID
458459
case models.FilterModeReviewRequested:
459460
opts.ReviewRequestedID = ctx.Doer.ID
460-
case models.FilterModeYourRepositories:
461-
if ctxUser.IsOrganization() && ctx.Org.Team != nil {
462-
// Fixes a issue whereby the user's ID would be used
463-
// to check if it's in the team(which possible isn't the case).
464-
opts.User = nil
465-
}
466-
opts.RepoCond = models.SearchRepositoryCondition(repoOpts)
467461
}
468462

469463
// keyword holds the search term entered into the search field.
@@ -595,13 +589,7 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) {
595589
Org: org,
596590
Team: team,
597591
}
598-
if filterMode == models.FilterModeYourRepositories {
599-
statsOpts.RepoCond = models.SearchRepositoryCondition(repoOpts)
600-
}
601-
// Detect when we only should search by team.
602-
if opts.User == nil {
603-
statsOpts.UserID = 0
604-
}
592+
605593
issueStats, err = models.GetUserIssueStats(statsOpts)
606594
if err != nil {
607595
ctx.ServerError("GetUserIssueStats Shown", err)

0 commit comments

Comments
 (0)