From f484e3d44ef28dc01ec706d096cb11ad497873ad Mon Sep 17 00:00:00 2001 From: Kemal Zebari Date: Wed, 24 Apr 2024 11:36:19 -0700 Subject: [PATCH] Handle error object in API and use HTTP status 422 --- routers/api/v1/repo/pull_review.go | 12 ++++++++++-- routers/web/repo/pull_review.go | 4 ++-- services/pull/review.go | 4 ++-- 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/routers/api/v1/repo/pull_review.go b/routers/api/v1/repo/pull_review.go index b527e90f10..18683e4a41 100644 --- a/routers/api/v1/repo/pull_review.go +++ b/routers/api/v1/repo/pull_review.go @@ -372,7 +372,11 @@ func CreatePullReview(ctx *context.APIContext) { // create review and associate all pending review comments review, _, err := pull_service.SubmitReview(ctx, ctx.Doer, ctx.Repo.GitRepo, pr.Issue, reviewType, opts.Body, opts.CommitID, nil) if err != nil { - ctx.Error(http.StatusInternalServerError, "SubmitReview", err) + if pull_service.IsErrSubmitReviewOnClosedPR(err) { + ctx.Error(http.StatusUnprocessableEntity, "", err) + } else { + ctx.Error(http.StatusInternalServerError, "SubmitReview", err) + } return } @@ -460,7 +464,11 @@ func SubmitPullReview(ctx *context.APIContext) { // create review and associate all pending review comments review, _, err = pull_service.SubmitReview(ctx, ctx.Doer, ctx.Repo.GitRepo, pr.Issue, reviewType, opts.Body, headCommitID, nil) if err != nil { - ctx.Error(http.StatusInternalServerError, "SubmitReview", err) + if pull_service.IsErrSubmitReviewOnClosedPR(err) { + ctx.Error(http.StatusUnprocessableEntity, "", err) + } else { + ctx.Error(http.StatusInternalServerError, "SubmitReview", err) + } return } diff --git a/routers/web/repo/pull_review.go b/routers/web/repo/pull_review.go index 080df93825..2456271547 100644 --- a/routers/web/repo/pull_review.go +++ b/routers/web/repo/pull_review.go @@ -264,8 +264,8 @@ func SubmitReview(ctx *context.Context) { if issues_model.IsContentEmptyErr(err) { 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.IsSubmitReviewOnClosedPR(err) { - ctx.Error(http.StatusForbidden) + } else if pull_service.IsErrSubmitReviewOnClosedPR(err) { + ctx.Error(http.StatusUnprocessableEntity) } else { ctx.ServerError("SubmitReview", err) } diff --git a/services/pull/review.go b/services/pull/review.go index 9d42976a36..52b46e55f6 100644 --- a/services/pull/review.go +++ b/services/pull/review.go @@ -46,8 +46,8 @@ func (err ErrDismissRequestOnClosedPR) Unwrap() error { // ErrSubmitReviewOnClosedPR represents an error when an user tries to submit an approve or reject review associated to a closed or merged PR. type ErrSubmitReviewOnClosedPR struct{} -// IsSubmitReviewOnClosedPR checks if an error is an ErrSubmitReviewOnClosedPR. -func IsSubmitReviewOnClosedPR(err error) bool { +// IsErrSubmitReviewOnClosedPR checks if an error is an ErrSubmitReviewOnClosedPR. +func IsErrSubmitReviewOnClosedPR(err error) bool { _, ok := err.(ErrSubmitReviewOnClosedPR) return ok }