Report refused approvals when submitting a review

What does this MR do and why?

Choosing "Approve" in the Submit review drawer on a merged merge request published the comments but silently dropped the approval. The request returned 200 and the UI showed no error. This MR reports the refusal to the user and disables the Approve option when the merge request is merged. The review still publishes, so no comments are lost.

Root cause, scope decisions, and tests

Root cause

Projects::MergeRequests::DraftsController#update_reviewer_state discarded the ServiceResponse returned by MergeRequests::ApprovalService. The publish endpoint (POST /-/merge_requests/:iid/drafts/publish, an internal Rails controller route, not the public REST or GraphQL API) returned 200 with {"review_id": null}. The same path also dropped other approval refusals: user not eligible to approve, incorrect approval password, and SAML re-authentication required (EE).

Why the status stays 200

The comments and the summary note are published before the approval runs. Failing the request would misreport a partial success. The frontend would also keep the already-published drafts in its local state, and a resubmit could duplicate the summary note. So the review still publishes and the status stays 200.

Changes

  1. Backend: when the approval is refused, the 200 JSON body now also includes reviewer_state_error with the service message (for example "Merge request is already merged"). The field is additive and review_id is unchanged.
    • :already_approved is not reported, because a repeat approve is an idempotent no-op.
    • Errors from UpdateReviewerStateService (comment, request changes) are intentionally not reported. "Reviewer not found" is a normal outcome when the merge request author submits a comment-only review.
  2. Frontend: the review drawer shows a warning alert: "Your review was submitted, but the merge request was not approved: %{reason}". The batch_comments store actions publishReview and publishReviewInBatches now resolve with the response body so the drawer can read it.
  3. Frontend: the Approve option in the drawer is disabled when the merge request is merged. It uses the notes store openState getter, the same check comment_form.vue uses.

Scope decision

The approve_merge_request policy is not changed. Unapprove, external status checks, and the approvals widget rely on it staying true after merge.

Tests

  • Controller spec: merged merge request publishes the review, returns 200, sets reviewer_state_error, and creates no approval row. Successful and already-approved submissions have no error key.
  • Jest: Approve is disabled on a merged merge request, the warning alert renders, and the store actions resolve with the response body.

References

Screenshots or screen recordings

Before After
Approve is selectable on a merged merge request. Approve is disabled on a merged merge request. approve-disabled-merged
Submitting closes the drawer with no error, and the approval is dropped. The review publishes and a warning explains why the approval did not apply (merge request merged while the drawer was open). approval-refused-warning

How to set up and validate locally

  1. Open a merged merge request where you are a reviewer and are allowed to approve.
  2. Open Your review / Submit review. Approve is disabled.
  3. To exercise the server path, open the drawer on an open merge request and select Approve. Merge it in another tab, then submit the review. The comments publish and a warning explains the merge request was not approved.

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Resolves #632686 (closed)

Edited by Kai Armstrong

Merge request reports

Loading
Loading