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.
8 years ago · e6ef75204b
4 changed files with 142 additions and 95 deletions
--- 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
 }
--- 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
--- 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)
-
-		if isPullList {
-			allCount += repo.NumOpenPulls
-			repo.NumOpenIssues = repo.NumOpenPulls
-			repo.NumClosedIssues = repo.NumClosedPulls
-		} else {
-			allCount += repo.NumOpenIssues
-		}
-
-		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)
+		userRepoIDs = append(userRepoIDs, repo.ID)
 	}

-		if repo.ID == repoID ||
-			(isShowClosed && repo.NumClosedIssues > 0) ||
-			(!isShowClosed && repo.NumOpenIssues > 0) {
-			showRepos = append(showRepos, repo)
-		}
-	}
-	ctx.Data["Repos"] = showRepos
-
-	issueStats := models.GetUserIssueStats(repoID, ctxUser.ID, repoIDs, filterMode, isPullList)
-	issueStats.AllCount = int64(allCount)
-
-	page := ctx.QueryInt("page")
-	if page <= 1 {
-		page = 1
-	}
+	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,
+		})

-	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)
+	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,
+		})

-	// Get issues.
-	issues, err := models.Issues(&models.IssuesOptions{
-		UserID:     ctxUser.ID,
-		AssigneeID: assigneeID,
+	case models.FM_CREATE:
+		// Get all issues created by this user.
+		issues, err = models.Issues(&models.IssuesOptions{
 			RepoID:   repoID,
-		PosterID:   posterID,
-		RepoIDs:    repoIDs,
+			PosterID: ctxUser.ID,
 			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)
+	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", issues[i].ID, err))
+			ctx.Handle(500, "GetRepositoryByID", fmt.Errorf("[#%d]%v", repoID, err))
 			return
 		}

-		if err = issues[i].Repo.GetOwner(); err != nil {
-			ctx.Handle(500, "GetOwner", fmt.Errorf("[#%d]%v", issues[i].ID, err))
+		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.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["Issues"] = issues

+	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 {
--- a/templates/user/dashboard/issues.tmpl
+++ b/templates/user/dashboard/issues.tmpl
@ -5,9 +5,9 @@
 		<div class="ui grid">
 			<div class="four wide column">
 				<div class="ui secondary vertical filter menu">
-					<a class="{{if eq .ViewType "all"}}ui basic blue button{{end}} item" href="{{.Link}}?repo={{.RepoID}}&sort={{$.SortType}}&state={{.State}}">
+					<a class="{{if eq .ViewType "your_repositories"}}ui basic blue button{{end}} item" href="{{.Link}}?type=your_repositories&repo={{.RepoID}}&sort={{$.SortType}}&state={{.State}}">
 						{{.i18n.Tr "home.issues.in_your_repos"}}
-						<strong class="ui right">{{.IssueStats.AllCount}}</strong>
+						<strong class="ui right">{{.IssueStats.YourRepositoriesCount}}</strong>
 					</a>
 					{{if not .ContextUser.IsOrganization}}
 						<a class="{{if eq .ViewType "assigned"}}ui basic blue button{{end}} item" href="{{.Link}}?type=assigned&repo={{.RepoID}}&sort={{$.SortType}}&state={{.State}}">
@ -22,7 +22,7 @@
 					<div class="ui divider"></div>
 					{{range .Repos}}
 						<a class="{{if eq $.RepoID .ID}}ui basic blue button{{end}} repo name item" href="{{$.Link}}?type={{$.ViewType}}{{if not (eq $.RepoID .ID)}}&repo={{.ID}}{{end}}&sort={{$.SortType}}&state={{$.State}}">
-							<span class="text truncate">{{$.ContextUser.Name}}/{{.Name}}</span>
+							<span class="text truncate">{{.FullName}}</span>
 							<div class="floating ui {{if $.IsShowClosed}}red{{else}}green{{end}} label">{{if $.IsShowClosed}}{{.NumClosedIssues}}{{else}}{{.NumOpenIssues}}{{end}}</div>
 						</a>
 					{{end}}
@ -61,7 +61,7 @@
 					{{range .Issues}}
 						{{ $timeStr:= TimeSince .Created $.Lang }}
 						<li class="item">
-							<div class="ui label">{{if not $.RepoID}}{{.Repo.Name}}{{end}}#{{.Index}}</div>
+							<div class="ui label">{{if not $.RepoID}}{{.Repo.FullName}}{{end}}#{{.Index}}</div>
 							<a class="title has-emoji" href="{{AppSubUrl}}/{{.Repo.Owner.Name}}/{{.Repo.Name}}/issues/{{.Index}}">{{.Title}}</a>

 							{{if .NumComments}}