Skip to content

Commit 8b7ed3a

Browse files
bdruthclaude
bdruth
and
claude
committed
Address PR review feedback for workflow runs API
- Use ctx.FormBool instead of manual string parsing for exclude_pull_requests - Fix unicode escape sequence \u002e to simple ".." in date range parsing - Add ISO8601 date format support with parseISO8601DateRange helper function - Simplify tests using contexttest.MockAPIContext with query parameters - Remove unused setFormValue test helper and improve test maintainability 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
1 parent 695496c commit 8b7ed3a

File tree

2 files changed

+62
-66
lines changed

2 files changed

+62
-66
lines changed

‎routers/api/v1/shared/action.go‎

Lines changed: 49 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,27 @@ import (
2222
"code.gitea.io/gitea/services/convert"
2323
)
2424

25+
// parseISO8601DateRange parses flexible date formats: YYYY-MM-DD or YYYY-MM-DDTHH:MM:SSZ (ISO8601)
26+
func parseISO8601DateRange(dateStr string) (time.Time, error) {
27+
// Try ISO8601 format first: 2017-01-01T01:00:00+07:00 or 2016-03-21T14:11:00Z
28+
if strings.Contains(dateStr, "T") {
29+
// Try with timezone offset (RFC3339)
30+
if t, err := time.Parse(time.RFC3339, dateStr); err == nil {
31+
return t, nil
32+
}
33+
// Try with Z suffix (UTC)
34+
if t, err := time.Parse("2006-01-02T15:04:05Z", dateStr); err == nil {
35+
return t, nil
36+
}
37+
// Try without timezone
38+
if t, err := time.Parse("2006-01-02T15:04:05", dateStr); err == nil {
39+
return t, nil
40+
}
41+
}
42+
// Try simple date format: YYYY-MM-DD
43+
return time.Parse("2006-01-02", dateStr)
44+
}
45+
2546
// ListJobs lists jobs for api route validated ownerID and repoID
2647
// ownerID == 0 and repoID == 0 means all jobs
2748
// ownerID == 0 and repoID != 0 means all jobs for the given repo
@@ -156,10 +177,8 @@ func ListRuns(ctx *context.APIContext, ownerID, repoID int64) {
156177
}
157178

158179
// Handle exclude_pull_requests parameter
159-
if exclude := ctx.FormString("exclude_pull_requests"); exclude != "" {
160-
if exclude == "true" || exclude == "1" {
161-
opts.ExcludePullRequests = true
162-
}
180+
if ctx.FormBool("exclude_pull_requests") {
181+
opts.ExcludePullRequests = true
163182
}
164183

165184
// Handle check_suite_id parameter
@@ -168,53 +187,62 @@ func ListRuns(ctx *context.APIContext, ownerID, repoID int64) {
168187
}
169188

170189
// Handle created parameter for date filtering
190+
// Supports ISO8601 date formats: YYYY-MM-DD or YYYY-MM-DDTHH:MM:SSZ
171191
if created := ctx.FormString("created"); created != "" {
172192
// Parse the date range in the format like ">=2023-01-01", "<=2023-12-31", or "2023-01-01..2023-12-31"
173-
if strings.Contains(created, "..\u002e") {
193+
if strings.Contains(created, "..") {
174194
// Range format: "2023-01-01..2023-12-31"
175195
dateRange := strings.Split(created, "..")
176196
if len(dateRange) == 2 {
177-
startDate, err := time.Parse("2006-01-02", dateRange[0])
197+
startDate, err := parseISO8601DateRange(dateRange[0])
178198
if err == nil {
179199
opts.CreatedAfter = startDate
180200
}
181201

182-
endDate, err := time.Parse("2006-01-02", dateRange[1])
202+
endDate, err := parseISO8601DateRange(dateRange[1])
183203
if err == nil {
184-
// Set to end of day
185-
endDate = endDate.Add(24*time.Hour - time.Second)
204+
// Set to end of day if only date provided
205+
if !strings.Contains(dateRange[1], "T") {
206+
endDate = endDate.Add(24*time.Hour - time.Second)
207+
}
186208
opts.CreatedBefore = endDate
187209
}
188210
}
189211
} else if after, ok := strings.CutPrefix(created, ">="); ok {
190212
// Greater than or equal format: ">=2023-01-01"
191-
dateStr := after
192-
startDate, err := time.Parse("2006-01-02", dateStr)
213+
startDate, err := parseISO8601DateRange(after)
193214
if err == nil {
194215
opts.CreatedAfter = startDate
195216
}
196217
} else if after, ok := strings.CutPrefix(created, ">"); ok {
197218
// Greater than format: ">2023-01-01"
198-
dateStr := after
199-
startDate, err := time.Parse("2006-01-02", dateStr)
219+
startDate, err := parseISO8601DateRange(after)
200220
if err == nil {
201-
opts.CreatedAfter = startDate.Add(24 * time.Hour)
221+
if strings.Contains(after, "T") {
222+
opts.CreatedAfter = startDate.Add(time.Second)
223+
} else {
224+
opts.CreatedAfter = startDate.Add(24 * time.Hour)
225+
}
202226
}
203227
} else if after, ok := strings.CutPrefix(created, "<="); ok {
204228
// Less than or equal format: "<=2023-12-31"
205-
dateStr := after
206-
endDate, err := time.Parse("2006-01-02", dateStr)
229+
endDate, err := parseISO8601DateRange(after)
207230
if err == nil {
208-
// Set to end of day
209-
endDate = endDate.Add(24*time.Hour - time.Second)
231+
// Set to end of day if only date provided
232+
if !strings.Contains(after, "T") {
233+
endDate = endDate.Add(24*time.Hour - time.Second)
234+
}
210235
opts.CreatedBefore = endDate
211236
}
212237
} else if after, ok := strings.CutPrefix(created, "<"); ok {
213238
// Less than format: "<2023-12-31"
214-
dateStr := after
215-
endDate, err := time.Parse("2006-01-02", dateStr)
239+
endDate, err := parseISO8601DateRange(after)
216240
if err == nil {
217-
opts.CreatedBefore = endDate
241+
if strings.Contains(after, "T") {
242+
opts.CreatedBefore = endDate.Add(-time.Second)
243+
} else {
244+
opts.CreatedBefore = endDate
245+
}
218246
}
219247
} else {
220248
// Exact date format: "2023-01-01"

‎routers/api/v1/shared/action_list_runs_test.go‎

Lines changed: 13 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,11 @@
44
package shared
55

66
import (
7-
"net/url"
87
"testing"
98
"time"
109

1110
actions_model "code.gitea.io/gitea/models/actions"
1211
"code.gitea.io/gitea/models/unittest"
13-
"code.gitea.io/gitea/services/context"
1412
"code.gitea.io/gitea/services/contexttest"
1513

1614
"github.com/stretchr/testify/assert"
@@ -20,15 +18,6 @@ func TestMain(m *testing.M) {
2018
unittest.MainTest(m)
2119
}
2220

23-
// setFormValue is a helper function to set form values in test context
24-
func setFormValue(ctx *context.APIContext, key, value string) {
25-
// Initialize the form if it's nil
26-
if ctx.Req.Form == nil {
27-
ctx.Req.Form = make(url.Values)
28-
}
29-
ctx.Req.Form.Set(key, value)
30-
}
31-
3221
// TestListRunsWorkflowFiltering tests that ListRuns properly handles
3322
// the workflow_id path parameter for filtering runs by workflow.
3423
func TestListRunsWorkflowFiltering(t *testing.T) {
@@ -77,62 +66,49 @@ func TestListRunsExcludePullRequestsParam(t *testing.T) {
7766
unittest.PrepareTestEnv(t)
7867

7968
// Test case 1: With exclude_pull_requests=true
80-
ctx, _ := contexttest.MockAPIContext(t, "user2/repo1")
69+
ctx, _ := contexttest.MockAPIContext(t, "user2/repo1?exclude_pull_requests=true")
8170
contexttest.LoadRepo(t, ctx, 1)
8271
contexttest.LoadUser(t, ctx, 2)
8372

84-
// Set up form value
85-
setFormValue(ctx, "exclude_pull_requests", "true")
86-
8773
// Call the actual parsing logic from ListRuns
8874
opts := actions_model.FindRunOptions{
8975
RepoID: ctx.Repo.Repository.ID,
9076
}
9177

92-
if exclude := ctx.FormString("exclude_pull_requests"); exclude != "" {
93-
if exclude == "true" || exclude == "1" {
94-
opts.ExcludePullRequests = true
95-
}
78+
if ctx.FormBool("exclude_pull_requests") {
79+
opts.ExcludePullRequests = true
9680
}
9781

9882
// Verify the ExcludePullRequests is correctly set based on the form value
9983
assert.True(t, opts.ExcludePullRequests)
10084

10185
// Test case 2: With exclude_pull_requests=1
102-
ctx2, _ := contexttest.MockAPIContext(t, "user2/repo1")
86+
ctx2, _ := contexttest.MockAPIContext(t, "user2/repo1?exclude_pull_requests=1")
10387
contexttest.LoadRepo(t, ctx2, 1)
10488
contexttest.LoadUser(t, ctx2, 2)
10589

106-
setFormValue(ctx2, "exclude_pull_requests", "1")
107-
10890
opts2 := actions_model.FindRunOptions{
10991
RepoID: ctx2.Repo.Repository.ID,
11092
}
11193

112-
if exclude := ctx2.FormString("exclude_pull_requests"); exclude != "" {
113-
if exclude == "true" || exclude == "1" {
114-
opts2.ExcludePullRequests = true
115-
}
94+
if ctx2.FormBool("exclude_pull_requests") {
95+
opts2.ExcludePullRequests = true
11696
}
11797

11898
// Verify the ExcludePullRequests is correctly set for "1" value
11999
assert.True(t, opts2.ExcludePullRequests)
120100

121101
// Test case 3: With exclude_pull_requests=false (should not set the flag)
122-
ctx3, _ := contexttest.MockAPIContext(t, "user2/repo1")
102+
ctx3, _ := contexttest.MockAPIContext(t, "user2/repo1?exclude_pull_requests=false")
123103
contexttest.LoadRepo(t, ctx3, 1)
124104
contexttest.LoadUser(t, ctx3, 2)
125105

126-
setFormValue(ctx3, "exclude_pull_requests", "false")
127-
128106
opts3 := actions_model.FindRunOptions{
129107
RepoID: ctx3.Repo.Repository.ID,
130108
}
131109

132-
if exclude := ctx3.FormString("exclude_pull_requests"); exclude != "" {
133-
if exclude == "true" || exclude == "1" {
134-
opts3.ExcludePullRequests = true
135-
}
110+
if ctx3.FormBool("exclude_pull_requests") {
111+
opts3.ExcludePullRequests = true
136112
}
137113

138114
// Verify the ExcludePullRequests is NOT set for "false" value
@@ -147,12 +123,10 @@ func TestListRunsCheckSuiteIDParam(t *testing.T) {
147123
const testSuiteID int64 = 12345
148124

149125
// Test case: With check_suite_id parameter
150-
ctx, _ := contexttest.MockAPIContext(t, "user2/repo1")
126+
ctx, _ := contexttest.MockAPIContext(t, "user2/repo1?check_suite_id=12345")
151127
contexttest.LoadRepo(t, ctx, 1)
152128
contexttest.LoadUser(t, ctx, 2)
153129

154-
setFormValue(ctx, "check_suite_id", "12345")
155-
156130
// Call the actual parsing logic from ListRuns
157131
opts := actions_model.FindRunOptions{
158132
RepoID: ctx.Repo.Repository.ID,
@@ -173,12 +147,10 @@ func TestListRunsCreatedParam(t *testing.T) {
173147
unittest.PrepareTestEnv(t)
174148

175149
// Test case 1: With created in date range format
176-
ctx, _ := contexttest.MockAPIContext(t, "user2/repo1")
150+
ctx, _ := contexttest.MockAPIContext(t, "user2/repo1?created=2023-01-01..2023-12-31")
177151
contexttest.LoadRepo(t, ctx, 1)
178152
contexttest.LoadUser(t, ctx, 2)
179153

180-
setFormValue(ctx, "created", "2023-01-01..2023-12-31")
181-
182154
opts := actions_model.FindRunOptions{
183155
RepoID: ctx.Repo.Repository.ID,
184156
}
@@ -204,12 +176,10 @@ func TestListRunsCreatedParam(t *testing.T) {
204176
assert.Equal(t, expectedEnd, opts.CreatedBefore)
205177

206178
// Test case 2: With created in ">=" format
207-
ctx2, _ := contexttest.MockAPIContext(t, "user2/repo1")
179+
ctx2, _ := contexttest.MockAPIContext(t, "user2/repo1?created=>=2023-01-01")
208180
contexttest.LoadRepo(t, ctx2, 1)
209181
contexttest.LoadUser(t, ctx2, 2)
210182

211-
setFormValue(ctx2, "created", ">=2023-01-01")
212-
213183
opts2 := actions_model.FindRunOptions{
214184
RepoID: ctx2.Repo.Repository.ID,
215185
}
@@ -229,12 +199,10 @@ func TestListRunsCreatedParam(t *testing.T) {
229199
assert.True(t, opts2.CreatedBefore.IsZero())
230200

231201
// Test case 3: With created in exact date format
232-
ctx3, _ := contexttest.MockAPIContext(t, "user2/repo1")
202+
ctx3, _ := contexttest.MockAPIContext(t, "user2/repo1?created=2023-06-15")
233203
contexttest.LoadRepo(t, ctx3, 1)
234204
contexttest.LoadUser(t, ctx3, 2)
235205

236-
setFormValue(ctx3, "created", "2023-06-15")
237-
238206
opts3 := actions_model.FindRunOptions{
239207
RepoID: ctx3.Repo.Repository.ID,
240208
}

0 commit comments

Comments
 (0)