Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issues overview should not show issues from archived repos #13220

Merged
merged 42 commits into from
Jan 13, 2021
Merged
Show file tree
Hide file tree
Changes from 33 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
91f207d
Add lots of comments to user.Issues()
eneuschild Oct 20, 2020
89bb53d
Answered some questions from comments
eneuschild Oct 20, 2020
c2f7acc
fix typo in comment
eneuschild Oct 20, 2020
19221f0
Refac user.Issues(): add func repoIDs
eneuschild Oct 21, 2020
355a90f
Refac user.Issues(): add func userRepoIDs
eneuschild Oct 22, 2020
b2e6d26
Refac user.Issues(): add func issueIDsFromSearch
eneuschild Oct 22, 2020
628c8b1
Merge branch 'master' into issue_13171
eneuschild Oct 26, 2020
9673406
Refac user.Issues(): improve error handling
eneuschild Oct 26, 2020
dbc22f7
Refac user.Issues(): add inline documentation and move variable decla…
eneuschild Oct 26, 2020
fdb2237
Refac user.Issues(): add func repoIDMap
eneuschild Oct 26, 2020
5e8d323
Refac user.Issues(): cleanup
eneuschild Oct 26, 2020
2317cac
Merge branch 'master' into issue_13171
eneuschild Oct 27, 2020
0294079
Refac: Separate Issues from Pulls during routing
eneuschild Oct 27, 2020
afb9af7
fix typo in comment
eneuschild Oct 27, 2020
4ff651f
Merge branch 'master' into issue_13171
eneuschild Oct 28, 2020
4cd5f17
Merge branch 'master' into issue_13171
eneuschild Oct 30, 2020
8359624
Merge branch 'master' into issue_13171
eneuschild Nov 3, 2020
7a0d75d
Merge branch 'master' into issue_13171
eneuschild Nov 4, 2020
faec896
Adapt Unittests to Refactoring
eneuschild Nov 10, 2020
5e4ca72
Issue13171: Issue and PR Overviews now ignore archived Repositories
eneuschild Nov 10, 2020
43848f9
Merge branch 'master' into issue_13171
eneuschild Nov 25, 2020
1be19ed
changed some verbatim SQL conditions to builder.Eq
eneuschild Nov 26, 2020
f637176
Merge branch 'master' into issue_13171
eneuschild Nov 27, 2020
d4021ad
models/issue.go: use OptionalBool properly
eneuschild Nov 30, 2020
d23d8cd
Use IsArchived rather than ExcludeArchivedRepos
eneuschild Nov 30, 2020
c44e9ca
Merge branch 'master' into issue_13171
eneuschild Dec 17, 2020
322ac00
Merge branch 'master' into issue_13171
eneuschild Dec 29, 2020
806ba05
fixed broken test after merge
Jan 4, 2021
5c16d41
Merge branch 'master' into issue_13171
Jan 4, 2021
93d4d0d
added nil check
Jan 5, 2021
cf25512
Added Unit Test securing Issue 13171 fix
Jan 5, 2021
a08e8b3
Merge branch 'master' into issue_13171
6543 Jan 5, 2021
497301a
Merge branch 'master' into issue_13171
6543 Jan 5, 2021
9de7cb4
Improved IsArchived filtering in issue.GetUserIssueStats
eneuschild Jan 6, 2021
ec6412c
Merge branch 'master' into issue_13171
eneuschild Jan 6, 2021
7019096
Merge branch 'master' into issue_13171
eneuschild Jan 7, 2021
4b1d4a8
Merge branch 'master' into issue_13171
eneuschild Jan 8, 2021
2992f54
Merge branch 'master' into issue_13171
eneuschild Jan 10, 2021
29df59a
Removed unused func
eneuschild Jan 11, 2021
3c6db2d
Added grouping to avoid returning duplicate repo IDs
eneuschild Jan 11, 2021
a24e2c7
Merge branch 'master' into issue_13171
techknowlogick Jan 13, 2021
4201024
Merge branch 'master' into issue_13171
techknowlogick Jan 13, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions integrations/api_issue_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,15 +186,15 @@ func TestAPISearchIssues(t *testing.T) {
req = NewRequest(t, "GET", link.String())
resp = session.MakeRequest(t, req, http.StatusOK)
DecodeJSON(t, resp, &apiIssues)
assert.EqualValues(t, "12", resp.Header().Get("X-Total-Count"))
assert.EqualValues(t, "14", resp.Header().Get("X-Total-Count"))
assert.Len(t, apiIssues, 10) //there are more but 10 is page item limit

query.Add("limit", "20")
link.RawQuery = query.Encode()
req = NewRequest(t, "GET", link.String())
resp = session.MakeRequest(t, req, http.StatusOK)
DecodeJSON(t, resp, &apiIssues)
assert.Len(t, apiIssues, 12)
assert.Len(t, apiIssues, 14)

query = url.Values{"assigned": {"true"}, "state": {"all"}}
link.RawQuery = query.Encode()
Expand Down
6 changes: 3 additions & 3 deletions integrations/api_repo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,9 @@ func TestAPISearchRepo(t *testing.T) {
expectedResults
}{
{name: "RepositoriesMax50", requestURL: "/api/v1/repos/search?limit=50&private=false", expectedResults: expectedResults{
nil: {count: 28},
user: {count: 28},
user2: {count: 28}},
nil: {count: 30},
user: {count: 30},
user2: {count: 30}},
},
{name: "RepositoriesMax10", requestURL: "/api/v1/repos/search?limit=10&private=false", expectedResults: expectedResults{
nil: {count: 10},
Expand Down
25 changes: 25 additions & 0 deletions models/fixtures/issue.yml
Original file line number Diff line number Diff line change
Expand Up @@ -147,3 +147,28 @@
is_pull: true
created_unix: 1602935696
updated_unix: 1602935696


-
id: 13
repo_id: 50
index: 0
poster_id: 2
name: issue in active repo
content: we'll be testing github issue 13171 with this.
is_closed: false
is_pull: false
created_unix: 1602935696
updated_unix: 1602935696

-
id: 14
repo_id: 51
index: 0
poster_id: 2
name: issue in archived repo
content: we'll be testing github issue 13171 with this.
is_closed: false
is_pull: false
created_unix: 1602935696
updated_unix: 1602935696
12 changes: 12 additions & 0 deletions models/fixtures/repo_unit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -532,3 +532,15 @@
repo_id: 3
type: 8
created_unix: 946684810

-
id: 78
repo_id: 50
type: 2
created_unix: 946684810

-
id: 79
repo_id: 51
type: 2
created_unix: 946684810
42 changes: 42 additions & 0 deletions models/fixtures/repository.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
owner_name: user2
lower_name: repo1
name: repo1
is_archived: false
is_empty: false
is_private: false
num_issues: 2
Expand All @@ -23,6 +24,7 @@
owner_name: user2
lower_name: repo2
name: repo2
is_archived: false
is_private: true
num_issues: 2
num_closed_issues: 1
Expand Down Expand Up @@ -693,3 +695,43 @@
num_issues: 0
is_mirror: false
status: 0

-
id: 50
owner_id: 30
owner_name: user30
lower_name: repo50
name: repo50
is_archived: false
is_empty: false
is_private: false
num_issues: 1
num_closed_issues: 0
num_pulls: 0
num_closed_pulls: 0
num_milestones: 0
num_closed_milestones: 0
num_watches: 0
num_projects: 0
num_closed_projects: 0
status: 0

-
id: 51
owner_id: 30
owner_name: user30
lower_name: repo51
name: repo51
is_archived: true
is_empty: false
is_private: false
num_issues: 1
num_closed_issues: 0
num_pulls: 0
num_closed_pulls: 0
num_milestones: 0
num_closed_milestones: 0
num_watches: 0
num_projects: 0
num_closed_projects: 0
status: 0
18 changes: 18 additions & 0 deletions models/fixtures/user.yml
Original file line number Diff line number Diff line change
Expand Up @@ -507,3 +507,21 @@
avatar_email: user29@example.com
num_repos: 0
is_active: true


-
id: 30
lower_name: user30
name: user30
full_name: User Thirty
email: user30@example.com
passwd_hash_algo: argon2
passwd: a3d5fcd92bae586c2e3dbe72daea7a0d27833a8d0227aa1704f4bbd775c1f3b03535b76dd93b0d4d8d22a519dca47df1547b # password
type: 0 # individual
salt: ZogKvWdyEx
is_admin: false
is_restricted: true
avatar: avatar29
avatar_email: user30@example.com
num_repos: 2
is_active: true
14 changes: 14 additions & 0 deletions models/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -1104,6 +1104,7 @@ type IssuesOptions struct {
UpdatedBeforeUnix int64
// prioritize issues from this repo
PriorityRepoID int64
IsArchived util.OptionalBool
}

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

if opts.IsArchived != util.OptionalBoolNone {
sess.Join("INNER", "repository", "issue.repo_id = repository.id").And(builder.Eq{"repository.is_archived": opts.IsArchived.IsTrue()})
}

if opts.LabelIDs != nil {
for i, labelID := range opts.LabelIDs {
if labelID > 0 {
Expand Down Expand Up @@ -1501,6 +1506,7 @@ type UserIssueStatsOptions struct {
IsPull bool
IsClosed bool
IssueIDs []int64
IsArchived util.OptionalBool
LabelIDs []int64
}

Expand All @@ -1517,6 +1523,14 @@ func GetUserIssueStats(opts UserIssueStatsOptions) (*IssueStats, error) {
if len(opts.IssueIDs) > 0 {
cond = cond.And(builder.In("issue.id", opts.IssueIDs))
}
if opts.IsArchived != util.OptionalBoolNone {
relevantRepoIDs := []int64{}
r := []*Repository{}
s := x.Table("repository").Where(builder.Eq{"is_archived": opts.IsArchived.IsTrue()})
s.Select("id").Find(&relevantRepoIDs)
s.Find(&r)
lunny marked this conversation as resolved.
Show resolved Hide resolved
cond = cond.And(builder.In("issue.repo_id", relevantRepoIDs))
}

sess := func(cond builder.Cond) *xorm.Session {
s := x.Where(cond)
Expand Down
21 changes: 20 additions & 1 deletion models/org.go
Original file line number Diff line number Diff line change
Expand Up @@ -737,6 +737,7 @@ func (org *User) GetUserTeams(userID int64) ([]*Team, error) {
type AccessibleReposEnvironment interface {
CountRepos() (int64, error)
RepoIDs(page, pageSize int) ([]int64, error)
lunny marked this conversation as resolved.
Show resolved Hide resolved
ActiveRepoIDs(page, pageSize int) ([]int64, error)
Repos(page, pageSize int) ([]*Repository, error)
MirrorRepos() ([]*Repository, error)
AddKeyword(keyword string)
Expand All @@ -753,7 +754,7 @@ type accessibleReposEnv struct {
orderBy SearchOrderBy
}

// AccessibleReposEnv an AccessibleReposEnvironment for the repositories in `org`
// AccessibleReposEnv builds an AccessibleReposEnvironment for the repositories in `org`
// that are accessible to the specified user.
func (org *User) AccessibleReposEnv(userID int64) (AccessibleReposEnvironment, error) {
return org.accessibleReposEnv(x, userID)
Expand Down Expand Up @@ -844,6 +845,24 @@ func (env *accessibleReposEnv) RepoIDs(page, pageSize int) ([]int64, error) {
Find(&repoIDs)
}

func (env *accessibleReposEnv) ActiveRepoIDs(page, pageSize int) ([]int64, error) {
if page <= 0 {
page = 1
}

repoIDs := make([]int64, 0, pageSize)
return repoIDs, env.e.
Table("repository").
Join("INNER", "team_repo", "`team_repo`.repo_id=`repository`.id").
Where(env.cond()).
Where(builder.Eq{"is_archived": false}).
GroupBy("`repository`.id,`repository`."+strings.Fields(string(env.orderBy))[0]).
OrderBy(string(env.orderBy)).
Limit(pageSize, (page-1)*pageSize).
Cols("`repository`.id").
Find(&repoIDs)
}

func (env *accessibleReposEnv) Repos(page, pageSize int) ([]*Repository, error) {
repoIDs, err := env.RepoIDs(page, pageSize)
if err != nil {
Expand Down
6 changes: 3 additions & 3 deletions models/repo_list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,10 +187,10 @@ func TestSearchRepository(t *testing.T) {
count: 14},
{name: "AllPublic/PublicRepositoriesOfUserIncludingCollaborative",
opts: &SearchRepoOptions{ListOptions: ListOptions{Page: 1, PageSize: 10}, OwnerID: 15, AllPublic: true, Template: util.OptionalBoolFalse},
count: 26},
count: 28},
{name: "AllPublic/PublicAndPrivateRepositoriesOfUserIncludingCollaborative",
opts: &SearchRepoOptions{ListOptions: ListOptions{Page: 1, PageSize: 10}, OwnerID: 15, Private: true, AllPublic: true, AllLimited: true, Template: util.OptionalBoolFalse},
count: 31},
count: 33},
{name: "AllPublic/PublicAndPrivateRepositoriesOfUserIncludingCollaborativeByName",
opts: &SearchRepoOptions{Keyword: "test", ListOptions: ListOptions{Page: 1, PageSize: 10}, OwnerID: 15, Private: true, AllPublic: true},
count: 15},
Expand All @@ -199,7 +199,7 @@ func TestSearchRepository(t *testing.T) {
count: 13},
{name: "AllPublic/PublicRepositoriesOfOrganization",
opts: &SearchRepoOptions{ListOptions: ListOptions{Page: 1, PageSize: 10}, OwnerID: 17, AllPublic: true, Collaborate: util.OptionalBoolFalse, Template: util.OptionalBoolFalse},
count: 26},
count: 28},
{name: "AllTemplates",
opts: &SearchRepoOptions{ListOptions: ListOptions{Page: 1, PageSize: 10}, Template: util.OptionalBoolTrue},
count: 2},
Expand Down
53 changes: 53 additions & 0 deletions models/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -490,6 +490,23 @@ func (u *User) GetRepositoryIDs(units ...UnitType) ([]int64, error) {
return ids, sess.Where("owner_id = ?", u.ID).Find(&ids)
}

// GetActiveRepositoryIDs returns non-archived repositories IDs where user owned and has unittypes
// Caller shall check that units is not globally disabled
func (u *User) GetActiveRepositoryIDs(units ...UnitType) ([]int64, error) {
eneuschild marked this conversation as resolved.
Show resolved Hide resolved
var ids []int64

sess := x.Table("repository").Cols("repository.id")

if len(units) > 0 {
sess = sess.Join("INNER", "repo_unit", "repository.id = repo_unit.repo_id")
sess = sess.In("repo_unit.type", units)
}

sess.Where(builder.Eq{"is_archived": false})

return ids, sess.Where("owner_id = ?", u.ID).Find(&ids)
}

// GetOrgRepositoryIDs returns repositories IDs where user's team owned and has unittypes
// Caller shall check that units is not globally disabled
func (u *User) GetOrgRepositoryIDs(units ...UnitType) ([]int64, error) {
Expand All @@ -511,6 +528,28 @@ func (u *User) GetOrgRepositoryIDs(units ...UnitType) ([]int64, error) {
return ids, nil
}

// GetActiveOrgRepositoryIDs returns non-archived repositories IDs where user's team owned and has unittypes
// Caller shall check that units is not globally disabled
func (u *User) GetActiveOrgRepositoryIDs(units ...UnitType) ([]int64, error) {
var ids []int64

if err := x.Table("repository").
Cols("repository.id").
Join("INNER", "team_user", "repository.owner_id = team_user.org_id").
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).
Where("team_user.uid = ?", u.ID).
Where(builder.Eq{"is_archived": false}).
GroupBy("repository.id").Find(&ids); err != nil {
return nil, err
}

if len(units) > 0 {
return FilterOutRepoIdsWithoutUnitAccess(u, ids, units...)
}

return ids, nil
}

// GetAccessRepoIDs returns all repositories IDs where user's or user is a team member organizations
// Caller shall check that units is not globally disabled
func (u *User) GetAccessRepoIDs(units ...UnitType) ([]int64, error) {
Expand All @@ -525,6 +564,20 @@ func (u *User) GetAccessRepoIDs(units ...UnitType) ([]int64, error) {
return append(ids, ids2...), nil
}

// GetActiveAccessRepoIDs returns all non-archived repositories IDs where user's or user is a team member organizations
// Caller shall check that units is not globally disabled
func (u *User) GetActiveAccessRepoIDs(units ...UnitType) ([]int64, error) {
ids, err := u.GetActiveRepositoryIDs(units...)
if err != nil {
return nil, err
}
ids2, err := u.GetActiveOrgRepositoryIDs(units...)
if err != nil {
return nil, err
}
return append(ids, ids2...), nil
}

// GetMirrorRepositories returns mirror repositories that user owns, including private repositories.
func (u *User) GetMirrorRepositories() ([]*Repository, error) {
return GetUserMirrorRepositories(u.ID)
Expand Down
4 changes: 2 additions & 2 deletions models/user_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,13 +136,13 @@ func TestSearchUsers(t *testing.T) {
}

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

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

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

testUserSuccess(&SearchUserOptions{Keyword: "user1", OrderBy: "id ASC", ListOptions: ListOptions{Page: 1}, IsActive: util.OptionalBoolTrue},
[]int64{1, 10, 11, 12, 13, 14, 15, 16, 18})
Expand Down
9 changes: 6 additions & 3 deletions routers/routes/macaron.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,8 @@ func RegisterMacaronRoutes(m *macaron.Macaron) {
}, ignSignIn)
m.Combo("/install", routers.InstallInit).Get(routers.Install).
Post(bindIgnErr(auth.InstallForm{}), routers.InstallPost)
m.Get("/^:type(issues|pulls)$", reqSignIn, user.Issues)
m.Get("/issues", reqSignIn, user.Issues)
m.Get("/pulls", reqSignIn, user.Pulls)
m.Get("/milestones", reqSignIn, reqMilestonesDashboardPageEnabled, user.Milestones)

// ***** START: User *****
Expand Down Expand Up @@ -445,8 +446,10 @@ func RegisterMacaronRoutes(m *macaron.Macaron) {
m.Group("/:org", func() {
m.Get("/dashboard", user.Dashboard)
m.Get("/dashboard/:team", user.Dashboard)
m.Get("/^:type(issues|pulls)$", user.Issues)
m.Get("/^:type(issues|pulls)$/:team", user.Issues)
m.Get("/issues", user.Issues)
m.Get("/issues/:team", user.Issues)
m.Get("/pulls", user.Pulls)
m.Get("/pulls/:team", user.Pulls)
m.Get("/milestones", reqMilestonesDashboardPageEnabled, user.Milestones)
m.Get("/milestones/:team", reqMilestonesDashboardPageEnabled, user.Milestones)
m.Get("/members", org.Members)
Expand Down
Loading