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
- Read the note as user A, user A thinks that
body
is1234
- Read the note as user B, user B thinks that
body
is1234
- User A submits a
updateNote
mutation and changes the body toabcde
- User B submits an
updateNote
changes the body towxyz
**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:
- 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 changing
1234but the comment body changed to
abcde`". - 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