From 57da7cab8b2adc00f3609064c36e1f925e162ae0 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sat, 9 May 2020 03:25:57 +0200 Subject: [PATCH 1/4] Fix nil exeption: #11313 --- models/issue_tracked_time.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/models/issue_tracked_time.go b/models/issue_tracked_time.go index 49fefd327985..a6ca34cf107f 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 } From 7c537b366ffb67987cdbd45e4fd832d3c985668a Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sat, 9 May 2020 03:26:16 +0200 Subject: [PATCH 2/4] fix 500 --- models/issue_tracked_time.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/models/issue_tracked_time.go b/models/issue_tracked_time.go index a6ca34cf107f..83fe216f5a57 100644 --- a/models/issue_tracked_time.go +++ b/models/issue_tracked_time.go @@ -303,13 +303,11 @@ 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 { + } else if !has || time.Deleted { return nil, ErrNotExist{ID: id} } return time, nil From 69d6c62bbd017851b460db71e05c7d0d2c8ad9ca Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sat, 9 May 2020 03:26:55 +0200 Subject: [PATCH 3/4] =?UTF-8?q?activate=20test=20=F0=9F=98=86?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- integrations/api_issue_tracked_time_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/integrations/api_issue_tracked_time_test.go b/integrations/api_issue_tracked_time_test.go index 97d401ff9d87..cf044858b809 100644 --- a/integrations/api_issue_tracked_time_test.go +++ b/integrations/api_issue_tracked_time_test.go @@ -72,12 +72,12 @@ 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}) From 5acfdbf4bbadb2ff36cc6fdb3594bf966cf396e7 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sat, 9 May 2020 03:35:54 +0200 Subject: [PATCH 4/4] move logic --- integrations/api_issue_tracked_time_test.go | 2 +- models/issue_tracked_time.go | 2 +- routers/api/v1/repo/issue_tracked_time.go | 4 ++++ 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/integrations/api_issue_tracked_time_test.go b/integrations/api_issue_tracked_time_test.go index cf044858b809..1a0ee99a43ad 100644 --- a/integrations/api_issue_tracked_time_test.go +++ b/integrations/api_issue_tracked_time_test.go @@ -82,7 +82,7 @@ func TestAPIDeleteTrackedTime(t *testing.T) { //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 83fe216f5a57..195f3e785034 100644 --- a/models/issue_tracked_time.go +++ b/models/issue_tracked_time.go @@ -307,7 +307,7 @@ func GetTrackedTimeByID(id int64) (*TrackedTime, error) { has, err := x.ID(id).Get(time) if err != nil { return nil, err - } else if !has || time.Deleted { + } else if !has { return nil, ErrNotExist{ID: id} } return time, nil diff --git a/routers/api/v1/repo/issue_tracked_time.go b/routers/api/v1/repo/issue_tracked_time.go index 66f8a0879ad7..19242d1fae78 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