From 054eb6d8a5aab6233eb7de23b4485897cf2084ff Mon Sep 17 00:00:00 2001 From: Nicolas Date: Mon, 2 Mar 2026 22:34:06 +0100 Subject: [PATCH 1/8] feat: Add Actions API rerun endpoints for runs and jobs (#36768) This PR adds official REST API endpoints to rerun Gitea Actions workflow runs and individual jobs: * POST /api/v1/repos/{owner}/{repo}/actions/runs/{run}/rerun * POST /api/v1/repos/{owner}/{repo}/actions/runs/{run}/jobs/{job_id}/rerun It reuses the existing rerun behavior from the web UI and exposes it through stable API routes. --------- Signed-off-by: wxiaoguang Co-authored-by: wxiaoguang Co-authored-by: Giteabot --- models/repo/repo.go | 49 ++--- models/repo/repo_unit.go | 2 + routers/api/v1/api.go | 2 + routers/api/v1/repo/action.go | 186 +++++++++++++++--- routers/web/repo/actions/view.go | 138 ++----------- services/actions/rerun.go | 141 +++++++++++++ templates/swagger/v1_json.tmpl | 111 +++++++++++ tests/integration/actions_concurrency_test.go | 11 +- tests/integration/api_actions_run_test.go | 123 ++++++++++++ 9 files changed, 580 insertions(+), 183 deletions(-) diff --git a/models/repo/repo.go b/models/repo/repo.go index 7b7f5adb41..25207cc28b 100644 --- a/models/repo/repo.go +++ b/models/repo/repo.go @@ -422,52 +422,37 @@ func (repo *Repository) UnitEnabled(ctx context.Context, tp unit.Type) bool { return false } -// MustGetUnit always returns a RepoUnit object +// MustGetUnit always returns a RepoUnit object even if the unit doesn't exist (not enabled) func (repo *Repository) MustGetUnit(ctx context.Context, tp unit.Type) *RepoUnit { ru, err := repo.GetUnit(ctx, tp) if err == nil { return ru } - + if !errors.Is(err, util.ErrNotExist) { + setting.PanicInDevOrTesting("Failed to get unit %v for repository %d: %v", tp, repo.ID, err) + } + ru = &RepoUnit{RepoID: repo.ID, Type: tp} switch tp { case unit.TypeExternalWiki: - return &RepoUnit{ - Type: tp, - Config: new(ExternalWikiConfig), - } + ru.Config = new(ExternalWikiConfig) case unit.TypeExternalTracker: - return &RepoUnit{ - Type: tp, - Config: new(ExternalTrackerConfig), - } + ru.Config = new(ExternalTrackerConfig) case unit.TypePullRequests: - return &RepoUnit{ - Type: tp, - Config: new(PullRequestsConfig), - } + ru.Config = new(PullRequestsConfig) case unit.TypeIssues: - return &RepoUnit{ - Type: tp, - Config: new(IssuesConfig), - } + ru.Config = new(IssuesConfig) case unit.TypeActions: - return &RepoUnit{ - Type: tp, - Config: new(ActionsConfig), - } + ru.Config = new(ActionsConfig) case unit.TypeProjects: - cfg := new(ProjectsConfig) - cfg.ProjectsMode = ProjectsModeNone - return &RepoUnit{ - Type: tp, - Config: cfg, + ru.Config = new(ProjectsConfig) + default: // other units don't have config + } + if ru.Config != nil { + if err = ru.Config.FromDB(nil); err != nil { + setting.PanicInDevOrTesting("Failed to load default config for unit %v of repository %d: %v", tp, repo.ID, err) } } - - return &RepoUnit{ - Type: tp, - Config: new(UnitConfig), - } + return ru } // GetUnit returns a RepoUnit object diff --git a/models/repo/repo_unit.go b/models/repo/repo_unit.go index d03d5e1e6a..1058a18a85 100644 --- a/models/repo/repo_unit.go +++ b/models/repo/repo_unit.go @@ -241,6 +241,8 @@ type ProjectsConfig struct { // FromDB fills up a ProjectsConfig from serialized format. func (cfg *ProjectsConfig) FromDB(bs []byte) error { + // TODO: remove GetProjectsMode, only use ProjectsMode + cfg.ProjectsMode = ProjectsModeAll return json.UnmarshalHandleDoubleEncode(bs, &cfg) } diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 50626cebbf..cb6bbe0954 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -1259,7 +1259,9 @@ func Routes() *web.Router { m.Group("/{run}", func() { m.Get("", repo.GetWorkflowRun) m.Delete("", reqToken(), reqRepoWriter(unit.TypeActions), repo.DeleteActionRun) + m.Post("/rerun", reqToken(), reqRepoWriter(unit.TypeActions), repo.RerunWorkflowRun) m.Get("/jobs", repo.ListWorkflowRunJobs) + m.Post("/jobs/{job_id}/rerun", reqToken(), reqRepoWriter(unit.TypeActions), repo.RerunWorkflowJob) m.Get("/artifacts", repo.GetArtifactsOfRun) }) }) diff --git a/routers/api/v1/repo/action.go b/routers/api/v1/repo/action.go index 4c3a0dceff..13da5aa815 100644 --- a/routers/api/v1/repo/action.go +++ b/routers/api/v1/repo/action.go @@ -12,6 +12,7 @@ import ( "fmt" "net/http" "net/url" + "slices" "strconv" "strings" "time" @@ -1103,6 +1104,33 @@ func ActionsEnableWorkflow(ctx *context.APIContext) { ctx.Status(http.StatusNoContent) } +func getCurrentRepoActionRunByID(ctx *context.APIContext) *actions_model.ActionRun { + runID := ctx.PathParamInt64("run") + run, err := actions_model.GetRunByRepoAndID(ctx, ctx.Repo.Repository.ID, runID) + if errors.Is(err, util.ErrNotExist) { + ctx.APIErrorNotFound(err) + return nil + } else if err != nil { + ctx.APIErrorInternal(err) + return nil + } + return run +} + +func getCurrentRepoActionRunJobsByID(ctx *context.APIContext) (*actions_model.ActionRun, actions_model.ActionJobList) { + run := getCurrentRepoActionRunByID(ctx) + if ctx.Written() { + return nil, nil + } + + jobs, err := actions_model.GetRunJobsByRunID(ctx, run.ID) + if err != nil { + ctx.APIErrorInternal(err) + return nil, nil + } + return run, jobs +} + // GetWorkflowRun Gets a specific workflow run. func GetWorkflowRun(ctx *context.APIContext) { // swagger:operation GET /repos/{owner}/{repo}/actions/runs/{run} repository GetWorkflowRun @@ -1134,19 +1162,12 @@ func GetWorkflowRun(ctx *context.APIContext) { // "404": // "$ref": "#/responses/notFound" - runID := ctx.PathParamInt64("run") - job, has, err := db.GetByID[actions_model.ActionRun](ctx, runID) - if err != nil { - ctx.APIErrorInternal(err) + run := getCurrentRepoActionRunByID(ctx) + if ctx.Written() { return } - if !has || job.RepoID != ctx.Repo.Repository.ID { - ctx.APIErrorNotFound(util.ErrNotExist) - return - } - - convertedRun, err := convert.ToActionWorkflowRun(ctx, ctx.Repo.Repository, job) + convertedRun, err := convert.ToActionWorkflowRun(ctx, ctx.Repo.Repository, run) if err != nil { ctx.APIErrorInternal(err) return @@ -1154,6 +1175,133 @@ func GetWorkflowRun(ctx *context.APIContext) { ctx.JSON(http.StatusOK, convertedRun) } +// RerunWorkflowRun Reruns an entire workflow run. +func RerunWorkflowRun(ctx *context.APIContext) { + // swagger:operation POST /repos/{owner}/{repo}/actions/runs/{run}/rerun repository rerunWorkflowRun + // --- + // summary: Reruns an entire workflow run + // produces: + // - application/json + // parameters: + // - name: owner + // in: path + // description: owner of the repo + // type: string + // required: true + // - name: repo + // in: path + // description: name of the repository + // type: string + // required: true + // - name: run + // in: path + // description: id of the run + // type: integer + // required: true + // responses: + // "201": + // "$ref": "#/responses/WorkflowRun" + // "400": + // "$ref": "#/responses/error" + // "403": + // "$ref": "#/responses/forbidden" + // "404": + // "$ref": "#/responses/notFound" + // "422": + // "$ref": "#/responses/validationError" + + run, jobs := getCurrentRepoActionRunJobsByID(ctx) + if ctx.Written() { + return + } + + if err := actions_service.RerunWorkflowRunJobs(ctx, ctx.Repo.Repository, run, jobs, nil); err != nil { + handleWorkflowRerunError(ctx, err) + return + } + + convertedRun, err := convert.ToActionWorkflowRun(ctx, ctx.Repo.Repository, run) + if err != nil { + ctx.APIErrorInternal(err) + return + } + ctx.JSON(http.StatusCreated, convertedRun) +} + +// RerunWorkflowJob Reruns a specific workflow job in a run. +func RerunWorkflowJob(ctx *context.APIContext) { + // swagger:operation POST /repos/{owner}/{repo}/actions/runs/{run}/jobs/{job_id}/rerun repository rerunWorkflowJob + // --- + // summary: Reruns a specific workflow job in a run + // produces: + // - application/json + // parameters: + // - name: owner + // in: path + // description: owner of the repo + // type: string + // required: true + // - name: repo + // in: path + // description: name of the repository + // type: string + // required: true + // - name: run + // in: path + // description: id of the run + // type: integer + // required: true + // - name: job_id + // in: path + // description: id of the job + // type: integer + // required: true + // responses: + // "201": + // "$ref": "#/responses/WorkflowJob" + // "400": + // "$ref": "#/responses/error" + // "403": + // "$ref": "#/responses/forbidden" + // "404": + // "$ref": "#/responses/notFound" + // "422": + // "$ref": "#/responses/validationError" + + run, jobs := getCurrentRepoActionRunJobsByID(ctx) + if ctx.Written() { + return + } + + jobID := ctx.PathParamInt64("job_id") + jobIdx := slices.IndexFunc(jobs, func(job *actions_model.ActionRunJob) bool { return job.ID == jobID }) + if jobIdx == -1 { + ctx.APIErrorNotFound(util.NewNotExistErrorf("workflow job with id %d", jobID)) + return + } + + targetJob := jobs[jobIdx] + if err := actions_service.RerunWorkflowRunJobs(ctx, ctx.Repo.Repository, run, jobs, targetJob); err != nil { + handleWorkflowRerunError(ctx, err) + return + } + + convertedJob, err := convert.ToActionWorkflowJob(ctx, ctx.Repo.Repository, nil, targetJob) + if err != nil { + ctx.APIErrorInternal(err) + return + } + ctx.JSON(http.StatusCreated, convertedJob) +} + +func handleWorkflowRerunError(ctx *context.APIContext, err error) { + if errors.Is(err, util.ErrInvalidArgument) { + ctx.APIError(http.StatusBadRequest, err) + return + } + ctx.APIErrorInternal(err) +} + // ListWorkflowRunJobs Lists all jobs for a workflow run. func ListWorkflowRunJobs(ctx *context.APIContext) { // swagger:operation GET /repos/{owner}/{repo}/actions/runs/{run}/jobs repository listWorkflowRunJobs @@ -1198,9 +1346,7 @@ func ListWorkflowRunJobs(ctx *context.APIContext) { // "404": // "$ref": "#/responses/notFound" - repoID := ctx.Repo.Repository.ID - - runID := ctx.PathParamInt64("run") + repoID, runID := ctx.Repo.Repository.ID, ctx.PathParamInt64("run") // Avoid the list all jobs functionality for this api route to be used with a runID == 0. if runID <= 0 { @@ -1300,10 +1446,8 @@ func GetArtifactsOfRun(ctx *context.APIContext) { // "404": // "$ref": "#/responses/notFound" - repoID := ctx.Repo.Repository.ID artifactName := ctx.Req.URL.Query().Get("name") - - runID := ctx.PathParamInt64("run") + repoID, runID := ctx.Repo.Repository.ID, ctx.PathParamInt64("run") artifacts, total, err := db.FindAndCount[actions_model.ActionArtifact](ctx, actions_model.FindArtifactsOptions{ RepoID: repoID, @@ -1364,15 +1508,11 @@ func DeleteActionRun(ctx *context.APIContext) { // "404": // "$ref": "#/responses/notFound" - runID := ctx.PathParamInt64("run") - run, err := actions_model.GetRunByRepoAndID(ctx, ctx.Repo.Repository.ID, runID) - if errors.Is(err, util.ErrNotExist) { - ctx.APIErrorNotFound(err) - return - } else if err != nil { - ctx.APIErrorInternal(err) + run := getCurrentRepoActionRunByID(ctx) + if ctx.Written() { return } + if !run.Status.IsDone() { ctx.APIError(http.StatusBadRequest, "this workflow run is not done") return diff --git a/routers/web/repo/actions/view.go b/routers/web/repo/actions/view.go index 4c023d9252..0eaa6cab41 100644 --- a/routers/web/repo/actions/view.go +++ b/routers/web/repo/actions/view.go @@ -36,8 +36,6 @@ import ( notify_service "code.gitea.io/gitea/services/notify" "github.com/nektos/act/pkg/model" - "go.yaml.in/yaml/v4" - "xorm.io/builder" ) func getRunIndex(ctx *context_module.Context) int64 { @@ -53,7 +51,7 @@ func getRunIndex(ctx *context_module.Context) int64 { func View(ctx *context_module.Context) { ctx.Data["PageIsActions"] = true runIndex := getRunIndex(ctx) - jobIndex := ctx.PathParamInt64("job") + jobIndex := ctx.PathParamInt("job") ctx.Data["RunIndex"] = runIndex ctx.Data["JobIndex"] = jobIndex ctx.Data["ActionsURL"] = ctx.Repo.RepoLink + "/actions" @@ -211,7 +209,7 @@ func getActionsViewArtifacts(ctx context.Context, repoID, runIndex int64) (artif func ViewPost(ctx *context_module.Context) { req := web.GetForm(ctx).(*ViewRequest) runIndex := getRunIndex(ctx) - jobIndex := ctx.PathParamInt64("job") + jobIndex := ctx.PathParamInt("job") current, jobs := getRunJobs(ctx, runIndex, jobIndex) if ctx.Written() { @@ -405,11 +403,8 @@ func convertToViewModel(ctx context.Context, locale translation.Locale, cursors // If jobIndexStr is a blank string, it means rerun all jobs func Rerun(ctx *context_module.Context) { runIndex := getRunIndex(ctx) - jobIndexStr := ctx.PathParam("job") - var jobIndex int64 - if jobIndexStr != "" { - jobIndex, _ = strconv.ParseInt(jobIndexStr, 10, 64) - } + jobIndexHas := ctx.PathParam("job") != "" + jobIndex := ctx.PathParamInt("job") run, err := actions_model.GetRunByIndex(ctx, ctx.Repo.Repository.ID, runIndex) if err != nil { @@ -431,130 +426,29 @@ func Rerun(ctx *context_module.Context) { return } - // reset run's start and stop time - run.PreviousDuration = run.Duration() - run.Started = 0 - run.Stopped = 0 - run.Status = actions_model.StatusWaiting - - vars, err := actions_model.GetVariablesOfRun(ctx, run) + jobs, err := actions_model.GetRunJobsByRunID(ctx, run.ID) if err != nil { - ctx.ServerError("GetVariablesOfRun", fmt.Errorf("get run %d variables: %w", run.ID, err)) + ctx.ServerError("GetRunJobsByRunID", err) return } - if run.RawConcurrency != "" { - var rawConcurrency model.RawConcurrency - if err := yaml.Unmarshal([]byte(run.RawConcurrency), &rawConcurrency); err != nil { - ctx.ServerError("UnmarshalRawConcurrency", fmt.Errorf("unmarshal raw concurrency: %w", err)) - return - } - - err = actions_service.EvaluateRunConcurrencyFillModel(ctx, run, &rawConcurrency, vars, nil) - if err != nil { - ctx.ServerError("EvaluateRunConcurrencyFillModel", err) - return - } - - run.Status, err = actions_service.PrepareToStartRunWithConcurrency(ctx, run) - if err != nil { - ctx.ServerError("PrepareToStartRunWithConcurrency", err) + var targetJob *actions_model.ActionRunJob // nil means rerun all jobs + if jobIndexHas { + if jobIndex < 0 || jobIndex >= len(jobs) { + ctx.JSONError(ctx.Locale.Tr("error.not_found")) return } + targetJob = jobs[jobIndex] // only rerun the selected job } - if err := actions_model.UpdateRun(ctx, run, "started", "stopped", "previous_duration", "status", "concurrency_group", "concurrency_cancel"); err != nil { - ctx.ServerError("UpdateRun", err) + + if err := actions_service.RerunWorkflowRunJobs(ctx, ctx.Repo.Repository, run, jobs, targetJob); err != nil { + ctx.ServerError("RerunWorkflowRunJobs", err) return } - if err := run.LoadAttributes(ctx); err != nil { - ctx.ServerError("run.LoadAttributes", err) - return - } - notify_service.WorkflowRunStatusUpdate(ctx, run.Repo, run.TriggerUser, run) - - job, jobs := getRunJobs(ctx, runIndex, jobIndex) - if ctx.Written() { - return - } - - isRunBlocked := run.Status == actions_model.StatusBlocked - if jobIndexStr == "" { // rerun all jobs - for _, j := range jobs { - // if the job has needs, it should be set to "blocked" status to wait for other jobs - shouldBlockJob := len(j.Needs) > 0 || isRunBlocked - if err := rerunJob(ctx, j, shouldBlockJob); err != nil { - ctx.ServerError("RerunJob", err) - return - } - } - ctx.JSONOK() - return - } - - rerunJobs := actions_service.GetAllRerunJobs(job, jobs) - - for _, j := range rerunJobs { - // jobs other than the specified one should be set to "blocked" status - shouldBlockJob := j.JobID != job.JobID || isRunBlocked - if err := rerunJob(ctx, j, shouldBlockJob); err != nil { - ctx.ServerError("RerunJob", err) - return - } - } - ctx.JSONOK() } -func rerunJob(ctx *context_module.Context, job *actions_model.ActionRunJob, shouldBlock bool) error { - status := job.Status - if !status.IsDone() { - return nil - } - - job.TaskID = 0 - job.Status = util.Iif(shouldBlock, actions_model.StatusBlocked, actions_model.StatusWaiting) - job.Started = 0 - job.Stopped = 0 - - job.ConcurrencyGroup = "" - job.ConcurrencyCancel = false - job.IsConcurrencyEvaluated = false - if err := job.LoadRun(ctx); err != nil { - return err - } - - vars, err := actions_model.GetVariablesOfRun(ctx, job.Run) - if err != nil { - return fmt.Errorf("get run %d variables: %w", job.Run.ID, err) - } - - if job.RawConcurrency != "" && !shouldBlock { - err = actions_service.EvaluateJobConcurrencyFillModel(ctx, job.Run, job, vars, nil) - if err != nil { - return fmt.Errorf("evaluate job concurrency: %w", err) - } - - job.Status, err = actions_service.PrepareToStartJobWithConcurrency(ctx, job) - if err != nil { - return err - } - } - - if err := db.WithTx(ctx, func(ctx context.Context) error { - updateCols := []string{"task_id", "status", "started", "stopped", "concurrency_group", "concurrency_cancel", "is_concurrency_evaluated"} - _, err := actions_model.UpdateRunJob(ctx, job, builder.Eq{"status": status}, updateCols...) - return err - }); err != nil { - return err - } - - actions_service.CreateCommitStatusForRunJobs(ctx, job.Run, job) - notify_service.WorkflowJobStatusUpdate(ctx, job.Run.Repo, job.Run.TriggerUser, job, nil) - - return nil -} - func Logs(ctx *context_module.Context) { runIndex := getRunIndex(ctx) jobIndex := ctx.PathParamInt64("job") @@ -715,7 +609,7 @@ func Delete(ctx *context_module.Context) { // getRunJobs gets the jobs of runIndex, and returns jobs[jobIndex], jobs. // Any error will be written to the ctx. // It never returns a nil job of an empty jobs, if the jobIndex is out of range, it will be treated as 0. -func getRunJobs(ctx *context_module.Context, runIndex, jobIndex int64) (*actions_model.ActionRunJob, []*actions_model.ActionRunJob) { +func getRunJobs(ctx *context_module.Context, runIndex int64, jobIndex int) (*actions_model.ActionRunJob, []*actions_model.ActionRunJob) { run, err := actions_model.GetRunByIndex(ctx, ctx.Repo.Repository.ID, runIndex) if err != nil { if errors.Is(err, util.ErrNotExist) { @@ -740,7 +634,7 @@ func getRunJobs(ctx *context_module.Context, runIndex, jobIndex int64) (*actions v.Run = run } - if jobIndex >= 0 && jobIndex < int64(len(jobs)) { + if jobIndex >= 0 && jobIndex < len(jobs) { return jobs[jobIndex], jobs } return jobs[0], jobs diff --git a/services/actions/rerun.go b/services/actions/rerun.go index 60f6650905..277da39b82 100644 --- a/services/actions/rerun.go +++ b/services/actions/rerun.go @@ -4,8 +4,20 @@ package actions import ( + "context" + "fmt" + actions_model "code.gitea.io/gitea/models/actions" + "code.gitea.io/gitea/models/db" + repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/models/unit" "code.gitea.io/gitea/modules/container" + "code.gitea.io/gitea/modules/util" + notify_service "code.gitea.io/gitea/services/notify" + + "github.com/nektos/act/pkg/model" + "go.yaml.in/yaml/v4" + "xorm.io/builder" ) // GetAllRerunJobs get all jobs that need to be rerun when job should be rerun @@ -36,3 +48,132 @@ func GetAllRerunJobs(job *actions_model.ActionRunJob, allJobs []*actions_model.A return rerunJobs } + +// RerunWorkflowRunJobs reruns all done jobs of a workflow run, +// or reruns a selected job and all of its downstream jobs when targetJob is specified. +func RerunWorkflowRunJobs(ctx context.Context, repo *repo_model.Repository, run *actions_model.ActionRun, jobs []*actions_model.ActionRunJob, targetJob *actions_model.ActionRunJob) error { + // Rerun is not allowed if the run is not done. + if !run.Status.IsDone() { + return util.NewInvalidArgumentErrorf("this workflow run is not done") + } + + cfgUnit := repo.MustGetUnit(ctx, unit.TypeActions) + + // Rerun is not allowed when workflow is disabled. + cfg := cfgUnit.ActionsConfig() + if cfg.IsWorkflowDisabled(run.WorkflowID) { + return util.NewInvalidArgumentErrorf("workflow %s is disabled", run.WorkflowID) + } + + // Reset run's timestamps and status. + run.PreviousDuration = run.Duration() + run.Started = 0 + run.Stopped = 0 + run.Status = actions_model.StatusWaiting + + vars, err := actions_model.GetVariablesOfRun(ctx, run) + if err != nil { + return fmt.Errorf("get run %d variables: %w", run.ID, err) + } + + if run.RawConcurrency != "" { + var rawConcurrency model.RawConcurrency + if err := yaml.Unmarshal([]byte(run.RawConcurrency), &rawConcurrency); err != nil { + return fmt.Errorf("unmarshal raw concurrency: %w", err) + } + + if err := EvaluateRunConcurrencyFillModel(ctx, run, &rawConcurrency, vars, nil); err != nil { + return err + } + + run.Status, err = PrepareToStartRunWithConcurrency(ctx, run) + if err != nil { + return err + } + } + + if err := actions_model.UpdateRun(ctx, run, "started", "stopped", "previous_duration", "status", "concurrency_group", "concurrency_cancel"); err != nil { + return err + } + + if err := run.LoadAttributes(ctx); err != nil { + return err + } + + for _, job := range jobs { + job.Run = run + } + + notify_service.WorkflowRunStatusUpdate(ctx, run.Repo, run.TriggerUser, run) + + isRunBlocked := run.Status == actions_model.StatusBlocked + + if targetJob == nil { + for _, job := range jobs { + // If the job has needs, it should be blocked to wait for its dependencies. + shouldBlockJob := len(job.Needs) > 0 || isRunBlocked + if err := rerunWorkflowJob(ctx, job, shouldBlockJob); err != nil { + return err + } + } + return nil + } + + rerunJobs := GetAllRerunJobs(targetJob, jobs) + for _, job := range rerunJobs { + // Jobs other than the selected one should wait for dependencies. + shouldBlockJob := job.JobID != targetJob.JobID || isRunBlocked + if err := rerunWorkflowJob(ctx, job, shouldBlockJob); err != nil { + return err + } + } + + return nil +} + +func rerunWorkflowJob(ctx context.Context, job *actions_model.ActionRunJob, shouldBlock bool) error { + status := job.Status + if !status.IsDone() { + return nil + } + + job.TaskID = 0 + job.Status = util.Iif(shouldBlock, actions_model.StatusBlocked, actions_model.StatusWaiting) + job.Started = 0 + job.Stopped = 0 + job.ConcurrencyGroup = "" + job.ConcurrencyCancel = false + job.IsConcurrencyEvaluated = false + + if err := job.LoadRun(ctx); err != nil { + return err + } + + vars, err := actions_model.GetVariablesOfRun(ctx, job.Run) + if err != nil { + return fmt.Errorf("get run %d variables: %w", job.Run.ID, err) + } + + if job.RawConcurrency != "" && !shouldBlock { + if err := EvaluateJobConcurrencyFillModel(ctx, job.Run, job, vars, nil); err != nil { + return fmt.Errorf("evaluate job concurrency: %w", err) + } + + job.Status, err = PrepareToStartJobWithConcurrency(ctx, job) + if err != nil { + return err + } + } + + if err := db.WithTx(ctx, func(ctx context.Context) error { + updateCols := []string{"task_id", "status", "started", "stopped", "concurrency_group", "concurrency_cancel", "is_concurrency_evaluated"} + _, err := actions_model.UpdateRunJob(ctx, job, builder.Eq{"status": status}, updateCols...) + return err + }); err != nil { + return err + } + + CreateCommitStatusForRunJobs(ctx, job.Run, job) + notify_service.WorkflowJobStatusUpdate(ctx, job.Run.Repo, job.Run.TriggerUser, job, nil) + return nil +} diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 644c9b3f83..4fc823d090 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -5297,6 +5297,117 @@ } } }, + "/repos/{owner}/{repo}/actions/runs/{run}/jobs/{job_id}/rerun": { + "post": { + "produces": [ + "application/json" + ], + "tags": [ + "repository" + ], + "summary": "Reruns a specific workflow job in a run", + "operationId": "rerunWorkflowJob", + "parameters": [ + { + "type": "string", + "description": "owner of the repo", + "name": "owner", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "name of the repository", + "name": "repo", + "in": "path", + "required": true + }, + { + "type": "integer", + "description": "id of the run", + "name": "run", + "in": "path", + "required": true + }, + { + "type": "integer", + "description": "id of the job", + "name": "job_id", + "in": "path", + "required": true + } + ], + "responses": { + "201": { + "$ref": "#/responses/WorkflowJob" + }, + "400": { + "$ref": "#/responses/error" + }, + "403": { + "$ref": "#/responses/forbidden" + }, + "404": { + "$ref": "#/responses/notFound" + }, + "422": { + "$ref": "#/responses/validationError" + } + } + } + }, + "/repos/{owner}/{repo}/actions/runs/{run}/rerun": { + "post": { + "produces": [ + "application/json" + ], + "tags": [ + "repository" + ], + "summary": "Reruns an entire workflow run", + "operationId": "rerunWorkflowRun", + "parameters": [ + { + "type": "string", + "description": "owner of the repo", + "name": "owner", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "name of the repository", + "name": "repo", + "in": "path", + "required": true + }, + { + "type": "integer", + "description": "id of the run", + "name": "run", + "in": "path", + "required": true + } + ], + "responses": { + "201": { + "$ref": "#/responses/WorkflowRun" + }, + "400": { + "$ref": "#/responses/error" + }, + "403": { + "$ref": "#/responses/forbidden" + }, + "404": { + "$ref": "#/responses/notFound" + }, + "422": { + "$ref": "#/responses/validationError" + } + } + } + }, "/repos/{owner}/{repo}/actions/secrets": { "get": { "produces": [ diff --git a/tests/integration/actions_concurrency_test.go b/tests/integration/actions_concurrency_test.go index b904230a95..f1baa68b71 100644 --- a/tests/integration/actions_concurrency_test.go +++ b/tests/integration/actions_concurrency_test.go @@ -643,7 +643,7 @@ jobs: assert.Equal(t, "job-main-v1.24.0", wf2Job2Rerun1Job.ConcurrencyGroup) // rerun wf2-job2 - req = NewRequest(t, "POST", fmt.Sprintf("/%s/%s/actions/runs/%d/jobs/%d/rerun", user2.Name, repo.Name, wf2Run.Index, 1)) + req = NewRequest(t, "POST", fmt.Sprintf("/%s/%s/actions/runs/%d/jobs/1/rerun", user2.Name, repo.Name, wf2Run.Index)) _ = session.MakeRequest(t, req, http.StatusOK) // (rerun2) fetch and exec wf2-job2 wf2Job2Rerun2Task := runner1.fetchTask(t) @@ -1064,11 +1064,10 @@ jobs: }) // rerun cancel true scenario - - req = NewRequest(t, "POST", fmt.Sprintf("/%s/%s/actions/runs/%d/jobs/%d/rerun", user2.Name, apiRepo.Name, run2.Index, 1)) + req = NewRequest(t, "POST", fmt.Sprintf("/%s/%s/actions/runs/%d/jobs/0/rerun", user2.Name, apiRepo.Name, run2.Index)) _ = session.MakeRequest(t, req, http.StatusOK) - req = NewRequest(t, "POST", fmt.Sprintf("/%s/%s/actions/runs/%d/jobs/%d/rerun", user2.Name, apiRepo.Name, run4.Index, 1)) + req = NewRequest(t, "POST", fmt.Sprintf("/%s/%s/actions/runs/%d/jobs/0/rerun", user2.Name, apiRepo.Name, run4.Index)) _ = session.MakeRequest(t, req, http.StatusOK) task5 := runner.fetchTask(t) @@ -1084,13 +1083,13 @@ jobs: // rerun cancel false scenario - req = NewRequest(t, "POST", fmt.Sprintf("/%s/%s/actions/runs/%d/jobs/%d/rerun", user2.Name, apiRepo.Name, run2.Index, 1)) + req = NewRequest(t, "POST", fmt.Sprintf("/%s/%s/actions/runs/%d/jobs/0/rerun", user2.Name, apiRepo.Name, run2.Index)) _ = session.MakeRequest(t, req, http.StatusOK) run2_2 := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{ID: run2.ID}) assert.Equal(t, actions_model.StatusWaiting, run2_2.Status) - req = NewRequest(t, "POST", fmt.Sprintf("/%s/%s/actions/runs/%d/jobs/%d/rerun", user2.Name, apiRepo.Name, run2.Index+1, 1)) + req = NewRequest(t, "POST", fmt.Sprintf("/%s/%s/actions/runs/%d/jobs/0/rerun", user2.Name, apiRepo.Name, run2.Index+1)) _ = session.MakeRequest(t, req, http.StatusOK) task6 := runner.fetchTask(t) diff --git a/tests/integration/api_actions_run_test.go b/tests/integration/api_actions_run_test.go index 4838409560..205f3f02ff 100644 --- a/tests/integration/api_actions_run_test.go +++ b/tests/integration/api_actions_run_test.go @@ -169,3 +169,126 @@ func testAPIActionsDeleteRunListTasks(t *testing.T, repo *repo_model.Repository, assert.Equal(t, expected, findTask1) assert.Equal(t, expected, findTask2) } + +func TestAPIActionsRerunWorkflowRun(t *testing.T) { + defer prepareTestEnvActionsArtifacts(t)() + + t.Run("NotDone", func(t *testing.T) { + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 4}) + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID}) + session := loginUser(t, user.Name) + writeToken := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) + + req := NewRequest(t, "POST", fmt.Sprintf("/api/v1/repos/%s/actions/runs/793/rerun", repo.FullName())). + AddTokenAuth(writeToken) + MakeRequest(t, req, http.StatusBadRequest) + }) + + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2}) + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID}) + session := loginUser(t, user.Name) + + writeToken := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) + readToken := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeReadRepository) + + t.Run("Success", func(t *testing.T) { + req := NewRequest(t, "POST", fmt.Sprintf("/api/v1/repos/%s/actions/runs/795/rerun", repo.FullName())). + AddTokenAuth(writeToken) + resp := MakeRequest(t, req, http.StatusCreated) + + var rerunResp api.ActionWorkflowRun + err := json.Unmarshal(resp.Body.Bytes(), &rerunResp) + require.NoError(t, err) + assert.Equal(t, int64(795), rerunResp.ID) + assert.Equal(t, "queued", rerunResp.Status) + assert.Equal(t, "c2d72f548424103f01ee1dc02889c1e2bff816b0", rerunResp.HeadSha) + + run, err := actions_model.GetRunByRepoAndID(t.Context(), repo.ID, 795) + require.NoError(t, err) + assert.Equal(t, actions_model.StatusWaiting, run.Status) + assert.Equal(t, timeutil.TimeStamp(0), run.Started) + assert.Equal(t, timeutil.TimeStamp(0), run.Stopped) + + job198, err := actions_model.GetRunJobByID(t.Context(), 198) + require.NoError(t, err) + assert.Equal(t, actions_model.StatusWaiting, job198.Status) + assert.Equal(t, int64(0), job198.TaskID) + + job199, err := actions_model.GetRunJobByID(t.Context(), 199) + require.NoError(t, err) + assert.Equal(t, actions_model.StatusWaiting, job199.Status) + assert.Equal(t, int64(0), job199.TaskID) + }) + + t.Run("ForbiddenWithoutWriteScope", func(t *testing.T) { + req := NewRequest(t, "POST", fmt.Sprintf("/api/v1/repos/%s/actions/runs/795/rerun", repo.FullName())). + AddTokenAuth(readToken) + MakeRequest(t, req, http.StatusForbidden) + }) + + t.Run("NotFound", func(t *testing.T) { + req := NewRequest(t, "POST", fmt.Sprintf("/api/v1/repos/%s/actions/runs/999999/rerun", repo.FullName())). + AddTokenAuth(writeToken) + MakeRequest(t, req, http.StatusNotFound) + }) +} + +func TestAPIActionsRerunWorkflowJob(t *testing.T) { + defer prepareTestEnvActionsArtifacts(t)() + + t.Run("NotDone", func(t *testing.T) { + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 4}) + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID}) + session := loginUser(t, user.Name) + writeToken := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) + + req := NewRequest(t, "POST", fmt.Sprintf("/api/v1/repos/%s/actions/runs/793/jobs/194/rerun", repo.FullName())). + AddTokenAuth(writeToken) + MakeRequest(t, req, http.StatusBadRequest) + }) + + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2}) + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID}) + session := loginUser(t, user.Name) + + writeToken := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) + readToken := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeReadRepository) + + t.Run("Success", func(t *testing.T) { + req := NewRequest(t, "POST", fmt.Sprintf("/api/v1/repos/%s/actions/runs/795/jobs/199/rerun", repo.FullName())). + AddTokenAuth(writeToken) + resp := MakeRequest(t, req, http.StatusCreated) + + var rerunResp api.ActionWorkflowJob + err := json.Unmarshal(resp.Body.Bytes(), &rerunResp) + require.NoError(t, err) + assert.Equal(t, int64(199), rerunResp.ID) + assert.Equal(t, "queued", rerunResp.Status) + + run, err := actions_model.GetRunByRepoAndID(t.Context(), repo.ID, 795) + require.NoError(t, err) + assert.Equal(t, actions_model.StatusWaiting, run.Status) + + job198, err := actions_model.GetRunJobByID(t.Context(), 198) + require.NoError(t, err) + assert.Equal(t, actions_model.StatusSuccess, job198.Status) + assert.Equal(t, int64(53), job198.TaskID) + + job199, err := actions_model.GetRunJobByID(t.Context(), 199) + require.NoError(t, err) + assert.Equal(t, actions_model.StatusWaiting, job199.Status) + assert.Equal(t, int64(0), job199.TaskID) + }) + + t.Run("ForbiddenWithoutWriteScope", func(t *testing.T) { + req := NewRequest(t, "POST", fmt.Sprintf("/api/v1/repos/%s/actions/runs/795/jobs/199/rerun", repo.FullName())). + AddTokenAuth(readToken) + MakeRequest(t, req, http.StatusForbidden) + }) + + t.Run("NotFoundJob", func(t *testing.T) { + req := NewRequest(t, "POST", fmt.Sprintf("/api/v1/repos/%s/actions/runs/795/jobs/999999/rerun", repo.FullName())). + AddTokenAuth(writeToken) + MakeRequest(t, req, http.StatusNotFound) + }) +} From 761b9d439b9c1f1bfe57e31ec74f7cf54cd4aef8 Mon Sep 17 00:00:00 2001 From: silverwind Date: Mon, 2 Mar 2026 23:08:53 +0100 Subject: [PATCH 2/8] Fix API not persisting pull request unit config when has_pull_requests is not set (#36718) The `PATCH /api/v1/repos/{owner}/{repo}` endpoint silently ignores pull request config fields (like `default_delete_branch_after_merge`, `allow_squash_merge`, etc.) unless `has_pull_requests: true` is also included in the request body. This is because the entire PR unit config block was gated behind `if opts.HasPullRequests != nil`. This PR restructures the logic so that PR config options are applied whenever the pull request unit already exists on the repo, without requiring `has_pull_requests` to be explicitly set. A new unit is only created when `has_pull_requests: true` is explicitly sent. Fixes https://github.com/go-gitea/gitea/issues/36466 Co-authored-by: Claude Opus 4.6 Co-authored-by: wxiaoguang Co-authored-by: Giteabot --- models/repo/repo_unit.go | 32 ++++--- modules/optional/option.go | 14 ++++ routers/api/v1/repo/repo.go | 107 ++++++++---------------- routers/web/repo/issue_view.go | 5 +- services/convert/repository.go | 2 +- services/repository/create.go | 10 +-- tests/integration/api_pull_test.go | 10 +-- tests/integration/api_repo_edit_test.go | 15 ++++ 8 files changed, 94 insertions(+), 101 deletions(-) diff --git a/models/repo/repo_unit.go b/models/repo/repo_unit.go index 1058a18a85..491e96770c 100644 --- a/models/repo/repo_unit.go +++ b/models/repo/repo_unit.go @@ -134,10 +134,25 @@ type PullRequestsConfig struct { DefaultTargetBranch string } +func DefaultPullRequestsConfig() *PullRequestsConfig { + cfg := &PullRequestsConfig{ + AllowMerge: true, + AllowRebase: true, + AllowRebaseMerge: true, + AllowSquash: true, + AllowFastForwardOnly: true, + AllowRebaseUpdate: true, + DefaultAllowMaintainerEdit: true, + } + cfg.DefaultMergeStyle = MergeStyle(setting.Repository.PullRequest.DefaultMergeStyle) + cfg.DefaultMergeStyle = util.IfZero(cfg.DefaultMergeStyle, MergeStyleMerge) + return cfg +} + // FromDB fills up a PullRequestsConfig from serialized format. func (cfg *PullRequestsConfig) FromDB(bs []byte) error { - // AllowRebaseUpdate = true as default for existing PullRequestConfig in DB - cfg.AllowRebaseUpdate = true + // set default values for existing PullRequestConfig in DB + *cfg = *DefaultPullRequestsConfig() return json.UnmarshalHandleDoubleEncode(bs, &cfg) } @@ -156,17 +171,8 @@ func (cfg *PullRequestsConfig) IsMergeStyleAllowed(mergeStyle MergeStyle) bool { mergeStyle == MergeStyleManuallyMerged && cfg.AllowManualMerge } -// GetDefaultMergeStyle returns the default merge style for this pull request -func (cfg *PullRequestsConfig) GetDefaultMergeStyle() MergeStyle { - if len(cfg.DefaultMergeStyle) != 0 { - return cfg.DefaultMergeStyle - } - - if setting.Repository.PullRequest.DefaultMergeStyle != "" { - return MergeStyle(setting.Repository.PullRequest.DefaultMergeStyle) - } - - return MergeStyleMerge +func DefaultPullRequestsUnit(repoID int64) RepoUnit { + return RepoUnit{RepoID: repoID, Type: unit.TypePullRequests, Config: DefaultPullRequestsConfig()} } type ActionsConfig struct { diff --git a/modules/optional/option.go b/modules/optional/option.go index cbecf86987..a278723bef 100644 --- a/modules/optional/option.go +++ b/modules/optional/option.go @@ -67,3 +67,17 @@ func ParseBool(s string) Option[bool] { } return Some(v) } + +func AssignPtrValue[T comparable](changed *bool, target, src *T) { + if src != nil && *src != *target { + *target = *src + *changed = true + } +} + +func AssignPtrString[TO, FROM ~string](changed *bool, target *TO, src *FROM) { + if src != nil && string(*src) != string(*target) { + *target = TO(*src) + *changed = true + } +} diff --git a/routers/api/v1/repo/repo.go b/routers/api/v1/repo/repo.go index bb6bda587d..cfdcf7b374 100644 --- a/routers/api/v1/repo/repo.go +++ b/routers/api/v1/repo/repo.go @@ -884,77 +884,44 @@ func updateRepoUnits(ctx *context.APIContext, opts api.EditRepoOption) error { } } - if opts.HasPullRequests != nil && !unit_model.TypePullRequests.UnitGlobalDisabled() { - if *opts.HasPullRequests { - // We do allow setting individual PR settings through the API, so - // we get the config settings and then set them - // if those settings were provided in the opts. - unit, err := repo.GetUnit(ctx, unit_model.TypePullRequests) - var config *repo_model.PullRequestsConfig - if err != nil { - // Unit type doesn't exist so we make a new config file with default values - config = &repo_model.PullRequestsConfig{ - IgnoreWhitespaceConflicts: false, - AllowMerge: true, - AllowRebase: true, - AllowRebaseMerge: true, - AllowSquash: true, - AllowFastForwardOnly: true, - AllowManualMerge: true, - AutodetectManualMerge: false, - AllowRebaseUpdate: true, - DefaultDeleteBranchAfterMerge: false, - DefaultMergeStyle: repo_model.MergeStyleMerge, - DefaultAllowMaintainerEdit: false, - } - } else { - config = unit.PullRequestsConfig() - } - - if opts.IgnoreWhitespaceConflicts != nil { - config.IgnoreWhitespaceConflicts = *opts.IgnoreWhitespaceConflicts - } - if opts.AllowMerge != nil { - config.AllowMerge = *opts.AllowMerge - } - if opts.AllowRebase != nil { - config.AllowRebase = *opts.AllowRebase - } - if opts.AllowRebaseMerge != nil { - config.AllowRebaseMerge = *opts.AllowRebaseMerge - } - if opts.AllowSquash != nil { - config.AllowSquash = *opts.AllowSquash - } - if opts.AllowFastForwardOnly != nil { - config.AllowFastForwardOnly = *opts.AllowFastForwardOnly - } - if opts.AllowManualMerge != nil { - config.AllowManualMerge = *opts.AllowManualMerge - } - if opts.AutodetectManualMerge != nil { - config.AutodetectManualMerge = *opts.AutodetectManualMerge - } - if opts.AllowRebaseUpdate != nil { - config.AllowRebaseUpdate = *opts.AllowRebaseUpdate - } - if opts.DefaultDeleteBranchAfterMerge != nil { - config.DefaultDeleteBranchAfterMerge = *opts.DefaultDeleteBranchAfterMerge - } - if opts.DefaultMergeStyle != nil { - config.DefaultMergeStyle = repo_model.MergeStyle(*opts.DefaultMergeStyle) - } - if opts.DefaultAllowMaintainerEdit != nil { - config.DefaultAllowMaintainerEdit = *opts.DefaultAllowMaintainerEdit - } - - units = append(units, repo_model.RepoUnit{ - RepoID: repo.ID, - Type: unit_model.TypePullRequests, - Config: config, - }) - } else { + if !unit_model.TypePullRequests.UnitGlobalDisabled() { + mustDeletePullRequestUnit := opts.HasPullRequests != nil && !*opts.HasPullRequests + mustInsertPullRequestUnit := opts.HasPullRequests != nil && *opts.HasPullRequests + if mustDeletePullRequestUnit { deleteUnitTypes = append(deleteUnitTypes, unit_model.TypePullRequests) + } else { + // We do allow setting individual PR settings through the API, + // so we get the config settings and then set them if those settings were provided in the opts. + unit, err := repo.GetUnit(ctx, unit_model.TypePullRequests) + if err != nil && !errors.Is(err, util.ErrNotExist) { + return err + } + if unit == nil { + // Unit doesn't exist yet but is being enabled, create with defaults + unit = new(repo_model.DefaultPullRequestsUnit(repo.ID)) + } + + changed := new(false) + config := unit.PullRequestsConfig() + optional.AssignPtrValue(changed, &config.IgnoreWhitespaceConflicts, opts.IgnoreWhitespaceConflicts) + optional.AssignPtrValue(changed, &config.AllowMerge, opts.AllowMerge) + optional.AssignPtrValue(changed, &config.AllowRebase, opts.AllowRebase) + optional.AssignPtrValue(changed, &config.AllowRebaseMerge, opts.AllowRebaseMerge) + optional.AssignPtrValue(changed, &config.AllowSquash, opts.AllowSquash) + optional.AssignPtrValue(changed, &config.AllowFastForwardOnly, opts.AllowFastForwardOnly) + optional.AssignPtrValue(changed, &config.AllowManualMerge, opts.AllowManualMerge) + optional.AssignPtrValue(changed, &config.AutodetectManualMerge, opts.AutodetectManualMerge) + optional.AssignPtrValue(changed, &config.AllowRebaseUpdate, opts.AllowRebaseUpdate) + optional.AssignPtrValue(changed, &config.DefaultDeleteBranchAfterMerge, opts.DefaultDeleteBranchAfterMerge) + optional.AssignPtrValue(changed, &config.DefaultAllowMaintainerEdit, opts.DefaultAllowMaintainerEdit) + optional.AssignPtrString(changed, &config.DefaultMergeStyle, opts.DefaultMergeStyle) + if *changed || mustInsertPullRequestUnit { + units = append(units, repo_model.RepoUnit{ + RepoID: repo.ID, + Type: unit_model.TypePullRequests, + Config: config, + }) + } } } diff --git a/routers/web/repo/issue_view.go b/routers/web/repo/issue_view.go index 1354c2d6f9..2cd8be4533 100644 --- a/routers/web/repo/issue_view.go +++ b/routers/web/repo/issue_view.go @@ -905,9 +905,8 @@ func preparePullViewReviewAndMerge(ctx *context.Context, issue *issues_model.Iss // Check correct values and select default if ms, ok := ctx.Data["MergeStyle"].(repo_model.MergeStyle); !ok || !prConfig.IsMergeStyleAllowed(ms) { - defaultMergeStyle := prConfig.GetDefaultMergeStyle() - if prConfig.IsMergeStyleAllowed(defaultMergeStyle) && !ok { - mergeStyle = defaultMergeStyle + if prConfig.IsMergeStyleAllowed(prConfig.DefaultMergeStyle) && !ok { + mergeStyle = prConfig.DefaultMergeStyle } else if prConfig.AllowMerge { mergeStyle = repo_model.MergeStyleMerge } else if prConfig.AllowRebase { diff --git a/services/convert/repository.go b/services/convert/repository.go index 658d31d55c..3c9cc83ccb 100644 --- a/services/convert/repository.go +++ b/services/convert/repository.go @@ -118,7 +118,7 @@ func innerToRepo(ctx context.Context, repo *repo_model.Repository, permissionInR allowManualMerge = config.AllowManualMerge autodetectManualMerge = config.AutodetectManualMerge defaultDeleteBranchAfterMerge = config.DefaultDeleteBranchAfterMerge - defaultMergeStyle = config.GetDefaultMergeStyle() + defaultMergeStyle = config.DefaultMergeStyle defaultAllowMaintainerEdit = config.DefaultAllowMaintainerEdit defaultTargetBranch = config.DefaultTargetBranch } diff --git a/services/repository/create.go b/services/repository/create.go index e027d3b979..a8b57b6707 100644 --- a/services/repository/create.go +++ b/services/repository/create.go @@ -386,15 +386,7 @@ func createRepositoryInDB(ctx context.Context, doer, u *user_model.User, repo *r }, }) case unit.TypePullRequests: - units = append(units, repo_model.RepoUnit{ - RepoID: repo.ID, - Type: tp, - Config: &repo_model.PullRequestsConfig{ - AllowMerge: true, AllowRebase: true, AllowRebaseMerge: true, AllowSquash: true, AllowFastForwardOnly: true, - DefaultMergeStyle: repo_model.MergeStyle(setting.Repository.PullRequest.DefaultMergeStyle), - AllowRebaseUpdate: true, - }, - }) + units = append(units, repo_model.DefaultPullRequestsUnit(repo.ID)) case unit.TypeProjects: units = append(units, repo_model.RepoUnit{ RepoID: repo.ID, diff --git a/tests/integration/api_pull_test.go b/tests/integration/api_pull_test.go index 61a37ddd01..fcd3554c52 100644 --- a/tests/integration/api_pull_test.go +++ b/tests/integration/api_pull_test.go @@ -279,10 +279,10 @@ func TestAPICreatePullSuccess(t *testing.T) { }).AddTokenAuth(token) MakeRequest(t, req, http.StatusCreated) - // Also test that AllowMaintainerEdit is false by default + // Also test that AllowMaintainerEdit is true by default, the "false" case is covered by TestAPICreatePullBasePermission prIssue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{Title: prTitle}) pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{IssueID: prIssue.ID}) - assert.False(t, pr.AllowMaintainerEdit) + assert.True(t, pr.AllowMaintainerEdit) MakeRequest(t, req, http.StatusUnprocessableEntity) // second request should fail } @@ -304,7 +304,7 @@ func TestAPICreatePullBasePermission(t *testing.T) { Base: "master", Title: prTitle, - AllowMaintainerEdit: new(true), + AllowMaintainerEdit: new(false), } req := NewRequestWithJSON(t, http.MethodPost, fmt.Sprintf("/api/v1/repos/%s/%s/pulls", owner10.Name, repo10.Name), &opts).AddTokenAuth(token) MakeRequest(t, req, http.StatusForbidden) @@ -317,10 +317,10 @@ func TestAPICreatePullBasePermission(t *testing.T) { req = NewRequestWithJSON(t, http.MethodPost, fmt.Sprintf("/api/v1/repos/%s/%s/pulls", owner10.Name, repo10.Name), &opts).AddTokenAuth(token) MakeRequest(t, req, http.StatusCreated) - // Also test that AllowMaintainerEdit is set to true + // Also test that AllowMaintainerEdit is set to false, the default "true" case is covered by TestAPICreatePullSuccess prIssue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{Title: prTitle}) pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{IssueID: prIssue.ID}) - assert.True(t, pr.AllowMaintainerEdit) + assert.False(t, pr.AllowMaintainerEdit) } func TestAPICreatePullHeadPermission(t *testing.T) { diff --git a/tests/integration/api_repo_edit_test.go b/tests/integration/api_repo_edit_test.go index 34d6990497..215b48b64b 100644 --- a/tests/integration/api_repo_edit_test.go +++ b/tests/integration/api_repo_edit_test.go @@ -418,5 +418,20 @@ func TestAPIRepoEdit(t *testing.T) { req = NewRequestWithJSON(t, "PATCH", fmt.Sprintf("/api/v1/repos/%s/%s", user2.Name, repo1.Name), &repoEditOption). AddTokenAuth(token4) MakeRequest(t, req, http.StatusForbidden) + + // Test updating pull request settings without setting has_pull_requests + repo1 = unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) + url = fmt.Sprintf("/api/v1/repos/%s/%s", user2.Name, repo1.Name) + req = NewRequestWithJSON(t, "PATCH", url, &api.EditRepoOption{ + DefaultDeleteBranchAfterMerge: &bTrue, + }).AddTokenAuth(token2) + resp = MakeRequest(t, req, http.StatusOK) + DecodeJSON(t, resp, &repo) + assert.True(t, repo.DefaultDeleteBranchAfterMerge) + // reset + req = NewRequestWithJSON(t, "PATCH", url, &api.EditRepoOption{ + DefaultDeleteBranchAfterMerge: &bFalse, + }).AddTokenAuth(token2) + _ = MakeRequest(t, req, http.StatusOK) }) } From 93e3be3018a35498c661f22a44ece0c59f847d42 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Wed, 4 Mar 2026 01:56:38 +0800 Subject: [PATCH 3/8] Fix CRAN package version validation to allow more than 4 version components (#36813) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: wxiaoguang <2114189+wxiaoguang@users.noreply.github.com> --- modules/packages/cran/metadata.go | 2 +- modules/packages/cran/metadata_test.go | 11 ++++++++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/modules/packages/cran/metadata.go b/modules/packages/cran/metadata.go index 0b0bfb07c6..0856565e10 100644 --- a/modules/packages/cran/metadata.go +++ b/modules/packages/cran/metadata.go @@ -34,7 +34,7 @@ var ( var ( fieldPattern = regexp.MustCompile(`\A\S+:`) namePattern = regexp.MustCompile(`\A[a-zA-Z][a-zA-Z0-9\.]*[a-zA-Z0-9]\z`) - versionPattern = regexp.MustCompile(`\A[0-9]+(?:[.\-][0-9]+){1,3}\z`) + versionPattern = regexp.MustCompile(`\A[0-9]+(?:[.\-][0-9]+)+\z`) authorReplacePattern = regexp.MustCompile(`[\[\(].+?[\]\)]`) ) diff --git a/modules/packages/cran/metadata_test.go b/modules/packages/cran/metadata_test.go index ff68c34c51..1d652a4a05 100644 --- a/modules/packages/cran/metadata_test.go +++ b/modules/packages/cran/metadata_test.go @@ -128,13 +128,22 @@ func TestParseDescription(t *testing.T) { }) t.Run("InvalidVersion", func(t *testing.T) { - for _, version := range []string{"1", "1 0", "1.2.3.4.5", "1-2-3-4-5", "1.", "1.0.", "1-", "1-0-"} { + for _, version := range []string{"1", "1 0", "1.", "1.0.", "1-", "1-0-"} { p, err := ParseDescription(createDescription(packageName, version)) assert.Nil(t, p) assert.ErrorIs(t, err, ErrInvalidVersion) } }) + t.Run("ValidVersionManyComponents", func(t *testing.T) { + for _, version := range []string{"0.3.4.0.2", "1.2.3.4.5", "1-2-3-4-5"} { + p, err := ParseDescription(createDescription(packageName, version)) + assert.NoError(t, err) + assert.NotNil(t, p) + assert.Equal(t, version, p.Version) + } + }) + t.Run("Valid", func(t *testing.T) { p, err := ParseDescription(createDescription(packageName, packageVersion)) assert.NoError(t, err) From 484eacb7bfe3de8d8ca002342f63cbe3ff360770 Mon Sep 17 00:00:00 2001 From: OptionalValue <167444167+OptionalValue@users.noreply.github.com> Date: Tue, 3 Mar 2026 20:23:27 +0100 Subject: [PATCH 4/8] fix: /repos/{owner}/{repo}/actions/{runs,jobs} requiring owner permissions (#36818) Resolves #36268 The REST endpoints: `/repos/{owner}/{repo}/actions/runs` `/repos/{owner}/{repo}/actions/jobs` currently require repository/organisation owner permissions, even though in GitHub they only need simple "read" permissions on the repo. In the web interface this is implemented correctly, where anyone with "read" permissions can see the list of action runs. --------- Co-authored-by: Leonard Immel --- routers/api/v1/api.go | 35 +++++++++++++++++++---------------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index cb6bbe0954..767e5533fd 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -895,34 +895,35 @@ func Routes() *web.Router { addActionsRoutes := func( m *web.Router, - reqChecker func(ctx *context.APIContext), + reqReaderCheck func(ctx *context.APIContext), + reqOwnerCheck func(ctx *context.APIContext), act actions.API, ) { m.Group("/actions", func() { m.Group("/secrets", func() { - m.Get("", reqToken(), reqChecker, act.ListActionsSecrets) + m.Get("", reqToken(), reqOwnerCheck, act.ListActionsSecrets) m.Combo("/{secretname}"). - Put(reqToken(), reqChecker, bind(api.CreateOrUpdateSecretOption{}), act.CreateOrUpdateSecret). - Delete(reqToken(), reqChecker, act.DeleteSecret) + Put(reqToken(), reqOwnerCheck, bind(api.CreateOrUpdateSecretOption{}), act.CreateOrUpdateSecret). + Delete(reqToken(), reqOwnerCheck, act.DeleteSecret) }) m.Group("/variables", func() { - m.Get("", reqToken(), reqChecker, act.ListVariables) + m.Get("", reqToken(), reqOwnerCheck, act.ListVariables) m.Combo("/{variablename}"). - Get(reqToken(), reqChecker, act.GetVariable). - Delete(reqToken(), reqChecker, act.DeleteVariable). - Post(reqToken(), reqChecker, bind(api.CreateVariableOption{}), act.CreateVariable). - Put(reqToken(), reqChecker, bind(api.UpdateVariableOption{}), act.UpdateVariable) + Get(reqToken(), reqOwnerCheck, act.GetVariable). + Delete(reqToken(), reqOwnerCheck, act.DeleteVariable). + Post(reqToken(), reqOwnerCheck, bind(api.CreateVariableOption{}), act.CreateVariable). + Put(reqToken(), reqOwnerCheck, bind(api.UpdateVariableOption{}), act.UpdateVariable) }) m.Group("/runners", func() { - m.Get("", reqToken(), reqChecker, act.ListRunners) - m.Post("/registration-token", reqToken(), reqChecker, act.CreateRegistrationToken) - m.Get("/{runner_id}", reqToken(), reqChecker, act.GetRunner) - m.Delete("/{runner_id}", reqToken(), reqChecker, act.DeleteRunner) + m.Get("", reqToken(), reqOwnerCheck, act.ListRunners) + m.Post("/registration-token", reqToken(), reqOwnerCheck, act.CreateRegistrationToken) + m.Get("/{runner_id}", reqToken(), reqOwnerCheck, act.GetRunner) + m.Delete("/{runner_id}", reqToken(), reqOwnerCheck, act.DeleteRunner) }) - m.Get("/runs", reqToken(), reqChecker, act.ListWorkflowRuns) - m.Get("/jobs", reqToken(), reqChecker, act.ListWorkflowJobs) + m.Get("/runs", reqToken(), reqReaderCheck, act.ListWorkflowRuns) + m.Get("/jobs", reqToken(), reqReaderCheck, act.ListWorkflowJobs) }) } @@ -1164,7 +1165,8 @@ func Routes() *web.Router { m.Post("/reject", repo.RejectTransfer) }, reqToken()) - addActionsRoutes(m, reqOwner(), repo.NewAction()) // it adds the routes for secrets/variables and runner management + // Adds the routes for secrets/variables and runner management + addActionsRoutes(m, reqRepoReader(unit.TypeActions), reqOwner(), repo.NewAction()) m.Group("/actions/workflows", func() { m.Get("", repo.ActionsListRepositoryWorkflows) @@ -1619,6 +1621,7 @@ func Routes() *web.Router { }) addActionsRoutes( m, + reqOrgMembership(), reqOrgOwnership(), org.NewAction(), ) From b874e0d8e571ca2eaafde72965e6f3adb6ea6073 Mon Sep 17 00:00:00 2001 From: GiteaBot Date: Wed, 4 Mar 2026 00:47:08 +0000 Subject: [PATCH 5/8] [skip ci] Updated translations via Crowdin --- options/locale/locale_ga-IE.json | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/options/locale/locale_ga-IE.json b/options/locale/locale_ga-IE.json index ad00325b02..4669252a6e 100644 --- a/options/locale/locale_ga-IE.json +++ b/options/locale/locale_ga-IE.json @@ -84,6 +84,7 @@ "save": "Sábháil", "add": "Cuir", "add_all": "Cuir Gach", + "dismiss": "Díbhe", "remove": "Bain", "remove_all": "Bain Gach", "remove_label_str": "Bain mír “%s”", @@ -224,7 +225,7 @@ "startpage.lightweight": "Éadrom", "startpage.lightweight_desc": "Tá íosta riachtanais íseal ag Gitea agus is féidir leo rith ar Raspberry Pi saor. Sábháil fuinneamh do mheaisín!", "startpage.license": "Foinse Oscailte", - "startpage.license_desc": "Téigh go bhfaighidh %[2]s! Bí linn trí cur leis chun an tionscadal seo a fheabhsú fós. Ná bíodh cúthail ort a bheith i do rannpháirtí!", + "startpage.license_desc": "Téigh agus faigh %[2]s! Bí linn trí cur leis chun an tionscadal seo a dhéanamh níos fearr fós. Ná bíodh leisce ort cur leis!", "install.install": "Suiteáil", "install.installing_desc": "Suiteáil anois, fan go fóill…", "install.title": "Cumraíocht Tosaigh", @@ -284,12 +285,6 @@ "install.register_confirm": "Deimhniú Ríomhphoist a cheangal le Clárú", "install.mail_notify": "Cumasaigh Fógraí Ríomhphoist", "install.server_service_title": "Socruithe Freastalaí agus Seirbhíse Tríú Páirtí", - "install.offline_mode": "Cumasaigh Mód Áitiúil", - "install.offline_mode_popup": "Díchumasaigh líonraí seachadta ábhair tríú páirtí agus freastal ar na hacmhainní go léir go háitiúil.", - "install.disable_gravatar": "Díchumasaigh Gravatar", - "install.disable_gravatar_popup": "Díchumasaigh foinsí abhatár Gravatar agus tríú páirtí. Úsáidfear abhatár réamhshocraithe mura n-uaslódálann úsáideoir abhatár go háitiúil.", - "install.federated_avatar_lookup": "Cumasaigh Abhatáir Chónaidhme", - "install.federated_avatar_lookup_popup": "Cumasaigh cuardach avatar cónaidhme ag baint úsáide as Libravatar.", "install.disable_registration": "Díchumasaigh Féin-Chlárú", "install.disable_registration_popup": "Díchumasaigh féinchlárú úsáideora. Ní bheidh ach riarthóirí in ann cuntais úsáideora nua a chruthú.", "install.allow_only_external_registration_popup": "Ceadaigh Clárú Trí Sheirbhísí Seachtracha amháin", @@ -871,7 +866,7 @@ "settings.permissions_list": "Ceadanna:", "settings.manage_oauth2_applications": "Bainistigh Feidhmchláir OAuth2", "settings.edit_oauth2_application": "Cuir Feidhmchlár OAuth2 in eagar", - "settings.oauth2_applications_desc": "Cumasaíonn feidhmchláir OAuth2 d’fheidhmchlár tríú páirtí úsáideoirí a fhíordheimhniú go slán ag an ásc Gitea seo.", + "settings.oauth2_applications_desc": "Cuireann feidhmchláir OAuth2 ar chumas d’fheidhmchlár tríú páirtí úsáideoirí a fhíordheimhniú go slán ag an gcás Gitea seo.", "settings.remove_oauth2_application": "Bain Feidhmchlár OAuth2", "settings.remove_oauth2_application_desc": "Ag baint feidhmchlár OAuth2, cúlghairfear rochtain ar gach comhartha rochtana sínithe. Lean ar aghaidh?", "settings.remove_oauth2_application_success": "Scriosadh an feidhmchlár.", @@ -890,7 +885,7 @@ "settings.oauth2_regenerate_secret_hint": "Chaill tú do rún?", "settings.oauth2_client_secret_hint": "Ní thaispeánfar an rún arís tar éis duit an leathanach seo a fhágáil nó a athnuachan. Déan cinnte le do thoil gur shábháil tú é.", "settings.oauth2_application_edit": "Cuir in eagar", - "settings.oauth2_application_create_description": "Tugann feidhmchláir OAuth2 rochtain d'iarratas tríú páirtí ar chuntais úsáideora ar an gcás seo.", + "settings.oauth2_application_create_description": "Tugann feidhmchláir OAuth2 rochtain do d’fheidhmchlár tríú páirtí ar chuntais úsáideora ar an gcás seo.", "settings.oauth2_application_remove_description": "Cuirfear feidhmchlár OAuth2 a bhaint cosc air rochtain a fháil ar chuntais úsáideora údaraithe ar an gcás seo. Lean ar aghaidh?", "settings.oauth2_application_locked": "Réamhchláraíonn Gitea roinnt feidhmchlár OAuth2 ar thosú má tá sé cumasaithe i gcumraíocht. Chun iompar gan choinne a chosc, ní féidir iad seo a chur in eagar ná a bhaint. Féach do thoil do dhoiciméadú OAuth2 le haghaidh tuilleadh faisnéise.", "settings.authorized_oauth2_applications": "Feidhmchláir Údaraithe OAuth2", @@ -1524,6 +1519,7 @@ "repo.issues.commented_at": "trácht %s ", "repo.issues.delete_comment_confirm": "An bhfuil tú cinnte gur mhaith leat an trácht seo a scriosadh?", "repo.issues.context.copy_link": "Cóipeáil Nasc", + "repo.issues.context.copy_source": "Cóipeáil Foinse", "repo.issues.context.quote_reply": "Luaigh Freagra", "repo.issues.context.reference_issue": "Tagairt in Eagrán Nua", "repo.issues.context.edit": "Cuir in eagar", @@ -3192,7 +3188,6 @@ "admin.config.custom_conf": "Cosán Comhad Cumraíochta", "admin.config.custom_file_root_path": "Cosán Fréamh Comhad Saincheaptha", "admin.config.domain": "Fearann ​​Freastalaí", - "admin.config.offline_mode": "Mód Áitiúil", "admin.config.disable_router_log": "Díchumasaigh Loga an Ródaire", "admin.config.run_user": "Rith Mar Ainm úsáideora", "admin.config.run_mode": "Mód Rith", @@ -3278,6 +3273,13 @@ "admin.config.cache_test_failed": "Theip ar an taisce a thaiscéaladh: %v.", "admin.config.cache_test_slow": "D'éirigh leis an tástáil taisce, ach tá an freagra mall: %s.", "admin.config.cache_test_succeeded": "D'éirigh leis an tástáil taisce, fuair sé freagra i %s.", + "admin.config.common.start_time": "Am tosaithe", + "admin.config.common.end_time": "Am deiridh", + "admin.config.common.skip_time_check": "Fág an t-am folamh (glan an réimse) chun seiceáil ama a scipeáil", + "admin.config.instance_maintenance": "Cothabháil Cásanna", + "admin.config.instance_maintenance_mode.admin_web_access_only": "Lig don riarthóir amháin rochtain a fháil ar chomhéadan gréasáin", + "admin.config.instance_web_banner.enabled": "Taispeáin meirge", + "admin.config.instance_web_banner.message_placeholder": "Teachtaireacht meirge (tacaíonn sé le Markdown)", "admin.config.session_config": "Cumraíocht Seisiúin", "admin.config.session_provider": "Soláthraí Seisiúin", "admin.config.provider_config": "Cumraíocht Soláthraí", @@ -3288,7 +3290,7 @@ "admin.config.cookie_life_time": "Am Saoil Fianán", "admin.config.picture_config": "Cumraíocht Pictiúr agus Avatar", "admin.config.picture_service": "Seirbhís Pictiúr", - "admin.config.disable_gravatar": "Díchumasaigh Gravatar", + "admin.config.enable_gravatar": "Cumasaigh Gravatar", "admin.config.enable_federated_avatar": "Cumasaigh Avatars Cónaidhme", "admin.config.open_with_editor_app_help": "Na heagarthóirí \"Oscailte le\" don roghchlár Clón. Má fhágtar folamh é, úsáidfear an réamhshocrú. Leathnaigh chun an réamhshocrú a fheiceáil.", "admin.config.git_guide_remote_name": "Ainm iargúlta stórais le haghaidh orduithe git sa treoir", @@ -3672,6 +3674,8 @@ "actions.runners.reset_registration_token_confirm": "Ar mhaith leat an comhartha reatha a neamhbhailiú agus ceann nua a ghiniúint?", "actions.runners.reset_registration_token_success": "D'éirigh le hathshocrú comhartha clárúcháin an dara háit", "actions.runs.all_workflows": "Gach Sreafaí Oibre", + "actions.runs.workflow_run_count_1": "%d rith sreabha oibre", + "actions.runs.workflow_run_count_n": "%d rith sreabha oibre", "actions.runs.commit": "Tiomantas", "actions.runs.scheduled": "Sceidealaithe", "actions.runs.pushed_by": "bhrú ag", From 315b947740a2e3a5ae8d4cbf594042e473c00734 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 3 Mar 2026 23:15:33 -0800 Subject: [PATCH 6/8] Harden render iframe open-link handling (#36811) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR hardens the handling of the “open-link” action in render iframes (external rendering iframes). It prevents iframes from triggering unsafe or unintended redirects or opening new windows via postMessage. Additionally, it improves iframe height reporting to reduce scrollbar and height mismatch issues, and adds unit test coverage. --------- Co-authored-by: wxiaoguang --- web_src/js/markup/render-iframe.test.ts | 46 +++++++++++++++++++ web_src/js/markup/render-iframe.ts | 39 ++++++++++++---- .../js/standalone/external-render-iframe.ts | 20 ++++++-- 3 files changed, 92 insertions(+), 13 deletions(-) create mode 100644 web_src/js/markup/render-iframe.test.ts diff --git a/web_src/js/markup/render-iframe.test.ts b/web_src/js/markup/render-iframe.test.ts new file mode 100644 index 0000000000..53c9dc3720 --- /dev/null +++ b/web_src/js/markup/render-iframe.test.ts @@ -0,0 +1,46 @@ +import {navigateToIframeLink} from './render-iframe.ts'; + +describe('navigateToIframeLink', () => { + const openSpy = vi.spyOn(window, 'open').mockImplementation(() => null); + const assignSpy = vi.spyOn(window.location, 'assign').mockImplementation(() => undefined); + + test('safe links', () => { + navigateToIframeLink('http://example.com', '_blank'); + expect(openSpy).toHaveBeenCalledWith('http://example.com/', '_blank', 'noopener,noreferrer'); + vi.clearAllMocks(); + + navigateToIframeLink('https://example.com', '_self'); + expect(assignSpy).toHaveBeenCalledWith('https://example.com/'); + vi.clearAllMocks(); + + navigateToIframeLink('https://example.com', null); + expect(assignSpy).toHaveBeenCalledWith('https://example.com/'); + vi.clearAllMocks(); + + navigateToIframeLink('/path', ''); + expect(assignSpy).toHaveBeenCalledWith('http://localhost:3000/path'); + vi.clearAllMocks(); + + // input can be any type & any value, keep the same behavior as `window.location.href = 0` + navigateToIframeLink(0, {}); + expect(assignSpy).toHaveBeenCalledWith('http://localhost:3000/0'); + vi.clearAllMocks(); + }); + + test('unsafe links', () => { + window.location.href = 'http://localhost:3000/'; + + // eslint-disable-next-line no-script-url + navigateToIframeLink('javascript:void(0);', '_blank'); + expect(openSpy).toHaveBeenCalledTimes(0); + expect(assignSpy).toHaveBeenCalledTimes(0); + expect(window.location.href).toBe('http://localhost:3000/'); + vi.clearAllMocks(); + + navigateToIframeLink('data:image/svg+xml;utf8,', ''); + expect(openSpy).toHaveBeenCalledTimes(0); + expect(assignSpy).toHaveBeenCalledTimes(0); + expect(window.location.href).toBe('http://localhost:3000/'); + vi.clearAllMocks(); + }); +}); diff --git a/web_src/js/markup/render-iframe.ts b/web_src/js/markup/render-iframe.ts index 1291dea4f8..531942e0b1 100644 --- a/web_src/js/markup/render-iframe.ts +++ b/web_src/js/markup/render-iframe.ts @@ -1,23 +1,46 @@ import {generateElemId, queryElemChildren} from '../utils/dom.ts'; import {isDarkTheme} from '../utils.ts'; +function safeRenderIframeLink(link: any): string | null { + try { + const url = new URL(`${link}`, window.location.href); + if (url.protocol !== 'http:' && url.protocol !== 'https:') { + console.error(`Unsupported link protocol: ${link}`); + return null; + } + return url.href; + } catch (e) { + console.error(`Failed to parse link: ${link}, error: ${e}`); + return null; + } +} + +// This function is only designed for "open-link" command from iframe, is not suitable for other contexts. +// Because other link protocols are directly handled by the iframe, but not here. +// Arguments can be any type & any value, they are from "message" event's data which is not controlled by us. +export function navigateToIframeLink(unsafeLink: any, target: any) { + const linkHref = safeRenderIframeLink(unsafeLink); + if (linkHref === null) return; + if (target === '_blank') { + window.open(linkHref, '_blank', 'noopener,noreferrer'); + return; + } + // treat all other targets including ("_top", "_self", etc.) as same tab navigation + window.location.assign(linkHref); +} + async function loadRenderIframeContent(iframe: HTMLIFrameElement) { const iframeSrcUrl = iframe.getAttribute('data-src')!; if (!iframe.id) iframe.id = generateElemId('gitea-iframe-'); window.addEventListener('message', (e) => { + if (e.source !== iframe.contentWindow) return; if (!e.data?.giteaIframeCmd || e.data?.giteaIframeId !== iframe.id) return; const cmd = e.data.giteaIframeCmd; if (cmd === 'resize') { - // TODO: sometimes the reported iframeHeight is not the size we need, need to figure why. Example: openapi swagger. - // As a workaround, add some pixels here. - iframe.style.height = `${e.data.iframeHeight + 2}px`; + iframe.style.height = `${e.data.iframeHeight}px`; } else if (cmd === 'open-link') { - if (e.data.anchorTarget === '_blank') { - window.open(e.data.openLink, '_blank'); - } else { - window.location.href = e.data.openLink; - } + navigateToIframeLink(e.data.openLink, e.data.anchorTarget); } else { throw new Error(`Unknown gitea iframe cmd: ${cmd}`); } diff --git a/web_src/js/standalone/external-render-iframe.ts b/web_src/js/standalone/external-render-iframe.ts index dcfeb50541..f8ec070785 100644 --- a/web_src/js/standalone/external-render-iframe.ts +++ b/web_src/js/standalone/external-render-iframe.ts @@ -20,7 +20,15 @@ function mainExternalRenderIframe() { window.parent.postMessage({giteaIframeCmd: cmd, giteaIframeId: iframeId, ...data}, '*'); }; - const updateIframeHeight = () => postIframeMsg('resize', {iframeHeight: document.documentElement.scrollHeight}); + const updateIframeHeight = () => { + // Don't use integer heights from the DOM node. + // Use getBoundingClientRect(), then ceil the height to avoid fractional pixels which causes incorrect scrollbars. + const rect = document.documentElement.getBoundingClientRect(); + postIframeMsg('resize', {iframeHeight: Math.ceil(rect.height)}); + // As long as the parent page is responsible for the iframe height, the iframe itself doesn't need scrollbars. + // This style should only be dynamically set here when our code can run. + document.documentElement.style.overflowY = 'hidden'; + }; const resizeObserver = new ResizeObserver(() => updateIframeHeight()); resizeObserver.observe(window.document.documentElement); @@ -29,16 +37,18 @@ function mainExternalRenderIframe() { // the easiest way to handle dynamic content changes and easy to debug, can be fine-tuned in the future setInterval(updateIframeHeight, 1000); - // no way to open an absolute link with CSP frame-src, it also needs some tricks like "postMessage" or "copy the link to clipboard" - const openIframeLink = (link: string, target: string) => postIframeMsg('open-link', {openLink: link, anchorTarget: target}); + // no way to open an absolute link with CSP frame-src, it needs some tricks like "postMessage" (let parent window to handle) or "copy the link to clipboard" (let users manually paste it to open). + // here we choose "postMessage" way for better user experience. + const openIframeLink = (link: string, target: string | null) => postIframeMsg('open-link', {openLink: link, anchorTarget: target}); document.addEventListener('click', (e) => { const el = e.target as HTMLAnchorElement; if (el.nodeName !== 'A') return; - const href = el.getAttribute('href') || ''; + const href = el.getAttribute('href') ?? ''; // safe links: "./any", "../any", "/any", "//host/any", "http://host/any", "https://host/any" if (href.startsWith('.') || href.startsWith('/') || href.startsWith('http://') || href.startsWith('https://')) { e.preventDefault(); - openIframeLink(href, el.getAttribute('target')!); + const forceTarget = (e.metaKey || e.ctrlKey) ? '_blank' : null; + openIframeLink(href, forceTarget ?? el.getAttribute('target')); } }); } From 79ae9ea97b8d0c6267901c62c9ceae830cd36089 Mon Sep 17 00:00:00 2001 From: Tyrone Yeh Date: Wed, 4 Mar 2026 21:23:17 +0800 Subject: [PATCH 7/8] fix(repo): unify DEFAULT_SHOW_FULL_NAME output in templates and dropdown (#36597) The design of DefaultShowFullName has some problems, which make the UI inconsistent, see the new comment in code This PR does a clean up for various legacy problems, and clarify some "user name display" behaviors. --------- Co-authored-by: wxiaoguang --- models/repo/user_repo.go | 16 +++-- models/repo/user_repo_test.go | 4 +- models/user/user.go | 64 ++++++++++--------- modules/setting/ui.go | 10 ++- modules/templates/helper.go | 3 - modules/templates/util_avatar.go | 9 +-- routers/web/repo/issue_content_history.go | 29 ++++----- routers/web/repo/issue_poster.go | 7 +- services/mailer/mail.go | 13 ++-- templates/admin/org/list.tmpl | 7 +- templates/repo/commits_list.tmpl | 21 +++--- templates/repo/graph/commits.tmpl | 11 ++-- .../repo/issue/filter_item_user_assign.tmpl | 2 +- templates/repo/latest_commit.tmpl | 22 +++---- templates/repo/search_name.tmpl | 2 +- tests/integration/repo_commits_test.go | 11 ++-- web_src/css/repo.css | 10 ++- web_src/js/features/repo-issue-list.ts | 9 +-- 18 files changed, 128 insertions(+), 122 deletions(-) diff --git a/models/repo/user_repo.go b/models/repo/user_repo.go index 08cf964bc8..e15a64b01e 100644 --- a/models/repo/user_repo.go +++ b/models/repo/user_repo.go @@ -147,19 +147,21 @@ func GetRepoAssignees(ctx context.Context, repo *Repository) (_ []*user_model.Us } // GetIssuePostersWithSearch returns users with limit of 30 whose username started with prefix that have authored an issue/pull request for the given repository -// If isShowFullName is set to true, also include full name prefix search -func GetIssuePostersWithSearch(ctx context.Context, repo *Repository, isPull bool, search string, isShowFullName bool) ([]*user_model.User, error) { +// It searches with the "user.name" and "user.full_name" fields case-insensitively. +func GetIssuePostersWithSearch(ctx context.Context, repo *Repository, isPull bool, search string) ([]*user_model.User, error) { users := make([]*user_model.User, 0, 30) - var prefixCond builder.Cond = builder.Like{"lower_name", strings.ToLower(search) + "%"} - if search != "" && isShowFullName { - prefixCond = prefixCond.Or(db.BuildCaseInsensitiveLike("full_name", "%"+search+"%")) - } cond := builder.In("`user`.id", builder.Select("poster_id").From("issue").Where( builder.Eq{"repo_id": repo.ID}. And(builder.Eq{"is_pull": isPull}), - ).GroupBy("poster_id")).And(prefixCond) + ).GroupBy("poster_id")) + + if search != "" { + var prefixCond builder.Cond = builder.Like{"lower_name", strings.ToLower(search) + "%"} + prefixCond = prefixCond.Or(db.BuildCaseInsensitiveLike("full_name", "%"+search+"%")) + cond = cond.And(prefixCond) + } return users, db.GetEngine(ctx). Where(cond). diff --git a/models/repo/user_repo_test.go b/models/repo/user_repo_test.go index a53cf39dc4..cd8a0f1a1f 100644 --- a/models/repo/user_repo_test.go +++ b/models/repo/user_repo_test.go @@ -44,12 +44,12 @@ func TestGetIssuePostersWithSearch(t *testing.T) { repo2 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2}) - users, err := repo_model.GetIssuePostersWithSearch(t.Context(), repo2, false, "USER", false /* full name */) + users, err := repo_model.GetIssuePostersWithSearch(t.Context(), repo2, false, "USER") require.NoError(t, err) require.Len(t, users, 1) assert.Equal(t, "user2", users[0].Name) - users, err = repo_model.GetIssuePostersWithSearch(t.Context(), repo2, false, "TW%O", true /* full name */) + users, err = repo_model.GetIssuePostersWithSearch(t.Context(), repo2, false, "TW%O") require.NoError(t, err) require.Len(t, users, 1) assert.Equal(t, "user2", users[0].Name) diff --git a/models/user/user.go b/models/user/user.go index d8f41b869e..a74662bb12 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -8,6 +8,7 @@ import ( "context" "encoding/hex" "fmt" + "html/template" "mime" "net/mail" "net/url" @@ -28,6 +29,7 @@ import ( "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/container" "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/htmlutil" "code.gitea.io/gitea/modules/httplib" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/optional" @@ -417,16 +419,6 @@ func (u *User) IsTokenAccessAllowed() bool { return u.Type == UserTypeIndividual || u.Type == UserTypeBot } -// DisplayName returns full name if it's not empty, -// returns username otherwise. -func (u *User) DisplayName() string { - trimmed := strings.TrimSpace(u.FullName) - if len(trimmed) > 0 { - return trimmed - } - return u.Name -} - // EmailTo returns a string suitable to be put into a e-mail `To:` header. func (u *User) EmailTo() string { sanitizedDisplayName := globalVars().emailToReplacer.Replace(u.DisplayName()) @@ -445,27 +437,45 @@ func (u *User) EmailTo() string { return fmt.Sprintf("%s <%s>", mime.QEncoding.Encode("utf-8", add.Name), add.Address) } -// GetDisplayName returns full name if it's not empty and DEFAULT_SHOW_FULL_NAME is set, -// returns username otherwise. +// TODO: DefaultShowFullName causes messy logic, there are already too many methods to display a user's "display name", need to refactor them +// * user.Name / user.FullName: directly used in templates +// * user.DisplayName(): always show FullName if it's not empty, otherwise show Name +// * user.GetDisplayName(): show FullName if it's not empty and DefaultShowFullName is set, otherwise show Name +// * user.ShortName(): used a lot in templates, but it should be removed and let frontend use "ellipsis" styles +// * activity action.ShortActUserName/GetActDisplayName/GetActDisplayNameTitle, etc: duplicate and messy + +// DisplayName returns full name if it's not empty, returns username otherwise. +func (u *User) DisplayName() string { + fullName := strings.TrimSpace(u.FullName) + if fullName != "" { + return fullName + } + return u.Name +} + +// GetDisplayName returns full name if it's not empty and DEFAULT_SHOW_FULL_NAME is set, otherwise, username. func (u *User) GetDisplayName() string { if setting.UI.DefaultShowFullName { - trimmed := strings.TrimSpace(u.FullName) - if len(trimmed) > 0 { - return trimmed + fullName := strings.TrimSpace(u.FullName) + if fullName != "" { + return fullName } } return u.Name } -// GetCompleteName returns the full name and username in the form of -// "Full Name (username)" if full name is not empty, otherwise it returns -// "username". -func (u *User) GetCompleteName() string { - trimmedFullName := strings.TrimSpace(u.FullName) - if len(trimmedFullName) > 0 { - return fmt.Sprintf("%s (%s)", trimmedFullName, u.Name) +// ShortName ellipses username to length (still used by many templates), it calls GetDisplayName and respects DEFAULT_SHOW_FULL_NAME +func (u *User) ShortName(length int) string { + return util.EllipsisDisplayString(u.GetDisplayName(), length) +} + +func (u *User) GetShortDisplayNameLinkHTML() template.HTML { + fullName := strings.TrimSpace(u.FullName) + displayName, displayTooltip := u.Name, fullName + if setting.UI.DefaultShowFullName && fullName != "" { + displayName, displayTooltip = fullName, u.Name } - return u.Name + return htmlutil.HTMLFormat(`%s`, u.HomeLink(), displayTooltip, displayName) } func gitSafeName(name string) string { @@ -488,14 +498,6 @@ func (u *User) GitName() string { return fmt.Sprintf("user-%d", u.ID) } -// ShortName ellipses username to length -func (u *User) ShortName(length int) string { - if setting.UI.DefaultShowFullName && len(u.FullName) > 0 { - return util.EllipsisDisplayString(u.FullName, length) - } - return util.EllipsisDisplayString(u.Name, length) -} - // IsMailable checks if a user is eligible to receive emails. // System users like Ghost and Gitea Actions are excluded. func (u *User) IsMailable() bool { diff --git a/modules/setting/ui.go b/modules/setting/ui.go index 77a5b45d0a..722341a71e 100644 --- a/modules/setting/ui.go +++ b/modules/setting/ui.go @@ -25,7 +25,6 @@ var UI = struct { ReactionMaxUserNum int MaxDisplayFileSize int64 ShowUserEmail bool - DefaultShowFullName bool DefaultTheme string Themes []string FileIconTheme string @@ -43,6 +42,15 @@ var UI = struct { AmbiguousUnicodeDetection bool + // TODO: DefaultShowFullName is introduced by https://github.com/go-gitea/gitea/pull/6710 + // But there are still many edge cases: + // * Many places still use "username", not respecting this setting + // * Many places use "Full Name" if it is not empty, cause inconsistent UI for users who have set their full name but some others don't + // * Even if DefaultShowFullName=false, many places still need to show the full name + // For most cases, either "username" or "username (Full Name)" should be used and are good enough. + // Only in very few cases (e.g.: unimportant lists, narrow layout), "username" or "Full Name" can be used. + DefaultShowFullName bool + Notification struct { MinTimeout time.Duration TimeoutStep time.Duration diff --git a/modules/templates/helper.go b/modules/templates/helper.go index 11c52bd5a7..82087568df 100644 --- a/modules/templates/helper.go +++ b/modules/templates/helper.go @@ -96,9 +96,6 @@ func NewFuncMap() template.FuncMap { "AssetVersion": func() string { return setting.AssetVersion }, - "DefaultShowFullName": func() bool { - return setting.UI.DefaultShowFullName - }, "ShowFooterTemplateLoadTime": func() bool { return setting.Other.ShowFooterTemplateLoadTime }, diff --git a/modules/templates/util_avatar.go b/modules/templates/util_avatar.go index ee9994ab0b..524c64d0b6 100644 --- a/modules/templates/util_avatar.go +++ b/modules/templates/util_avatar.go @@ -16,6 +16,7 @@ import ( user_model "code.gitea.io/gitea/models/user" gitea_html "code.gitea.io/gitea/modules/htmlutil" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/util" ) type AvatarUtils struct { @@ -29,13 +30,9 @@ func NewAvatarUtils(ctx context.Context) *AvatarUtils { // AvatarHTML creates the HTML for an avatar func AvatarHTML(src string, size int, class, name string) template.HTML { sizeStr := strconv.Itoa(size) - - if name == "" { - name = "avatar" - } - + name = util.IfZero(name, "avatar") // use empty alt, otherwise if the image fails to load, the width will follow the "alt" text's width - return template.HTML(``) + return template.HTML(``) } // Avatar renders user avatars. args: user, size (int), class (string) diff --git a/routers/web/repo/issue_content_history.go b/routers/web/repo/issue_content_history.go index a56df78163..23cedfcb80 100644 --- a/routers/web/repo/issue_content_history.go +++ b/routers/web/repo/issue_content_history.go @@ -6,13 +6,14 @@ package repo import ( "bytes" "html" + "html/template" "net/http" "strings" "code.gitea.io/gitea/models/avatars" issues_model "code.gitea.io/gitea/models/issues" + "code.gitea.io/gitea/modules/htmlutil" "code.gitea.io/gitea/modules/log" - "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/templates" "code.gitea.io/gitea/services/context" @@ -53,29 +54,25 @@ func GetContentHistoryList(ctx *context.Context) { // value is historyId var results []map[string]any for _, item := range items { - var actionText string + var actionHTML template.HTML if item.IsDeleted { - actionTextDeleted := ctx.Locale.TrString("repo.issues.content_history.deleted") - actionText = "" + actionTextDeleted + "" + actionHTML = htmlutil.HTMLFormat(`%s`, ctx.Locale.TrString("repo.issues.content_history.deleted")) } else if item.IsFirstCreated { - actionText = ctx.Locale.TrString("repo.issues.content_history.created") + actionHTML = ctx.Locale.Tr("repo.issues.content_history.created") } else { - actionText = ctx.Locale.TrString("repo.issues.content_history.edited") + actionHTML = ctx.Locale.Tr("repo.issues.content_history.edited") } - username := item.UserName - if setting.UI.DefaultShowFullName && strings.TrimSpace(item.UserFullName) != "" { - username = strings.TrimSpace(item.UserFullName) + var fullNameHTML template.HTML + userName, fullName := item.UserName, strings.TrimSpace(item.UserFullName) + if fullName != "" { + fullNameHTML = htmlutil.HTMLFormat(` (%s)`, fullName) } - src := html.EscapeString(item.UserAvatarLink) - class := avatars.DefaultAvatarClass + " tw-mr-2" - name := html.EscapeString(username) - avatarHTML := string(templates.AvatarHTML(src, 28, class, username)) - timeSinceHTML := string(templates.TimeSince(item.EditedUnix)) - + avatarHTML := templates.AvatarHTML(item.UserAvatarLink, 24, avatars.DefaultAvatarClass+" tw-mr-2", userName) + timeSinceHTML := templates.TimeSince(item.EditedUnix) results = append(results, map[string]any{ - "name": avatarHTML + "" + name + " " + actionText + " " + timeSinceHTML, + "name": htmlutil.HTMLFormat("%s %s%s %s %s", avatarHTML, userName, fullNameHTML, actionHTML, timeSinceHTML), "value": item.HistoryID, }) } diff --git a/routers/web/repo/issue_poster.go b/routers/web/repo/issue_poster.go index 07059b9b7b..4f00f40a91 100644 --- a/routers/web/repo/issue_poster.go +++ b/routers/web/repo/issue_poster.go @@ -10,7 +10,6 @@ import ( repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" - "code.gitea.io/gitea/modules/setting" shared_user "code.gitea.io/gitea/routers/web/shared/user" "code.gitea.io/gitea/services/context" ) @@ -34,7 +33,7 @@ func IssuePullPosters(ctx *context.Context) { func issuePosters(ctx *context.Context, isPullList bool) { repo := ctx.Repo.Repository search := strings.TrimSpace(ctx.FormString("q")) - posters, err := repo_model.GetIssuePostersWithSearch(ctx, repo, isPullList, search, setting.UI.DefaultShowFullName) + posters, err := repo_model.GetIssuePostersWithSearch(ctx, repo, isPullList, search) if err != nil { ctx.JSON(http.StatusInternalServerError, err) return @@ -54,9 +53,7 @@ func issuePosters(ctx *context.Context, isPullList bool) { resp.Results = make([]*userSearchInfo, len(posters)) for i, user := range posters { resp.Results[i] = &userSearchInfo{UserID: user.ID, UserName: user.Name, AvatarLink: user.AvatarLink(ctx)} - if setting.UI.DefaultShowFullName { - resp.Results[i].FullName = user.FullName - } + resp.Results[i].FullName = user.FullName } ctx.JSON(http.StatusOK, resp) } diff --git a/services/mailer/mail.go b/services/mailer/mail.go index 8f831f89ad..a08ed71480 100644 --- a/services/mailer/mail.go +++ b/services/mailer/mail.go @@ -158,18 +158,23 @@ func (b64embedder *mailAttachmentBase64Embedder) AttachmentSrcToBase64DataURI(ct func fromDisplayName(u *user_model.User) string { if setting.MailService.FromDisplayNameFormatTemplate != nil { - var ctx bytes.Buffer - err := setting.MailService.FromDisplayNameFormatTemplate.Execute(&ctx, map[string]any{ + var buf bytes.Buffer + err := setting.MailService.FromDisplayNameFormatTemplate.Execute(&buf, map[string]any{ "DisplayName": u.DisplayName(), "AppName": setting.AppName, "Domain": setting.Domain, }) if err == nil { - return mime.QEncoding.Encode("utf-8", ctx.String()) + return mime.QEncoding.Encode("utf-8", buf.String()) } log.Error("fromDisplayName: %w", err) } - return u.GetCompleteName() + def := u.Name + if fullName := strings.TrimSpace(u.FullName); fullName != "" { + // use "Full Name (username)" for email's sender name if Full Name is not empty + def = fullName + " (" + u.Name + ")" + } + return def } func generateMetadataHeaders(repo *repo_model.Repository) map[string]string { diff --git a/templates/admin/org/list.tmpl b/templates/admin/org/list.tmpl index a4317b8c4e..e6de93c5f8 100644 --- a/templates/admin/org/list.tmpl +++ b/templates/admin/org/list.tmpl @@ -48,11 +48,14 @@ - {{range .Users}} + {{range $org := .Users}} {{.ID}} - {{if and DefaultShowFullName .FullName}}{{.FullName}} ({{.Name}}){{else}}{{.Name}}{{end}} + + {{$org.Name}} + {{if $org.FullName}}({{$org.FullName}}){{end}} + {{if .Visibility.IsPrivate}} {{svg "octicon-lock"}} {{end}} diff --git a/templates/repo/commits_list.tmpl b/templates/repo/commits_list.tmpl index 1a236582a2..a0722307a7 100644 --- a/templates/repo/commits_list.tmpl +++ b/templates/repo/commits_list.tmpl @@ -14,18 +14,15 @@ {{range .Commits}} -
- {{$userName := .Author.Name}} - {{if .User}} - {{if and .User.FullName DefaultShowFullName}} - {{$userName = .User.FullName}} - {{end}} - {{ctx.AvatarUtils.Avatar .User 28 "tw-mr-2"}}{{$userName}} - {{else}} - {{ctx.AvatarUtils.AvatarByEmail .Author.Email .Author.Name 28 "tw-mr-2"}} - {{$userName}} - {{end}} -
+ + {{- if .User -}} + {{- ctx.AvatarUtils.Avatar .User 20 "tw-mr-2" -}} + {{- .User.GetShortDisplayNameLinkHTML -}} + {{- else -}} + {{- ctx.AvatarUtils.AvatarByEmail .Author.Email .Author.Name 20 "tw-mr-2" -}} + {{- .Author.Name -}} + {{- end -}} + {{$commitBaseLink := ""}} diff --git a/templates/repo/graph/commits.tmpl b/templates/repo/graph/commits.tmpl index d92be9c5ed..d86f73fe65 100644 --- a/templates/repo/graph/commits.tmpl +++ b/templates/repo/graph/commits.tmpl @@ -41,16 +41,13 @@ - {{$userName := $commit.Commit.Author.Name}} {{if $commit.User}} - {{if and $commit.User.FullName DefaultShowFullName}} - {{$userName = $commit.User.FullName}} - {{end}} {{ctx.AvatarUtils.Avatar $commit.User 18}} - {{$userName}} + {{$commit.User.GetShortDisplayNameLinkHTML}} {{else}} - {{ctx.AvatarUtils.AvatarByEmail $commit.Commit.Author.Email $userName 18}} - {{$userName}} + {{$gitUserName := $commit.Commit.Author.Name}} + {{ctx.AvatarUtils.AvatarByEmail $commit.Commit.Author.Email $gitUserName 18}} + {{$gitUserName}} {{end}} diff --git a/templates/repo/issue/filter_item_user_assign.tmpl b/templates/repo/issue/filter_item_user_assign.tmpl index 42886edaa0..5ca8a8079c 100644 --- a/templates/repo/issue/filter_item_user_assign.tmpl +++ b/templates/repo/issue/filter_item_user_assign.tmpl @@ -10,7 +10,7 @@ {{$queryLink := .QueryLink}}
{{$.TextFilterTitle}} {{svg "octicon-triangle-down" 14 "dropdown icon"}} -
+
{{svg "octicon-search" 16}} diff --git a/templates/repo/latest_commit.tmpl b/templates/repo/latest_commit.tmpl index cff338949f..c8f1ad5492 100644 --- a/templates/repo/latest_commit.tmpl +++ b/templates/repo/latest_commit.tmpl @@ -2,19 +2,15 @@ {{if not .LatestCommit}} … {{else}} - {{if .LatestCommitUser}} - {{ctx.AvatarUtils.Avatar .LatestCommitUser 24}} - {{if and .LatestCommitUser.FullName DefaultShowFullName}} - {{.LatestCommitUser.FullName}} - {{else}} - {{if .LatestCommit.Author}}{{.LatestCommit.Author.Name}}{{else}}{{.LatestCommitUser.Name}}{{end}} - {{end}} - {{else}} - {{if .LatestCommit.Author}} - {{ctx.AvatarUtils.AvatarByEmail .LatestCommit.Author.Email .LatestCommit.Author.Name 24}} - {{.LatestCommit.Author.Name}} - {{end}} - {{end}} + + {{- if .LatestCommitUser -}} + {{- ctx.AvatarUtils.Avatar .LatestCommitUser 20 "tw-mr-2" -}} + {{.LatestCommitUser.GetShortDisplayNameLinkHTML}} + {{- else if .LatestCommit.Author -}} + {{- ctx.AvatarUtils.AvatarByEmail .LatestCommit.Author.Email .LatestCommit.Author.Name 20 "tw-mr-2" -}} + {{.LatestCommit.Author.Name}} + {{- end -}} + {{template "repo/commit_sign_badge" dict "Commit" .LatestCommit "CommitBaseLink" (print .RepoLink "/commit") "CommitSignVerification" .LatestCommitVerification}} diff --git a/templates/repo/search_name.tmpl b/templates/repo/search_name.tmpl index 951f168866..636c83330f 100644 --- a/templates/repo/search_name.tmpl +++ b/templates/repo/search_name.tmpl @@ -1 +1 @@ -{{.Name}}{{if DefaultShowFullName}} {{.FullName}}{{end}} +{{.Name}} {{if .FullName}}({{.FullName}}){{end}} diff --git a/tests/integration/repo_commits_test.go b/tests/integration/repo_commits_test.go index 461af0db64..3bcea83f43 100644 --- a/tests/integration/repo_commits_test.go +++ b/tests/integration/repo_commits_test.go @@ -7,6 +7,7 @@ import ( "fmt" "net/http" "path" + "strings" "sync" "testing" @@ -37,7 +38,7 @@ func TestRepoCommits(t *testing.T) { doc.doc.Find("#commits-table .commit-id-short").Each(func(i int, s *goquery.Selection) { commits = append(commits, path.Base(s.AttrOr("href", ""))) }) - doc.doc.Find("#commits-table .author-wrapper").Each(func(i int, s *goquery.Selection) { + doc.doc.Find("#commits-table .author-wrapper a").Each(func(i int, s *goquery.Selection) { userHrefs = append(userHrefs, s.AttrOr("href", "")) }) assert.Equal(t, []string{"69554a64c1e6030f051e5c3f94bfbd773cd6a324", "27566bd5738fc8b4e3fef3c5e72cce608537bd95", "5099b81332712fe655e34e8dd63574f503f61811"}, commits) @@ -49,7 +50,7 @@ func TestRepoCommits(t *testing.T) { resp := session.MakeRequest(t, req, http.StatusOK) doc := NewHTMLParser(t, resp.Body) commitHref := doc.doc.Find(".latest-commit .commit-id-short").AttrOr("href", "") - authorHref := doc.doc.Find(".latest-commit .author-wrapper").AttrOr("href", "") + authorHref := doc.doc.Find(".latest-commit .author-wrapper a").AttrOr("href", "") assert.Equal(t, "/user2/repo16/commit/69554a64c1e6030f051e5c3f94bfbd773cd6a324", commitHref) assert.Equal(t, "/user2", authorHref) }) @@ -65,8 +66,7 @@ func TestRepoCommits(t *testing.T) { commitHref := doc.doc.Find("#commits-table tr:first-child .commit-id-short").AttrOr("href", "") assert.Equal(t, "/user2/repo1/commit/985f0301dba5e7b34be866819cd15ad3d8f508ee", commitHref) authorElem := doc.doc.Find("#commits-table tr:first-child .author-wrapper") - assert.Equal(t, "6543", authorElem.Text()) - assert.Equal(t, "span", authorElem.Nodes[0].Data) + assert.Equal(t, "6543", strings.TrimSpace(authorElem.Text())) }) t.Run("LastCommitNonExistingCommiter", func(t *testing.T) { @@ -76,8 +76,7 @@ func TestRepoCommits(t *testing.T) { commitHref := doc.doc.Find(".latest-commit .commit-id-short").AttrOr("href", "") assert.Equal(t, "/user2/repo1/commit/985f0301dba5e7b34be866819cd15ad3d8f508ee", commitHref) authorElem := doc.doc.Find(".latest-commit .author-wrapper") - assert.Equal(t, "6543", authorElem.Text()) - assert.Equal(t, "span", authorElem.Nodes[0].Data) + assert.Equal(t, "6543", strings.TrimSpace(authorElem.Text())) }) } diff --git a/web_src/css/repo.css b/web_src/css/repo.css index 83df3e5c29..3938071bf4 100644 --- a/web_src/css/repo.css +++ b/web_src/css/repo.css @@ -1969,7 +1969,15 @@ tbody.commit-list { padding-right: 0.5em; /* To match the alignment with the "required" label */ } -.search-fullname { +.username-display { + max-width: 100%; /* the inner part might have "gt-ellipsis" */ + min-width: 0; /* if it is the top flex container, "max-width" works; but if it is inside another flex container, the parent needs to handle the x-axis and here also needs "min-width" */ + display: inline-flex; + gap: var(--gap-inline); + align-items: center; +} + +.username-display > .username-fullname { color: var(--color-text-light-2); } diff --git a/web_src/js/features/repo-issue-list.ts b/web_src/js/features/repo-issue-list.ts index 14f7d867b2..c0cd3c7f22 100644 --- a/web_src/js/features/repo-issue-list.ts +++ b/web_src/js/features/repo-issue-list.ts @@ -1,6 +1,6 @@ import {updateIssuesMeta} from './repo-common.ts'; import {toggleElem, queryElems, isElemVisible} from '../utils/dom.ts'; -import {html} from '../utils/html.ts'; +import {html, htmlRaw} from '../utils/html.ts'; import {confirmModal} from './comp/ConfirmModal.ts'; import {showErrorToast} from '../modules/toast.ts'; import {createSortable} from '../modules/sortable.ts'; @@ -138,10 +138,11 @@ function initDropdownUserRemoteSearch(el: Element) { // the content is provided by backend IssuePosters handler processedResults.length = 0; for (const item of resp.results) { - let nameHtml = html`${item.username}`; - if (item.full_name) nameHtml += html`${item.full_name}`; + const htmlAvatar = html``; + const htmlFullName = item.full_name ? html`(${item.full_name})` : ''; + const htmlItem = html`${htmlRaw(htmlAvatar)}${item.username}${htmlRaw(htmlFullName)}`; if (selectedUsername.toLowerCase() === item.username.toLowerCase()) selectedUsername = item.username; - processedResults.push({value: item.username, name: nameHtml}); + processedResults.push({value: item.username, name: htmlItem}); } resp.results = processedResults; return resp; From 5d87bb3d4566e71b791a8114bfc9e25c037ab5fe Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 4 Mar 2026 12:11:42 -0800 Subject: [PATCH 8/8] Add a git grep search timeout (#36809) --- modules/git/grep.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/modules/git/grep.go b/modules/git/grep.go index 051a7a1d40..47f66b88b2 100644 --- a/modules/git/grep.go +++ b/modules/git/grep.go @@ -11,6 +11,7 @@ import ( "slices" "strconv" "strings" + "time" "code.gitea.io/gitea/modules/git/gitcmd" "code.gitea.io/gitea/modules/util" @@ -39,6 +40,10 @@ type GrepOptions struct { PathspecList []string } +// grepSearchTimeout is the timeout for git grep search, it should be long enough to get results +// but not too long to cause performance issues +const grepSearchTimeout = 30 * time.Second + func GrepSearch(ctx context.Context, repo *Repository, search string, opts GrepOptions) ([]*GrepResult, error) { /* The output is like this ( "^@" means \x00): @@ -76,6 +81,7 @@ func GrepSearch(ctx context.Context, repo *Repository, search string, opts GrepO stdoutReader, stdoutReaderClose := cmd.MakeStdoutPipe() defer stdoutReaderClose() err := cmd.WithDir(repo.Path). + WithTimeout(grepSearchTimeout). WithPipelineFunc(func(ctx gitcmd.Context) error { isInBlock := false rd := bufio.NewReaderSize(stdoutReader, util.IfZero(opts.MaxLineLength, 16*1024))