Skip to content

Commit 3fff19d

Browse files
committed
Signed-off-by: workjs1124 <[email protected]>
fix(issue): Replace stopwatch toggle with explicit start/stop actions The stopwatch toggle mechanism caused state desynchronization issues when used across multiple browser tabs, leading to a "stop" action incorrectly starting the timer again. This commit replaces the single toggle endpoint with two explicit endpoints: `/start` and `/stop`. This ensures that the user's intent is always clear and prevents unexpected behavior, resolving the state inconsistency bug.
1 parent 29b2800 commit 3fff19d

File tree

5 files changed

+45
-21
lines changed

5 files changed

+45
-21
lines changed

options/locale/locale_en-US.ini

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1726,6 +1726,7 @@ issues.remove_time_estimate_at = removed time estimate %s
17261726
issues.time_estimate_invalid = Time estimate format is invalid
17271727
issues.start_tracking_history = started working %s
17281728
issues.tracker_auto_close = Timer will be stopped automatically when this issue gets closed
1729+
issues.tracker_already_stopped = The timer for this issue is already stopped
17291730
issues.tracking_already_started = `You have already started time tracking on <a href="%s">another issue</a>!`
17301731
issues.stop_tracking = Stop Timer
17311732
issues.stop_tracking_history = worked for <b>%[1]s</b> %[2]s

routers/web/repo/issue_stopwatch.go

Lines changed: 29 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -10,31 +10,49 @@ import (
1010
"code.gitea.io/gitea/services/context"
1111
)
1212

13-
// IssueStopwatch creates or stops a stopwatch for the given issue.
14-
func IssueStopwatch(c *context.Context) {
13+
// IssueStartStopwatch creates a stopwatch for the given issue.
14+
func IssueStartStopwatch(c *context.Context) {
1515
issue := GetActionIssue(c)
1616
if c.Written() {
1717
return
1818
}
1919

20-
var showSuccessMessage bool
20+
if !c.Repo.CanUseTimetracker(c, issue, c.Doer) {
21+
c.NotFound(nil)
22+
return
23+
}
2124

22-
if !issues_model.StopwatchExists(c, c.Doer.ID, issue.ID) {
23-
showSuccessMessage = true
25+
if err := issues_model.CreateIssueStopwatch(c, c.Doer, issue); err != nil {
26+
c.ServerError("CreateIssueStopwatch", err)
27+
return
2428
}
2529

26-
if !c.Repo.CanUseTimetracker(c, issue, c.Doer) {
27-
c.NotFound(nil)
30+
c.Flash.Success(c.Tr("repo.issues.tracker_auto_close"))
31+
32+
c.JSONRedirect("")
33+
}
34+
35+
// IssueStopStopwatch stops a stopwatch for the given issue.
36+
func IssueStopStopwatch(c *context.Context) {
37+
issue := GetActionIssue(c)
38+
if c.Written() {
2839
return
2940
}
3041

31-
if err := issues_model.CreateOrStopIssueStopwatch(c, c.Doer, issue); err != nil {
32-
c.ServerError("CreateOrStopIssueStopwatch", err)
42+
if !c.Repo.CanUseTimetracker(c, issue, c.Doer) {
43+
c.NotFound(nil)
3344
return
3445
}
3546

36-
if showSuccessMessage {
37-
c.Flash.Success(c.Tr("repo.issues.tracker_auto_close"))
47+
wasRunning := issues_model.StopwatchExists(c, c.Doer.ID, issue.ID)
48+
49+
if wasRunning {
50+
if err := issues_model.FinishIssueStopwatch(c, c.Doer, issue); err != nil {
51+
c.ServerError("FinishIssueStopwatch", err)
52+
return
53+
}
54+
} else {
55+
c.Flash.Warning(c.Tr("repo.issues.tracker_already_stopped"))
3856
}
3957

4058
c.JSONRedirect("")

routers/web/web.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1253,7 +1253,8 @@ func registerWebRoutes(m *web.Router) {
12531253
m.Post("/add", web.Bind(forms.AddTimeManuallyForm{}), repo.AddTimeManually)
12541254
m.Post("/{timeid}/delete", repo.DeleteTime)
12551255
m.Group("/stopwatch", func() {
1256-
m.Post("/toggle", repo.IssueStopwatch)
1256+
m.Post("/start", repo.IssueStartStopwatch)
1257+
m.Post("/stop", repo.IssueStopStopwatch)
12571258
m.Post("/cancel", repo.CancelStopwatch)
12581259
})
12591260
})

templates/repo/issue/sidebar/stopwatch_timetracker.tmpl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,14 @@
1616
</a>
1717
<div class="divider"></div>
1818
{{if $.IsStopwatchRunning}}
19-
<a class="item issue-stop-time link-action" data-url="{{.Issue.Link}}/times/stopwatch/toggle">
19+
<a class="item issue-stop-time link-action" data-url="{{.Issue.Link}}/times/stopwatch/stop">
2020
{{svg "octicon-stopwatch"}} {{ctx.Locale.Tr "repo.issues.timetracker_timer_stop"}}
2121
</a>
2222
<a class="item issue-cancel-time link-action" data-url="{{.Issue.Link}}/times/stopwatch/cancel">
2323
{{svg "octicon-trash"}} {{ctx.Locale.Tr "repo.issues.timetracker_timer_discard"}}
2424
</a>
2525
{{else}}
26-
<a class="item issue-start-time link-action" data-url="{{.Issue.Link}}/times/stopwatch/toggle">
26+
<a class="item issue-start-time link-action" data-url="{{.Issue.Link}}/times/stopwatch/start">
2727
{{svg "octicon-stopwatch"}} {{ctx.Locale.Tr "repo.issues.timetracker_timer_start"}}
2828
</a>
2929
<a class="item issue-add-time show-modal" data-modal="#issue-time-manually-add-modal">

tests/integration/timetracking_test.go

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -46,11 +46,12 @@ func testViewTimetrackingControls(t *testing.T, session *TestSession, user, repo
4646
AssertHTMLElement(t, htmlDoc, ".issue-add-time", canTrackTime)
4747

4848
issueLink := path.Join(user, repo, "issues", issue)
49-
req = NewRequestWithValues(t, "POST", path.Join(issueLink, "times", "stopwatch", "toggle"), map[string]string{
50-
"_csrf": htmlDoc.GetCSRF(),
51-
})
49+
5250
if canTrackTime {
53-
session.MakeRequest(t, req, http.StatusOK)
51+
reqStart := NewRequestWithValues(t, "POST", path.Join(issueLink, "times", "stopwatch", "start"), map[string]string{
52+
"_csrf": htmlDoc.GetCSRF(),
53+
})
54+
session.MakeRequest(t, reqStart, http.StatusOK)
5455

5556
req = NewRequest(t, "GET", issueLink)
5657
resp = session.MakeRequest(t, req, http.StatusOK)
@@ -65,10 +66,10 @@ func testViewTimetrackingControls(t *testing.T, session *TestSession, user, repo
6566
// Sleep for 1 second to not get wrong order for stopping timer
6667
time.Sleep(time.Second)
6768

68-
req = NewRequestWithValues(t, "POST", path.Join(issueLink, "times", "stopwatch", "toggle"), map[string]string{
69+
reqStop := NewRequestWithValues(t, "POST", path.Join(issueLink, "times", "stopwatch", "stop"), map[string]string{
6970
"_csrf": htmlDoc.GetCSRF(),
7071
})
71-
session.MakeRequest(t, req, http.StatusOK)
72+
session.MakeRequest(t, reqStop, http.StatusOK)
7273

7374
req = NewRequest(t, "GET", issueLink)
7475
resp = session.MakeRequest(t, req, http.StatusOK)
@@ -77,6 +78,9 @@ func testViewTimetrackingControls(t *testing.T, session *TestSession, user, repo
7778
events = htmlDoc.doc.Find(".event > span.text")
7879
assert.Contains(t, events.Last().Text(), "worked for ")
7980
} else {
80-
session.MakeRequest(t, req, http.StatusNotFound)
81+
reqStart := NewRequestWithValues(t, "POST", path.Join(issueLink, "times", "stopwatch", "start"), map[string]string{
82+
"_csrf": htmlDoc.GetCSRF(),
83+
})
84+
session.MakeRequest(t, reqStart, http.StatusNotFound)
8185
}
8286
}

0 commit comments

Comments
 (0)