times Add filters (#9373)

(extend #9200)
 * add query param for GET functions (created Bevore & after)
 * add test
 * generalize func GetQueryBeforeSince

Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>

6 files changed, 234 insertions, 32 deletions

diff --git a/integrations/api_issue_tracked_time_test.go b/integrations/api_issue_tracked_time_test.go
index ed6c036db6..97d401ff9d 100644
--- a/integrations/api_issue_tracked_time_test.go
+++ b/integrations/api_issue_tracked_time_test.go
@@ -44,6 +44,18 @@ func TestAPIGetTrackedTimes(t *testing.T) {
 		assert.NoError(t, err)
 		assert.Equal(t, user.Name, apiTimes[i].UserName)
 	}
+
+	// test filter
+	since := "2000-01-01T00%3A00%3A02%2B00%3A00"  //946684802
+	before := "2000-01-01T00%3A00%3A12%2B00%3A00" //946684812
+
+	req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/issues/%d/times?since=%s&before=%s&token=%s", user2.Name, issue2.Repo.Name, issue2.Index, since, before, token)
+	resp = session.MakeRequest(t, req, http.StatusOK)
+	var filterAPITimes api.TrackedTimeList
+	DecodeJSON(t, resp, &filterAPITimes)
+	assert.Len(t, filterAPITimes, 2)
+	assert.Equal(t, int64(3), filterAPITimes[0].ID)
+	assert.Equal(t, int64(6), filterAPITimes[1].ID)
 }
 
 func TestAPIDeleteTrackedTime(t *testing.T) {
diff --git a/models/issue_tracked_time.go b/models/issue_tracked_time.go
index bcb163f3c5..b84adbc59a 100644
--- a/models/issue_tracked_time.go
+++ b/models/issue_tracked_time.go
@@ -100,10 +100,12 @@ func (tl TrackedTimeList) APIFormat() api.TrackedTimeList {
 
 // FindTrackedTimesOptions represent the filters for tracked times. If an ID is 0 it will be ignored.
 type FindTrackedTimesOptions struct {
-	IssueID      int64
-	UserID       int64
-	RepositoryID int64
-	MilestoneID  int64
+	IssueID           int64
+	UserID            int64
+	RepositoryID      int64
+	MilestoneID       int64
+	CreatedAfterUnix  int64
+	CreatedBeforeUnix int64
 }
 
 // ToCond will convert each condition into a xorm-Cond
@@ -121,6 +123,12 @@ func (opts *FindTrackedTimesOptions) ToCond() builder.Cond {
 	if opts.MilestoneID != 0 {
 		cond = cond.And(builder.Eq{"issue.milestone_id": opts.MilestoneID})
 	}
+	if opts.CreatedAfterUnix != 0 {
+		cond = cond.And(builder.Gte{"tracked_time.created_unix": opts.CreatedAfterUnix})
+	}
+	if opts.CreatedBeforeUnix != 0 {
+		cond = cond.And(builder.Lte{"tracked_time.created_unix": opts.CreatedBeforeUnix})
+	}
 	return cond
 }
 
diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go
index 3f766c7a74..9f18951893 100644
--- a/routers/api/v1/api.go
+++ b/routers/api/v1/api.go
@@ -654,7 +654,7 @@ func RegisterRoutes(m *macaron.Macaron) {
 				m.Group("/times", func() {
 					m.Combo("").Get(repo.ListTrackedTimesByRepository)
 					m.Combo("/:timetrackingusername").Get(repo.ListTrackedTimesByUser)
-				}, mustEnableIssues)
+				}, mustEnableIssues, reqToken())
 				m.Group("/issues", func() {
 					m.Combo("").Get(repo.ListIssues).
 						Post(reqToken(), mustNotBeArchived, bind(api.CreateIssueOption{}), repo.CreateIssue)
@@ -688,12 +688,12 @@ func RegisterRoutes(m *macaron.Macaron) {
 							m.Delete("/:id", reqToken(), repo.DeleteIssueLabel)
 						})
 						m.Group("/times", func() {
-							m.Combo("", reqToken()).
+							m.Combo("").
 								Get(repo.ListTrackedTimes).
 								Post(bind(api.AddTimeOption{}), repo.AddTime).
 								Delete(repo.ResetIssueTime)
-							m.Delete("/:id", reqToken(), repo.DeleteTime)
-						})
+							m.Delete("/:id", repo.DeleteTime)
+						}, reqToken())
 						m.Combo("/deadline").Post(reqToken(), bind(api.EditDeadlineOption{}), repo.UpdateIssueDeadline)
 						m.Group("/stopwatch", func() {
 							m.Post("/start", reqToken(), repo.StartIssueStopwatch)
diff --git a/routers/api/v1/repo/issue_tracked_time.go b/routers/api/v1/repo/issue_tracked_time.go
index 80830e2fe6..dd959192c9 100644
--- a/routers/api/v1/repo/issue_tracked_time.go
+++ b/routers/api/v1/repo/issue_tracked_time.go
@@ -5,12 +5,15 @@
 package repo
 
 import (
+	"fmt"
 	"net/http"
+	"strings"
 	"time"
 
 	"code.gitea.io/gitea/models"
 	"code.gitea.io/gitea/modules/context"
 	api "code.gitea.io/gitea/modules/structs"
+	"code.gitea.io/gitea/routers/api/v1/utils"
 )
 
 // ListTrackedTimes list all the tracked times of an issue
@@ -37,6 +40,16 @@ func ListTrackedTimes(ctx *context.APIContext) {
 	//   type: integer
 	//   format: int64
 	//   required: true
+	// - name: since
+	//   in: query
+	//   description: Only show times updated after the given time. This is a timestamp in RFC 3339 format
+	//   type: string
+	//   format: date-time
+	// - name: before
+	//   in: query
+	//   description: Only show times updated before the given time. This is a timestamp in RFC 3339 format
+	//   type: string
+	//   format: date-time
 	// responses:
 	//   "200":
 	//     "$ref": "#/responses/TrackedTimeList"
@@ -62,6 +75,11 @@ func ListTrackedTimes(ctx *context.APIContext) {
 		IssueID:      issue.ID,
 	}
 
+	if opts.CreatedBeforeUnix, opts.CreatedAfterUnix, err = utils.GetQueryBeforeSince(ctx); err != nil {
+		ctx.InternalServerError(err)
+		return
+	}
+
 	if !ctx.IsUserRepoAdmin() && !ctx.User.IsAdmin {
 		opts.UserID = ctx.User.ID
 	}
@@ -141,7 +159,7 @@ func AddTime(ctx *context.APIContext, form api.AddTimeOption) {
 			//allow only RepoAdmin, Admin and User to add time
 			user, err = models.GetUserByName(form.User)
 			if err != nil {
-				ctx.Error(500, "GetUserByName", err)
+				ctx.Error(http.StatusInternalServerError, "GetUserByName", err)
 			}
 		}
 	}
@@ -195,33 +213,33 @@ func ResetIssueTime(ctx *context.APIContext) {
 	//   "400":
 	//     "$ref": "#/responses/error"
 	//   "403":
-	//     "$ref": "#/responses/error"
+	//     "$ref": "#/responses/forbidden"
 
 	issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index"))
 	if err != nil {
 		if models.IsErrIssueNotExist(err) {
 			ctx.NotFound(err)
 		} else {
-			ctx.Error(500, "GetIssueByIndex", err)
+			ctx.Error(http.StatusInternalServerError, "GetIssueByIndex", err)
 		}
 		return
 	}
 
 	if !ctx.Repo.CanUseTimetracker(issue, ctx.User) {
 		if !ctx.Repo.Repository.IsTimetrackerEnabled() {
-			ctx.JSON(400, struct{ Message string }{Message: "time tracking disabled"})
+			ctx.JSON(http.StatusBadRequest, struct{ Message string }{Message: "time tracking disabled"})
 			return
 		}
-		ctx.Status(403)
+		ctx.Status(http.StatusForbidden)
 		return
 	}
 
 	err = models.DeleteIssueUserTimes(issue, ctx.User)
 	if err != nil {
 		if models.IsErrNotExist(err) {
-			ctx.Error(404, "DeleteIssueUserTimes", err)
+			ctx.Error(http.StatusNotFound, "DeleteIssueUserTimes", err)
 		} else {
-			ctx.Error(500, "DeleteIssueUserTimes", err)
+			ctx.Error(http.StatusInternalServerError, "DeleteIssueUserTimes", err)
 		}
 		return
 	}
@@ -266,52 +284,53 @@ func DeleteTime(ctx *context.APIContext) {
 	//   "400":
 	//     "$ref": "#/responses/error"
 	//   "403":
-	//     "$ref": "#/responses/error"
+	//     "$ref": "#/responses/forbidden"
 
 	issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index"))
 	if err != nil {
 		if models.IsErrIssueNotExist(err) {
 			ctx.NotFound(err)
 		} else {
-			ctx.Error(500, "GetIssueByIndex", err)
+			ctx.Error(http.StatusInternalServerError, "GetIssueByIndex", err)
 		}
 		return
 	}
 
 	if !ctx.Repo.CanUseTimetracker(issue, ctx.User) {
 		if !ctx.Repo.Repository.IsTimetrackerEnabled() {
-			ctx.JSON(400, struct{ Message string }{Message: "time tracking disabled"})
+			ctx.JSON(http.StatusBadRequest, struct{ Message string }{Message: "time tracking disabled"})
 			return
 		}
-		ctx.Status(403)
+		ctx.Status(http.StatusForbidden)
 		return
 	}
 
 	time, err := models.GetTrackedTimeByID(ctx.ParamsInt64(":id"))
 	if err != nil {
-		ctx.Error(500, "GetTrackedTimeByID", err)
+		ctx.Error(http.StatusInternalServerError, "GetTrackedTimeByID", err)
 		return
 	}
 
 	if !ctx.User.IsAdmin && time.UserID != ctx.User.ID {
 		//Only Admin and User itself can delete their time
-		ctx.Status(403)
+		ctx.Status(http.StatusForbidden)
 		return
 	}
 
 	err = models.DeleteTime(time)
 	if err != nil {
-		ctx.Error(500, "DeleteTime", err)
+		ctx.Error(http.StatusInternalServerError, "DeleteTime", err)
 		return
 	}
-	ctx.Status(204)
+	ctx.Status(http.StatusNoContent)
 }
 
 // ListTrackedTimesByUser  lists all tracked times of the user
 func ListTrackedTimesByUser(ctx *context.APIContext) {
-	// swagger:operation GET /repos/{owner}/{repo}/times/{user} user userTrackedTimes
+	// swagger:operation GET /repos/{owner}/{repo}/times/{user} repository userTrackedTimes
 	// ---
 	// summary: List a user's tracked times in a repo
+	// deprecated: true
 	// produces:
 	// - application/json
 	// parameters:
@@ -335,6 +354,8 @@ func ListTrackedTimesByUser(ctx *context.APIContext) {
 	//     "$ref": "#/responses/TrackedTimeList"
 	//   "400":
 	//     "$ref": "#/responses/error"
+	//   "403":
+	//     "$ref": "#/responses/forbidden"
 
 	if !ctx.Repo.Repository.IsTimetrackerEnabled() {
 		ctx.Error(http.StatusBadRequest, "", "time tracking disabled")
@@ -353,9 +374,23 @@ func ListTrackedTimesByUser(ctx *context.APIContext) {
 		ctx.NotFound()
 		return
 	}
-	trackedTimes, err := models.GetTrackedTimes(models.FindTrackedTimesOptions{
+
+	if !ctx.IsUserRepoAdmin() && !ctx.User.IsAdmin && ctx.User.ID != user.ID {
+		ctx.Error(http.StatusForbidden, "", fmt.Errorf("query user not allowed not enouth rights"))
+		return
+	}
+
+	if !ctx.IsUserRepoAdmin() && !ctx.User.IsAdmin && ctx.User.ID != user.ID {
+		ctx.Error(http.StatusForbidden, "", fmt.Errorf("query user not allowed not enouth rights"))
+		return
+	}
+
+	opts := models.FindTrackedTimesOptions{
 		UserID:       user.ID,
-		RepositoryID: ctx.Repo.Repository.ID})
+		RepositoryID: ctx.Repo.Repository.ID,
+	}
+
+	trackedTimes, err := models.GetTrackedTimes(opts)
 	if err != nil {
 		ctx.Error(http.StatusInternalServerError, "GetTrackedTimes", err)
 		return
@@ -385,11 +420,27 @@ func ListTrackedTimesByRepository(ctx *context.APIContext) {
 	//   description: name of the repo
 	//   type: string
 	//   required: true
+	// - name: user
+	//   in: query
+	//   description: optional filter by user
+	//   type: string
+	// - name: since
+	//   in: query
+	//   description: Only show times updated after the given time. This is a timestamp in RFC 3339 format
+	//   type: string
+	//   format: date-time
+	// - name: before
+	//   in: query
+	//   description: Only show times updated before the given time. This is a timestamp in RFC 3339 format
+	//   type: string
+	//   format: date-time
 	// responses:
 	//   "200":
 	//     "$ref": "#/responses/TrackedTimeList"
 	//   "400":
 	//     "$ref": "#/responses/error"
+	//   "403":
+	//     "$ref": "#/responses/forbidden"
 
 	if !ctx.Repo.Repository.IsTimetrackerEnabled() {
 		ctx.Error(http.StatusBadRequest, "", "time tracking disabled")
@@ -400,8 +451,30 @@ func ListTrackedTimesByRepository(ctx *context.APIContext) {
 		RepositoryID: ctx.Repo.Repository.ID,
 	}
 
+	// Filters
+	qUser := strings.Trim(ctx.Query("user"), " ")
+	if qUser != "" {
+		user, err := models.GetUserByName(qUser)
+		if err != nil {
+			ctx.Error(http.StatusInternalServerError, "GetUserByName", err)
+			return
+		}
+		opts.UserID = user.ID
+	}
+
+	var err error
+	if opts.CreatedBeforeUnix, opts.CreatedAfterUnix, err = utils.GetQueryBeforeSince(ctx); err != nil {
+		ctx.InternalServerError(err)
+		return
+	}
+
 	if !ctx.IsUserRepoAdmin() && !ctx.User.IsAdmin {
-		opts.UserID = ctx.User.ID
+		if opts.UserID == 0 {
+			opts.UserID = ctx.User.ID
+		} else {
+			ctx.Error(http.StatusForbidden, "", fmt.Errorf("query user not allowed not enouth rights"))
+			return
+		}
 	}
 
 	trackedTimes, err := models.GetTrackedTimes(opts)
@@ -423,18 +496,39 @@ func ListMyTrackedTimes(ctx *context.APIContext) {
 	// summary: List the current user's tracked times
 	// produces:
 	// - application/json
+	// parameters:
+	// - name: since
+	//   in: query
+	//   description: Only show times updated after the given time. This is a timestamp in RFC 3339 format
+	//   type: string
+	//   format: date-time
+	// - name: before
+	//   in: query
+	//   description: Only show times updated before the given time. This is a timestamp in RFC 3339 format
+	//   type: string
+	//   format: date-time
 	// responses:
 	//   "200":
 	//     "$ref": "#/responses/TrackedTimeList"
 
-	trackedTimes, err := models.GetTrackedTimes(models.FindTrackedTimesOptions{UserID: ctx.User.ID})
+	opts := models.FindTrackedTimesOptions{UserID: ctx.User.ID}
+
+	var err error
+	if opts.CreatedBeforeUnix, opts.CreatedAfterUnix, err = utils.GetQueryBeforeSince(ctx); err != nil {
+		ctx.InternalServerError(err)
+		return
+	}
+
+	trackedTimes, err := models.GetTrackedTimes(opts)
 	if err != nil {
 		ctx.Error(http.StatusInternalServerError, "GetTrackedTimesByUser", err)
 		return
 	}
+
 	if err = trackedTimes.LoadAttributes(); err != nil {
 		ctx.Error(http.StatusInternalServerError, "LoadAttributes", err)
 		return
 	}
+
 	ctx.JSON(http.StatusOK, trackedTimes.APIFormat())
 }
diff --git a/routers/api/v1/utils/utils.go b/routers/api/v1/utils/utils.go
index f7c2b224d5..35f4873964 100644
--- a/routers/api/v1/utils/utils.go
+++ b/routers/api/v1/utils/utils.go
@@ -4,7 +4,12 @@
 
 package utils
 
-import "code.gitea.io/gitea/modules/context"
+import (
+	"strings"
+	"time"
+
+	"code.gitea.io/gitea/modules/context"
+)
 
 // UserID user ID of authenticated user, or 0 if not authenticated
 func UserID(ctx *context.APIContext) int64 {
@@ -13,3 +18,29 @@ func UserID(ctx *context.APIContext) int64 {
 	}
 	return ctx.User.ID
 }
+
+// GetQueryBeforeSince return parsed time (unix format) from URL query's before and since
+func GetQueryBeforeSince(ctx *context.APIContext) (before, since int64, err error) {
+	qCreatedBefore := strings.Trim(ctx.Query("before"), " ")
+	if qCreatedBefore != "" {
+		createdBefore, err := time.Parse(time.RFC3339, qCreatedBefore)
+		if err != nil {
+			return 0, 0, err
+		}
+		if !createdBefore.IsZero() {
+			before = createdBefore.Unix()
+		}
+	}
+
+	qCreatedAfter := strings.Trim(ctx.Query("since"), " ")
+	if qCreatedAfter != "" {
+		createdAfter, err := time.Parse(time.RFC3339, qCreatedAfter)
+		if err != nil {
+			return 0, 0, err
+		}
+		if !createdAfter.IsZero() {
+			since = createdAfter.Unix()
+		}
+	}
+	return before, since, nil
+}
diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl
index f37c900cca..d3e2ac6113 100644
--- a/templates/swagger/v1_json.tmpl
+++ b/templates/swagger/v1_json.tmpl
@@ -4433,6 +4433,20 @@
             "name": "index",
             "in": "path",
             "required": true
+          },
+          {
+            "type": "string",
+            "format": "date-time",
+            "description": "Only show times updated after the given time. This is a timestamp in RFC 3339 format",
+            "name": "since",
+            "in": "query"
+          },
+          {
+            "type": "string",
+            "format": "date-time",
+            "description": "Only show times updated before the given time. This is a timestamp in RFC 3339 format",
+            "name": "before",
+            "in": "query"
           }
         ],
         "responses": {
@@ -4543,7 +4557,7 @@
             "$ref": "#/responses/error"
           },
           "403": {
-            "$ref": "#/responses/error"
+            "$ref": "#/responses/forbidden"
           }
         }
       }
@@ -4601,7 +4615,7 @@
             "$ref": "#/responses/error"
           },
           "403": {
-            "$ref": "#/responses/error"
+            "$ref": "#/responses/forbidden"
           }
         }
       }
@@ -6419,6 +6433,26 @@
             "name": "repo",
             "in": "path",
             "required": true
+          },
+          {
+            "type": "string",
+            "description": "optional filter by user",
+            "name": "user",
+            "in": "query"
+          },
+          {
+            "type": "string",
+            "format": "date-time",
+            "description": "Only show times updated after the given time. This is a timestamp in RFC 3339 format",
+            "name": "since",
+            "in": "query"
+          },
+          {
+            "type": "string",
+            "format": "date-time",
+            "description": "Only show times updated before the given time. This is a timestamp in RFC 3339 format",
+            "name": "before",
+            "in": "query"
           }
         ],
         "responses": {
@@ -6427,6 +6461,9 @@
           },
           "400": {
             "$ref": "#/responses/error"
+          },
+          "403": {
+            "$ref": "#/responses/forbidden"
           }
         }
       }
@@ -6437,10 +6474,11 @@
           "application/json"
         ],
         "tags": [
-          "user"
+          "repository"
         ],
         "summary": "List a user's tracked times in a repo",
         "operationId": "userTrackedTimes",
+        "deprecated": true,
         "parameters": [
           {
             "type": "string",
@@ -6470,6 +6508,9 @@
           },
           "400": {
             "$ref": "#/responses/error"
+          },
+          "403": {
+            "$ref": "#/responses/forbidden"
           }
         }
       }
@@ -7685,6 +7726,22 @@
         ],
         "summary": "List the current user's tracked times",
         "operationId": "userCurrentTrackedTimes",
+        "parameters": [
+          {
+            "type": "string",
+            "format": "date-time",
+            "description": "Only show times updated after the given time. This is a timestamp in RFC 3339 format",
+            "name": "since",
+            "in": "query"
+          },
+          {
+            "type": "string",
+            "format": "date-time",
+            "description": "Only show times updated before the given time. This is a timestamp in RFC 3339 format",
+            "name": "before",
+            "in": "query"
+          }
+        ],
         "responses": {
           "200": {
             "$ref": "#/responses/TrackedTimeList"