Fix bugs in issue dashboard stats (#3073)

4 files changed, 195 insertions, 47 deletions

diff --git a/models/fixtures/issue.yml b/models/fixtures/issue.yml
index 2592ad2de0..ff514e706c 100644
--- a/models/fixtures/issue.yml
+++ b/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
diff --git a/models/issue.go b/models/issue.go
index 984e6e31cf..13973b7d1c 100644
--- a/models/issue.go
+++ b/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
+		}
+	}
+
+	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
+	return stats, nil
 }
 
 // GetRepoIssueStats returns number of open and closed repository issues by given filter mode.
diff --git a/models/issue_indexer.go b/models/issue_indexer.go
index 3a2ad157c3..26d053a5d7 100644
--- a/models/issue_indexer.go
+++ b/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,
 			})
diff --git a/models/issue_test.go b/models/issue_test.go
index 47bb4feeaa..851fe684fb 100644
--- a/models/issue_test.go
+++ b/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)
+	}
+}