]> source.dussan.org Git - gitea.git/commitdiff
Fix counts on issues dashboard (#2215)
authorEthan Koenig <etk39@cornell.edu>
Thu, 3 Aug 2017 05:09:16 +0000 (22:09 -0700)
committerLunny Xiao <xiaolunwen@gmail.com>
Thu, 3 Aug 2017 05:09:16 +0000 (13:09 +0800)
* Fix counts on issues dashboard

* setupSess -> setupSession

* Unit test

* Load repo owners for issues

12 files changed:
models/issue.go
models/issue_indexer.go
models/main_test.go
models/repo_list.go
models/unit_tests.go
modules/test/context_tests.go [new file with mode: 0644]
routers/api/v1/repo/issue.go
routers/repo/issue.go
routers/user/home.go
routers/user/home_test.go [new file with mode: 0644]
routers/user/main_test.go [new file with mode: 0644]
templates/user/dashboard/issues.tmpl

index d40b81eb32ff9cba5ab4f493bb2494b24866d766..709ebc35f9fd622c8da50644241f9b7c455d667f 100644 (file)
@@ -1057,6 +1057,7 @@ type IssuesOptions struct {
        MilestoneID int64
        RepoIDs     []int64
        Page        int
+       PageSize    int
        IsClosed    util.OptionalBool
        IsPull      util.OptionalBool
        Labels      string
@@ -1085,21 +1086,16 @@ func sortIssuesSession(sess *xorm.Session, sortType string) {
        }
 }
 
-// Issues returns a list of issues by given conditions.
-func Issues(opts *IssuesOptions) ([]*Issue, error) {
-       var sess *xorm.Session
-       if opts.Page >= 0 {
+func (opts *IssuesOptions) setupSession(sess *xorm.Session) error {
+       if opts.Page >= 0 && opts.PageSize > 0 {
                var start int
                if opts.Page == 0 {
                        start = 0
                } else {
-                       start = (opts.Page - 1) * setting.UI.IssuePagingNum
+                       start = (opts.Page - 1) * opts.PageSize
                }
-               sess = x.Limit(setting.UI.IssuePagingNum, start)
-       } else {
-               sess = x.NewSession()
+               sess.Limit(opts.PageSize, start)
        }
-       defer sess.Close()
 
        if len(opts.IssueIDs) > 0 {
                sess.In("issue.id", opts.IssueIDs)
@@ -1144,12 +1140,10 @@ func Issues(opts *IssuesOptions) ([]*Issue, error) {
                sess.And("issue.is_pull=?", false)
        }
 
-       sortIssuesSession(sess, opts.SortType)
-
        if len(opts.Labels) > 0 && opts.Labels != "0" {
                labelIDs, err := base.StringsToInt64s(strings.Split(opts.Labels, ","))
                if err != nil {
-                       return nil, err
+                       return err
                }
                if len(labelIDs) > 0 {
                        sess.
@@ -1157,6 +1151,45 @@ func Issues(opts *IssuesOptions) ([]*Issue, error) {
                                In("issue_label.label_id", labelIDs)
                }
        }
+       return nil
+}
+
+// CountIssuesByRepo map from repoID to number of issues matching the options
+func CountIssuesByRepo(opts *IssuesOptions) (map[int64]int64, error) {
+       sess := x.NewSession()
+       defer sess.Close()
+
+       if err := opts.setupSession(sess); err != nil {
+               return nil, err
+       }
+
+       countsSlice := make([]*struct {
+               RepoID int64
+               Count  int64
+       }, 0, 10)
+       if err := sess.GroupBy("issue.repo_id").
+               Select("issue.repo_id AS repo_id, COUNT(*) AS count").
+               Table("issue").
+               Find(&countsSlice); err != nil {
+               return nil, err
+       }
+
+       countMap := make(map[int64]int64, len(countsSlice))
+       for _, c := range countsSlice {
+               countMap[c.RepoID] = c.Count
+       }
+       return countMap, nil
+}
+
+// Issues returns a list of issues by given conditions.
+func Issues(opts *IssuesOptions) ([]*Issue, error) {
+       sess := x.NewSession()
+       defer sess.Close()
+
+       if err := opts.setupSession(sess); err != nil {
+               return nil, err
+       }
+       sortIssuesSession(sess, opts.SortType)
 
        issues := make([]*Issue, 0, setting.UI.IssuePagingNum)
        if err := sess.Find(&issues); err != nil {
index c2cb8b429eab93bad36bbda8a19ae20a35bd6861..05b324f535f0c8e862e699c56d60fbf99543cd39 100644 (file)
@@ -133,7 +133,6 @@ func populateIssueIndexer() error {
                                RepoID:   repo.ID,
                                IsClosed: util.OptionalBoolNone,
                                IsPull:   util.OptionalBoolNone,
-                               Page:     -1, // do not page
                        })
                        if err != nil {
                                return fmt.Errorf("Issues: %v", err)
index 57e72a57fc16db14e16e851efc88a695152b83a0..451b5e4b21638a13ec608c81d6d28ef606f5d05a 100644 (file)
@@ -8,11 +8,8 @@ import (
 
        "code.gitea.io/gitea/modules/setting"
 
-       "github.com/go-xorm/core"
-       "github.com/go-xorm/xorm"
        _ "github.com/mattn/go-sqlite3" // for the test engine
        "github.com/stretchr/testify/assert"
-       "gopkg.in/testfixtures.v2"
 )
 
 // TestFixturesAreConsistent assert that test fixtures are consistent
@@ -21,23 +18,8 @@ func TestFixturesAreConsistent(t *testing.T) {
        CheckConsistencyForAll(t)
 }
 
-// CreateTestEngine create an xorm engine for testing
-func CreateTestEngine() error {
-       var err error
-       x, err = xorm.NewEngine("sqlite3", "file::memory:?cache=shared")
-       if err != nil {
-               return err
-       }
-       x.SetMapper(core.GonicMapper{})
-       if err = x.StoreEngine("InnoDB").Sync2(tables...); err != nil {
-               return err
-       }
-
-       return InitFixtures(&testfixtures.SQLite{}, "fixtures/")
-}
-
 func TestMain(m *testing.M) {
-       if err := CreateTestEngine(); err != nil {
+       if err := CreateTestEngine("fixtures/"); err != nil {
                fmt.Printf("Error creating test engine: %v\n", err)
                os.Exit(1)
        }
index a2dae85c842bee2cfa625f28a373c30d3acb7395..2158cfe676a544b2d0801f5c8a989ae06738b5df 100644 (file)
@@ -15,6 +15,11 @@ import (
 // RepositoryList contains a list of repositories
 type RepositoryList []*Repository
 
+// RepositoryListOfMap make list from values of map
+func RepositoryListOfMap(repoMap map[int64]*Repository) RepositoryList {
+       return RepositoryList(valuesRepository(repoMap))
+}
+
 func (repos RepositoryList) loadAttributes(e Engine) error {
        if len(repos) == 0 {
                return nil
index b4b36ba6b7f205e14f78fd1d6fe3668db9cb428a..315627d8e0c51ac360c29a0e2ae5659c3b7fffa1 100644 (file)
@@ -7,13 +7,31 @@ package models
 import (
        "testing"
 
+       "github.com/go-xorm/core"
        "github.com/go-xorm/xorm"
        "github.com/stretchr/testify/assert"
+       "gopkg.in/testfixtures.v2"
 )
 
 // NonexistentID an ID that will never exist
 const NonexistentID = 9223372036854775807
 
+// CreateTestEngine create in-memory sqlite database for unit tests
+// Any package that calls this must import github.com/mattn/go-sqlite3
+func CreateTestEngine(fixturesDir string) error {
+       var err error
+       x, err = xorm.NewEngine("sqlite3", "file::memory:?cache=shared")
+       if err != nil {
+               return err
+       }
+       x.SetMapper(core.GonicMapper{})
+       if err = x.StoreEngine("InnoDB").Sync2(tables...); err != nil {
+               return err
+       }
+
+       return InitFixtures(&testfixtures.SQLite{}, fixturesDir)
+}
+
 // PrepareTestDatabase load test fixtures into test database
 func PrepareTestDatabase() error {
        return LoadFixtures()
diff --git a/modules/test/context_tests.go b/modules/test/context_tests.go
new file mode 100644 (file)
index 0000000..6bb7ffe
--- /dev/null
@@ -0,0 +1,150 @@
+// Copyright 2017 The Gitea Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package test
+
+import (
+       "net/http"
+       "net/url"
+       "testing"
+
+       "code.gitea.io/gitea/modules/context"
+
+       "github.com/stretchr/testify/assert"
+       macaron "gopkg.in/macaron.v1"
+)
+
+// MockContext mock context for unit tests
+func MockContext(t *testing.T) *context.Context {
+       var macaronContext *macaron.Context
+       mac := macaron.New()
+       mac.Get("*/", func(ctx *macaron.Context) {
+               macaronContext = ctx
+       })
+       req, err := http.NewRequest("GET", "star", nil)
+       assert.NoError(t, err)
+       req.Form = url.Values{}
+       mac.ServeHTTP(&mockResponseWriter{}, req)
+       assert.NotNil(t, macaronContext)
+       assert.EqualValues(t, req, macaronContext.Req.Request)
+       macaronContext.Locale = &mockLocale{}
+       macaronContext.Resp = &mockResponseWriter{}
+       macaronContext.Render = &mockRender{ResponseWriter: macaronContext.Resp}
+       return &context.Context{
+               Context: macaronContext,
+       }
+}
+
+type mockLocale struct{}
+
+func (l mockLocale) Language() string {
+       return "en"
+}
+
+func (l mockLocale) Tr(s string, _ ...interface{}) string {
+       return "test translation"
+}
+
+type mockResponseWriter struct {
+       status int
+       size   int
+}
+
+func (rw *mockResponseWriter) Header() http.Header {
+       return map[string][]string{}
+}
+
+func (rw *mockResponseWriter) Write(b []byte) (int, error) {
+       rw.size += len(b)
+       return len(b), nil
+}
+
+func (rw *mockResponseWriter) WriteHeader(status int) {
+       rw.status = status
+}
+
+func (rw *mockResponseWriter) Flush() {
+}
+
+func (rw *mockResponseWriter) Status() int {
+       return rw.status
+}
+
+func (rw *mockResponseWriter) Written() bool {
+       return rw.status > 0
+}
+
+func (rw *mockResponseWriter) Size() int {
+       return rw.size
+}
+
+func (rw *mockResponseWriter) Before(b macaron.BeforeFunc) {
+       b(rw)
+}
+
+type mockRender struct {
+       http.ResponseWriter
+}
+
+func (tr *mockRender) SetResponseWriter(rw http.ResponseWriter) {
+       tr.ResponseWriter = rw
+}
+
+func (tr *mockRender) JSON(int, interface{}) {
+}
+
+func (tr *mockRender) JSONString(interface{}) (string, error) {
+       return "", nil
+}
+
+func (tr *mockRender) RawData(status int, _ []byte) {
+       tr.Status(status)
+}
+
+func (tr *mockRender) PlainText(status int, _ []byte) {
+       tr.Status(status)
+}
+
+func (tr *mockRender) HTML(status int, _ string, _ interface{}, _ ...macaron.HTMLOptions) {
+       tr.Status(status)
+}
+
+func (tr *mockRender) HTMLSet(status int, _ string, _ string, _ interface{}, _ ...macaron.HTMLOptions) {
+       tr.Status(status)
+}
+
+func (tr *mockRender) HTMLSetString(string, string, interface{}, ...macaron.HTMLOptions) (string, error) {
+       return "", nil
+}
+
+func (tr *mockRender) HTMLString(string, interface{}, ...macaron.HTMLOptions) (string, error) {
+       return "", nil
+}
+
+func (tr *mockRender) HTMLSetBytes(string, string, interface{}, ...macaron.HTMLOptions) ([]byte, error) {
+       return nil, nil
+}
+
+func (tr *mockRender) HTMLBytes(string, interface{}, ...macaron.HTMLOptions) ([]byte, error) {
+       return nil, nil
+}
+
+func (tr *mockRender) XML(status int, _ interface{}) {
+       tr.Status(status)
+}
+
+func (tr *mockRender) Error(status int, _ ...string) {
+       tr.Status(status)
+}
+
+func (tr *mockRender) Status(status int) {
+       tr.ResponseWriter.WriteHeader(status)
+}
+
+func (tr *mockRender) SetTemplatePath(string, string) {
+}
+
+func (tr *mockRender) HasTemplateSet(string) bool {
+       return true
+}
index 253bdb43d2c17b4f0b9799b6c002a8098369a4e0..9f51022f35492e8ff73492639f7887d587669f39 100644 (file)
@@ -31,6 +31,7 @@ func ListIssues(ctx *context.APIContext) {
        issues, err := models.Issues(&models.IssuesOptions{
                RepoID:   ctx.Repo.Repository.ID,
                Page:     ctx.QueryInt("page"),
+               PageSize: setting.UI.IssuePagingNum,
                IsClosed: isClosed,
        })
        if err != nil {
index 473e82971393d3f1b0d14f367ae81ca1b1485611..e4ed10d98068f1d695605ddb1f7e896dc808eb6b 100644 (file)
@@ -193,6 +193,7 @@ func Issues(ctx *context.Context) {
                        MentionedID: mentionedID,
                        MilestoneID: milestoneID,
                        Page:        pager.Current(),
+                       PageSize:    setting.UI.IssuePagingNum,
                        IsClosed:    util.OptionalBoolOf(isShowClosed),
                        IsPull:      util.OptionalBoolOf(isPullList),
                        Labels:      selectLabels,
index 71288906908f683c1cab34a8a0e510ff257d3512..b2096bc2ceeadc43a3f9b3af97c0c5c2892d9c6d 100644 (file)
@@ -270,94 +270,77 @@ func Issues(ctx *context.Context) {
                userRepoIDs = []int64{-1}
        }
 
-       var issues []*models.Issue
+       opts := &models.IssuesOptions{
+               RepoID:   repoID,
+               IsClosed: util.OptionalBoolOf(isShowClosed),
+               IsPull:   util.OptionalBoolOf(isPullList),
+               SortType: sortType,
+       }
+
        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,
-               })
-
+               opts.RepoIDs = userRepoIDs
        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,
-               })
-
+               opts.AssigneeID = ctxUser.ID
        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,
-               })
+               opts.PosterID = ctxUser.ID
        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,
-               })
+               opts.MentionedID = ctxUser.ID
        }
 
+       counts, err := models.CountIssuesByRepo(opts)
        if err != nil {
-               ctx.Handle(500, "Issues", err)
+               ctx.Handle(500, "CountIssuesByRepo", err)
                return
        }
 
-       showRepos, err := models.IssueList(issues).LoadRepositories()
+       opts.Page = page
+       opts.PageSize = setting.UI.IssuePagingNum
+       issues, err := models.Issues(opts)
        if err != nil {
-               ctx.Handle(500, "LoadRepositories", fmt.Errorf("%v", err))
+               ctx.Handle(500, "Issues", err)
                return
        }
 
-       if repoID > 0 {
-               var theRepo *models.Repository
-               for _, repo := range showRepos {
-                       if repo.ID == repoID {
-                               theRepo = repo
-                               break
-                       }
+       showReposMap := make(map[int64]*models.Repository, len(counts))
+       for repoID := range counts {
+               repo, err := models.GetRepositoryByID(repoID)
+               if err != nil {
+                       ctx.Handle(500, "GetRepositoryByID", err)
+                       return
                }
+               showReposMap[repoID] = repo
+       }
 
-               if theRepo == nil {
-                       theRepo, err = models.GetRepositoryByID(repoID)
+       if repoID > 0 {
+               if _, ok := showReposMap[repoID]; !ok {
+                       repo, err := models.GetRepositoryByID(repoID)
                        if err != nil {
-                               ctx.Handle(500, "GetRepositoryByID", fmt.Errorf("[#%d]%v", repoID, err))
+                               ctx.Handle(500, "GetRepositoryByID", fmt.Errorf("[%d]%v", repoID, err))
                                return
                        }
-                       showRepos = append(showRepos, theRepo)
+                       showReposMap[repoID] = repo
                }
 
+               repo := showReposMap[repoID]
+
                // Check if user has access to given repository.
-               if !theRepo.IsOwnedBy(ctxUser.ID) && !theRepo.HasAccess(ctxUser) {
-                       ctx.Handle(404, "Issues", fmt.Errorf("#%d", repoID))
+               if !repo.IsOwnedBy(ctxUser.ID) && !repo.HasAccess(ctxUser) {
+                       ctx.Status(404)
                        return
                }
        }
 
-       err = models.RepositoryList(showRepos).LoadAttributes()
-       if err != nil {
+       showRepos := models.RepositoryListOfMap(showReposMap)
+       if err = showRepos.LoadAttributes(); err != nil {
                ctx.Handle(500, "LoadAttributes", fmt.Errorf("%v", err))
                return
        }
 
+       for _, issue := range issues {
+               issue.Repo = showReposMap[issue.RepoID]
+       }
+
        issueStats := models.GetUserIssueStats(repoID, ctxUser.ID, userRepoIDs, filterMode, isPullList)
 
        var total int
@@ -369,6 +352,7 @@ func Issues(ctx *context.Context) {
 
        ctx.Data["Issues"] = issues
        ctx.Data["Repos"] = showRepos
+       ctx.Data["Counts"] = counts
        ctx.Data["Page"] = paginater.New(total, setting.UI.IssuePagingNum, page, 5)
        ctx.Data["IssueStats"] = issueStats
        ctx.Data["ViewType"] = viewType
diff --git a/routers/user/home_test.go b/routers/user/home_test.go
new file mode 100644 (file)
index 0000000..beca936
--- /dev/null
@@ -0,0 +1,33 @@
+// Copyright 2017 The Gitea Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package user
+
+import (
+       "net/http"
+       "testing"
+
+       "code.gitea.io/gitea/models"
+       "code.gitea.io/gitea/modules/test"
+
+       "code.gitea.io/gitea/modules/setting"
+       "github.com/stretchr/testify/assert"
+)
+
+func TestIssues(t *testing.T) {
+       setting.UI.IssuePagingNum = 1
+       assert.NoError(t, models.LoadFixtures())
+
+       ctx := test.MockContext(t)
+       ctx.User = models.AssertExistsAndLoadBean(t, &models.User{ID: 2}).(*models.User)
+       ctx.SetParams(":type", "issues")
+       ctx.Req.Form.Set("state", "closed")
+       Issues(ctx)
+       assert.EqualValues(t, http.StatusOK, ctx.Resp.Status())
+
+       assert.EqualValues(t, map[int64]int64{1: 1, 2: 1}, ctx.Data["Counts"])
+       assert.EqualValues(t, true, ctx.Data["IsShowClosed"])
+       assert.Len(t, ctx.Data["Issues"], 1)
+       assert.Len(t, ctx.Data["Repos"], 2)
+}
diff --git a/routers/user/main_test.go b/routers/user/main_test.go
new file mode 100644 (file)
index 0000000..83c0c65
--- /dev/null
@@ -0,0 +1,33 @@
+// Copyright 2017 The Gitea Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package user
+
+import (
+       "fmt"
+       "os"
+       "path/filepath"
+       "testing"
+
+       "code.gitea.io/gitea/models"
+       "code.gitea.io/gitea/modules/setting"
+
+       _ "github.com/mattn/go-sqlite3" // for the test engine
+)
+
+func TestMain(m *testing.M) {
+       if err := models.CreateTestEngine("../../models/fixtures/"); err != nil {
+               fmt.Printf("Error creating test engine: %v\n", err)
+               os.Exit(1)
+       }
+
+       setting.AppURL = "https://try.gitea.io/"
+       setting.RunUser = "runuser"
+       setting.SSH.Port = 3000
+       setting.SSH.Domain = "try.gitea.io"
+       setting.RepoRootPath = filepath.Join(os.TempDir(), "repos")
+       setting.AppDataPath = filepath.Join(os.TempDir(), "appdata")
+
+       os.Exit(m.Run())
+}
index a146250c18fec8f1eba731a432acb1813f0adb2e..954b868214031865ef8c75841536603d5e3e81e2 100644 (file)
@@ -23,7 +23,7 @@
                                        {{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">{{.FullName}}</span>
-                                                       <div class="floating ui {{if $.IsShowClosed}}red{{else}}green{{end}} label">{{if $.IsShowClosed}}{{if $.PageIsPulls}}{{.NumClosedPulls}}{{else}}{{.NumClosedIssues}}{{end}}{{else}}{{if $.PageIsPulls}}{{.NumOpenPulls}}{{else}}{{.NumOpenIssues}}{{end}}{{end}}</div>
+                                                       <div class="floating ui {{if $.IsShowClosed}}red{{else}}green{{end}} label">{{index $.Counts .ID}}</div>
                                                </a>
                                        {{end}}
                                </div>