From e6ef75204bed1fc5438d1e0aefb62838c29d4829 Mon Sep 17 00:00:00 2001 From: hgaiser Date: Tue, 27 Dec 2016 11:54:24 +0100 Subject: [PATCH] Fix assigned/created issues in dashboard. (#3560) * Fix assigned/created issues in dashboard. * Use GetUserIssueStats for getting all Dashboard stats. * Use gofmt to format the file properly. * Replace &Issue{} with new(Issue). * Check if user has access to given repository. * Remove unnecessary filtering of issues. * Return 404 error if invalid repository is given. * Use correct number of issues in paginater. --- models/issue.go | 62 +++++----- routers/repo/issue.go | 2 +- routers/user/home.go | 165 ++++++++++++++++----------- templates/user/dashboard/issues.tmpl | 8 +- 4 files changed, 142 insertions(+), 95 deletions(-) diff --git a/models/issue.go b/models/issue.go index 161ff1885..1081695a8 100644 --- a/models/issue.go +++ b/models/issue.go @@ -1069,7 +1069,7 @@ func updateIssueMentions(e Engine, issueID int64, mentions []string) error { // IssueStats represents issue statistic information. type IssueStats struct { OpenCount, ClosedCount int64 - AllCount int64 + YourRepositoriesCount int64 AssignCount int64 CreateCount int64 MentionCount int64 @@ -1077,7 +1077,7 @@ type IssueStats struct { // Filter modes. const ( - FM_ALL = iota + FM_YOUR_REPOSITORIES = iota FM_ASSIGN FM_CREATE FM_MENTION @@ -1129,38 +1129,38 @@ func GetIssueStats(opts *IssueStatsOptions) *IssueStats { } switch opts.FilterMode { - case FM_ALL, FM_ASSIGN: + case FM_YOUR_REPOSITORIES, FM_ASSIGN: stats.OpenCount, _ = countSession(opts). And("is_closed = ?", false). - Count(&Issue{}) + Count(new(Issue)) stats.ClosedCount, _ = countSession(opts). And("is_closed = ?", true). - Count(&Issue{}) + Count(new(Issue)) case FM_CREATE: stats.OpenCount, _ = countSession(opts). And("poster_id = ?", opts.UserID). And("is_closed = ?", false). - Count(&Issue{}) + Count(new(Issue)) stats.ClosedCount, _ = countSession(opts). And("poster_id = ?", opts.UserID). And("is_closed = ?", true). - Count(&Issue{}) + Count(new(Issue)) case FM_MENTION: stats.OpenCount, _ = countSession(opts). Join("INNER", "issue_user", "issue.id = issue_user.issue_id"). And("issue_user.uid = ?", opts.UserID). And("issue_user.is_mentioned = ?", true). And("issue.is_closed = ?", false). - Count(&Issue{}) + Count(new(Issue)) stats.ClosedCount, _ = countSession(opts). Join("INNER", "issue_user", "issue.id = issue_user.issue_id"). And("issue_user.uid = ?", opts.UserID). And("issue_user.is_mentioned = ?", true). And("issue.is_closed = ?", true). - Count(&Issue{}) + Count(new(Issue)) } return stats } @@ -1172,38 +1172,48 @@ func GetUserIssueStats(repoID, uid int64, repoIDs []int64, filterMode int, isPul countSession := func(isClosed, isPull bool, repoID int64, repoIDs []int64) *xorm.Session { sess := x.Where("issue.is_closed = ?", isClosed).And("issue.is_pull = ?", isPull) - if repoID > 0 || len(repoIDs) == 0 { + if repoID > 0 { sess.And("repo_id = ?", repoID) - } else { + } else if repoIDs != nil { sess.In("repo_id", repoIDs) } return sess } - stats.AssignCount, _ = countSession(false, isPull, repoID, repoIDs). + stats.AssignCount, _ = countSession(false, isPull, repoID, nil). And("assignee_id = ?", uid). - Count(&Issue{}) + Count(new(Issue)) - stats.CreateCount, _ = countSession(false, isPull, repoID, repoIDs). + stats.CreateCount, _ = countSession(false, isPull, repoID, nil). And("poster_id = ?", uid). - Count(&Issue{}) + Count(new(Issue)) - openCountSession := countSession(false, isPull, repoID, repoIDs) - closedCountSession := countSession(true, isPull, repoID, repoIDs) + stats.YourRepositoriesCount, _ = countSession(false, isPull, repoID, repoIDs). + Count(new(Issue)) switch filterMode { + case FM_YOUR_REPOSITORIES: + stats.OpenCount, _ = countSession(false, isPull, repoID, repoIDs). + Count(new(Issue)) + stats.ClosedCount, _ = countSession(true, isPull, repoID, repoIDs). + Count(new(Issue)) case FM_ASSIGN: - openCountSession.And("assignee_id = ?", uid) - closedCountSession.And("assignee_id = ?", uid) + stats.OpenCount, _ = countSession(false, isPull, repoID, nil). + And("assignee_id = ?", uid). + Count(new(Issue)) + stats.ClosedCount, _ = countSession(true, isPull, repoID, nil). + And("assignee_id = ?", uid). + Count(new(Issue)) case FM_CREATE: - openCountSession.And("poster_id = ?", uid) - closedCountSession.And("poster_id = ?", uid) + stats.OpenCount, _ = countSession(false, isPull, repoID, nil). + And("poster_id = ?", uid). + Count(new(Issue)) + stats.ClosedCount, _ = countSession(true, isPull, repoID, nil). + And("poster_id = ?", uid). + Count(new(Issue)) } - stats.OpenCount, _ = openCountSession.Count(&Issue{}) - stats.ClosedCount, _ = closedCountSession.Count(&Issue{}) - return stats } @@ -1229,8 +1239,8 @@ func GetRepoIssueStats(repoID, uid int64, filterMode int, isPull bool) (numOpen closedCountSession.And("poster_id = ?", uid) } - openResult, _ := openCountSession.Count(&Issue{}) - closedResult, _ := closedCountSession.Count(&Issue{}) + openResult, _ := openCountSession.Count(new(Issue)) + closedResult, _ := closedCountSession.Count(new(Issue)) return openResult, closedResult } diff --git a/routers/repo/issue.go b/routers/repo/issue.go index ad2d1fdfd..3f0f61950 100644 --- a/routers/repo/issue.go +++ b/routers/repo/issue.go @@ -121,7 +121,7 @@ func Issues(ctx *context.Context) { assigneeID = ctx.QueryInt64("assignee") posterID int64 ) - filterMode := models.FM_ALL + filterMode := models.FM_YOUR_REPOSITORIES switch viewType { case "assigned": filterMode = models.FM_ASSIGN diff --git a/routers/user/home.go b/routers/user/home.go index 83b4b2616..df178a936 100644 --- a/routers/user/home.go +++ b/routers/user/home.go @@ -172,35 +172,39 @@ func Issues(ctx *context.Context) { var ( viewType string sortType = ctx.Query("sort") - filterMode = models.FM_ALL - assigneeID int64 - posterID int64 + filterMode = models.FM_YOUR_REPOSITORIES ) if ctxUser.IsOrganization() { - viewType = "all" + viewType = "your_repositories" } else { viewType = ctx.Query("type") - types := []string{"assigned", "created_by"} + types := []string{"your_repositories", "assigned", "created_by"} if !com.IsSliceContainsStr(types, viewType) { - viewType = "all" + viewType = "your_repositories" } switch viewType { + case "your_repositories": + filterMode = models.FM_YOUR_REPOSITORIES case "assigned": filterMode = models.FM_ASSIGN - assigneeID = ctxUser.ID case "created_by": filterMode = models.FM_CREATE - posterID = ctxUser.ID } } + page := ctx.QueryInt("page") + if page <= 1 { + page = 1 + } + repoID := ctx.QueryInt64("repo") isShowClosed := ctx.Query("state") == "closed" // Get repositories. var err error var repos []*models.Repository + userRepoIDs := make([]int64, 0, len(repos)) if ctxUser.IsOrganization() { repos, _, err = ctxUser.GetUserRepositories(ctx.User.ID, 1, ctxUser.NumRepos) if err != nil { @@ -215,9 +219,6 @@ func Issues(ctx *context.Context) { repos = ctxUser.Repos } - allCount := 0 - repoIDs := make([]int64, 0, len(repos)) - showRepos := make([]*models.Repository, 0, len(repos)) for _, repo := range repos { if (isPullList && repo.NumPulls == 0) || (!isPullList && @@ -225,83 +226,119 @@ func Issues(ctx *context.Context) { continue } - repoIDs = append(repoIDs, repo.ID) + userRepoIDs = append(userRepoIDs, repo.ID) + } - if isPullList { - allCount += repo.NumOpenPulls - repo.NumOpenIssues = repo.NumOpenPulls - repo.NumClosedIssues = repo.NumClosedPulls - } else { - allCount += repo.NumOpenIssues + var issues []*models.Issue + switch filterMode { + case models.FM_YOUR_REPOSITORIES: + // Get all issues from repositories from this user. + issues, err = models.Issues(&models.IssuesOptions{ + RepoIDs: userRepoIDs, + RepoID: repoID, + Page: page, + IsClosed: isShowClosed, + IsPull: isPullList, + SortType: sortType, + }) + + case models.FM_ASSIGN: + // Get all issues assigned to this user. + issues, err = models.Issues(&models.IssuesOptions{ + RepoID: repoID, + AssigneeID: ctxUser.ID, + Page: page, + IsClosed: isShowClosed, + IsPull: isPullList, + SortType: sortType, + }) + + case models.FM_CREATE: + // Get all issues created by this user. + issues, err = models.Issues(&models.IssuesOptions{ + RepoID: repoID, + PosterID: ctxUser.ID, + Page: page, + IsClosed: isShowClosed, + IsPull: isPullList, + SortType: sortType, + }) + } + + if err != nil { + ctx.Handle(500, "Issues", err) + return + } + + showRepos := make([]*models.Repository, 0, len(issues)) + showReposSet := make(map[int64]bool) + + if repoID > 0 { + repo, err := models.GetRepositoryByID(repoID) + if err != nil { + ctx.Handle(500, "GetRepositoryByID", fmt.Errorf("[#%d]%v", repoID, err)) + return } - if filterMode != models.FM_ALL { - // Calculate repository issue count with filter mode. - numOpen, numClosed := repo.IssueStats(ctxUser.ID, filterMode, isPullList) - repo.NumOpenIssues, repo.NumClosedIssues = int(numOpen), int(numClosed) + if err = repo.GetOwner(); err != nil { + ctx.Handle(500, "GetOwner", fmt.Errorf("[#%d]%v", repoID, err)) + return } - if repo.ID == repoID || - (isShowClosed && repo.NumClosedIssues > 0) || - (!isShowClosed && repo.NumOpenIssues > 0) { - showRepos = append(showRepos, repo) + // Check if user has access to given repository. + if !repo.IsOwnedBy(ctxUser.ID) && !repo.HasAccess(ctxUser) { + ctx.Handle(404, "Issues", fmt.Errorf("#%d", repoID)) + return } + + showReposSet[repoID] = true + showRepos = append(showRepos, repo) } - ctx.Data["Repos"] = showRepos - issueStats := models.GetUserIssueStats(repoID, ctxUser.ID, repoIDs, filterMode, isPullList) - issueStats.AllCount = int64(allCount) + for _, issue := range issues { + // Get Repository data. + issue.Repo, err = models.GetRepositoryByID(issue.RepoID) + if err != nil { + ctx.Handle(500, "GetRepositoryByID", fmt.Errorf("[#%d]%v", issue.RepoID, err)) + return + } - page := ctx.QueryInt("page") - if page <= 1 { - page = 1 + // Get Owner data. + if err = issue.Repo.GetOwner(); err != nil { + ctx.Handle(500, "GetOwner", fmt.Errorf("[#%d]%v", issue.RepoID, err)) + return + } + + // Append repo to list of shown repos + if filterMode == models.FM_YOUR_REPOSITORIES { + // Use a map to make sure we don't add the same Repository twice. + _, ok := showReposSet[issue.RepoID] + if !ok { + showReposSet[issue.RepoID] = true + // Append to list of shown Repositories. + showRepos = append(showRepos, issue.Repo) + } + } } + issueStats := models.GetUserIssueStats(repoID, ctxUser.ID, userRepoIDs, filterMode, isPullList) + var total int if !isShowClosed { total = int(issueStats.OpenCount) } else { total = int(issueStats.ClosedCount) } - ctx.Data["Page"] = paginater.New(total, setting.UI.IssuePagingNum, page, 5) - // Get issues. - issues, err := models.Issues(&models.IssuesOptions{ - UserID: ctxUser.ID, - AssigneeID: assigneeID, - RepoID: repoID, - PosterID: posterID, - RepoIDs: repoIDs, - Page: page, - IsClosed: isShowClosed, - IsPull: isPullList, - SortType: sortType, - }) - if err != nil { - ctx.Handle(500, "Issues", err) - return - } - - // Get posters and repository. - for i := range issues { - issues[i].Repo, err = models.GetRepositoryByID(issues[i].RepoID) - if err != nil { - ctx.Handle(500, "GetRepositoryByID", fmt.Errorf("[#%d]%v", issues[i].ID, err)) - return - } - - if err = issues[i].Repo.GetOwner(); err != nil { - ctx.Handle(500, "GetOwner", fmt.Errorf("[#%d]%v", issues[i].ID, err)) - return - } - } ctx.Data["Issues"] = issues - + ctx.Data["Repos"] = showRepos + ctx.Data["Page"] = paginater.New(total, setting.UI.IssuePagingNum, page, 5) ctx.Data["IssueStats"] = issueStats ctx.Data["ViewType"] = viewType ctx.Data["SortType"] = sortType ctx.Data["RepoID"] = repoID ctx.Data["IsShowClosed"] = isShowClosed + if isShowClosed { ctx.Data["State"] = "closed" } else { diff --git a/templates/user/dashboard/issues.tmpl b/templates/user/dashboard/issues.tmpl index 5aae8b4fa..1497935af 100644 --- a/templates/user/dashboard/issues.tmpl +++ b/templates/user/dashboard/issues.tmpl @@ -5,9 +5,9 @@
- + {{.i18n.Tr "home.issues.in_your_repos"}} - {{.IssueStats.AllCount}} + {{.IssueStats.YourRepositoriesCount}} {{if not .ContextUser.IsOrganization}} @@ -22,7 +22,7 @@
{{range .Repos}} - {{$.ContextUser.Name}}/{{.Name}} + {{.FullName}}
{{if $.IsShowClosed}}{{.NumClosedIssues}}{{else}}{{.NumOpenIssues}}{{end}}
 {{end}} @@ -61,7 +61,7 @@ {{range .Issues}} {{ $timeStr:= TimeSince .Created $.Lang }}
  • -
    {{if not $.RepoID}}{{.Repo.Name}}{{end}}#{{.Index}}
    +
    {{if not $.RepoID}}{{.Repo.FullName}}{{end}}#{{.Index}}
    {{.Title}} {{if .NumComments}}