Skip to content

fix(graphql): allow update mutations to set an @id field to its existing value #9702

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
shiva-istari wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix(graphql): allow update mutations to set an @id field to its existing value #9702

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6a65b2f
fix id directive update issue
May 13, 2026
693d67b
fix tests
May 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 32 additions & 22 deletions graphql/e2e/auth/add_mutation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1225,29 +1225,39 @@ func TestUpdateMutationWithIDFields(t *testing.T) {
}`,
error: "mutation updateEmployer failed because GraphQL debug: only one node is allowed" +
" in the filter while updating fields with @id directive",
}, {
name: "update mutation gives error when given @id field already exist in some node",
query: `mutation update($patch: UpdateEmployerInput!) {
updateEmployer(input: $patch) {
numUids
}
}`,
variables: `{
"patch": {
"filter": {
"name": {
"in": "ABC"
}
},
"set": {
"company": "ABC tech"
}
}
}`,
error: "couldn't rewrite mutation updateEmployer because failed to rewrite mutation" +
" payload because GraphQL debug: id ABC tech already exists for field company" +
" inside type Employer",
},
// TODO(reviewer): This test case is commented out because the scenario it tests is
// semantically incorrect. It sets ABC employer's company to "ABC tech" — its own
// current value. With the deferred conflict check fix, a node updating its own @id
// value is no longer treated as a conflict (same-node check passes), so the mutation
// succeeds instead of returning an error. The original test was an accidental
// false-positive: the old eager check rejected self-updates that should be valid.
// Please advise: should this be replaced with a genuine cross-node conflict test
// (e.g. ABC tries to set company = " XYZ tech"), or removed entirely?
//
// {
// name: "update mutation gives error when given @id field already exist in some node",
// query: `mutation update($patch: UpdateEmployerInput!) {
// updateEmployer(input: $patch) {
// numUids
// }
// }`,
// variables: `{
// "patch": {
// "filter": {
// "name": {
// "in": "ABC"
// }
// },
// "set": {
// "company": "ABC tech"
// }
// }
// }`,
// error: "couldn't rewrite mutation updateEmployer because failed to rewrite mutation" +
// " payload because GraphQL debug: id ABC tech already exists for field company" +
// " inside type Employer",
// },
{
name: "update mutation gives error when multiple nodes are found at nested level" +
"while linking rot object to nested object",
Expand Down
45 changes: 26 additions & 19 deletions graphql/e2e/auth/debug_off/debugoff_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,26 +259,33 @@ func TestUpdateMutationWithIDFields(t *testing.T) {
}
}
}`,
}, {
name: "update mutation gives error when given @id field already exist in some node",
query: `mutation update($patch: UpdateEmployerInput!) {
updateEmployer(input: $patch) {
numUids
}
}`,
variables: `{
"patch": {
"filter": {
"name": {
"in": "ABC"
}
},
"set": {
"company": "ABC tech"
}
}
}`,
},
// TODO(reviewer): Commented out for the same reason as the corresponding case in
// add_mutation_test.go — this sets ABC employer's company to its own current value
// ("ABC tech"), which our fix now correctly allows (same-node, no conflict). The
// mutation succeeds with NumUids=1 instead of 0. Please advise whether to replace
// this with a genuine cross-node conflict test (ABC sets company=" XYZ tech") or remove.
//
// {
// name: "update mutation gives error when given @id field already exist in some node",
// query: `mutation update($patch: UpdateEmployerInput!) {
// updateEmployer(input: $patch) {
// numUids
// }
// }`,
// variables: `{
// "patch": {
// "filter": {
// "name": {
// "in": "ABC"
// }
// },
// "set": {
// "company": "ABC tech"
// }
// }
// }`,
// },
{
name: "update mutation gives error when multiple nodes are found at nested level" +
"while linking rot object to nested object",
Expand Down
14 changes: 14 additions & 0 deletions graphql/e2e/common/common.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -939,6 +939,20 @@ func RunAll(t *testing.T) {
t.Run("Update language tag fields", updateLangTagFields)
t.Run("mutation with @id field and interface arg", addMutationWithIDFieldHavingInterfaceArg)
t.Run("xid update and nullable tests", xidUpdateAndNullableTests)
t.Run("update mutation with unchanged @id field succeeds", updateMutationWithUnchangedXID)
t.Run("update mutation with nested unchanged @id field succeeds", updateMutationWithNestedUnchangedXID)
t.Run("update parent mutation with nested unchanged @id reference succeeds", updateMutationWithNestedUnchangedXIDOnParent)
t.Run("update mutation errors on @id conflict with different node", updateMutationXIDConflictStillErrors)
t.Run("update mutation Int @id unchanged succeeds (B1)", updateMutationIntXIDUnchanged)
t.Run("update mutation Int @id conflict errors (B2)", updateMutationIntXIDConflictErrors)
t.Run("update mutation Int64 @id unchanged succeeds (B3)", updateMutationInt64XIDUnchanged)
t.Run("update mutation nullable @id unchanged succeeds (C1)", updateMutationNullableXIDUnchanged)
t.Run("update mutation nullable @id set for first time succeeds (C2)", updateMutationNullableXIDFirstTimeSet)
t.Run("update mutation nullable @id cleared via remove patch succeeds (C3)", updateMutationNullableXIDClearedViaRemove)
t.Run("update mutation remove patch on non-@id field succeeds (C3-contrast)", updateMutationRemovePatchNonXIDField)
t.Run("update mutation delete and recreate same @id succeeds (F1)", updateMutationDeleteRecreateXID)
t.Run("update mutation concurrent unchanged @id all succeed (H1)", updateMutationConcurrentUnchangedXID)
t.Run("update mutation concurrent @id theft all error (H2)", updateMutationConcurrentXIDTheftAllError)

// error tests
t.Run("graphql completion on", graphQLCompletionOn)
Expand Down
Loading
Loading