diff options
| -rw-r--r-- | integrations/api_issue_test.go | 4 | ||||
| -rw-r--r-- | integrations/api_repo_test.go | 6 | ||||
| -rw-r--r-- | models/fixtures/issue.yml | 25 | ||||
| -rw-r--r-- | models/fixtures/repo_unit.yml | 12 | ||||
| -rw-r--r-- | models/fixtures/repository.yml | 42 | ||||
| -rw-r--r-- | models/fixtures/user.yml | 18 | ||||
| -rw-r--r-- | models/issue.go | 10 | ||||
| -rw-r--r-- | models/org.go | 2 | ||||
| -rw-r--r-- | models/repo_list_test.go | 6 | ||||
| -rw-r--r-- | models/user.go | 53 | ||||
| -rw-r--r-- | models/user_test.go | 4 | ||||
| -rw-r--r-- | routers/routes/macaron.go | 9 | ||||
| -rw-r--r-- | routers/user/home.go | 414 | ||||
| -rw-r--r-- | routers/user/home_test.go | 48 |
14 files changed, 490 insertions, 163 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) } -// Regexp for repos query -var issueReposQueryPattern = regexp.MustCompile(`^\[\d+(,\d+)*,?\]$`) +// 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 render the user issues page +// Issues renders the user's issues overview 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 - } + 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("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 + buildIssueOverview(ctx, models.UnitTypeIssues) +} - ctx.Data["Title"] = ctx.Tr("issues") - ctx.Data["PageIsIssues"] = true - } +// Regexp for repos query +var issueReposQueryPattern = regexp.MustCompile(`^\[\d+(,\d+)*,?\]$`) + +func buildIssueOverview(ctx *context.Context, unitType models.UnitType) { + + // ---------------------------------------------------- + // 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 - } + // -------------------------------------------------------------------------- + // 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 + // -------------------------------------------------------------------------- - 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") - } + isPullList := unitType == models.UnitTypePullRequests + opts := &models.IssuesOptions{ + IsPull: util.OptionalBoolOf(isPullList), + SortType: sortType, + IsArchived: util.OptionalBoolFalse, } - 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 - } + // Get repository IDs where User/Org/Team has access. + var team *models.Team + if ctx.Org != nil { + team = ctx.Org.Team } - if len(userRepoIDs) == 0 { - userRepoIDs = []int64{-1} - } - - opts := &models.IssuesOptions{ - IsPull: util.OptionalBoolOf(isPullList), - SortType: sortType, + 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"), " ") + // keyword holds the search term entered into the search field. + keyword := strings.Trim(ctx.Query("q"), " ") + ctx.Data["Keyword"] = keyword - 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 - } + // 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 } - ctx.Data["Keyword"] = keyword + // 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() - if err != nil { - ctx.ServerError("ApprovalCounts", 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] + // ---------------------------------- + // Add repository pointers to Issues. + // ---------------------------------- - // 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) - } + // showReposMap maps repository IDs to their Repository pointers. + showReposMap, err := repoIDMap(ctxUser, issueCountByRepo, unitType) + if err != nil { + if models.IsErrRepoNotExist(err) { + ctx.NotFound("GetRepositoryByID", err) + return } + ctx.ServerError("repoIDMap", err) + return } + // 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()) |