Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix assigned issues dashboard #920

Merged
merged 2 commits into from
Feb 14, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 61 additions & 28 deletions models/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
}
stats.ClosedCount, err = countSession(opts).
And("is_closed = ?", true).
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))
}
return stats, nil
return stats, err
}

// GetUserIssueStats returns issue statistic information for dashboard by given conditions.
Expand All @@ -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).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like this. We're essentially assuming that the repositories in repoIDs are exactly the set of repositories that some user owns. Moreover, this will return the wrong count if repoID > 0.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice if we could change the interface of GetUserIssueStats so that we don't need to make these sort of implicit assumptions.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cherry-pick from gogs/gogs#3560 and resolved the conflicts. I think we can improve this in further PRs.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok fine

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
}

Expand All @@ -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
}
Expand Down
23 changes: 1 addition & 22 deletions routers/repo/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"fmt"
"io"
"io/ioutil"
"net/url"
"strings"
"time"

Expand Down Expand Up @@ -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")
Expand Down
Loading