Fix tracked time issues (#11349) (#11354)

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>

diff --git a/integrations/api_issue_tracked_time_test.go b/integrations/api_issue_tracked_time_test.go
index ed6c036db6f9975a5c95d6f022645d7319785a3b..d3d1b256903ec6d7cc8208003f7209d3efec6192 100644 (file)
--- 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 bcb163f3c5af415c598d46299f4d41bffdbdada6..6feccb5670319e378ca6435ecdde5a6f72b4a095 100644 (file)
--- 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 ec49fdbfdfbccea766fbc4643a49a0267e513379..18ce8a12448d8cffdd25afe4e24081f837111c5e 100644 (file)
--- 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