diff options
author | Kemal Zebari <60799661+kemzeb@users.noreply.github.com> | 2024-03-28 16:19:24 +0100 |
---|---|---|
committer | Earl Warren <contact@earl-warren.org> | 2024-03-30 07:17:32 +0100 |
commit | 23676bfea7ccbbe166a554115ea1f5f02800e379 (patch) | |
tree | f9fd10ac0a8fd6e95ff90e4f595b91684fb2e8c3 /routers | |
parent | Render code tags in commit messages (#30146) (diff) | |
download | forgejo-23676bfea7ccbbe166a554115ea1f5f02800e379.tar.xz forgejo-23676bfea7ccbbe166a554115ea1f5f02800e379.zip |
Prevent re-review and dismiss review actions on closed and merged PRs (#30065)
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)
Diffstat (limited to 'routers')
-rw-r--r-- | routers/api/v1/repo/pull_review.go | 17 | ||||
-rw-r--r-- | routers/web/repo/issue.go | 4 | ||||
-rw-r--r-- | routers/web/repo/pull_review.go | 4 |
3 files changed, 19 insertions, 6 deletions
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 } |