Merge pull request '[BUG] Fix pull request reopen conditions' (#2373) from gusted/forgejo-reopen-pr into forgejo

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/2373
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
Reviewed-by: Otto <otto@codeberg.org>

options/locale/locale_en-US.ini

@@ -1859,6 +1859,8 @@ pulls.cmd_instruction_merge_title = Merge
 pulls.cmd_instruction_merge_desc = Merge the changes and update on Forgejo.
 pulls.clear_merge_message = Clear merge message
 pulls.clear_merge_message_hint = Clearing the merge message will only remove the commit message content and keep generated git trailers such as "Co-Authored-By …".
+pulls.reopen_failed.head_branch = The pull request cannot be reopened, because the head branch doesn't exist anymore.
+pulls.reopen_failed.base_branch = The pull request cannot be reopened, because the base branch doesn't exist anymore.
 
 pulls.auto_merge_button_when_succeed = (When checks succeed)
 pulls.auto_merge_when_succeed = Auto merge when all checks succeed

routers/web/repo/issue.go

@@ -3028,27 +3028,34 @@ func NewComment(ctx *context.Context) {
 				// check whether the ref of PR <refs/pulls/pr_index/head> in base repo is consistent with the head commit of head branch in the head repo
 				// get head commit of PR
 				if pull.Flow == issues_model.PullRequestFlowGithub {
-					prHeadRef := pull.GetGitRefName()
 					if err := pull.LoadBaseRepo(ctx); err != nil {
 						ctx.ServerError("Unable to load base repo", err)
 						return
 					}
+					if err := pull.LoadHeadRepo(ctx); err != nil {
+						ctx.ServerError("Unable to load head repo", err)
+						return
+					}
+
+					// Check if the base branch of the pull request still exists.
+					if ok := git.IsBranchExist(ctx, pull.BaseRepo.RepoPath(), pull.BaseBranch); !ok {
+						ctx.JSONError(ctx.Tr("repo.pulls.reopen_failed.base_branch"))
+						return
+					}
+
+					// Check if the head branch of the pull request still exists.
+					if ok := git.IsBranchExist(ctx, pull.HeadRepo.RepoPath(), pull.HeadBranch); !ok {
+						ctx.JSONError(ctx.Tr("repo.pulls.reopen_failed.head_branch"))
+						return
+					}
+
+					prHeadRef := pull.GetGitRefName()
 					prHeadCommitID, err := git.GetFullCommitID(ctx, pull.BaseRepo.RepoPath(), prHeadRef)
 					if err != nil {
 						ctx.ServerError("Get head commit Id of pr fail", err)
 						return
 					}
 
-					// get head commit of branch in the head repo
-					if err := pull.LoadHeadRepo(ctx); err != nil {
-						ctx.ServerError("Unable to load head repo", err)
-						return
-					}
-					if ok := git.IsBranchExist(ctx, pull.HeadRepo.RepoPath(), pull.BaseBranch); !ok {
-						// todo localize
-						ctx.JSONError("The origin branch is delete, cannot reopen.")
-						return
-					}
 					headBranchRef := pull.GetGitHeadBranchRefName()
 					headBranchCommitID, err := git.GetFullCommitID(ctx, pull.HeadRepo.RepoPath(), headBranchRef)
 					if err != nil {

tests/integration/pull_reopen_test.go

@@ -0,0 +1,214 @@
+// Copyright 2024 The Forgejo Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package integration
+
+import (
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"strings"
+	"testing"
+	"time"
+
+	"code.gitea.io/gitea/models/db"
+	git_model "code.gitea.io/gitea/models/git"
+	issues_model "code.gitea.io/gitea/models/issues"
+	unit_model "code.gitea.io/gitea/models/unit"
+	"code.gitea.io/gitea/models/unittest"
+	user_model "code.gitea.io/gitea/models/user"
+	gitea_context "code.gitea.io/gitea/modules/context"
+	"code.gitea.io/gitea/modules/git"
+	"code.gitea.io/gitea/modules/translation"
+	issue_service "code.gitea.io/gitea/services/issue"
+	pull_service "code.gitea.io/gitea/services/pull"
+	repo_service "code.gitea.io/gitea/services/repository"
+	files_service "code.gitea.io/gitea/services/repository/files"
+	"code.gitea.io/gitea/tests"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestPullrequestReopen(t *testing.T) {
+	onGiteaRun(t, func(t *testing.T, u *url.URL) {
+		user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
+		org26 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 26})
+
+		// Create an base repository.
+		baseRepo, _, f := CreateDeclarativeRepo(t, user2, "reopen-base",
+			[]unit_model.Type{unit_model.TypePullRequests}, nil, nil,
+		)
+		defer f()
+
+		// Create a new branch on the base branch, so it can be deleted later.
+		_, err := files_service.ChangeRepoFiles(git.DefaultContext, baseRepo, user2, &files_service.ChangeRepoFilesOptions{
+			Files: []*files_service.ChangeRepoFile{
+				{
+					Operation:     "update",
+					TreePath:      "README.md",
+					ContentReader: strings.NewReader("New README.md"),
+				},
+			},
+			Message:   "Modify README for base",
+			OldBranch: "main",
+			NewBranch: "base-branch",
+			Author: &files_service.IdentityOptions{
+				Name:  user2.Name,
+				Email: user2.Email,
+			},
+			Committer: &files_service.IdentityOptions{
+				Name:  user2.Name,
+				Email: user2.Email,
+			},
+			Dates: &files_service.CommitDateOptions{
+				Author:    time.Now(),
+				Committer: time.Now(),
+			},
+		})
+		assert.NoError(t, err)
+
+		// Create an head repository.
+		headRepo, err := repo_service.ForkRepository(git.DefaultContext, user2, org26, repo_service.ForkRepoOptions{
+			BaseRepo: baseRepo,
+			Name:     "reopen-head",
+		})
+		assert.NoError(t, err)
+		assert.NotEmpty(t, headRepo)
+
+		// Add a change to the head repository, so a pull request can be opened.
+		_, err = files_service.ChangeRepoFiles(git.DefaultContext, headRepo, user2, &files_service.ChangeRepoFilesOptions{
+			Files: []*files_service.ChangeRepoFile{
+				{
+					Operation:     "update",
+					TreePath:      "README.md",
+					ContentReader: strings.NewReader("Updated README.md"),
+				},
+			},
+			Message:   "Modify README for head",
+			OldBranch: "main",
+			NewBranch: "head-branch",
+			Author: &files_service.IdentityOptions{
+				Name:  user2.Name,
+				Email: user2.Email,
+			},
+			Committer: &files_service.IdentityOptions{
+				Name:  user2.Name,
+				Email: user2.Email,
+			},
+			Dates: &files_service.CommitDateOptions{
+				Author:    time.Now(),
+				Committer: time.Now(),
+			},
+		})
+		assert.NoError(t, err)
+
+		// Create the pull reuqest.
+		pullIssue := &issues_model.Issue{
+			RepoID:   baseRepo.ID,
+			Title:    "Testing reopen functionality",
+			PosterID: user2.ID,
+			Poster:   user2,
+			IsPull:   true,
+		}
+		pullRequest := &issues_model.PullRequest{
+			HeadRepoID: headRepo.ID,
+			BaseRepoID: baseRepo.ID,
+			HeadBranch: "head-branch",
+			BaseBranch: "base-branch",
+			HeadRepo:   headRepo,
+			BaseRepo:   baseRepo,
+			Type:       issues_model.PullRequestGitea,
+		}
+		err = pull_service.NewPullRequest(git.DefaultContext, baseRepo, pullIssue, nil, nil, pullRequest, nil)
+		assert.NoError(t, err)
+
+		issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{Title: "Testing reopen functionality"})
+
+		// Close the PR.
+		err = issue_service.ChangeStatus(db.DefaultContext, issue, user2, "", true)
+		assert.NoError(t, err)
+
+		session := loginUser(t, "user2")
+
+		reopenPR := func(t *testing.T, expectedStatus int) *httptest.ResponseRecorder {
+			t.Helper()
+
+			link := fmt.Sprintf("%s/pulls/%d/comments", baseRepo.FullName(), issue.Index)
+			req := NewRequestWithValues(t, "POST", link, map[string]string{
+				"_csrf":  GetCSRF(t, session, fmt.Sprintf("%s/pulls/%d", baseRepo.FullName(), issue.Index)),
+				"status": "reopen",
+			})
+			return session.MakeRequest(t, req, expectedStatus)
+		}
+
+		restoreBranch := func(t *testing.T, repoName, branchName string, branchID int64) {
+			t.Helper()
+
+			link := fmt.Sprintf("/%s/branches", repoName)
+			req := NewRequestWithValues(t, "POST", fmt.Sprintf("%s/restore?branch_id=%d&name=%s", link, branchID, branchName), map[string]string{
+				"_csrf": GetCSRF(t, session, link),
+			})
+			session.MakeRequest(t, req, http.StatusOK)
+
+			flashCookie := session.GetCookie(gitea_context.CookieNameFlash)
+			assert.NotNil(t, flashCookie)
+			assert.Contains(t, flashCookie.Value, "success%3DBranch%2B%2522"+branchName+"%2522%2Bhas%2Bbeen%2Brestored.")
+		}
+
+		deleteBranch := func(t *testing.T, repoName, branchName string) {
+			t.Helper()
+
+			link := fmt.Sprintf("/%s/branches", repoName)
+			req := NewRequestWithValues(t, "POST", fmt.Sprintf("%s/delete?name=%s", link, branchName), map[string]string{
+				"_csrf": GetCSRF(t, session, link),
+			})
+			session.MakeRequest(t, req, http.StatusOK)
+
+			flashCookie := session.GetCookie(gitea_context.CookieNameFlash)
+			assert.NotNil(t, flashCookie)
+			assert.Contains(t, flashCookie.Value, "success%3DBranch%2B%2522"+branchName+"%2522%2Bhas%2Bbeen%2Bdeleted.")
+		}
+
+		type errorJSON struct {
+			Error string `json:"errorMessage"`
+		}
+
+		t.Run("Base branch deleted", func(t *testing.T) {
+			defer tests.PrintCurrentTest(t)()
+
+			branch := unittest.AssertExistsAndLoadBean(t, &git_model.Branch{Name: "base-branch", RepoID: baseRepo.ID})
+			defer func() {
+				restoreBranch(t, baseRepo.FullName(), branch.Name, branch.ID)
+			}()
+
+			deleteBranch(t, baseRepo.FullName(), branch.Name)
+			resp := reopenPR(t, http.StatusBadRequest)
+
+			var errorResp errorJSON
+			DecodeJSON(t, resp, &errorResp)
+			assert.EqualValues(t, translation.NewLocale("en-US").Tr("repo.pulls.reopen_failed.base_branch"), errorResp.Error)
+		})
+
+		t.Run("Head branch deleted", func(t *testing.T) {
+			defer tests.PrintCurrentTest(t)()
+
+			branch := unittest.AssertExistsAndLoadBean(t, &git_model.Branch{Name: "head-branch", RepoID: headRepo.ID})
+			defer func() {
+				restoreBranch(t, headRepo.FullName(), branch.Name, branch.ID)
+			}()
+
+			deleteBranch(t, headRepo.FullName(), branch.Name)
+			resp := reopenPR(t, http.StatusBadRequest)
+
+			var errorResp errorJSON
+			DecodeJSON(t, resp, &errorResp)
+			assert.EqualValues(t, translation.NewLocale("en-US").Tr("repo.pulls.reopen_failed.head_branch"), errorResp.Error)
+		})
+
+		t.Run("Normal", func(t *testing.T) {
+			defer tests.PrintCurrentTest(t)()
+
+			reopenPR(t, http.StatusOK)
+		})
+	})
+}