Skip to content

Commit 0909695

Browse files
lunnylafriks
lunny
and
lafriks
authored
Merge all deleteBranch as one function and also fix bug when delete branch don't close related PRs (#16067)
* Fix bug when delete branch don't close related PRs * Merge all deletebranch as one method * Add missed branch.go * fix comment Co-authored-by: Lauris BH <[email protected]>
1 parent 3607f79 commit 0909695

File tree

4 files changed

+111
-146
lines changed

4 files changed

+111
-146
lines changed

‎routers/api/v1/repo/branch.go‎

Lines changed: 10 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,14 @@
66
package repo
77

88
import (
9+
"errors"
910
"fmt"
1011
"net/http"
1112

1213
"code.gitea.io/gitea/models"
1314
"code.gitea.io/gitea/modules/context"
1415
"code.gitea.io/gitea/modules/convert"
1516
"code.gitea.io/gitea/modules/git"
16-
"code.gitea.io/gitea/modules/log"
1717
repo_module "code.gitea.io/gitea/modules/repository"
1818
api "code.gitea.io/gitea/modules/structs"
1919
"code.gitea.io/gitea/modules/web"
@@ -117,62 +117,20 @@ func DeleteBranch(ctx *context.APIContext) {
117117

118118
branchName := ctx.Params("*")
119119

120-
if ctx.Repo.Repository.DefaultBranch == branchName {
121-
ctx.Error(http.StatusForbidden, "DefaultBranch", fmt.Errorf("can not delete default branch"))
122-
return
123-
}
124-
125-
isProtected, err := ctx.Repo.Repository.IsProtectedBranch(branchName, ctx.User)
126-
if err != nil {
127-
ctx.InternalServerError(err)
128-
return
129-
}
130-
if isProtected {
131-
ctx.Error(http.StatusForbidden, "IsProtectedBranch", fmt.Errorf("branch protected"))
132-
return
133-
}
134-
135-
branch, err := repo_module.GetBranch(ctx.Repo.Repository, branchName)
136-
if err != nil {
137-
if git.IsErrBranchNotExist(err) {
120+
if err := repo_service.DeleteBranch(ctx.User, ctx.Repo.Repository, ctx.Repo.GitRepo, branchName); err != nil {
121+
switch {
122+
case git.IsErrBranchNotExist(err):
138123
ctx.NotFound(err)
139-
} else {
140-
ctx.Error(http.StatusInternalServerError, "GetBranch", err)
124+
case errors.Is(err, repo_service.ErrBranchIsDefault):
125+
ctx.Error(http.StatusForbidden, "DefaultBranch", fmt.Errorf("can not delete default branch"))
126+
case errors.Is(err, repo_service.ErrBranchIsProtected):
127+
ctx.Error(http.StatusForbidden, "IsProtectedBranch", fmt.Errorf("branch protected"))
128+
default:
129+
ctx.Error(http.StatusInternalServerError, "DeleteBranch", err)
141130
}
142131
return
143132
}
144133

145-
c, err := branch.GetCommit()
146-
if err != nil {
147-
ctx.Error(http.StatusInternalServerError, "GetCommit", err)
148-
return
149-
}
150-
151-
if err := ctx.Repo.GitRepo.DeleteBranch(branchName, git.DeleteBranchOptions{
152-
Force: true,
153-
}); err != nil {
154-
ctx.Error(http.StatusInternalServerError, "DeleteBranch", err)
155-
return
156-
}
157-
158-
// Don't return error below this
159-
if err := repo_service.PushUpdate(
160-
&repo_module.PushUpdateOptions{
161-
RefFullName: git.BranchPrefix + branchName,
162-
OldCommitID: c.ID.String(),
163-
NewCommitID: git.EmptySHA,
164-
PusherID: ctx.User.ID,
165-
PusherName: ctx.User.Name,
166-
RepoUserName: ctx.Repo.Owner.Name,
167-
RepoName: ctx.Repo.Repository.Name,
168-
}); err != nil {
169-
log.Error("Update: %v", err)
170-
}
171-
172-
if err := ctx.Repo.Repository.AddDeletedBranch(branchName, c.ID.String(), ctx.User.ID); err != nil {
173-
log.Warn("AddDeletedBranch: %v", err)
174-
}
175-
176134
ctx.Status(http.StatusNoContent)
177135
}
178136

‎routers/repo/branch.go‎

Lines changed: 16 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
package repo
77

88
import (
9+
"errors"
910
"fmt"
1011
"net/http"
1112
"strings"
@@ -83,34 +84,23 @@ func Branches(ctx *context.Context) {
8384
func DeleteBranchPost(ctx *context.Context) {
8485
defer redirect(ctx)
8586
branchName := ctx.Query("name")
86-
if branchName == ctx.Repo.Repository.DefaultBranch {
87-
log.Debug("DeleteBranch: Can't delete default branch '%s'", branchName)
88-
ctx.Flash.Error(ctx.Tr("repo.branch.default_deletion_failed", branchName))
89-
return
90-
}
91-
92-
isProtected, err := ctx.Repo.Repository.IsProtectedBranch(branchName, ctx.User)
93-
if err != nil {
94-
log.Error("DeleteBranch: %v", err)
95-
ctx.Flash.Error(ctx.Tr("repo.branch.deletion_failed", branchName))
96-
return
97-
}
98-
99-
if isProtected {
100-
log.Debug("DeleteBranch: Can't delete protected branch '%s'", branchName)
101-
ctx.Flash.Error(ctx.Tr("repo.branch.protected_deletion_failed", branchName))
102-
return
103-
}
10487

105-
if !ctx.Repo.GitRepo.IsBranchExist(branchName) {
106-
log.Debug("DeleteBranch: Can't delete non existing branch '%s'", branchName)
107-
ctx.Flash.Error(ctx.Tr("repo.branch.deletion_failed", branchName))
108-
return
109-
}
88+
if err := repo_service.DeleteBranch(ctx.User, ctx.Repo.Repository, ctx.Repo.GitRepo, branchName); err != nil {
89+
switch {
90+
case git.IsErrBranchNotExist(err):
91+
log.Debug("DeleteBranch: Can't delete non existing branch '%s'", branchName)
92+
ctx.Flash.Error(ctx.Tr("repo.branch.deletion_failed", branchName))
93+
case errors.Is(err, repo_service.ErrBranchIsDefault):
94+
log.Debug("DeleteBranch: Can't delete default branch '%s'", branchName)
95+
ctx.Flash.Error(ctx.Tr("repo.branch.default_deletion_failed", branchName))
96+
case errors.Is(err, repo_service.ErrBranchIsProtected):
97+
log.Debug("DeleteBranch: Can't delete protected branch '%s'", branchName)
98+
ctx.Flash.Error(ctx.Tr("repo.branch.protected_deletion_failed", branchName))
99+
default:
100+
log.Error("DeleteBranch: %v", err)
101+
ctx.Flash.Error(ctx.Tr("repo.branch.deletion_failed", branchName))
102+
}
110103

111-
if err := deleteBranch(ctx, branchName); err != nil {
112-
log.Error("DeleteBranch: %v", err)
113-
ctx.Flash.Error(ctx.Tr("repo.branch.deletion_failed", branchName))
114104
return
115105
}
116106

@@ -169,41 +159,6 @@ func redirect(ctx *context.Context) {
169159
})
170160
}
171161

172-
func deleteBranch(ctx *context.Context, branchName string) error {
173-
commit, err := ctx.Repo.GitRepo.GetBranchCommit(branchName)
174-
if err != nil {
175-
log.Error("GetBranchCommit: %v", err)
176-
return err
177-
}
178-
179-
if err := ctx.Repo.GitRepo.DeleteBranch(branchName, git.DeleteBranchOptions{
180-
Force: true,
181-
}); err != nil {
182-
log.Error("DeleteBranch: %v", err)
183-
return err
184-
}
185-
186-
// Don't return error below this
187-
if err := repo_service.PushUpdate(
188-
&repo_module.PushUpdateOptions{
189-
RefFullName: git.BranchPrefix + branchName,
190-
OldCommitID: commit.ID.String(),
191-
NewCommitID: git.EmptySHA,
192-
PusherID: ctx.User.ID,
193-
PusherName: ctx.User.Name,
194-
RepoUserName: ctx.Repo.Owner.Name,
195-
RepoName: ctx.Repo.Repository.Name,
196-
}); err != nil {
197-
log.Error("Update: %v", err)
198-
}
199-
200-
if err := ctx.Repo.Repository.AddDeletedBranch(branchName, commit.ID.String(), ctx.User.ID); err != nil {
201-
log.Warn("AddDeletedBranch: %v", err)
202-
}
203-
204-
return nil
205-
}
206-
207162
// loadBranches loads branches from the repository limited by page & pageSize.
208163
// NOTE: May write to context on error.
209164
func loadBranches(ctx *context.Context, skip, limit int) ([]*Branch, int) {

‎routers/repo/pull.go‎

Lines changed: 13 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ package repo
99
import (
1010
"container/list"
1111
"crypto/subtle"
12+
"errors"
1213
"fmt"
1314
"net/http"
1415
"path"
@@ -21,7 +22,6 @@ import (
2122
"code.gitea.io/gitea/modules/git"
2223
"code.gitea.io/gitea/modules/log"
2324
"code.gitea.io/gitea/modules/notification"
24-
repo_module "code.gitea.io/gitea/modules/repository"
2525
"code.gitea.io/gitea/modules/setting"
2626
"code.gitea.io/gitea/modules/structs"
2727
"code.gitea.io/gitea/modules/upload"
@@ -1186,20 +1186,6 @@ func CleanUpPullRequest(ctx *context.Context) {
11861186
})
11871187
}()
11881188

1189-
if pr.HeadBranch == pr.HeadRepo.DefaultBranch || !gitRepo.IsBranchExist(pr.HeadBranch) {
1190-
ctx.Flash.Error(ctx.Tr("repo.branch.deletion_failed", fullBranchName))
1191-
return
1192-
}
1193-
1194-
// Check if branch is not protected
1195-
if protected, err := pr.HeadRepo.IsProtectedBranch(pr.HeadBranch, ctx.User); err != nil || protected {
1196-
if err != nil {
1197-
log.Error("HeadRepo.IsProtectedBranch: %v", err)
1198-
}
1199-
ctx.Flash.Error(ctx.Tr("repo.branch.deletion_failed", fullBranchName))
1200-
return
1201-
}
1202-
12031189
// Check if branch has no new commits
12041190
headCommitID, err := gitBaseRepo.GetRefCommitID(pr.GetGitRefName())
12051191
if err != nil {
@@ -1218,27 +1204,21 @@ func CleanUpPullRequest(ctx *context.Context) {
12181204
return
12191205
}
12201206

1221-
if err := gitRepo.DeleteBranch(pr.HeadBranch, git.DeleteBranchOptions{
1222-
Force: true,
1223-
}); err != nil {
1224-
log.Error("DeleteBranch: %v", err)
1225-
ctx.Flash.Error(ctx.Tr("repo.branch.deletion_failed", fullBranchName))
1207+
if err := repo_service.DeleteBranch(ctx.User, pr.HeadRepo, gitRepo, pr.HeadBranch); err != nil {
1208+
switch {
1209+
case git.IsErrBranchNotExist(err):
1210+
ctx.Flash.Error(ctx.Tr("repo.branch.deletion_failed", fullBranchName))
1211+
case errors.Is(err, repo_service.ErrBranchIsDefault):
1212+
ctx.Flash.Error(ctx.Tr("repo.branch.deletion_failed", fullBranchName))
1213+
case errors.Is(err, repo_service.ErrBranchIsProtected):
1214+
ctx.Flash.Error(ctx.Tr("repo.branch.deletion_failed", fullBranchName))
1215+
default:
1216+
log.Error("DeleteBranch: %v", err)
1217+
ctx.Flash.Error(ctx.Tr("repo.branch.deletion_failed", fullBranchName))
1218+
}
12261219
return
12271220
}
12281221

1229-
if err := repo_service.PushUpdate(
1230-
&repo_module.PushUpdateOptions{
1231-
RefFullName: git.BranchPrefix + pr.HeadBranch,
1232-
OldCommitID: branchCommitID,
1233-
NewCommitID: git.EmptySHA,
1234-
PusherID: ctx.User.ID,
1235-
PusherName: ctx.User.Name,
1236-
RepoUserName: pr.HeadRepo.Owner.Name,
1237-
RepoName: pr.HeadRepo.Name,
1238-
}); err != nil {
1239-
log.Error("Update: %v", err)
1240-
}
1241-
12421222
if err := models.AddDeletePRBranchComment(ctx.User, pr.BaseRepo, issue.ID, pr.HeadBranch); err != nil {
12431223
// Do not fail here as branch has already been deleted
12441224
log.Error("DeleteBranch: %v", err)

‎services/repository/branch.go‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
// Copyright 2021 The Gitea Authors. All rights reserved.
2+
// Use of this source code is governed by a MIT-style
3+
// license that can be found in the LICENSE file.
4+
5+
package repository
6+
7+
import (
8+
"errors"
9+
10+
"code.gitea.io/gitea/models"
11+
"code.gitea.io/gitea/modules/git"
12+
"code.gitea.io/gitea/modules/log"
13+
repo_module "code.gitea.io/gitea/modules/repository"
14+
pull_service "code.gitea.io/gitea/services/pull"
15+
)
16+
17+
// enmuerates all branch related errors
18+
var (
19+
ErrBranchIsDefault = errors.New("branch is default")
20+
ErrBranchIsProtected = errors.New("branch is protected")
21+
)
22+
23+
// DeleteBranch delete branch
24+
func DeleteBranch(doer *models.User, repo *models.Repository, gitRepo *git.Repository, branchName string) error {
25+
if branchName == repo.DefaultBranch {
26+
return ErrBranchIsDefault
27+
}
28+
29+
isProtected, err := repo.IsProtectedBranch(branchName, doer)
30+
if err != nil {
31+
return err
32+
}
33+
34+
if isProtected {
35+
return ErrBranchIsProtected
36+
}
37+
38+
commit, err := gitRepo.GetBranchCommit(branchName)
39+
if err != nil {
40+
return err
41+
}
42+
43+
if err := gitRepo.DeleteBranch(branchName, git.DeleteBranchOptions{
44+
Force: true,
45+
}); err != nil {
46+
return err
47+
}
48+
49+
if err := pull_service.CloseBranchPulls(doer, repo.ID, branchName); err != nil {
50+
return err
51+
}
52+
53+
// Don't return error below this
54+
if err := PushUpdate(
55+
&repo_module.PushUpdateOptions{
56+
RefFullName: git.BranchPrefix + branchName,
57+
OldCommitID: commit.ID.String(),
58+
NewCommitID: git.EmptySHA,
59+
PusherID: doer.ID,
60+
PusherName: doer.Name,
61+
RepoUserName: repo.OwnerName,
62+
RepoName: repo.Name,
63+
}); err != nil {
64+
log.Error("Update: %v", err)
65+
}
66+
67+
if err := repo.AddDeletedBranch(branchName, commit.ID.String(), doer.ID); err != nil {
68+
log.Warn("AddDeletedBranch: %v", err)
69+
}
70+
71+
return nil
72+
}

0 commit comments

Comments
 (0)