Add integration tests and just return the HTTP status in the handler

routers/web/repo/pull_review.go

@@ -265,7 +265,7 @@ func SubmitReview(ctx *context.Context) {
 			ctx.Flash.Error(ctx.Tr("repo.issues.review.content.empty"))
 			ctx.JSONRedirect(fmt.Sprintf("%s/pulls/%d/files", ctx.Repo.RepoLink, issue.Index))
 		} else if pull_service.IsErrSubmitReviewOnClosedPR(err) {
-			ctx.Error(http.StatusUnprocessableEntity)
+			ctx.Status(http.StatusUnprocessableEntity)
 		} else {
 			ctx.ServerError("SubmitReview", err)
 		}

tests/integration/pull_review_test.go

@@ -5,12 +5,15 @@ package integration
 
 import (
 	"net/http"
+	"net/http/httptest"
 	"net/url"
+	"path"
 	"strings"
 	"testing"
 
 	"code.gitea.io/gitea/models/db"
 	issues_model "code.gitea.io/gitea/models/issues"
+	repo_model "code.gitea.io/gitea/models/repo"
 	"code.gitea.io/gitea/models/unittest"
 	user_model "code.gitea.io/gitea/models/user"
 	"code.gitea.io/gitea/modules/git"
@@ -176,3 +179,83 @@ func TestPullView_CodeOwner(t *testing.T) {
 		})
 	})
 }
+
+func TestPullView_GivenApproveOrRejectReviewOnClosedPR(t *testing.T) {
+	onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) {
+		user1Session := loginUser(t, "user1")
+		user2Session := loginUser(t, "user2")
+
+		// Have user1 create a fork of repo1.
+		testRepoFork(t, user1Session, "user2", "repo1", "user1", "repo1")
+
+		t.Run("Submit approve/reject review on merged PR", func(t *testing.T) {
+			// Create a merged PR (made by user1) in the upstream repo1.
+			testEditFile(t, user1Session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n")
+			resp := testPullCreate(t, user1Session, "user1", "repo1", false, "master", "master", "This is a pull title")
+			elem := strings.Split(test.RedirectURL(resp), "/")
+			assert.EqualValues(t, "pulls", elem[3])
+			testPullMerge(t, user1Session, elem[1], elem[2], elem[4], repo_model.MergeStyleMerge, true)
+
+			// Grab the CSRF token.
+			req := NewRequest(t, "GET", path.Join(elem[1], elem[2], "pulls", elem[4]))
+			resp = user2Session.MakeRequest(t, req, http.StatusOK)
+			htmlDoc := NewHTMLParser(t, resp.Body)
+
+			// Submit an approve review on the PR.
+			testSubmitReview(t, user2Session, htmlDoc.GetCSRF(), "user2", "repo1", elem[4], "approve", http.StatusUnprocessableEntity)
+
+			// Submit a reject review on the PR.
+			testSubmitReview(t, user2Session, htmlDoc.GetCSRF(), "user2", "repo1", elem[4], "reject", http.StatusUnprocessableEntity)
+		})
+
+		t.Run("Submit approve/reject review on closed PR", func(t *testing.T) {
+			// Created a closed PR (made by user1) in the upstream repo1.
+			testEditFile(t, user1Session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n")
+			resp := testPullCreate(t, user1Session, "user1", "repo1", false, "master", "master", "This is a pull title")
+			elem := strings.Split(test.RedirectURL(resp), "/")
+			assert.EqualValues(t, "pulls", elem[3])
+			testIssueClose(t, user1Session, elem[1], elem[2], elem[4])
+
+			// Grab the CSRF token.
+			req := NewRequest(t, "GET", path.Join(elem[1], elem[2], "pulls", elem[4]))
+			resp = user2Session.MakeRequest(t, req, http.StatusOK)
+			htmlDoc := NewHTMLParser(t, resp.Body)
+
+			// Submit an approve review on the PR.
+			testSubmitReview(t, user2Session, htmlDoc.GetCSRF(), "user2", "repo1", elem[4], "approve", http.StatusUnprocessableEntity)
+
+			// Submit a reject review on the PR.
+			testSubmitReview(t, user2Session, htmlDoc.GetCSRF(), "user2", "repo1", elem[4], "reject", http.StatusUnprocessableEntity)
+
+		})
+	})
+}
+
+func testSubmitReview(t *testing.T, session *TestSession, csrf, owner, repo, pullNumber, reviewType string, expectedSubmitStatus int) *httptest.ResponseRecorder {
+	options := map[string]string{
+		"_csrf":     csrf,
+		"commit_id": "",
+		"content":   "test",
+		"type":      reviewType,
+	}
+
+	submitUrl := path.Join(owner, repo, "pulls", pullNumber, "files", "reviews", "submit")
+	req := NewRequestWithValues(t, "POST", submitUrl, options)
+	return session.MakeRequest(t, req, expectedSubmitStatus)
+}
+
+func testIssueClose(t *testing.T, session *TestSession, owner, repo, issueNumber string) *httptest.ResponseRecorder {
+	req := NewRequest(t, "GET", path.Join(owner, repo, "pulls", issueNumber))
+	resp := session.MakeRequest(t, req, http.StatusOK)
+
+	htmlDoc := NewHTMLParser(t, resp.Body)
+	closeUrl := path.Join(owner, repo, "issues", issueNumber, "comments")
+
+	options := map[string]string{
+		"_csrf":  htmlDoc.GetCSRF(),
+		"status": "close",
+	}
+
+	req = NewRequestWithValues(t, "POST", closeUrl, options)
+	return session.MakeRequest(t, req, http.StatusOK)
+}