Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add checkbox to delete pull branch after successful merge #16049

Merged
merged 18 commits into from
Jul 12, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 9 additions & 8 deletions models/repo_unit.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 2 additions & 0 deletions modules/structs/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions options/locale/locale_en-US.ini
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
34 changes: 34 additions & 0 deletions routers/api/v1/repo/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
package repo

import (
"errors"
"fmt"
"math"
"net/http"
Expand All @@ -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
Expand Down Expand Up @@ -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)
}

Expand Down
20 changes: 12 additions & 8 deletions routers/api/v1/repo/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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)
}
Expand Down
59 changes: 48 additions & 11 deletions routers/web/repo/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}

Expand Down Expand Up @@ -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
var gitBaseRepo *git.Repository

// 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()
}
defer gitRepo.Close()

gitBaseRepo, err := git.OpenRepository(pr.BaseRepo.RepoPath())
if err != nil {
ctx.ServerError(fmt.Sprintf("OpenRepository[%s]", pr.BaseRepo.RepoPath()), err)
return
// 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{}{
Expand All @@ -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):
Expand All @@ -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)
}
Expand Down
17 changes: 9 additions & 8 deletions routers/web/repo/setting.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
12 changes: 7 additions & 5 deletions services/forms/repo_form.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ type RepoSettingForm struct {
PullsAllowManualMerge bool
PullsDefaultMergeStyle string
EnableAutodetectManualMerge bool
DefaultDeleteBranchAfterMerge bool
EnableTimetracker bool
AllowOnlyContributorsToTrackTime bool
EnableIssueDependencies bool
Expand Down Expand Up @@ -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
Expand Down
6 changes: 5 additions & 1 deletion services/pull/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
jpraet marked this conversation as resolved.
Show resolved Hide resolved
} else {
log.Error("GetDiverging: %v", err)
}
} else {
err = pr.UpdateCommitDivergence(divergence.Ahead, divergence.Behind)
if err != nil {
Expand Down
7 changes: 6 additions & 1 deletion services/pull/temp_repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
4 changes: 3 additions & 1 deletion services/pull/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
30 changes: 30 additions & 0 deletions templates/repo/issue/view_content/pull.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -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}}
Expand All @@ -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}}
Expand All @@ -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}}
Expand All @@ -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}}
Expand All @@ -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}}
Expand Down
6 changes: 6 additions & 0 deletions templates/repo/settings/options.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -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"}}
Expand Down
Loading