fix: do not hide previous attempts without task for latest attempt

models/actions/run_job.go

@@ -153,6 +153,21 @@ func (job *ActionRunJob) CanBeRerun(ctx context.Context) (bool, error) {
 	return job.Status.IsDone(), nil
 }
 
+// GetAllAttempts retrieve all the attempts of this job. Limited fields are queried to avoid loading the LogIndexes blob
+// when not needed.
+func (job *ActionRunJob) GetAllAttempts(ctx context.Context) ([]*ActionTask, error) {
+	var attempts []*ActionTask
+	err := db.GetEngine(ctx).
+		Cols("id", "attempt", "status", "started").
+		Where("job_id=?", job.ID).
+		Desc("attempt").
+		Find(&attempts)
+	if err != nil {
+		return nil, err
+	}
+	return attempts, nil
+}
+
 func GetRunJobByID(ctx context.Context, id int64) (*ActionRunJob, error) {
 	var job ActionRunJob
 	has, err := db.GetEngine(ctx).Where("id=?", id).Get(&job)

models/actions/run_job_test.go

@@ -45,7 +45,7 @@ func TestActionRunJob_HTMLURL(t *testing.T) {
 	}{
 		{
 			id:       192,
-			expected: "https://try.gitea.io/user5/repo4/actions/runs/187/jobs/0/attempt/1",
+			expected: "https://try.gitea.io/user5/repo4/actions/runs/187/jobs/0/attempt/3",
 		},
 		{
 			id:       393,
@@ -502,3 +502,22 @@ func TestActionRunJob_CanBeRerun(t *testing.T) {
 		})
 	}
 }
+
+func TestActionTask_GetAllAttempts(t *testing.T) {
+	require.NoError(t, unittest.PrepareTestDatabase())
+
+	job2 := unittest.AssertExistsAndLoadBean(t, &ActionRunJob{ID: 192})
+
+	allAttempts, err := job2.GetAllAttempts(t.Context())
+	require.NoError(t, err)
+
+	require.Len(t, allAttempts, 3)
+	assert.EqualValues(t, 47, allAttempts[0].ID, "ordered by attempt, 1")
+	assert.EqualValues(t, 53, allAttempts[1].ID, "ordered by attempt, 2")
+	assert.EqualValues(t, 52, allAttempts[2].ID, "ordered by attempt, 3")
+
+	// GetAllAttempts doesn't populate all fields; so check expected fields from one of the records
+	assert.EqualValues(t, 3, allAttempts[0].Attempt, "read Attempt field")
+	assert.Equal(t, StatusRunning, allAttempts[0].Status, "read Status field")
+	assert.Equal(t, timeutil.TimeStamp(1683636528), allAttempts[0].Started, "read Started field")
+}

models/actions/task.go

@@ -161,21 +161,6 @@ func (task *ActionTask) UpdateToken(ctx context.Context) error {
 	return UpdateTask(ctx, task, "token_hash", "token_salt", "token_last_eight")
 }
 
-// Retrieve all the attempts from the same job as the target `ActionTask`.  Limited fields are queried to avoid loading
-// the LogIndexes blob when not needed.
-func (task *ActionTask) GetAllAttempts(ctx context.Context) ([]*ActionTask, error) {
-	var attempts []*ActionTask
-	err := db.GetEngine(ctx).
-		Cols("id", "attempt", "status", "started").
-		Where("job_id=?", task.JobID).
-		Desc("attempt").
-		Find(&attempts)
-	if err != nil {
-		return nil, err
-	}
-	return attempts, nil
-}
-
 func GetTaskByID(ctx context.Context, id int64) (*ActionTask, error) {
 	var task ActionTask
 	has, err := db.GetEngine(ctx).Where("id=?", id).Get(&task)

models/actions/task_test.go

@@ -6,35 +6,12 @@ package actions
 import (
 	"testing"
 
-	"forgejo.org/models/db"
 	"forgejo.org/models/unittest"
-	"forgejo.org/modules/timeutil"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
-func TestActionTask_GetAllAttempts(t *testing.T) {
-	require.NoError(t, unittest.PrepareTestDatabase())
-
-	var task ActionTask
-	has, err := db.GetEngine(t.Context()).Where("id=?", 47).Get(&task)
-	require.NoError(t, err)
-	require.True(t, has, "load ActionTask from fixture")
-
-	allAttempts, err := task.GetAllAttempts(t.Context())
-	require.NoError(t, err)
-	require.Len(t, allAttempts, 3)
-	assert.EqualValues(t, 47, allAttempts[0].ID, "ordered by attempt, 1")
-	assert.EqualValues(t, 53, allAttempts[1].ID, "ordered by attempt, 2")
-	assert.EqualValues(t, 52, allAttempts[2].ID, "ordered by attempt, 3")
-
-	// GetAllAttempts doesn't populate all fields; so check expected fields from one of the records
-	assert.EqualValues(t, 3, allAttempts[0].Attempt, "read Attempt field")
-	assert.Equal(t, StatusRunning, allAttempts[0].Status, "read Status field")
-	assert.Equal(t, timeutil.TimeStamp(1683636528), allAttempts[0].Started, "read Started field")
-}
-
 func TestActionTask_GetTaskByJobAttempt(t *testing.T) {
 	require.NoError(t, unittest.PrepareTestDatabase())
 
@@ -58,7 +35,7 @@ func TestActionTask_CreatePlaceholderTask(t *testing.T) {
 
 	assert.NotEqualValues(t, 0, task.ID)
 	assert.Equal(t, job.ID, task.JobID)
-	assert.EqualValues(t, 0, task.Attempt)
+	assert.EqualValues(t, 1, task.Attempt)
 	assert.NotEqualValues(t, 0, task.Started)
 	assert.NotEqualValues(t, 0, task.Stopped)
 	assert.Equal(t, job.Status, task.Status)

models/fixtures/action_run_job.yml

@@ -6,7 +6,7 @@
   commit_sha: c2d72f548424103f01ee1dc02889c1e2bff816b0
   is_fork_pull_request: false
   name: job_2
-  attempt: 1
+  attempt: 3
   job_id: job_2
   task_id: 47
   status: 1
@@ -199,7 +199,7 @@
   commit_sha: 985f0301dba5e7b34be866819cd15ad3d8f508ee
   is_fork_pull_request: false
   name: job_2
-  attempt: 0
+  attempt: 1
   job_id: job_2
   task_id: null
   status: 5

options/locale_next/locale_en-US.json

@@ -848,6 +848,7 @@
 	"actions.runs.delete.button": "Delete run",
 	"actions.runs.delete.error": "Could not delete the workflow run.",
 	"actions.runs.delete.confirm_action": "Do you really want to delete this workflow run?",
+	"actions.jobs.not_started": "not started",
 	"actions.workflow.job_parsing_error": "Unable to parse jobs in workflow: %v",
 	"actions.workflow.event_detection_error": "Unable to parse supported events in workflow: %v",
 	"actions.workflow.persistent_incomplete_matrix": "Unable to evaluate `strategy.matrix` of job %[1]s due to a `needs` expression that was invalid. It may reference a job that is not in it's 'needs' list (%[2]s), or an output that doesn't exist on one of those jobs.",

routers/web/repo/actions/view.go

@@ -349,6 +349,39 @@ func getViewResponse(ctx *app_context.Context, req *ViewRequest, runIndex, jobIn
 		Branch:         branch,
 	}
 
+	taskAttempts, err := current.GetAllAttempts(ctx)
+	if err != nil {
+		ctx.Error(http.StatusInternalServerError, err.Error())
+		return nil
+	}
+
+	var allAttempts []*TaskAttempt
+	// If the latest attempt has not been taken up yet by a runner, there is no task that could be displayed. As a
+	// stopgap, inject a phantom task that provides the necessary information until a real tasks is created, if ever.
+	if len(taskAttempts) == 0 || taskAttempts[0].Attempt != current.Attempt {
+		taskAttempt := &TaskAttempt{
+			Number:            current.Attempt,
+			Status:            current.Status.String(),
+			Started:           template.HTML(ctx.Locale.TrString("actions.jobs.not_started")),
+			StatusDiagnostics: statusDiagnostics(current.Status, current, ctx.Locale),
+		}
+		allAttempts = append(allAttempts, taskAttempt)
+	}
+	for _, actionTask := range taskAttempts {
+		taskAttempt := &TaskAttempt{
+			Number:            actionTask.Attempt,
+			Started:           templates.TimeSince(actionTask.Started),
+			Status:            actionTask.Status.String(),
+			StatusDiagnostics: statusDiagnostics(actionTask.Status, current, ctx.Locale),
+		}
+		allAttempts = append(allAttempts, taskAttempt)
+	}
+
+	resp.State.CurrentJob.Title = current.Name
+	resp.State.CurrentJob.Details = statusDiagnostics(current.Status, current, ctx.Locale)
+	resp.State.CurrentJob.Steps = make([]*ViewJobStep, 0) // marshal to '[]' instead of 'null' in json
+	resp.State.CurrentJob.AllAttempts = allAttempts
+
 	var task *actions_model.ActionTask
 	// TaskID will be set only when the ActionRunJob has been picked by a runner, resulting in an ActionTask being
 	// created representing the specific task.  If current.TaskID is not set, then the user is attempting to view a job
@@ -369,29 +402,9 @@ func getViewResponse(ctx *app_context.Context, req *ViewRequest, runIndex, jobIn
 		}
 	}
 
-	resp.State.CurrentJob.Title = current.Name
-	resp.State.CurrentJob.Details = statusDiagnostics(current.Status, current, ctx.Locale)
-
-	resp.State.CurrentJob.Steps = make([]*ViewJobStep, 0) // marshal to '[]' instead of 'null' in json
-	resp.Logs.StepsLog = make([]*ViewStepLog, 0)          // marshal to '[]' instead of 'null' in json
+	resp.Logs.StepsLog = make([]*ViewStepLog, 0) // marshal to '[]' instead of 'null' in json
 	// As noted above with TaskID; task will be nil when the job hasn't be picked yet...
 	if task != nil {
-		taskAttempts, err := task.GetAllAttempts(ctx)
-		if err != nil {
-			ctx.Error(http.StatusInternalServerError, err.Error())
-			return nil
-		}
-		allAttempts := make([]*TaskAttempt, len(taskAttempts))
-		for i, actionTask := range taskAttempts {
-			allAttempts[i] = &TaskAttempt{
-				Number:            actionTask.Attempt,
-				Started:           templates.TimeSince(actionTask.Started),
-				Status:            actionTask.Status.String(),
-				StatusDiagnostics: statusDiagnostics(actionTask.Status, task.Job, ctx.Locale),
-			}
-		}
-		resp.State.CurrentJob.AllAttempts = allAttempts
-
 		steps := actions.FullSteps(task)
 		for _, v := range steps {
 			resp.State.CurrentJob.Steps = append(resp.State.CurrentJob.Steps, &ViewJobStep{

routers/web/repo/actions/view_test.go

@@ -340,7 +340,14 @@ func TestActionsViewViewPost(t *testing.T) {
 				// in-sync with the RepoActionView 'view non-picked action run job' test.
 				resp.State.CurrentJob.Details = []template.HTML{"actions.status.diagnostics.waiting"}
 				resp.State.CurrentJob.Steps = []*ViewJobStep{}
-				resp.State.CurrentJob.AllAttempts = nil
+				resp.State.CurrentJob.AllAttempts = []*TaskAttempt{
+					{
+						Number:            1,
+						Started:           template.HTML("actions.jobs.not_started"),
+						Status:            "waiting",
+						StatusDiagnostics: []template.HTML{"actions.status.diagnostics.waiting"},
+					},
+				}
 			},
 		},
 	}

tests/integration/actions_view_test.go

@@ -147,8 +147,8 @@ func TestActionViewsView(t *testing.T) {
 			url:               "/user5/repo4/actions/runs/187",
 			runIndex:          187,
 			jobIndex:          0,
-			attempt:           1,
-			expectedJSON:      "{\"state\":{\"run\":{\"preExecutionError\":\"\",\"link\":\"/user5/repo4/actions/runs/187\",\"title\":\"update actions\",\"titleHTML\":\"update actions\",\"status\":\"success\",\"canCancel\":false,\"canDelete\":false,\"canApprove\":false,\"canRerun\":false,\"canDeleteArtifact\":false,\"description\":\"Commit <a href=\\\"/user5/repo4/commit/c2d72f548424103f01ee1dc02889c1e2bff816b0\\\">c2d72f5484</a> pushed by <a href=\\\"/user1\\\">user1</a>\",\"done\":true,\"jobs\":[{\"id\":192,\"name\":\"job_2\",\"status\":\"success\",\"canRerun\":false,\"duration\":\"_duration_\"}],\"commit\":{\"localeWorkflow\":\"Workflow\",\"localeAllRuns\":\"all runs\",\"shortSHA\":\"c2d72f5484\",\"link\":\"/user5/repo4/commit/c2d72f548424103f01ee1dc02889c1e2bff816b0\",\"pusher\":{\"displayName\":\"user1\",\"link\":\"/user1\"},\"branch\":{\"name\":\"master\",\"link\":\"/user5/repo4/src/branch/master\",\"isDeleted\":false}}},\"currentJob\":{\"title\":\"job_2\",\"details\":[\"Success\"],\"steps\":[{\"summary\":\"Set up job\",\"duration\":\"_duration_\",\"status\":\"success\"},{\"summary\":\"Complete job\",\"duration\":\"_duration_\",\"status\":\"success\"}],\"allAttempts\":[{\"number\":3,\"time_since_started_html\":\"_time_\",\"status\":\"running\",\"status_diagnostics\":[\"Running\"]},{\"number\":2,\"time_since_started_html\":\"_time_\",\"status\":\"success\",\"status_diagnostics\":[\"Success\"]},{\"number\":1,\"time_since_started_html\":\"_time_\",\"status\":\"success\",\"status_diagnostics\":[\"Success\"]}]}},\"logs\":{\"stepsLog\":[]}}\n",
+			attempt:           3,
+			expectedJSON:      "{\"state\":{\"run\":{\"preExecutionError\":\"\",\"link\":\"/user5/repo4/actions/runs/187\",\"title\":\"update actions\",\"titleHTML\":\"update actions\",\"status\":\"success\",\"canCancel\":false,\"canDelete\":false,\"canApprove\":false,\"canRerun\":false,\"canDeleteArtifact\":false,\"description\":\"Commit <a href=\\\"/user5/repo4/commit/c2d72f548424103f01ee1dc02889c1e2bff816b0\\\">c2d72f5484</a> pushed by <a href=\\\"/user1\\\">user1</a>\",\"done\":true,\"jobs\":[{\"id\":192,\"name\":\"job_2\",\"status\":\"success\",\"canRerun\":false,\"duration\":\"_duration_\"}],\"commit\":{\"localeWorkflow\":\"Workflow\",\"localeAllRuns\":\"all runs\",\"shortSHA\":\"c2d72f5484\",\"link\":\"/user5/repo4/commit/c2d72f548424103f01ee1dc02889c1e2bff816b0\",\"pusher\":{\"displayName\":\"user1\",\"link\":\"/user1\"},\"branch\":{\"name\":\"master\",\"link\":\"/user5/repo4/src/branch/master\",\"isDeleted\":false}}},\"currentJob\":{\"title\":\"job_2\",\"details\":[\"Success\"],\"steps\":[{\"summary\":\"Set up job\",\"duration\":\"_duration_\",\"status\":\"running\"},{\"summary\":\"Complete job\",\"duration\":\"_duration_\",\"status\":\"waiting\"}],\"allAttempts\":[{\"number\":3,\"time_since_started_html\":\"_time_\",\"status\":\"running\",\"status_diagnostics\":[\"Running\"]},{\"number\":2,\"time_since_started_html\":\"_time_\",\"status\":\"success\",\"status_diagnostics\":[\"Success\"]},{\"number\":1,\"time_since_started_html\":\"_time_\",\"status\":\"success\",\"status_diagnostics\":[\"Success\"]}]}},\"logs\":{\"stepsLog\":[]}}\n",
 			expectedArtifacts: "{\"artifacts\":[{\"name\":\"multi-file-download\",\"size\":2048,\"status\":\"completed\"}]}\n",
 		},
 		{
@@ -230,7 +230,7 @@ func TestActionViewsViewAttemptOutOfRange(t *testing.T) {
 		re = regexp.MustCompile(pattern)
 		actualClean = re.ReplaceAllString(actualClean, `"time_since_started_html":"_time_"`)
 
-		assert.JSONEq(t, "{\"state\":{\"run\":{\"preExecutionError\":\"\",\"link\":\"/user5/repo4/actions/runs/190\",\"title\":\"job output\",\"titleHTML\":\"job output\",\"status\":\"success\",\"canCancel\":false,\"canDelete\":false,\"canApprove\":false,\"canRerun\":false,\"canDeleteArtifact\":false,\"description\":\"Commit <a href=\\\"/user5/repo4/commit/c2d72f548424103f01ee1dc02889c1e2bff816b0\\\">c2d72f5484</a> pushed by <a href=\\\"/user1\\\">user1</a>\",\"done\":false,\"jobs\":[{\"id\":396,\"name\":\"job_2\",\"status\":\"waiting\",\"canRerun\":false,\"duration\":\"_duration_\"}],\"commit\":{\"localeWorkflow\":\"Workflow\",\"localeAllRuns\":\"all runs\",\"shortSHA\":\"c2d72f5484\",\"link\":\"/user5/repo4/commit/c2d72f548424103f01ee1dc02889c1e2bff816b0\",\"pusher\":{\"displayName\":\"user1\",\"link\":\"/user1\"},\"branch\":{\"name\":\"test\",\"link\":\"/user5/repo4/src/branch/test\",\"isDeleted\":true}}},\"currentJob\":{\"title\":\"job_2\",\"details\":[\"Waiting for a runner with the following label: fedora\"],\"steps\":[],\"allAttempts\":null}},\"logs\":{\"stepsLog\":[]}}\n", actualClean)
+		assert.JSONEq(t, "{\"state\":{\"run\":{\"preExecutionError\":\"\",\"link\":\"/user5/repo4/actions/runs/190\",\"title\":\"job output\",\"titleHTML\":\"job output\",\"status\":\"success\",\"canCancel\":false,\"canDelete\":false,\"canApprove\":false,\"canRerun\":false,\"canDeleteArtifact\":false,\"description\":\"Commit <a href=\\\"/user5/repo4/commit/c2d72f548424103f01ee1dc02889c1e2bff816b0\\\">c2d72f5484</a> pushed by <a href=\\\"/user1\\\">user1</a>\",\"done\":false,\"jobs\":[{\"id\":396,\"name\":\"job_2\",\"status\":\"waiting\",\"canRerun\":false,\"duration\":\"_duration_\"}],\"commit\":{\"localeWorkflow\":\"Workflow\",\"localeAllRuns\":\"all runs\",\"shortSHA\":\"c2d72f5484\",\"link\":\"/user5/repo4/commit/c2d72f548424103f01ee1dc02889c1e2bff816b0\",\"pusher\":{\"displayName\":\"user1\",\"link\":\"/user1\"},\"branch\":{\"name\":\"test\",\"link\":\"/user5/repo4/src/branch/test\",\"isDeleted\":true}}},\"currentJob\":{\"title\":\"job_2\",\"details\":[\"Waiting for a runner with the following label: fedora\"],\"steps\":[],\"allAttempts\":[{\"number\":1,\"status\":\"waiting\",\"status_diagnostics\":[\"Waiting for a runner with the following label: fedora\"],\"time_since_started_html\":\"not started\"}]}},\"logs\":{\"stepsLog\":[]}}\n", actualClean)
 	})
 	htmlDoc.AssertAttrEqual(t, selector, "data-initial-artifacts-response", "{\"artifacts\":[]}\n")
 }