Skip to content

Commit ce2ade0

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) (#16097)
* Fix bug when delete branch don't close related PRs * Merge all deletebranch as one method Co-authored-by: Lauris BH <[email protected]>
1 parent 1e76f7b commit ce2ade0

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
"strings"
1112

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

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

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

@@ -168,41 +158,6 @@ func redirect(ctx *context.Context) {
168158
})
169159
}
170160

171-
func deleteBranch(ctx *context.Context, branchName string) error {
172-
commit, err := ctx.Repo.GitRepo.GetBranchCommit(branchName)
173-
if err != nil {
174-
log.Error("GetBranchCommit: %v", err)
175-
return err
176-
}
177-
178-
if err := ctx.Repo.GitRepo.DeleteBranch(branchName, git.DeleteBranchOptions{
179-
Force: true,
180-
}); err != nil {
181-
log.Error("DeleteBranch: %v", err)
182-
return err
183-
}
184-
185-
// Don't return error below this
186-
if err := repo_service.PushUpdate(
187-
&repo_module.PushUpdateOptions{
188-
RefFullName: git.BranchPrefix + branchName,
189-
OldCommitID: commit.ID.String(),
190-
NewCommitID: git.EmptySHA,
191-
PusherID: ctx.User.ID,
192-
PusherName: ctx.User.Name,
193-
RepoUserName: ctx.Repo.Owner.Name,
194-
RepoName: ctx.Repo.Repository.Name,
195-
}); err != nil {
196-
log.Error("Update: %v", err)
197-
}
198-
199-
if err := ctx.Repo.Repository.AddDeletedBranch(branchName, commit.ID.String(), ctx.User.ID); err != nil {
200-
log.Warn("AddDeletedBranch: %v", err)
201-
}
202-
203-
return nil
204-
}
205-
206161
// loadBranches loads branches from the repository limited by page & pageSize.
207162
// NOTE: May write to context on error.
208163
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"
@@ -22,7 +23,6 @@ import (
2223
"code.gitea.io/gitea/modules/git"
2324
"code.gitea.io/gitea/modules/log"
2425
"code.gitea.io/gitea/modules/notification"
25-
repo_module "code.gitea.io/gitea/modules/repository"
2626
"code.gitea.io/gitea/modules/setting"
2727
"code.gitea.io/gitea/modules/structs"
2828
"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)