From 14a96874442a13bb212affb13a585f0536d89c2a Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Wed, 8 Jan 2020 22:14:00 +0100 Subject: [PATCH] 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 --- integrations/api_issue_tracked_time_test.go | 12 ++ models/issue_tracked_time.go | 16 ++- routers/api/v1/api.go | 8 +- routers/api/v1/repo/issue_tracked_time.go | 134 +++++++++++++++++--- routers/api/v1/utils/utils.go | 33 ++++- templates/swagger/v1_json.tmpl | 63 ++++++++- 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 ed6c036db..97d401ff9 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 bcb163f3c..b84adbc59 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 3f766c7a7..9f1895189 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 80830e2fe..dd959192c 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 f7c2b224d..35f487396 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 f37c900cc..d3e2ac611 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"