Improve the way how branches are deleted

Delete branch from HeadRepo instead of BaseRepo
Prevent the deletion of a master branch
Show a yes/no overlay when you press the delete branch button

Author: Bwko

options/locale/locale_en-US.ini

@@ -591,7 +591,6 @@ pulls.cannot_auto_merge_desc = This pull request can't be merged automatically b
 pulls.cannot_auto_merge_helper = Please merge manually in order to resolve the conflicts.
 pulls.merge_pull_request = Merge Pull Request
 pulls.open_unmerged_pull_exists = `You can't perform reopen operation because there is already an open pull request (#%d) from same repository with same merge information and is waiting for merging.`
-pulls.delete_branch = Delete Branch
 
 milestones.new = New Milestone
 milestones.open_tab = %d Open
@@ -815,6 +814,14 @@ release.tag_name_already_exist = Release with this tag name already exists.
 release.tag_name_invalid = Tag name is not valid.
 release.downloads = Downloads
 
+branch.delete = Delete Branch %s
+branch.delete_desc = Once you delete a branch, there is no going back. Please be certain.
+branch.delete_notices_1 = - This operation <strong>CANNOT</strong> be undone.
+branch.delete_notices_2 = - This operation will permanently delete everything of branch %s.
+branch.deletion_success = %s has been deleted successfully!
+branch.deletion_failed = Failed to delete branch %s.
+branch.delete_branch_has_new_commits =  %s cannot be deleted because it has new commits after mergence.
+
 [org]
 org_name_holder = Organization Name
 org_full_name_holder = Organization Full Name

routers/repo/branch.go

@@ -8,6 +8,7 @@ import (
 	"code.gitea.io/git"
 	"code.gitea.io/gitea/modules/base"
 	"code.gitea.io/gitea/modules/context"
+	"code.gitea.io/gitea/modules/log"
 )
 
 const (
@@ -35,17 +36,46 @@ func Branches(ctx *context.Context) {
 // DeleteBranchPost responses for delete merged branch
 func DeleteBranchPost(ctx *context.Context) {
 	branchName := ctx.Params(":name")
+	commitID := ctx.Query("commit")
+
+	defer func() {
+		redirectTo := ctx.Query("redirect_to")
+		if len(redirectTo) == 0 {
+			redirectTo = ctx.Repo.RepoLink
+		}
+
+		ctx.JSON(200, map[string]interface{}{
+			"redirect": redirectTo,
+		})
+	}()
+
+	fullBranchName := ctx.Repo.Owner.Name + "/" + branchName
+
+	if !ctx.Repo.GitRepo.IsBranchExist(branchName) || branchName == "master" {
+		ctx.Flash.Error(ctx.Tr("repo.branch.deletion_failed", fullBranchName))
+		return
+	}
+
+	if len(commitID) > 0 {
+		branchCommitID, err := ctx.Repo.GitRepo.GetBranchCommitID(branchName)
+		if err != nil {
+			log.Error(4, "GetBranchCommitID: %v", err)
+			return
+		}
+
+		if branchCommitID != commitID {
+			ctx.Flash.Error(ctx.Tr("repo.branch.delete_branch_has_new_commits", fullBranchName))
+			return
+		}
+	}
 
 	if err := ctx.Repo.GitRepo.DeleteBranch(branchName, git.DeleteBranchOptions{
 		Force: false,
 	}); err != nil {
-		ctx.Handle(500, "DeleteBranch", err)
+		log.Error(4, "DeleteBranch: %v", err)
+		ctx.Flash.Error(ctx.Tr("repo.branch.deletion_failed", fullBranchName))
 		return
 	}
 
-	redirectTo := ctx.Query("redirect_to")
-	if len(redirectTo) == 0 {
-		redirectTo = ctx.Repo.RepoLink
-	}
-	ctx.Redirect(redirectTo)
+	ctx.Flash.Success(ctx.Tr("repo.branch.deletion_success", fullBranchName))
 }

routers/repo/issue.go

@@ -17,6 +17,7 @@ import (
 	"github.com/Unknwon/com"
 	"github.com/Unknwon/paginater"
 
+	"code.gitea.io/git"
 	"code.gitea.io/gitea/models"
 	"code.gitea.io/gitea/modules/auth"
 	"code.gitea.io/gitea/modules/base"
@@ -182,7 +183,6 @@ func Issues(ctx *context.Context) {
 	pager := paginater.New(total, setting.UI.IssuePagingNum, page, 5)
 	ctx.Data["Page"] = pager
 
-
 	var issues []*models.Issue
 	if forceEmpty {
 		issues = []*models.Issue{}
@@ -663,11 +663,21 @@ func ViewIssue(ctx *context.Context) {
 
 	if issue.IsPull {
 		pull := issue.PullRequest
-		ctx.Data["IsPullBranchDeletable"] = ctx.Repo.IsWriter() && ctx.Repo.GitRepo.IsBranchExist(pull.HeadBranch)
+		canDelete := false
+
+		if ctx.IsSigned && pull.HeadBranch != "master" {
+
+			if err := pull.GetHeadRepo(); err != nil {
+				log.Error(4, "GetHeadRepo: %v", err)
+			} else if ctx.User.IsWriterOfRepo(pull.HeadRepo) {
+				canDelete = true
+				deleteBranchURL := pull.HeadRepo.Link() + "/branches/" + pull.HeadBranch + "/delete"
+				ctx.Data["DeleteBranchLink"] = fmt.Sprintf("%s?commit=%s&redirect_to=%s", deleteBranchURL, pull.MergedCommitID, ctx.Data["Link"])
+
+			}
+		}
 
-		deleteBranchURL := ctx.Repo.RepoLink + "/branches/" + pull.HeadBranch + "/delete"
-		queryParams := "?redirect_to=" + ctx.Data["Link"].(string)
-		ctx.Data["DeleteBranchLink"] = deleteBranchURL + queryParams
+		ctx.Data["IsPullBranchDeletable"] = canDelete && git.IsBranchExist(pull.HeadRepo.RepoPath(), pull.HeadBranch)
 	}
 
 	ctx.Data["Participants"] = participants

templates/repo/issue/view_content.tmpl

@@ -166,10 +166,7 @@
 								{{if .IsPullBranchDeletable}}
 									<div class="ui divider"></div>
 									<div>
-										<form class="ui form" action="{{.DeleteBranchLink}}" method="post">
-											{{.CsrfTokenHtml}}
-											<button class="ui red button">{{$.i18n.Tr "repo.pulls.delete_branch"}}</button>
-										</form>
+										<a class="delete-button ui red button" href="" data-url="{{.DeleteBranchLink}}">{{$.i18n.Tr "repo.branch.delete" .HeadTarget}}</a>
 									</div>
 								{{end}}
 							{{else if .Issue.IsClosed}}
@@ -380,3 +377,16 @@
 <div class="hide" id="no-content">
 	<span class="no-content">{{.i18n.Tr "repo.issues.no_content"}}</span>
 </div>
+
+<div class="ui small basic delete modal">
+	<div class="ui icon header">
+		<i class="trash icon"></i>
+		{{.i18n.Tr "repo.branch.delete" .HeadTarget | Safe}}
+	</div>
+	<div class="content">
+		<p>{{.i18n.Tr "repo.branch.delete_desc" | Safe}}</p>
+		{{.i18n.Tr "repo.branch.delete_notices_1" | Safe}}<br>
+		{{.i18n.Tr "repo.branch.delete_notices_2" .HeadTarget | Safe}}<br>
+	</div>
+	{{template "base/delete_modal_actions" .}}
+</div>