diff --git a/models/actions/run.go b/models/actions/run.go
index 6e0c70f8cf..dc5f522317 100644
--- a/models/actions/run.go
+++ b/models/actions/run.go
@@ -309,15 +309,26 @@ func GetLatestRunForBranchAndWorkflow(ctx context.Context, repoID int64, branch,
}
func GetRunByID(ctx context.Context, id int64) (*ActionRun, error) {
- var run ActionRun
- has, err := db.GetEngine(ctx).Where("id=?", id).Get(&run)
+ 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
+ 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 &run, true, nil
}
func GetRunByIndex(ctx context.Context, repoID, index int64) (*ActionRun, error) {
diff --git a/release-notes/9023.md b/release-notes/9023.md
new file mode 100644
index 0000000000..47db1abc0b
--- /dev/null
+++ b/release-notes/9023.md
@@ -0,0 +1 @@
+The `artifact-url` ouput [returned by the upload-artifact@v4 action](https://code.forgejo.org/actions/upload-artifact#outputs) can be used to download the artifact. It was previously 404. To implement this compatibility fix, the web UI URL to download artifacts (i.e. `/{owner}/{repo}/actions/runs/{run_id}/artifacts/{artifact_name}`) now relies on an identifier that is unique accross the instance. URLs to download artifacts that were bookmarked or copied prior to this change use an id relative to the repository and will no longer work. It previously was `/{owner}/{repo}/actions/runs/{run_index}/artifacts/{artifact_name}`, note the difference between `{run_id}` and `{run_index}`. The new URL can be obtained again by visiting the parent page, which still uses the relative id (`/{owner}/{repo}/actions/runs/{run_index}`).
diff --git a/routers/web/repo/actions/view.go b/routers/web/repo/actions/view.go
index a325c51afe..b0d9346247 100644
--- a/routers/web/repo/actions/view.go
+++ b/routers/web/repo/actions/view.go
@@ -53,6 +53,7 @@ func View(ctx *context_module.Context) {
workflowName := job.Run.WorkflowID
ctx.Data["RunIndex"] = runIndex
+ ctx.Data["RunID"] = job.Run.ID
ctx.Data["JobIndex"] = jobIndex
ctx.Data["ActionsURL"] = ctx.Repo.RepoLink + "/actions"
ctx.Data["WorkflowName"] = workflowName
@@ -668,6 +669,31 @@ func ArtifactsDeleteView(ctx *context_module.Context) {
ctx.JSON(http.StatusOK, struct{}{})
}
+func getRunByID(ctx *context_module.Context, runID int64) *actions_model.ActionRun {
+ if runID == 0 {
+ log.Debug("Requested runID is zero.")
+ ctx.Error(http.StatusNotFound, "zero is not a valid run ID")
+ return nil
+ }
+
+ run, has, err := actions_model.GetRunByIDWithHas(ctx, runID)
+ if err != nil {
+ ctx.Error(http.StatusInternalServerError, err.Error())
+ return nil
+ }
+ if !has {
+ log.Debug("Requested runID[%d] not found.", runID)
+ ctx.Error(http.StatusNotFound, fmt.Sprintf("no such run %d", runID))
+ return nil
+ }
+ if run.RepoID != ctx.Repo.Repository.ID {
+ log.Debug("Requested runID[%d] does not belong to repo[%-v].", runID, ctx.Repo.Repository)
+ ctx.Error(http.StatusNotFound, "no such run")
+ return nil
+ }
+ return run
+}
+
func artifactsFind(ctx *context_module.Context, opts actions_model.FindArtifactsOptions) []*actions_model.ActionArtifact {
artifacts, err := db.Find[actions_model.ActionArtifact](ctx, opts)
if err != nil {
@@ -711,18 +737,11 @@ func artifactsFindByNameOrID(ctx *context_module.Context, runID int64, nameOrID
}
func ArtifactsDownloadView(ctx *context_module.Context) {
- runIndex := ctx.ParamsInt64("run")
- artifactNameOrID := ctx.Params("artifact_name_or_id")
-
- run, err := actions_model.GetRunByIndex(ctx, ctx.Repo.Repository.ID, runIndex)
- if err != nil {
- if errors.Is(err, util.ErrNotExist) {
- ctx.Error(http.StatusNotFound, err.Error())
- return
- }
- ctx.Error(http.StatusInternalServerError, err.Error())
+ run := getRunByID(ctx, ctx.ParamsInt64("run"))
+ if ctx.Written() {
return
}
+ artifactNameOrID := ctx.Params("artifact_name_or_id")
artifacts := artifactsFindByNameOrID(ctx, run.ID, artifactNameOrID)
if ctx.Written() {
diff --git a/routers/web/repo/actions/view_test.go b/routers/web/repo/actions/view_test.go
index d6a390befc..dae1b47e6e 100644
--- a/routers/web/repo/actions/view_test.go
+++ b/routers/web/repo/actions/view_test.go
@@ -8,12 +8,54 @@ import (
"testing"
actions_model "forgejo.org/models/actions"
+ repo_model "forgejo.org/models/repo"
unittest "forgejo.org/models/unittest"
"forgejo.org/services/contexttest"
"github.com/stretchr/testify/assert"
)
+func Test_getRunByID(t *testing.T) {
+ unittest.PrepareTestEnv(t)
+
+ repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerID: 5, ID: 4})
+
+ for _, testCase := range []struct {
+ name string
+ runID int64
+ err string
+ }{
+ {
+ name: "Found",
+ runID: 792,
+ },
+ {
+ name: "NotFound",
+ runID: 24344,
+ err: "no such run",
+ },
+ {
+ name: "ZeroNotFound",
+ runID: 0,
+ err: "zero is not a valid run ID",
+ },
+ } {
+ t.Run(testCase.name, func(t *testing.T) {
+ ctx, resp := contexttest.MockContext(t, fmt.Sprintf("user5/repo4/actions/runs/%v/artifacts/some-name", testCase.runID))
+ ctx.Repo.Repository = repo
+ run := getRunByID(ctx, testCase.runID)
+ if testCase.err == "" {
+ assert.NotNil(t, run)
+ assert.False(t, ctx.Written(), resp.Body.String())
+ } else {
+ assert.Nil(t, run)
+ assert.True(t, ctx.Written())
+ assert.Contains(t, resp.Body.String(), testCase.err)
+ }
+ })
+ }
+}
+
func Test_artifactsFind(t *testing.T) {
unittest.PrepareTestEnv(t)
diff --git a/templates/repo/actions/view.tmpl b/templates/repo/actions/view.tmpl
index b787762719..abd3c5c764 100644
--- a/templates/repo/actions/view.tmpl
+++ b/templates/repo/actions/view.tmpl
@@ -4,6 +4,7 @@
{{template "repo/header" .}}
{
expect(wrapper.get('.job-step-section:nth-of-type(2) .job-log-line:nth-of-type(2) .log-msg').text()).toEqual('Step #2 Log #2');
expect(wrapper.get('.job-step-section:nth-of-type(2) .job-log-line:nth-of-type(3) .log-msg').text()).toEqual('Step #2 Log #3');
});
+
+test('artifacts download links', async () => {
+ Object.defineProperty(document.documentElement, 'lang', {value: 'en'});
+ vi.spyOn(global, 'fetch').mockImplementation((url, opts) => {
+ if (url.endsWith('/artifacts')) {
+ return Promise.resolve({
+ ok: true,
+ json: vi.fn().mockResolvedValue(
+ {
+ artifacts: [
+ {name: 'artifactname1', size: 111, status: 'completed'},
+ {name: 'artifactname2', size: 222, status: 'expired'},
+ ],
+ },
+ ),
+ });
+ }
+
+ const postBody = JSON.parse(opts.body);
+ const stepsLog_value = [];
+ for (const cursor of postBody.logCursors) {
+ if (cursor.expanded) {
+ stepsLog_value.push(
+ {
+ step: cursor.step,
+ cursor: 0,
+ lines: [
+ {index: 1, message: `Step #${cursor.step + 1} Log #1`, timestamp: 0},
+ ],
+ },
+ );
+ }
+ }
+ const jobs_value = {
+ state: {
+ run: {
+ status: 'success',
+ commit: {
+ pusher: {},
+ },
+ },
+ currentJob: {
+ steps: [
+ {
+ summary: 'Test Step #1',
+ duration: '1s',
+ status: 'success',
+ },
+ ],
+ },
+ },
+ logs: {
+ stepsLog: opts.body?.includes('"cursor":null') ? stepsLog_value : [],
+ },
+ };
+
+ return Promise.resolve({
+ ok: true,
+ json: vi.fn().mockResolvedValue(
+ jobs_value,
+ ),
+ });
+ });
+
+ const wrapper = mount(RepoActionView, {
+ props: {
+ actionsURL: 'https://example.com/example-org/example-repo/actions',
+ runIndex: '10',
+ runID: '1001',
+ jobIndex: '2',
+ locale: {
+ approve: '',
+ cancel: '',
+ rerun: '',
+ artifactsTitle: 'artifactTitleHere',
+ areYouSure: '',
+ confirmDeleteArtifact: '',
+ rerun_all: '',
+ showTimeStamps: '',
+ showLogSeconds: '',
+ showFullScreen: '',
+ downloadLogs: '',
+ status: {
+ unknown: '',
+ waiting: '',
+ running: '',
+ success: '',
+ failure: '',
+ cancelled: '',
+ skipped: '',
+ blocked: '',
+ },
+ },
+ },
+ });
+ await flushPromises();
+
+ expect(wrapper.get('.job-artifacts .job-artifacts-title').text()).toEqual('artifactTitleHere');
+ expect(wrapper.get('.job-artifacts .job-artifacts-item:nth-of-type(1) .job-artifacts-link').attributes('href')).toEqual('https://example.com/example-org/example-repo/actions/runs/1001/artifacts/artifactname1');
+ expect(wrapper.get('.job-artifacts .job-artifacts-item:nth-of-type(2) .job-artifacts-link').attributes('href')).toEqual('https://example.com/example-org/example-repo/actions/runs/1001/artifacts/artifactname2');
+});
diff --git a/web_src/js/components/RepoActionView.vue b/web_src/js/components/RepoActionView.vue
index 4d34960e46..3f89bc9806 100644
--- a/web_src/js/components/RepoActionView.vue
+++ b/web_src/js/components/RepoActionView.vue
@@ -15,6 +15,7 @@ const sfc = {
},
props: {
runIndex: String,
+ runID: String,
jobIndex: String,
actionsURL: String,
workflowName: String,
@@ -386,6 +387,7 @@ export function initRepositoryActionView() {
const view = createApp(sfc, {
runIndex: el.getAttribute('data-run-index'),
+ runID: el.getAttribute('data-run-id'),
jobIndex: el.getAttribute('data-job-index'),
actionsURL: el.getAttribute('data-actions-url'),
workflowName: el.getAttribute('data-workflow-name'),
@@ -473,7 +475,7 @@ export function initRepositoryActionView() {