Return 304 for repeat merge request approval actions
What does this MR do and why?
Return 304 for repeat merge request approval actions
Re-approving an already-approved MR returned a misleading 401 Unauthorized, and unapproving one that was never approved returned a misleading 404 Not Found. Both implied an authentication or existence problem when the request was actually valid but a no-op. Guard both endpoints the same way subscribe/unsubscribe and vulnerability resolve/dismiss already do, returning 304 Not Modified for repeat actions on a resource already in the target state.
Resolves #604754 (closed)
Changelog: fixed
References
Existing precedent for 304 Not Modified on repeat/no-op actions in this API:
ee/lib/api/vulnerabilities.rb#L73-95—resolve/dismiss/confirm/reverteach guard withnot_modified! if @vulnerability.<state>?before calling the servicelib/api/subscriptions.rb#L77-107—subscribe/unsubscribe, the direct symmetric precedent for ourapprove/unapprovepairlib/api/projects.rb#L763-793—star/unstar, same idiom as anif/elsebranch instead of a guard clause
Screenshots or screen recordings
| Before | After |
|---|---|
How to set up and validate locally
-
Pick a merge request you can approve and approve it
curl -i --request POST --header "PRIVATE-TOKEN: <your_token>" \ "http://gdk.test:3000/api/v4/projects/<project_id>/merge_requests/<mr_iid>/approve" # Returns 201 Created. -
Send the same request again.
- Before this change this returned
401 Unauthorized; it now returns304 Not Modified.
- Before this change this returned
-
Unapprove it, then send the same unapprove request a second time:
curl -i --request POST --header "PRIVATE-TOKEN: <your_token>" \ "http://gdk.test:3000/api/v4/projects/<project_id>/merge_requests/<mr_iid>/unapprove" # Returns 201 Created.- Before this change this returned
404 Not Found; it now returns304 Not Modified.
- Before this change this returned
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.