Fix bugs in issue dashboard stats (#3073)

models/fixtures/issue.yml

@@ -47,6 +47,8 @@
   content: content for the fourth issue
   is_closed: true
   is_pull: false
+  created_unix: 946684830
+  updated_unix: 978307200
 
 -
   id: 5
@@ -57,6 +59,9 @@
   content: content for the fifth issue
   is_closed: true
   is_pull: false
+  created_unix: 946684840
+  updated_unix: 978307200
+
 -
   id: 6
   repo_id: 3
@@ -68,5 +73,5 @@
   is_closed: false
   is_pull: false
   num_comments: 0
-  created_unix: 946684800
+  created_unix: 946684850
   updated_unix: 978307200

models/issue.go

@@ -10,14 +10,15 @@ import (
 	"sort"
 	"strings"
 
-	api "code.gitea.io/sdk/gitea"
-	"github.com/Unknwon/com"
-	"github.com/go-xorm/xorm"
-
 	"code.gitea.io/gitea/modules/base"
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/util"
+	api "code.gitea.io/sdk/gitea"
+
+	"github.com/Unknwon/com"
+	"github.com/go-xorm/builder"
+	"github.com/go-xorm/xorm"
 )
 
 // Issue represents an issue or pull request of repository.
@@ -1022,12 +1023,11 @@ func GetIssuesByIDs(issueIDs []int64) ([]*Issue, error) {
 
 // IssuesOptions represents options of an issue.
 type IssuesOptions struct {
-	RepoID      int64
+	RepoIDs     []int64 // include all repos if empty
 	AssigneeID  int64
 	PosterID    int64
 	MentionedID int64
 	MilestoneID int64
-	RepoIDs     []int64
 	Page        int
 	PageSize    int
 	IsClosed    util.OptionalBool
@@ -1073,9 +1073,7 @@ func (opts *IssuesOptions) setupSession(sess *xorm.Session) error {
 		sess.In("issue.id", opts.IssueIDs)
 	}
 
-	if opts.RepoID > 0 {
-		sess.And("issue.repo_id=?", opts.RepoID)
-	} else if len(opts.RepoIDs) > 0 {
+	if len(opts.RepoIDs) > 0 {
 		// In case repository IDs are provided but actually no repository has issue.
 		sess.In("issue.repo_id", opts.RepoIDs)
 	}
@@ -1339,58 +1337,92 @@ func GetIssueStats(opts *IssueStatsOptions) (*IssueStats, error) {
 	return stats, err
 }
 
+// UserIssueStatsOptions contains parameters accepted by GetUserIssueStats.
+type UserIssueStatsOptions struct {
+	UserID      int64
+	RepoID      int64
+	UserRepoIDs []int64
+	FilterMode  int
+	IsPull      bool
+	IsClosed    bool
+}
+
 // GetUserIssueStats returns issue statistic information for dashboard by given conditions.
-func GetUserIssueStats(repoID, uid int64, repoIDs []int64, filterMode int, isPull bool) *IssueStats {
+func GetUserIssueStats(opts UserIssueStatsOptions) (*IssueStats, error) {
+	var err error
 	stats := &IssueStats{}
 
-	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 {
-			sess.And("repo_id = ?", repoID)
-		} else if len(repoIDs) > 0 {
-			sess.In("repo_id", repoIDs)
-		}
-
-		return sess
+	cond := builder.NewCond()
+	cond = cond.And(builder.Eq{"issue.is_pull": opts.IsPull})
+	if opts.RepoID > 0 {
+		cond = cond.And(builder.Eq{"issue.repo_id": opts.RepoID})
 	}
 
-	stats.AssignCount, _ = countSession(false, isPull, repoID, nil).
-		And("assignee_id = ?", uid).
-		Count(new(Issue))
-
-	stats.CreateCount, _ = countSession(false, isPull, repoID, nil).
-		And("poster_id = ?", uid).
-		Count(new(Issue))
-
-	stats.YourRepositoriesCount, _ = countSession(false, isPull, repoID, repoIDs).
-		Count(new(Issue))
-
-	switch filterMode {
+	switch opts.FilterMode {
 	case FilterModeAll:
-		stats.OpenCount, _ = countSession(false, isPull, repoID, repoIDs).
+		stats.OpenCount, err = x.Where(cond).And("is_closed = ?", false).
+			And(builder.In("issue.repo_id", opts.UserRepoIDs)).
 			Count(new(Issue))
-		stats.ClosedCount, _ = countSession(true, isPull, repoID, repoIDs).
+		if err != nil {
+			return nil, err
+		}
+		stats.ClosedCount, err = x.Where(cond).And("is_closed = ?", true).
+			And(builder.In("issue.repo_id", opts.UserRepoIDs)).
 			Count(new(Issue))
+		if err != nil {
+			return nil, err
+		}
 	case FilterModeAssign:
-		stats.OpenCount, _ = countSession(false, isPull, repoID, nil).
-			And("assignee_id = ?", uid).
+		stats.OpenCount, err = x.Where(cond).And("is_closed = ?", false).
+			And("assignee_id = ?", opts.UserID).
 			Count(new(Issue))
-		stats.ClosedCount, _ = countSession(true, isPull, repoID, nil).
-			And("assignee_id = ?", uid).
+		if err != nil {
+			return nil, err
+		}
+		stats.ClosedCount, err = x.Where(cond).And("is_closed = ?", true).
+			And("assignee_id = ?", opts.UserID).
 			Count(new(Issue))
+		if err != nil {
+			return nil, err
+		}
 	case FilterModeCreate:
-		stats.OpenCount, _ = countSession(false, isPull, repoID, nil).
-			And("poster_id = ?", uid).
+		stats.OpenCount, err = x.Where(cond).And("is_closed = ?", false).
+			And("poster_id = ?", opts.UserID).
 			Count(new(Issue))
-		stats.ClosedCount, _ = countSession(true, isPull, repoID, nil).
-			And("poster_id = ?", uid).
+		if err != nil {
+			return nil, err
+		}
+		stats.ClosedCount, err = x.Where(cond).And("is_closed = ?", true).
+			And("poster_id = ?", opts.UserID).
 			Count(new(Issue))
+		if err != nil {
+			return nil, err
+		}
 	}
 
-	return stats
+	cond = cond.And(builder.Eq{"issue.is_closed": opts.IsClosed})
+	stats.AssignCount, err = x.Where(cond).
+		And("assignee_id = ?", opts.UserID).
+		Count(new(Issue))
+	if err != nil {
+		return nil, err
+	}
+
+	stats.CreateCount, err = x.Where(cond).
+		And("poster_id = ?", opts.UserID).
+		Count(new(Issue))
+	if err != nil {
+		return nil, err
+	}
+
+	stats.YourRepositoriesCount, err = x.Where(cond).
+		And(builder.In("issue.repo_id", opts.UserRepoIDs)).
+		Count(new(Issue))
+	if err != nil {
+		return nil, err
+	}
+
+	return stats, nil
 }
 
 // GetRepoIssueStats returns number of open and closed repository issues by given filter mode.

models/issue_indexer.go

@@ -42,7 +42,7 @@ func populateIssueIndexer() error {
 		}
 		for _, repo := range repos {
 			issues, err := Issues(&IssuesOptions{
-				RepoID:   repo.ID,
+				RepoIDs:  []int64{repo.ID},
 				IsClosed: util.OptionalBoolNone,
 				IsPull:   util.OptionalBoolNone,
 			})

models/issue_test.go

@@ -168,3 +168,114 @@ func TestUpdateIssueCols(t *testing.T) {
 	assert.EqualValues(t, prevContent, updatedIssue.Content)
 	AssertInt64InRange(t, now, then, int64(updatedIssue.UpdatedUnix))
 }
+
+func TestIssues(t *testing.T) {
+	assert.NoError(t, PrepareTestDatabase())
+	for _, test := range []struct {
+		Opts             IssuesOptions
+		ExpectedIssueIDs []int64
+	}{
+		{
+			IssuesOptions{
+				AssigneeID: 1,
+				SortType:   "oldest",
+			},
+			[]int64{1, 6},
+		},
+		{
+			IssuesOptions{
+				RepoIDs:  []int64{1, 3},
+				SortType: "oldest",
+				Page:     1,
+				PageSize: 4,
+			},
+			[]int64{1, 2, 3, 5},
+		},
+		{
+			IssuesOptions{
+				Labels:   "1,2",
+				Page:     1,
+				PageSize: 4,
+			},
+			[]int64{5, 2, 1},
+		},
+	} {
+		issues, err := Issues(&test.Opts)
+		assert.NoError(t, err)
+		if assert.Len(t, issues, len(test.ExpectedIssueIDs)) {
+			for i, issue := range issues {
+				assert.EqualValues(t, test.ExpectedIssueIDs[i], issue.ID)
+			}
+		}
+	}
+}
+
+func TestGetUserIssueStats(t *testing.T) {
+	assert.NoError(t, PrepareTestDatabase())
+	for _, test := range []struct {
+		Opts               UserIssueStatsOptions
+		ExpectedIssueStats IssueStats
+	}{
+		{
+			UserIssueStatsOptions{
+				UserID:     1,
+				RepoID:     1,
+				FilterMode: FilterModeAll,
+			},
+			IssueStats{
+				YourRepositoriesCount: 0,
+				AssignCount:           1,
+				CreateCount:           1,
+				OpenCount:             0,
+				ClosedCount:           0,
+			},
+		},
+		{
+			UserIssueStatsOptions{
+				UserID:     1,
+				FilterMode: FilterModeAssign,
+			},
+			IssueStats{
+				YourRepositoriesCount: 0,
+				AssignCount:           2,
+				CreateCount:           2,
+				OpenCount:             2,
+				ClosedCount:           0,
+			},
+		},
+		{
+			UserIssueStatsOptions{
+				UserID:     1,
+				FilterMode: FilterModeCreate,
+			},
+			IssueStats{
+				YourRepositoriesCount: 0,
+				AssignCount:           2,
+				CreateCount:           2,
+				OpenCount:             2,
+				ClosedCount:           0,
+			},
+		},
+		{
+			UserIssueStatsOptions{
+				UserID:      2,
+				UserRepoIDs: []int64{1, 2},
+				FilterMode:  FilterModeAll,
+				IsClosed:    true,
+			},
+			IssueStats{
+				YourRepositoriesCount: 2,
+				AssignCount:           0,
+				CreateCount:           2,
+				OpenCount:             1,
+				ClosedCount:           2,
+			},
+		},
+	} {
+		stats, err := GetUserIssueStats(test.Opts)
+		if !assert.NoError(t, err) {
+			continue
+		}
+		assert.Equal(t, test.ExpectedIssueStats, *stats)
+	}
+}

routers/api/v1/repo/issue.go

@@ -56,7 +56,7 @@ func ListIssues(ctx *context.APIContext) {
 	}
 
 	issues, err := models.Issues(&models.IssuesOptions{
-		RepoID:   ctx.Repo.Repository.ID,
+		RepoIDs:  []int64{ctx.Repo.Repository.ID},
 		Page:     ctx.QueryInt("page"),
 		PageSize: setting.UI.IssuePagingNum,
 		IsClosed: isClosed,

routers/repo/issue.go

@@ -190,8 +190,8 @@ func Issues(ctx *context.Context) {
 		issues = []*models.Issue{}
 	} else {
 		issues, err = models.Issues(&models.IssuesOptions{
+			RepoIDs:     []int64{repo.ID},
 			AssigneeID:  assigneeID,
-			RepoID:      repo.ID,
 			PosterID:    posterID,
 			MentionedID: mentionedID,
 			MilestoneID: milestoneID,

routers/user/home.go

@@ -9,13 +9,14 @@ import (
 	"fmt"
 	"sort"
 
-	"github.com/Unknwon/paginater"
-
 	"code.gitea.io/gitea/models"
 	"code.gitea.io/gitea/modules/base"
 	"code.gitea.io/gitea/modules/context"
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/util"
+
+	"github.com/Unknwon/com"
+	"github.com/Unknwon/paginater"
 )
 
 const (
@@ -231,21 +232,30 @@ func Issues(ctx *context.Context) {
 			return
 		}
 	}
-
 	if len(userRepoIDs) <= 0 {
 		userRepoIDs = []int64{-1}
 	}
 
 	opts := &models.IssuesOptions{
-		RepoID:   repoID,
 		IsClosed: util.OptionalBoolOf(isShowClosed),
 		IsPull:   util.OptionalBoolOf(isPullList),
 		SortType: sortType,
 	}
 
+	if repoID > 0 {
+		opts.RepoIDs = []int64{repoID}
+	}
+
 	switch filterMode {
 	case models.FilterModeAll:
-		opts.RepoIDs = userRepoIDs
+		if repoID > 0 {
+			if !com.IsSliceContainsInt64(userRepoIDs, repoID) {
+				// force an empty result
+				opts.RepoIDs = []int64{-1}
+			}
+		} else {
+			opts.RepoIDs = userRepoIDs
+		}
 	case models.FilterModeAssign:
 		opts.AssigneeID = ctxUser.ID
 	case models.FilterModeCreate:
@@ -308,7 +318,18 @@ func Issues(ctx *context.Context) {
 		issue.Repo = showReposMap[issue.RepoID]
 	}
 
-	issueStats := models.GetUserIssueStats(repoID, ctxUser.ID, userRepoIDs, filterMode, isPullList)
+	issueStats, err := models.GetUserIssueStats(models.UserIssueStatsOptions{
+		UserID:      ctxUser.ID,
+		RepoID:      repoID,
+		UserRepoIDs: userRepoIDs,
+		FilterMode:  filterMode,
+		IsPull:      isPullList,
+		IsClosed:    isShowClosed,
+	})
+	if err != nil {
+		ctx.Handle(500, "GetUserIssueStats", err)
+		return
+	}
 
 	var total int
 	if !isShowClosed {