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]>

Author: bdruth

models/actions/run_list.go

@@ -77,7 +77,6 @@ type FindRunOptions struct {
 	CreatedAfter        time.Time
 	CreatedBefore       time.Time
 	ExcludePullRequests bool
-	CheckSuiteID        int64
 }
 
 func (opts FindRunOptions) ToConds() builder.Cond {
@@ -115,9 +114,6 @@ func (opts FindRunOptions) ToConds() builder.Cond {
 	if opts.ExcludePullRequests {
 		cond = cond.And(builder.Neq{"`action_run`.trigger_event": webhook_module.HookEventPullRequest})
 	}
-	if opts.CheckSuiteID > 0 {
-		cond = cond.And(builder.Eq{"`action_run`.check_suite_id": opts.CheckSuiteID})
-	}
 	return cond
 }
 

models/actions/run_list_test.go

@@ -28,22 +28,6 @@ func TestFindRunOptions_ToConds_ExcludePullRequests(t *testing.T) {
 	assert.Contains(t, args, webhook.HookEventPullRequest)
 }
 
-func TestFindRunOptions_ToConds_CheckSuiteID(t *testing.T) {
-	// Test when CheckSuiteID is set
-	const testSuiteID int64 = 12345
-	opts := FindRunOptions{
-		CheckSuiteID: testSuiteID,
-	}
-	cond := opts.ToConds()
-
-	// Convert the condition to SQL for assertion
-	sql, args, err := builder.ToSQL(cond)
-	assert.NoError(t, err)
-	// The condition should contain the check_suite_id equal to the test value
-	assert.Contains(t, sql, "`action_run`.check_suite_id=")
-	assert.Contains(t, args, testSuiteID)
-}
-
 func TestFindRunOptions_ToConds_CreatedDateRange(t *testing.T) {
 	// Test when CreatedAfter and CreatedBefore are set
 	startDate := time.Date(2023, 1, 1, 0, 0, 0, 0, time.UTC)

routers/api/v1/repo/action.go

@@ -1149,7 +1149,7 @@ func ActionsListWorkflowRuns(ctx *context.APIContext) {
 	//   default: false
 	// - name: check_suite_id
 	//   in: query
-	//   description: Returns workflow runs with the check_suite_id that you specify.
+	//   description: Not supported in Gitea API. Returns workflow runs with the check_suite_id that you specify (GitHub API compatibility - parameter ignored).
 	//   type: integer
 	// - name: head_sha
 	//   in: query

routers/api/v1/shared/action.go

@@ -22,6 +22,27 @@ import (
 	"code.gitea.io/gitea/services/convert"
 )
 
+// parseISO8601DateRange parses flexible date formats: YYYY-MM-DD or YYYY-MM-DDTHH:MM:SSZ (ISO8601)
+func parseISO8601DateRange(dateStr string) (time.Time, error) {
+	// Try ISO8601 format first: 2017-01-01T01:00:00+07:00 or 2016-03-21T14:11:00Z
+	if strings.Contains(dateStr, "T") {
+		// Try with timezone offset (RFC3339)
+		if t, err := time.Parse(time.RFC3339, dateStr); err == nil {
+			return t, nil
+		}
+		// Try with Z suffix (UTC)
+		if t, err := time.Parse("2006-01-02T15:04:05Z", dateStr); err == nil {
+			return t, nil
+		}
+		// Try without timezone
+		if t, err := time.Parse("2006-01-02T15:04:05", dateStr); err == nil {
+			return t, nil
+		}
+	}
+	// Try simple date format: YYYY-MM-DD
+	return time.Parse("2006-01-02", dateStr)
+}
+
 // ListJobs lists jobs for api route validated ownerID and repoID
 // ownerID == 0 and repoID == 0 means all jobs
 // ownerID == 0 and repoID != 0 means all jobs for the given repo
@@ -156,65 +177,67 @@ func ListRuns(ctx *context.APIContext, ownerID, repoID int64) {
 	}
 
 	// Handle exclude_pull_requests parameter
-	if exclude := ctx.FormString("exclude_pull_requests"); exclude != "" {
-		if exclude == "true" || exclude == "1" {
-			opts.ExcludePullRequests = true
-		}
-	}
-
-	// Handle check_suite_id parameter
-	if checkSuiteID := ctx.FormInt64("check_suite_id"); checkSuiteID > 0 {
-		opts.CheckSuiteID = checkSuiteID
+	if ctx.FormBool("exclude_pull_requests") {
+		opts.ExcludePullRequests = true
 	}
 
 	// Handle created parameter for date filtering
+	// Supports ISO8601 date formats: YYYY-MM-DD or YYYY-MM-DDTHH:MM:SSZ
 	if created := ctx.FormString("created"); created != "" {
 		// Parse the date range in the format like ">=2023-01-01", "<=2023-12-31", or "2023-01-01..2023-12-31"
-		if strings.Contains(created, "..\u002e") {
+		if strings.Contains(created, "..") {
 			// Range format: "2023-01-01..2023-12-31"
 			dateRange := strings.Split(created, "..")
 			if len(dateRange) == 2 {
-				startDate, err := time.Parse("2006-01-02", dateRange[0])
+				startDate, err := parseISO8601DateRange(dateRange[0])
 				if err == nil {
 					opts.CreatedAfter = startDate
 				}
 
-				endDate, err := time.Parse("2006-01-02", dateRange[1])
+				endDate, err := parseISO8601DateRange(dateRange[1])
 				if err == nil {
-					// Set to end of day
-					endDate = endDate.Add(24*time.Hour - time.Second)
+					// Set to end of day if only date provided
+					if !strings.Contains(dateRange[1], "T") {
+						endDate = endDate.Add(24*time.Hour - time.Second)
+					}
 					opts.CreatedBefore = endDate
 				}
 			}
 		} else if after, ok := strings.CutPrefix(created, ">="); ok {
 			// Greater than or equal format: ">=2023-01-01"
-			dateStr := after
-			startDate, err := time.Parse("2006-01-02", dateStr)
+			startDate, err := parseISO8601DateRange(after)
 			if err == nil {
 				opts.CreatedAfter = startDate
 			}
 		} else if after, ok := strings.CutPrefix(created, ">"); ok {
 			// Greater than format: ">2023-01-01"
-			dateStr := after
-			startDate, err := time.Parse("2006-01-02", dateStr)
+			startDate, err := parseISO8601DateRange(after)
 			if err == nil {
-				opts.CreatedAfter = startDate.Add(24 * time.Hour)
+				if strings.Contains(after, "T") {
+					opts.CreatedAfter = startDate.Add(time.Second)
+				} else {
+					opts.CreatedAfter = startDate.Add(24 * time.Hour)
+				}
 			}
 		} else if after, ok := strings.CutPrefix(created, "<="); ok {
 			// Less than or equal format: "<=2023-12-31"
-			dateStr := after
-			endDate, err := time.Parse("2006-01-02", dateStr)
+			endDate, err := parseISO8601DateRange(after)
 			if err == nil {
-				// Set to end of day
-				endDate = endDate.Add(24*time.Hour - time.Second)
+				// Set to end of day if only date provided
+				if !strings.Contains(after, "T") {
+					endDate = endDate.Add(24*time.Hour - time.Second)
+				}
 				opts.CreatedBefore = endDate
 			}
 		} else if after, ok := strings.CutPrefix(created, "<"); ok {
 			// Less than format: "<2023-12-31"
-			dateStr := after
-			endDate, err := time.Parse("2006-01-02", dateStr)
+			endDate, err := parseISO8601DateRange(after)
 			if err == nil {
-				opts.CreatedBefore = endDate
+				if strings.Contains(after, "T") {
+					opts.CreatedBefore = endDate.Add(-time.Second)
+				} else {
+					opts.CreatedBefore = endDate
+				}
 			}
 		} else {
 			// Exact date format: "2023-01-01"

routers/api/v1/shared/action_list_runs_test.go

@@ -4,13 +4,11 @@
 package shared
 
 import (
-	"net/url"
 	"testing"
 	"time"
 
 	actions_model "code.gitea.io/gitea/models/actions"
 	"code.gitea.io/gitea/models/unittest"
-	"code.gitea.io/gitea/services/context"
 	"code.gitea.io/gitea/services/contexttest"
 
 	"github.com/stretchr/testify/assert"
@@ -20,15 +18,6 @@ func TestMain(m *testing.M) {
 	unittest.MainTest(m)
 }
 
-// setFormValue is a helper function to set form values in test context
-func setFormValue(ctx *context.APIContext, key, value string) {
-	// Initialize the form if it's nil
-	if ctx.Req.Form == nil {
-		ctx.Req.Form = make(url.Values)
-	}
-	ctx.Req.Form.Set(key, value)
-}
-
 // TestListRunsWorkflowFiltering tests that ListRuns properly handles
 // the workflow_id path parameter for filtering runs by workflow.
 func TestListRunsWorkflowFiltering(t *testing.T) {
@@ -77,108 +66,65 @@ func TestListRunsExcludePullRequestsParam(t *testing.T) {
 	unittest.PrepareTestEnv(t)
 
 	// Test case 1: With exclude_pull_requests=true
-	ctx, _ := contexttest.MockAPIContext(t, "user2/repo1")
+	ctx, _ := contexttest.MockAPIContext(t, "user2/repo1?exclude_pull_requests=true")
 	contexttest.LoadRepo(t, ctx, 1)
 	contexttest.LoadUser(t, ctx, 2)
 
-	// Set up form value
-	setFormValue(ctx, "exclude_pull_requests", "true")
-
 	// Call the actual parsing logic from ListRuns
 	opts := actions_model.FindRunOptions{
 		RepoID: ctx.Repo.Repository.ID,
 	}
 
-	if exclude := ctx.FormString("exclude_pull_requests"); exclude != "" {
-		if exclude == "true" || exclude == "1" {
-			opts.ExcludePullRequests = true
-		}
+	if ctx.FormBool("exclude_pull_requests") {
+		opts.ExcludePullRequests = true
 	}
 
 	// Verify the ExcludePullRequests is correctly set based on the form value
 	assert.True(t, opts.ExcludePullRequests)
 
 	// Test case 2: With exclude_pull_requests=1
-	ctx2, _ := contexttest.MockAPIContext(t, "user2/repo1")
+	ctx2, _ := contexttest.MockAPIContext(t, "user2/repo1?exclude_pull_requests=1")
 	contexttest.LoadRepo(t, ctx2, 1)
 	contexttest.LoadUser(t, ctx2, 2)
 
-	setFormValue(ctx2, "exclude_pull_requests", "1")
-
 	opts2 := actions_model.FindRunOptions{
 		RepoID: ctx2.Repo.Repository.ID,
 	}
 
-	if exclude := ctx2.FormString("exclude_pull_requests"); exclude != "" {
-		if exclude == "true" || exclude == "1" {
-			opts2.ExcludePullRequests = true
-		}
+	if ctx2.FormBool("exclude_pull_requests") {
+		opts2.ExcludePullRequests = true
 	}
 
 	// Verify the ExcludePullRequests is correctly set for "1" value
 	assert.True(t, opts2.ExcludePullRequests)
 
 	// Test case 3: With exclude_pull_requests=false (should not set the flag)
-	ctx3, _ := contexttest.MockAPIContext(t, "user2/repo1")
+	ctx3, _ := contexttest.MockAPIContext(t, "user2/repo1?exclude_pull_requests=false")
 	contexttest.LoadRepo(t, ctx3, 1)
 	contexttest.LoadUser(t, ctx3, 2)
 
-	setFormValue(ctx3, "exclude_pull_requests", "false")
-
 	opts3 := actions_model.FindRunOptions{
 		RepoID: ctx3.Repo.Repository.ID,
 	}
 
-	if exclude := ctx3.FormString("exclude_pull_requests"); exclude != "" {
-		if exclude == "true" || exclude == "1" {
-			opts3.ExcludePullRequests = true
-		}
+	if ctx3.FormBool("exclude_pull_requests") {
+		opts3.ExcludePullRequests = true
 	}
 
 	// Verify the ExcludePullRequests is NOT set for "false" value
 	assert.False(t, opts3.ExcludePullRequests)
 }
 
-// TestListRunsCheckSuiteIDParam tests that ListRuns properly handles
-// the check_suite_id parameter.
-func TestListRunsCheckSuiteIDParam(t *testing.T) {
-	unittest.PrepareTestEnv(t)
-
-	const testSuiteID int64 = 12345
-
-	// Test case: With check_suite_id parameter
-	ctx, _ := contexttest.MockAPIContext(t, "user2/repo1")
-	contexttest.LoadRepo(t, ctx, 1)
-	contexttest.LoadUser(t, ctx, 2)
-
-	setFormValue(ctx, "check_suite_id", "12345")
-
-	// Call the actual parsing logic from ListRuns
-	opts := actions_model.FindRunOptions{
-		RepoID: ctx.Repo.Repository.ID,
-	}
-
-	// This simulates the logic in ListRuns
-	if checkSuiteID := ctx.FormInt64("check_suite_id"); checkSuiteID > 0 {
-		opts.CheckSuiteID = checkSuiteID
-	}
-
-	// Verify the CheckSuiteID is correctly set based on the form value
-	assert.Equal(t, testSuiteID, opts.CheckSuiteID)
-}
-
 // TestListRunsCreatedParam tests that ListRuns properly handles
 // the created parameter for date filtering.
 func TestListRunsCreatedParam(t *testing.T) {
 	unittest.PrepareTestEnv(t)
 
 	// Test case 1: With created in date range format
-	ctx, _ := contexttest.MockAPIContext(t, "user2/repo1")
+	ctx, _ := contexttest.MockAPIContext(t, "user2/repo1?created=2023-01-01..2023-12-31")
 	contexttest.LoadRepo(t, ctx, 1)
 	contexttest.LoadUser(t, ctx, 2)
 
-	setFormValue(ctx, "created", "2023-01-01..2023-12-31")
-
 	opts := actions_model.FindRunOptions{
 		RepoID: ctx.Repo.Repository.ID,
 	}
@@ -204,12 +150,10 @@ func TestListRunsCreatedParam(t *testing.T) {
 	assert.Equal(t, expectedEnd, opts.CreatedBefore)
 
 	// Test case 2: With created in ">=" format
-	ctx2, _ := contexttest.MockAPIContext(t, "user2/repo1")
+	ctx2, _ := contexttest.MockAPIContext(t, "user2/repo1?created=>=2023-01-01")
 	contexttest.LoadRepo(t, ctx2, 1)
 	contexttest.LoadUser(t, ctx2, 2)
 
-	setFormValue(ctx2, "created", ">=2023-01-01")
-
 	opts2 := actions_model.FindRunOptions{
 		RepoID: ctx2.Repo.Repository.ID,
 	}
@@ -229,12 +173,10 @@ func TestListRunsCreatedParam(t *testing.T) {
 	assert.True(t, opts2.CreatedBefore.IsZero())
 
 	// Test case 3: With created in exact date format
-	ctx3, _ := contexttest.MockAPIContext(t, "user2/repo1")
+	ctx3, _ := contexttest.MockAPIContext(t, "user2/repo1?created=2023-06-15")
 	contexttest.LoadRepo(t, ctx3, 1)
 	contexttest.LoadUser(t, ctx3, 2)
 
-	setFormValue(ctx3, "created", "2023-06-15")
-
 	opts3 := actions_model.FindRunOptions{
 		RepoID: ctx3.Repo.Repository.ID,
 	}