lunny/gitea
Archived
1
1
Fork 1
Code Issues 1.7k Pull Requests 154 Projects Releases 128 Wiki Activity

Add checkbox to delete pull branch after successful merge #16049

Merged
lunny merged 18 commits from delete-pull-branch-on-merge into main 2021-07-12 23:26:25 +00:00
Conversation 0 Commits 18 Files Changed 14 +182 -44
14 changed files with 182 additions and 44 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
models/repo_unit.go
View File

@ -91,14 +91,15 @@ func (cfg *IssuesConfig) ToDB() ([]byte, error) {
// PullRequestsConfig describes pull requests config
type PullRequestsConfig struct {
IgnoreWhitespaceConflicts bool
AllowMerge bool
AllowRebase bool
AllowRebaseMerge bool
AllowSquash bool
AllowManualMerge bool
AutodetectManualMerge bool
DefaultMergeStyle MergeStyle
IgnoreWhitespaceConflicts bool
AllowMerge bool
AllowRebase bool
AllowRebaseMerge bool
AllowSquash bool
AllowManualMerge bool
AutodetectManualMerge bool
DefaultDeleteBranchAfterMerge bool
DefaultMergeStyle MergeStyle
}
// FromDB fills up a PullRequestsConfig from serialized format.

2
modules/structs/repo.go
View File

@ -172,6 +172,8 @@ type EditRepoOption struct {
AllowManualMerge *bool `json:"allow_manual_merge,omitempty"`
// either `true` to enable AutodetectManualMerge, or `false` to prevent it. `has_pull_requests` must be `true`, Note: In some special cases, misjudgments can occur.
AutodetectManualMerge *bool `json:"autodetect_manual_merge,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", or "squash". `has_pull_requests` must be `true`.
DefaultMergeStyle *string `json:"default_merge_style,omitempty"`
// set to `true` to archive this repository.

1
options/locale/locale_en-US.ini
View File

@ -1664,6 +1664,7 @@ settings.pulls.allow_rebase_merge_commit = Enable Rebasing with explicit merge c
settings.pulls.allow_squash_commits = Enable Squashing to Merge Commits
settings.pulls.allow_manual_merge = Enable Mark PR as manually merged
settings.pulls.enable_autodetect_manual_merge = Enable autodetect manual merge (Note: In some special cases, misjudgments can occur)
settings.pulls.default_delete_branch_after_merge = Delete pull request branch after merge by default
settings.projects_desc = Enable Repository Projects
settings.admin_settings = Administrator Settings
settings.admin_enable_health_check = Enable Repository Health Checks (git fsck)

34
routers/api/v1/repo/pull.go
View File

@ -5,6 +5,7 @@
package repo
import (
"errors"
"fmt"
"math"
"net/http"
@ -25,6 +26,7 @@ import (
"code.gitea.io/gitea/services/forms"
issue_service "code.gitea.io/gitea/services/issue"
pull_service "code.gitea.io/gitea/services/pull"
repo_service "code.gitea.io/gitea/services/repository"
)
// ListPullRequests returns a list of all PRs
@ -878,6 +880,38 @@ func MergePullRequest(ctx *context.APIContext) {
}
log.Trace("Pull request merged: %d", pr.ID)
if form.DeleteBranchAfterMerge {
var headRepo *git.Repository
if ctx.Repo != nil && ctx.Repo.Repository != nil && ctx.Repo.Repository.ID == pr.HeadRepoID && ctx.Repo.GitRepo != nil {
headRepo = ctx.Repo.GitRepo
} else {
headRepo, err = git.OpenRepository(pr.HeadRepo.RepoPath())
if err != nil {
ctx.ServerError(fmt.Sprintf("OpenRepository[%s]", pr.HeadRepo.RepoPath()), err)
return
}
defer headRepo.Close()
}
if err := repo_service.DeleteBranch(ctx.User, pr.HeadRepo, headRepo, pr.HeadBranch); err != nil {
switch {
case git.IsErrBranchNotExist(err):
ctx.NotFound(err)
case errors.Is(err, repo_service.ErrBranchIsDefault):
ctx.Error(http.StatusForbidden, "DefaultBranch", fmt.Errorf("can not delete default branch"))
case errors.Is(err, repo_service.ErrBranchIsProtected):
ctx.Error(http.StatusForbidden, "IsProtectedBranch", fmt.Errorf("branch protected"))
default:
ctx.Error(http.StatusInternalServerError, "DeleteBranch", err)
}
return
}
if err := models.AddDeletePRBranchComment(ctx.User, pr.BaseRepo, pr.Issue.ID, pr.HeadBranch); err != nil {
// Do not fail here as branch has already been deleted
log.Error("DeleteBranch: %v", err)
}
}
ctx.Status(http.StatusOK)
}

20
routers/api/v1/repo/repo.go
View File

@ -833,14 +833,15 @@ func updateRepoUnits(ctx *context.APIContext, opts api.EditRepoOption) error {
if err != nil {
// Unit type doesn't exist so we make a new config file with default values
config = &models.PullRequestsConfig{
IgnoreWhitespaceConflicts: false,
AllowMerge: true,
AllowRebase: true,
AllowRebaseMerge: true,
AllowSquash: true,
AllowManualMerge: true,
AutodetectManualMerge: false,
DefaultMergeStyle: models.MergeStyleMerge,
IgnoreWhitespaceConflicts: false,
AllowMerge: true,
AllowRebase: true,
AllowRebaseMerge: true,
AllowSquash: true,
AllowManualMerge: true,
AutodetectManualMerge: false,
DefaultDeleteBranchAfterMerge: false,
DefaultMergeStyle: models.MergeStyleMerge,
}
} else {
config = unit.PullRequestsConfig()
@ -867,6 +868,9 @@ func updateRepoUnits(ctx *context.APIContext, opts api.EditRepoOption) error {
if opts.AutodetectManualMerge != nil {
config.AutodetectManualMerge = *opts.AutodetectManualMerge
}
if opts.DefaultDeleteBranchAfterMerge != nil {
config.DefaultDeleteBranchAfterMerge = *opts.DefaultDeleteBranchAfterMerge
}
if opts.DefaultMergeStyle != nil {
config.DefaultMergeStyle = models.MergeStyle(*opts.DefaultMergeStyle)
}

61
routers/web/repo/pull.go
View File

@ -965,6 +965,22 @@ func MergePullRequest(ctx *context.Context) {
}
log.Trace("Pull request merged: %d", pr.ID)
if form.DeleteBranchAfterMerge {
var headRepo *git.Repository
if ctx.Repo != nil && ctx.Repo.Repository != nil && pr.HeadRepoID == ctx.Repo.Repository.ID && ctx.Repo.GitRepo != nil {
headRepo = ctx.Repo.GitRepo
} else {
headRepo, err = git.OpenRepository(pr.HeadRepo.RepoPath())
if err != nil {
ctx.ServerError(fmt.Sprintf("OpenRepository[%s]", pr.HeadRepo.RepoPath()), err)
return
}
defer headRepo.Close()
}
deleteBranch(ctx, pr, headRepo)
}
ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + fmt.Sprint(pr.Index))
}
@ -1170,19 +1186,35 @@ func CleanUpPullRequest(ctx *context.Context) {
fullBranchName := pr.HeadRepo.Owner.Name + "/" + pr.HeadBranch
gitRepo, err := git.OpenRepository(pr.HeadRepo.RepoPath())
if err != nil {
ctx.ServerError(fmt.Sprintf("OpenRepository[%s]", pr.HeadRepo.RepoPath()), err)
return
}
defer gitRepo.Close()
var gitBaseRepo *git.Repository
gitBaseRepo, err := git.OpenRepository(pr.BaseRepo.RepoPath())
if err != nil {
ctx.ServerError(fmt.Sprintf("OpenRepository[%s]", pr.BaseRepo.RepoPath()), err)
return
// Assume that the base repo is the current context (almost certainly)
if ctx.Repo != nil && ctx.Repo.Repository != nil && ctx.Repo.Repository.ID == pr.BaseRepoID && ctx.Repo.GitRepo != nil {
gitBaseRepo = ctx.Repo.GitRepo
} else {
// If not just open it
gitBaseRepo, err = git.OpenRepository(pr.BaseRepo.RepoPath())
if err != nil {
ctx.ServerError(fmt.Sprintf("OpenRepository[%s]", pr.BaseRepo.RepoPath()), err)
return
}
defer gitBaseRepo.Close()
}
// Now assume that the head repo is the same as the base repo (reasonable chance)
gitRepo := gitBaseRepo
// But if not: is it the same as the context?
if pr.BaseRepoID != pr.HeadRepoID && ctx.Repo != nil && ctx.Repo.Repository != nil && ctx.Repo.Repository.ID == pr.HeadRepoID && ctx.Repo.GitRepo != nil {
gitRepo = ctx.Repo.GitRepo
} else if pr.BaseRepoID != pr.HeadRepoID {
// Otherwise just load it up
gitRepo, err = git.OpenRepository(pr.HeadRepo.RepoPath())
if err != nil {
ctx.ServerError(fmt.Sprintf("OpenRepository[%s]", pr.HeadRepo.RepoPath()), err)
return
}
defer gitRepo.Close()
}
defer gitBaseRepo.Close()
defer func() {
ctx.JSON(http.StatusOK, map[string]interface{}{
@ -1208,6 +1240,11 @@ func CleanUpPullRequest(ctx *context.Context) {
return
}
deleteBranch(ctx, pr, gitRepo)
}
func deleteBranch(ctx *context.Context, pr *models.PullRequest, gitRepo *git.Repository) {
fullBranchName := pr.HeadRepo.Owner.Name + "/" + pr.HeadBranch
if err := repo_service.DeleteBranch(ctx.User, pr.HeadRepo, gitRepo, pr.HeadBranch); err != nil {
switch {
case git.IsErrBranchNotExist(err):
@ -1223,7 +1260,7 @@ func CleanUpPullRequest(ctx *context.Context) {
return
}
if err := models.AddDeletePRBranchComment(ctx.User, pr.BaseRepo, issue.ID, pr.HeadBranch); err != nil {
if err := models.AddDeletePRBranchComment(ctx.User, pr.BaseRepo, pr.IssueID, pr.HeadBranch); err != nil {
// Do not fail here as branch has already been deleted
log.Error("DeleteBranch: %v", err)
}

17
routers/web/repo/setting.go
View File

@ -416,14 +416,15 @@ func SettingsPost(ctx *context.Context) {
RepoID: repo.ID,
Type: models.UnitTypePullRequests,
Config: &models.PullRequestsConfig{
IgnoreWhitespaceConflicts: form.PullsIgnoreWhitespace,
AllowMerge: form.PullsAllowMerge,
AllowRebase: form.PullsAllowRebase,
AllowRebaseMerge: form.PullsAllowRebaseMerge,
AllowSquash: form.PullsAllowSquash,
AllowManualMerge: form.PullsAllowManualMerge,
AutodetectManualMerge: form.EnableAutodetectManualMerge,
DefaultMergeStyle: models.MergeStyle(form.PullsDefaultMergeStyle),
IgnoreWhitespaceConflicts: form.PullsIgnoreWhitespace,
AllowMerge: form.PullsAllowMerge,
AllowRebase: form.PullsAllowRebase,
AllowRebaseMerge: form.PullsAllowRebaseMerge,
AllowSquash: form.PullsAllowSquash,
AllowManualMerge: form.PullsAllowManualMerge,
AutodetectManualMerge: form.EnableAutodetectManualMerge,
DefaultDeleteBranchAfterMerge: form.DefaultDeleteBranchAfterMerge,
DefaultMergeStyle: models.MergeStyle(form.PullsDefaultMergeStyle),
},
})
} else if !models.UnitTypePullRequests.UnitGlobalDisabled() {

12
services/forms/repo_form.go
View File

@ -151,6 +151,7 @@ type RepoSettingForm struct {
PullsAllowManualMerge bool
PullsDefaultMergeStyle string
EnableAutodetectManualMerge bool
DefaultDeleteBranchAfterMerge bool
EnableTimetracker bool
AllowOnlyContributorsToTrackTime bool
EnableIssueDependencies bool
@ -551,11 +552,12 @@ func (f *InitializeLabelsForm) Validate(req *http.Request, errs binding.Errors)
type MergePullRequestForm struct {
// required: true
// enum: merge,rebase,rebase-merge,squash,manually-merged
Do string `binding:"Required;In(merge,rebase,rebase-merge,squash,manually-merged)"`
MergeTitleField string
MergeMessageField string
MergeCommitID string // only used for manually-merged
ForceMerge *bool `json:"force_merge,omitempty"`
Do string `binding:"Required;In(merge,rebase,rebase-merge,squash,manually-merged)"`
MergeTitleField string
MergeMessageField string
MergeCommitID string // only used for manually-merged
ForceMerge *bool `json:"force_merge,omitempty"`
DeleteBranchAfterMerge bool `json:"delete_branch_after_merge,omitempty"`
}
// Validate validates the fields

6
services/pull/pull.go
View File

@ -303,7 +303,11 @@ func AddTestPullRequestTask(doer *models.User, repoID int64, branch string, isSy
for _, pr := range prs {
divergence, err := GetDiverging(pr)
if err != nil {
log.Error("GetDiverging: %v", err)
if models.IsErrBranchDoesNotExist(err) && !git.IsBranchExist(pr.HeadRepo.RepoPath(), pr.HeadBranch) {
log.Warn("Cannot test PR %s/%d: head_branch %s no longer exists", pr.BaseRepo.Name, pr.IssueID, pr.HeadBranch)
} else {
jpraet commented 2021-06-30 19:47:21 +00:00
Review
Copy Link

I no longer observe this warning when testing this feature on my local instance, whereas previously I saw it most of the time.
Maybe it has been fixed by another PR. Should I revert these changes to the error handling?

I no longer observe this warning when testing this feature on my local instance, whereas previously I saw it most of the time. Maybe it has been fixed by another PR. Should I revert these changes to the error handling?
zeripath commented 2021-06-30 20:18:51 +00:00
Review
Copy Link

leave it in.

leave it in.
log.Error("GetDiverging: %v", err)
}
} else {
err = pr.UpdateCommitDivergence(divergence.Ahead, divergence.Behind)
if err != nil {

7
services/pull/temp_repo.go
View File

@ -141,10 +141,15 @@ func createTemporaryRepo(pr *models.PullRequest) (string, error) {
trackingBranch := "tracking"
// Fetch head branch
if err := git.NewCommand("fetch", "--no-tags", remoteRepoName, git.BranchPrefix+pr.HeadBranch+":"+trackingBranch).RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil {
log.Error("Unable to fetch head_repo head branch [%s:%s -> tracking in %s]: %v:\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, tmpBasePath, err, outbuf.String(), errbuf.String())
if err := models.RemoveTemporaryPath(tmpBasePath); err != nil {
log.Error("CreateTempRepo: RemoveTemporaryPath: %s", err)
}
if !git.IsBranchExist(pr.HeadRepo.RepoPath(), pr.HeadBranch) {
return "", models.ErrBranchDoesNotExist{
BranchName: pr.HeadBranch,
}
}
log.Error("Unable to fetch head_repo head branch [%s:%s -> tracking in %s]: %v:\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, tmpBasePath, err, outbuf.String(), errbuf.String())
return "", fmt.Errorf("Unable to fetch head_repo head branch [%s:%s -> tracking in tmpBasePath]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, err, outbuf.String(), errbuf.String())
}
outbuf.Reset()

4
services/pull/update.go
View File

@ -88,7 +88,9 @@ func GetDiverging(pr *models.PullRequest) (*git.DivergeObject, error) {
tmpRepo, err := createTemporaryRepo(pr)
if err != nil {
log.Error("CreateTemporaryPath: %v", err)
if !models.IsErrBranchDoesNotExist(err) {
log.Error("CreateTemporaryRepo: %v", err)
}
return nil, err
}
defer func() {

30
templates/repo/issue/view_content/pull.tmpl
View File

@ -315,6 +315,12 @@
<button class="ui button merge-cancel">
{{$.i18n.Tr "cancel"}}
</button>
{{if .IsPullBranchDeletable}}
<div class="ui checkbox ml-2">
<input name="delete_branch_after_merge" type="checkbox" {{if $prUnit.PullRequestsConfig.DefaultDeleteBranchAfterMerge}}checked{{end}}>
<label>{{$.i18n.Tr "repo.branch.delete" .HeadTarget}}</label>
</div>
{{end}}
</form>
</div>
{{end}}
@ -328,6 +334,12 @@
<button class="ui button merge-cancel">
{{$.i18n.Tr "cancel"}}
</button>
{{if .IsPullBranchDeletable}}
<div class="ui checkbox ml-2">
<input name="delete_branch_after_merge" type="checkbox" {{if $prUnit.PullRequestsConfig.DefaultDeleteBranchAfterMerge}}checked{{end}}>
<label>{{$.i18n.Tr "repo.branch.delete" .HeadTarget}}</label>
</div>
{{end}}
</form>
</div>
{{end}}
@ -347,6 +359,12 @@
<button class="ui button merge-cancel">
{{$.i18n.Tr "cancel"}}
</button>
{{if .IsPullBranchDeletable}}
<div class="ui checkbox ml-2">
<input name="delete_branch_after_merge" type="checkbox" {{if $prUnit.PullRequestsConfig.DefaultDeleteBranchAfterMerge}}checked{{end}}>
<label>{{$.i18n.Tr "repo.branch.delete" .HeadTarget}}</label>
</div>
{{end}}
</form>
</div>
{{end}}
@ -366,6 +384,12 @@
<button class="ui button merge-cancel">
{{$.i18n.Tr "cancel"}}
</button>
{{if .IsPullBranchDeletable}}
<div class="ui checkbox ml-2">
<input name="delete_branch_after_merge" type="checkbox" {{if $prUnit.PullRequestsConfig.DefaultDeleteBranchAfterMerge}}checked{{end}}>
<label>{{$.i18n.Tr "repo.branch.delete" .HeadTarget}}</label>
</div>
{{end}}
</form>
</div>
{{end}}
@ -382,6 +406,12 @@
<button class="ui button merge-cancel">
{{$.i18n.Tr "cancel"}}
</button>
{{if .IsPullBranchDeletable}}
<div class="ui checkbox ml-2">
<input name="delete_branch_after_merge" type="checkbox" {{if $prUnit.PullRequestsConfig.DefaultDeleteBranchAfterMerge}}checked{{end}}>
<label>{{$.i18n.Tr "repo.branch.delete" .HeadTarget}}</label>
</div>
{{end}}
</form>
</div>
{{end}}

6
templates/repo/settings/options.tmpl
View File

@ -445,6 +445,12 @@
<label>{{.i18n.Tr "repo.settings.pulls.enable_autodetect_manual_merge"}}</label>
</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}}>
<label>{{.i18n.Tr "repo.settings.pulls.default_delete_branch_after_merge"}}</label>
</div>
</div>
<div class="field">
<p>
{{.i18n.Tr "repo.settings.default_merge_style_desc"}}

9
templates/swagger/v1_json.tmpl
View File

@ -14058,6 +14058,11 @@
"type": "string",
"x-go-name": "DefaultBranch"
},
"default_delete_branch_after_merge": {
"description": "set to `true` to delete pr branch after merge by default",
"type": "boolean",
"x-go-name": "DefaultDeleteBranchAfterMerge"
},
"default_merge_style": {
"description": "set to a merge style to be used by this repository: \"merge\", \"rebase\", \"rebase-merge\", or \"squash\". `has_pull_requests` must be `true`.",
"type": "string",
@ -15137,6 +15142,10 @@
"MergeTitleField": {
"type": "string"
},
"delete_branch_after_merge": {
"type": "boolean",
"x-go-name": "DeleteBranchAfterMerge"
},
"force_merge": {
"type": "boolean",
"x-go-name": "ForceMerge"