[API] Extend times API (#9200)

Extensively extend the times API.

close #8833; close #8513; close #8559

11 files changed, 323 insertions, 47 deletions

diff --git a/models/fixtures/tracked_time.yml b/models/fixtures/tracked_time.yml
index 06a71c5ad9..768af38d9e 100644
--- a/models/fixtures/tracked_time.yml
+++ b/models/fixtures/tracked_time.yml
@@ -4,6 +4,7 @@
   issue_id: 1
   time: 400
   created_unix: 946684800
+  deleted: false
 
 -
   id: 2
@@ -11,6 +12,7 @@
   issue_id: 2
   time: 3661
   created_unix: 946684801
+  deleted: false
 
 -
   id: 3
@@ -18,17 +20,52 @@
   issue_id: 2
   time: 1
   created_unix: 946684802
+  deleted: false
 
 -
   id: 4
   user_id: -1
   issue_id: 4
   time: 1
-  created_unix: 946684802
+  created_unix: 946684803
+  deleted: false
 
 -
   id: 5
   user_id: 2
   issue_id: 5
   time: 1
-  created_unix: 946684802
+  created_unix: 946684804
+  deleted: false
+
+-
+  id: 6
+  user_id: 1
+  issue_id: 2
+  time: 20
+  created_unix: 946684812
+  deleted: false
+
+-
+  id: 7
+  user_id: 2
+  issue_id: 4
+  time: 3
+  created_unix: 946684813
+  deleted: false
+
+-
+  id: 8
+  user_id: 1
+  issue_id: 4
+  time: 71
+  created_unix: 947688814
+  deleted: false
+
+-
+  id: 9
+  user_id: 2
+  issue_id: 2
+  time: 100000
+  created_unix: 947688815
+  deleted: true
diff --git a/models/issue_comment.go b/models/issue_comment.go
index aeaee68003..3ba6790216 100644
--- a/models/issue_comment.go
+++ b/models/issue_comment.go
@@ -84,6 +84,8 @@ const (
 	CommentTypeUnlock
 	// Change pull request's target branch
 	CommentTypeChangeTargetBranch
+	// Delete time manual for time tracking
+	CommentTypeDeleteTimeManual
 )
 
 // CommentTag defines comment tag type
@@ -100,7 +102,7 @@ const (
 // Comment represents a comment in commit and issue page.
 type Comment struct {
 	ID               int64       `xorm:"pk autoincr"`
-	Type             CommentType `xorm:"index"`
+	Type             CommentType `xorm:"INDEX"`
 	PosterID         int64       `xorm:"INDEX"`
 	Poster           *User       `xorm:"-"`
 	OriginalAuthor   string
diff --git a/models/issue_list.go b/models/issue_list.go
index e3516b55b9..4554f906c4 100644
--- a/models/issue_list.go
+++ b/models/issue_list.go
@@ -429,6 +429,7 @@ func (issues IssueList) loadTotalTrackedTimes(e Engine) (err error) {
 
 		// 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").
+			Where("deleted = ?", false).
 			Select("issue_id, sum(time) as time").
 			In("issue_id", ids[:limit]).
 			GroupBy("issue_id").
diff --git a/models/issue_list_test.go b/models/issue_list_test.go
index 9197e0615a..f5a91702f2 100644
--- a/models/issue_list_test.go
+++ b/models/issue_list_test.go
@@ -66,7 +66,7 @@ func TestIssueList_LoadAttributes(t *testing.T) {
 		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)
+			assert.Equal(t, int64(3682), issue.TotalTrackedTime)
 		}
 	}
 }
diff --git a/models/issue_milestone.go b/models/issue_milestone.go
index b7191f66ff..f33ad19ada 100644
--- a/models/issue_milestone.go
+++ b/models/issue_milestone.go
@@ -10,8 +10,8 @@ import (
 	"code.gitea.io/gitea/modules/setting"
 	api "code.gitea.io/gitea/modules/structs"
 	"code.gitea.io/gitea/modules/timeutil"
-	"xorm.io/builder"
 
+	"xorm.io/builder"
 	"xorm.io/xorm"
 )
 
@@ -153,6 +153,7 @@ func (milestones MilestoneList) loadTotalTrackedTimes(e Engine) error {
 	rows, err := e.Table("issue").
 		Join("INNER", "milestone", "issue.milestone_id = milestone.id").
 		Join("LEFT", "tracked_time", "tracked_time.issue_id = issue.id").
+		Where("tracked_time.deleted = ?", false).
 		Select("milestone_id, sum(time) as time").
 		In("milestone_id", milestones.getMilestoneIDs()).
 		GroupBy("milestone_id").
@@ -187,6 +188,7 @@ func (m *Milestone) loadTotalTrackedTime(e Engine) error {
 	has, err := e.Table("issue").
 		Join("INNER", "milestone", "issue.milestone_id = milestone.id").
 		Join("LEFT", "tracked_time", "tracked_time.issue_id = issue.id").
+		Where("tracked_time.deleted = ?", false).
 		Select("milestone_id, sum(time) as time").
 		Where("milestone_id = ?", m.ID).
 		GroupBy("milestone_id").
diff --git a/models/issue_milestone_test.go b/models/issue_milestone_test.go
index 787b849cce..5b7d19aa4d 100644
--- a/models/issue_milestone_test.go
+++ b/models/issue_milestone_test.go
@@ -287,7 +287,7 @@ func TestMilestoneList_LoadTotalTrackedTimes(t *testing.T) {
 
 	assert.NoError(t, miles.LoadTotalTrackedTimes())
 
-	assert.Equal(t, miles[0].TotalTrackedTime, int64(3662))
+	assert.Equal(t, int64(3682), miles[0].TotalTrackedTime)
 }
 
 func TestCountMilestonesByRepoIDs(t *testing.T) {
@@ -361,7 +361,7 @@ func TestLoadTotalTrackedTime(t *testing.T) {
 
 	assert.NoError(t, milestone.LoadTotalTrackedTime())
 
-	assert.Equal(t, milestone.TotalTrackedTime, int64(3662))
+	assert.Equal(t, int64(3682), milestone.TotalTrackedTime)
 }
 
 func TestGetMilestonesStats(t *testing.T) {
diff --git a/models/issue_test.go b/models/issue_test.go
index d369b0acf5..ec4867d075 100644
--- a/models/issue_test.go
+++ b/models/issue_test.go
@@ -259,7 +259,7 @@ func TestIssue_loadTotalTimes(t *testing.T) {
 	ms, err := GetIssueByID(2)
 	assert.NoError(t, err)
 	assert.NoError(t, ms.loadTotalTimes(x))
-	assert.Equal(t, int64(3662), ms.TotalTrackedTime)
+	assert.Equal(t, int64(3682), ms.TotalTrackedTime)
 }
 
 func TestIssue_SearchIssueIDsByKeyword(t *testing.T) {
diff --git a/models/issue_tracked_time.go b/models/issue_tracked_time.go
index f616836c85..bcb163f3c5 100644
--- a/models/issue_tracked_time.go
+++ b/models/issue_tracked_time.go
@@ -16,28 +16,86 @@ import (
 
 // TrackedTime represents a time that was spent for a specific issue.
 type TrackedTime struct {
-	ID          int64     `xorm:"pk autoincr" json:"id"`
-	IssueID     int64     `xorm:"INDEX" json:"issue_id"`
-	UserID      int64     `xorm:"INDEX" json:"user_id"`
-	Created     time.Time `xorm:"-" json:"created"`
-	CreatedUnix int64     `xorm:"created" json:"-"`
-	Time        int64     `json:"time"`
+	ID          int64     `xorm:"pk autoincr"`
+	IssueID     int64     `xorm:"INDEX"`
+	Issue       *Issue    `xorm:"-"`
+	UserID      int64     `xorm:"INDEX"`
+	User        *User     `xorm:"-"`
+	Created     time.Time `xorm:"-"`
+	CreatedUnix int64     `xorm:"created"`
+	Time        int64     `xorm:"NOT NULL"`
+	Deleted     bool      `xorm:"NOT NULL DEFAULT false"`
 }
 
+// TrackedTimeList is a List of TrackedTime's
+type TrackedTimeList []*TrackedTime
+
 // AfterLoad is invoked from XORM after setting the values of all fields of this object.
 func (t *TrackedTime) AfterLoad() {
 	t.Created = time.Unix(t.CreatedUnix, 0).In(setting.DefaultUILocation)
 }
 
+// LoadAttributes load Issue, User
+func (t *TrackedTime) LoadAttributes() (err error) {
+	return t.loadAttributes(x)
+}
+
+func (t *TrackedTime) loadAttributes(e Engine) (err error) {
+	if t.Issue == nil {
+		t.Issue, err = getIssueByID(e, t.IssueID)
+		if err != nil {
+			return
+		}
+		err = t.Issue.loadRepo(e)
+		if err != nil {
+			return
+		}
+	}
+	if t.User == nil {
+		t.User, err = getUserByID(e, t.UserID)
+		if err != nil {
+			return
+		}
+	}
+	return
+}
+
 // APIFormat converts TrackedTime to API format
-func (t *TrackedTime) APIFormat() *api.TrackedTime {
-	return &api.TrackedTime{
-		ID:      t.ID,
-		IssueID: t.IssueID,
-		UserID:  t.UserID,
-		Time:    t.Time,
-		Created: t.Created,
+func (t *TrackedTime) APIFormat() (apiT *api.TrackedTime) {
+	apiT = &api.TrackedTime{
+		ID:       t.ID,
+		IssueID:  t.IssueID,
+		UserID:   t.UserID,
+		UserName: t.User.Name,
+		Time:     t.Time,
+		Created:  t.Created,
+	}
+	if t.Issue != nil {
+		apiT.Issue = t.Issue.APIFormat()
+	}
+	if t.User != nil {
+		apiT.UserName = t.User.Name
+	}
+	return
+}
+
+// LoadAttributes load Issue, User
+func (tl TrackedTimeList) LoadAttributes() (err error) {
+	for _, t := range tl {
+		if err = t.LoadAttributes(); err != nil {
+			return err
+		}
+	}
+	return
+}
+
+// APIFormat converts TrackedTimeList to API format
+func (tl TrackedTimeList) APIFormat() api.TrackedTimeList {
+	result := make([]*api.TrackedTime, 0, len(tl))
+	for _, t := range tl {
+		result = append(result, t.APIFormat())
 	}
+	return result
 }
 
 // FindTrackedTimesOptions represent the filters for tracked times. If an ID is 0 it will be ignored.
@@ -50,7 +108,7 @@ type FindTrackedTimesOptions struct {
 
 // ToCond will convert each condition into a xorm-Cond
 func (opts *FindTrackedTimesOptions) ToCond() builder.Cond {
-	cond := builder.NewCond()
+	cond := builder.NewCond().And(builder.Eq{"tracked_time.deleted": false})
 	if opts.IssueID != 0 {
 		cond = cond.And(builder.Eq{"issue_id": opts.IssueID})
 	}
@@ -71,37 +129,73 @@ 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())
+	return e.Where(opts.ToCond())
 }
 
-// GetTrackedTimes returns all tracked times that fit to the given options.
-func GetTrackedTimes(options FindTrackedTimesOptions) (trackedTimes []*TrackedTime, err error) {
-	err = options.ToSession(x).Find(&trackedTimes)
+func getTrackedTimes(e Engine, options FindTrackedTimesOptions) (trackedTimes TrackedTimeList, err error) {
+	err = options.ToSession(e).Find(&trackedTimes)
 	return
 }
 
+// GetTrackedTimes returns all tracked times that fit to the given options.
+func GetTrackedTimes(opts FindTrackedTimesOptions) (TrackedTimeList, error) {
+	return getTrackedTimes(x, opts)
+}
+
+func getTrackedSeconds(e Engine, opts FindTrackedTimesOptions) (trackedSeconds int64, err error) {
+	return opts.ToSession(e).SumInt(&TrackedTime{}, "time")
+}
+
+// GetTrackedSeconds return sum of seconds
+func GetTrackedSeconds(opts FindTrackedTimesOptions) (int64, error) {
+	return getTrackedSeconds(x, opts)
+}
+
 // AddTime will add the given time (in seconds) to the issue
-func AddTime(user *User, issue *Issue, time int64) (*TrackedTime, error) {
-	tt := &TrackedTime{
-		IssueID: issue.ID,
-		UserID:  user.ID,
-		Time:    time,
+func AddTime(user *User, issue *Issue, amount int64, created time.Time) (*TrackedTime, error) {
+	sess := x.NewSession()
+	defer sess.Close()
+
+	if err := sess.Begin(); err != nil {
+		return nil, err
 	}
-	if _, err := x.Insert(tt); err != nil {
+
+	t, err := addTime(sess, user, issue, amount, created)
+	if err != nil {
 		return nil, err
 	}
-	if err := issue.loadRepo(x); err != nil {
+
+	if err := issue.loadRepo(sess); err != nil {
 		return nil, err
 	}
-	if _, err := CreateComment(&CreateCommentOptions{
+
+	if _, err := createComment(sess, &CreateCommentOptions{
 		Issue:   issue,
 		Repo:    issue.Repo,
 		Doer:    user,
-		Content: SecToTime(time),
+		Content: SecToTime(amount),
 		Type:    CommentTypeAddTimeManual,
 	}); err != nil {
 		return nil, err
 	}
+
+	return t, sess.Commit()
+}
+
+func addTime(e Engine, user *User, issue *Issue, amount int64, created time.Time) (*TrackedTime, error) {
+	if created.IsZero() {
+		created = time.Now()
+	}
+	tt := &TrackedTime{
+		IssueID: issue.ID,
+		UserID:  user.ID,
+		Time:    amount,
+		Created: created,
+	}
+	if _, err := e.Insert(tt); err != nil {
+		return nil, err
+	}
+
 	return tt, nil
 }
 
@@ -131,3 +225,101 @@ func TotalTimes(options FindTrackedTimesOptions) (map[*User]string, error) {
 	}
 	return totalTimes, nil
 }
+
+// DeleteIssueUserTimes deletes times for issue
+func DeleteIssueUserTimes(issue *Issue, user *User) error {
+	sess := x.NewSession()
+	defer sess.Close()
+
+	if err := sess.Begin(); err != nil {
+		return err
+	}
+
+	opts := FindTrackedTimesOptions{
+		IssueID: issue.ID,
+		UserID:  user.ID,
+	}
+
+	removedTime, err := deleteTimes(sess, opts)
+	if err != nil {
+		return err
+	}
+	if removedTime == 0 {
+		return ErrNotExist{}
+	}
+
+	if err := issue.loadRepo(sess); err != nil {
+		return err
+	}
+	if _, err := createComment(sess, &CreateCommentOptions{
+		Issue:   issue,
+		Repo:    issue.Repo,
+		Doer:    user,
+		Content: "- " + SecToTime(removedTime),
+		Type:    CommentTypeDeleteTimeManual,
+	}); err != nil {
+		return err
+	}
+
+	return sess.Commit()
+}
+
+// DeleteTime delete a specific Time
+func DeleteTime(t *TrackedTime) error {
+	sess := x.NewSession()
+	defer sess.Close()
+
+	if err := sess.Begin(); err != nil {
+		return err
+	}
+
+	if err := deleteTime(sess, t); err != nil {
+		return err
+	}
+
+	if _, err := createComment(sess, &CreateCommentOptions{
+		Issue:   t.Issue,
+		Repo:    t.Issue.Repo,
+		Doer:    t.User,
+		Content: "- " + SecToTime(t.Time),
+		Type:    CommentTypeDeleteTimeManual,
+	}); err != nil {
+		return err
+	}
+
+	return sess.Commit()
+}
+
+func deleteTimes(e Engine, opts FindTrackedTimesOptions) (removedTime int64, err error) {
+
+	removedTime, err = getTrackedSeconds(e, opts)
+	if err != nil || removedTime == 0 {
+		return
+	}
+
+	_, err = opts.ToSession(e).Table("tracked_time").Cols("deleted").Update(&TrackedTime{Deleted: true})
+	return
+}
+
+func deleteTime(e Engine, t *TrackedTime) error {
+	if t.Deleted {
+		return ErrNotExist{ID: t.ID}
+	}
+	t.Deleted = true
+	_, err := e.ID(t.ID).Cols("deleted").Update(t)
+	return err
+}
+
+// GetTrackedTimeByID returns raw TrackedTime without loading attributes by id
+func GetTrackedTimeByID(id int64) (*TrackedTime, error) {
+	time := &TrackedTime{
+		ID: id,
+	}
+	has, err := x.Get(time)
+	if err != nil {
+		return nil, err
+	} else if !has {
+		return nil, ErrNotExist{ID: id}
+	}
+	return time, nil
+}
diff --git a/models/issue_tracked_time_test.go b/models/issue_tracked_time_test.go
index 130e8f33e2..551c918d73 100644
--- a/models/issue_tracked_time_test.go
+++ b/models/issue_tracked_time_test.go
@@ -1,7 +1,12 @@
+// Copyright 2019 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 models
 
 import (
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/assert"
 )
@@ -16,14 +21,14 @@ func TestAddTime(t *testing.T) {
 	assert.NoError(t, err)
 
 	//3661 = 1h 1min 1s
-	trackedTime, err := AddTime(user3, issue1, 3661)
+	trackedTime, err := AddTime(user3, issue1, 3661, time.Now())
 	assert.NoError(t, err)
 	assert.Equal(t, int64(3), trackedTime.UserID)
 	assert.Equal(t, int64(1), trackedTime.IssueID)
 	assert.Equal(t, int64(3661), trackedTime.Time)
 
 	tt := AssertExistsAndLoadBean(t, &TrackedTime{UserID: 3, IssueID: 1}).(*TrackedTime)
-	assert.Equal(t, tt.Time, int64(3661))
+	assert.Equal(t, int64(3661), tt.Time)
 
 	comment := AssertExistsAndLoadBean(t, &Comment{Type: CommentTypeAddTimeManual, PosterID: 3, IssueID: 1}).(*Comment)
 	assert.Equal(t, comment.Content, "1h 1min 1s")
@@ -36,7 +41,7 @@ func TestGetTrackedTimes(t *testing.T) {
 	times, err := GetTrackedTimes(FindTrackedTimesOptions{IssueID: 1})
 	assert.NoError(t, err)
 	assert.Len(t, times, 1)
-	assert.Equal(t, times[0].Time, int64(400))
+	assert.Equal(t, int64(400), times[0].Time)
 
 	times, err = GetTrackedTimes(FindTrackedTimesOptions{IssueID: -1})
 	assert.NoError(t, err)
@@ -45,8 +50,8 @@ func TestGetTrackedTimes(t *testing.T) {
 	// by User
 	times, err = GetTrackedTimes(FindTrackedTimesOptions{UserID: 1})
 	assert.NoError(t, err)
-	assert.Len(t, times, 1)
-	assert.Equal(t, times[0].Time, int64(400))
+	assert.Len(t, times, 3)
+	assert.Equal(t, int64(400), times[0].Time)
 
 	times, err = GetTrackedTimes(FindTrackedTimesOptions{UserID: 3})
 	assert.NoError(t, err)
@@ -55,15 +60,15 @@ func TestGetTrackedTimes(t *testing.T) {
 	// by Repo
 	times, err = GetTrackedTimes(FindTrackedTimesOptions{RepositoryID: 2})
 	assert.NoError(t, err)
-	assert.Len(t, times, 1)
-	assert.Equal(t, times[0].Time, int64(1))
+	assert.Len(t, times, 3)
+	assert.Equal(t, int64(1), times[0].Time)
 	issue, err := GetIssueByID(times[0].IssueID)
 	assert.NoError(t, err)
 	assert.Equal(t, issue.RepoID, int64(2))
 
 	times, err = GetTrackedTimes(FindTrackedTimesOptions{RepositoryID: 1})
 	assert.NoError(t, err)
-	assert.Len(t, times, 4)
+	assert.Len(t, times, 5)
 
 	times, err = GetTrackedTimes(FindTrackedTimesOptions{RepositoryID: 10})
 	assert.NoError(t, err)
@@ -83,10 +88,15 @@ func TestTotalTimes(t *testing.T) {
 
 	total, err = TotalTimes(FindTrackedTimesOptions{IssueID: 2})
 	assert.NoError(t, err)
-	assert.Len(t, total, 1)
+	assert.Len(t, total, 2)
 	for user, time := range total {
-		assert.Equal(t, int64(2), user.ID)
-		assert.Equal(t, "1h 1min 2s", time)
+		if user.ID == 2 {
+			assert.Equal(t, "1h 1min 2s", time)
+		} else if user.ID == 1 {
+			assert.Equal(t, "20s", time)
+		} else {
+			assert.Error(t, assert.AnError)
+		}
 	}
 
 	total, err = TotalTimes(FindTrackedTimesOptions{IssueID: 5})
@@ -99,5 +109,5 @@ func TestTotalTimes(t *testing.T) {
 
 	total, err = TotalTimes(FindTrackedTimesOptions{IssueID: 4})
 	assert.NoError(t, err)
-	assert.Len(t, total, 0)
+	assert.Len(t, total, 2)
 }
diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go
index 00d836695f..e8bb3f16d4 100644
--- a/models/migrations/migrations.go
+++ b/models/migrations/migrations.go
@@ -286,6 +286,8 @@ var migrations = []Migration{
 	NewMigration("Remove authentication credentials from stored URL", sanitizeOriginalURL),
 	// v115 -> v116
 	NewMigration("add user_id prefix to existing user avatar name", renameExistingUserAvatarName),
+	// v116 -> v117
+	NewMigration("Extend TrackedTimes", extendTrackedTimes),
 }
 
 // Migrate database to current version
diff --git a/models/migrations/v116.go b/models/migrations/v116.go
new file mode 100644
index 0000000000..6587d02f13
--- /dev/null
+++ b/models/migrations/v116.go
@@ -0,0 +1,30 @@
+// Copyright 2019 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 migrations
+
+import (
+	"code.gitea.io/gitea/models"
+
+	"xorm.io/xorm"
+)
+
+func extendTrackedTimes(x *xorm.Engine) error {
+	sess := x.NewSession()
+	defer sess.Close()
+
+	if err := sess.Begin(); err != nil {
+		return err
+	}
+
+	if _, err := sess.Exec("DELETE FROM tracked_time WHERE time IS NULL"); err != nil {
+		return err
+	}
+
+	if err := sess.Sync2(new(models.TrackedTime)); err != nil {
+		return err
+	}
+
+	return sess.Commit()
+}