Skip to content

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 and command_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:

  1. Go to your GDK: http://gdk.test:3000/-/graphql-explorer

  2. 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
        }
      }
    }
  3. 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.

Edited by Stan Hu

Merge request reports

Loading