Skip to content

Commit 714ecd9

Browse files
lunnyzeripath
lunny
and
zeripath
authored
Fix close issue but time watcher still running (#17761)
* Fix bug * Update models/issue_stopwatch.go Co-authored-by: zeripath <[email protected]> Co-authored-by: zeripath <[email protected]>
1 parent a088566 commit 714ecd9

File tree

6 files changed

+151
-94
lines changed

6 files changed

+151
-94
lines changed

‎models/issue_stopwatch.go‎

Lines changed: 126 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,26 @@ import (
1313
"xorm.io/xorm"
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"`
@@ -74,91 +94,141 @@ func hasUserStopwatch(e Engine, userID int64) (exists bool, sw *Stopwatch, err e
7494
return
7595
}
7696

97+
// FinishIssueStopwatchIfPossible if stopwatch exist then finish it otherwise ignore
98+
func FinishIssueStopwatchIfPossible(user *User, issue *Issue) error {
99+
_, exists, err := getStopwatch(x, user.ID, issue.ID)
100+
if err != nil {
101+
return err
102+
}
103+
if !exists {
104+
return nil
105+
}
106+
return FinishIssueStopwatch(user, issue)
107+
}
108+
77109
// CreateOrStopIssueStopwatch will create or remove a stopwatch and will log it into issue's timeline.
78110
func CreateOrStopIssueStopwatch(user *User, issue *Issue) error {
111+
_, exists, err := getStopwatch(x, user.ID, issue.ID)
112+
if err != nil {
113+
return err
114+
}
115+
if exists {
116+
return FinishIssueStopwatch(user, issue)
117+
}
118+
return CreateIssueStopwatch(user, issue)
119+
}
120+
121+
// FinishIssueStopwatch if stopwatch exist then finish it otherwise return an error
122+
func FinishIssueStopwatch(user *User, issue *Issue) error {
79123
sess := x.NewSession()
80124
defer sess.Close()
81125
if err := sess.Begin(); err != nil {
82126
return err
83127
}
84-
if err := createOrStopIssueStopwatch(sess, user, issue); err != nil {
128+
if err := finishIssueStopwatch(sess, user, issue); err != nil {
85129
return err
86130
}
87131
return sess.Commit()
88132
}
89133

90-
func createOrStopIssueStopwatch(e *xorm.Session, user *User, issue *Issue) error {
134+
func finishIssueStopwatch(e *xorm.Session, user *User, issue *Issue) error {
91135
sw, exists, err := getStopwatch(e, user.ID, issue.ID)
92136
if err != nil {
93137
return err
94138
}
95-
if err := issue.loadRepo(e); err != nil {
139+
if !exists {
140+
return ErrIssueStopwatchNotExist{
141+
UserID: user.ID,
142+
IssueID: issue.ID,
143+
}
144+
}
145+
146+
// Create tracked time out of the time difference between start date and actual date
147+
timediff := time.Now().Unix() - int64(sw.CreatedUnix)
148+
149+
// Create TrackedTime
150+
tt := &TrackedTime{
151+
Created: time.Now(),
152+
IssueID: issue.ID,
153+
UserID: user.ID,
154+
Time: timediff,
155+
}
156+
157+
if _, err := e.Insert(tt); err != nil {
96158
return err
97159
}
98160

99-
if exists {
100-
// Create tracked time out of the time difference between start date and actual date
101-
timediff := time.Now().Unix() - int64(sw.CreatedUnix)
161+
if err := issue.loadRepo(e); err != nil {
162+
return err
163+
}
164+
if _, err := createComment(e, &CreateCommentOptions{
165+
Doer: user,
166+
Issue: issue,
167+
Repo: issue.Repo,
168+
Content: SecToTime(timediff),
169+
Type: CommentTypeStopTracking,
170+
TimeID: tt.ID,
171+
}); err != nil {
172+
return err
173+
}
174+
_, err = e.Delete(sw)
175+
return err
176+
}
102177

103-
// Create TrackedTime
104-
tt := &TrackedTime{
105-
Created: time.Now(),
106-
IssueID: issue.ID,
107-
UserID: user.ID,
108-
Time: timediff,
109-
}
178+
// CreateIssueStopwatch creates a stopwatch if not exist, otherwise return an error
179+
func CreateIssueStopwatch(user *User, issue *Issue) error {
180+
sess := x.NewSession()
181+
defer sess.Close()
182+
if err := sess.Begin(); err != nil {
183+
return err
184+
}
185+
if err := createIssueStopwatch(sess, user, issue); err != nil {
186+
return err
187+
}
188+
return sess.Commit()
189+
}
110190

111-
if _, err := e.Insert(tt); err != nil {
112-
return err
113-
}
191+
func createIssueStopwatch(e *xorm.Session, user *User, issue *Issue) error {
192+
if err := issue.loadRepo(e); err != nil {
193+
return err
194+
}
114195

115-
if _, err := createComment(e, &CreateCommentOptions{
116-
Doer: user,
117-
Issue: issue,
118-
Repo: issue.Repo,
119-
Content: SecToTime(timediff),
120-
Type: CommentTypeStopTracking,
121-
TimeID: tt.ID,
122-
}); err != nil {
123-
return err
124-
}
125-
if _, err := e.Delete(sw); err != nil {
126-
return err
127-
}
128-
} else {
129-
// if another stopwatch is running: stop it
130-
exists, sw, err := hasUserStopwatch(e, user.ID)
196+
// if another stopwatch is running: stop it
197+
exists, sw, err := hasUserStopwatch(e, user.ID)
198+
if err != nil {
199+
return err
200+
}
201+
if exists {
202+
issue, err := getIssueByID(e, sw.IssueID)
131203
if err != nil {
132204
return err
133205
}
134-
if exists {
135-
issue, err := getIssueByID(e, sw.IssueID)
136-
if err != nil {
137-
return err
138-
}
139-
if err := createOrStopIssueStopwatch(e, user, issue); err != nil {
140-
return err
141-
}
206+
if err := finishIssueStopwatch(e, user, issue); err != nil {
207+
return err
142208
}
209+
}
143210

144-
// Create stopwatch
145-
sw = &Stopwatch{
146-
UserID: user.ID,
147-
IssueID: issue.ID,
148-
}
211+
// Create stopwatch
212+
sw = &Stopwatch{
213+
UserID: user.ID,
214+
IssueID: issue.ID,
215+
}
149216

150-
if _, err := e.Insert(sw); err != nil {
151-
return err
152-
}
217+
if _, err := e.Insert(sw); err != nil {
218+
return err
219+
}
153220

154-
if _, err := createComment(e, &CreateCommentOptions{
155-
Doer: user,
156-
Issue: issue,
157-
Repo: issue.Repo,
158-
Type: CommentTypeStartTracking,
159-
}); err != nil {
160-
return err
161-
}
221+
if err := issue.loadRepo(e); err != nil {
222+
return err
223+
}
224+
225+
if _, err := createComment(e, &CreateCommentOptions{
226+
Doer: user,
227+
Issue: issue,
228+
Repo: issue.Repo,
229+
Type: CommentTypeStartTracking,
230+
}); err != nil {
231+
return err
162232
}
163233
return nil
164234
}

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,8 @@ func StartIssueStopwatch(ctx *context.APIContext) {
5555
return
5656
}
5757

58-
if err := models.CreateOrStopIssueStopwatch(ctx.User, issue); err != nil {
59-
ctx.Error(http.StatusInternalServerError, "CreateOrStopIssueStopwatch", err)
58+
if err := models.CreateIssueStopwatch(ctx.User, issue); err != nil {
59+
ctx.Error(http.StatusInternalServerError, "CreateIssueStopwatch", err)
6060
return
6161
}
6262

@@ -104,8 +104,8 @@ func StopIssueStopwatch(ctx *context.APIContext) {
104104
return
105105
}
106106

107-
if err := models.CreateOrStopIssueStopwatch(ctx.User, issue); err != nil {
108-
ctx.Error(http.StatusInternalServerError, "CreateOrStopIssueStopwatch", err)
107+
if err := models.FinishIssueStopwatch(ctx.User, issue); err != nil {
108+
ctx.Error(http.StatusInternalServerError, "FinishIssueStopwatch", err)
109109
return
110110
}
111111

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

Lines changed: 5 additions & 30 deletions
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
"fmt"
@@ -13,7 +13,6 @@ import (
1313
"time"
1414

1515
"code.gitea.io/gitea/models"
16-
"code.gitea.io/gitea/modules/notification"
1716
"code.gitea.io/gitea/modules/references"
1817
"code.gitea.io/gitea/modules/repository"
1918
)
@@ -95,33 +94,6 @@ func issueAddTime(issue *models.Issue, doer *models.User, time time.Time, timeLo
9594
return err
9695
}
9796

98-
func changeIssueStatus(repo *models.Repository, issue *models.Issue, doer *models.User, closed bool) error {
99-
stopTimerIfAvailable := func(doer *models.User, issue *models.Issue) error {
100-
101-
if models.StopwatchExists(doer.ID, issue.ID) {
102-
if err := models.CreateOrStopIssueStopwatch(doer, issue); err != nil {
103-
return err
104-
}
105-
}
106-
107-
return nil
108-
}
109-
110-
issue.Repo = repo
111-
comment, err := issue.ChangeStatus(doer, closed)
112-
if err != nil {
113-
// Don't return an error when dependencies are open as this would let the push fail
114-
if models.IsErrDependenciesLeft(err) {
115-
return stopTimerIfAvailable(doer, issue)
116-
}
117-
return err
118-
}
119-
120-
notification.NotifyIssueChangeStatus(doer, issue, comment, closed)
121-
122-
return stopTimerIfAvailable(doer, issue)
123-
}
124-
12597
// UpdateIssuesCommit checks if issues are manipulated by commit message.
12698
func UpdateIssuesCommit(doer *models.User, repo *models.Repository, commits []*repository.PushCommit, branchName string) error {
12799
// Commits are appended in the reverse order.
@@ -208,7 +180,10 @@ func UpdateIssuesCommit(doer *models.User, repo *models.Repository, commits []*r
208180
}
209181
}
210182
if close != refIssue.IsClosed {
211-
if err := changeIssueStatus(refRepo, refIssue, doer, close); err != nil {
183+
if refIssue.Repo != nil {
184+
refIssue.Repo = refRepo
185+
}
186+
if err := ChangeStatus(refIssue, doer, close); err != nil {
212187
return err
213188
}
214189
}

‎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"

‎services/issue/status.go‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,19 @@ import (
1313
func ChangeStatus(issue *models.Issue, doer *models.User, isClosed bool) (err error) {
1414
comment, err := issue.ChangeStatus(doer, isClosed)
1515
if err != nil {
16-
return
16+
// Don't return an error when dependencies are open as this would let the push fail
17+
if models.IsErrDependenciesLeft(err) {
18+
if isClosed {
19+
return models.FinishIssueStopwatchIfPossible(doer, issue)
20+
}
21+
}
22+
return err
23+
}
24+
25+
if isClosed {
26+
if err := models.FinishIssueStopwatchIfPossible(doer, issue); err != nil {
27+
return err
28+
}
1729
}
1830

1931
notification.NotifyIssueChangeStatus(doer, issue, comment, isClosed)

‎services/repository/push.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,9 @@ import (
1616
"code.gitea.io/gitea/modules/log"
1717
"code.gitea.io/gitea/modules/notification"
1818
"code.gitea.io/gitea/modules/queue"
19-
"code.gitea.io/gitea/modules/repofiles"
2019
repo_module "code.gitea.io/gitea/modules/repository"
2120
"code.gitea.io/gitea/modules/setting"
21+
issue_service "code.gitea.io/gitea/services/issue"
2222
pull_service "code.gitea.io/gitea/services/pull"
2323
)
2424

@@ -194,7 +194,7 @@ func pushUpdates(optsList []*repo_module.PushUpdateOptions) error {
194194
commits := repo_module.ListToPushCommits(l)
195195
commits.HeadCommit = repo_module.CommitToPushCommit(newCommit)
196196

197-
if err := repofiles.UpdateIssuesCommit(pusher, repo, commits.Commits, refName); err != nil {
197+
if err := issue_service.UpdateIssuesCommit(pusher, repo, commits.Commits, refName); err != nil {
198198
log.Error("updateIssuesCommit: %v", err)
199199
}
200200

0 commit comments

Comments
 (0)