From 2cfe58c69909e69064a668a59733e4b974fc7f92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Ga=C4=87e=C5=A1a?= Date: Fri, 12 May 2023 13:53:29 +0200 Subject: [PATCH] block merge if there are unresolved comments --- internal/api/controller/pullreq/merge.go | 17 +++++++++++++++++ internal/api/handler/pullreq/review_submit.go | 4 ++-- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/internal/api/controller/pullreq/merge.go b/internal/api/controller/pullreq/merge.go index 73fb14190..ac05b0632 100644 --- a/internal/api/controller/pullreq/merge.go +++ b/internal/api/controller/pullreq/merge.go @@ -83,10 +83,27 @@ func (c *Controller) Merge( return types.MergeResponse{}, usererror.BadRequest("Pull request must be open") } + if pr.UnresolvedCount > 0 { + return types.MergeResponse{}, usererror.BadRequest("Pull requests with unresolved comments can't be merged. Resolve all the comments first.") + } + if pr.IsDraft { return types.MergeResponse{}, usererror.BadRequest("Draft pull requests can't be merged. Clear the draft flag first.") } + reviewers, err := c.reviewerStore.List(ctx, pr.ID) + if err != nil { + return types.MergeResponse{}, fmt.Errorf("failed to load list of reviwers: %w", err) + } + + // TODO: We need to extend this section. A review decision might be for an older commit. + // TODO: Repository admin users should be able to override this and proceed with the merge. + for _, reviewer := range reviewers { + if reviewer.ReviewDecision == enum.PullReqReviewDecisionChangeReq { + return types.MergeResponse{}, usererror.BadRequest("At least one reviewer still requests changes.") + } + } + sourceRepo := targetRepo if pr.SourceRepoID != pr.TargetRepoID { sourceRepo, err = c.repoStore.Find(ctx, pr.SourceRepoID) diff --git a/internal/api/handler/pullreq/review_submit.go b/internal/api/handler/pullreq/review_submit.go index 76edba689..35767fc31 100644 --- a/internal/api/handler/pullreq/review_submit.go +++ b/internal/api/handler/pullreq/review_submit.go @@ -38,12 +38,12 @@ func HandleReviewSubmit(pullreqCtrl *pullreq.Controller) http.HandlerFunc { return } - review, err := pullreqCtrl.ReviewSubmit(ctx, session, repoRef, pullreqNumber, in) + _, err = pullreqCtrl.ReviewSubmit(ctx, session, repoRef, pullreqNumber, in) if err != nil { render.TranslatedUserError(w, err) return } - render.JSON(w, http.StatusOK, review) + w.WriteHeader(http.StatusNoContent) } }