chore: re-enable nilnil lint for models/actions/run.go (#12507)

`GetRunByID` is now the primary function to get a action run. `GetRunBefore` now returns `util.ErrNotExist` to avoid the nilnil pattern.

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12507
Reviewed-by: Gusted <gusted@noreply.codeberg.org>

.golangci.yml

@@ -189,9 +189,6 @@ linters:
         text: "(ST1005|ST1003|QF1001):"
 
       # TODO: eventually remove this section entirely
-      - path: models/actions/run.go
-        linters:
-          - nilnil
       - path: models/actions/task.go
         linters:
           - nilnil

models/actions/run.go

@@ -477,7 +477,7 @@ func GetLatestRun(ctx context.Context, repoID int64) (*ActionRun, error) {
 func GetRunBefore(ctx context.Context, _ *ActionRun) (*ActionRun, error) {
 	// TODO return the most recent run related to the run given in argument
 	// see https://codeberg.org/forgejo/user-research/issues/63 for context
-	return nil, nil
+	return nil, util.ErrNotExist
 }
 
 func GetLatestRunForBranchAndWorkflow(ctx context.Context, repoID int64, branch, workflowFile, event string) (*ActionRun, error) {
@@ -499,26 +499,16 @@ func GetLatestRunForBranchAndWorkflow(ctx context.Context, repoID int64, branch,
 }
 
 func GetRunByID(ctx context.Context, id int64) (*ActionRun, error) {
-	run, has, err := GetRunByIDWithHas(ctx, id)
-	if err != nil {
-		return nil, err
-	} else if !has {
-		return nil, fmt.Errorf("run with id %d: %w", id, util.ErrNotExist)
-	}
-
-	return run, nil
-}
-
-func GetRunByIDWithHas(ctx context.Context, id int64) (*ActionRun, bool, error) {
 	var run ActionRun
 	has, err := db.GetEngine(ctx).Where("id=?", id).Get(&run)
 	if err != nil {
-		return nil, false, err
-	} else if !has {
-		return nil, false, nil
+		return nil, err
+	}
+	if !has {
+		return nil, fmt.Errorf("run with id %d: %w", id, util.ErrNotExist)
 	}
 
-	return &run, true, nil
+	return &run, nil
 }
 
 func GetRunByIndex(ctx context.Context, repoID, index int64) (*ActionRun, error) {

models/actions/run_test.go

@@ -13,6 +13,7 @@ import (
 	"forgejo.org/modules/cache"
 	"forgejo.org/modules/setting"
 	"forgejo.org/modules/test"
+	"forgejo.org/modules/util"
 
 	"code.forgejo.org/forgejo/runner/v12/act/jobparser"
 	"github.com/stretchr/testify/assert"
@@ -692,3 +693,29 @@ func TestActionRunLoadAttributes(t *testing.T) {
 	require.NoError(t, run.LoadAttributes(t.Context()))
 	assert.Equal(t, "ghost", run.TriggerUser.LowerName)
 }
+
+func TestGetRunByID(t *testing.T) {
+	const (
+		existingRunID    = 0xdeadbeef
+		nonexistingRunID = 0xffffffff
+	)
+
+	require.NoError(t, unittest.PrepareTestDatabase())
+
+	_, err := db.GetEngine(t.Context()).Insert(ActionRun{
+		ID: existingRunID,
+	})
+	require.NoError(t, err)
+
+	// ActionRun exists
+
+	run, err := GetRunByID(t.Context(), existingRunID)
+	require.NoError(t, err)
+	assert.NotNil(t, run)
+
+	// ActionRun does not exist
+
+	run, err = GetRunByID(t.Context(), nonexistingRunID)
+	require.ErrorIs(t, err, util.ErrNotExist)
+	assert.Nil(t, run)
+}

routers/web/repo/actions/view.go

@@ -739,12 +739,13 @@ func getRunByID(ctx *app_context.Context, runID int64) *actions_model.ActionRun
 		return nil
 	}
 
-	run, has, err := actions_model.GetRunByIDWithHas(ctx, runID)
-	if err != nil {
+	run, err := actions_model.GetRunByID(ctx, runID)
+	if err != nil && !errors.Is(err, util.ErrNotExist) {
 		ctx.Error(http.StatusInternalServerError, err.Error())
 		return nil
 	}
-	if !has {
+
+	if errors.Is(err, util.ErrNotExist) {
 		log.Debug("Requested runID[%d] not found.", runID)
 		ctx.Error(http.StatusNotFound, fmt.Sprintf("no such run %d", runID))
 		return nil