Skip to content

Commit 8a05ad7

Browse files
author
Gusted
committed
Fix issue overview for teams
- 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 go-gitea#19636
1 parent 5ca224a commit 8a05ad7

File tree

2 files changed

+59
-30
lines changed

2 files changed

+59
-30
lines changed

models/issue.go

Lines changed: 55 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1241,7 +1241,7 @@ func sortIssuesSession(sess *xorm.Session, sortType string, priorityRepoID int64
12411241
}
12421242
}
12431243

1244-
func (opts *IssuesOptions) setupSessionWithLimit(sess *xorm.Session) {
1244+
func (opts *IssuesOptions) setupSessionWithLimit(sess *xorm.Session) error {
12451245
if opts.Page >= 0 && opts.PageSize > 0 {
12461246
var start int
12471247
if opts.Page == 0 {
@@ -1251,10 +1251,10 @@ func (opts *IssuesOptions) setupSessionWithLimit(sess *xorm.Session) {
12511251
}
12521252
sess.Limit(opts.PageSize, start)
12531253
}
1254-
opts.setupSessionNoLimit(sess)
1254+
return opts.setupSessionNoLimit(sess)
12551255
}
12561256

1257-
func (opts *IssuesOptions) setupSessionNoLimit(sess *xorm.Session) {
1257+
func (opts *IssuesOptions) setupSessionNoLimit(sess *xorm.Session) error {
12581258
if len(opts.IssueIDs) > 0 {
12591259
sess.In("issue.id", opts.IssueIDs)
12601260
}
@@ -1348,22 +1348,50 @@ func (opts *IssuesOptions) setupSessionNoLimit(sess *xorm.Session) {
13481348
}
13491349

13501350
if opts.User != nil {
1351-
sess.And(
1352-
issuePullAccessibleRepoCond("issue.repo_id", opts.User.ID, opts.Org, opts.Team, opts.IsPull.IsTrue()),
1353-
)
1351+
repoCond, err := issuePullAccessibleRepoCond("issue.repo_id", opts.User.ID, opts.Org, opts.Team, opts.IsPull.IsTrue())
1352+
if err != nil {
1353+
return err
1354+
}
1355+
sess.And(repoCond)
13541356
}
1357+
return nil
13551358
}
13561359

13571360
// issuePullAccessibleRepoCond userID must not be zero, this condition require join repository table
1358-
func issuePullAccessibleRepoCond(repoIDstr string, userID int64, org *organization.Organization, team *organization.Team, isPull bool) builder.Cond {
1361+
func issuePullAccessibleRepoCond(repoIDstr string, userID int64, org *organization.Organization, team *organization.Team, isPull bool) (builder.Cond, error) {
13591362
cond := builder.NewCond()
13601363
unitType := unit.TypeIssues
13611364
if isPull {
13621365
unitType = unit.TypePullRequests
13631366
}
13641367
if org != nil {
13651368
if team != nil {
1366-
cond = cond.And(teamUnitsRepoCond(repoIDstr, userID, org.ID, team.ID, unitType)) // special team member repos
1369+
// If the current user is a admin, it doesn't necesarly mean
1370+
// it has joined the team, but we still should return all repo's
1371+
// of that team.
1372+
isAdmin, err := org.IsOwnedBy(userID)
1373+
if err != nil {
1374+
return nil, err
1375+
}
1376+
if isAdmin {
1377+
cond = cond.And(builder.In(repoIDstr,
1378+
builder.Select("repo_id").From("team_repo").Where(
1379+
builder.Eq{
1380+
"team_id": team.ID,
1381+
}.And(
1382+
builder.In(
1383+
"team_id", builder.Select("team_id").From("team_unit").Where(
1384+
builder.Eq{
1385+
"`team_unit`.org_id": org.ID,
1386+
"`team_unit`.type": unitType,
1387+
},
1388+
),
1389+
),
1390+
),
1391+
)))
1392+
} else {
1393+
cond = cond.And(teamUnitsRepoCond(repoIDstr, userID, org.ID, team.ID, unitType)) // special team member repos
1394+
}
13671395
} else {
13681396
cond = cond.And(
13691397
builder.Or(
@@ -1383,7 +1411,7 @@ func issuePullAccessibleRepoCond(repoIDstr string, userID int64, org *organizati
13831411
),
13841412
)
13851413
}
1386-
return cond
1414+
return cond, nil
13871415
}
13881416

13891417
func applyAssigneeCondition(sess *xorm.Session, assigneeID int64) *xorm.Session {
@@ -1416,7 +1444,9 @@ func CountIssuesByRepo(opts *IssuesOptions) (map[int64]int64, error) {
14161444

14171445
sess := e.Join("INNER", "repository", "`issue`.repo_id = `repository`.id")
14181446

1419-
opts.setupSessionNoLimit(sess)
1447+
if err := opts.setupSessionNoLimit(sess); err != nil {
1448+
return nil, fmt.Errorf("setupSessionNoLimit: %v", err)
1449+
}
14201450

14211451
countsSlice := make([]*struct {
14221452
RepoID int64
@@ -1443,7 +1473,9 @@ func GetRepoIDsForIssuesOptions(opts *IssuesOptions, user *user_model.User) ([]i
14431473

14441474
sess := e.Join("INNER", "repository", "`issue`.repo_id = `repository`.id")
14451475

1446-
opts.setupSessionNoLimit(sess)
1476+
if err := opts.setupSessionNoLimit(sess); err != nil {
1477+
return nil, fmt.Errorf("setupSessionNoLimit: %v", err)
1478+
}
14471479

14481480
accessCond := accessibleRepositoryCondition(user)
14491481
if err := sess.Where(accessCond).
@@ -1461,7 +1493,9 @@ func Issues(opts *IssuesOptions) ([]*Issue, error) {
14611493
e := db.GetEngine(db.DefaultContext)
14621494

14631495
sess := e.Join("INNER", "repository", "`issue`.repo_id = `repository`.id")
1464-
opts.setupSessionWithLimit(sess)
1496+
if err := opts.setupSessionWithLimit(sess); err != nil {
1497+
return nil, fmt.Errorf("setupSessionWithLimit: %v", err)
1498+
}
14651499
sortIssuesSession(sess, opts.SortType, opts.PriorityRepoID)
14661500

14671501
issues := make([]*Issue, 0, opts.ListOptions.PageSize)
@@ -1482,7 +1516,10 @@ func CountIssues(opts *IssuesOptions) (int64, error) {
14821516

14831517
sess := e.Select("COUNT(issue.id) AS count").Table("issue")
14841518
sess.Join("INNER", "repository", "`issue`.repo_id = `repository`.id")
1485-
opts.setupSessionNoLimit(sess)
1519+
if err := opts.setupSessionNoLimit(sess); err != nil {
1520+
return 0, fmt.Errorf("setupSessionNoLimit: %v", err)
1521+
}
1522+
14861523
return sess.Count()
14871524
}
14881525

@@ -1709,7 +1746,11 @@ func GetUserIssueStats(opts UserIssueStatsOptions) (*IssueStats, error) {
17091746
}
17101747

17111748
if opts.UserID > 0 {
1712-
cond = cond.And(issuePullAccessibleRepoCond("issue.repo_id", opts.UserID, opts.Org, opts.Team, opts.IsPull))
1749+
repoCond, err := issuePullAccessibleRepoCond("issue.repo_id", opts.UserID, opts.Org, opts.Team, opts.IsPull)
1750+
if err != nil {
1751+
return nil, err
1752+
}
1753+
cond = cond.And(repoCond)
17131754
}
17141755

17151756
sess := func(cond builder.Cond) *xorm.Session {

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)