From 13ca6c14f16cf2f76243f5fd1bb7efb763de8a36 Mon Sep 17 00:00:00 2001 From: George Bartolomey Date: Sat, 14 Dec 2024 10:12:04 +0300 Subject: [PATCH] feat: allow changing default branch update style This commit allows chaning default branch update style through global and repository settings. The setting affects "Update branch" button in PR view (button shows when some commits are ahead of master branch). When default update style is set to "rebase", dropdown button updates branch by rebase by default. When update style is set to other value, dropdown button updates branch by merge. Any of these actions may be selected using dropdown in any case. Signed-off-by: George Bartolomey --- models/repo/git.go | 9 ++ models/repo/repo_unit.go | 20 ++++ models/repo/repo_unit_test.go | 49 +++++++++ modules/repository/create.go | 5 +- modules/setting/repository.go | 3 + modules/structs/repo.go | 3 + options/locale/locale_en-US.ini | 1 + routers/api/v1/repo/repo.go | 4 + routers/web/repo/issue.go | 15 +++ routers/web/repo/setting/setting.go | 1 + services/convert/repository.go | 3 + services/forms/repo_form.go | 1 + .../view_content/update_branch_by_merge.tmpl | 18 +++- templates/repo/settings/units/pulls.tmpl | 23 ++++ templates/swagger/v1_json.tmpl | 9 ++ tests/integration/pull_update_test.go | 100 ++++++++++++++++++ 16 files changed, 257 insertions(+), 7 deletions(-) diff --git a/models/repo/git.go b/models/repo/git.go index 388bf86522..d39915e869 100644 --- a/models/repo/git.go +++ b/models/repo/git.go @@ -29,6 +29,15 @@ const ( MergeStyleRebaseUpdate MergeStyle = "rebase-update-only" ) +type UpdateStyle string + +const ( + // UpdateStyleMerge create merge commit to update + UpdateStyleMerge UpdateStyle = "merge" + // UpdateStyleRebase rebase to update + 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/repo_unit.go b/models/repo/repo_unit.go index ed553844fc..5b4066d994 100644 --- a/models/repo/repo_unit.go +++ b/models/repo/repo_unit.go @@ -159,6 +159,7 @@ type PullRequestsConfig struct { AllowRebaseUpdate bool DefaultDeleteBranchAfterMerge bool DefaultMergeStyle MergeStyle + DefaultUpdateStyle UpdateStyle DefaultAllowMaintainerEdit bool } @@ -197,6 +198,25 @@ func (cfg *PullRequestsConfig) GetDefaultMergeStyle() MergeStyle { return MergeStyleMerge } +// IsUpdateStyleAllowed returns if update style is allowed +func (cfg *PullRequestsConfig) IsUpdateStyleAllowed(updateStyle UpdateStyle) bool { + return updateStyle == UpdateStyleMerge || + updateStyle == UpdateStyleRebase && cfg.AllowRebaseUpdate +} + +// GetDefaultUpdateStyle returns the default update style for this pull request +func (cfg *PullRequestsConfig) GetDefaultUpdateStyle() UpdateStyle { + if len(cfg.DefaultUpdateStyle) != 0 { + return cfg.DefaultUpdateStyle + } + + if setting.Repository.PullRequest.DefaultUpdateStyle != "" { + return UpdateStyle(setting.Repository.PullRequest.DefaultUpdateStyle) + } + + return UpdateStyleMerge +} + type ActionsConfig struct { DisabledWorkflows []string } diff --git a/models/repo/repo_unit_test.go b/models/repo/repo_unit_test.go index deee1a7472..129d913cfb 100644 --- a/models/repo/repo_unit_test.go +++ b/models/repo/repo_unit_test.go @@ -7,6 +7,8 @@ import ( "testing" "code.gitea.io/gitea/models/perm" + "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/test" "github.com/stretchr/testify/assert" ) @@ -37,3 +39,50 @@ func TestRepoUnitAccessMode(t *testing.T) { assert.Equal(t, perm.AccessModeWrite, UnitAccessModeWrite.ToAccessMode(perm.AccessModeAdmin)) assert.Equal(t, perm.AccessModeRead, UnitAccessModeUnset.ToAccessMode(perm.AccessModeRead)) } + +func TestRepoPRIsUpdateStyleAllowed(t *testing.T) { + var cfg PullRequestsConfig + cfg = PullRequestsConfig{ + AllowRebaseUpdate: true, + } + assert.True(t, cfg.IsUpdateStyleAllowed(UpdateStyleMerge)) + assert.True(t, cfg.IsUpdateStyleAllowed(UpdateStyleRebase)) + + cfg = PullRequestsConfig{ + AllowRebaseUpdate: false, + } + assert.True(t, cfg.IsUpdateStyleAllowed(UpdateStyleMerge)) + assert.False(t, cfg.IsUpdateStyleAllowed(UpdateStyleRebase)) +} + +func TestRepoPRGetDefaultUpdateStyle(t *testing.T) { + defer test.MockVariableValue(&setting.Repository.PullRequest.DefaultUpdateStyle, "merge")() + + var cfg PullRequestsConfig + cfg = PullRequestsConfig{ + DefaultUpdateStyle: "", + } + assert.Equal(t, UpdateStyleMerge, cfg.GetDefaultUpdateStyle()) + cfg = PullRequestsConfig{ + DefaultUpdateStyle: "rebase", + } + assert.Equal(t, UpdateStyleRebase, cfg.GetDefaultUpdateStyle()) + cfg = PullRequestsConfig{ + DefaultUpdateStyle: "merge", + } + assert.Equal(t, UpdateStyleMerge, cfg.GetDefaultUpdateStyle()) + + setting.Repository.PullRequest.DefaultUpdateStyle = "rebase" + cfg = PullRequestsConfig{ + DefaultUpdateStyle: "", + } + assert.Equal(t, UpdateStyleRebase, cfg.GetDefaultUpdateStyle()) + cfg = PullRequestsConfig{ + DefaultUpdateStyle: "rebase", + } + assert.Equal(t, UpdateStyleRebase, cfg.GetDefaultUpdateStyle()) + cfg = PullRequestsConfig{ + DefaultUpdateStyle: "merge", + } + assert.Equal(t, UpdateStyleMerge, cfg.GetDefaultUpdateStyle()) +} diff --git a/modules/repository/create.go b/modules/repository/create.go index ca2150b972..32c6235544 100644 --- a/modules/repository/create.go +++ b/modules/repository/create.go @@ -89,8 +89,9 @@ func CreateRepositoryByExample(ctx context.Context, doer, u *user_model.User, re 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, + DefaultMergeStyle: repo_model.MergeStyle(setting.Repository.PullRequest.DefaultMergeStyle), + DefaultUpdateStyle: repo_model.UpdateStyle(setting.Repository.PullRequest.DefaultUpdateStyle), + AllowRebaseUpdate: true, }, }) } else { diff --git a/modules/setting/repository.go b/modules/setting/repository.go index 6086dd1d57..d13f25f554 100644 --- a/modules/setting/repository.go +++ b/modules/setting/repository.go @@ -87,6 +87,7 @@ var ( DefaultMergeMessageAllAuthors bool DefaultMergeMessageMaxApprovers int DefaultMergeMessageOfficialApproversOnly bool + DefaultUpdateStyle string PopulateSquashCommentWithCommitMessages bool AddCoCommitterTrailers bool TestConflictingPatchesWithGitApply bool @@ -216,6 +217,7 @@ var ( DefaultMergeMessageAllAuthors bool DefaultMergeMessageMaxApprovers int DefaultMergeMessageOfficialApproversOnly bool + DefaultUpdateStyle string PopulateSquashCommentWithCommitMessages bool AddCoCommitterTrailers bool TestConflictingPatchesWithGitApply bool @@ -232,6 +234,7 @@ var ( DefaultMergeMessageAllAuthors: false, DefaultMergeMessageMaxApprovers: 10, DefaultMergeMessageOfficialApproversOnly: true, + DefaultUpdateStyle: "merge", PopulateSquashCommentWithCommitMessages: false, AddCoCommitterTrailers: true, RetargetChildrenOnMerge: true, diff --git a/modules/structs/repo.go b/modules/structs/repo.go index 36190fe36b..b5f54a2a7a 100644 --- a/modules/structs/repo.go +++ b/modules/structs/repo.go @@ -105,6 +105,7 @@ type Repository struct { DefaultDeleteBranchAfterMerge bool `json:"default_delete_branch_after_merge"` DefaultMergeStyle string `json:"default_merge_style"` DefaultAllowMaintainerEdit bool `json:"default_allow_maintainer_edit"` + DefaultUpdateStyle string `json:"default_update_style"` AvatarURL string `json:"avatar_url"` Internal bool `json:"internal"` MirrorInterval string `json:"mirror_interval"` @@ -225,6 +226,8 @@ type EditRepoOption struct { 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 a update style to be used by this repository: "rebase" or "merge" + 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.ini b/options/locale/locale_en-US.ini index 6e1524f583..3b413b8f92 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -2246,6 +2246,7 @@ settings.pulls_desc = Enable repository pull requests settings.pulls.ignore_whitespace = Ignore whitespace for conflicts settings.pulls.enable_autodetect_manual_merge = Enable autodetect manual merge (Note: In some special cases, misjudgments can occur) settings.pulls.allow_rebase_update = Enable updating pull request branch by rebase +settings.default_update_style_desc=Default update style used for updating pull requests that are behind the base branch. settings.pulls.default_delete_branch_after_merge = Delete pull request branch after merge by default settings.pulls.default_allow_edits_from_maintainers = Allow edits from maintainers by default settings.releases_desc = Enable repository releases diff --git a/routers/api/v1/repo/repo.go b/routers/api/v1/repo/repo.go index 93d40d2dbf..04c6fde453 100644 --- a/routers/api/v1/repo/repo.go +++ b/routers/api/v1/repo/repo.go @@ -937,6 +937,7 @@ func updateRepoUnits(ctx *context.APIContext, opts api.EditRepoOption) error { AllowRebaseUpdate: true, DefaultDeleteBranchAfterMerge: false, DefaultMergeStyle: repo_model.MergeStyleMerge, + DefaultUpdateStyle: repo_model.UpdateStyleMerge, DefaultAllowMaintainerEdit: false, } } else { @@ -976,6 +977,9 @@ func updateRepoUnits(ctx *context.APIContext, opts api.EditRepoOption) error { if opts.DefaultMergeStyle != nil { config.DefaultMergeStyle = repo_model.MergeStyle(*opts.DefaultMergeStyle) } + if opts.DefaultUpdateStyle != nil { + config.DefaultUpdateStyle = repo_model.UpdateStyle(*opts.DefaultUpdateStyle) + } if opts.DefaultAllowMaintainerEdit != nil { config.DefaultAllowMaintainerEdit = *opts.DefaultAllowMaintainerEdit } diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 78fb5e6c01..c154a9b665 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -1918,6 +1918,21 @@ func ViewIssue(ctx *context.Context) { ctx.Data["MergeStyle"] = mergeStyle + var updateStyle repo_model.UpdateStyle + // Check correct values and select default + if ms, ok := ctx.Data["UpdateStyle"].(repo_model.UpdateStyle); !ok || + !prConfig.IsUpdateStyleAllowed(ms) { + defaultUpdateStyle := prConfig.GetDefaultUpdateStyle() + if prConfig.IsUpdateStyleAllowed(defaultUpdateStyle) && !ok { + updateStyle = defaultUpdateStyle + } else if prConfig.AllowMerge { + updateStyle = repo_model.UpdateStyleMerge + } else if prConfig.AllowRebase { + updateStyle = repo_model.UpdateStyleRebase + } + } + ctx.Data["UpdateStyle"] = updateStyle + defaultMergeMessage, defaultMergeBody, err := pull_service.GetDefaultMergeMessage(ctx, ctx.Repo.GitRepo, pull, mergeStyle) if err != nil { ctx.ServerError("GetDefaultMergeMessage", err) diff --git a/routers/web/repo/setting/setting.go b/routers/web/repo/setting/setting.go index ce506eafb1..2c6ca77a6a 100644 --- a/routers/web/repo/setting/setting.go +++ b/routers/web/repo/setting/setting.go @@ -262,6 +262,7 @@ func UnitsPost(ctx *context.Context) { AllowRebaseUpdate: form.PullsAllowRebaseUpdate, DefaultDeleteBranchAfterMerge: form.DefaultDeleteBranchAfterMerge, DefaultMergeStyle: repo_model.MergeStyle(form.PullsDefaultMergeStyle), + DefaultUpdateStyle: repo_model.UpdateStyle(form.PullsDefaultUpdateStyle), DefaultAllowMaintainerEdit: form.DefaultAllowMaintainerEdit, }, }) diff --git a/services/convert/repository.go b/services/convert/repository.go index 2fb6f6d7c0..e4b2c7b8bc 100644 --- a/services/convert/repository.go +++ b/services/convert/repository.go @@ -101,6 +101,7 @@ func innerToRepo(ctx context.Context, repo *repo_model.Repository, permissionInR allowRebaseUpdate := false defaultDeleteBranchAfterMerge := false defaultMergeStyle := repo_model.MergeStyleMerge + defaultUpdateStyle := repo_model.UpdateStyleMerge defaultAllowMaintainerEdit := false if unit, err := repo.GetUnit(ctx, unit_model.TypePullRequests); err == nil { config := unit.PullRequestsConfig() @@ -114,6 +115,7 @@ func innerToRepo(ctx context.Context, repo *repo_model.Repository, permissionInR allowRebaseUpdate = config.AllowRebaseUpdate defaultDeleteBranchAfterMerge = config.DefaultDeleteBranchAfterMerge defaultMergeStyle = config.GetDefaultMergeStyle() + defaultUpdateStyle = config.GetDefaultUpdateStyle() defaultAllowMaintainerEdit = config.DefaultAllowMaintainerEdit } hasProjects := false @@ -231,6 +233,7 @@ func innerToRepo(ctx context.Context, repo *repo_model.Repository, permissionInR AllowRebaseUpdate: allowRebaseUpdate, DefaultDeleteBranchAfterMerge: defaultDeleteBranchAfterMerge, DefaultMergeStyle: string(defaultMergeStyle), + DefaultUpdateStyle: string(defaultUpdateStyle), DefaultAllowMaintainerEdit: defaultAllowMaintainerEdit, AvatarURL: repo.AvatarLink(ctx), Internal: !repo.IsPrivate && repo.Owner.Visibility == api.VisibleTypePrivate, diff --git a/services/forms/repo_form.go b/services/forms/repo_form.go index f6e184fcb6..1ce9b298ad 100644 --- a/services/forms/repo_form.go +++ b/services/forms/repo_form.go @@ -189,6 +189,7 @@ type RepoUnitSettingForm struct { PullsAllowFastForwardOnly bool PullsAllowManualMerge bool PullsDefaultMergeStyle string + PullsDefaultUpdateStyle string EnableAutodetectManualMerge bool PullsAllowRebaseUpdate bool DefaultDeleteBranchAfterMerge bool 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 adce052dee..9fbc8b018b 100644 --- a/templates/repo/issue/view_content/update_branch_by_merge.tmpl +++ b/templates/repo/issue/view_content/update_branch_by_merge.tmpl @@ -9,23 +9,31 @@ {{if and $.UpdateAllowed $.UpdateByRebaseAllowed}}
-
{{svg "octicon-triangle-down"}}
- {{ctx.Locale.Tr "repo.pulls.update_branch"}} - {{ctx.Locale.Tr "repo.pulls.update_branch_rebase"}} + + {{ctx.Locale.Tr "repo.pulls.update_branch"}} + + + {{ctx.Locale.Tr "repo.pulls.update_branch_rebase"}} +
{{end}} {{if and $.UpdateAllowed (not $.UpdateByRebaseAllowed)}} -
+ {{$.CsrfTokenHtml}}