From 34fd3c9f0614182b5a7098af4a53e8e47161c976 Mon Sep 17 00:00:00 2001 From: Nicolas Date: Sat, 16 May 2026 12:06:40 +0200 Subject: [PATCH] feat: Add default PR branch update style setting (#37410) Adds repository-level settings for pull request branch updates so admins can choose the default update method and disable merge or rebase updates. --------- Co-authored-by: OpenAI Codex (GPT-5) Co-authored-by: silverwind Co-authored-by: Claude (Opus 4.7) Co-authored-by: wxiaoguang --- models/repo/git.go | 10 +++ models/repo/pull_request_default_test.go | 80 +++++++++++++++++++ models/repo/repo_unit.go | 34 +++++++- modules/structs/repo.go | 6 ++ options/locale/locale_en-US.json | 2 + routers/api/v1/repo/pull.go | 8 +- routers/api/v1/repo/repo.go | 10 +++ routers/web/repo/issue_view.go | 45 ++++++++++- routers/web/repo/pull.go | 19 +++-- routers/web/repo/setting/setting.go | 50 +++++++++++- services/convert/repository.go | 6 ++ services/forms/repo_form.go | 2 + services/pull/update.go | 54 +++++++------ services/pull/update_test.go | 41 +++++++--- .../view_content/update_branch_by_merge.tmpl | 16 ++-- templates/repo/settings/options.tmpl | 26 +++++- templates/swagger/v1_json.tmpl | 18 +++++ templates/swagger/v1_openapi3_json.tmpl | 18 +++++ tests/integration/api_repo_edit_test.go | 44 ++++++++++ tests/integration/pull_update_test.go | 64 ++++++++++++++- 20 files changed, 493 insertions(+), 60 deletions(-) diff --git a/models/repo/git.go b/models/repo/git.go index 388bf865223..b2e434a076d 100644 --- a/models/repo/git.go +++ b/models/repo/git.go @@ -29,6 +29,16 @@ const ( MergeStyleRebaseUpdate MergeStyle = "rebase-update-only" ) +// UpdateStyle is a pull request branch update style +type UpdateStyle string + +const ( + // UpdateStyleMerge merges the base branch into the pull request branch + UpdateStyleMerge UpdateStyle = "merge" + // UpdateStyleRebase rebases the pull request branch onto the base branch + UpdateStyleRebase UpdateStyle = "rebase" +) + // UpdateDefaultBranch updates the default branch func UpdateDefaultBranch(ctx context.Context, repo *Repository) error { _, err := db.GetEngine(ctx).ID(repo.ID).Cols("default_branch").Update(repo) diff --git a/models/repo/pull_request_default_test.go b/models/repo/pull_request_default_test.go index b1653f2f1ae..9fad9eaad34 100644 --- a/models/repo/pull_request_default_test.go +++ b/models/repo/pull_request_default_test.go @@ -8,6 +8,7 @@ import ( "code.gitea.io/gitea/models/unit" "code.gitea.io/gitea/models/unittest" + "code.gitea.io/gitea/modules/util" "github.com/stretchr/testify/assert" ) @@ -30,3 +31,82 @@ func TestDefaultTargetBranchSelection(t *testing.T) { repo.Units = nil assert.Equal(t, "branch2", repo.GetPullRequestTargetBranch(ctx)) } + +func TestPullRequestConfigFromDB(t *testing.T) { + cases := []struct { + // name describes the row shape under test; the comments capture why each row matters. + name string + json string + wantMergeUpdate bool + wantRebaseUpdate bool + wantDefaultStyle UpdateStyle + wantValidatesPass bool + }{ + { + // Empty object exercises the all-defaults path (e.g. fresh repos created via low-level paths). + name: "defaults", json: "{}", + wantMergeUpdate: true, wantRebaseUpdate: true, + wantDefaultStyle: UpdateStyleMerge, wantValidatesPass: true, + }, + { + // Realistic upgrade case: pre-PR JSON lacks the new fields and has AllowRebaseUpdate=false. + // Historical setting must be preserved while new fields take safe defaults. + name: "legacy without new fields", + json: `{"AllowMerge":true,"AllowRebase":true,"AllowRebaseMerge":true,"AllowSquash":true,"AllowRebaseUpdate":false}`, + wantMergeUpdate: true, wantRebaseUpdate: false, + wantDefaultStyle: UpdateStyleMerge, wantValidatesPass: true, + }, + { + // Partially-migrated row with explicit empty string must normalize so ValidateUpdateSettings passes. + name: "empty default style", json: `{"DefaultUpdateStyle":""}`, + wantMergeUpdate: true, wantRebaseUpdate: true, + wantDefaultStyle: UpdateStyleMerge, wantValidatesPass: true, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + cfg := new(PullRequestsConfig) + assert.NoError(t, cfg.FromDB([]byte(tc.json))) + assert.Equal(t, tc.wantMergeUpdate, cfg.AllowMergeUpdate) + assert.Equal(t, tc.wantRebaseUpdate, cfg.AllowRebaseUpdate) + assert.Equal(t, tc.wantDefaultStyle, cfg.DefaultUpdateStyle) + if tc.wantValidatesPass { + assert.NoError(t, cfg.ValidateUpdateSettings()) + } + }) + } +} + +func TestPullRequestConfigValidateUpdateSettingsInvalidArgument(t *testing.T) { + cases := []struct { + name string + cfg PullRequestsConfig + }{ + { + name: "invalid default style", + cfg: PullRequestsConfig{ + AllowMergeUpdate: true, + AllowRebaseUpdate: true, + DefaultUpdateStyle: "invalid", + }, + }, + { + name: "no update style enabled", + cfg: PullRequestsConfig{ + DefaultUpdateStyle: UpdateStyleMerge, + }, + }, + { + name: "default update style disabled", + cfg: PullRequestsConfig{ + AllowRebaseUpdate: true, + DefaultUpdateStyle: UpdateStyleMerge, + }, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + assert.ErrorIs(t, tc.cfg.ValidateUpdateSettings(), util.ErrInvalidArgument) + }) + } +} diff --git a/models/repo/repo_unit.go b/models/repo/repo_unit.go index c32adbbcd43..e7204923a25 100644 --- a/models/repo/repo_unit.go +++ b/models/repo/repo_unit.go @@ -125,7 +125,9 @@ type PullRequestsConfig struct { AllowFastForwardOnly bool AllowManualMerge bool AutodetectManualMerge bool + AllowMergeUpdate bool AllowRebaseUpdate bool + DefaultUpdateStyle UpdateStyle DefaultDeleteBranchAfterMerge bool DefaultMergeStyle MergeStyle DefaultAllowMaintainerEdit bool @@ -139,7 +141,9 @@ func DefaultPullRequestsConfig() *PullRequestsConfig { AllowRebaseMerge: true, AllowSquash: true, AllowFastForwardOnly: true, + AllowMergeUpdate: true, AllowRebaseUpdate: true, + DefaultUpdateStyle: UpdateStyleMerge, DefaultAllowMaintainerEdit: true, } cfg.DefaultDeleteBranchAfterMerge = setting.Repository.PullRequest.DefaultDeleteBranchAfterMerge @@ -152,7 +156,9 @@ func DefaultPullRequestsConfig() *PullRequestsConfig { func (cfg *PullRequestsConfig) FromDB(bs []byte) error { // set default values for existing PullRequestConfig in DB *cfg = *DefaultPullRequestsConfig() - return json.UnmarshalHandleDoubleEncode(bs, &cfg) + _ = json.UnmarshalHandleDoubleEncode(bs, &cfg) // don't let corrupted database value cause unnecessary 500 error + cfg.DefaultUpdateStyle = util.IfZero(cfg.DefaultUpdateStyle, UpdateStyleMerge) + return nil } // ToDB exports a PullRequestsConfig to a serialized format. @@ -170,6 +176,32 @@ func (cfg *PullRequestsConfig) IsMergeStyleAllowed(mergeStyle MergeStyle) bool { mergeStyle == MergeStyleManuallyMerged && cfg.AllowManualMerge } +// IsUpdateStyleAllowed returns if a pull request branch update style is allowed +func (cfg *PullRequestsConfig) IsUpdateStyleAllowed(updateStyle UpdateStyle) bool { + switch updateStyle { + case UpdateStyleMerge: + return cfg.AllowMergeUpdate + case UpdateStyleRebase: + return cfg.AllowRebaseUpdate + default: + return false + } +} + +// ValidateUpdateSettings checks that the AllowMerge/RebaseUpdate flags and DefaultUpdateStyle are mutually consistent. +func (cfg *PullRequestsConfig) ValidateUpdateSettings() error { + if cfg.DefaultUpdateStyle != UpdateStyleMerge && cfg.DefaultUpdateStyle != UpdateStyleRebase { + return util.NewInvalidArgumentErrorf("default update style must be merge or rebase") + } + if !cfg.AllowMergeUpdate && !cfg.AllowRebaseUpdate { + return util.NewInvalidArgumentErrorf("at least one pull request branch update style must be enabled") + } + if !cfg.IsUpdateStyleAllowed(cfg.DefaultUpdateStyle) { + return util.NewInvalidArgumentErrorf("default update style must be enabled") + } + return nil +} + func DefaultPullRequestsUnit(repoID int64) RepoUnit { return RepoUnit{RepoID: repoID, Type: unit.TypePullRequests, Config: DefaultPullRequestsConfig()} } diff --git a/modules/structs/repo.go b/modules/structs/repo.go index 5626f7e14bf..eca1b15b026 100644 --- a/modules/structs/repo.go +++ b/modules/structs/repo.go @@ -113,11 +113,13 @@ type Repository struct { AllowRebaseMerge bool `json:"allow_rebase_explicit"` AllowSquash bool `json:"allow_squash_merge"` AllowFastForwardOnly bool `json:"allow_fast_forward_only_merge"` + AllowMergeUpdate bool `json:"allow_merge_update"` AllowRebaseUpdate bool `json:"allow_rebase_update"` AllowManualMerge bool `json:"allow_manual_merge"` AutodetectManualMerge bool `json:"autodetect_manual_merge"` DefaultDeleteBranchAfterMerge bool `json:"default_delete_branch_after_merge"` DefaultMergeStyle string `json:"default_merge_style"` + DefaultUpdateStyle string `json:"default_update_style"` DefaultAllowMaintainerEdit bool `json:"default_allow_maintainer_edit"` AvatarURL string `json:"avatar_url"` Internal bool `json:"internal"` @@ -224,12 +226,16 @@ type EditRepoOption struct { AllowManualMerge *bool `json:"allow_manual_merge,omitempty"` // either `true` to enable AutodetectManualMerge, or `false` to prevent it. Note: In some special cases, misjudgments can occur. AutodetectManualMerge *bool `json:"autodetect_manual_merge,omitempty"` + // either `true` to allow updating pull request branch by merge, or `false` to prevent it. + AllowMergeUpdate *bool `json:"allow_merge_update,omitempty"` // either `true` to allow updating pull request branch by rebase, or `false` to prevent it. AllowRebaseUpdate *bool `json:"allow_rebase_update,omitempty"` // set to `true` to delete pr branch after merge by default DefaultDeleteBranchAfterMerge *bool `json:"default_delete_branch_after_merge,omitempty"` // set to a merge style to be used by this repository: "merge", "rebase", "rebase-merge", "squash", or "fast-forward-only". DefaultMergeStyle *string `json:"default_merge_style,omitempty"` + // set to an update style to be used by this repository: "merge" or "rebase". + DefaultUpdateStyle *string `json:"default_update_style,omitempty"` // set to `true` to allow edits from maintainers by default DefaultAllowMaintainerEdit *bool `json:"default_allow_maintainer_edit,omitempty"` // set to `true` to archive this repository. diff --git a/options/locale/locale_en-US.json b/options/locale/locale_en-US.json index e6248944e71..25fdafb5509 100644 --- a/options/locale/locale_en-US.json +++ b/options/locale/locale_en-US.json @@ -2138,7 +2138,9 @@ "repo.settings.pulls_desc": "Enable Repository Pull Requests", "repo.settings.pulls.ignore_whitespace": "Ignore Whitespace for Conflicts", "repo.settings.pulls.enable_autodetect_manual_merge": "Enable autodetect manual merge (Note: In some special cases, misjudgments can occur)", + "repo.settings.pulls.allow_merge_update": "Enable updating pull request branch by merge", "repo.settings.pulls.allow_rebase_update": "Enable updating pull request branch by rebase", + "repo.settings.pulls.default_update_style": "Default branch update style", "repo.settings.pulls.default_target_branch": "Default target branch for new pull requests", "repo.settings.pulls.default_target_branch_default": "Default branch (%s)", "repo.settings.pulls.default_delete_branch_after_merge": "Delete pull request branch after merge by default", diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 2702f6864fa..d20fc702a9f 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -1253,15 +1253,17 @@ func UpdatePullRequest(ctx *context.APIContext) { return } - rebase := ctx.FormString("style") == "rebase" + // keep API back-compat: when no style is given, default to "merge" rather than the repo's DefaultUpdateStyle, + // so existing API clients keep getting a merge update. + rebase := repo_model.UpdateStyle(ctx.FormString("style", string(repo_model.UpdateStyleMerge))) == repo_model.UpdateStyleRebase - allowedUpdateByMerge, allowedUpdateByRebase, err := pull_service.IsUserAllowedToUpdate(ctx, pr, ctx.Doer) + userUpdateStyles, err := pull_service.CheckUserAllowedToUpdate(ctx, pr, ctx.Doer) if err != nil { ctx.APIErrorInternal(err) return } - if (!allowedUpdateByMerge && !rebase) || (rebase && !allowedUpdateByRebase) { + if (rebase && !userUpdateStyles.RebaseAllowed) || (!rebase && !userUpdateStyles.MergeAllowed) { ctx.Status(http.StatusForbidden) return } diff --git a/routers/api/v1/repo/repo.go b/routers/api/v1/repo/repo.go index 361a80abcc0..12e525464c8 100644 --- a/routers/api/v1/repo/repo.go +++ b/routers/api/v1/repo/repo.go @@ -885,10 +885,20 @@ func updateRepoUnits(ctx *context.APIContext, opts api.EditRepoOption) error { 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.AllowMergeUpdate, opts.AllowMergeUpdate) 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) + optional.AssignPtrString(changed, &config.DefaultUpdateStyle, opts.DefaultUpdateStyle) + // only validate update-style fields when the caller is actually changing one of them, + // so unrelated PATCH calls don't reject historical configs. + if opts.AllowMergeUpdate != nil || opts.AllowRebaseUpdate != nil || opts.DefaultUpdateStyle != nil { + if err := config.ValidateUpdateSettings(); err != nil { + ctx.APIError(http.StatusUnprocessableEntity, err) + return err + } + } if *changed || mustInsertPullRequestUnit { units = append(units, repo_model.RepoUnit{ RepoID: repo.ID, diff --git a/routers/web/repo/issue_view.go b/routers/web/repo/issue_view.go index c0cb77f497c..1a42884ef89 100644 --- a/routers/web/repo/issue_view.go +++ b/routers/web/repo/issue_view.go @@ -867,10 +867,8 @@ func (prInfo *pullRequestViewInfo) prepareMergeBox(ctx *context.Context, issue * if !prInfo.IsPullRequestBroken { data.ShowUpdatePullInfo = pull.CommitsBehind > 0 && !issue.IsClosed && !pull.IsChecking() && !pull.IsFilesConflicted() && !prInfo.IsPullRequestBroken - var err error - data.UpdateAllowed, data.UpdateByRebaseAllowed, err = pull_service.IsUserAllowedToUpdate(ctx, pull, ctx.Doer) - if err != nil { - ctx.ServerError("IsUserAllowedToUpdate", err) + prInfo.preparePullUpdateActions(ctx) + if ctx.Written() { return } } @@ -975,6 +973,45 @@ func (prInfo *pullRequestViewInfo) prepareMergeBox(ctx *context.Context, issue * ctx.Data["PullMergeBoxData"] = prInfo.MergeBoxData } +func (prInfo *pullRequestViewInfo) preparePullUpdateActions(ctx *context.Context) { + pull := prInfo.issue.PullRequest + data := prInfo.MergeBoxData + userUpdateStyles, err := pull_service.CheckUserAllowedToUpdate(ctx, pull, ctx.Doer) + if err != nil { + ctx.ServerError("IsUserAllowedToUpdate", err) + return + } + if !userUpdateStyles.MergeAllowed && !userUpdateStyles.RebaseAllowed { + return + } + + issueLink := prInfo.issue.Link() + mergeAction := &pullUpdateAction{ + URL: issueLink + "/update?style=merge", + Text: ctx.Tr("repo.pulls.update_branch"), + } + rebaseAction := &pullUpdateAction{ + URL: issueLink + "/update?style=rebase", + Text: ctx.Tr("repo.pulls.update_branch_rebase"), + } + + if userUpdateStyles.MergeAllowed { + data.UpdateStyleOptions = append(data.UpdateStyleOptions, mergeAction) + } + if userUpdateStyles.RebaseAllowed { + data.UpdateStyleOptions = append(data.UpdateStyleOptions, rebaseAction) + } + + if userUpdateStyles.DefaultUpdateStyle == repo_model.UpdateStyleRebase && userUpdateStyles.RebaseAllowed { + data.UpdatePrimaryAction = rebaseAction + } else if userUpdateStyles.DefaultUpdateStyle == repo_model.UpdateStyleMerge && userUpdateStyles.MergeAllowed { + data.UpdatePrimaryAction = mergeAction + } else { + data.UpdatePrimaryAction = data.UpdateStyleOptions[0] + } + data.UpdatePrimaryAction.Selected = true +} + func (prInfo *pullRequestViewInfo) prepareMergeBoxProtectionChecks(ctx *context.Context) { pb, err := git_model.GetFirstMatchProtectedBranchRule(ctx, ctx.Repo.Repository.ID, prInfo.issue.PullRequest.BaseBranch) if err != nil { diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index d20bbdc36cb..bf96dbd2bea 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -280,9 +280,9 @@ type pullMergeBoxData struct { CanMergeNow bool // PR is mergeable, either no blocker, or doer is admin and can bypass the blockers allowMerge bool // doer has permission to merge - ShowUpdatePullInfo bool - UpdateAllowed bool - UpdateByRebaseAllowed bool + ShowUpdatePullInfo bool + UpdatePrimaryAction *pullUpdateAction + UpdateStyleOptions []*pullUpdateAction MergeFormProps map[string]any ShowPullCommands bool @@ -308,6 +308,12 @@ type pullMergeBoxData struct { InfoSections []*pullInfoSection } +type pullUpdateAction struct { + URL string + Text template.HTML + Selected bool +} + // pullRequestViewInfo is a structured type for viewing pull request // Refactoring plan: // * move dynamic template-data-based variable into this struct @@ -957,8 +963,6 @@ func UpdatePullRequest(ctx *context.Context) { return } - rebase := ctx.FormString("style") == "rebase" - if err := issue.PullRequest.LoadBaseRepo(ctx); err != nil { ctx.ServerError("LoadBaseRepo", err) return @@ -968,13 +972,14 @@ func UpdatePullRequest(ctx *context.Context) { return } - allowedUpdateByMerge, allowedUpdateByRebase, err := pull_service.IsUserAllowedToUpdate(ctx, issue.PullRequest, ctx.Doer) + userUpdateStyles, err := pull_service.CheckUserAllowedToUpdate(ctx, issue.PullRequest, ctx.Doer) if err != nil { ctx.ServerError("IsUserAllowedToMerge", err) return } - if (!allowedUpdateByMerge && !rebase) || (rebase && !allowedUpdateByRebase) { + rebase := ctx.FormString("style", string(userUpdateStyles.DefaultUpdateStyle)) == string(repo_model.UpdateStyleRebase) + if (rebase && !userUpdateStyles.RebaseAllowed) || (!rebase && !userUpdateStyles.MergeAllowed) { ctx.JSONError(ctx.Tr("repo.pulls.update_not_allowed")) return } diff --git a/routers/web/repo/setting/setting.go b/routers/web/repo/setting/setting.go index 816fd91cd8b..a9276f1bc00 100644 --- a/routers/web/repo/setting/setting.go +++ b/routers/web/repo/setting/setting.go @@ -6,6 +6,7 @@ package setting import ( "errors" + "html/template" "net/http" "strings" "time" @@ -49,6 +50,12 @@ const ( tplDeployKeys templates.TplName = "repo/settings/deploy_keys" ) +type selectOption struct { + Value string + Text template.HTML + Selected bool +} + // SettingsCtxData is a middleware that sets all the general context data for the // settings template. func SettingsCtxData(ctx *context.Context) { @@ -66,6 +73,7 @@ func SettingsCtxData(ctx *context.Context) { ctx.Data["SigningKeyAvailable"] = signing != nil ctx.Data["SigningSettings"] = setting.Repository.Signing ctx.Data["IsRepoIndexerEnabled"] = setting.Indexer.RepoIndexerEnabled + preparePullRequestSettings(ctx) if ctx.Doer.IsAdmin { if setting.Indexer.RepoIndexerEnabled { @@ -96,6 +104,35 @@ func SettingsCtxData(ctx *context.Context) { } } +func preparePullRequestSettings(ctx *context.Context) { + defaultUpdateStyle := repo_model.UpdateStyleMerge + if ctx.Repo.Repository.UnitEnabled(ctx, unit_model.TypePullRequests) { + prUnit := ctx.Repo.Repository.MustGetUnit(ctx, unit_model.TypePullRequests) + defaultUpdateStyle = util.IfZero(prUnit.PullRequestsConfig().DefaultUpdateStyle, repo_model.UpdateStyleMerge) + } + + updateBranchText := ctx.Tr("repo.pulls.update_branch") + rebaseUpdateText := ctx.Tr("repo.pulls.update_branch_rebase") + defaultUpdateStyleText := updateBranchText + if defaultUpdateStyle == repo_model.UpdateStyleRebase { + defaultUpdateStyleText = rebaseUpdateText + } + + ctx.Data["PullsDefaultUpdateStyleText"] = defaultUpdateStyleText + ctx.Data["PullsDefaultUpdateStyleOptions"] = []selectOption{ + { + Value: string(repo_model.UpdateStyleMerge), + Text: updateBranchText, + Selected: defaultUpdateStyle == repo_model.UpdateStyleMerge, + }, + { + Value: string(repo_model.UpdateStyleRebase), + Text: rebaseUpdateText, + Selected: defaultUpdateStyle == repo_model.UpdateStyleRebase, + }, + } +} + // Settings show a repository's settings page func Settings(ctx *context.Context) { ctx.HTML(http.StatusOK, tplSettingsOptions) @@ -616,7 +653,8 @@ func handleSettingsPostAdvanced(ctx *context.Context) { } if form.EnablePulls && !unit_model.TypePullRequests.UnitGlobalDisabled() { - units = append(units, newRepoUnit(repo, unit_model.TypePullRequests, &repo_model.PullRequestsConfig{ + defaultUpdateStyle := util.IfZero(repo_model.UpdateStyle(form.PullsDefaultUpdateStyle), repo_model.UpdateStyleMerge) + prConfig := &repo_model.PullRequestsConfig{ IgnoreWhitespaceConflicts: form.PullsIgnoreWhitespace, AllowMerge: form.PullsAllowMerge, AllowRebase: form.PullsAllowRebase, @@ -625,12 +663,20 @@ func handleSettingsPostAdvanced(ctx *context.Context) { AllowFastForwardOnly: form.PullsAllowFastForwardOnly, AllowManualMerge: form.PullsAllowManualMerge, AutodetectManualMerge: form.EnableAutodetectManualMerge, + AllowMergeUpdate: form.PullsAllowMergeUpdate, AllowRebaseUpdate: form.PullsAllowRebaseUpdate, + DefaultUpdateStyle: defaultUpdateStyle, DefaultDeleteBranchAfterMerge: form.DefaultDeleteBranchAfterMerge, DefaultMergeStyle: repo_model.MergeStyle(form.PullsDefaultMergeStyle), DefaultAllowMaintainerEdit: form.DefaultAllowMaintainerEdit, DefaultTargetBranch: strings.TrimSpace(form.DefaultTargetBranch), - })) + } + if err := prConfig.ValidateUpdateSettings(); err != nil { + ctx.Flash.Error(err.Error()) + ctx.Redirect(repo.Link() + "/settings") + return + } + units = append(units, newRepoUnit(repo, unit_model.TypePullRequests, prConfig)) } else if !unit_model.TypePullRequests.UnitGlobalDisabled() { deleteUnitTypes = append(deleteUnitTypes, unit_model.TypePullRequests) } diff --git a/services/convert/repository.go b/services/convert/repository.go index ec010ee3750..a32402fae10 100644 --- a/services/convert/repository.go +++ b/services/convert/repository.go @@ -98,11 +98,13 @@ func innerToRepo(ctx context.Context, repo *repo_model.Repository, permissionInR allowRebaseMerge := false allowSquash := false allowFastForwardOnly := false + allowMergeUpdate := false allowRebaseUpdate := false allowManualMerge := true autodetectManualMerge := false defaultDeleteBranchAfterMerge := false defaultMergeStyle := repo_model.MergeStyleMerge + defaultUpdateStyle := repo_model.UpdateStyleMerge defaultAllowMaintainerEdit := false defaultTargetBranch := "" if unit, err := repo.GetUnit(ctx, unit_model.TypePullRequests); err == nil { @@ -114,11 +116,13 @@ func innerToRepo(ctx context.Context, repo *repo_model.Repository, permissionInR allowRebaseMerge = config.AllowRebaseMerge allowSquash = config.AllowSquash allowFastForwardOnly = config.AllowFastForwardOnly + allowMergeUpdate = config.AllowMergeUpdate allowRebaseUpdate = config.AllowRebaseUpdate allowManualMerge = config.AllowManualMerge autodetectManualMerge = config.AutodetectManualMerge defaultDeleteBranchAfterMerge = config.DefaultDeleteBranchAfterMerge defaultMergeStyle = config.DefaultMergeStyle + defaultUpdateStyle = config.DefaultUpdateStyle defaultAllowMaintainerEdit = config.DefaultAllowMaintainerEdit defaultTargetBranch = config.DefaultTargetBranch } @@ -240,11 +244,13 @@ func innerToRepo(ctx context.Context, repo *repo_model.Repository, permissionInR AllowRebaseMerge: allowRebaseMerge, AllowSquash: allowSquash, AllowFastForwardOnly: allowFastForwardOnly, + AllowMergeUpdate: allowMergeUpdate, AllowRebaseUpdate: allowRebaseUpdate, AllowManualMerge: allowManualMerge, AutodetectManualMerge: autodetectManualMerge, DefaultDeleteBranchAfterMerge: defaultDeleteBranchAfterMerge, DefaultMergeStyle: string(defaultMergeStyle), + DefaultUpdateStyle: string(defaultUpdateStyle), DefaultAllowMaintainerEdit: defaultAllowMaintainerEdit, DefaultTargetBranch: defaultTargetBranch, AvatarURL: repo.AvatarLink(ctx), diff --git a/services/forms/repo_form.go b/services/forms/repo_form.go index d8e019f8609..9c60e44fc5b 100644 --- a/services/forms/repo_form.go +++ b/services/forms/repo_form.go @@ -142,7 +142,9 @@ type RepoSettingForm struct { PullsAllowManualMerge bool PullsDefaultMergeStyle string EnableAutodetectManualMerge bool + PullsAllowMergeUpdate bool PullsAllowRebaseUpdate bool + PullsDefaultUpdateStyle string DefaultDeleteBranchAfterMerge bool DefaultAllowMaintainerEdit bool DefaultTargetBranch string diff --git a/services/pull/update.go b/services/pull/update.go index 4033e11e2bc..46fe2d82e4a 100644 --- a/services/pull/update.go +++ b/services/pull/update.go @@ -131,74 +131,82 @@ func isUserAllowedToPushOrForcePushInRepoBranch(ctx context.Context, user *user_ return pushAllowed, forcePushAllowed, nil } -// IsUserAllowedToUpdate check if user is allowed to update PR with given permissions and branch protections +// CheckUserAllowedToUpdate check if user is allowed to update PR with given permissions and branch protections // update PR means send new commits to PR head branch from base branch -func IsUserAllowedToUpdate(ctx context.Context, pull *issues_model.PullRequest, user *user_model.User) (pushAllowed, rebaseAllowed bool, err error) { +func CheckUserAllowedToUpdate(ctx context.Context, pull *issues_model.PullRequest, user *user_model.User) (ret struct { + MergeAllowed, RebaseAllowed bool + DefaultUpdateStyle repo_model.UpdateStyle +}, err error, +) { if user == nil { - return false, false, nil + return ret, nil } if err := pull.LoadBaseRepo(ctx); err != nil { - return false, false, err + return ret, err } if err := pull.LoadHeadRepo(ctx); err != nil { - return false, false, err + return ret, err } // 1. check whether pull request enabled. prBaseUnit, err := pull.BaseRepo.GetUnit(ctx, unit.TypePullRequests) if repo_model.IsErrUnitTypeNotExist(err) { - return false, false, nil // the PR unit is disabled in base repo means no update allowed + return ret, nil // the PR unit is disabled in base repo means no update allowed } else if err != nil { - return false, false, fmt.Errorf("get base repo unit: %v", err) + return ret, fmt.Errorf("get base repo unit: %v", err) } // 2. only support Github style pull request if pull.Flow == issues_model.PullRequestFlowAGit { - return false, false, nil + return ret, nil } // 3. check user push permission on head repository - pushAllowed, rebaseAllowed, err = isUserAllowedToPushOrForcePushInRepoBranch(ctx, user, pull.HeadRepo, pull.HeadBranch) + ret.MergeAllowed, ret.RebaseAllowed, err = isUserAllowedToPushOrForcePushInRepoBranch(ctx, user, pull.HeadRepo, pull.HeadBranch) if err != nil { - return false, false, err + return ret, err } // 4. if the pull creator allows maintainer to edit, we need to check whether // user is a maintainer (has permission to merge into base branch) and inherit pull request poster's permission - if pull.AllowMaintainerEdit && (!pushAllowed || !rebaseAllowed) { + if pull.AllowMaintainerEdit && (!ret.MergeAllowed || !ret.RebaseAllowed) { baseRepoPerm, err := access_model.GetDoerRepoPermission(ctx, pull.BaseRepo, user) if err != nil { - return false, false, err + return ret, err } userAllowedToMergePR, err := isUserAllowedToMergeInRepoBranch(ctx, pull.BaseRepoID, pull.BaseBranch, baseRepoPerm, user) if err != nil { - return false, false, err + return ret, err } if userAllowedToMergePR { // the user is maintainer (can merge PR), and this PR is allowed to be edited by maintainers, // then the user should inherit the PR poster's push/rebase permission for the head branch if err := pull.LoadIssue(ctx); err != nil { - return false, false, err + return ret, err } if err := pull.Issue.LoadPoster(ctx); err != nil { - return false, false, err + return ret, err } posterPushAllowed, posterRebaseAllowed, err := isUserAllowedToPushOrForcePushInRepoBranch(ctx, pull.Issue.Poster, pull.HeadRepo, pull.HeadBranch) if err != nil { - return false, false, err + return ret, err } - if !pushAllowed { - pushAllowed = posterPushAllowed + if !ret.MergeAllowed { + ret.MergeAllowed = posterPushAllowed } - if !rebaseAllowed { - rebaseAllowed = posterRebaseAllowed + if !ret.RebaseAllowed { + ret.RebaseAllowed = posterRebaseAllowed } } } - // 5. check base repository's AllowRebaseUpdate configuration - // it is a config in base repo but controls the head (fork) repo's "Update" behavior - return pushAllowed, rebaseAllowed && prBaseUnit.PullRequestsConfig().AllowRebaseUpdate, nil + // 5. apply base repository's update configuration; it is a config on the base repo, + // but it controls the head (fork) repo's "Update" behavior. + prConfig := prBaseUnit.PullRequestsConfig() + ret.MergeAllowed = ret.MergeAllowed && prConfig.AllowMergeUpdate + ret.RebaseAllowed = ret.RebaseAllowed && prConfig.AllowRebaseUpdate + ret.DefaultUpdateStyle = prConfig.DefaultUpdateStyle + return ret, nil } func syncCommitDivergence(ctx context.Context, pr *issues_model.PullRequest) error { diff --git a/services/pull/update_test.go b/services/pull/update_test.go index 0bb67544455..a0bd6f11900 100644 --- a/services/pull/update_test.go +++ b/services/pull/update_test.go @@ -4,6 +4,7 @@ package pull import ( + "context" "testing" "code.gitea.io/gitea/models/db" @@ -23,11 +24,21 @@ import ( func TestIsUserAllowedToUpdate(t *testing.T) { require.NoError(t, unittest.PrepareTestDatabase()) - setRepoAllowRebaseUpdate := func(t *testing.T, repoID int64, allow bool) { + updatePRConfig := func(t *testing.T, repoID int64, update func(*repo_model.PullRequestsConfig)) { repoUnit := unittest.AssertExistsAndLoadBean(t, &repo_model.RepoUnit{RepoID: repoID, Type: unit.TypePullRequests}) - repoUnit.PullRequestsConfig().AllowRebaseUpdate = allow + update(repoUnit.PullRequestsConfig()) require.NoError(t, repo_model.UpdateRepoUnitConfig(t.Context(), repoUnit)) } + setRepoAllowRebaseUpdate := func(t *testing.T, repoID int64, allow bool) { + updatePRConfig(t, repoID, func(c *repo_model.PullRequestsConfig) { c.AllowRebaseUpdate = allow }) + } + setRepoAllowMergeUpdate := func(t *testing.T, repoID int64, allow bool) { + updatePRConfig(t, repoID, func(c *repo_model.PullRequestsConfig) { c.AllowMergeUpdate = allow }) + } + checkUserAllowedToUpdate := func(ctx context.Context, pull *issues_model.PullRequest, user *user_model.User) (bool, bool, repo_model.UpdateStyle, error) { + ret, err := CheckUserAllowedToUpdate(ctx, pull, user) + return ret.MergeAllowed, ret.RebaseAllowed, ret.DefaultUpdateStyle, err + } user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) @@ -43,21 +54,33 @@ func TestIsUserAllowedToUpdate(t *testing.T) { require.NoError(t, err) defer db.DeleteByBean(t.Context(), protectedBranch) - pushAllowed, rebaseAllowed, err := IsUserAllowedToUpdate(t.Context(), pr2, user2) + pushAllowed, rebaseAllowed, defaultMergeStyle, err := checkUserAllowedToUpdate(t.Context(), pr2, user2) assert.NoError(t, err) assert.False(t, pushAllowed) assert.False(t, rebaseAllowed) + assert.Equal(t, repo_model.UpdateStyleMerge, defaultMergeStyle) }) t.Run("DisallowRebaseWhenConfigDisabled", func(t *testing.T) { pr2 := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2}) setRepoAllowRebaseUpdate(t, pr2.BaseRepoID, false) - pushAllowed, rebaseAllowed, err := IsUserAllowedToUpdate(t.Context(), pr2, user2) + pushAllowed, rebaseAllowed, _, err := checkUserAllowedToUpdate(t.Context(), pr2, user2) assert.NoError(t, err) assert.True(t, pushAllowed) assert.False(t, rebaseAllowed) }) + t.Run("DisallowMergeWhenConfigDisabled", func(t *testing.T) { + pr2 := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2}) + setRepoAllowRebaseUpdate(t, pr2.BaseRepoID, true) + setRepoAllowMergeUpdate(t, pr2.BaseRepoID, false) + pushAllowed, rebaseAllowed, _, err := checkUserAllowedToUpdate(t.Context(), pr2, user2) + assert.NoError(t, err) + assert.False(t, pushAllowed) + assert.True(t, rebaseAllowed) + setRepoAllowMergeUpdate(t, pr2.BaseRepoID, true) + }) + t.Run("ReadOnlyAccessDenied", func(t *testing.T) { pr2 := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2}) user4 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4}) @@ -73,7 +96,7 @@ func TestIsUserAllowedToUpdate(t *testing.T) { require.NoError(t, pr2.LoadHeadRepo(t.Context())) assert.NoError(t, access_model.RecalculateUserAccess(t.Context(), pr2.HeadRepo, user4.ID)) - pushAllowed, rebaseAllowed, err := IsUserAllowedToUpdate(t.Context(), pr2, user4) + pushAllowed, rebaseAllowed, _, err := checkUserAllowedToUpdate(t.Context(), pr2, user4) assert.NoError(t, err) assert.False(t, pushAllowed) assert.False(t, rebaseAllowed) @@ -91,7 +114,7 @@ func TestIsUserAllowedToUpdate(t *testing.T) { require.NoError(t, err) defer db.DeleteByBean(t.Context(), protectedBranch) - pushAllowed, rebaseAllowed, err := IsUserAllowedToUpdate(t.Context(), pr2, user2) + pushAllowed, rebaseAllowed, _, err := checkUserAllowedToUpdate(t.Context(), pr2, user2) assert.NoError(t, err) assert.True(t, pushAllowed) assert.False(t, rebaseAllowed) @@ -102,7 +125,7 @@ func TestIsUserAllowedToUpdate(t *testing.T) { t.Run("MaintainerEditRespectsPosterPermissions", func(t *testing.T) { pr3 := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 3}) pr3.AllowMaintainerEdit = true - pushAllowed, rebaseAllowed, err := IsUserAllowedToUpdate(t.Context(), pr3, pr3Poster) + pushAllowed, rebaseAllowed, _, err := checkUserAllowedToUpdate(t.Context(), pr3, pr3Poster) assert.NoError(t, err) assert.False(t, pushAllowed) assert.False(t, rebaseAllowed) @@ -132,7 +155,7 @@ func TestIsUserAllowedToUpdate(t *testing.T) { require.NoError(t, pr3.LoadHeadRepo(t.Context())) assert.NoError(t, access_model.RecalculateUserAccess(t.Context(), pr3.HeadRepo, pr3Poster.ID)) - pushAllowed, rebaseAllowed, err := IsUserAllowedToUpdate(t.Context(), pr3, pr3Poster) + pushAllowed, rebaseAllowed, _, err := checkUserAllowedToUpdate(t.Context(), pr3, pr3Poster) assert.NoError(t, err) assert.True(t, pushAllowed) assert.True(t, rebaseAllowed) @@ -164,7 +187,7 @@ func TestIsUserAllowedToUpdate(t *testing.T) { setRepoAllowRebaseUpdate(t, pr3.BaseRepoID, false) - pushAllowed, rebaseAllowed, err := IsUserAllowedToUpdate(t.Context(), pr3, pr3Poster) + pushAllowed, rebaseAllowed, _, err := checkUserAllowedToUpdate(t.Context(), pr3, pr3Poster) assert.NoError(t, err) assert.True(t, pushAllowed) assert.False(t, rebaseAllowed) diff --git a/templates/repo/issue/view_content/update_branch_by_merge.tmpl b/templates/repo/issue/view_content/update_branch_by_merge.tmpl index fcdf65b74d5..975133469ae 100644 --- a/templates/repo/issue/view_content/update_branch_by_merge.tmpl +++ b/templates/repo/issue/view_content/update_branch_by_merge.tmpl @@ -1,21 +1,21 @@ {{$data := $.MergeBoxData}} -{{$issueLink := $.IssueLink}}
{{svg "octicon-alert"}} {{ctx.Locale.Tr "repo.pulls.outdated_with_base_branch"}}
- {{if $data.UpdateAllowed}} -
- - {{if $data.UpdateByRebaseAllowed}} + {{if gt (len $data.UpdateStyleOptions) 1}}
{{svg "octicon-triangle-down"}}
- {{ctx.Locale.Tr "repo.pulls.update_branch"}} - {{ctx.Locale.Tr "repo.pulls.update_branch_rebase"}} + {{range $data.UpdateStyleOptions}} + {{.Text}} + {{end}}
{{end}} diff --git a/templates/repo/settings/options.tmpl b/templates/repo/settings/options.tmpl index 24942517253..d4491e58d84 100644 --- a/templates/repo/settings/options.tmpl +++ b/templates/repo/settings/options.tmpl @@ -617,10 +617,34 @@
- + + +
+
+
+
+
+
+ +
+ {{svg "octicon-triangle-down" 14 "dropdown icon"}} +
+ {{.PullsDefaultUpdateStyleText}} +
+
+ {{range .PullsDefaultUpdateStyleOptions}} +
{{.Text}}
+ {{end}} +
+
+
diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 3c4be0d9ac7..bed3a32203b 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -25451,6 +25451,11 @@ "type": "boolean", "x-go-name": "AllowMerge" }, + "allow_merge_update": { + "description": "either `true` to allow updating pull request branch by merge, or `false` to prevent it.", + "type": "boolean", + "x-go-name": "AllowMergeUpdate" + }, "allow_rebase": { "description": "either `true` to allow rebase-merging pull requests, or `false` to prevent rebase-merging.", "type": "boolean", @@ -25501,6 +25506,11 @@ "type": "string", "x-go-name": "DefaultMergeStyle" }, + "default_update_style": { + "description": "set to an update style to be used by this repository: \"merge\" or \"rebase\".", + "type": "string", + "x-go-name": "DefaultUpdateStyle" + }, "description": { "description": "a short description of the repository.", "type": "string", @@ -28926,6 +28936,10 @@ "type": "boolean", "x-go-name": "AllowMerge" }, + "allow_merge_update": { + "type": "boolean", + "x-go-name": "AllowMergeUpdate" + }, "allow_rebase": { "type": "boolean", "x-go-name": "AllowRebase" @@ -28993,6 +29007,10 @@ "type": "string", "x-go-name": "DefaultTargetBranch" }, + "default_update_style": { + "type": "string", + "x-go-name": "DefaultUpdateStyle" + }, "description": { "type": "string", "x-go-name": "Description" diff --git a/templates/swagger/v1_openapi3_json.tmpl b/templates/swagger/v1_openapi3_json.tmpl index b337bb4b548..f96418a8996 100644 --- a/templates/swagger/v1_openapi3_json.tmpl +++ b/templates/swagger/v1_openapi3_json.tmpl @@ -5619,6 +5619,11 @@ "type": "boolean", "x-go-name": "AllowMerge" }, + "allow_merge_update": { + "description": "either `true` to allow updating pull request branch by merge, or `false` to prevent it.", + "type": "boolean", + "x-go-name": "AllowMergeUpdate" + }, "allow_rebase": { "description": "either `true` to allow rebase-merging pull requests, or `false` to prevent rebase-merging.", "type": "boolean", @@ -5669,6 +5674,11 @@ "type": "string", "x-go-name": "DefaultMergeStyle" }, + "default_update_style": { + "description": "set to an update style to be used by this repository: \"merge\" or \"rebase\".", + "type": "string", + "x-go-name": "DefaultUpdateStyle" + }, "description": { "description": "a short description of the repository.", "type": "string", @@ -9115,6 +9125,10 @@ "type": "boolean", "x-go-name": "AllowMerge" }, + "allow_merge_update": { + "type": "boolean", + "x-go-name": "AllowMergeUpdate" + }, "allow_rebase": { "type": "boolean", "x-go-name": "AllowRebase" @@ -9184,6 +9198,10 @@ "type": "string", "x-go-name": "DefaultTargetBranch" }, + "default_update_style": { + "type": "string", + "x-go-name": "DefaultUpdateStyle" + }, "description": { "type": "string", "x-go-name": "Description" diff --git a/tests/integration/api_repo_edit_test.go b/tests/integration/api_repo_edit_test.go index e37b214fa88..b1d0d64f80a 100644 --- a/tests/integration/api_repo_edit_test.go +++ b/tests/integration/api_repo_edit_test.go @@ -18,6 +18,7 @@ import ( "code.gitea.io/gitea/modules/gitrepo" api "code.gitea.io/gitea/modules/structs" mirror_service "code.gitea.io/gitea/services/mirror" + "code.gitea.io/gitea/tests" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -69,6 +70,9 @@ func getRepoEditOptionFromRepo(repo *repo_model.Repository) *api.EditRepoOption allowRebaseMerge := false allowSquash := false allowFastForwardOnly := false + allowMergeUpdate := false + allowRebaseUpdate := false + defaultUpdateStyle := string(repo_model.UpdateStyleMerge) if unit, err := repo.GetUnit(ctx, unit_model.TypePullRequests); err == nil { config := unit.PullRequestsConfig() hasPullRequests = true @@ -78,6 +82,9 @@ func getRepoEditOptionFromRepo(repo *repo_model.Repository) *api.EditRepoOption allowRebaseMerge = config.AllowRebaseMerge allowSquash = config.AllowSquash allowFastForwardOnly = config.AllowFastForwardOnly + allowMergeUpdate = config.AllowMergeUpdate + allowRebaseUpdate = config.AllowRebaseUpdate + defaultUpdateStyle = string(config.DefaultUpdateStyle) } archived := repo.IsArchived hasProjects := false @@ -122,6 +129,9 @@ func getRepoEditOptionFromRepo(repo *repo_model.Repository) *api.EditRepoOption AllowRebaseMerge: &allowRebaseMerge, AllowSquash: &allowSquash, AllowFastForwardOnly: &allowFastForwardOnly, + AllowMergeUpdate: &allowMergeUpdate, + AllowRebaseUpdate: &allowRebaseUpdate, + DefaultUpdateStyle: &defaultUpdateStyle, Archived: &archived, } } @@ -148,6 +158,9 @@ func getNewRepoEditOption(opts *api.EditRepoOption) *api.EditRepoOption { allowRebase := !*opts.AllowRebase allowRebaseMerge := !*opts.AllowRebaseMerge allowSquash := !*opts.AllowSquash + allowMergeUpdate := false + allowRebaseUpdate := true + defaultUpdateStyle := string(repo_model.UpdateStyleRebase) archived := !*opts.Archived return &api.EditRepoOption{ @@ -169,6 +182,9 @@ func getNewRepoEditOption(opts *api.EditRepoOption) *api.EditRepoOption { AllowRebase: &allowRebase, AllowRebaseMerge: &allowRebaseMerge, AllowSquash: &allowSquash, + AllowMergeUpdate: &allowMergeUpdate, + AllowRebaseUpdate: &allowRebaseUpdate, + DefaultUpdateStyle: &defaultUpdateStyle, Archived: &archived, } } @@ -488,3 +504,31 @@ func TestAPIRepoEdit(t *testing.T) { assert.Equal(t, token, password) }) } + +func TestAPIRepoEditPullUpdateSettingsValidation(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) + + session := loginUser(t, user2.Name) + token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) + repoURL := fmt.Sprintf("/api/v1/repos/%s/%s", user2.Name, repo1.Name) + + allowMergeUpdate := false + allowRebaseUpdate := false + req := NewRequestWithJSON(t, "PATCH", repoURL, &api.EditRepoOption{ + AllowMergeUpdate: &allowMergeUpdate, + AllowRebaseUpdate: &allowRebaseUpdate, + }).AddTokenAuth(token) + MakeRequest(t, req, http.StatusUnprocessableEntity) + + allowRebaseUpdate = true + defaultUpdateStyle := string(repo_model.UpdateStyleMerge) + req = NewRequestWithJSON(t, "PATCH", repoURL, &api.EditRepoOption{ + AllowMergeUpdate: &allowMergeUpdate, + AllowRebaseUpdate: &allowRebaseUpdate, + DefaultUpdateStyle: &defaultUpdateStyle, + }).AddTokenAuth(token) + MakeRequest(t, req, http.StatusUnprocessableEntity) +} diff --git a/tests/integration/pull_update_test.go b/tests/integration/pull_update_test.go index 73262af9689..bbe0077fe52 100644 --- a/tests/integration/pull_update_test.go +++ b/tests/integration/pull_update_test.go @@ -61,14 +61,34 @@ func TestAPIPullUpdate(t *testing.T) { }) } -func enableRepoAllowUpdateWithRebase(t *testing.T, repoID int64, allow bool) { +func updateRepoPullRequestConfig(t *testing.T, repoID int64, update func(*repo_model.PullRequestsConfig)) { t.Helper() repoUnit := unittest.AssertExistsAndLoadBean(t, &repo_model.RepoUnit{RepoID: repoID, Type: unit.TypePullRequests}) - repoUnit.PullRequestsConfig().AllowRebaseUpdate = allow + update(repoUnit.PullRequestsConfig()) assert.NoError(t, repo_model.UpdateRepoUnitConfig(t.Context(), repoUnit)) } +func enableRepoAllowUpdateWithRebase(t *testing.T, repoID int64, allow bool) { + updateRepoPullRequestConfig(t, repoID, func(c *repo_model.PullRequestsConfig) { c.AllowRebaseUpdate = allow }) +} + +// setupOutdatedPRWithConfig creates an outdated PR (user2 base, org26 fork), loads its +// base repo, head repo, and issue, then applies the supplied PullRequestsConfig mutation. +func setupOutdatedPRWithConfig(t *testing.T, mutate func(*repo_model.PullRequestsConfig)) *issues_model.PullRequest { + t.Helper() + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + org26 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 26}) + pr := createOutdatedPR(t, user, org26) + require.NoError(t, pr.LoadBaseRepo(t.Context())) + require.NoError(t, pr.LoadHeadRepo(t.Context())) + require.NoError(t, pr.LoadIssue(t.Context())) + if mutate != nil { + updateRepoPullRequestConfig(t, pr.BaseRepo.ID, mutate) + } + return pr +} + func TestAPIPullUpdateByRebase(t *testing.T) { onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { // Create PR to test @@ -120,6 +140,46 @@ func TestAPIPullUpdateByRebase(t *testing.T) { }) } +// TestAPIPullUpdateStyleSettings first checks that a disabled explicit style is +// forbidden, then guards back-compat: even when the repo's DefaultUpdateStyle is +// rebase, an API call with no `style` parameter must still perform a merge +// update so existing API clients don't silently flip behavior. +func TestAPIPullUpdateStyleSettings(t *testing.T) { + onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { + pr := setupOutdatedPRWithConfig(t, func(c *repo_model.PullRequestsConfig) { + c.AllowMergeUpdate = false + c.AllowRebaseUpdate = true + c.DefaultUpdateStyle = repo_model.UpdateStyleRebase + }) + + user40 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 40}) + require.NoError(t, repo_service.AddOrUpdateCollaborator(t.Context(), pr.BaseRepo, user40, perm.AccessModeWrite)) + require.NoError(t, repo_service.AddOrUpdateCollaborator(t.Context(), pr.HeadRepo, user40, perm.AccessModeWrite)) + + session := loginUser(t, "user40") + token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) + + req := NewRequestf(t, "POST", "/api/v1/repos/%s/%s/pulls/%d/update?style=merge", pr.BaseRepo.OwnerName, pr.BaseRepo.Name, pr.Issue.Index). + AddTokenAuth(token) + session.MakeRequest(t, req, http.StatusForbidden) + + updateRepoPullRequestConfig(t, pr.BaseRepo.ID, func(c *repo_model.PullRequestsConfig) { + c.AllowMergeUpdate = true + }) + + req = NewRequestf(t, "POST", "/api/v1/repos/%s/%s/pulls/%d/update", pr.BaseRepo.OwnerName, pr.BaseRepo.Name, pr.Issue.Index). + AddTokenAuth(token) + session.MakeRequest(t, req, http.StatusOK) + + // merge update produces a merge commit on top of the head commit (Ahead=2), + // rebase update would replay the head commit alone (Ahead=1). + diffCount, err := gitrepo.GetDivergingCommits(t.Context(), pr.BaseRepo, pr.BaseBranch, pr.GetGitHeadRefName()) + require.NoError(t, err) + assert.Equal(t, 0, diffCount.Behind) + assert.Equal(t, 2, diffCount.Ahead) + }) +} + func createOutdatedPR(t *testing.T, actor, forkOrg *user_model.User) *issues_model.PullRequest { baseRepo, err := repo_service.CreateRepository(t.Context(), actor, actor, repo_service.CreateRepoOptions{ Name: "repo-pr-update",