Add branch auto deletion for scheduled PRs

This commit is contained in:
Tom Neuber 2024-10-21 21:21:50 +02:00
parent 972813f93f
commit 471303a670
Signed by: tom
GPG key ID: F17EFE4272D89FF6
10 changed files with 53 additions and 25 deletions

View file

@ -82,6 +82,8 @@ var migrations = []*Migration{
NewMigration("Add SSH keypair to `pull_mirror` table", AddSSHKeypairToPushMirror), NewMigration("Add SSH keypair to `pull_mirror` table", AddSSHKeypairToPushMirror),
// v22 -> v23 // v22 -> v23
NewMigration("Add `legacy` to `web_authn_credential` table", AddLegacyToWebAuthnCredential), NewMigration("Add `legacy` to `web_authn_credential` table", AddLegacyToWebAuthnCredential),
// v23 -> v24
NewMigration("Add `delete_branch_after_merge` to `auto_merge` table", AddDeleteBranchAfterMergeToAutoMerge),
} }
// GetCurrentDBVersion returns the current Forgejo database version. // GetCurrentDBVersion returns the current Forgejo database version.

View file

@ -0,0 +1,16 @@
// Copyright 2024 The Forgejo Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package forgejo_migrations //nolint:revive
import "xorm.io/xorm"
// AddDeleteBranchAfterMergeToAutoMerge: add DeleteBranchAfterMerge column, setting existing rows to false
func AddDeleteBranchAfterMergeToAutoMerge(x *xorm.Engine) error {
type AutoMerge struct {
ID int64 `xorm:"pk autoincr"`
DeleteBranchAfterMerge bool `xorm:"NOT NULL DEFAULT false"`
}
return x.Sync(&AutoMerge{})
}

View file

@ -15,13 +15,14 @@ import (
// AutoMerge represents a pull request scheduled for merging when checks succeed // AutoMerge represents a pull request scheduled for merging when checks succeed
type AutoMerge struct { type AutoMerge struct {
ID int64 `xorm:"pk autoincr"` ID int64 `xorm:"pk autoincr"`
PullID int64 `xorm:"UNIQUE"` PullID int64 `xorm:"UNIQUE"`
DoerID int64 `xorm:"INDEX NOT NULL"` DoerID int64 `xorm:"INDEX NOT NULL"`
Doer *user_model.User `xorm:"-"` Doer *user_model.User `xorm:"-"`
MergeStyle repo_model.MergeStyle `xorm:"varchar(30)"` MergeStyle repo_model.MergeStyle `xorm:"varchar(30)"`
Message string `xorm:"LONGTEXT"` Message string `xorm:"LONGTEXT"`
CreatedUnix timeutil.TimeStamp `xorm:"created"` DeleteBranchAfterMerge bool `xorm:"NOT NULL DEFAULT false"`
CreatedUnix timeutil.TimeStamp `xorm:"created"`
} }
// TableName return database table name for xorm // TableName return database table name for xorm
@ -49,7 +50,7 @@ func IsErrAlreadyScheduledToAutoMerge(err error) bool {
} }
// ScheduleAutoMerge schedules a pull request to be merged when all checks succeed // ScheduleAutoMerge schedules a pull request to be merged when all checks succeed
func ScheduleAutoMerge(ctx context.Context, doer *user_model.User, pullID int64, style repo_model.MergeStyle, message string) error { func ScheduleAutoMerge(ctx context.Context, doer *user_model.User, pullID int64, style repo_model.MergeStyle, message string, deleteBranch bool) error {
// Check if we already have a merge scheduled for that pull request // Check if we already have a merge scheduled for that pull request
if exists, _, err := GetScheduledMergeByPullID(ctx, pullID); err != nil { if exists, _, err := GetScheduledMergeByPullID(ctx, pullID); err != nil {
return err return err
@ -58,10 +59,11 @@ func ScheduleAutoMerge(ctx context.Context, doer *user_model.User, pullID int64,
} }
_, err := db.GetEngine(ctx).Insert(&AutoMerge{ _, err := db.GetEngine(ctx).Insert(&AutoMerge{
DoerID: doer.ID, DoerID: doer.ID,
PullID: pullID, PullID: pullID,
MergeStyle: style, MergeStyle: style,
Message: message, Message: message,
DeleteBranchAfterMerge: deleteBranch,
}) })
return err return err
} }

View file

@ -965,7 +965,7 @@ func MergePullRequest(ctx *context.APIContext) {
} }
if form.MergeWhenChecksSucceed { if form.MergeWhenChecksSucceed {
scheduled, err := automerge.ScheduleAutoMerge(ctx, ctx.Doer, pr, repo_model.MergeStyle(form.Do), message) scheduled, err := automerge.ScheduleAutoMerge(ctx, ctx.Doer, pr, repo_model.MergeStyle(form.Do), message, form.DeleteBranchAfterMerge)
if err != nil { if err != nil {
if pull_model.IsErrAlreadyScheduledToAutoMerge(err) { if pull_model.IsErrAlreadyScheduledToAutoMerge(err) {
ctx.Error(http.StatusConflict, "ScheduleAutoMerge", err) ctx.Error(http.StatusConflict, "ScheduleAutoMerge", err)

View file

@ -28,7 +28,7 @@ func TestHandlePullRequestMerging(t *testing.T) {
user1 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) user1 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
err = pull_model.ScheduleAutoMerge(db.DefaultContext, user1, pr.ID, repo_model.MergeStyleSquash, "squash merge a pr") err = pull_model.ScheduleAutoMerge(db.DefaultContext, user1, pr.ID, repo_model.MergeStyleSquash, "squash merge a pr", false)
require.NoError(t, err) require.NoError(t, err)
autoMerge := unittest.AssertExistsAndLoadBean(t, &pull_model.AutoMerge{PullID: pr.ID}) autoMerge := unittest.AssertExistsAndLoadBean(t, &pull_model.AutoMerge{PullID: pr.ID})

View file

@ -1302,7 +1302,7 @@ func MergePullRequest(ctx *context.Context) {
// delete all scheduled auto merges // delete all scheduled auto merges
_ = pull_model.DeleteScheduledAutoMerge(ctx, pr.ID) _ = pull_model.DeleteScheduledAutoMerge(ctx, pr.ID)
// schedule auto merge // schedule auto merge
scheduled, err := automerge.ScheduleAutoMerge(ctx, ctx.Doer, pr, repo_model.MergeStyle(form.Do), message) scheduled, err := automerge.ScheduleAutoMerge(ctx, ctx.Doer, pr, repo_model.MergeStyle(form.Do), message, form.DeleteBranchAfterMerge)
if err != nil { if err != nil {
ctx.ServerError("ScheduleAutoMerge", err) ctx.ServerError("ScheduleAutoMerge", err)
return return

View file

@ -22,6 +22,7 @@ import (
"code.gitea.io/gitea/modules/queue" "code.gitea.io/gitea/modules/queue"
notify_service "code.gitea.io/gitea/services/notify" notify_service "code.gitea.io/gitea/services/notify"
pull_service "code.gitea.io/gitea/services/pull" pull_service "code.gitea.io/gitea/services/pull"
repo_service "code.gitea.io/gitea/services/repository"
shared_automerge "code.gitea.io/gitea/services/shared/automerge" shared_automerge "code.gitea.io/gitea/services/shared/automerge"
) )
@ -52,9 +53,9 @@ func handler(items ...string) []string {
} }
// ScheduleAutoMerge if schedule is false and no error, pull can be merged directly // ScheduleAutoMerge if schedule is false and no error, pull can be merged directly
func ScheduleAutoMerge(ctx context.Context, doer *user_model.User, pull *issues_model.PullRequest, style repo_model.MergeStyle, message string) (scheduled bool, err error) { func ScheduleAutoMerge(ctx context.Context, doer *user_model.User, pull *issues_model.PullRequest, style repo_model.MergeStyle, message string, deleteBranch bool) (scheduled bool, err error) {
err = db.WithTx(ctx, func(ctx context.Context) error { err = db.WithTx(ctx, func(ctx context.Context) error {
if err := pull_model.ScheduleAutoMerge(ctx, doer, pull.ID, style, message); err != nil { if err := pull_model.ScheduleAutoMerge(ctx, doer, pull.ID, style, message, deleteBranch); err != nil {
return err return err
} }
scheduled = true scheduled = true
@ -210,4 +211,11 @@ func handlePullRequestAutoMerge(pullID int64, sha string) {
// on the pull request page. But this should not be finished in a bug fix PR which will be backport to release branch. // on the pull request page. But this should not be finished in a bug fix PR which will be backport to release branch.
return return
} }
if scheduledPRM.DeleteBranchAfterMerge {
err := repo_service.DeleteBranchAfterMerge(ctx, doer, pr, headGitRepo)
if err != nil {
log.Error("%d repo_service.DeleteBranchIfUnused: %v", pr.ID, err)
}
}
} }

View file

@ -34,7 +34,7 @@ func TestActionsAutomerge(t *testing.T) {
assert.False(t, pr.HasMerged, "PR should not be merged") assert.False(t, pr.HasMerged, "PR should not be merged")
assert.Equal(t, issues_model.PullRequestStatusMergeable, pr.Status, "PR should be mergeable") assert.Equal(t, issues_model.PullRequestStatusMergeable, pr.Status, "PR should be mergeable")
scheduled, err := automerge.ScheduleAutoMerge(ctx, user, pr, repo_model.MergeStyleMerge, "Dummy") scheduled, err := automerge.ScheduleAutoMerge(ctx, user, pr, repo_model.MergeStyleMerge, "Dummy", false)
require.NoError(t, err, "PR should be scheduled for automerge") require.NoError(t, err, "PR should be scheduled for automerge")
assert.True(t, scheduled, "PR should be scheduled for automerge") assert.True(t, scheduled, "PR should be scheduled for automerge")

View file

@ -881,12 +881,12 @@ func TestPullAutoMergeAfterCommitStatusSucceed(t *testing.T) {
session.MakeRequest(t, req, http.StatusSeeOther) session.MakeRequest(t, req, http.StatusSeeOther)
// first time insert automerge record, return true // first time insert automerge record, return true
scheduled, err := automerge.ScheduleAutoMerge(db.DefaultContext, user1, pr, repo_model.MergeStyleMerge, "auto merge test") scheduled, err := automerge.ScheduleAutoMerge(db.DefaultContext, user1, pr, repo_model.MergeStyleMerge, "auto merge test", false)
require.NoError(t, err) require.NoError(t, err)
assert.True(t, scheduled) assert.True(t, scheduled)
// second time insert automerge record, return false because it does exist // second time insert automerge record, return false because it does exist
scheduled, err = automerge.ScheduleAutoMerge(db.DefaultContext, user1, pr, repo_model.MergeStyleMerge, "auto merge test") scheduled, err = automerge.ScheduleAutoMerge(db.DefaultContext, user1, pr, repo_model.MergeStyleMerge, "auto merge test", false)
require.Error(t, err) require.Error(t, err)
assert.False(t, scheduled) assert.False(t, scheduled)
@ -965,12 +965,12 @@ func TestPullAutoMergeAfterCommitStatusSucceedAndApproval(t *testing.T) {
session.MakeRequest(t, req, http.StatusSeeOther) session.MakeRequest(t, req, http.StatusSeeOther)
// first time insert automerge record, return true // first time insert automerge record, return true
scheduled, err := automerge.ScheduleAutoMerge(db.DefaultContext, user1, pr, repo_model.MergeStyleMerge, "auto merge test") scheduled, err := automerge.ScheduleAutoMerge(db.DefaultContext, user1, pr, repo_model.MergeStyleMerge, "auto merge test", false)
require.NoError(t, err) require.NoError(t, err)
assert.True(t, scheduled) assert.True(t, scheduled)
// second time insert automerge record, return false because it does exist // second time insert automerge record, return false because it does exist
scheduled, err = automerge.ScheduleAutoMerge(db.DefaultContext, user1, pr, repo_model.MergeStyleMerge, "auto merge test") scheduled, err = automerge.ScheduleAutoMerge(db.DefaultContext, user1, pr, repo_model.MergeStyleMerge, "auto merge test", false)
require.Error(t, err) require.Error(t, err)
assert.False(t, scheduled) assert.False(t, scheduled)
@ -1094,12 +1094,12 @@ func TestPullAutoMergeAfterCommitStatusSucceedAndApprovalForAgitFlow(t *testing.
user1 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) user1 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
// first time insert automerge record, return true // first time insert automerge record, return true
scheduled, err := automerge.ScheduleAutoMerge(db.DefaultContext, user1, pr, repo_model.MergeStyleMerge, "auto merge test") scheduled, err := automerge.ScheduleAutoMerge(db.DefaultContext, user1, pr, repo_model.MergeStyleMerge, "auto merge test", false)
require.NoError(t, err) require.NoError(t, err)
assert.True(t, scheduled) assert.True(t, scheduled)
// second time insert automerge record, return false because it does exist // second time insert automerge record, return false because it does exist
scheduled, err = automerge.ScheduleAutoMerge(db.DefaultContext, user1, pr, repo_model.MergeStyleMerge, "auto merge test") scheduled, err = automerge.ScheduleAutoMerge(db.DefaultContext, user1, pr, repo_model.MergeStyleMerge, "auto merge test", false)
require.Error(t, err) require.Error(t, err)
assert.False(t, scheduled) assert.False(t, scheduled)

View file

@ -130,7 +130,7 @@ export default {
{{ mergeForm.textCancel }} {{ mergeForm.textCancel }}
</button> </button>
<div class="ui checkbox tw-ml-1" v-if="mergeForm.isPullBranchDeletable && !autoMergeWhenSucceed"> <div class="ui checkbox tw-ml-1" v-if="mergeForm.isPullBranchDeletable">
<input name="delete_branch_after_merge" type="checkbox" v-model="deleteBranchAfterMerge" id="delete-branch-after-merge"> <input name="delete_branch_after_merge" type="checkbox" v-model="deleteBranchAfterMerge" id="delete-branch-after-merge">
<label for="delete-branch-after-merge">{{ mergeForm.textDeleteBranch }}</label> <label for="delete-branch-after-merge">{{ mergeForm.textDeleteBranch }}</label>
</div> </div>