Skip to content

GraphQL updateNote mutation doesn't prevent data loss (no optimistic locking)

Summary

This whole issue is about being able to safely update a comment (note) text (body).

The updateNote mutation (no documentation) takes only the new note body as an input.

e.g.

mutation UpdateNoteBody($noteId: NoteID!, $body: String) {
  updateNote(input: { id: $noteId, body: $body }) {
    errors
  }
}

We have no way of detecting/preventing that another mutation changed the body before our got served.

Steps to reproduce

  1. Read the note as user A, user A thinks that body is 1234
  2. Read the note as user B, user B thinks that body is 1234
  3. User A submits a updateNote mutation and changes the body to abcde
  4. User B submits an updateNote changes the body to wxyz **without having a chance of knowing they are overriding data they've never seen.

Example Project

What is the current bug behavior?

The API doesn't provide any mechanism to prevent this behaviour.

What is the expected correct behavior?

Possible fixes

There are two possibilities:

  1. The best solution is to introduce another optional input to the mutation (e.g. originalBody) the user B from the "Steps to reproduce" section would submit the mutation with { body: 'wxyz', originalBody: '1234 }and they would get back an error saying "You think you are changing1234but the comment body changed toabcde`".
  2. Less safe workaround would be to expose a single note through a GraphQL query (#22791) but there doesn't seem to be an appetite to do that.

Current workaround

Use the REST endpoint to get a single note just before you update the note and validate that the note hasn't changed in the meantime.

Implemented for example in gitlab-vscode-extension!197 (merged).

Edited by Tomas Vik