From cd4f7ba5bf7191c2ca91438d8903ee9f28f406b9 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sat, 9 May 2020 16:18:44 +0200 Subject: [PATCH] Fix tracked time issues (#11349) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix nil exeption: #11313 * fix 500 * activate test 😆 * move logic --- integrations/api_issue_tracked_time_test.go | 6 +++--- models/issue_tracked_time.go | 10 ++++++---- routers/api/v1/repo/issue_tracked_time.go | 4 ++++ 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/integrations/api_issue_tracked_time_test.go b/integrations/api_issue_tracked_time_test.go index 97d401ff9d8..1a0ee99a43a 100644 --- a/integrations/api_issue_tracked_time_test.go +++ b/integrations/api_issue_tracked_time_test.go @@ -72,17 +72,17 @@ func TestAPIDeleteTrackedTime(t *testing.T) { //Deletion not allowed req := NewRequestf(t, "DELETE", "/api/v1/repos/%s/%s/issues/%d/times/%d?token=%s", user2.Name, issue2.Repo.Name, issue2.Index, time6.ID, token) session.MakeRequest(t, req, http.StatusForbidden) - /* Delete own time <-- ToDo: timout without reason + time3 := models.AssertExistsAndLoadBean(t, &models.TrackedTime{ID: 3}).(*models.TrackedTime) req = NewRequestf(t, "DELETE", "/api/v1/repos/%s/%s/issues/%d/times/%d?token=%s", user2.Name, issue2.Repo.Name, issue2.Index, time3.ID, token) session.MakeRequest(t, req, http.StatusNoContent) //Delete non existing time - session.MakeRequest(t, req, http.StatusInternalServerError) */ + session.MakeRequest(t, req, http.StatusNotFound) //Reset time of user 2 on issue 2 trackedSeconds, err := models.GetTrackedSeconds(models.FindTrackedTimesOptions{IssueID: 2, UserID: 2}) assert.NoError(t, err) - assert.Equal(t, int64(3662), trackedSeconds) + assert.Equal(t, int64(3661), trackedSeconds) req = NewRequestf(t, "DELETE", "/api/v1/repos/%s/%s/issues/%d/times?token=%s", user2.Name, issue2.Repo.Name, issue2.Index, token) session.MakeRequest(t, req, http.StatusNoContent) diff --git a/models/issue_tracked_time.go b/models/issue_tracked_time.go index 49fefd32798..195f3e78503 100644 --- a/models/issue_tracked_time.go +++ b/models/issue_tracked_time.go @@ -260,6 +260,10 @@ func DeleteTime(t *TrackedTime) error { return err } + if err := t.loadAttributes(sess); err != nil { + return err + } + if err := deleteTime(sess, t); err != nil { return err } @@ -299,10 +303,8 @@ func deleteTime(e Engine, t *TrackedTime) error { // GetTrackedTimeByID returns raw TrackedTime without loading attributes by id func GetTrackedTimeByID(id int64) (*TrackedTime, error) { - time := &TrackedTime{ - ID: id, - } - has, err := x.Get(time) + time := new(TrackedTime) + has, err := x.ID(id).Get(time) if err != nil { return nil, err } else if !has { diff --git a/routers/api/v1/repo/issue_tracked_time.go b/routers/api/v1/repo/issue_tracked_time.go index 66f8a0879ad..19242d1fae7 100644 --- a/routers/api/v1/repo/issue_tracked_time.go +++ b/routers/api/v1/repo/issue_tracked_time.go @@ -324,6 +324,10 @@ func DeleteTime(ctx *context.APIContext) { ctx.Error(http.StatusInternalServerError, "GetTrackedTimeByID", err) return } + if time.Deleted { + ctx.NotFound(fmt.Errorf("tracked time [%d] already deleted", time.ID)) + return + } if !ctx.User.IsAdmin && time.UserID != ctx.User.ID { //Only Admin and User itself can delete their time