Skip to content

Commit bede0e2

Browse files
lafrikslafriks
committed
Make branch deletion URL more like GitHub's, fixes go-gitea#1397 
1 parent 165cf33 commit bede0e2

File tree

4 files changed

+118
-68
lines changed

4 files changed

+118
-68
lines changed

‎routers/repo/branch.go

-59
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,8 @@
55
package repo
66

77
import (
8-
"code.gitea.io/git"
9-
"code.gitea.io/gitea/models"
108
"code.gitea.io/gitea/modules/base"
119
"code.gitea.io/gitea/modules/context"
12-
"code.gitea.io/gitea/modules/log"
1310
)
1411

1512
const (
@@ -33,59 +30,3 @@ func Branches(ctx *context.Context) {
3330
ctx.Data["Branches"] = brs
3431
ctx.HTML(200, tplBranch)
3532
}
36-
37-
// DeleteBranchPost responses for delete merged branch
38-
func DeleteBranchPost(ctx *context.Context) {
39-
branchName := ctx.Params(":name")
40-
commitID := ctx.Query("commit")
41-
42-
defer func() {
43-
redirectTo := ctx.Query("redirect_to")
44-
if len(redirectTo) == 0 {
45-
redirectTo = ctx.Repo.RepoLink
46-
}
47-
48-
ctx.JSON(200, map[string]interface{}{
49-
"redirect": redirectTo,
50-
})
51-
}()
52-
53-
fullBranchName := ctx.Repo.Owner.Name + "/" + branchName
54-
55-
if !ctx.Repo.GitRepo.IsBranchExist(branchName) || branchName == "master" {
56-
ctx.Flash.Error(ctx.Tr("repo.branch.deletion_failed", fullBranchName))
57-
return
58-
}
59-
60-
if len(commitID) > 0 {
61-
branchCommitID, err := ctx.Repo.GitRepo.GetBranchCommitID(branchName)
62-
if err != nil {
63-
log.Error(4, "GetBranchCommitID: %v", err)
64-
return
65-
}
66-
67-
if branchCommitID != commitID {
68-
ctx.Flash.Error(ctx.Tr("repo.branch.delete_branch_has_new_commits", fullBranchName))
69-
return
70-
}
71-
}
72-
73-
if err := ctx.Repo.GitRepo.DeleteBranch(branchName, git.DeleteBranchOptions{
74-
Force: true,
75-
}); err != nil {
76-
log.Error(4, "DeleteBranch: %v", err)
77-
ctx.Flash.Error(ctx.Tr("repo.branch.deletion_failed", fullBranchName))
78-
return
79-
}
80-
81-
issueID := ctx.QueryInt64("issue_id")
82-
if issueID > 0 {
83-
if err := models.AddDeletePRBranchComment(ctx.User, ctx.Repo.Repository, issueID, branchName); err != nil {
84-
log.Error(4, "DeleteBranch: %v", err)
85-
ctx.Flash.Error(ctx.Tr("repo.branch.deletion_failed", fullBranchName))
86-
return
87-
}
88-
}
89-
90-
ctx.Flash.Success(ctx.Tr("repo.branch.deletion_success", fullBranchName))
91-
}

‎routers/repo/issue.go

+3-6
Original file line numberDiff line numberDiff line change
@@ -647,15 +647,12 @@ func ViewIssue(ctx *context.Context) {
647647
pull := issue.PullRequest
648648
canDelete := false
649649

650-
if ctx.IsSigned && pull.HeadBranch != "master" {
650+
if ctx.IsSigned {
651651
if err := pull.GetHeadRepo(); err != nil {
652652
log.Error(4, "GetHeadRepo: %v", err)
653-
} else if ctx.User.IsWriterOfRepo(pull.HeadRepo) {
653+
} else if pull.HeadBranch != pull.HeadRepo.DefaultBranch && ctx.User.IsWriterOfRepo(pull.HeadRepo) {
654654
canDelete = true
655-
deleteBranchURL := pull.HeadRepo.Link() + "/branches/" + pull.HeadBranch + "/delete"
656-
ctx.Data["DeleteBranchLink"] = fmt.Sprintf("%s?commit=%s&redirect_to=%s&issue_id=%d",
657-
deleteBranchURL, pull.MergedCommitID, ctx.Data["Link"], issue.ID)
658-
655+
ctx.Data["DeleteBranchLink"] = ctx.Repo.RepoLink + "/pulls/" + com.ToStr(issue.Index) + "/cleanup"
659656
}
660657
}
661658

‎routers/repo/pull.go

+114
Original file line numberDiff line numberDiff line change
@@ -757,3 +757,117 @@ func TriggerTask(ctx *context.Context) {
757757
go models.AddTestPullRequestTask(pusher, repo.ID, branch, true)
758758
ctx.Status(202)
759759
}
760+
761+
// CleanUpPullRequest responses for delete merged branch when PR has been merged
762+
func CleanUpPullRequest(ctx *context.Context) {
763+
issue := checkPullInfo(ctx)
764+
if ctx.Written() {
765+
return
766+
}
767+
768+
pr, err := models.GetPullRequestByIssueID(issue.ID)
769+
if err != nil {
770+
if models.IsErrPullRequestNotExist(err) {
771+
ctx.Handle(404, "GetPullRequestByIssueID", nil)
772+
} else {
773+
ctx.Handle(500, "GetPullRequestByIssueID", err)
774+
}
775+
return
776+
}
777+
778+
// Allow cleanup only for merged PR
779+
if !pr.HasMerged {
780+
ctx.Handle(404, "CleanUpPullRequest", nil)
781+
return
782+
}
783+
784+
if err = pr.GetHeadRepo(); err != nil {
785+
ctx.Handle(500, "GetHeadRepo", err)
786+
return
787+
} else if pr.GetBaseRepo(); err != nil {
788+
ctx.Handle(500, "GetBaseRepo", err)
789+
return
790+
} else if pr.HeadRepo.GetOwner(); err != nil {
791+
ctx.Handle(500, "GetOwner", err)
792+
return
793+
}
794+
795+
if !ctx.User.IsWriterOfRepo(pr.HeadRepo) {
796+
ctx.Handle(404, "CleanUpPullRequest", nil)
797+
return
798+
}
799+
800+
fullBranchName := pr.HeadRepo.Owner.Name + "/" + pr.HeadBranch
801+
802+
gitRepo, err := git.OpenRepository(pr.HeadRepo.RepoPath())
803+
if err != nil {
804+
ctx.Handle(500, fmt.Sprintf("OpenRepository[%s]", pr.HeadRepo.RepoPath()), err)
805+
return
806+
}
807+
808+
gitBaseRepo, err := git.OpenRepository(pr.BaseRepo.RepoPath())
809+
if err != nil {
810+
ctx.Handle(500, fmt.Sprintf("OpenRepository[%s]", pr.BaseRepo.RepoPath()), err)
811+
return
812+
}
813+
814+
defer func() {
815+
ctx.JSON(200, map[string]interface{}{
816+
"redirect": pr.BaseRepo.Link() + "/pulls/" + com.ToStr(issue.Index),
817+
})
818+
}()
819+
820+
if pr.HeadBranch == pr.HeadRepo.DefaultBranch || !gitRepo.IsBranchExist(pr.HeadBranch) {
821+
ctx.Flash.Error(ctx.Tr("repo.branch.deletion_failed", fullBranchName))
822+
return
823+
}
824+
825+
// Check if branch has no new commits
826+
if len(pr.MergedCommitID) > 0 {
827+
branchCommitID, err := gitRepo.GetBranchCommitID(pr.HeadBranch)
828+
if err != nil {
829+
log.Error(4, "GetBranchCommitID: %v", err)
830+
ctx.Flash.Error(ctx.Tr("repo.branch.deletion_failed", fullBranchName))
831+
return
832+
}
833+
834+
commit, err := gitBaseRepo.GetCommit(pr.MergedCommitID)
835+
if err != nil {
836+
log.Error(4, "GetCommit: %v", err)
837+
ctx.Flash.Error(ctx.Tr("repo.branch.deletion_failed", fullBranchName))
838+
return
839+
}
840+
841+
isParent := false
842+
for i := 0; i < commit.ParentCount(); i++ {
843+
if parent, err := commit.Parent(i); err != nil {
844+
log.Error(4, "Parent: %v", err)
845+
ctx.Flash.Error(ctx.Tr("repo.branch.deletion_failed", fullBranchName))
846+
return
847+
} else if parent.ID.String() == branchCommitID {
848+
isParent = true
849+
break
850+
}
851+
}
852+
853+
if !isParent {
854+
ctx.Flash.Error(ctx.Tr("repo.branch.delete_branch_has_new_commits", fullBranchName))
855+
return
856+
}
857+
}
858+
859+
if err := gitRepo.DeleteBranch(pr.HeadBranch, git.DeleteBranchOptions{
860+
Force: true,
861+
}); err != nil {
862+
log.Error(4, "DeleteBranch: %v", err)
863+
ctx.Flash.Error(ctx.Tr("repo.branch.deletion_failed", fullBranchName))
864+
return
865+
}
866+
867+
if err := models.AddDeletePRBranchComment(ctx.User, pr.BaseRepo, issue.ID, pr.HeadBranch); err != nil {
868+
// Do not fail here as branch has already been deleted
869+
log.Error(4, "DeleteBranch: %v", err)
870+
}
871+
872+
ctx.Flash.Success(ctx.Tr("repo.branch.deletion_success", fullBranchName))
873+
}

‎routers/routes/routes.go

+1-3
Original file line numberDiff line numberDiff line change
@@ -562,9 +562,6 @@ func RegisterRoutes(m *macaron.Macaron) {
562562
m.Get("/milestones", repo.Milestones)
563563
}, context.RepoRef())
564564

565-
// m.Get("/branches", repo.Branches)
566-
m.Post("/branches/:name/delete", reqSignIn, reqRepoWriter, repo.MustBeNotBare, repo.DeleteBranchPost)
567-
568565
m.Group("/wiki", func() {
569566
m.Get("/?:page", repo.Wiki)
570567
m.Get("/_pages", repo.WikiPages)
@@ -589,6 +586,7 @@ func RegisterRoutes(m *macaron.Macaron) {
589586
m.Get("/commits", context.RepoRef(), repo.ViewPullCommits)
590587
m.Get("/files", context.RepoRef(), repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.ViewPullFiles)
591588
m.Post("/merge", reqRepoWriter, repo.MergePullRequest)
589+
m.Post("/cleanup", context.RepoRef(), repo.CleanUpPullRequest)
592590
}, repo.MustAllowPulls, context.CheckUnit(models.UnitTypePullRequests))
593591

594592
m.Group("", func() {

0 commit comments

Comments
 (0)