Skip to content

Commit 3dc2724

Browse files
authored
Fix cannot reopen after pushing commits to a closed PR (#23189) (#23322)
Backport: #23189 Close: #22784 1. On GH, we can reopen a PR which was closed before after pushing commits. After reopening PR, we can see the commits that were pushed after closing PR in the time line. So the case of [issue](#22784) is a bug which needs to be fixed. 2. After closing a PR and pushing commits, `headBranchSha` is not equal to `sha`(which is the last commit ID string of reference). If the judgement exists, the button of reopen will not display. So, skip the judgement if the status of PR is closed. ![image](https://user-images.githubusercontent.com/33891828/222037529-651fccf9-0bba-433e-b2f0-79c17e0cc812.png) 3. Even if PR is already close, we should still insert comment record into DB when we push commits. So we should still call function `CreatePushPullComment()`. https://github.com/go-gitea/gitea/blob/067b0c2664d127c552ccdfd264257caca4907a77/services/pull/pull.go#L260-L282 So, I add a switch(`includeClosed`) to the `GetUnmergedPullRequestsByHeadInfo` func to control whether the status of PR must be open. In this case, by setting `includeClosed` to `true`, we can query the closed PR. ![image](https://user-images.githubusercontent.com/33891828/222621045-bb80987c-10c5-4eac-aa0c-1fb9c6aefb51.png) 4. In the loop of comments, I use the`latestCloseCommentID` variable to record the last occurrence of the close comment. In the go template, if the status of PR is closed, the comments whose type is `CommentTypePullRequestPush(29)` after `latestCloseCommentID` won't be rendered. ![image](https://user-images.githubusercontent.com/33891828/222058913-c91cf3e3-819b-40c5-8015-654b31eeccff.png) e.g. 1). The initial status of the PR is opened. ![image](https://user-images.githubusercontent.com/33891828/222453617-33c5093e-f712-4cd6-8489-9f87e2075869.png) 2). Then I click the button of `Close`. PR is closed now. ![image](https://user-images.githubusercontent.com/33891828/222453694-25c588a9-c121-4897-9ae5-0b13cf33d20b.png) 3). I try to push a commit to this PR, even though its current status is closed. ![image](https://user-images.githubusercontent.com/33891828/222453916-361678fb-7321-410d-9e37-5a26e8095638.png) But in comments list, this commit do not display.This is as expected :) ![image](https://user-images.githubusercontent.com/33891828/222454169-7617a791-78d2-404e-be5e-77d555f93313.png) 4). Click the `Reopen` button, the commit which is pushed after closing PR display now. ![image](https://user-images.githubusercontent.com/33891828/222454533-897893b6-b96e-4701-b5cb-b1800f382b8f.png)
1 parent 93fe020 commit 3dc2724

File tree

6 files changed

+30
-16
lines changed

6 files changed

+30
-16
lines changed

models/issues/pull_list.go

+9-6
Original file line numberDiff line numberDiff line change
@@ -52,13 +52,16 @@ func listPullRequestStatement(baseRepoID int64, opts *PullRequestsOptions) (*xor
5252

5353
// GetUnmergedPullRequestsByHeadInfo returns all pull requests that are open and has not been merged
5454
// by given head information (repo and branch).
55-
func GetUnmergedPullRequestsByHeadInfo(repoID int64, branch string) ([]*PullRequest, error) {
55+
// arg `includeClosed` controls whether the SQL returns closed PRs
56+
func GetUnmergedPullRequestsByHeadInfo(repoID int64, branch string, includeClosed bool) ([]*PullRequest, error) {
5657
prs := make([]*PullRequest, 0, 2)
57-
return prs, db.GetEngine(db.DefaultContext).
58-
Where("head_repo_id = ? AND head_branch = ? AND has_merged = ? AND issue.is_closed = ? AND flow = ?",
59-
repoID, branch, false, false, PullRequestFlowGithub).
58+
sess := db.GetEngine(db.DefaultContext).
6059
Join("INNER", "issue", "issue.id = pull_request.issue_id").
61-
Find(&prs)
60+
Where("head_repo_id = ? AND head_branch = ? AND has_merged = ? AND flow = ?", repoID, branch, false, PullRequestFlowGithub)
61+
if !includeClosed {
62+
sess.Where("issue.is_closed = ?", false)
63+
}
64+
return prs, sess.Find(&prs)
6265
}
6366

6467
// CanMaintainerWriteToBranch check whether user is a maintainer and could write to the branch
@@ -71,7 +74,7 @@ func CanMaintainerWriteToBranch(p access_model.Permission, branch string, user *
7174
return false
7275
}
7376

74-
prs, err := GetUnmergedPullRequestsByHeadInfo(p.Units[0].RepoID, branch)
77+
prs, err := GetUnmergedPullRequestsByHeadInfo(p.Units[0].RepoID, branch, false)
7578
if err != nil {
7679
return false
7780
}

models/issues/pull_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ func TestHasUnmergedPullRequestsByHeadInfo(t *testing.T) {
119119

120120
func TestGetUnmergedPullRequestsByHeadInfo(t *testing.T) {
121121
assert.NoError(t, unittest.PrepareTestDatabase())
122-
prs, err := issues_model.GetUnmergedPullRequestsByHeadInfo(1, "branch2")
122+
prs, err := issues_model.GetUnmergedPullRequestsByHeadInfo(1, "branch2", false)
123123
assert.NoError(t, err)
124124
assert.Len(t, prs, 1)
125125
for _, pr := range prs {

routers/web/repo/issue.go

+12-5
Original file line numberDiff line numberDiff line change
@@ -1375,11 +1375,12 @@ func ViewIssue(ctx *context.Context) {
13751375
}
13761376

13771377
var (
1378-
role issues_model.RoleDescriptor
1379-
ok bool
1380-
marked = make(map[int64]issues_model.RoleDescriptor)
1381-
comment *issues_model.Comment
1382-
participants = make([]*user_model.User, 1, 10)
1378+
role issues_model.RoleDescriptor
1379+
ok bool
1380+
marked = make(map[int64]issues_model.RoleDescriptor)
1381+
comment *issues_model.Comment
1382+
participants = make([]*user_model.User, 1, 10)
1383+
latestCloseCommentID int64
13831384
)
13841385
if ctx.Repo.Repository.IsTimetrackerEnabled() {
13851386
if ctx.IsSigned {
@@ -1586,9 +1587,15 @@ func ViewIssue(ctx *context.Context) {
15861587
comment.Type == issues_model.CommentTypeStopTracking {
15871588
// drop error since times could be pruned from DB..
15881589
_ = comment.LoadTime()
1590+
} else if comment.Type == issues_model.CommentTypeClose {
1591+
// record ID of latest closed comment.
1592+
// if PR is closed, the comments whose type is CommentTypePullRequestPush(29) after latestCloseCommentID won't be rendered.
1593+
latestCloseCommentID = comment.ID
15891594
}
15901595
}
15911596

1597+
ctx.Data["LatestCloseCommentID"] = latestCloseCommentID
1598+
15921599
// Combine multiple label assignments into a single comment
15931600
combineLabelComments(issue)
15941601

routers/web/repo/pull.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -574,7 +574,7 @@ func PrepareViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *git.C
574574
ctx.Data["HeadBranchCommitID"] = headBranchSha
575575
ctx.Data["PullHeadCommitID"] = sha
576576

577-
if pull.HeadRepo == nil || !headBranchExist || headBranchSha != sha {
577+
if pull.HeadRepo == nil || !headBranchExist || (!pull.Issue.IsClosed && (headBranchSha != sha)) {
578578
ctx.Data["IsPullRequestBroken"] = true
579579
if pull.IsSameRepo() {
580580
ctx.Data["HeadTarget"] = pull.HeadBranch

services/pull/pull.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,7 @@ func AddTestPullRequestTask(doer *user_model.User, repoID int64, branch string,
258258
// If you don't let it run all the way then you will lose data
259259
// TODO: graceful: AddTestPullRequestTask needs to become a queue!
260260

261-
prs, err := issues_model.GetUnmergedPullRequestsByHeadInfo(repoID, branch)
261+
prs, err := issues_model.GetUnmergedPullRequestsByHeadInfo(repoID, branch, true)
262262
if err != nil {
263263
log.Error("Find pull requests [head_repo_id: %d, head_branch: %s]: %v", repoID, branch, err)
264264
return
@@ -502,7 +502,7 @@ func (errs errlist) Error() string {
502502

503503
// CloseBranchPulls close all the pull requests who's head branch is the branch
504504
func CloseBranchPulls(doer *user_model.User, repoID int64, branch string) error {
505-
prs, err := issues_model.GetUnmergedPullRequestsByHeadInfo(repoID, branch)
505+
prs, err := issues_model.GetUnmergedPullRequestsByHeadInfo(repoID, branch, false)
506506
if err != nil {
507507
return err
508508
}
@@ -538,7 +538,7 @@ func CloseRepoBranchesPulls(ctx context.Context, doer *user_model.User, repo *re
538538

539539
var errs errlist
540540
for _, branch := range branches {
541-
prs, err := issues_model.GetUnmergedPullRequestsByHeadInfo(repo.ID, branch.Name)
541+
prs, err := issues_model.GetUnmergedPullRequestsByHeadInfo(repo.ID, branch.Name, false)
542542
if err != nil {
543543
return err
544544
}

templates/repo/issue/view_content/comments.tmpl

+4
Original file line numberDiff line numberDiff line change
@@ -697,6 +697,10 @@
697697
</span>
698698
</div>
699699
{{else if and (eq .Type 29) (or (gt .CommitsNum 0) .IsForcePush)}}
700+
<!-- If PR is closed, the comments whose type is CommentTypePullRequestPush(29) after latestCloseCommentID won't be rendered. //-->
701+
{{if and .Issue.IsClosed (gt .ID $.LatestCloseCommentID)}}
702+
{{continue}}
703+
{{end}}
700704
<div class="timeline-item event" id="{{.HashTag}}">
701705
<span class="badge">{{svg "octicon-repo-push"}}</span>
702706
<span class="text grey muted-links">

0 commit comments

Comments
 (0)