From 0b216f40fde79361c09f33eef98081edd97b5e01 Mon Sep 17 00:00:00 2001
From: guillep2k <18600385+guillep2k@users.noreply.github.com>
Date: Sat, 9 May 2020 14:08:41 -0300
Subject: [PATCH] Fix tracked time issues (#11349) (#11354)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Backport #11349

* Fix tracked time issues (#11349)

* Fix nil exeption: #11313

* fix 500

* activate test 😆

* move logic

* Add missing import

Co-authored-by: 6543 <6543@obermui.de>
Co-authored-by: Guillermo Prandi <guillep2k@users.noreply.github.com>
---
 integrations/api_issue_tracked_time_test.go |  6 +++---
 models/issue_tracked_time.go                | 10 ++++++----
 routers/api/v1/repo/issue_tracked_time.go   |  5 +++++
 3 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/integrations/api_issue_tracked_time_test.go b/integrations/api_issue_tracked_time_test.go
index ed6c036db..d3d1b2569 100644
--- a/integrations/api_issue_tracked_time_test.go
+++ b/integrations/api_issue_tracked_time_test.go
@@ -60,17 +60,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 bcb163f3c..6feccb567 100644
--- a/models/issue_tracked_time.go
+++ b/models/issue_tracked_time.go
@@ -273,6 +273,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
 	}
@@ -312,10 +316,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 ec49fdbfd..18ce8a124 100644
--- a/routers/api/v1/repo/issue_tracked_time.go
+++ b/routers/api/v1/repo/issue_tracked_time.go
@@ -5,6 +5,7 @@
 package repo
 
 import (
+	"fmt"
 	"net/http"
 	"time"
 
@@ -296,6 +297,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