From 7a9a5c8a69c2a3ba107a4dbc199f052d58262033 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 14 Feb 2017 22:15:18 +0800 Subject: [PATCH] Fix assigned issues dashboard (#920) * 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. * fix issues on dashboard --- models/issue.go | 89 +++++++++---- routers/repo/issue.go | 23 +--- routers/user/home.go | 184 +++++++++++++++++---------- templates/user/dashboard/issues.tmpl | 8 +- 4 files changed, 181 insertions(+), 123 deletions(-) diff --git a/models/issue.go b/models/issue.go index 30a6cf8a4c..3ae34009bf 100644 --- a/models/issue.go +++ b/models/issue.go @@ -1184,7 +1184,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 @@ -1210,6 +1210,7 @@ func parseCountResult(results []map[string][]byte) int64 { // IssueStatsOptions contains parameters accepted by GetIssueStats. type IssueStatsOptions struct { + FilterMode int RepoID int64 Labels string MilestoneID int64 @@ -1265,19 +1266,41 @@ func GetIssueStats(opts *IssueStatsOptions) (*IssueStats, error) { } var err error - stats.OpenCount, err = countSession(opts). - And("is_closed = ?", false). - Count(&Issue{}) - if err != nil { - return nil, err + switch opts.FilterMode { + case FilterModeAll, FilterModeAssign: + stats.OpenCount, err = countSession(opts). + And("is_closed = ?", false). + Count(new(Issue)) + + stats.ClosedCount, err = countSession(opts). + And("is_closed = ?", true). + Count(new(Issue)) + case FilterModeCreate: + stats.OpenCount, err = countSession(opts). + And("poster_id = ?", opts.PosterID). + And("is_closed = ?", false). + Count(new(Issue)) + + stats.ClosedCount, err = countSession(opts). + And("poster_id = ?", opts.PosterID). + And("is_closed = ?", true). + Count(new(Issue)) + case FilterModeMention: + stats.OpenCount, err = countSession(opts). + Join("INNER", "issue_user", "issue.id = issue_user.issue_id"). + And("issue_user.uid = ?", opts.PosterID). + And("issue_user.is_mentioned = ?", true). + And("issue.is_closed = ?", false). + Count(new(Issue)) + + stats.ClosedCount, err = countSession(opts). + Join("INNER", "issue_user", "issue.id = issue_user.issue_id"). + And("issue_user.uid = ?", opts.PosterID). + And("issue_user.is_mentioned = ?", true). + And("issue.is_closed = ?", true). + Count(new(Issue)) } - stats.ClosedCount, err = countSession(opts). - And("is_closed = ?", true). - Count(&Issue{}) - if err != nil { - return nil, err - } - return stats, nil + return stats, err } // GetUserIssueStats returns issue statistic information for dashboard by given conditions. @@ -1298,29 +1321,39 @@ func GetUserIssueStats(repoID, uid int64, repoIDs []int64, filterMode int, isPul 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 FilterModeAll: + stats.OpenCount, _ = countSession(false, isPull, repoID, repoIDs). + Count(new(Issue)) + stats.ClosedCount, _ = countSession(true, isPull, repoID, repoIDs). + Count(new(Issue)) case FilterModeAssign: - 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 FilterModeCreate: - 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 } @@ -1347,8 +1380,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 e9b60175f3..a06f21b859 100644 --- a/routers/repo/issue.go +++ b/routers/repo/issue.go @@ -10,7 +10,6 @@ import ( "fmt" "io" "io/ioutil" - "net/url" "strings" "time" @@ -108,37 +107,17 @@ func Issues(ctx *context.Context) { viewType := ctx.Query("type") sortType := ctx.Query("sort") - types := []string{"assigned", "created_by", "mentioned"} + types := []string{"all", "assigned", "created_by", "mentioned"} if !com.IsSliceContainsStr(types, viewType) { viewType = "all" } - // Must sign in to see issues about you. - if viewType != "all" && !ctx.IsSigned { - ctx.SetCookie("redirect_to", "/"+url.QueryEscape(setting.AppSubURL+ctx.Req.RequestURI), 0, setting.AppSubURL) - ctx.Redirect(setting.AppSubURL + "/user/login") - return - } - var ( assigneeID = ctx.QueryInt64("assignee") posterID int64 mentionedID int64 forceEmpty bool ) - switch viewType { - case "assigned": - if assigneeID > 0 && ctx.User.ID != assigneeID { - // two different assignees, must be empty - forceEmpty = true - } else { - assigneeID = ctx.User.ID - } - case "created_by": - posterID = ctx.User.ID - case "mentioned": - mentionedID = ctx.User.ID - } repo := ctx.Repo.Repository selectLabels := ctx.Query("labels") diff --git a/routers/user/home.go b/routers/user/home.go index f5f6a0950d..00c417cb91 100644 --- a/routers/user/home.go +++ b/routers/user/home.go @@ -183,34 +183,39 @@ func Issues(ctx *context.Context) { viewType string sortType = ctx.Query("sort") filterMode = models.FilterModeAll - assigneeID int64 - posterID int64 ) + if ctxUser.IsOrganization() { viewType = "all" } else { viewType = ctx.Query("type") - types := []string{"assigned", "created_by"} + types := []string{"all", "assigned", "created_by"} if !com.IsSliceContainsStr(types, viewType) { viewType = "all" } switch viewType { + case "all": + filterMode = models.FilterModeAll case "assigned": filterMode = models.FilterModeAssign - assigneeID = ctxUser.ID case "created_by": filterMode = models.FilterModeCreate - 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() { env, err := ctxUser.AccessibleReposEnv(ctx.User.ID) if err != nil { @@ -230,9 +235,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 && @@ -240,85 +242,129 @@ func Issues(ctx *context.Context) { continue } - repoIDs = append(repoIDs, repo.ID) - - if isPullList { - allCount += repo.NumOpenPulls - repo.NumOpenIssues = repo.NumOpenPulls - repo.NumClosedIssues = repo.NumClosedPulls - } else { - allCount += repo.NumOpenIssues - } - - if filterMode != models.FilterModeAll { - // Calculate repository issue count with filter mode. - numOpen, numClosed := repo.IssueStats(ctxUser.ID, filterMode, isPullList) - repo.NumOpenIssues, repo.NumClosedIssues = int(numOpen), int(numClosed) - } - - if repo.ID == repoID || - (isShowClosed && repo.NumClosedIssues > 0) || - (!isShowClosed && repo.NumOpenIssues > 0) { - showRepos = append(showRepos, repo) - } - } - ctx.Data["Repos"] = showRepos - if len(repoIDs) == 0 { - repoIDs = []int64{-1} + userRepoIDs = append(userRepoIDs, repo.ID) } - issueStats := models.GetUserIssueStats(repoID, ctxUser.ID, repoIDs, filterMode, isPullList) - issueStats.AllCount = int64(allCount) + var issues []*models.Issue + switch filterMode { + case models.FilterModeAll: + // Get all issues from repositories from this user. + issues, err = models.Issues(&models.IssuesOptions{ + RepoIDs: userRepoIDs, + RepoID: repoID, + Page: page, + IsClosed: util.OptionalBoolOf(isShowClosed), + IsPull: util.OptionalBoolOf(isPullList), + SortType: sortType, + }) - page := ctx.QueryInt("page") - if page <= 1 { - page = 1 + case models.FilterModeAssign: + // Get all issues assigned to this user. + issues, err = models.Issues(&models.IssuesOptions{ + RepoID: repoID, + AssigneeID: ctxUser.ID, + Page: page, + IsClosed: util.OptionalBoolOf(isShowClosed), + IsPull: util.OptionalBoolOf(isPullList), + SortType: sortType, + }) + + case models.FilterModeCreate: + // Get all issues created by this user. + issues, err = models.Issues(&models.IssuesOptions{ + RepoID: repoID, + PosterID: ctxUser.ID, + Page: page, + IsClosed: util.OptionalBoolOf(isShowClosed), + IsPull: util.OptionalBoolOf(isPullList), + SortType: sortType, + }) + case models.FilterModeMention: + // Get all issues created by this user. + issues, err = models.Issues(&models.IssuesOptions{ + RepoID: repoID, + MentionedID: ctxUser.ID, + Page: page, + IsClosed: util.OptionalBoolOf(isShowClosed), + IsPull: util.OptionalBoolOf(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 err = repo.GetOwner(); err != nil { + ctx.Handle(500, "GetOwner", fmt.Errorf("[#%d]%v", repoID, err)) + return + } + + // 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) + } + + 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 + } + + // 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.FilterModeAll { + // 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{ - AssigneeID: assigneeID, - RepoID: repoID, - PosterID: posterID, - RepoIDs: repoIDs, - Page: page, - IsClosed: util.OptionalBoolOf(isShowClosed), - IsPull: util.OptionalBoolOf(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 615a2fe995..daf9b26dd1 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}} {{range .Labels}}