Merge pull request 'Allow changing default branch update style' (#6265) from george.bartolomey/forgejo:1-respect-update-branch-method into forgejo

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/6265
Reviewed-by: 0ko <0ko@noreply.codeberg.org>
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
This commit is contained in:
0ko 2024-12-24 05:08:01 +00:00
commit 5f685bf069
16 changed files with 257 additions and 7 deletions

View file

@ -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)

View file

@ -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
}

View file

@ -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())
}

View file

@ -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 {

View file

@ -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,

View file

@ -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.

View file

@ -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

View file

@ -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
}

View file

@ -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)

View file

@ -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,
},
})

View file

@ -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,

View file

@ -189,6 +189,7 @@ type RepoUnitSettingForm struct {
PullsAllowFastForwardOnly bool
PullsAllowManualMerge bool
PullsDefaultMergeStyle string
PullsDefaultUpdateStyle string
EnableAutodetectManualMerge bool
PullsAllowRebaseUpdate bool
DefaultDeleteBranchAfterMerge bool

View file

@ -9,23 +9,31 @@
{{if and $.UpdateAllowed $.UpdateByRebaseAllowed}}
<div class="tw-inline-block">
<div class="ui buttons update-button">
<button class="ui button" data-do="{{$.Link}}/update" data-redirect="{{$.Link}}">
<button class="ui button" data-do="{{$.Link}}/update?style={{$.UpdateStyle}}" data-redirect="{{$.Link}}">
<span class="button-text">
{{ctx.Locale.Tr "repo.pulls.update_branch"}}
{{if eq $.UpdateStyle "rebase"}}
{{ctx.Locale.Tr "repo.pulls.update_branch_rebase"}}
{{else}}
{{ctx.Locale.Tr "repo.pulls.update_branch"}}
{{end}}
</span>
</button>
<div class="ui dropdown icon button">
{{svg "octicon-triangle-down"}}
<div class="menu">
<a class="item active selected" data-do="{{$.Link}}/update">{{ctx.Locale.Tr "repo.pulls.update_branch"}}</a>
<a class="item" data-do="{{$.Link}}/update?style=rebase">{{ctx.Locale.Tr "repo.pulls.update_branch_rebase"}}</a>
<a class="item {{if ne $.UpdateStyle "rebase"}}active selected{{end}}" data-do="{{$.Link}}/update?style=merge">
{{ctx.Locale.Tr "repo.pulls.update_branch"}}
</a>
<a class="item {{if eq $.UpdateStyle "rebase"}}active selected{{end}}" data-do="{{$.Link}}/update?style=rebase">
{{ctx.Locale.Tr "repo.pulls.update_branch_rebase"}}
</a>
</div>
</div>
</div>
</div>
{{end}}
{{if and $.UpdateAllowed (not $.UpdateByRebaseAllowed)}}
<form action="{{$.Link}}/update" method="post" class="ui update-branch-form">
<form action="{{$.Link}}/update?style=merge" method="post" class="ui update-branch-form">
{{$.CsrfTokenHtml}}
<button class="ui compact button">
<span class="ui text">{{ctx.Locale.Tr "repo.pulls.update_branch"}}</span>

View file

@ -105,6 +105,29 @@
<label>{{ctx.Locale.Tr "repo.settings.pulls.allow_rebase_update"}}</label>
</div>
</div>
<div class="field">
<p>
{{ctx.Locale.Tr "repo.settings.default_update_style_desc"}}
</p>
<div class="ui dropdown selection">
<select name="pulls_default_update_style">
<option value="merge" {{if or (not $pullRequestEnabled) (eq $prUnit.PullRequestsConfig.DefaultUpdateStyle "merge")}}selected{{end}}>{{ctx.Locale.Tr "repo.pulls.update_branch"}}</option>
<option value="rebase" {{if or (not $pullRequestEnabled) (eq $prUnit.PullRequestsConfig.DefaultUpdateStyle "rebase")}}selected{{end}}>{{ctx.Locale.Tr "repo.pulls.update_branch_rebase"}}</option>
</select>{{svg "octicon-triangle-down" 14 "dropdown icon"}}
<div class="default text">
{{if (eq $prUnit.PullRequestsConfig.DefaultUpdateStyle "merge")}}
{{ctx.Locale.Tr "repo.pulls.update_branch"}}
{{end}}
{{if (eq $prUnit.PullRequestsConfig.DefaultUpdateStyle "rebase")}}
{{ctx.Locale.Tr "repo.pulls.update_branch_rebase"}}
{{end}}
</div>
<div class="menu">
<div class="item" data-value="merge">{{ctx.Locale.Tr "repo.pulls.update_branch"}}</div>
<div class="item" data-value="rebase">{{ctx.Locale.Tr "repo.pulls.update_branch_rebase"}}</div>
</div>
</div>
</div>
<div class="field">
<div class="ui checkbox">
<input name="default_delete_branch_after_merge" type="checkbox" {{if or (not $pullRequestEnabled) ($prUnit.PullRequestsConfig.DefaultDeleteBranchAfterMerge)}}checked{{end}}>

View file

@ -23321,6 +23321,11 @@
"type": "string",
"x-go-name": "DefaultMergeStyle"
},
"default_update_style": {
"description": "set to a update style to be used by this repository: \"rebase\" or \"merge\"",
"type": "string",
"x-go-name": "DefaultUpdateStyle"
},
"description": {
"description": "a short description of the repository.",
"type": "string",
@ -26605,6 +26610,10 @@
"type": "string",
"x-go-name": "DefaultMergeStyle"
},
"default_update_style": {
"type": "string",
"x-go-name": "DefaultUpdateStyle"
},
"description": {
"type": "string",
"x-go-name": "Description"

View file

@ -4,6 +4,7 @@
package integration
import (
"fmt"
"net/http"
"net/url"
"strings"
@ -16,6 +17,9 @@ import (
"code.gitea.io/gitea/models/unittest"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/setting"
api "code.gitea.io/gitea/modules/structs"
"code.gitea.io/gitea/modules/test"
pull_service "code.gitea.io/gitea/services/pull"
repo_service "code.gitea.io/gitea/services/repository"
files_service "code.gitea.io/gitea/services/repository/files"
@ -83,6 +87,102 @@ func TestAPIPullUpdateByRebase(t *testing.T) {
})
}
func TestAPIViewUpdateSettings(t *testing.T) {
onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) {
defer tests.PrepareTestEnv(t)()
// Create PR to test
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
org26 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 26})
pr := createOutdatedPR(t, user, org26)
// Test GetDiverging
diffCount, err := pull_service.GetDiverging(git.DefaultContext, pr)
require.NoError(t, err)
assert.EqualValues(t, 1, diffCount.Behind)
assert.EqualValues(t, 1, diffCount.Ahead)
require.NoError(t, pr.LoadBaseRepo(db.DefaultContext))
require.NoError(t, pr.LoadIssue(db.DefaultContext))
session := loginUser(t, "user2")
token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeAll)
defaultUpdateStyle := "rebase"
editOption := api.EditRepoOption{
DefaultUpdateStyle: &defaultUpdateStyle,
}
req := NewRequestWithJSON(t, "PATCH", fmt.Sprintf("/api/v1/repos/%s/%s", pr.BaseRepo.OwnerName, pr.BaseRepo.Name), editOption).AddTokenAuth(token)
session.MakeRequest(t, req, http.StatusOK)
assertViewPullUpdate(t, pr, session, "rebase", true)
defaultUpdateStyle = "merge"
req = NewRequestWithJSON(t, "PATCH", fmt.Sprintf("/api/v1/repos/%s/%s", pr.BaseRepo.OwnerName, pr.BaseRepo.Name), editOption).AddTokenAuth(token)
session.MakeRequest(t, req, http.StatusOK)
assertViewPullUpdate(t, pr, session, "merge", true)
})
}
func TestViewPullUpdateByMerge(t *testing.T) {
onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) {
testViewPullUpdate(t, "merge")
})
}
func TestViewPullUpdateByRebase(t *testing.T) {
onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) {
testViewPullUpdate(t, "rebase")
})
}
func testViewPullUpdate(t *testing.T, updateStyle string) {
defer test.MockVariableValue(&setting.Repository.PullRequest.DefaultUpdateStyle, updateStyle)()
defer tests.PrepareTestEnv(t)()
// Create PR to test
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
org26 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 26})
pr := createOutdatedPR(t, user, org26)
// Test GetDiverging
diffCount, err := pull_service.GetDiverging(git.DefaultContext, pr)
require.NoError(t, err)
assert.EqualValues(t, 1, diffCount.Behind)
assert.EqualValues(t, 1, diffCount.Ahead)
require.NoError(t, pr.LoadBaseRepo(db.DefaultContext))
require.NoError(t, pr.LoadIssue(db.DefaultContext))
session := loginUser(t, "user2")
assertViewPullUpdate(t, pr, session, updateStyle, true)
}
func assertViewPullUpdate(t *testing.T, pr *issues_model.PullRequest, session *TestSession, expectedStyle string, dropdownExpected bool) {
req := NewRequest(t, "GET", fmt.Sprintf("%s/%s/pulls/%d", pr.BaseRepo.OwnerName, pr.BaseRepo.Name, pr.Issue.Index))
resp := session.MakeRequest(t, req, http.StatusOK)
htmlDoc := NewHTMLParser(t, resp.Body)
// Verify that URL of the update button is shown correctly.
var mainExpectedURL string
mergeExpectedURL := fmt.Sprintf("/%s/%s/pulls/%d/update?style=merge", pr.BaseRepo.OwnerName, pr.BaseRepo.Name, pr.Issue.Index)
rebaseExpectedURL := fmt.Sprintf("/%s/%s/pulls/%d/update?style=rebase", pr.BaseRepo.OwnerName, pr.BaseRepo.Name, pr.Issue.Index)
if expectedStyle == "rebase" {
mainExpectedURL = rebaseExpectedURL
if dropdownExpected {
htmlDoc.AssertElement(t, fmt.Sprintf(".update-button .dropdown .menu .item[data-do=\"%s\"]:not(.active.selected)", mergeExpectedURL), true)
htmlDoc.AssertElement(t, fmt.Sprintf(".update-button .dropdown .menu .active.selected.item[data-do=\"%s\"]", rebaseExpectedURL), true)
}
} else {
mainExpectedURL = mergeExpectedURL
if dropdownExpected {
htmlDoc.AssertElement(t, fmt.Sprintf(".update-button .dropdown .menu .active.selected.item[data-do=\"%s\"]", mergeExpectedURL), true)
htmlDoc.AssertElement(t, fmt.Sprintf(".update-button .dropdown .menu .item[data-do=\"%s\"]:not(.active.selected)", rebaseExpectedURL), true)
}
}
if dropdownExpected {
htmlDoc.AssertElement(t, fmt.Sprintf(".update-button .button[data-do=\"%s\"]", mainExpectedURL), true)
} else {
htmlDoc.AssertElement(t, fmt.Sprintf("form[action=\"%s\"]", mainExpectedURL), true)
}
}
func createOutdatedPR(t *testing.T, actor, forkOrg *user_model.User) *issues_model.PullRequest {
baseRepo, _, _ := tests.CreateDeclarativeRepo(t, actor, "repo-pr-update", nil, nil, nil)