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
- Backend: when the approval is refused, the 200 JSON body now also includes
reviewer_state_errorwith the service message (for example "Merge request is already merged"). The field is additive andreview_idis unchanged.:already_approvedis 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.
- Frontend: the review drawer shows a warning alert: "Your review was submitted, but the merge request was not approved: %{reason}". The
batch_commentsstore actionspublishReviewandpublishReviewInBatchesnow resolve with the response body so the drawer can read it. - Frontend: the Approve option in the drawer is disabled when the merge request is merged. It uses the notes store
openStategetter, the same checkcomment_form.vueuses.
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
- #632686 (closed)
- Related commit 7fbdff14, which made
ApprovalServicereturn aServiceResponsewith named reasons.
Screenshots or screen recordings
How to set up and validate locally
- Open a merged merge request where you are a reviewer and are allowed to approve.
- Open Your review / Submit review. Approve is disabled.
- 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)

