Skip to content

Commit 0add627

Browse files
lunnylunny
authored
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 parent ab09296 commit 0add627

File tree

6 files changed

+156
-115
lines changed

6 files changed

+156
-115
lines changed

‎models/issue_stopwatch.go‎

Lines changed: 120 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,26 @@ import (
1313
"code.gitea.io/gitea/modules/timeutil"
1414
)
1515

16+
// ErrIssueStopwatchNotExist represents an error that stopwatch is not exist
17+
type ErrIssueStopwatchNotExist struct {
18+
UserID int64
19+
IssueID int64
20+
}
21+
22+
func (err ErrIssueStopwatchNotExist) Error() string {
23+
return fmt.Sprintf("issue stopwatch doesn't exist[uid: %d, issue_id: %d", err.UserID, err.IssueID)
24+
}
25+
26+
// ErrIssueStopwatchAlreadyExist represents an error that stopwatch is already exist
27+
type ErrIssueStopwatchAlreadyExist struct {
28+
UserID int64
29+
IssueID int64
30+
}
31+
32+
func (err ErrIssueStopwatchAlreadyExist) Error() string {
33+
return fmt.Sprintf("issue stopwatch already exists[uid: %d, issue_id: %d", err.UserID, err.IssueID)
34+
}
35+
1636
// Stopwatch represents a stopwatch for time tracking.
1737
type Stopwatch struct {
1838
ID int64 `xorm:"pk autoincr"`
@@ -35,9 +55,9 @@ func (s Stopwatch) Duration() string {
3555
return SecToTime(s.Seconds())
3656
}
3757

38-
func getStopwatch(e db.Engine, userID, issueID int64) (sw *Stopwatch, exists bool, err error) {
58+
func getStopwatch(ctx context.Context, userID, issueID int64) (sw *Stopwatch, exists bool, err error) {
3959
sw = new(Stopwatch)
40-
exists, err = e.
60+
exists, err = db.GetEngine(ctx).
4161
Where("user_id = ?", userID).
4262
And("issue_id = ?", issueID).
4363
Get(sw)
@@ -66,7 +86,7 @@ func CountUserStopwatches(userID int64) (int64, error) {
6686

6787
// StopwatchExists returns true if the stopwatch exists
6888
func StopwatchExists(userID, issueID int64) bool {
69-
_, exists, _ := getStopwatch(db.GetEngine(db.DefaultContext), userID, issueID)
89+
_, exists, _ := getStopwatch(db.DefaultContext, userID, issueID)
7090
return exists
7191
}
7292

@@ -83,93 +103,122 @@ func hasUserStopwatch(e db.Engine, userID int64) (exists bool, sw *Stopwatch, er
83103
return
84104
}
85105

86-
// CreateOrStopIssueStopwatch will create or remove a stopwatch and will log it into issue's timeline.
87-
func CreateOrStopIssueStopwatch(user *User, issue *Issue) error {
88-
ctx, committer, err := db.TxContext()
106+
// FinishIssueStopwatchIfPossible if stopwatch exist then finish it otherwise ignore
107+
func FinishIssueStopwatchIfPossible(ctx context.Context, user *User, issue *Issue) error {
108+
_, exists, err := getStopwatch(ctx, user.ID, issue.ID)
89109
if err != nil {
90110
return err
91111
}
92-
defer committer.Close()
93-
if err := createOrStopIssueStopwatch(ctx, user, issue); err != nil {
112+
if !exists {
113+
return nil
114+
}
115+
return FinishIssueStopwatch(ctx, user, issue)
116+
}
117+
118+
// CreateOrStopIssueStopwatch create an issue stopwatch if it's not exist, otherwise finish it
119+
func CreateOrStopIssueStopwatch(user *User, issue *Issue) error {
120+
_, exists, err := getStopwatch(db.DefaultContext, user.ID, issue.ID)
121+
if err != nil {
94122
return err
95123
}
96-
return committer.Commit()
124+
if exists {
125+
return FinishIssueStopwatch(db.DefaultContext, user, issue)
126+
}
127+
return CreateIssueStopwatch(db.DefaultContext, user, issue)
97128
}
98129

99-
func createOrStopIssueStopwatch(ctx context.Context, user *User, issue *Issue) error {
100-
e := db.GetEngine(ctx)
101-
sw, exists, err := getStopwatch(e, user.ID, issue.ID)
130+
// FinishIssueStopwatch if stopwatch exist then finish it otherwise return an error
131+
func FinishIssueStopwatch(ctx context.Context, user *User, issue *Issue) error {
132+
sw, exists, err := getStopwatch(ctx, user.ID, issue.ID)
102133
if err != nil {
103134
return err
104135
}
105-
if err := issue.loadRepo(e); err != nil {
136+
if !exists {
137+
return ErrIssueStopwatchNotExist{
138+
UserID: user.ID,
139+
IssueID: issue.ID,
140+
}
141+
}
142+
143+
// Create tracked time out of the time difference between start date and actual date
144+
timediff := time.Now().Unix() - int64(sw.CreatedUnix)
145+
146+
// Create TrackedTime
147+
tt := &TrackedTime{
148+
Created: time.Now(),
149+
IssueID: issue.ID,
150+
UserID: user.ID,
151+
Time: timediff,
152+
}
153+
154+
if err := db.Insert(ctx, tt); err != nil {
106155
return err
107156
}
108157

109-
if exists {
110-
// Create tracked time out of the time difference between start date and actual date
111-
timediff := time.Now().Unix() - int64(sw.CreatedUnix)
158+
if err := issue.loadRepo(db.GetEngine(ctx)); err != nil {
159+
return err
160+
}
112161

113-
// Create TrackedTime
114-
tt := &TrackedTime{
115-
Created: time.Now(),
116-
IssueID: issue.ID,
117-
UserID: user.ID,
118-
Time: timediff,
119-
}
162+
if _, err := createComment(ctx, &CreateCommentOptions{
163+
Doer: user,
164+
Issue: issue,
165+
Repo: issue.Repo,
166+
Content: SecToTime(timediff),
167+
Type: CommentTypeStopTracking,
168+
TimeID: tt.ID,
169+
}); err != nil {
170+
return err
171+
}
172+
_, err = db.GetEngine(ctx).Delete(sw)
173+
return err
174+
}
120175

121-
if _, err := e.Insert(tt); err != nil {
122-
return err
123-
}
176+
// CreateIssueStopwatch creates a stopwatch if not exist, otherwise return an error
177+
func CreateIssueStopwatch(ctx context.Context, user *User, issue *Issue) error {
178+
e := db.GetEngine(ctx)
179+
if err := issue.loadRepo(e); err != nil {
180+
return err
181+
}
124182

125-
if _, err := createComment(ctx, &CreateCommentOptions{
126-
Doer: user,
127-
Issue: issue,
128-
Repo: issue.Repo,
129-
Content: SecToTime(timediff),
130-
Type: CommentTypeStopTracking,
131-
TimeID: tt.ID,
132-
}); err != nil {
133-
return err
134-
}
135-
if _, err := e.Delete(sw); err != nil {
136-
return err
137-
}
138-
} else {
139-
// if another stopwatch is running: stop it
140-
exists, sw, err := hasUserStopwatch(e, user.ID)
183+
// if another stopwatch is running: stop it
184+
exists, sw, err := hasUserStopwatch(e, user.ID)
185+
if err != nil {
186+
return err
187+
}
188+
if exists {
189+
issue, err := getIssueByID(e, sw.IssueID)
141190
if err != nil {
142191
return err
143192
}
144-
if exists {
145-
issue, err := getIssueByID(e, sw.IssueID)
146-
if err != nil {
147-
return err
148-
}
149-
if err := createOrStopIssueStopwatch(ctx, user, issue); err != nil {
150-
return err
151-
}
152-
}
153193

154-
// Create stopwatch
155-
sw = &Stopwatch{
156-
UserID: user.ID,
157-
IssueID: issue.ID,
158-
}
159-
160-
if err := db.Insert(ctx, sw); err != nil {
194+
if err := FinishIssueStopwatch(ctx, user, issue); err != nil {
161195
return err
162196
}
197+
}
163198

164-
if _, err := createComment(ctx, &CreateCommentOptions{
165-
Doer: user,
166-
Issue: issue,
167-
Repo: issue.Repo,
168-
Type: CommentTypeStartTracking,
169-
}); err != nil {
170-
return err
171-
}
199+
// Create stopwatch
200+
sw = &Stopwatch{
201+
UserID: user.ID,
202+
IssueID: issue.ID,
203+
}
204+
205+
if err := db.Insert(ctx, sw); err != nil {
206+
return err
172207
}
208+
209+
if err := issue.loadRepo(db.GetEngine(ctx)); err != nil {
210+
return err
211+
}
212+
213+
if _, err := createComment(ctx, &CreateCommentOptions{
214+
Doer: user,
215+
Issue: issue,
216+
Repo: issue.Repo,
217+
Type: CommentTypeStartTracking,
218+
}); err != nil {
219+
return err
220+
}
221+
173222
return nil
174223
}
175224

@@ -188,7 +237,7 @@ func CancelStopwatch(user *User, issue *Issue) error {
188237

189238
func cancelStopwatch(ctx context.Context, user *User, issue *Issue) error {
190239
e := db.GetEngine(ctx)
191-
sw, exists, err := getStopwatch(e, user.ID, issue.ID)
240+
sw, exists, err := getStopwatch(ctx, user.ID, issue.ID)
192241
if err != nil {
193242
return err
194243
}
@@ -202,6 +251,10 @@ func cancelStopwatch(ctx context.Context, user *User, issue *Issue) error {
202251
return err
203252
}
204253

254+
if err := issue.loadRepo(db.GetEngine(ctx)); err != nil {
255+
return err
256+
}
257+
205258
if _, err := createComment(ctx, &CreateCommentOptions{
206259
Doer: user,
207260
Issue: issue,

‎routers/api/v1/repo/issue_stopwatch.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"net/http"
1010

1111
"code.gitea.io/gitea/models"
12+
"code.gitea.io/gitea/models/db"
1213
"code.gitea.io/gitea/modules/context"
1314
"code.gitea.io/gitea/modules/convert"
1415
"code.gitea.io/gitea/routers/api/v1/utils"
@@ -55,7 +56,7 @@ func StartIssueStopwatch(ctx *context.APIContext) {
5556
return
5657
}
5758

58-
if err := models.CreateOrStopIssueStopwatch(ctx.User, issue); err != nil {
59+
if err := models.CreateIssueStopwatch(db.DefaultContext, ctx.User, issue); err != nil {
5960
ctx.Error(http.StatusInternalServerError, "CreateOrStopIssueStopwatch", err)
6061
return
6162
}
@@ -104,7 +105,7 @@ func StopIssueStopwatch(ctx *context.APIContext) {
104105
return
105106
}
106107

107-
if err := models.CreateOrStopIssueStopwatch(ctx.User, issue); err != nil {
108+
if err := models.FinishIssueStopwatch(db.DefaultContext, ctx.User, issue); err != nil {
108109
ctx.Error(http.StatusInternalServerError, "CreateOrStopIssueStopwatch", err)
109110
return
110111
}

‎modules/repofiles/action.go‎ renamed to ‎services/issue/commit.go‎

Lines changed: 12 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
// Copyright 2019 The Gitea Authors. All rights reserved.
1+
// Copyright 2021 The Gitea Authors. All rights reserved.
22
// Use of this source code is governed by a MIT-style
33
// license that can be found in the LICENSE file.
44

5-
package repofiles
5+
package issue
66

77
import (
88
"fmt"
@@ -14,7 +14,6 @@ import (
1414
"time"
1515

1616
"code.gitea.io/gitea/models"
17-
"code.gitea.io/gitea/modules/notification"
1817
"code.gitea.io/gitea/modules/references"
1918
"code.gitea.io/gitea/modules/repository"
2019
)
@@ -29,19 +28,6 @@ const (
2928

3029
var reDuration = regexp.MustCompile(`(?i)^(?:(\d+([\.,]\d+)?)(?:mo))?(?:(\d+([\.,]\d+)?)(?:w))?(?:(\d+([\.,]\d+)?)(?:d))?(?:(\d+([\.,]\d+)?)(?:h))?(?:(\d+([\.,]\d+)?)(?:m))?$`)
3130

32-
// getIssueFromRef returns the issue referenced by a ref. Returns a nil *Issue
33-
// if the provided ref references a non-existent issue.
34-
func getIssueFromRef(repo *models.Repository, index int64) (*models.Issue, error) {
35-
issue, err := models.GetIssueByIndex(repo.ID, index)
36-
if err != nil {
37-
if models.IsErrIssueNotExist(err) {
38-
return nil, nil
39-
}
40-
return nil, err
41-
}
42-
return issue, nil
43-
}
44-
4531
// timeLogToAmount parses time log string and returns amount in seconds
4632
func timeLogToAmount(str string) int64 {
4733
matches := reDuration.FindAllStringSubmatch(str, -1)
@@ -96,31 +82,17 @@ func issueAddTime(issue *models.Issue, doer *models.User, time time.Time, timeLo
9682
return err
9783
}
9884

99-
func changeIssueStatus(repo *models.Repository, issue *models.Issue, doer *models.User, closed bool) error {
100-
stopTimerIfAvailable := func(doer *models.User, issue *models.Issue) error {
101-
102-
if models.StopwatchExists(doer.ID, issue.ID) {
103-
if err := models.CreateOrStopIssueStopwatch(doer, issue); err != nil {
104-
return err
105-
}
106-
}
107-
108-
return nil
109-
}
110-
111-
issue.Repo = repo
112-
comment, err := issue.ChangeStatus(doer, closed)
85+
// getIssueFromRef returns the issue referenced by a ref. Returns a nil *Issue
86+
// if the provided ref references a non-existent issue.
87+
func getIssueFromRef(repo *models.Repository, index int64) (*models.Issue, error) {
88+
issue, err := models.GetIssueByIndex(repo.ID, index)
11389
if err != nil {
114-
// Don't return an error when dependencies are open as this would let the push fail
115-
if models.IsErrDependenciesLeft(err) {
116-
return stopTimerIfAvailable(doer, issue)
90+
if models.IsErrIssueNotExist(err) {
91+
return nil, nil
11792
}
118-
return err
93+
return nil, err
11994
}
120-
121-
notification.NotifyIssueChangeStatus(doer, issue, comment, closed)
122-
123-
return stopTimerIfAvailable(doer, issue)
95+
return issue, nil
12496
}
12597

12698
// UpdateIssuesCommit checks if issues are manipulated by commit message.
@@ -209,7 +181,8 @@ func UpdateIssuesCommit(doer *models.User, repo *models.Repository, commits []*r
209181
}
210182
}
211183
if close != refIssue.IsClosed {
212-
if err := changeIssueStatus(refRepo, refIssue, doer, close); err != nil {
184+
refIssue.Repo = refRepo
185+
if err := ChangeStatus(refIssue, doer, close); err != nil {
213186
return err
214187
}
215188
}

‎modules/repofiles/action_test.go‎ renamed to ‎services/issue/commit_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Use of this source code is governed by a MIT-style
33
// license that can be found in the LICENSE file.
44

5-
package repofiles
5+
package issue
66

77
import (
88
"testing"

0 commit comments

Comments
 (0)