From 23676bfea7ccbbe166a554115ea1f5f02800e379 Mon Sep 17 00:00:00 2001
From: Kemal Zebari <60799661+kemzeb@users.noreply.github.com>
Date: Thu, 28 Mar 2024 08:19:24 -0700
Subject: Prevent re-review and dismiss review actions on closed and merged PRs
 (#30065)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Resolves #29965.

---
Manually tested this by:
- Following the
[installation](https://docs.gitea.com/next/installation/install-with-docker#basics)
guide (but built a local Docker image instead)
- Creating 2 users, one who is the `Owner` of a newly-created repository
and the other a `Collaborator`
- Had the `Collaborator` create a PR that the `Owner` reviews
- `Collaborator` resolves conversation and `Owner` merges PR

And with this change we see that we can no longer see re-request review
button for the `Owner`:

<img width="1351" alt="Screenshot 2024-03-25 at 12 39 18 AM"
src="https://github.com/go-gitea/gitea/assets/60799661/bcd9c579-3cf7-474f-a51e-b436fe1a39a4">

(cherry picked from commit 242b331260925e604150346e61329097d5731e77)
---
 routers/api/v1/repo/pull_review.go | 17 +++++++++++------
 routers/web/repo/issue.go          |  4 ++++
 routers/web/repo/pull_review.go    |  4 ++++
 3 files changed, 19 insertions(+), 6 deletions(-)

(limited to 'routers')

diff --git a/routers/api/v1/repo/pull_review.go b/routers/api/v1/repo/pull_review.go
index 9ccbb57c52..77c0d25e2a 100644
--- a/routers/api/v1/repo/pull_review.go
+++ b/routers/api/v1/repo/pull_review.go
@@ -787,6 +787,8 @@ func DeleteReviewRequests(ctx *context.APIContext) {
 	//     "$ref": "#/responses/empty"
 	//   "422":
 	//     "$ref": "#/responses/validationError"
+	//   "403":
+	//     "$ref": "#/responses/forbidden"
 	//   "404":
 	//     "$ref": "#/responses/notFound"
 	opts := web.GetForm(ctx).(*api.PullReviewRequestOptions)
@@ -855,6 +857,10 @@ func apiReviewRequest(ctx *context.APIContext, opts api.PullReviewRequestOptions
 	for _, reviewer := range reviewers {
 		comment, err := issue_service.ReviewRequest(ctx, pr.Issue, ctx.Doer, reviewer, isAdd)
 		if err != nil {
+			if issues_model.IsErrReviewRequestOnClosedPR(err) {
+				ctx.Error(http.StatusForbidden, "", err)
+				return
+			}
 			ctx.Error(http.StatusInternalServerError, "ReviewRequest", err)
 			return
 		}
@@ -1068,7 +1074,7 @@ func dismissReview(ctx *context.APIContext, msg string, isDismiss, dismissPriors
 		ctx.Error(http.StatusForbidden, "", "Must be repo admin")
 		return
 	}
-	review, pr, isWrong := prepareSingleReview(ctx)
+	review, _, isWrong := prepareSingleReview(ctx)
 	if isWrong {
 		return
 	}
@@ -1078,13 +1084,12 @@ func dismissReview(ctx *context.APIContext, msg string, isDismiss, dismissPriors
 		return
 	}
 
-	if pr.Issue.IsClosed {
-		ctx.Error(http.StatusForbidden, "", "not need to dismiss this review because this pr is closed")
-		return
-	}
-
 	_, err := pull_service.DismissReview(ctx, review.ID, ctx.Repo.Repository.ID, msg, ctx.Doer, isDismiss, dismissPriors)
 	if err != nil {
+		if pull_service.IsErrDismissRequestOnClosedPR(err) {
+			ctx.Error(http.StatusForbidden, "", err)
+			return
+		}
 		ctx.Error(http.StatusInternalServerError, "pull_service.DismissReview", err)
 		return
 	}
diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go
index 7647e89c56..a4e3f7165a 100644
--- a/routers/web/repo/issue.go
+++ b/routers/web/repo/issue.go
@@ -2494,6 +2494,10 @@ func UpdatePullReviewRequest(ctx *context.Context) {
 
 		_, err = issue_service.ReviewRequest(ctx, issue, ctx.Doer, reviewer, action == "attach")
 		if err != nil {
+			if issues_model.IsErrReviewRequestOnClosedPR(err) {
+				ctx.Status(http.StatusForbidden)
+				return
+			}
 			ctx.ServerError("ReviewRequest", err)
 			return
 		}
diff --git a/routers/web/repo/pull_review.go b/routers/web/repo/pull_review.go
index ead7592fd0..afa3b17d31 100644
--- a/routers/web/repo/pull_review.go
+++ b/routers/web/repo/pull_review.go
@@ -263,6 +263,10 @@ func DismissReview(ctx *context.Context) {
 	form := web.GetForm(ctx).(*forms.DismissReviewForm)
 	comm, err := pull_service.DismissReview(ctx, form.ReviewID, ctx.Repo.Repository.ID, form.Message, ctx.Doer, true, true)
 	if err != nil {
+		if pull_service.IsErrDismissRequestOnClosedPR(err) {
+			ctx.Status(http.StatusForbidden)
+			return
+		}
 		ctx.ServerError("pull_service.DismissReview", err)
 		return
 	}
-- 
cgit v1.2.3