Skip to content

Commit 5640303

Browse files
eneuschild6543Giteatechknowlogick
authored
Issues overview should not show issues from archived repos (go-gitea#13220)
* Add lots of comments to user.Issues() * Answered some questions from comments * fix typo in comment * Refac user.Issues(): add func repoIDs * Refac user.Issues(): add func userRepoIDs * Refac user.Issues(): add func issueIDsFromSearch * Refac user.Issues(): improve error handling * Refac user.Issues(): add inline documentation and move variable declarations closer to their usages * Refac user.Issues(): add func repoIDMap * Refac user.Issues(): cleanup * Refac: Separate Issues from Pulls during routing * fix typo in comment * Adapt Unittests to Refactoring * Issue13171: Issue and PR Overviews now ignore archived Repositories * changed some verbatim SQL conditions to builder.Eq * models/issue.go: use OptionalBool properly Co-authored-by: 6543 <6543@obermui.de> * Use IsArchived rather than ExcludeArchivedRepos * fixed broken test after merge * added nil check * Added Unit Test securing Issue 13171 fix * Improved IsArchived filtering in issue.GetUserIssueStats * Removed unused func * Added grouping to avoid returning duplicate repo IDs Co-authored-by: 6543 <6543@obermui.de> Co-authored-by: Gitea <gitea@fake.local> Co-authored-by: techknowlogick <techknowlogick@gitea.io>
1 parent 81467e6 commit 5640303

14 files changed

+490
-163
lines changed

‎integrations/api_issue_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -186,15 +186,15 @@ func TestAPISearchIssues(t *testing.T) {
186186
req = NewRequest(t, "GET", link.String())
187187
resp = session.MakeRequest(t, req, http.StatusOK)
188188
DecodeJSON(t, resp, &apiIssues)
189-
assert.EqualValues(t, "12", resp.Header().Get("X-Total-Count"))
189+
assert.EqualValues(t, "14", resp.Header().Get("X-Total-Count"))
190190
assert.Len(t, apiIssues, 10) //there are more but 10 is page item limit
191191

192192
query.Add("limit", "20")
193193
link.RawQuery = query.Encode()
194194
req = NewRequest(t, "GET", link.String())
195195
resp = session.MakeRequest(t, req, http.StatusOK)
196196
DecodeJSON(t, resp, &apiIssues)
197-
assert.Len(t, apiIssues, 12)
197+
assert.Len(t, apiIssues, 14)
198198

199199
query = url.Values{"assigned": {"true"}, "state": {"all"}}
200200
link.RawQuery = query.Encode()

‎integrations/api_repo_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,9 +77,9 @@ func TestAPISearchRepo(t *testing.T) {
7777
expectedResults
7878
}{
7979
{name: "RepositoriesMax50", requestURL: "/api/v1/repos/search?limit=50&private=false", expectedResults: expectedResults{
80-
nil: {count: 28},
81-
user: {count: 28},
82-
user2: {count: 28}},
80+
nil: {count: 30},
81+
user: {count: 30},
82+
user2: {count: 30}},
8383
},
8484
{name: "RepositoriesMax10", requestURL: "/api/v1/repos/search?limit=10&private=false", expectedResults: expectedResults{
8585
nil: {count: 10},

‎models/fixtures/issue.yml

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,3 +147,28 @@
147147
is_pull: true
148148
created_unix: 1602935696
149149
updated_unix: 1602935696
150+
151+
152+
-
153+
id: 13
154+
repo_id: 50
155+
index: 0
156+
poster_id: 2
157+
name: issue in active repo
158+
content: we'll be testing github issue 13171 with this.
159+
is_closed: false
160+
is_pull: false
161+
created_unix: 1602935696
162+
updated_unix: 1602935696
163+
164+
-
165+
id: 14
166+
repo_id: 51
167+
index: 0
168+
poster_id: 2
169+
name: issue in archived repo
170+
content: we'll be testing github issue 13171 with this.
171+
is_closed: false
172+
is_pull: false
173+
created_unix: 1602935696
174+
updated_unix: 1602935696

‎models/fixtures/repo_unit.yml

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -532,3 +532,15 @@
532532
repo_id: 3
533533
type: 8
534534
created_unix: 946684810
535+
536+
-
537+
id: 78
538+
repo_id: 50
539+
type: 2
540+
created_unix: 946684810
541+
542+
-
543+
id: 79
544+
repo_id: 51
545+
type: 2
546+
created_unix: 946684810

‎models/fixtures/repository.yml

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
owner_name: user2
55
lower_name: repo1
66
name: repo1
7+
is_archived: false
78
is_empty: false
89
is_private: false
910
num_issues: 2
@@ -23,6 +24,7 @@
2324
owner_name: user2
2425
lower_name: repo2
2526
name: repo2
27+
is_archived: false
2628
is_private: true
2729
num_issues: 2
2830
num_closed_issues: 1
@@ -693,3 +695,43 @@
693695
num_issues: 0
694696
is_mirror: false
695697
status: 0
698+
699+
-
700+
id: 50
701+
owner_id: 30
702+
owner_name: user30
703+
lower_name: repo50
704+
name: repo50
705+
is_archived: false
706+
is_empty: false
707+
is_private: false
708+
num_issues: 1
709+
num_closed_issues: 0
710+
num_pulls: 0
711+
num_closed_pulls: 0
712+
num_milestones: 0
713+
num_closed_milestones: 0
714+
num_watches: 0
715+
num_projects: 0
716+
num_closed_projects: 0
717+
status: 0
718+
719+
-
720+
id: 51
721+
owner_id: 30
722+
owner_name: user30
723+
lower_name: repo51
724+
name: repo51
725+
is_archived: true
726+
is_empty: false
727+
is_private: false
728+
num_issues: 1
729+
num_closed_issues: 0
730+
num_pulls: 0
731+
num_closed_pulls: 0
732+
num_milestones: 0
733+
num_closed_milestones: 0
734+
num_watches: 0
735+
num_projects: 0
736+
num_closed_projects: 0
737+
status: 0

‎models/fixtures/user.yml

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -507,3 +507,21 @@
507507
avatar_email: user29@example.com
508508
num_repos: 0
509509
is_active: true
510+
511+
512+
-
513+
id: 30
514+
lower_name: user30
515+
name: user30
516+
full_name: User Thirty
517+
email: user30@example.com
518+
passwd_hash_algo: argon2
519+
passwd: a3d5fcd92bae586c2e3dbe72daea7a0d27833a8d0227aa1704f4bbd775c1f3b03535b76dd93b0d4d8d22a519dca47df1547b # password
520+
type: 0 # individual
521+
salt: ZogKvWdyEx
522+
is_admin: false
523+
is_restricted: true
524+
avatar: avatar29
525+
avatar_email: user30@example.com
526+
num_repos: 2
527+
is_active: true

‎models/issue.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1104,6 +1104,7 @@ type IssuesOptions struct {
11041104
UpdatedBeforeUnix int64
11051105
// prioritize issues from this repo
11061106
PriorityRepoID int64
1107+
IsArchived util.OptionalBool
11071108
}
11081109

11091110
// sortIssuesSession sort an issues-related session based on the provided
@@ -1207,6 +1208,10 @@ func (opts *IssuesOptions) setupSession(sess *xorm.Session) {
12071208
sess.And("issue.is_pull=?", false)
12081209
}
12091210

1211+
if opts.IsArchived != util.OptionalBoolNone {
1212+
sess.Join("INNER", "repository", "issue.repo_id = repository.id").And(builder.Eq{"repository.is_archived": opts.IsArchived.IsTrue()})
1213+
}
1214+
12101215
if opts.LabelIDs != nil {
12111216
for i, labelID := range opts.LabelIDs {
12121217
if labelID > 0 {
@@ -1501,6 +1506,7 @@ type UserIssueStatsOptions struct {
15011506
IsPull bool
15021507
IsClosed bool
15031508
IssueIDs []int64
1509+
IsArchived util.OptionalBool
15041510
LabelIDs []int64
15051511
}
15061512

@@ -1524,6 +1530,10 @@ func GetUserIssueStats(opts UserIssueStatsOptions) (*IssueStats, error) {
15241530
s.Join("INNER", "issue_label", "issue_label.issue_id = issue.id").
15251531
In("issue_label.label_id", opts.LabelIDs)
15261532
}
1533+
if opts.IsArchived != util.OptionalBoolNone {
1534+
s.Join("INNER", "repository", "issue.repo_id = repository.id").
1535+
And(builder.Eq{"repository.is_archived": opts.IsArchived.IsTrue()})
1536+
}
15271537
return s
15281538
}
15291539

‎models/org.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -753,7 +753,7 @@ type accessibleReposEnv struct {
753753
orderBy SearchOrderBy
754754
}
755755

756-
// AccessibleReposEnv an AccessibleReposEnvironment for the repositories in `org`
756+
// AccessibleReposEnv builds an AccessibleReposEnvironment for the repositories in `org`
757757
// that are accessible to the specified user.
758758
func (org *User) AccessibleReposEnv(userID int64) (AccessibleReposEnvironment, error) {
759759
return org.accessibleReposEnv(x, userID)

‎models/repo_list_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -187,10 +187,10 @@ func TestSearchRepository(t *testing.T) {
187187
count: 14},
188188
{name: "AllPublic/PublicRepositoriesOfUserIncludingCollaborative",
189189
opts: &SearchRepoOptions{ListOptions: ListOptions{Page: 1, PageSize: 10}, OwnerID: 15, AllPublic: true, Template: util.OptionalBoolFalse},
190-
count: 26},
190+
count: 28},
191191
{name: "AllPublic/PublicAndPrivateRepositoriesOfUserIncludingCollaborative",
192192
opts: &SearchRepoOptions{ListOptions: ListOptions{Page: 1, PageSize: 10}, OwnerID: 15, Private: true, AllPublic: true, AllLimited: true, Template: util.OptionalBoolFalse},
193-
count: 31},
193+
count: 33},
194194
{name: "AllPublic/PublicAndPrivateRepositoriesOfUserIncludingCollaborativeByName",
195195
opts: &SearchRepoOptions{Keyword: "test", ListOptions: ListOptions{Page: 1, PageSize: 10}, OwnerID: 15, Private: true, AllPublic: true},
196196
count: 15},
@@ -199,7 +199,7 @@ func TestSearchRepository(t *testing.T) {
199199
count: 13},
200200
{name: "AllPublic/PublicRepositoriesOfOrganization",
201201
opts: &SearchRepoOptions{ListOptions: ListOptions{Page: 1, PageSize: 10}, OwnerID: 17, AllPublic: true, Collaborate: util.OptionalBoolFalse, Template: util.OptionalBoolFalse},
202-
count: 26},
202+
count: 28},
203203
{name: "AllTemplates",
204204
opts: &SearchRepoOptions{ListOptions: ListOptions{Page: 1, PageSize: 10}, Template: util.OptionalBoolTrue},
205205
count: 2},

‎models/user.go

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -503,6 +503,23 @@ func (u *User) GetRepositoryIDs(units ...UnitType) ([]int64, error) {
503503
return ids, sess.Where("owner_id = ?", u.ID).Find(&ids)
504504
}
505505

506+
// GetActiveRepositoryIDs returns non-archived repositories IDs where user owned and has unittypes
507+
// Caller shall check that units is not globally disabled
508+
func (u *User) GetActiveRepositoryIDs(units ...UnitType) ([]int64, error) {
509+
var ids []int64
510+
511+
sess := x.Table("repository").Cols("repository.id")
512+
513+
if len(units) > 0 {
514+
sess = sess.Join("INNER", "repo_unit", "repository.id = repo_unit.repo_id")
515+
sess = sess.In("repo_unit.type", units)
516+
}
517+
518+
sess.Where(builder.Eq{"is_archived": false})
519+
520+
return ids, sess.Where("owner_id = ?", u.ID).GroupBy("repository.id").Find(&ids)
521+
}
522+
506523
// GetOrgRepositoryIDs returns repositories IDs where user's team owned and has unittypes
507524
// Caller shall check that units is not globally disabled
508525
func (u *User) GetOrgRepositoryIDs(units ...UnitType) ([]int64, error) {
@@ -524,6 +541,28 @@ func (u *User) GetOrgRepositoryIDs(units ...UnitType) ([]int64, error) {
524541
return ids, nil
525542
}
526543

544+
// GetActiveOrgRepositoryIDs returns non-archived repositories IDs where user's team owned and has unittypes
545+
// Caller shall check that units is not globally disabled
546+
func (u *User) GetActiveOrgRepositoryIDs(units ...UnitType) ([]int64, error) {
547+
var ids []int64
548+
549+
if err := x.Table("repository").
550+
Cols("repository.id").
551+
Join("INNER", "team_user", "repository.owner_id = team_user.org_id").
552+
Join("INNER", "team_repo", "(? != ? and repository.is_private != ?) OR (team_user.team_id = team_repo.team_id AND repository.id = team_repo.repo_id)", true, u.IsRestricted, true).
553+
Where("team_user.uid = ?", u.ID).
554+
Where(builder.Eq{"is_archived": false}).
555+
GroupBy("repository.id").Find(&ids); err != nil {
556+
return nil, err
557+
}
558+
559+
if len(units) > 0 {
560+
return FilterOutRepoIdsWithoutUnitAccess(u, ids, units...)
561+
}
562+
563+
return ids, nil
564+
}
565+
527566
// GetAccessRepoIDs returns all repositories IDs where user's or user is a team member organizations
528567
// Caller shall check that units is not globally disabled
529568
func (u *User) GetAccessRepoIDs(units ...UnitType) ([]int64, error) {
@@ -538,6 +577,20 @@ func (u *User) GetAccessRepoIDs(units ...UnitType) ([]int64, error) {
538577
return append(ids, ids2...), nil
539578
}
540579

580+
// GetActiveAccessRepoIDs returns all non-archived repositories IDs where user's or user is a team member organizations
581+
// Caller shall check that units is not globally disabled
582+
func (u *User) GetActiveAccessRepoIDs(units ...UnitType) ([]int64, error) {
583+
ids, err := u.GetActiveRepositoryIDs(units...)
584+
if err != nil {
585+
return nil, err
586+
}
587+
ids2, err := u.GetActiveOrgRepositoryIDs(units...)
588+
if err != nil {
589+
return nil, err
590+
}
591+
return append(ids, ids2...), nil
592+
}
593+
541594
// GetMirrorRepositories returns mirror repositories that user owns, including private repositories.
542595
func (u *User) GetMirrorRepositories() ([]*Repository, error) {
543596
return GetUserMirrorRepositories(u.ID)

‎models/user_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -136,13 +136,13 @@ func TestSearchUsers(t *testing.T) {
136136
}
137137

138138
testUserSuccess(&SearchUserOptions{OrderBy: "id ASC", ListOptions: ListOptions{Page: 1}},
139-
[]int64{1, 2, 4, 5, 8, 9, 10, 11, 12, 13, 14, 15, 16, 18, 20, 21, 24, 27, 28, 29})
139+
[]int64{1, 2, 4, 5, 8, 9, 10, 11, 12, 13, 14, 15, 16, 18, 20, 21, 24, 27, 28, 29, 30})
140140

141141
testUserSuccess(&SearchUserOptions{ListOptions: ListOptions{Page: 1}, IsActive: util.OptionalBoolFalse},
142142
[]int64{9})
143143

144144
testUserSuccess(&SearchUserOptions{OrderBy: "id ASC", ListOptions: ListOptions{Page: 1}, IsActive: util.OptionalBoolTrue},
145-
[]int64{1, 2, 4, 5, 8, 10, 11, 12, 13, 14, 15, 16, 18, 20, 21, 24, 28, 29})
145+
[]int64{1, 2, 4, 5, 8, 10, 11, 12, 13, 14, 15, 16, 18, 20, 21, 24, 28, 29, 30})
146146

147147
testUserSuccess(&SearchUserOptions{Keyword: "user1", OrderBy: "id ASC", ListOptions: ListOptions{Page: 1}, IsActive: util.OptionalBoolTrue},
148148
[]int64{1, 10, 11, 12, 13, 14, 15, 16, 18})

‎routers/routes/macaron.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,8 @@ func RegisterMacaronRoutes(m *macaron.Macaron) {
199199
}, ignSignIn)
200200
m.Combo("/install", routers.InstallInit).Get(routers.Install).
201201
Post(bindIgnErr(auth.InstallForm{}), routers.InstallPost)
202-
m.Get("/^:type(issues|pulls)$", reqSignIn, user.Issues)
202+
m.Get("/issues", reqSignIn, user.Issues)
203+
m.Get("/pulls", reqSignIn, user.Pulls)
203204
m.Get("/milestones", reqSignIn, reqMilestonesDashboardPageEnabled, user.Milestones)
204205

205206
// ***** START: User *****
@@ -447,8 +448,10 @@ func RegisterMacaronRoutes(m *macaron.Macaron) {
447448
m.Group("/:org", func() {
448449
m.Get("/dashboard", user.Dashboard)
449450
m.Get("/dashboard/:team", user.Dashboard)
450-
m.Get("/^:type(issues|pulls)$", user.Issues)
451-
m.Get("/^:type(issues|pulls)$/:team", user.Issues)
451+
m.Get("/issues", user.Issues)
452+
m.Get("/issues/:team", user.Issues)
453+
m.Get("/pulls", user.Pulls)
454+
m.Get("/pulls/:team", user.Pulls)
452455
m.Get("/milestones", reqMilestonesDashboardPageEnabled, user.Milestones)
453456
m.Get("/milestones/:team", reqMilestonesDashboardPageEnabled, user.Milestones)
454457
m.Get("/members", org.Members)

0 commit comments

Comments
 (0)