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>
release/v1.15
Elena Neuschild 2021-01-13 05:19:17 +01:00 committed by GitHub
parent 81467e6f35
commit 564030336d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
14 changed files with 494 additions and 167 deletions

View File

@ -186,7 +186,7 @@ func TestAPISearchIssues(t *testing.T) {
req = NewRequest(t, "GET", link.String()) req = NewRequest(t, "GET", link.String())
resp = session.MakeRequest(t, req, http.StatusOK) resp = session.MakeRequest(t, req, http.StatusOK)
DecodeJSON(t, resp, &apiIssues) 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 assert.Len(t, apiIssues, 10) //there are more but 10 is page item limit
query.Add("limit", "20") query.Add("limit", "20")
@ -194,7 +194,7 @@ func TestAPISearchIssues(t *testing.T) {
req = NewRequest(t, "GET", link.String()) req = NewRequest(t, "GET", link.String())
resp = session.MakeRequest(t, req, http.StatusOK) resp = session.MakeRequest(t, req, http.StatusOK)
DecodeJSON(t, resp, &apiIssues) DecodeJSON(t, resp, &apiIssues)
assert.Len(t, apiIssues, 12) assert.Len(t, apiIssues, 14)
query = url.Values{"assigned": {"true"}, "state": {"all"}} query = url.Values{"assigned": {"true"}, "state": {"all"}}
link.RawQuery = query.Encode() link.RawQuery = query.Encode()

View File

@ -77,9 +77,9 @@ func TestAPISearchRepo(t *testing.T) {
expectedResults expectedResults
}{ }{
{name: "RepositoriesMax50", requestURL: "/api/v1/repos/search?limit=50&private=false", expectedResults: expectedResults{ {name: "RepositoriesMax50", requestURL: "/api/v1/repos/search?limit=50&private=false", expectedResults: expectedResults{
nil: {count: 28}, nil: {count: 30},
user: {count: 28}, user: {count: 30},
user2: {count: 28}}, user2: {count: 30}},
}, },
{name: "RepositoriesMax10", requestURL: "/api/v1/repos/search?limit=10&private=false", expectedResults: expectedResults{ {name: "RepositoriesMax10", requestURL: "/api/v1/repos/search?limit=10&private=false", expectedResults: expectedResults{
nil: {count: 10}, nil: {count: 10},

View File

@ -147,3 +147,28 @@
is_pull: true is_pull: true
created_unix: 1602935696 created_unix: 1602935696
updated_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

View File

@ -532,3 +532,15 @@
repo_id: 3 repo_id: 3
type: 8 type: 8
created_unix: 946684810 created_unix: 946684810
-
id: 78
repo_id: 50
type: 2
created_unix: 946684810
-
id: 79
repo_id: 51
type: 2
created_unix: 946684810

View File

@ -4,6 +4,7 @@
owner_name: user2 owner_name: user2
lower_name: repo1 lower_name: repo1
name: repo1 name: repo1
is_archived: false
is_empty: false is_empty: false
is_private: false is_private: false
num_issues: 2 num_issues: 2
@ -23,6 +24,7 @@
owner_name: user2 owner_name: user2
lower_name: repo2 lower_name: repo2
name: repo2 name: repo2
is_archived: false
is_private: true is_private: true
num_issues: 2 num_issues: 2
num_closed_issues: 1 num_closed_issues: 1
@ -693,3 +695,43 @@
num_issues: 0 num_issues: 0
is_mirror: false is_mirror: false
status: 0 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

View File

@ -507,3 +507,21 @@
avatar_email: user29@example.com avatar_email: user29@example.com
num_repos: 0 num_repos: 0
is_active: true 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

View File

@ -1104,6 +1104,7 @@ type IssuesOptions struct {
UpdatedBeforeUnix int64 UpdatedBeforeUnix int64
// prioritize issues from this repo // prioritize issues from this repo
PriorityRepoID int64 PriorityRepoID int64
IsArchived util.OptionalBool
} }
// sortIssuesSession sort an issues-related session based on the provided // 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) 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 { if opts.LabelIDs != nil {
for i, labelID := range opts.LabelIDs { for i, labelID := range opts.LabelIDs {
if labelID > 0 { if labelID > 0 {
@ -1501,6 +1506,7 @@ type UserIssueStatsOptions struct {
IsPull bool IsPull bool
IsClosed bool IsClosed bool
IssueIDs []int64 IssueIDs []int64
IsArchived util.OptionalBool
LabelIDs []int64 LabelIDs []int64
} }
@ -1524,6 +1530,10 @@ func GetUserIssueStats(opts UserIssueStatsOptions) (*IssueStats, error) {
s.Join("INNER", "issue_label", "issue_label.issue_id = issue.id"). s.Join("INNER", "issue_label", "issue_label.issue_id = issue.id").
In("issue_label.label_id", opts.LabelIDs) 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 return s
} }

View File

@ -753,7 +753,7 @@ type accessibleReposEnv struct {
orderBy SearchOrderBy 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. // that are accessible to the specified user.
func (org *User) AccessibleReposEnv(userID int64) (AccessibleReposEnvironment, error) { func (org *User) AccessibleReposEnv(userID int64) (AccessibleReposEnvironment, error) {
return org.accessibleReposEnv(x, userID) return org.accessibleReposEnv(x, userID)

View File

@ -187,10 +187,10 @@ func TestSearchRepository(t *testing.T) {
count: 14}, count: 14},
{name: "AllPublic/PublicRepositoriesOfUserIncludingCollaborative", {name: "AllPublic/PublicRepositoriesOfUserIncludingCollaborative",
opts: &SearchRepoOptions{ListOptions: ListOptions{Page: 1, PageSize: 10}, OwnerID: 15, AllPublic: true, Template: util.OptionalBoolFalse}, opts: &SearchRepoOptions{ListOptions: ListOptions{Page: 1, PageSize: 10}, OwnerID: 15, AllPublic: true, Template: util.OptionalBoolFalse},
count: 26}, count: 28},
{name: "AllPublic/PublicAndPrivateRepositoriesOfUserIncludingCollaborative", {name: "AllPublic/PublicAndPrivateRepositoriesOfUserIncludingCollaborative",
opts: &SearchRepoOptions{ListOptions: ListOptions{Page: 1, PageSize: 10}, OwnerID: 15, Private: true, AllPublic: true, AllLimited: true, Template: util.OptionalBoolFalse}, 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", {name: "AllPublic/PublicAndPrivateRepositoriesOfUserIncludingCollaborativeByName",
opts: &SearchRepoOptions{Keyword: "test", ListOptions: ListOptions{Page: 1, PageSize: 10}, OwnerID: 15, Private: true, AllPublic: true}, opts: &SearchRepoOptions{Keyword: "test", ListOptions: ListOptions{Page: 1, PageSize: 10}, OwnerID: 15, Private: true, AllPublic: true},
count: 15}, count: 15},
@ -199,7 +199,7 @@ func TestSearchRepository(t *testing.T) {
count: 13}, count: 13},
{name: "AllPublic/PublicRepositoriesOfOrganization", {name: "AllPublic/PublicRepositoriesOfOrganization",
opts: &SearchRepoOptions{ListOptions: ListOptions{Page: 1, PageSize: 10}, OwnerID: 17, AllPublic: true, Collaborate: util.OptionalBoolFalse, Template: util.OptionalBoolFalse}, opts: &SearchRepoOptions{ListOptions: ListOptions{Page: 1, PageSize: 10}, OwnerID: 17, AllPublic: true, Collaborate: util.OptionalBoolFalse, Template: util.OptionalBoolFalse},
count: 26}, count: 28},
{name: "AllTemplates", {name: "AllTemplates",
opts: &SearchRepoOptions{ListOptions: ListOptions{Page: 1, PageSize: 10}, Template: util.OptionalBoolTrue}, opts: &SearchRepoOptions{ListOptions: ListOptions{Page: 1, PageSize: 10}, Template: util.OptionalBoolTrue},
count: 2}, count: 2},

View File

@ -503,6 +503,23 @@ func (u *User) GetRepositoryIDs(units ...UnitType) ([]int64, error) {
return ids, sess.Where("owner_id = ?", u.ID).Find(&ids) 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 // GetOrgRepositoryIDs returns repositories IDs where user's team owned and has unittypes
// Caller shall check that units is not globally disabled // Caller shall check that units is not globally disabled
func (u *User) GetOrgRepositoryIDs(units ...UnitType) ([]int64, error) { func (u *User) GetOrgRepositoryIDs(units ...UnitType) ([]int64, error) {
@ -524,6 +541,28 @@ func (u *User) GetOrgRepositoryIDs(units ...UnitType) ([]int64, error) {
return ids, nil 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 // GetAccessRepoIDs returns all repositories IDs where user's or user is a team member organizations
// Caller shall check that units is not globally disabled // Caller shall check that units is not globally disabled
func (u *User) GetAccessRepoIDs(units ...UnitType) ([]int64, error) { 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 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. // GetMirrorRepositories returns mirror repositories that user owns, including private repositories.
func (u *User) GetMirrorRepositories() ([]*Repository, error) { func (u *User) GetMirrorRepositories() ([]*Repository, error) {
return GetUserMirrorRepositories(u.ID) return GetUserMirrorRepositories(u.ID)

View File

@ -136,13 +136,13 @@ func TestSearchUsers(t *testing.T) {
} }
testUserSuccess(&SearchUserOptions{OrderBy: "id ASC", ListOptions: ListOptions{Page: 1}}, 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}, testUserSuccess(&SearchUserOptions{ListOptions: ListOptions{Page: 1}, IsActive: util.OptionalBoolFalse},
[]int64{9}) []int64{9})
testUserSuccess(&SearchUserOptions{OrderBy: "id ASC", ListOptions: ListOptions{Page: 1}, IsActive: util.OptionalBoolTrue}, 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}, testUserSuccess(&SearchUserOptions{Keyword: "user1", OrderBy: "id ASC", ListOptions: ListOptions{Page: 1}, IsActive: util.OptionalBoolTrue},
[]int64{1, 10, 11, 12, 13, 14, 15, 16, 18}) []int64{1, 10, 11, 12, 13, 14, 15, 16, 18})

View File

@ -199,7 +199,8 @@ func RegisterMacaronRoutes(m *macaron.Macaron) {
}, ignSignIn) }, ignSignIn)
m.Combo("/install", routers.InstallInit).Get(routers.Install). m.Combo("/install", routers.InstallInit).Get(routers.Install).
Post(bindIgnErr(auth.InstallForm{}), routers.InstallPost) 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) m.Get("/milestones", reqSignIn, reqMilestonesDashboardPageEnabled, user.Milestones)
// ***** START: User ***** // ***** START: User *****
@ -447,8 +448,10 @@ func RegisterMacaronRoutes(m *macaron.Macaron) {
m.Group("/:org", func() { m.Group("/:org", func() {
m.Get("/dashboard", user.Dashboard) m.Get("/dashboard", user.Dashboard)
m.Get("/dashboard/:team", user.Dashboard) m.Get("/dashboard/:team", user.Dashboard)
m.Get("/^:type(issues|pulls)$", user.Issues) m.Get("/issues", user.Issues)
m.Get("/^:type(issues|pulls)$/:team", 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", reqMilestonesDashboardPageEnabled, user.Milestones)
m.Get("/milestones/:team", reqMilestonesDashboardPageEnabled, user.Milestones) m.Get("/milestones/:team", reqMilestonesDashboardPageEnabled, user.Milestones)
m.Get("/members", org.Members) m.Get("/members", org.Members)

View File

@ -37,7 +37,7 @@ const (
tplProfile base.TplName = "user/profile" 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 { func getDashboardContextUser(ctx *context.Context) *models.User {
ctxUser := ctx.User ctxUser := ctx.User
orgName := ctx.Params(":org") orgName := ctx.Params(":org")
@ -324,46 +324,66 @@ func Milestones(ctx *context.Context) {
ctx.HTML(200, tplMilestones) 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 // Regexp for repos query
var issueReposQueryPattern = regexp.MustCompile(`^\[\d+(,\d+)*,?\]$`) var issueReposQueryPattern = regexp.MustCompile(`^\[\d+(,\d+)*,?\]$`)
// Issues render the user issues page func buildIssueOverview(ctx *context.Context, unitType models.UnitType) {
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
}
ctx.Data["Title"] = ctx.Tr("pull_requests") // ----------------------------------------------------
ctx.Data["PageIsPulls"] = true // Determine user; can be either user or organization.
unitType = models.UnitTypePullRequests // Return with NotFound or ServerError if unsuccessful.
} 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
}
ctxUser := getDashboardContextUser(ctx) ctxUser := getDashboardContextUser(ctx)
if ctx.Written() { if ctx.Written() {
return return
} }
// Organization does not have view type and filter mode.
var ( var (
viewType string viewType string
sortType = ctx.Query("sort") sortType = ctx.Query("sort")
filterMode = models.FilterModeAll 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") viewType = ctx.Query("type")
switch viewType { switch viewType {
case "assigned": case "assigned":
@ -377,72 +397,30 @@ func Issues(ctx *context.Context) {
viewType = "your_repositories" viewType = "your_repositories"
} }
page := ctx.QueryInt("page") // --------------------------------------------------------------------------
if page <= 1 { // Build opts (IssuesOptions), which contains filter information.
page = 1 // 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
reposQuery := ctx.Query("repos") // - Count Issues by repo
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}
}
isPullList := unitType == models.UnitTypePullRequests
opts := &models.IssuesOptions{ opts := &models.IssuesOptions{
IsPull: util.OptionalBoolOf(isPullList), IsPull: util.OptionalBoolOf(isPullList),
SortType: sortType, 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 { switch filterMode {
@ -460,47 +438,56 @@ func Issues(ctx *context.Context) {
opts.RepoIDs = userRepoIDs opts.RepoIDs = userRepoIDs
} }
var forceEmpty bool // keyword holds the search term entered into the search field.
var issueIDsFromSearch []int64 keyword := strings.Trim(ctx.Query("q"), " ")
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
}
}
ctx.Data["Keyword"] = keyword 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) 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 { if !forceEmpty {
counts, err = models.CountIssuesByRepo(opts) issueCountByRepo, err = models.CountIssuesByRepo(opts)
if err != nil { if err != nil {
ctx.ServerError("CountIssuesByRepo", err) ctx.ServerError("CountIssuesByRepo", err)
return 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.Page = page
opts.PageSize = setting.UI.IssuePagingNum opts.PageSize = setting.UI.IssuePagingNum
// Get IDs for labels (a filter option for issues/pulls).
// Required for IssuesOptions.
var labelIDs []int64 var labelIDs []int64
selectLabels := ctx.Query("labels") selectedLabels := ctx.Query("labels")
if len(selectLabels) > 0 && selectLabels != "0" { if len(selectedLabels) > 0 && selectedLabels != "0" {
labelIDs, err = base.StringsToInt64s(strings.Split(selectLabels, ",")) labelIDs, err = base.StringsToInt64s(strings.Split(selectedLabels, ","))
if err != nil { if err != nil {
ctx.ServerError("StringsToInt64s", err) ctx.ServerError("StringsToInt64s", err)
return return
@ -508,10 +495,19 @@ func Issues(ctx *context.Context) {
} }
opts.LabelIDs = labelIDs 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 { if len(repoIDs) > 0 {
opts.RepoIDs = repoIDs 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 var issues []*models.Issue
if !forceEmpty { if !forceEmpty {
issues, err = models.Issues(opts) issues, err = models.Issues(opts)
@ -523,40 +519,22 @@ func Issues(ctx *context.Context) {
issues = []*models.Issue{} 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 { if err != nil {
ctx.ServerError("ApprovalCounts", err) if models.IsErrRepoNotExist(err) {
ctx.NotFound("GetRepositoryByID", err)
return
}
ctx.ServerError("repoIDMap", err)
return return
} }
showReposMap := make(map[int64]*models.Repository, len(counts)) // a RepositoryList
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)
}
}
}
showRepos := models.RepositoryListOfMap(showReposMap) showRepos := models.RepositoryListOfMap(showReposMap)
sort.Sort(showRepos) sort.Sort(showRepos)
if err = showRepos.LoadAttributes(); err != nil { if err = showRepos.LoadAttributes(); err != nil {
@ -564,6 +542,7 @@ func Issues(ctx *context.Context) {
return return
} }
// maps pull request IDs to their CommitStatus. Will be posted to ctx.Data.
var commitStatus = make(map[int64]*models.CommitStatus, len(issues)) var commitStatus = make(map[int64]*models.CommitStatus, len(issues))
for _, issue := range issues { for _, issue := range issues {
issue.Repo = showReposMap[issue.RepoID] issue.Repo = showReposMap[issue.RepoID]
@ -574,12 +553,17 @@ func Issues(ctx *context.Context) {
} }
} }
// -------------------------------
// Fill stats to post to ctx.Data.
// -------------------------------
userIssueStatsOpts := models.UserIssueStatsOptions{ userIssueStatsOpts := models.UserIssueStatsOptions{
UserID: ctx.User.ID, UserID: ctx.User.ID,
UserRepoIDs: userRepoIDs, UserRepoIDs: userRepoIDs,
FilterMode: filterMode, FilterMode: filterMode,
IsPull: isPullList, IsPull: isPullList,
IsClosed: isShowClosed, IsClosed: isShowClosed,
IsArchived: util.OptionalBoolFalse,
LabelIDs: opts.LabelIDs, LabelIDs: opts.LabelIDs,
} }
if len(repoIDs) > 0 { if len(repoIDs) > 0 {
@ -603,6 +587,7 @@ func Issues(ctx *context.Context) {
IsPull: isPullList, IsPull: isPullList,
IsClosed: isShowClosed, IsClosed: isShowClosed,
IssueIDs: issueIDsFromSearch, IssueIDs: issueIDsFromSearch,
IsArchived: util.OptionalBoolFalse,
LabelIDs: opts.LabelIDs, LabelIDs: opts.LabelIDs,
} }
if len(repoIDs) > 0 { if len(repoIDs) > 0 {
@ -628,6 +613,7 @@ func Issues(ctx *context.Context) {
IsPull: isPullList, IsPull: isPullList,
IsClosed: isShowClosed, IsClosed: isShowClosed,
IssueIDs: issueIDsFromSearch, IssueIDs: issueIDsFromSearch,
IsArchived: util.OptionalBoolFalse,
LabelIDs: opts.LabelIDs, LabelIDs: opts.LabelIDs,
} }
if ctxUser.IsOrganization() { if ctxUser.IsOrganization() {
@ -642,20 +628,28 @@ func Issues(ctx *context.Context) {
allIssueStats = &models.IssueStats{} allIssueStats = &models.IssueStats{}
} }
// Will be posted to ctx.Data.
var shownIssues int var shownIssues int
var totalIssues int
if !isShowClosed { if !isShowClosed {
shownIssues = int(shownIssueStats.OpenCount) shownIssues = int(shownIssueStats.OpenCount)
totalIssues = int(allIssueStats.OpenCount) ctx.Data["TotalIssueCount"] = int(allIssueStats.OpenCount)
} else { } else {
shownIssues = int(shownIssueStats.ClosedCount) shownIssues = int(shownIssueStats.ClosedCount)
totalIssues = int(allIssueStats.ClosedCount) ctx.Data["TotalIssueCount"] = int(allIssueStats.ClosedCount)
} }
ctx.Data["IsShowClosed"] = isShowClosed
ctx.Data["IssueRefEndNames"], ctx.Data["IssueRefURLs"] = ctx.Data["IssueRefEndNames"], ctx.Data["IssueRefURLs"] =
issue_service.GetRefEndNamesAndURLs(issues, ctx.Query("RepoLink")) issue_service.GetRefEndNamesAndURLs(issues, ctx.Query("RepoLink"))
ctx.Data["Issues"] = issues 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 { ctx.Data["ApprovalCounts"] = func(issueID int64, typ string) int64 {
counts, ok := approvalCounts[issueID] counts, ok := approvalCounts[issueID]
if !ok || len(counts) == 0 { if !ok || len(counts) == 0 {
@ -676,15 +670,14 @@ func Issues(ctx *context.Context) {
} }
ctx.Data["CommitStatus"] = commitStatus ctx.Data["CommitStatus"] = commitStatus
ctx.Data["Repos"] = showRepos ctx.Data["Repos"] = showRepos
ctx.Data["Counts"] = counts ctx.Data["Counts"] = issueCountByRepo
ctx.Data["IssueStats"] = userIssueStats ctx.Data["IssueStats"] = userIssueStats
ctx.Data["ShownIssueStats"] = shownIssueStats ctx.Data["ShownIssueStats"] = shownIssueStats
ctx.Data["ViewType"] = viewType ctx.Data["ViewType"] = viewType
ctx.Data["SortType"] = sortType ctx.Data["SortType"] = sortType
ctx.Data["RepoIDs"] = repoIDs ctx.Data["RepoIDs"] = repoIDs
ctx.Data["IsShowClosed"] = isShowClosed ctx.Data["IsShowClosed"] = isShowClosed
ctx.Data["TotalIssueCount"] = totalIssues ctx.Data["SelectLabels"] = selectedLabels
ctx.Data["SelectLabels"] = selectLabels
if isShowClosed { if isShowClosed {
ctx.Data["State"] = "closed" ctx.Data["State"] = "closed"
@ -711,6 +704,131 @@ func Issues(ctx *context.Context) {
ctx.HTML(200, tplIssues) 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 // ShowSSHKeys output all the ssh keys of user by uid
func ShowSSHKeys(ctx *context.Context, uid int64) { func ShowSSHKeys(ctx *context.Context, uid int64) {
keys, err := models.ListPublicKeys(uid, models.ListOptions{}) keys, err := models.ListPublicKeys(uid, models.ListOptions{})

View File

@ -15,13 +15,46 @@ import (
"github.com/stretchr/testify/assert" "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) { func TestIssues(t *testing.T) {
setting.UI.IssuePagingNum = 1 setting.UI.IssuePagingNum = 1
assert.NoError(t, models.LoadFixtures()) assert.NoError(t, models.LoadFixtures())
ctx := test.MockContext(t, "issues") ctx := test.MockContext(t, "issues")
test.LoadUser(t, ctx, 2) test.LoadUser(t, ctx, 2)
ctx.SetParams(":type", "issues")
ctx.Req.Form.Set("state", "closed") ctx.Req.Form.Set("state", "closed")
Issues(ctx) Issues(ctx)
assert.EqualValues(t, http.StatusOK, ctx.Resp.Status()) assert.EqualValues(t, http.StatusOK, ctx.Resp.Status())
@ -32,6 +65,19 @@ func TestIssues(t *testing.T) {
assert.Len(t, ctx.Data["Repos"], 2) 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) { func TestMilestones(t *testing.T) {
setting.UI.IssuePagingNum = 1 setting.UI.IssuePagingNum = 1
assert.NoError(t, models.LoadFixtures()) assert.NoError(t, models.LoadFixtures())