Fix close issue but time watcher still running (#17643)

* Fix close issue but time watcher still running

* refactor stopwatch codes

* Fix test

* Fix test

* Fix typo

* Fix test

1 files changed, 120 insertions, 67 deletions

diff --git a/models/issue_stopwatch.go b/models/issue_stopwatch.go
index e9c38b9165..eaec5d924e 100644
--- a/models/issue_stopwatch.go
+++ b/models/issue_stopwatch.go
@@ -13,6 +13,26 @@ import (
 	"code.gitea.io/gitea/modules/timeutil"
 )
 
+// ErrIssueStopwatchNotExist represents an error that stopwatch is not exist
+type ErrIssueStopwatchNotExist struct {
+	UserID  int64
+	IssueID int64
+}
+
+func (err ErrIssueStopwatchNotExist) Error() string {
+	return fmt.Sprintf("issue stopwatch doesn't exist[uid: %d, issue_id: %d", err.UserID, err.IssueID)
+}
+
+// ErrIssueStopwatchAlreadyExist represents an error that stopwatch is already exist
+type ErrIssueStopwatchAlreadyExist struct {
+	UserID  int64
+	IssueID int64
+}
+
+func (err ErrIssueStopwatchAlreadyExist) Error() string {
+	return fmt.Sprintf("issue stopwatch already exists[uid: %d, issue_id: %d", err.UserID, err.IssueID)
+}
+
 // Stopwatch represents a stopwatch for time tracking.
 type Stopwatch struct {
 	ID          int64              `xorm:"pk autoincr"`
@@ -35,9 +55,9 @@ func (s Stopwatch) Duration() string {
 	return SecToTime(s.Seconds())
 }
 
-func getStopwatch(e db.Engine, userID, issueID int64) (sw *Stopwatch, exists bool, err error) {
+func getStopwatch(ctx context.Context, userID, issueID int64) (sw *Stopwatch, exists bool, err error) {
 	sw = new(Stopwatch)
-	exists, err = e.
+	exists, err = db.GetEngine(ctx).
 		Where("user_id = ?", userID).
 		And("issue_id = ?", issueID).
 		Get(sw)
@@ -66,7 +86,7 @@ func CountUserStopwatches(userID int64) (int64, error) {
 
 // StopwatchExists returns true if the stopwatch exists
 func StopwatchExists(userID, issueID int64) bool {
-	_, exists, _ := getStopwatch(db.GetEngine(db.DefaultContext), userID, issueID)
+	_, exists, _ := getStopwatch(db.DefaultContext, userID, issueID)
 	return exists
 }
 
@@ -83,93 +103,122 @@ func hasUserStopwatch(e db.Engine, userID int64) (exists bool, sw *Stopwatch, er
 	return
 }
 
-// CreateOrStopIssueStopwatch will create or remove a stopwatch and will log it into issue's timeline.
-func CreateOrStopIssueStopwatch(user *User, issue *Issue) error {
-	ctx, committer, err := db.TxContext()
+// FinishIssueStopwatchIfPossible if stopwatch exist then finish it otherwise ignore
+func FinishIssueStopwatchIfPossible(ctx context.Context, user *User, issue *Issue) error {
+	_, exists, err := getStopwatch(ctx, user.ID, issue.ID)
 	if err != nil {
 		return err
 	}
-	defer committer.Close()
-	if err := createOrStopIssueStopwatch(ctx, user, issue); err != nil {
+	if !exists {
+		return nil
+	}
+	return FinishIssueStopwatch(ctx, user, issue)
+}
+
+// CreateOrStopIssueStopwatch create an issue stopwatch if it's not exist, otherwise finish it
+func CreateOrStopIssueStopwatch(user *User, issue *Issue) error {
+	_, exists, err := getStopwatch(db.DefaultContext, user.ID, issue.ID)
+	if err != nil {
 		return err
 	}
-	return committer.Commit()
+	if exists {
+		return FinishIssueStopwatch(db.DefaultContext, user, issue)
+	}
+	return CreateIssueStopwatch(db.DefaultContext, user, issue)
 }
 
-func createOrStopIssueStopwatch(ctx context.Context, user *User, issue *Issue) error {
-	e := db.GetEngine(ctx)
-	sw, exists, err := getStopwatch(e, user.ID, issue.ID)
+// FinishIssueStopwatch if stopwatch exist then finish it otherwise return an error
+func FinishIssueStopwatch(ctx context.Context, user *User, issue *Issue) error {
+	sw, exists, err := getStopwatch(ctx, user.ID, issue.ID)
 	if err != nil {
 		return err
 	}
-	if err := issue.loadRepo(e); err != nil {
+	if !exists {
+		return ErrIssueStopwatchNotExist{
+			UserID:  user.ID,
+			IssueID: issue.ID,
+		}
+	}
+
+	// Create tracked time out of the time difference between start date and actual date
+	timediff := time.Now().Unix() - int64(sw.CreatedUnix)
+
+	// Create TrackedTime
+	tt := &TrackedTime{
+		Created: time.Now(),
+		IssueID: issue.ID,
+		UserID:  user.ID,
+		Time:    timediff,
+	}
+
+	if err := db.Insert(ctx, tt); err != nil {
 		return err
 	}
 
-	if exists {
-		// Create tracked time out of the time difference between start date and actual date
-		timediff := time.Now().Unix() - int64(sw.CreatedUnix)
+	if err := issue.loadRepo(db.GetEngine(ctx)); err != nil {
+		return err
+	}
 
-		// Create TrackedTime
-		tt := &TrackedTime{
-			Created: time.Now(),
-			IssueID: issue.ID,
-			UserID:  user.ID,
-			Time:    timediff,
-		}
+	if _, err := createComment(ctx, &CreateCommentOptions{
+		Doer:    user,
+		Issue:   issue,
+		Repo:    issue.Repo,
+		Content: SecToTime(timediff),
+		Type:    CommentTypeStopTracking,
+		TimeID:  tt.ID,
+	}); err != nil {
+		return err
+	}
+	_, err = db.GetEngine(ctx).Delete(sw)
+	return err
+}
 
-		if _, err := e.Insert(tt); err != nil {
-			return err
-		}
+// CreateIssueStopwatch creates a stopwatch if not exist, otherwise return an error
+func CreateIssueStopwatch(ctx context.Context, user *User, issue *Issue) error {
+	e := db.GetEngine(ctx)
+	if err := issue.loadRepo(e); err != nil {
+		return err
+	}
 
-		if _, err := createComment(ctx, &CreateCommentOptions{
-			Doer:    user,
-			Issue:   issue,
-			Repo:    issue.Repo,
-			Content: SecToTime(timediff),
-			Type:    CommentTypeStopTracking,
-			TimeID:  tt.ID,
-		}); err != nil {
-			return err
-		}
-		if _, err := e.Delete(sw); err != nil {
-			return err
-		}
-	} else {
-		// if another stopwatch is running: stop it
-		exists, sw, err := hasUserStopwatch(e, user.ID)
+	// if another stopwatch is running: stop it
+	exists, sw, err := hasUserStopwatch(e, user.ID)
+	if err != nil {
+		return err
+	}
+	if exists {
+		issue, err := getIssueByID(e, sw.IssueID)
 		if err != nil {
 			return err
 		}
-		if exists {
-			issue, err := getIssueByID(e, sw.IssueID)
-			if err != nil {
-				return err
-			}
-			if err := createOrStopIssueStopwatch(ctx, user, issue); err != nil {
-				return err
-			}
-		}
 
-		// Create stopwatch
-		sw = &Stopwatch{
-			UserID:  user.ID,
-			IssueID: issue.ID,
-		}
-
-		if err := db.Insert(ctx, sw); err != nil {
+		if err := FinishIssueStopwatch(ctx, user, issue); err != nil {
 			return err
 		}
+	}
 
-		if _, err := createComment(ctx, &CreateCommentOptions{
-			Doer:  user,
-			Issue: issue,
-			Repo:  issue.Repo,
-			Type:  CommentTypeStartTracking,
-		}); err != nil {
-			return err
-		}
+	// Create stopwatch
+	sw = &Stopwatch{
+		UserID:  user.ID,
+		IssueID: issue.ID,
+	}
+
+	if err := db.Insert(ctx, sw); err != nil {
+		return err
 	}
+
+	if err := issue.loadRepo(db.GetEngine(ctx)); err != nil {
+		return err
+	}
+
+	if _, err := createComment(ctx, &CreateCommentOptions{
+		Doer:  user,
+		Issue: issue,
+		Repo:  issue.Repo,
+		Type:  CommentTypeStartTracking,
+	}); err != nil {
+		return err
+	}
+
 	return nil
 }
 
@@ -188,7 +237,7 @@ func CancelStopwatch(user *User, issue *Issue) error {
 
 func cancelStopwatch(ctx context.Context, user *User, issue *Issue) error {
 	e := db.GetEngine(ctx)
-	sw, exists, err := getStopwatch(e, user.ID, issue.ID)
+	sw, exists, err := getStopwatch(ctx, user.ID, issue.ID)
 	if err != nil {
 		return err
 	}
@@ -202,6 +251,10 @@ func cancelStopwatch(ctx context.Context, user *User, issue *Issue) error {
 			return err
 		}
 
+		if err := issue.loadRepo(db.GetEngine(ctx)); err != nil {
+			return err
+		}
+
 		if _, err := createComment(ctx, &CreateCommentOptions{
 			Doer:  user,
 			Issue: issue,