From e15b1c2addff895ac0c6b4cd42d0701822ec91b5 Mon Sep 17 00:00:00 2001 From: Nicolas Date: Mon, 13 Apr 2026 20:55:23 +0200 Subject: [PATCH] address review comments on workflow runs API - Update workflow_id swagger description to clarify filename-only - Add exclude_pull_requests query parameter - Return 404 for nonexistent workflow in ActionsListWorkflowRuns - Remove break from test loop so all runs are verified - Add actor and head_sha filter coverage to testAPIWorkflowRunsByWorkflowID - Update nonexistent workflow test to expect 404 --- models/actions/run_list.go | 24 +++++++++++-------- routers/api/v1/repo/action.go | 21 +++++++++++++++- routers/api/v1/shared/action.go | 3 +++ templates/swagger/v1_json.tmpl | 14 ++++++++++- .../workflow_run_api_check_test.go | 8 +++---- 5 files changed, 53 insertions(+), 17 deletions(-) diff --git a/models/actions/run_list.go b/models/actions/run_list.go index 2628c4712f..e948d49665 100644 --- a/models/actions/run_list.go +++ b/models/actions/run_list.go @@ -64,16 +64,17 @@ func (runs RunList) LoadRepos(ctx context.Context) error { type FindRunOptions struct { db.ListOptions - RepoID int64 - OwnerID int64 - WorkflowID string - Ref string // the commit/tag/… that caused this workflow - TriggerUserID int64 - TriggerEvent webhook_module.HookEventType - Approved bool // not util.OptionalBool, it works only when it's true - Status []Status - ConcurrencyGroup string - CommitSHA string + RepoID int64 + OwnerID int64 + WorkflowID string + Ref string // the commit/tag/… that caused this workflow + TriggerUserID int64 + TriggerEvent webhook_module.HookEventType + Approved bool // not util.OptionalBool, it works only when it's true + Status []Status + ConcurrencyGroup string + CommitSHA string + ExcludePullRequests bool } func (opts FindRunOptions) ToConds() builder.Cond { @@ -102,6 +103,9 @@ func (opts FindRunOptions) ToConds() builder.Cond { if opts.CommitSHA != "" { cond = cond.And(builder.Eq{"`action_run`.commit_sha": opts.CommitSHA}) } + if opts.ExcludePullRequests { + cond = cond.And(builder.Neq{"`action_run`.trigger_event": "pull_request"}) + } if len(opts.ConcurrencyGroup) > 0 { if opts.RepoID == 0 { panic("Invalid FindRunOptions: repo_id is required") diff --git a/routers/api/v1/repo/action.go b/routers/api/v1/repo/action.go index 37ea45e14e..cd24d1bb2d 100644 --- a/routers/api/v1/repo/action.go +++ b/routers/api/v1/repo/action.go @@ -763,6 +763,11 @@ func (Action) ListWorkflowRuns(ctx *context.APIContext) { // description: triggering sha of the workflow run // type: string // required: false + // - name: exclude_pull_requests + // in: query + // description: if true, pull request events are omitted from the results + // type: boolean + // required: false // - name: page // in: query // description: page number of results to return (1-based) @@ -971,7 +976,7 @@ func ActionsListWorkflowRuns(ctx *context.APIContext) { // required: true // - name: workflow_id // in: path - // description: id of the workflow + // description: id of the workflow, must be the workflow file name (e.g. `build.yml`) // type: string // required: true // - name: event @@ -999,6 +1004,11 @@ func ActionsListWorkflowRuns(ctx *context.APIContext) { // description: triggering sha of the workflow run // type: string // required: false + // - name: exclude_pull_requests + // in: query + // description: if true, pull request events are omitted from the results + // type: boolean + // required: false // - name: page // in: query // description: page number of results to return (1-based) @@ -1018,6 +1028,15 @@ func ActionsListWorkflowRuns(ctx *context.APIContext) { // "$ref": "#/responses/notFound" workflowID := ctx.PathParam("workflow_id") + if _, err := convert.GetActionWorkflow(ctx, ctx.Repo.GitRepo, ctx.Repo.Repository, workflowID); err != nil { + if errors.Is(err, util.ErrNotExist) { + ctx.APIError(http.StatusNotFound, err) + } else { + ctx.APIErrorInternal(err) + } + return + } + repoID := ctx.Repo.Repository.ID shared.ListRuns(ctx, 0, repoID, workflowID) diff --git a/routers/api/v1/shared/action.go b/routers/api/v1/shared/action.go index 8f2db38f7c..9a1e807865 100644 --- a/routers/api/v1/shared/action.go +++ b/routers/api/v1/shared/action.go @@ -156,6 +156,9 @@ func ListRuns(ctx *context.APIContext, ownerID, repoID int64, workflowID string) if headSHA := ctx.FormString("head_sha"); headSHA != "" { opts.CommitSHA = headSHA } + if ctx.FormString("exclude_pull_requests") == "true" { + opts.ExcludePullRequests = true + } runs, total, err := db.FindAndCount[actions_model.ActionRun](ctx, opts) if err != nil { diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index cf62394dbc..899be8ae2d 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -5305,6 +5305,12 @@ "name": "head_sha", "in": "query" }, + { + "type": "boolean", + "description": "if true, pull request events are omitted from the results", + "name": "exclude_pull_requests", + "in": "query" + }, { "type": "integer", "description": "page number of results to return (1-based)", @@ -6471,7 +6477,7 @@ }, { "type": "string", - "description": "id of the workflow", + "description": "id of the workflow, must be the workflow file name (e.g. `build.yml`)", "name": "workflow_id", "in": "path", "required": true @@ -6506,6 +6512,12 @@ "name": "head_sha", "in": "query" }, + { + "type": "boolean", + "description": "if true, pull request events are omitted from the results", + "name": "exclude_pull_requests", + "in": "query" + }, { "type": "integer", "description": "page number of results to return (1-based)", diff --git a/tests/integration/workflow_run_api_check_test.go b/tests/integration/workflow_run_api_check_test.go index 3c7e605c68..f518c161a0 100644 --- a/tests/integration/workflow_run_api_check_test.go +++ b/tests/integration/workflow_run_api_check_test.go @@ -50,18 +50,16 @@ func testAPIWorkflowRunsByWorkflowID(t *testing.T, owner, repo, workflowID, user verifyWorkflowRunCanbeFoundWithStatusFilter(t, workflowRunsURL, token, run.ID, "", run.Status, "", "", "", "") verifyWorkflowRunCanbeFoundWithStatusFilter(t, workflowRunsURL, token, run.ID, "", "", "", run.HeadBranch, "", "") verifyWorkflowRunCanbeFoundWithStatusFilter(t, workflowRunsURL, token, run.ID, "", "", run.Event, "", "", "") + verifyWorkflowRunCanbeFoundWithStatusFilter(t, workflowRunsURL, token, run.ID, "", "", "", "", run.TriggerActor.UserName, "") + verifyWorkflowRunCanbeFoundWithStatusFilter(t, workflowRunsURL, token, run.ID, "", "", "", "", run.TriggerActor.UserName, run.HeadSha) if run.ID == expectedRunID { found = true - break } } assert.True(t, found, "expected to find run with ID %d in workflow %s runs", expectedRunID, workflowID) req = NewRequest(t, "GET", fmt.Sprintf("/api/v1/repos/%s/%s/actions/workflows/nonexistent.yaml/runs", owner, repo)).AddTokenAuth(token) - resp = MakeRequest(t, req, http.StatusOK) - emptyList := api.ActionWorkflowRunsResponse{} - DecodeJSON(t, resp, &emptyList) - assert.Empty(t, emptyList.Entries, "nonexistent workflow should return no runs") + MakeRequest(t, req, http.StatusNotFound) } func testAPIWorkflowRunBasic(t *testing.T, apiRootURL, userUsername string, runID int64, scope ...auth_model.AccessTokenScope) {