Skip to content

Commit 16f7b43

Browse files
lunnylafriks
andauthored
Fix pull view when head repository or head branch missed and close related pull requests when delete head repository or head branch (#9927) (#9974)
* fix pull view when head repository or head branch missed and close related pull requests when delete branch * fix pull view broken when head repository deleted * close pull requests when head repositories deleted * Add tests for broken pull request head repository or branch * fix typo * ignore special error when close pull request Co-authored-by: Lauris BH <[email protected]> Co-authored-by: Lauris BH <[email protected]>
1 parent 043febd commit 16f7b43

File tree

5 files changed

+158
-6
lines changed

5 files changed

+158
-6
lines changed

integrations/pull_create_test.go

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,3 +106,57 @@ func TestPullCreate_TitleEscape(t *testing.T) {
106106
assert.Equal(t, "&lt;u&gt;XSS PR&lt;/u&gt;", titleHTML)
107107
})
108108
}
109+
110+
func testUIDeleteBranch(t *testing.T, session *TestSession, ownerName, repoName, branchName string) {
111+
relURL := "/" + path.Join(ownerName, repoName, "branches")
112+
req := NewRequest(t, "GET", relURL)
113+
resp := session.MakeRequest(t, req, http.StatusOK)
114+
htmlDoc := NewHTMLParser(t, resp.Body)
115+
116+
req = NewRequestWithValues(t, "POST", relURL+"/delete", map[string]string{
117+
"_csrf": getCsrf(t, htmlDoc.doc),
118+
"name": branchName,
119+
})
120+
session.MakeRequest(t, req, http.StatusOK)
121+
}
122+
123+
func testDeleteRepository(t *testing.T, session *TestSession, ownerName, repoName string) {
124+
relURL := "/" + path.Join(ownerName, repoName, "settings")
125+
req := NewRequest(t, "GET", relURL)
126+
resp := session.MakeRequest(t, req, http.StatusOK)
127+
htmlDoc := NewHTMLParser(t, resp.Body)
128+
129+
req = NewRequestWithValues(t, "POST", relURL+"?action=delete", map[string]string{
130+
"_csrf": getCsrf(t, htmlDoc.doc),
131+
"repo_name": repoName,
132+
})
133+
session.MakeRequest(t, req, http.StatusFound)
134+
}
135+
136+
func TestPullBranchDelete(t *testing.T) {
137+
onGiteaRun(t, func(t *testing.T, u *url.URL) {
138+
defer prepareTestEnv(t)()
139+
140+
session := loginUser(t, "user1")
141+
testRepoFork(t, session, "user2", "repo1", "user1", "repo1")
142+
testCreateBranch(t, session, "user1", "repo1", "branch/master", "master1", http.StatusFound)
143+
testEditFile(t, session, "user1", "repo1", "master1", "README.md", "Hello, World (Edited)\n")
144+
resp := testPullCreate(t, session, "user1", "repo1", "master1", "This is a pull title")
145+
146+
// check the redirected URL
147+
url := resp.HeaderMap.Get("Location")
148+
assert.Regexp(t, "^/user2/repo1/pulls/[0-9]*$", url)
149+
req := NewRequest(t, "GET", url)
150+
session.MakeRequest(t, req, http.StatusOK)
151+
152+
// delete head branch and confirm pull page is ok
153+
testUIDeleteBranch(t, session, "user1", "repo1", "master1")
154+
req = NewRequest(t, "GET", url)
155+
session.MakeRequest(t, req, http.StatusOK)
156+
157+
// delete head repository and confirm pull page is ok
158+
testDeleteRepository(t, session, "user1", "repo1")
159+
req = NewRequest(t, "GET", url)
160+
session.MakeRequest(t, req, http.StatusOK)
161+
})
162+
}

models/pull.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,11 @@ type PullRequest struct {
6868
// MustHeadUserName returns the HeadRepo's username if failed return blank
6969
func (pr *PullRequest) MustHeadUserName() string {
7070
if err := pr.LoadHeadRepo(); err != nil {
71-
log.Error("LoadHeadRepo: %v", err)
71+
if !IsErrRepoNotExist(err) {
72+
log.Error("LoadHeadRepo: %v", err)
73+
} else {
74+
log.Warn("LoadHeadRepo %d but repository does not exist: %v", pr.HeadRepoID, err)
75+
}
7276
return ""
7377
}
7478
return pr.HeadRepo.MustOwnerName()

modules/repofiles/update.go

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -475,9 +475,18 @@ func PushUpdate(repo *models.Repository, branch string, opts PushUpdateOptions)
475475
return err
476476
}
477477

478-
log.Trace("TriggerTask '%s/%s' by %s", repo.Name, branch, pusher.Name)
478+
if !isDelRef {
479+
if err = models.RemoveDeletedBranch(repo.ID, opts.Branch); err != nil {
480+
log.Error("models.RemoveDeletedBranch %s/%s failed: %v", repo.ID, opts.Branch, err)
481+
}
482+
483+
log.Trace("TriggerTask '%s/%s' by %s", repo.Name, branch, pusher.Name)
479484

480-
go pull_service.AddTestPullRequestTask(pusher, repo.ID, branch, true)
485+
go pull_service.AddTestPullRequestTask(pusher, repo.ID, branch, true)
486+
// close all related pulls
487+
} else if err = pull_service.CloseBranchPulls(pusher, repo.ID, branch); err != nil {
488+
log.Error("close related pull request failed: %v", err)
489+
}
481490

482491
if err = models.WatchIfAuto(opts.PusherID, repo.ID, true); err != nil {
483492
log.Warn("Fail to perform auto watch on user %v for repo %v: %v", opts.PusherID, repo.ID, err)
@@ -524,11 +533,14 @@ func PushUpdates(repo *models.Repository, optsList []*PushUpdateOptions) error {
524533
if err = models.RemoveDeletedBranch(repo.ID, opts.Branch); err != nil {
525534
log.Error("models.RemoveDeletedBranch %s/%s failed: %v", repo.ID, opts.Branch, err)
526535
}
527-
}
528536

529-
log.Trace("TriggerTask '%s/%s' by %s", repo.Name, opts.Branch, pusher.Name)
537+
log.Trace("TriggerTask '%s/%s' by %s", repo.Name, opts.Branch, pusher.Name)
530538

531-
go pull_service.AddTestPullRequestTask(pusher, repo.ID, opts.Branch, true)
539+
go pull_service.AddTestPullRequestTask(pusher, repo.ID, opts.Branch, true)
540+
// close all related pulls
541+
} else if err = pull_service.CloseBranchPulls(pusher, repo.ID, opts.Branch); err != nil {
542+
log.Error("close related pull request failed: %v", err)
543+
}
532544

533545
if err = models.WatchIfAuto(opts.PusherID, repo.ID, true); err != nil {
534546
log.Warn("Fail to perform auto watch on user %v for repo %v: %v", opts.PusherID, repo.ID, err)

services/pull/pull.go

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"fmt"
1010
"os"
1111
"path"
12+
"strings"
1213

1314
"code.gitea.io/gitea/models"
1415
"code.gitea.io/gitea/modules/git"
@@ -261,3 +262,79 @@ func PushToBaseRepo(pr *models.PullRequest) (err error) {
261262

262263
return nil
263264
}
265+
266+
type errlist []error
267+
268+
func (errs errlist) Error() string {
269+
if len(errs) > 0 {
270+
var buf strings.Builder
271+
for i, err := range errs {
272+
if i > 0 {
273+
buf.WriteString(", ")
274+
}
275+
buf.WriteString(err.Error())
276+
}
277+
return buf.String()
278+
}
279+
return ""
280+
}
281+
282+
// CloseBranchPulls close all the pull requests who's head branch is the branch
283+
func CloseBranchPulls(doer *models.User, repoID int64, branch string) error {
284+
prs, err := models.GetUnmergedPullRequestsByHeadInfo(repoID, branch)
285+
if err != nil {
286+
return err
287+
}
288+
289+
prs2, err := models.GetUnmergedPullRequestsByBaseInfo(repoID, branch)
290+
if err != nil {
291+
return err
292+
}
293+
294+
prs = append(prs, prs2...)
295+
if err := models.PullRequestList(prs).LoadAttributes(); err != nil {
296+
return err
297+
}
298+
299+
var errs errlist
300+
for _, pr := range prs {
301+
if err = issue_service.ChangeStatus(pr.Issue, doer, true); err != nil && !models.IsErrIssueWasClosed(err) {
302+
errs = append(errs, err)
303+
}
304+
}
305+
if len(errs) > 0 {
306+
return errs
307+
}
308+
return nil
309+
}
310+
311+
// CloseRepoBranchesPulls close all pull requests which head branches are in the given repository
312+
func CloseRepoBranchesPulls(doer *models.User, repo *models.Repository) error {
313+
branches, err := git.GetBranchesByPath(repo.RepoPath())
314+
if err != nil {
315+
return err
316+
}
317+
318+
var errs errlist
319+
for _, branch := range branches {
320+
prs, err := models.GetUnmergedPullRequestsByHeadInfo(repo.ID, branch.Name)
321+
if err != nil {
322+
return err
323+
}
324+
325+
if err = models.PullRequestList(prs).LoadAttributes(); err != nil {
326+
return err
327+
}
328+
329+
for _, pr := range prs {
330+
if err = issue_service.ChangeStatus(pr.Issue, doer, true); err != nil && !models.IsErrIssueWasClosed(err) {
331+
errs = append(errs, err)
332+
}
333+
}
334+
}
335+
336+
if len(errs) > 0 {
337+
return errs
338+
}
339+
return nil
340+
}

services/repository/repository.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"code.gitea.io/gitea/models"
1111
"code.gitea.io/gitea/modules/log"
1212
"code.gitea.io/gitea/modules/notification"
13+
pull_service "code.gitea.io/gitea/services/pull"
1314
)
1415

1516
// CreateRepository creates a repository for the user/organization.
@@ -48,6 +49,10 @@ func ForkRepository(doer, u *models.User, oldRepo *models.Repository, name, desc
4849

4950
// DeleteRepository deletes a repository for a user or organization.
5051
func DeleteRepository(doer *models.User, repo *models.Repository) error {
52+
if err := pull_service.CloseRepoBranchesPulls(doer, repo); err != nil {
53+
log.Error("CloseRepoBranchesPulls failed: %v", err)
54+
}
55+
5156
if err := models.DeleteRepository(doer, repo.OwnerID, repo.ID); err != nil {
5257
return err
5358
}

0 commit comments

Comments
 (0)