bug: unify RepoActionRun and ActionRun structs (#8250)

Two pull requests were merged at the same time

- https://codeberg.org/forgejo/forgejo/pulls/7699
- https://codeberg.org/forgejo/forgejo/pulls/7508

And added conflicting structs ActionRun modules/structs.  That broke
the forgejo development branch and a quick fix was made to resolve
the name conflict.

- https://codeberg.org/forgejo/forgejo/pulls/8066

However that creates an undesirable duplication of two structures that
serve the same purpose but are different.

- Remove RepoActionRun and replace it with ActionRun
- convert.ToActionRun has one more argument, the doer, because it
  is determined differently in the context of webhooks or API

### Tests

- No need because the two pull requests involved already have good coverage.

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8250
Reviewed-by: Michael Kriese <michael.kriese@gmx.de>
Reviewed-by: klausfyhn <klausfyhn@noreply.codeberg.org>
Reviewed-by: Christopher Besch <mail@chris-besch.com>
Co-authored-by: Earl Warren <contact@earl-warren.org>
Co-committed-by: Earl Warren <contact@earl-warren.org>

modules/structs/action.go

@@ -78,3 +78,9 @@ type ActionRun struct {
 	// the url of this action run
 	HTMLURL string `json:"html_url"`
 }
+
+// ListActionRunResponse return a list of ActionRun
+type ListActionRunResponse struct {
+	Entries    []*ActionRun `json:"workflow_runs"`
+	TotalCount int64        `json:"total_count"`
+}

modules/structs/repo_actions.go

@@ -32,23 +32,3 @@ type ActionTaskResponse struct {
 	Entries    []*ActionTask `json:"workflow_runs"`
 	TotalCount int64         `json:"total_count"`
 }
-
-// ActionRun represents an ActionRun
-type RepoActionRun struct {
-	ID              int64  `json:"id"`
-	Name            string `json:"name"`
-	RunNumber       int64  `json:"run_number"`
-	Event           string `json:"event"`
-	Status          string `json:"status"`
-	HeadBranch      string `json:"head_branch"`
-	HeadSHA         string `json:"head_sha"`
-	WorkflowID      string `json:"workflow_id"`
-	URL             string `json:"url"`
-	TriggeringActor *User  `json:"triggering_actor"`
-}
-
-// ListActionRunResponse return a list of ActionRun
-type ListRepoActionRunResponse struct {
-	Entries    []*RepoActionRun `json:"workflow_runs"`
-	TotalCount int64            `json:"total_count"`
-}

routers/api/v1/repo/action.go

@@ -748,7 +748,7 @@ func ListActionRuns(ctx *context.APIContext) {
 	//   type: string
 	// responses:
 	//   "200":
-	//     "$ref": "#/responses/RepoActionRunList"
+	//     "$ref": "#/responses/ActionRunList"
 	//   "400":
 	//     "$ref": "#/responses/error"
 	//   "403":
@@ -779,16 +779,16 @@ func ListActionRuns(ctx *context.APIContext) {
 		return
 	}
 
-	res := new(api.ListRepoActionRunResponse)
+	res := new(api.ListActionRunResponse)
 	res.TotalCount = total
 
-	res.Entries = make([]*api.RepoActionRun, len(runs))
+	res.Entries = make([]*api.ActionRun, len(runs))
 	for i, r := range runs {
-		cr, err := convert.ToRepoActionRun(ctx, r)
+		if err := r.LoadAttributes(ctx); err != nil {
-		if err != nil {
+			ctx.Error(http.StatusInternalServerError, "LoadAttributes", err)
-			ctx.Error(http.StatusInternalServerError, "ToActionRun", err)
 			return
 		}
+		cr := convert.ToActionRun(ctx, r, ctx.Doer)
 		res.Entries[i] = cr
 	}
 
@@ -821,7 +821,7 @@ func GetActionRun(ctx *context.APIContext) {
 	//   required: true
 	// responses:
 	//   "200":
-	//     "$ref": "#/responses/RepoActionRun"
+	//     "$ref": "#/responses/ActionRun"
 	//   "400":
 	//     "$ref": "#/responses/error"
 	//   "403":
@@ -839,16 +839,17 @@ func GetActionRun(ctx *context.APIContext) {
 		return
 	}
 
+	// Action runs lives in its own table, therefore we check that the
+	// run with the requested ID is owned by the repository
 	if ctx.Repo.Repository.ID != run.RepoID {
 		ctx.Error(http.StatusNotFound, "GetRunById", util.ErrNotExist)
 		return
 	}
 
-	res, err := convert.ToRepoActionRun(ctx, run)
+	if err := run.LoadAttributes(ctx); err != nil {
-	if err != nil {
+		ctx.Error(http.StatusInternalServerError, "LoadAttributes", err)
-		ctx.Error(http.StatusInternalServerError, "ToRepoActionRun", err)
 		return
 	}
 
-	ctx.JSON(http.StatusOK, res)
+	ctx.JSON(http.StatusOK, convert.ToActionRun(ctx, run, ctx.Doer))
 }

routers/api/v1/swagger/repo.go

@@ -463,16 +463,16 @@ type swaggerSyncForkInfo struct {
 	Body []api.SyncForkInfo `json:"body"`
 }
 
-// RepoActionRunList
+// ActionRunList
-// swagger:response RepoActionRunList
+// swagger:response ActionRunList
-type swaggerRepoActionRunList struct {
+type swaggerActionRunList struct {
 	// in:body
-	Body api.ListRepoActionRunResponse `json:"body"`
+	Body api.ListActionRunResponse `json:"body"`
 }
 
-// RepoActionRun
+// ActionRun
-// swagger:response RepoActionRun
+// swagger:response ActionRun
-type swaggerRepoActionRun struct {
+type swaggerActionRun struct {
 	// in:body
-	Body api.RepoActionRun `json:"body"`
+	Body api.ActionRun `json:"body"`
 }

services/convert/action.go

@@ -8,22 +8,17 @@ import (
 
 	actions_model "forgejo.org/models/actions"
 	access_model "forgejo.org/models/perm/access"
+	user_model "forgejo.org/models/user"
 	api "forgejo.org/modules/structs"
 )
 
 // ToActionRun convert actions_model.User to api.ActionRun
 // the run needs all attributes loaded
-func ToActionRun(ctx context.Context, run *actions_model.ActionRun) *api.ActionRun {
+func ToActionRun(ctx context.Context, run *actions_model.ActionRun, doer *user_model.User) *api.ActionRun {
 	if run == nil {
 		return nil
 	}
 
-	// The doer is the one whose perspective is used to view this ActionRun.
-	// In the best case we use the user that created the webhook.
-	// Unfortunately we don't know who that was.
-	// So instead we use the repo owner, who is able to create webhooks and allow others to do so by making them repo admins.
-	// This is pretty close to perfect.
-	doer := run.Repo.Owner
 	permissionInRepo, _ := access_model.GetUserRepoPermission(ctx, run.Repo, doer)
 
 	return &api.ActionRun{

services/convert/convert.go

@@ -222,29 +222,6 @@ func ToActionTask(ctx context.Context, t *actions_model.ActionTask) (*api.Action
 	}, nil
 }
 
-// ToRepoActionRun convert a actions_model.ActionRun to an api.RepoActionRun
-func ToRepoActionRun(ctx context.Context, r *actions_model.ActionRun) (*api.RepoActionRun, error) {
-	if err := r.LoadAttributes(ctx); err != nil {
-		return nil, err
-	}
-
-	url := strings.TrimSuffix(setting.AppURL, "/") + r.Link()
-	actor := ToUser(ctx, r.TriggerUser, nil)
-
-	return &api.RepoActionRun{
-		ID:              r.ID,
-		Name:            r.Title,
-		HeadBranch:      r.PrettyRef(),
-		HeadSHA:         r.CommitSHA,
-		RunNumber:       r.Index,
-		Event:           r.TriggerEvent,
-		Status:          r.Status.String(),
-		WorkflowID:      r.WorkflowID,
-		URL:             url,
-		TriggeringActor: actor,
-	}, nil
-}
-
 // ToVerification convert a git.Commit.Signature to an api.PayloadCommitVerification
 func ToVerification(ctx context.Context, c *git.Commit) *api.PayloadCommitVerification {
 	verif := asymkey_model.ParseCommitWithSignature(ctx, c)

services/webhook/notifier.go

@@ -894,9 +894,16 @@ func (m *webhookNotifier) ActionRunNowDone(ctx context.Context, run *actions_mod
 		Owner:      run.TriggerUser,
 	}
 
+	// The doer is the one whose perspective is used to view this ActionRun.
+	// In the best case we use the user that created the webhook.
+	// Unfortunately we don't know who that was.
+	// So instead we use the repo owner, who is able to create webhooks and allow others to do so by making them repo admins.
+	// This is pretty close to perfect.
+	doer := run.Repo.Owner
+
 	payload := &api.ActionPayload{
-		Run:         convert.ToActionRun(ctx, run),
+		Run:         convert.ToActionRun(ctx, run, doer),
-		LastRun:     convert.ToActionRun(ctx, lastRun),
+		LastRun:     convert.ToActionRun(ctx, lastRun, doer),
 		PriorStatus: priorStatus.String(),
 	}
 

templates/swagger/v1_json.tmpl

@@ -4985,7 +4985,7 @@
         ],
         "responses": {
           "200": {
-            "$ref": "#/responses/RepoActionRunList"
+            "$ref": "#/responses/ActionRunList"
           },
           "400": {
             "$ref": "#/responses/error"
@@ -5032,7 +5032,7 @@
         ],
         "responses": {
           "200": {
-            "$ref": "#/responses/RepoActionRun"
+            "$ref": "#/responses/ActionRun"
           },
           "400": {
             "$ref": "#/responses/error"
@@ -21120,6 +21120,129 @@
       },
       "x-go-package": "forgejo.org/modules/structs"
     },
+    "ActionRun": {
+      "description": "ActionRun represents an action run",
+      "type": "object",
+      "properties": {
+        "ScheduleID": {
+          "description": "the cron id for the schedule trigger",
+          "type": "integer",
+          "format": "int64"
+        },
+        "approved_by": {
+          "description": "who approved this action run",
+          "type": "integer",
+          "format": "int64",
+          "x-go-name": "ApprovedBy"
+        },
+        "commit_sha": {
+          "description": "the commit sha the action run ran on",
+          "type": "string",
+          "x-go-name": "CommitSHA"
+        },
+        "created": {
+          "description": "when the action run was created",
+          "type": "string",
+          "format": "date-time",
+          "x-go-name": "Created"
+        },
+        "duration": {
+          "$ref": "#/definitions/Duration"
+        },
+        "event": {
+          "description": "the webhook event that causes the workflow to run",
+          "type": "string",
+          "x-go-name": "Event"
+        },
+        "event_payload": {
+          "description": "the payload of the webhook event that causes the workflow to run",
+          "type": "string",
+          "x-go-name": "EventPayload"
+        },
+        "html_url": {
+          "description": "the url of this action run",
+          "type": "string",
+          "x-go-name": "HTMLURL"
+        },
+        "id": {
+          "description": "the action run id",
+          "type": "integer",
+          "format": "int64",
+          "x-go-name": "ID"
+        },
+        "index_in_repo": {
+          "description": "a unique number for each run of a repository",
+          "type": "integer",
+          "format": "int64",
+          "x-go-name": "Index"
+        },
+        "is_fork_pull_request": {
+          "description": "If this is triggered by a PR from a forked repository or an untrusted user, we need to check if it is approved and limit permissions when running the workflow.",
+          "type": "boolean",
+          "x-go-name": "IsForkPullRequest"
+        },
+        "is_ref_deleted": {
+          "description": "has the commit/tag/… the action run ran on been deleted",
+          "type": "boolean",
+          "x-go-name": "IsRefDeleted"
+        },
+        "need_approval": {
+          "description": "may need approval if it's a fork pull request",
+          "type": "boolean",
+          "x-go-name": "NeedApproval"
+        },
+        "prettyref": {
+          "description": "the commit/tag/… the action run ran on",
+          "type": "string",
+          "x-go-name": "PrettyRef"
+        },
+        "repository": {
+          "$ref": "#/definitions/Repository"
+        },
+        "started": {
+          "description": "when the action run was started",
+          "type": "string",
+          "format": "date-time",
+          "x-go-name": "Started"
+        },
+        "status": {
+          "description": "the current status of this run",
+          "type": "string",
+          "x-go-name": "Status"
+        },
+        "stopped": {
+          "description": "when the action run was stopped",
+          "type": "string",
+          "format": "date-time",
+          "x-go-name": "Stopped"
+        },
+        "title": {
+          "description": "the action run's title",
+          "type": "string",
+          "x-go-name": "Title"
+        },
+        "trigger_event": {
+          "description": "the trigger event defined in the `on` configuration of the triggered workflow",
+          "type": "string",
+          "x-go-name": "TriggerEvent"
+        },
+        "trigger_user": {
+          "$ref": "#/definitions/User"
+        },
+        "updated": {
+          "description": "when the action run was last updated",
+          "type": "string",
+          "format": "date-time",
+          "x-go-name": "Updated"
+        },
+        "workflow_id": {
+          "description": "the name of workflow file",
+          "type": "string",
+          "x-go-name": "WorkflowID"
+        }
+      },
+      "x-go-package": "forgejo.org/modules/structs"
+    },
     "ActionRunJob": {
       "description": "ActionRunJob represents a job of a run",
       "type": "object",
@@ -23610,6 +23733,12 @@
       },
       "x-go-package": "forgejo.org/modules/structs"
     },
+    "Duration": {
+      "description": "A Duration represents the elapsed time between two instants\nas an int64 nanosecond count. The representation limits the\nlargest representable duration to approximately 290 years.",
+      "type": "integer",
+      "format": "int64",
+      "x-go-package": "time"
+    },
     "EditAttachmentOptions": {
       "description": "EditAttachmentOptions options for editing attachments",
       "type": "object",
@@ -25576,7 +25705,7 @@
       },
       "x-go-package": "forgejo.org/modules/structs"
     },
-    "ListRepoActionRunResponse": {
+    "ListActionRunResponse": {
       "description": "ListActionRunResponse return a list of ActionRun",
       "type": "object",
       "properties": {
@@ -25588,7 +25717,7 @@
         "workflow_runs": {
           "type": "array",
           "items": {
-            "$ref": "#/definitions/RepoActionRun"
+            "$ref": "#/definitions/ActionRun"
           },
           "x-go-name": "Entries"
         }
@@ -27353,54 +27482,6 @@
       },
       "x-go-package": "forgejo.org/modules/structs"
     },
-    "RepoActionRun": {
-      "description": "ActionRun represents an ActionRun",
-      "type": "object",
-      "properties": {
-        "event": {
-          "type": "string",
-          "x-go-name": "Event"
-        },
-        "head_branch": {
-          "type": "string",
-          "x-go-name": "HeadBranch"
-        },
-        "head_sha": {
-          "type": "string",
-          "x-go-name": "HeadSHA"
-        },
-        "id": {
-          "type": "integer",
-          "format": "int64",
-          "x-go-name": "ID"
-        },
-        "name": {
-          "type": "string",
-          "x-go-name": "Name"
-        },
-        "run_number": {
-          "type": "integer",
-          "format": "int64",
-          "x-go-name": "RunNumber"
-        },
-        "status": {
-          "type": "string",
-          "x-go-name": "Status"
-        },
-        "triggering_actor": {
-          "$ref": "#/definitions/User"
-        },
-        "url": {
-          "type": "string",
-          "x-go-name": "URL"
-        },
-        "workflow_id": {
-          "type": "string",
-          "x-go-name": "WorkflowID"
-        }
-      },
-      "x-go-package": "forgejo.org/modules/structs"
-    },
     "RepoCollaboratorPermission": {
       "description": "RepoCollaboratorPermission to get repository permission for a collaborator",
       "type": "object",
@@ -28847,6 +28928,18 @@
         }
       }
     },
+    "ActionRun": {
+      "description": "ActionRun",
+      "schema": {
+        "$ref": "#/definitions/ActionRun"
+      }
+    },
+    "ActionRunList": {
+      "description": "ActionRunList",
+      "schema": {
+        "$ref": "#/definitions/ListActionRunResponse"
+      }
+    },
     "ActionVariable": {
       "description": "ActionVariable",
       "schema": {
@@ -29616,18 +29709,6 @@
         }
       }
     },
-    "RepoActionRun": {
-      "description": "RepoActionRun",
-      "schema": {
-        "$ref": "#/definitions/RepoActionRun"
-      }
-    },
-    "RepoActionRunList": {
-      "description": "RepoActionRunList",
-      "schema": {
-        "$ref": "#/definitions/ListRepoActionRunResponse"
-      }
-    },
     "RepoCollaboratorPermission": {
       "description": "RepoCollaboratorPermission",
       "schema": {

tests/integration/api_repo_actions_test.go

@@ -169,7 +169,7 @@ func TestAPIGetListActionRun(t *testing.T) {
 			req.AddTokenAuth(token)
 
 			res := MakeRequest(t, req, http.StatusOK)
-			apiRuns := new(api.ListRepoActionRunResponse)
+			apiRuns := new(api.ListActionRunResponse)
 			DecodeJSON(t, res, apiRuns)
 
 			assert.Equal(t, int64(len(tt.expectedIDs)), apiRuns.TotalCount)
@@ -231,13 +231,13 @@ func TestAPIGetActionRun(t *testing.T) {
 			}
 
 			dbRun := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{ID: tt.runID})
-			apiRun := new(api.RepoActionRun)
+			apiRun := new(api.ActionRun)
 			DecodeJSON(t, res, apiRun)
 
-			assert.Equal(t, dbRun.Index, apiRun.RunNumber)
+			assert.Equal(t, dbRun.Index, apiRun.Index)
 			assert.Equal(t, dbRun.Status.String(), apiRun.Status)
-			assert.Equal(t, dbRun.CommitSHA, apiRun.HeadSHA)
+			assert.Equal(t, dbRun.CommitSHA, apiRun.CommitSHA)
-			assert.Equal(t, dbRun.TriggerUserID, apiRun.TriggeringActor.ID)
+			assert.Equal(t, dbRun.TriggerUserID, apiRun.TriggerUser.ID)
 		})
 	}
 }