Skip to content

Commit 3859942

Browse files
Zettat123Copilot
andauthored
Replace index with id in actions routes (#36842)
This PR migrates the web Actions run/job routes from index-based `runIndex` or `jobIndex` to database IDs. **⚠️ BREAKING ⚠️**: Existing saved links/bookmarks that use the old index-based URLs will no longer resolve after this change. Improvements of this change: - Previously, `jobIndex` depended on list order, making it hard to locate a specific job. Using `jobID` provides stable addressing. - Web routes now align with API, which already use IDs. - Behavior is closer to GitHub, which exposes run/job IDs in URLs. - Provides a cleaner base for future features without relying on list order. - #36388 this PR improves the support for reusable workflows. If a job uses a reusable workflow, it may contain multiple child jobs, which makes relying on job index to locate a job much more complicated --------- Signed-off-by: Zettat123 <zettat123@gmail.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent 6e8f78a commit 3859942

33 files changed

Lines changed: 713 additions & 228 deletions

models/actions/run.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,14 +70,14 @@ func (run *ActionRun) HTMLURL() string {
7070
if run.Repo == nil {
7171
return ""
7272
}
73-
return fmt.Sprintf("%s/actions/runs/%d", run.Repo.HTMLURL(), run.Index)
73+
return fmt.Sprintf("%s/actions/runs/%d", run.Repo.HTMLURL(), run.ID)
7474
}
7575

7676
func (run *ActionRun) Link() string {
7777
if run.Repo == nil {
7878
return ""
7979
}
80-
return fmt.Sprintf("%s/actions/runs/%d", run.Repo.Link(), run.Index)
80+
return fmt.Sprintf("%s/actions/runs/%d", run.Repo.Link(), run.ID)
8181
}
8282

8383
func (run *ActionRun) WorkflowLink() string {
@@ -299,7 +299,7 @@ func CancelJobs(ctx context.Context, jobs []*ActionRunJob) ([]*ActionRunJob, err
299299
if err := StopTask(ctx, job.TaskID, StatusCancelled); err != nil {
300300
return cancelledJobs, err
301301
}
302-
updatedJob, err := GetRunJobByID(ctx, job.ID)
302+
updatedJob, err := GetRunJobByRunAndID(ctx, job.RunID, job.ID)
303303
if err != nil {
304304
return cancelledJobs, fmt.Errorf("get job: %w", err)
305305
}

models/actions/run_job.go

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -118,13 +118,25 @@ func (job *ActionRunJob) ParseJob() (*jobparser.Job, error) {
118118
return workflowJob, nil
119119
}
120120

121-
func GetRunJobByID(ctx context.Context, id int64) (*ActionRunJob, error) {
121+
func GetRunJobByRepoAndID(ctx context.Context, repoID, jobID int64) (*ActionRunJob, error) {
122122
var job ActionRunJob
123-
has, err := db.GetEngine(ctx).Where("id=?", id).Get(&job)
123+
has, err := db.GetEngine(ctx).Where("id=? AND repo_id=?", jobID, repoID).Get(&job)
124124
if err != nil {
125125
return nil, err
126126
} else if !has {
127-
return nil, fmt.Errorf("run job with id %d: %w", id, util.ErrNotExist)
127+
return nil, fmt.Errorf("run job with id %d: %w", jobID, util.ErrNotExist)
128+
}
129+
130+
return &job, nil
131+
}
132+
133+
func GetRunJobByRunAndID(ctx context.Context, runID, jobID int64) (*ActionRunJob, error) {
134+
var job ActionRunJob
135+
has, err := db.GetEngine(ctx).Where("id=? AND run_id=?", jobID, runID).Get(&job)
136+
if err != nil {
137+
return nil, err
138+
} else if !has {
139+
return nil, fmt.Errorf("run job with id %d: %w", jobID, util.ErrNotExist)
128140
}
129141

130142
return &job, nil
@@ -168,7 +180,7 @@ func UpdateRunJob(ctx context.Context, job *ActionRunJob, cond builder.Cond, col
168180

169181
if job.RunID == 0 {
170182
var err error
171-
if job, err = GetRunJobByID(ctx, job.ID); err != nil {
183+
if job, err = GetRunJobByRepoAndID(ctx, job.RepoID, job.ID); err != nil {
172184
return 0, err
173185
}
174186
}

models/actions/task.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ func (task *ActionTask) GetRepoLink() string {
114114

115115
func (task *ActionTask) LoadJob(ctx context.Context) error {
116116
if task.Job == nil {
117-
job, err := GetRunJobByID(ctx, task.JobID)
117+
job, err := GetRunJobByRepoAndID(ctx, task.RepoID, task.JobID)
118118
if err != nil {
119119
return err
120120
}
@@ -388,6 +388,7 @@ func UpdateTaskByState(ctx context.Context, runnerID int64, state *runnerv1.Task
388388
}
389389
if _, err := UpdateRunJob(ctx, &ActionRunJob{
390390
ID: task.JobID,
391+
RepoID: task.RepoID,
391392
Status: task.Status,
392393
Stopped: task.Stopped,
393394
}, nil); err != nil {
@@ -449,6 +450,7 @@ func StopTask(ctx context.Context, taskID int64, status Status) error {
449450
task.Stopped = now
450451
if _, err := UpdateRunJob(ctx, &ActionRunJob{
451452
ID: task.JobID,
453+
RepoID: task.RepoID,
452454
Status: task.Status,
453455
Stopped: task.Stopped,
454456
}, nil); err != nil {

models/git/commit_status_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,7 @@ func TestCommitStatusesHideActionsURL(t *testing.T) {
243243
statuses := []*git_model.CommitStatus{
244244
{
245245
RepoID: repo.ID,
246-
TargetURL: fmt.Sprintf("%s/jobs/%d", run.Link(), run.Index),
246+
TargetURL: fmt.Sprintf("%s/jobs/%d", run.Link(), run.ID),
247247
},
248248
{
249249
RepoID: repo.ID,
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
# type ActionRun struct {
2+
# ID int64 `xorm:"pk autoincr"`
3+
# RepoID int64 `xorm:"index"`
4+
# Index int64
5+
# }
6+
-
7+
id: 106
8+
repo_id: 1
9+
index: 7
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
# type ActionRunJob struct {
2+
# ID int64 `xorm:"pk autoincr"`
3+
# RunID int64 `xorm:"index"`
4+
# }
5+
-
6+
id: 530
7+
run_id: 106
8+
-
9+
id: 531
10+
run_id: 106
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
# type CommitStatus struct {
2+
# ID int64 `xorm:"pk autoincr"`
3+
# RepoID int64 `xorm:"index"`
4+
# TargetURL string
5+
# }
6+
-
7+
id: 10
8+
repo_id: 1
9+
target_url: /testuser/repo1/actions/runs/7/jobs/0
10+
-
11+
id: 11
12+
repo_id: 1
13+
target_url: /testuser/repo1/actions/runs/7/jobs/1
14+
-
15+
id: 12
16+
repo_id: 1
17+
target_url: /otheruser/badrepo/actions/runs/7/jobs/0
18+
-
19+
id: 13
20+
repo_id: 1
21+
target_url: /testuser/repo1/actions/runs/10/jobs/0
22+
-
23+
id: 14
24+
repo_id: 1
25+
target_url: /testuser/repo1/actions/runs/7/jobs/3
26+
-
27+
id: 15
28+
repo_id: 1
29+
target_url: https://ci.example.com/build/123
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
# type CommitStatusSummary struct {
2+
# ID int64 `xorm:"pk autoincr"`
3+
# RepoID int64 `xorm:"index"`
4+
# SHA string `xorm:"VARCHAR(64) NOT NULL"`
5+
# State string `xorm:"VARCHAR(7) NOT NULL"`
6+
# TargetURL string
7+
# }
8+
-
9+
id: 20
10+
repo_id: 1
11+
sha: "012345"
12+
state: success
13+
target_url: /testuser/repo1/actions/runs/7/jobs/0
14+
-
15+
id: 21
16+
repo_id: 1
17+
sha: "678901"
18+
state: success
19+
target_url: https://ci.example.com/build/123
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
# type Repository struct {
2+
# ID int64 `xorm:"pk autoincr"`
3+
# OwnerName string
4+
# Name string
5+
# }
6+
-
7+
id: 1
8+
owner_name: testuser
9+
name: repo1

models/migrations/migrations.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -400,6 +400,7 @@ func prepareMigrationTasks() []*migration {
400400
newMigration(323, "Add support for actions concurrency", v1_26.AddActionsConcurrency),
401401
newMigration(324, "Fix closed milestone completeness for milestones with no issues", v1_26.FixClosedMilestoneCompleteness),
402402
newMigration(325, "Fix missed repo_id when migrate attachments", v1_26.FixMissedRepoIDWhenMigrateAttachments),
403+
newMigration(326, "Migrate commit status target URL to use run ID and job ID", v1_26.FixCommitStatusTargetURLToUseRunAndJobID),
403404
}
404405
return preparedMigrations
405406
}

0 commit comments

Comments
 (0)