Shows total tracked time in issue and milestone list (#3341)

* Show total tracked time in issue and milestone list
Show total tracked time at issue page

Signed-off-by: Jonas Franz <[email protected]>

* Optimizing TotalTimes by using SumInt

Signed-off-by: Jonas Franz <[email protected]>

* Fixing wrong total times for milestones caused by a missing JOIN
Adding unit tests for total times

Signed-off-by: Jonas Franz <[email protected]>

* Logging error instead of ignoring it

Signed-off-by: Jonas Franz <[email protected]>

* Correcting spelling mistakes

Signed-off-by: Jonas Franz <[email protected]>

* Change error message to a short version

Signed-off-by: Jonas Franz <[email protected]>

* Add error handling to TotalTimes
Add variable for totalTimes

Signed-off-by: Jonas Franz <[email protected]>

* Introduce TotalTrackedTimes as variable of issue
Load TotalTrackedTimes by loading attributes of IssueList
Load TotalTrackedTimes by loading attributes of single issue
Add Sec2Time as helper to use it in templates

Signed-off-by: Jonas Franz <[email protected]>

* Fixed test + gofmt

Signed-off-by: Jonas Franz <[email protected]>

* Load TotalTrackedTimes via MilestoneList instead of single requests

Signed-off-by: Jonas Franz <[email protected]>

* Add documentation for MilestoneList

Signed-off-by: Jonas Franz <[email protected]>

* Add documentation for MilestoneList

Signed-off-by: Jonas Franz <[email protected]>

* Fix test

Signed-off-by: Jonas Franz <[email protected]>

* Change comment from SQL query to description

Signed-off-by: Jonas Franz <[email protected]>

* Fix unit test by using int64 instead of int

Signed-off-by: Jonas Franz <[email protected]>

* Fix unit test by using int64 instead of int

Signed-off-by: Jonas Franz <[email protected]>

* Check if timetracker is enabled

Signed-off-by: Jonas Franz <[email protected]>

* Fix test by enabling timetracking

Signed-off-by: Jonas Franz <[email protected]>

Author: jonasfranz

models/issue.go

@@ -51,9 +51,10 @@ type Issue struct {
 	UpdatedUnix  util.TimeStamp `xorm:"INDEX updated"`
 	ClosedUnix   util.TimeStamp `xorm:"INDEX"`
 
-	Attachments []*Attachment `xorm:"-"`
-	Comments    []*Comment    `xorm:"-"`
-	Reactions   ReactionList  `xorm:"-"`
+	Attachments      []*Attachment `xorm:"-"`
+	Comments         []*Comment    `xorm:"-"`
+	Reactions        ReactionList  `xorm:"-"`
+	TotalTrackedTime int64         `xorm:"-"`
 }
 
 var (
@@ -69,6 +70,15 @@ func init() {
 	issueTasksDonePat = regexp.MustCompile(issueTasksDoneRegexpStr)
 }
 
+func (issue *Issue) loadTotalTimes(e Engine) (err error) {
+	opts := FindTrackedTimesOptions{IssueID: issue.ID}
+	issue.TotalTrackedTime, err = opts.ToSession(e).SumInt(&TrackedTime{}, "time")
+	if err != nil {
+		return err
+	}
+	return nil
+}
+
 func (issue *Issue) loadRepo(e Engine) (err error) {
 	if issue.Repo == nil {
 		issue.Repo, err = getRepositoryByID(e, issue.RepoID)
@@ -79,6 +89,15 @@ func (issue *Issue) loadRepo(e Engine) (err error) {
 	return nil
 }
 
+// IsTimetrackerEnabled returns true if the repo enables timetracking
+func (issue *Issue) IsTimetrackerEnabled() bool {
+	if err := issue.loadRepo(x); err != nil {
+		log.Error(4, fmt.Sprintf("loadRepo: %v", err))
+		return false
+	}
+	return issue.Repo.IsTimetrackerEnabled()
+}
+
 // GetPullRequest returns the issue pull request
 func (issue *Issue) GetPullRequest() (pr *PullRequest, err error) {
 	if !issue.IsPull {
@@ -225,6 +244,11 @@ func (issue *Issue) loadAttributes(e Engine) (err error) {
 	if err = issue.loadComments(e); err != nil {
 		return err
 	}
+	if issue.IsTimetrackerEnabled() {
+		if err = issue.loadTotalTimes(e); err != nil {
+			return err
+		}
+	}
 
 	return issue.loadReactions(e)
 }

models/issue_list.go

@@ -290,6 +290,50 @@ func (issues IssueList) loadComments(e Engine) (err error) {
 	return nil
 }
 
+func (issues IssueList) loadTotalTrackedTimes(e Engine) (err error) {
+	type totalTimesByIssue struct {
+		IssueID int64
+		Time    int64
+	}
+	if len(issues) == 0 {
+		return nil
+	}
+	var trackedTimes = make(map[int64]int64, len(issues))
+
+	var ids = make([]int64, 0, len(issues))
+	for _, issue := range issues {
+		if issue.Repo.IsTimetrackerEnabled() {
+			ids = append(ids, issue.ID)
+		}
+	}
+
+	// select issue_id, sum(time) from tracked_time where issue_id in (<issue ids in current page>) group by issue_id
+	rows, err := e.Table("tracked_time").
+		Select("issue_id, sum(time) as time").
+		In("issue_id", ids).
+		GroupBy("issue_id").
+		Rows(new(totalTimesByIssue))
+	if err != nil {
+		return err
+	}
+
+	defer rows.Close()
+
+	for rows.Next() {
+		var totalTime totalTimesByIssue
+		err = rows.Scan(&totalTime)
+		if err != nil {
+			return err
+		}
+		trackedTimes[totalTime.IssueID] = totalTime.Time
+	}
+
+	for _, issue := range issues {
+		issue.TotalTrackedTime = trackedTimes[issue.ID]
+	}
+	return nil
+}
+
 // loadAttributes loads all attributes, expect for attachments and comments
 func (issues IssueList) loadAttributes(e Engine) (err error) {
 	if _, err = issues.loadRepositories(e); err != nil {
@@ -316,6 +360,10 @@ func (issues IssueList) loadAttributes(e Engine) (err error) {
 		return
 	}
 
+	if err = issues.loadTotalTrackedTimes(e); err != nil {
+		return
+	}
+
 	return nil
 }
 

models/issue_list_test.go

@@ -7,6 +7,8 @@ package models
 import (
 	"testing"
 
+	"code.gitea.io/gitea/modules/setting"
+
 	"github.com/stretchr/testify/assert"
 )
 
@@ -29,7 +31,7 @@ func TestIssueList_LoadRepositories(t *testing.T) {
 
 func TestIssueList_LoadAttributes(t *testing.T) {
 	assert.NoError(t, PrepareTestDatabase())
-
+	setting.Service.EnableTimetracking = true
 	issueList := IssueList{
 		AssertExistsAndLoadBean(t, &Issue{ID: 1}).(*Issue),
 		AssertExistsAndLoadBean(t, &Issue{ID: 2}).(*Issue),
@@ -61,5 +63,10 @@ func TestIssueList_LoadAttributes(t *testing.T) {
 		for _, comment := range issue.Comments {
 			assert.EqualValues(t, issue.ID, comment.IssueID)
 		}
+		if issue.ID == int64(1) {
+			assert.Equal(t, int64(400), issue.TotalTrackedTime)
+		} else if issue.ID == int64(2) {
+			assert.Equal(t, int64(3662), issue.TotalTrackedTime)
+		}
 	}
 }

models/issue_milestone.go

@@ -29,6 +29,8 @@ type Milestone struct {
 	DeadlineString string `xorm:"-"`
 	DeadlineUnix   util.TimeStamp
 	ClosedDateUnix util.TimeStamp
+
+	TotalTrackedTime int64 `xorm:"-"`
 }
 
 // BeforeUpdate is invoked from XORM before updating this object.
@@ -118,14 +120,69 @@ func GetMilestoneByRepoID(repoID, id int64) (*Milestone, error) {
 	return getMilestoneByRepoID(x, repoID, id)
 }
 
+// MilestoneList is a list of milestones offering additional functionality
+type MilestoneList []*Milestone
+
+func (milestones MilestoneList) loadTotalTrackedTimes(e Engine) error {
+	type totalTimesByMilestone struct {
+		MilestoneID int64
+		Time        int64
+	}
+	if len(milestones) == 0 {
+		return nil
+	}
+	var trackedTimes = make(map[int64]int64, len(milestones))
+
+	// Get total tracked time by milestone_id
+	rows, err := e.Table("issue").
+		Join("INNER", "milestone", "issue.milestone_id = milestone.id").
+		Join("LEFT", "tracked_time", "tracked_time.issue_id = issue.id").
+		Select("milestone_id, sum(time) as time").
+		In("milestone_id", milestones.getMilestoneIDs()).
+		GroupBy("milestone_id").
+		Rows(new(totalTimesByMilestone))
+	if err != nil {
+		return err
+	}
+
+	defer rows.Close()
+
+	for rows.Next() {
+		var totalTime totalTimesByMilestone
+		err = rows.Scan(&totalTime)
+		if err != nil {
+			return err
+		}
+		trackedTimes[totalTime.MilestoneID] = totalTime.Time
+	}
+
+	for _, milestone := range milestones {
+		milestone.TotalTrackedTime = trackedTimes[milestone.ID]
+	}
+	return nil
+}
+
+// LoadTotalTrackedTimes loads for every milestone in the list the TotalTrackedTime by a batch request
+func (milestones MilestoneList) LoadTotalTrackedTimes() error {
+	return milestones.loadTotalTrackedTimes(x)
+}
+
+func (milestones MilestoneList) getMilestoneIDs() []int64 {
+	var ids = make([]int64, 0, len(milestones))
+	for _, ms := range milestones {
+		ids = append(ids, ms.ID)
+	}
+	return ids
+}
+
 // GetMilestonesByRepoID returns all milestones of a repository.
-func GetMilestonesByRepoID(repoID int64) ([]*Milestone, error) {
+func GetMilestonesByRepoID(repoID int64) (MilestoneList, error) {
 	miles := make([]*Milestone, 0, 10)
 	return miles, x.Where("repo_id = ?", repoID).Find(&miles)
 }
 
 // GetMilestones returns a list of milestones of given repository and status.
-func GetMilestones(repoID int64, page int, isClosed bool, sortType string) ([]*Milestone, error) {
+func GetMilestones(repoID int64, page int, isClosed bool, sortType string) (MilestoneList, error) {
 	miles := make([]*Milestone, 0, setting.UI.IssuePagingNum)
 	sess := x.Where("repo_id = ? AND is_closed = ?", repoID, isClosed)
 	if page > 0 {
@@ -146,7 +203,6 @@ func GetMilestones(repoID int64, page int, isClosed bool, sortType string) ([]*M
 	default:
 		sess.Asc("deadline_unix")
 	}
-
 	return miles, sess.Find(&miles)
 }
 

models/issue_milestone_test.go

@@ -253,3 +253,14 @@ func TestDeleteMilestoneByRepoID(t *testing.T) {
 
 	assert.NoError(t, DeleteMilestoneByRepoID(NonexistentID, NonexistentID))
 }
+
+func TestMilestoneList_LoadTotalTrackedTimes(t *testing.T) {
+	assert.NoError(t, PrepareTestDatabase())
+	miles := MilestoneList{
+		AssertExistsAndLoadBean(t, &Milestone{ID: 1}).(*Milestone),
+	}
+
+	assert.NoError(t, miles.LoadTotalTrackedTimes())
+
+	assert.Equal(t, miles[0].TotalTrackedTime, int64(3662))
+}

models/issue_stopwatch.go

@@ -69,7 +69,7 @@ func CreateOrStopIssueStopwatch(user *User, issue *Issue) error {
 			Doer:    user,
 			Issue:   issue,
 			Repo:    issue.Repo,
-			Content: secToTime(timediff),
+			Content: SecToTime(timediff),
 			Type:    CommentTypeStopTracking,
 		}); err != nil {
 			return err
@@ -124,7 +124,8 @@ func CancelStopwatch(user *User, issue *Issue) error {
 	return nil
 }
 
-func secToTime(duration int64) string {
+// SecToTime converts an amount of seconds to a human-readable string (example: 66s -> 1min 6s)
+func SecToTime(duration int64) string {
 	seconds := duration % 60
 	minutes := (duration / (60)) % 60
 	hours := duration / (60 * 60)

models/issue_test.go

@@ -279,3 +279,11 @@ func TestGetUserIssueStats(t *testing.T) {
 		assert.Equal(t, test.ExpectedIssueStats, *stats)
 	}
 }
+
+func TestIssue_loadTotalTimes(t *testing.T) {
+	assert.NoError(t, PrepareTestDatabase())
+	ms, err := GetIssueByID(2)
+	assert.NoError(t, err)
+	assert.NoError(t, ms.loadTotalTimes(x))
+	assert.Equal(t, int64(3662), ms.TotalTrackedTime)
+}

models/issue_tracked_time.go

@@ -11,6 +11,7 @@ import (
 	api "code.gitea.io/sdk/gitea"
 
 	"github.com/go-xorm/builder"
+	"github.com/go-xorm/xorm"
 )
 
 // TrackedTime represents a time that was spent for a specific issue.
@@ -44,6 +45,7 @@ type FindTrackedTimesOptions struct {
 	IssueID      int64
 	UserID       int64
 	RepositoryID int64
+	MilestoneID  int64
 }
 
 // ToCond will convert each condition into a xorm-Cond
@@ -58,16 +60,23 @@ func (opts *FindTrackedTimesOptions) ToCond() builder.Cond {
 	if opts.RepositoryID != 0 {
 		cond = cond.And(builder.Eq{"issue.repo_id": opts.RepositoryID})
 	}
+	if opts.MilestoneID != 0 {
+		cond = cond.And(builder.Eq{"issue.milestone_id": opts.MilestoneID})
+	}
 	return cond
 }
 
+// ToSession will convert the given options to a xorm Session by using the conditions from ToCond and joining with issue table if required
+func (opts *FindTrackedTimesOptions) ToSession(e Engine) *xorm.Session {
+	if opts.RepositoryID > 0 || opts.MilestoneID > 0 {
+		return e.Join("INNER", "issue", "issue.id = tracked_time.issue_id").Where(opts.ToCond())
+	}
+	return x.Where(opts.ToCond())
+}
+
 // GetTrackedTimes returns all tracked times that fit to the given options.
 func GetTrackedTimes(options FindTrackedTimesOptions) (trackedTimes []*TrackedTime, err error) {
-	if options.RepositoryID > 0 {
-		err = x.Join("INNER", "issue", "issue.id = tracked_time.issue_id").Where(options.ToCond()).Find(&trackedTimes)
-		return
-	}
-	err = x.Where(options.ToCond()).Find(&trackedTimes)
+	err = options.ToSession(x).Find(&trackedTimes)
 	return
 }
 
@@ -85,7 +94,7 @@ func AddTime(user *User, issue *Issue, time int64) (*TrackedTime, error) {
 		Issue:   issue,
 		Repo:    issue.Repo,
 		Doer:    user,
-		Content: secToTime(time),
+		Content: SecToTime(time),
 		Type:    CommentTypeAddTimeManual,
 	}); err != nil {
 		return nil, err
@@ -115,7 +124,7 @@ func TotalTimes(options FindTrackedTimesOptions) (map[*User]string, error) {
 			}
 			return nil, err
 		}
-		totalTimes[user] = secToTime(total)
+		totalTimes[user] = SecToTime(total)
 	}
 	return totalTimes, nil
 }

modules/templates/helper.go

@@ -179,8 +179,9 @@ func NewFuncMap() []template.FuncMap {
 			}
 			return dict, nil
 		},
-		"Printf": fmt.Sprintf,
-		"Escape": Escape,
+		"Printf":   fmt.Sprintf,
+		"Escape":   Escape,
+		"Sec2Time": models.SecToTime,
 	}}
 }
 

options/locale/locale_en-US.ini

@@ -736,7 +736,8 @@ issues.add_time_minutes = Minutes
 issues.add_time_sum_to_small = No time was entered.
 issues.cancel_tracking = Cancel
 issues.cancel_tracking_history = `cancelled time tracking %s`
-issues.time_spent_total = Total Time Spent
+issues.time_spent_from_all_authors = `Total Time Spent: %s`
+
 
 pulls.desc = Enable merge requests and code reviews.
 pulls.new = New Pull Request

routers/repo/issue.go

@@ -1139,6 +1139,12 @@ func Milestones(ctx *context.Context) {
 		ctx.ServerError("GetMilestones", err)
 		return
 	}
+	if ctx.Repo.Repository.IsTimetrackerEnabled() {
+		if miles.LoadTotalTrackedTimes(); err != nil {
+			ctx.ServerError("LoadTotalTrackedTimes", err)
+			return
+		}
+	}
 	for _, m := range miles {
 		m.RenderedContent = string(markdown.Render([]byte(m.Content), ctx.Repo.RepoLink, ctx.Repo.Repository.ComposeMetas()))
 	}

templates/repo/issue/list.tmpl

@@ -198,6 +198,10 @@
 						<span class="comment ui right"><i class="octicon octicon-comment"></i> {{.NumComments}}</span>
 					{{end}}
 
+					{{if .TotalTrackedTime}}
+						<span class="comment ui right"><i class="octicon octicon-clock"></i> {{.TotalTrackedTime | Sec2Time}}</span>
+					{{end}}
+
 					<p class="desc">
 						{{$.i18n.Tr "repo.issues.opened_by" $timeStr .Poster.HomeLink .Poster.Name | Safe}}
 						{{$tasks := .GetTasks}}