Refactor notes quick action handling
What does this MR do and why?
Previously whenever a quick action was processed in a note,
NotesController#create
and NotesController#update
stored the
message and commands inside ActiveModel error attributes,
commands_only
and command_names
. The returned JSON looked
something like:
{
"commands_changes": {},
"valid": false,
"errors": {
"commands_only": [
"Commands applied"
],
"command_names": ["/close"]
}
}
This is quite confusing and a bit of hack because the errors
section
suggests there was an actual error, but in the example above the quick
action succeeded.
The frontend distinguishes between a failed and successful attempt by
looking at the HTTP code (422 vs. 200). To determine which code to
set, previously the controller looked at all the attributes stored on
the Note
ActiveModel record:
-
If only
commands_only
andcommand_names
were present, consider the operation a success and return 200. -
If there are other error attributes (e.g.
validation
), then the operation is deemed a failure and return 422.
In the failure case, the controller previously only sent whatever
message was stored in the commands_only
attribute. However, if a
quick action fails, we may want to display the details as to why the
failure occurred, which might be present in another ActiveModel error attribute. To do that, we should stop using the ActiveModel
errors as a placeholder for information and only use it when actual
failures occur.
This message refactors the quick action handling between the frontend and backend. Now a JSON return payload looks more like:
{
"commands_changes": {},
"valid": false,
"quick_actions_status": {
"messages": [
"Commands applied."
],
"command_names": ["/close"],
"commands_only": true,
"error": false
}
}
A similar change was made to the GraphQL endpoint:
-
Go to your GDK:
http://gdk.test:3000/-/graphql-explorer
-
Get a merge request ID (e.g. 1). Submit the following query:
mutation{ createNote(input: { noteableId: "gid://gitlab/MergeRequest/1", body: "/approve" }) { errors note { body } } }
-
In
master
, you would see this error:{ "data": { "createNote": { "errors": [ "Commands only Approved the current merge request." ], "note": null } } }
On this branch, you can include
quickActionsStatus
:mutation{ createNote(input: { noteableId: "gid://gitlab/MergeRequest/4", body: "/approve" }) { errors note { body } quickActionsStatus { messages commandsOnly errorMessages } } }
Then you get:
{ "data": { "createNote": { "errors": [], "note": null, "quickActionsStatus": { "messages": [ "Approved the current merge request." ], "commandsOnly": true, "errorMessages": null } } }, "correlationId": "01JCH8EMVAF1DRN79B29V5Y3W2" }
References
Relates to #346557 (closed)
MR acceptance checklist
Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.
Screenshots or screen recordings
Screenshots are required for UI changes, and strongly recommended for all other merge requests.
Before | After |
---|---|
How to set up and validate locally
See above.