From 564030336dbfe1227ec458ecdedc0cfabdd4c1cc Mon Sep 17 00:00:00 2001
From: Elena Neuschild <eneuschild@gmail.com>
Date: Wed, 13 Jan 2021 05:19:17 +0100
Subject: [PATCH] Issues overview should not show issues from archived repos
(#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>
---
integrations/api_issue_test.go | 4 +-
integrations/api_repo_test.go | 6 +-
models/fixtures/issue.yml | 25 ++
models/fixtures/repo_unit.yml | 12 +
models/fixtures/repository.yml | 42 ++++
models/fixtures/user.yml | 18 ++
models/issue.go | 10 +
models/org.go | 2 +-
models/repo_list_test.go | 6 +-
models/user.go | 53 +++++
models/user_test.go | 4 +-
routers/routes/macaron.go | 9 +-
routers/user/home.go | 422 +++++++++++++++++++++------------
routers/user/home_test.go | 48 +++-
14 files changed, 494 insertions(+), 167 deletions(-)
diff --git a/integrations/api_issue_test.go b/integrations/api_issue_test.go
index 81e5c44873..f932f79fee 100644
--- a/integrations/api_issue_test.go
+++ b/integrations/api_issue_test.go
@@ -186,7 +186,7 @@ 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")
@@ -194,7 +194,7 @@ func TestAPISearchIssues(t *testing.T) {
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()
diff --git a/integrations/api_repo_test.go b/integrations/api_repo_test.go
index 8294a01773..b6d41fe1e0 100644
--- a/integrations/api_repo_test.go
+++ b/integrations/api_repo_test.go
@@ -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},
diff --git a/models/fixtures/issue.yml b/models/fixtures/issue.yml
index 3e836bf5d1..31df00d9e6 100644
--- a/models/fixtures/issue.yml
+++ b/models/fixtures/issue.yml
@@ -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
diff --git a/models/fixtures/repo_unit.yml b/models/fixtures/repo_unit.yml
index 726abf9af9..59ab618340 100644
--- a/models/fixtures/repo_unit.yml
+++ b/models/fixtures/repo_unit.yml
@@ -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
diff --git a/models/fixtures/repository.yml b/models/fixtures/repository.yml
index 7898538e2c..952408b0c2 100644
--- a/models/fixtures/repository.yml
+++ b/models/fixtures/repository.yml
@@ -4,6 +4,7 @@
owner_name: user2
lower_name: repo1
name: repo1
+ is_archived: false
is_empty: false
is_private: false
num_issues: 2
@@ -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
@@ -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
\ No newline at end of file
diff --git a/models/fixtures/user.yml b/models/fixtures/user.yml
index 655129580c..d903a7942f 100644
--- a/models/fixtures/user.yml
+++ b/models/fixtures/user.yml
@@ -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
diff --git a/models/issue.go b/models/issue.go
index 18d01d57d4..b517f334c4 100644
--- a/models/issue.go
+++ b/models/issue.go
@@ -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
@@ -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 {
@@ -1501,6 +1506,7 @@ type UserIssueStatsOptions struct {
IsPull bool
IsClosed bool
IssueIDs []int64
+ IsArchived util.OptionalBool
LabelIDs []int64
}
@@ -1524,6 +1530,10 @@ func GetUserIssueStats(opts UserIssueStatsOptions) (*IssueStats, error) {
s.Join("INNER", "issue_label", "issue_label.issue_id = issue.id").
In("issue_label.label_id", opts.LabelIDs)
}
+ if opts.IsArchived != util.OptionalBoolNone {
+ s.Join("INNER", "repository", "issue.repo_id = repository.id").
+ And(builder.Eq{"repository.is_archived": opts.IsArchived.IsTrue()})
+ }
return s
}
diff --git a/models/org.go b/models/org.go
index c93a30fd77..f45c9af7a7 100644
--- a/models/org.go
+++ b/models/org.go
@@ -753,7 +753,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)
diff --git a/models/repo_list_test.go b/models/repo_list_test.go
index 97047b7ffa..37af9d598d 100644
--- a/models/repo_list_test.go
+++ b/models/repo_list_test.go
@@ -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},
@@ -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},
diff --git a/models/user.go b/models/user.go
index dbd2372fcf..de12b804fd 100644
--- a/models/user.go
+++ b/models/user.go
@@ -503,6 +503,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) {
+ 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).GroupBy("repository.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) {
@@ -524,6 +541,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) {
@@ -538,6 +577,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)
diff --git a/models/user_test.go b/models/user_test.go
index 69cb21b975..ac40015969 100644
--- a/models/user_test.go
+++ b/models/user_test.go
@@ -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})
diff --git a/routers/routes/macaron.go b/routers/routes/macaron.go
index d54cd580d3..66c39b7f82 100644
--- a/routers/routes/macaron.go
+++ b/routers/routes/macaron.go
@@ -199,7 +199,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 *****
@@ -447,8 +448,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)
diff --git a/routers/user/home.go b/routers/user/home.go
index 2b59b971aa..3c27bbe2a8 100644
--- a/routers/user/home.go
+++ b/routers/user/home.go
@@ -37,7 +37,7 @@ const (
tplProfile base.TplName = "user/profile"
)
-// getDashboardContextUser finds out dashboard is viewing as which context user.
+// getDashboardContextUser finds out which context user dashboard is being viewed as .
func getDashboardContextUser(ctx *context.Context) *models.User {
ctxUser := ctx.User
orgName := ctx.Params(":org")
@@ -324,46 +324,66 @@ func Milestones(ctx *context.Context) {
ctx.HTML(200, tplMilestones)
}
+// Pulls renders the user's pull request overview page
+func Pulls(ctx *context.Context) {
+ if models.UnitTypePullRequests.UnitGlobalDisabled() {
+ log.Debug("Pull request overview page not available as it is globally disabled.")
+ ctx.Status(404)
+ return
+ }
+
+ ctx.Data["Title"] = ctx.Tr("pull_requests")
+ ctx.Data["PageIsPulls"] = true
+ buildIssueOverview(ctx, models.UnitTypePullRequests)
+}
+
+// Issues renders the user's issues overview page
+func Issues(ctx *context.Context) {
+ if models.UnitTypeIssues.UnitGlobalDisabled() {
+ log.Debug("Issues overview page not available as it is globally disabled.")
+ ctx.Status(404)
+ return
+ }
+
+ ctx.Data["Title"] = ctx.Tr("issues")
+ ctx.Data["PageIsIssues"] = true
+ buildIssueOverview(ctx, models.UnitTypeIssues)
+}
+
// Regexp for repos query
var issueReposQueryPattern = regexp.MustCompile(`^\[\d+(,\d+)*,?\]$`)
-// Issues render the user issues page
-func Issues(ctx *context.Context) {
- isPullList := ctx.Params(":type") == "pulls"
- unitType := models.UnitTypeIssues
- if isPullList {
- if models.UnitTypePullRequests.UnitGlobalDisabled() {
- log.Debug("Pull request overview page not available as it is globally disabled.")
- ctx.Status(404)
- return
- }
+func buildIssueOverview(ctx *context.Context, unitType models.UnitType) {
- ctx.Data["Title"] = ctx.Tr("pull_requests")
- ctx.Data["PageIsPulls"] = true
- unitType = models.UnitTypePullRequests
- } else {
- if models.UnitTypeIssues.UnitGlobalDisabled() {
- log.Debug("Issues overview page not available as it is globally disabled.")
- ctx.Status(404)
- return
- }
-
- ctx.Data["Title"] = ctx.Tr("issues")
- ctx.Data["PageIsIssues"] = true
- }
+ // ----------------------------------------------------
+ // Determine user; can be either user or organization.
+ // Return with NotFound or ServerError if unsuccessful.
+ // ----------------------------------------------------
ctxUser := getDashboardContextUser(ctx)
if ctx.Written() {
return
}
- // Organization does not have view type and filter mode.
var (
viewType string
sortType = ctx.Query("sort")
filterMode = models.FilterModeAll
)
+ // --------------------------------------------------------------------------------
+ // Distinguish User from Organization.
+ // Org:
+ // - Remember pre-determined viewType string for later. Will be posted to ctx.Data.
+ // Organization does not have view type and filter mode.
+ // User:
+ // - Use ctx.Query("type") to determine filterMode.
+ // The type is set when clicking for example "assigned to me" on the overview page.
+ // - Remember either this or a fallback. Will be posted to ctx.Data.
+ // --------------------------------------------------------------------------------
+
+ // TODO: distinguish during routing
+
viewType = ctx.Query("type")
switch viewType {
case "assigned":
@@ -377,72 +397,30 @@ func Issues(ctx *context.Context) {
viewType = "your_repositories"
}
- page := ctx.QueryInt("page")
- if page <= 1 {
- page = 1
- }
-
- reposQuery := ctx.Query("repos")
- var repoIDs []int64
- if len(reposQuery) != 0 {
- if issueReposQueryPattern.MatchString(reposQuery) {
- // remove "[" and "]" from string
- reposQuery = reposQuery[1 : len(reposQuery)-1]
- //for each ID (delimiter ",") add to int to repoIDs
- for _, rID := range strings.Split(reposQuery, ",") {
- // Ensure nonempty string entries
- if rID != "" && rID != "0" {
- rIDint64, err := strconv.ParseInt(rID, 10, 64)
- if err == nil {
- repoIDs = append(repoIDs, rIDint64)
- }
- }
- }
- } else {
- log.Warn("issueReposQueryPattern not match with query")
- }
- }
-
- isShowClosed := ctx.Query("state") == "closed"
-
- // Get repositories.
- var err error
- var userRepoIDs []int64
- if ctxUser.IsOrganization() {
- var env models.AccessibleReposEnvironment
- if ctx.Org.Team != nil {
- env = ctxUser.AccessibleTeamReposEnv(ctx.Org.Team)
- } else {
- env, err = ctxUser.AccessibleReposEnv(ctx.User.ID)
- if err != nil {
- ctx.ServerError("AccessibleReposEnv", err)
- return
- }
- }
- userRepoIDs, err = env.RepoIDs(1, ctxUser.NumRepos)
- if err != nil {
- ctx.ServerError("env.RepoIDs", err)
- return
- }
- userRepoIDs, err = models.FilterOutRepoIdsWithoutUnitAccess(ctx.User, userRepoIDs, unitType)
- if err != nil {
- ctx.ServerError("FilterOutRepoIdsWithoutUnitAccess", err)
- return
- }
- } else {
- userRepoIDs, err = ctxUser.GetAccessRepoIDs(unitType)
- if err != nil {
- ctx.ServerError("ctxUser.GetAccessRepoIDs", err)
- return
- }
- }
- if len(userRepoIDs) == 0 {
- userRepoIDs = []int64{-1}
- }
+ // --------------------------------------------------------------------------
+ // Build opts (IssuesOptions), which contains filter information.
+ // Will eventually be used to retrieve issues relevant for the overview page.
+ // Note: Non-final states of opts are used in-between, namely for:
+ // - Keyword search
+ // - Count Issues by repo
+ // --------------------------------------------------------------------------
+ isPullList := unitType == models.UnitTypePullRequests
opts := &models.IssuesOptions{
- IsPull: util.OptionalBoolOf(isPullList),
- SortType: sortType,
+ IsPull: util.OptionalBoolOf(isPullList),
+ SortType: sortType,
+ IsArchived: util.OptionalBoolFalse,
+ }
+
+ // Get repository IDs where User/Org/Team has access.
+ var team *models.Team
+ if ctx.Org != nil {
+ team = ctx.Org.Team
+ }
+ userRepoIDs, err := getActiveUserRepoIDs(ctxUser, team, unitType)
+ if err != nil {
+ ctx.ServerError("userRepoIDs", err)
+ return
}
switch filterMode {
@@ -460,47 +438,56 @@ func Issues(ctx *context.Context) {
opts.RepoIDs = userRepoIDs
}
- var forceEmpty bool
- var issueIDsFromSearch []int64
- var keyword = strings.Trim(ctx.Query("q"), " ")
-
- if len(keyword) > 0 {
- searchRepoIDs, err := models.GetRepoIDsForIssuesOptions(opts, ctxUser)
- if err != nil {
- ctx.ServerError("GetRepoIDsForIssuesOptions", err)
- return
- }
- issueIDsFromSearch, err = issue_indexer.SearchIssuesByKeyword(searchRepoIDs, keyword)
- if err != nil {
- ctx.ServerError("SearchIssuesByKeyword", err)
- return
- }
- if len(issueIDsFromSearch) > 0 {
- opts.IssueIDs = issueIDsFromSearch
- } else {
- forceEmpty = true
- }
- }
-
+ // keyword holds the search term entered into the search field.
+ keyword := strings.Trim(ctx.Query("q"), " ")
ctx.Data["Keyword"] = keyword
+ // Execute keyword search for issues.
+ // USING NON-FINAL STATE OF opts FOR A QUERY.
+ issueIDsFromSearch, err := issueIDsFromSearch(ctxUser, keyword, opts)
+ if err != nil {
+ ctx.ServerError("issueIDsFromSearch", err)
+ return
+ }
+
+ // Ensure no issues are returned if a keyword was provided that didn't match any issues.
+ var forceEmpty bool
+
+ if len(issueIDsFromSearch) > 0 {
+ opts.IssueIDs = issueIDsFromSearch
+ } else if len(keyword) > 0 {
+ forceEmpty = true
+ }
+
+ // Educated guess: Do or don't show closed issues.
+ isShowClosed := ctx.Query("state") == "closed"
opts.IsClosed = util.OptionalBoolOf(isShowClosed)
- var counts map[int64]int64
+ // Filter repos and count issues in them. Count will be used later.
+ // USING NON-FINAL STATE OF opts FOR A QUERY.
+ var issueCountByRepo map[int64]int64
if !forceEmpty {
- counts, err = models.CountIssuesByRepo(opts)
+ issueCountByRepo, err = models.CountIssuesByRepo(opts)
if err != nil {
ctx.ServerError("CountIssuesByRepo", err)
return
}
}
+ // Make sure page number is at least 1. Will be posted to ctx.Data.
+ page := ctx.QueryInt("page")
+ if page <= 1 {
+ page = 1
+ }
opts.Page = page
opts.PageSize = setting.UI.IssuePagingNum
+
+ // Get IDs for labels (a filter option for issues/pulls).
+ // Required for IssuesOptions.
var labelIDs []int64
- selectLabels := ctx.Query("labels")
- if len(selectLabels) > 0 && selectLabels != "0" {
- labelIDs, err = base.StringsToInt64s(strings.Split(selectLabels, ","))
+ selectedLabels := ctx.Query("labels")
+ if len(selectedLabels) > 0 && selectedLabels != "0" {
+ labelIDs, err = base.StringsToInt64s(strings.Split(selectedLabels, ","))
if err != nil {
ctx.ServerError("StringsToInt64s", err)
return
@@ -508,10 +495,19 @@ func Issues(ctx *context.Context) {
}
opts.LabelIDs = labelIDs
+ // Parse ctx.Query("repos") and remember matched repo IDs for later.
+ // Gets set when clicking filters on the issues overview page.
+ repoIDs := getRepoIDs(ctx.Query("repos"))
if len(repoIDs) > 0 {
opts.RepoIDs = repoIDs
}
+ // ------------------------------
+ // Get issues as defined by opts.
+ // ------------------------------
+
+ // Slice of Issues that will be displayed on the overview page
+ // USING FINAL STATE OF opts FOR A QUERY.
var issues []*models.Issue
if !forceEmpty {
issues, err = models.Issues(opts)
@@ -523,40 +519,22 @@ func Issues(ctx *context.Context) {
issues = []*models.Issue{}
}
- approvalCounts, err := models.IssueList(issues).GetApprovalCounts()
+ // ----------------------------------
+ // Add repository pointers to Issues.
+ // ----------------------------------
+
+ // showReposMap maps repository IDs to their Repository pointers.
+ showReposMap, err := repoIDMap(ctxUser, issueCountByRepo, unitType)
if err != nil {
- ctx.ServerError("ApprovalCounts", err)
+ if models.IsErrRepoNotExist(err) {
+ ctx.NotFound("GetRepositoryByID", err)
+ return
+ }
+ ctx.ServerError("repoIDMap", err)
return
}
- showReposMap := make(map[int64]*models.Repository, len(counts))
- for repoID := range counts {
- if repoID > 0 {
- if _, ok := showReposMap[repoID]; !ok {
- repo, err := models.GetRepositoryByID(repoID)
- if models.IsErrRepoNotExist(err) {
- ctx.NotFound("GetRepositoryByID", err)
- return
- } else if err != nil {
- ctx.ServerError("GetRepositoryByID", fmt.Errorf("[%d]%v", repoID, err))
- return
- }
- showReposMap[repoID] = repo
- }
- repo := showReposMap[repoID]
-
- // Check if user has access to given repository.
- perm, err := models.GetUserRepoPermission(repo, ctxUser)
- if err != nil {
- ctx.ServerError("GetUserRepoPermission", fmt.Errorf("[%d]%v", repoID, err))
- return
- }
- if !perm.CanRead(unitType) {
- log.Debug("User created Issues in Repository which they no longer have access to: [%d]", repoID)
- }
- }
- }
-
+ // a RepositoryList
showRepos := models.RepositoryListOfMap(showReposMap)
sort.Sort(showRepos)
if err = showRepos.LoadAttributes(); err != nil {
@@ -564,6 +542,7 @@ func Issues(ctx *context.Context) {
return
}
+ // maps pull request IDs to their CommitStatus. Will be posted to ctx.Data.
var commitStatus = make(map[int64]*models.CommitStatus, len(issues))
for _, issue := range issues {
issue.Repo = showReposMap[issue.RepoID]
@@ -574,12 +553,17 @@ func Issues(ctx *context.Context) {
}
}
+ // -------------------------------
+ // Fill stats to post to ctx.Data.
+ // -------------------------------
+
userIssueStatsOpts := models.UserIssueStatsOptions{
UserID: ctx.User.ID,
UserRepoIDs: userRepoIDs,
FilterMode: filterMode,
IsPull: isPullList,
IsClosed: isShowClosed,
+ IsArchived: util.OptionalBoolFalse,
LabelIDs: opts.LabelIDs,
}
if len(repoIDs) > 0 {
@@ -603,6 +587,7 @@ func Issues(ctx *context.Context) {
IsPull: isPullList,
IsClosed: isShowClosed,
IssueIDs: issueIDsFromSearch,
+ IsArchived: util.OptionalBoolFalse,
LabelIDs: opts.LabelIDs,
}
if len(repoIDs) > 0 {
@@ -628,6 +613,7 @@ func Issues(ctx *context.Context) {
IsPull: isPullList,
IsClosed: isShowClosed,
IssueIDs: issueIDsFromSearch,
+ IsArchived: util.OptionalBoolFalse,
LabelIDs: opts.LabelIDs,
}
if ctxUser.IsOrganization() {
@@ -642,20 +628,28 @@ func Issues(ctx *context.Context) {
allIssueStats = &models.IssueStats{}
}
+ // Will be posted to ctx.Data.
var shownIssues int
- var totalIssues int
if !isShowClosed {
shownIssues = int(shownIssueStats.OpenCount)
- totalIssues = int(allIssueStats.OpenCount)
+ ctx.Data["TotalIssueCount"] = int(allIssueStats.OpenCount)
} else {
shownIssues = int(shownIssueStats.ClosedCount)
- totalIssues = int(allIssueStats.ClosedCount)
+ ctx.Data["TotalIssueCount"] = int(allIssueStats.ClosedCount)
}
+ ctx.Data["IsShowClosed"] = isShowClosed
+
ctx.Data["IssueRefEndNames"], ctx.Data["IssueRefURLs"] =
issue_service.GetRefEndNamesAndURLs(issues, ctx.Query("RepoLink"))
ctx.Data["Issues"] = issues
+
+ approvalCounts, err := models.IssueList(issues).GetApprovalCounts()
+ if err != nil {
+ ctx.ServerError("ApprovalCounts", err)
+ return
+ }
ctx.Data["ApprovalCounts"] = func(issueID int64, typ string) int64 {
counts, ok := approvalCounts[issueID]
if !ok || len(counts) == 0 {
@@ -676,15 +670,14 @@ func Issues(ctx *context.Context) {
}
ctx.Data["CommitStatus"] = commitStatus
ctx.Data["Repos"] = showRepos
- ctx.Data["Counts"] = counts
+ ctx.Data["Counts"] = issueCountByRepo
ctx.Data["IssueStats"] = userIssueStats
ctx.Data["ShownIssueStats"] = shownIssueStats
ctx.Data["ViewType"] = viewType
ctx.Data["SortType"] = sortType
ctx.Data["RepoIDs"] = repoIDs
ctx.Data["IsShowClosed"] = isShowClosed
- ctx.Data["TotalIssueCount"] = totalIssues
- ctx.Data["SelectLabels"] = selectLabels
+ ctx.Data["SelectLabels"] = selectedLabels
if isShowClosed {
ctx.Data["State"] = "closed"
@@ -711,6 +704,131 @@ func Issues(ctx *context.Context) {
ctx.HTML(200, tplIssues)
}
+func getRepoIDs(reposQuery string) []int64 {
+ if len(reposQuery) == 0 {
+ return []int64{}
+ }
+ if !issueReposQueryPattern.MatchString(reposQuery) {
+ log.Warn("issueReposQueryPattern does not match query")
+ return []int64{}
+ }
+
+ var repoIDs []int64
+ // remove "[" and "]" from string
+ reposQuery = reposQuery[1 : len(reposQuery)-1]
+ //for each ID (delimiter ",") add to int to repoIDs
+ for _, rID := range strings.Split(reposQuery, ",") {
+ // Ensure nonempty string entries
+ if rID != "" && rID != "0" {
+ rIDint64, err := strconv.ParseInt(rID, 10, 64)
+ if err == nil {
+ repoIDs = append(repoIDs, rIDint64)
+ }
+ }
+ }
+
+ return repoIDs
+}
+
+func getActiveUserRepoIDs(ctxUser *models.User, team *models.Team, unitType models.UnitType) ([]int64, error) {
+ var userRepoIDs []int64
+ var err error
+
+ if ctxUser.IsOrganization() {
+ userRepoIDs, err = getActiveTeamOrOrgRepoIds(ctxUser, team, unitType)
+ if err != nil {
+ return nil, fmt.Errorf("orgRepoIds: %v", err)
+ }
+ } else {
+ userRepoIDs, err = ctxUser.GetActiveAccessRepoIDs(unitType)
+ if err != nil {
+ return nil, fmt.Errorf("ctxUser.GetAccessRepoIDs: %v", err)
+ }
+ }
+
+ if len(userRepoIDs) == 0 {
+ userRepoIDs = []int64{-1}
+ }
+
+ return userRepoIDs, nil
+}
+
+// getActiveTeamOrOrgRepoIds gets RepoIDs for ctxUser as Organization.
+// Should be called if and only if ctxUser.IsOrganization == true.
+func getActiveTeamOrOrgRepoIds(ctxUser *models.User, team *models.Team, unitType models.UnitType) ([]int64, error) {
+ var orgRepoIDs []int64
+ var err error
+ var env models.AccessibleReposEnvironment
+
+ if team != nil {
+ env = ctxUser.AccessibleTeamReposEnv(team)
+ if err != nil {
+ return nil, fmt.Errorf("AccessibleTeamReposEnv: %v", err)
+ }
+ } else {
+ env, err = ctxUser.AccessibleReposEnv(ctxUser.ID)
+ if err != nil {
+ return nil, fmt.Errorf("AccessibleReposEnv: %v", err)
+ }
+ }
+ orgRepoIDs, err = env.RepoIDs(1, ctxUser.NumRepos)
+ if err != nil {
+ return nil, fmt.Errorf("env.RepoIDs: %v", err)
+ }
+ orgRepoIDs, err = models.FilterOutRepoIdsWithoutUnitAccess(ctxUser, orgRepoIDs, unitType)
+ if err != nil {
+ return nil, fmt.Errorf("FilterOutRepoIdsWithoutUnitAccess: %v", err)
+ }
+
+ return orgRepoIDs, nil
+}
+
+func issueIDsFromSearch(ctxUser *models.User, keyword string, opts *models.IssuesOptions) ([]int64, error) {
+ if len(keyword) == 0 {
+ return []int64{}, nil
+ }
+
+ searchRepoIDs, err := models.GetRepoIDsForIssuesOptions(opts, ctxUser)
+ if err != nil {
+ return nil, fmt.Errorf("GetRepoIDsForIssuesOptions: %v", err)
+ }
+ issueIDsFromSearch, err := issue_indexer.SearchIssuesByKeyword(searchRepoIDs, keyword)
+ if err != nil {
+ return nil, fmt.Errorf("SearchIssuesByKeyword: %v", err)
+ }
+
+ return issueIDsFromSearch, nil
+}
+
+func repoIDMap(ctxUser *models.User, issueCountByRepo map[int64]int64, unitType models.UnitType) (map[int64]*models.Repository, error) {
+ repoByID := make(map[int64]*models.Repository, len(issueCountByRepo))
+ for id := range issueCountByRepo {
+ if id <= 0 {
+ continue
+ }
+ if _, ok := repoByID[id]; !ok {
+ repo, err := models.GetRepositoryByID(id)
+ if models.IsErrRepoNotExist(err) {
+ return nil, err
+ } else if err != nil {
+ return nil, fmt.Errorf("GetRepositoryByID: [%d]%v", id, err)
+ }
+ repoByID[id] = repo
+ }
+ repo := repoByID[id]
+
+ // Check if user has access to given repository.
+ perm, err := models.GetUserRepoPermission(repo, ctxUser)
+ if err != nil {
+ return nil, fmt.Errorf("GetUserRepoPermission: [%d]%v", id, err)
+ }
+ if !perm.CanRead(unitType) {
+ log.Debug("User created Issues in Repository which they no longer have access to: [%d]", id)
+ }
+ }
+ return repoByID, nil
+}
+
// ShowSSHKeys output all the ssh keys of user by uid
func ShowSSHKeys(ctx *context.Context, uid int64) {
keys, err := models.ListPublicKeys(uid, models.ListOptions{})
diff --git a/routers/user/home_test.go b/routers/user/home_test.go
index ff48953d44..ecc02fd33a 100644
--- a/routers/user/home_test.go
+++ b/routers/user/home_test.go
@@ -15,13 +15,46 @@ import (
"github.com/stretchr/testify/assert"
)
+func TestArchivedIssues(t *testing.T) {
+ // Arrange
+ setting.UI.IssuePagingNum = 1
+ assert.NoError(t, models.LoadFixtures())
+
+ ctx := test.MockContext(t, "issues")
+ test.LoadUser(t, ctx, 30)
+ ctx.Req.Form.Set("state", "open")
+
+ // Assume: User 30 has access to two Repos with Issues, one of the Repos being archived.
+ repos, _, _ := models.GetUserRepositories(&models.SearchRepoOptions{Actor: ctx.User})
+ assert.Len(t, repos, 2)
+ IsArchived := make(map[int64]bool)
+ NumIssues := make(map[int64]int)
+ for _, repo := range repos {
+ IsArchived[repo.ID] = repo.IsArchived
+ NumIssues[repo.ID] = repo.NumIssues
+ }
+ assert.EqualValues(t, false, IsArchived[50])
+ assert.EqualValues(t, 1, NumIssues[50])
+ assert.EqualValues(t, true, IsArchived[51])
+ assert.EqualValues(t, 1, NumIssues[51])
+
+ // Act
+ Issues(ctx)
+
+ // Assert: One Issue (ID 30) from one Repo (ID 50) is retrieved, while nothing from archived Repo 51 is retrieved
+ assert.EqualValues(t, http.StatusOK, ctx.Resp.Status())
+
+ assert.EqualValues(t, map[int64]int64{50: 1}, ctx.Data["Counts"])
+ assert.Len(t, ctx.Data["Issues"], 1)
+ assert.Len(t, ctx.Data["Repos"], 1)
+}
+
func TestIssues(t *testing.T) {
setting.UI.IssuePagingNum = 1
assert.NoError(t, models.LoadFixtures())
ctx := test.MockContext(t, "issues")
test.LoadUser(t, ctx, 2)
- ctx.SetParams(":type", "issues")
ctx.Req.Form.Set("state", "closed")
Issues(ctx)
assert.EqualValues(t, http.StatusOK, ctx.Resp.Status())
@@ -32,6 +65,19 @@ func TestIssues(t *testing.T) {
assert.Len(t, ctx.Data["Repos"], 2)
}
+func TestPulls(t *testing.T) {
+ setting.UI.IssuePagingNum = 20
+ assert.NoError(t, models.LoadFixtures())
+
+ ctx := test.MockContext(t, "pulls")
+ test.LoadUser(t, ctx, 2)
+ ctx.Req.Form.Set("state", "open")
+ Pulls(ctx)
+ assert.EqualValues(t, http.StatusOK, ctx.Resp.Status())
+
+ assert.Len(t, ctx.Data["Issues"], 3)
+}
+
func TestMilestones(t *testing.T) {
setting.UI.IssuePagingNum = 1
assert.NoError(t, models.LoadFixtures())