GraphQL `createDiffNote` forces clients to compute redundant information
Summary
The API requires the client to send (and often compute) redundant information that is available in the API.
I believe this is the case for REST API as well.
Example
The most obvious example is when creating a note on an unchanged line, the client needs to compute the position of the line on both sides of the diff. On the following image, you can see that the unchanged line has index 134 on the old diff and 149 on the new diff.
The createDiffNote
requires both of those lines even though one should be sufficient. The API anyway calculates the other line because it fails validation if they don't match.
DiffPositionInput
argument for the createDiffNote
mutation
Property | Type | Description |
---|---|---|
headSha | String! | SHA of the HEAD at the time the comment was made. |
baseSha | String | Merge base of the branch the comment was made on. |
startSha | String! | SHA of the branch being compared against. |
paths | DiffPathsInput! | The paths of the file that was changed. Both of the properties of this input are optional, but at least one of them is required |
oldLine | Int | Line on start SHA that was changed. |
newLine | Int | Line on HEAD SHA that was changed. |
DiffPathsInput
Property | Type | Description |
---|---|---|
oldPath | String | The path of the file on the start sha. |
newPath | String | The path of the file on the head sha. |
Steps to reproduce
I use https://gitlab.com/-/graphql-explorer to run the queries.
Mutation requires both paths
When the user creates a note on the new part of the diff, only the new path should be required (and vice versa).
mutation CreateDiffNote6 {
createDiffNote(
input: {
noteableId: "gid://gitlab/MergeRequest/87196674"
body: "new note"
position: {
baseSha: "5e6dffa282c5129aa67cd227a0429be21bfdaf80"
headSha: "a2616e0fea06c8b5188e4064a76d623afbf97b0c"
startSha: "5e6dffa282c5129aa67cd227a0429be21bfdaf80"
paths: { newPath: "test.ts" }
newLine: 15
}
}
) {
errors
note {
id
}
}
}
(notice only the newPath
being present)
This results in 500 Internal Server Error
. It only works when we change the paths
to paths: { newPath: "test.ts", oldPath: "test.js" }
(specifying both paths).
Forces user to compute both line indexes for unchanged lines:
mutation CreateDiffNoteNonChanged {
createDiffNote(
input: {
noteableId: "gid://gitlab/MergeRequest/87196674"
body: "new note"
position: {
baseSha: "5e6dffa282c5129aa67cd227a0429be21bfdaf80"
headSha: "a2616e0fea06c8b5188e4064a76d623afbf97b0c"
startSha: "5e6dffa282c5129aa67cd227a0429be21bfdaf80"
paths: { oldPath: "test.js", newPath: "test.ts" }
oldLine: 17
}
}
) {
errors
}
}
fails with message
{
"data": {
"createDiffNote": {
"errors": [
"Line code can't be blank",
"Line code must be a valid line code"
]
}
}
}
Only adding the newLine: 18
fixes the issue. The client needs to keep track of the fact that the new line index has been shifted by 1
Example Project
I use viktomas/test-project!5 (diffs)
Output of checks
This bug happens on GitLab.com