-
Notifications
You must be signed in to change notification settings - Fork 150
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.
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
Relationship is settable via update mutation, but settable directive is set to false. #5012
Comments
Many thanks for raising this bug report @AccsoSG. 🐛 We will now attempt to reproduce the bug based on the steps you have provided. Please ensure that you've provided the necessary information for a minimal reproduction, including but not limited to:
If you have a support agreement with Neo4j, please link this GitHub issue to a new or existing Zendesk ticket. Thanks again! 🙏 |
Hi @AccsoSG! I can see how this could be misleading due to the multiple inputs for performing the same action. As you say, adding This prevents users of the api from updating the values of the |
Hi @Liam-Doodson But if I remove Connect/Disconnect, then I can no longer set this relationship in a Create-Operation, can I? type Identifier {
value: String!
identifierType: IdentifierType!
@relationship(
type: "HAS_IDENTIFIER_TYPE",
direction: OUT,
nestedOperations: [], #<----------------------------
aggregate: false)
@selectable(onRead: true, onAggregate: false)
@settable(onCreate: true, onUpdate: false)
@filterable(byValue: true, byAggregate: false)
}
type IdentifierType {
name: String!
} |
Are you able to just remove the |
Yes, that would work, but there is still the corresponding CONNECT in the generated API. This CONNECT will not work, but it will still be visible. My expectation of the settable directive in combination with the relationship directive is different... |
So perhaps it is more of a feature request. The settable directive should really also consider this case. |
As in you want to have the connect option on a create but not on an update mutation? If this is the case I think it would be more appropriate to add this as an option to the nestedOperation argument potentially with an interface like this: |
I generally mean that the settable directive implies that a field cannot be set in the update case if onUpdate=false. But the fact is that this field can be set if it is a relationship. That is not very intuitive I would not modify the nestedOperations of the relationship directive as you have suggested. I think that is too complicated. I would simply adapt the implementation of the settable directive so that the connect/disconnect options also disappears from the API if the settable directive is set to false. |
@Liam-Doodson I would be grateful if we could turn this ticket into a feature request instead of closing it. |
Describe the bug
Setting a field with relationship is possible with an update mutation, although settable onUpdate is set to false.
Type definitions
To Reproduce
Steps to reproduce the behavior:
This mutation is valid, but does not work because of the cardinality (Identifier.identifierType required exactly once):
This mutation, however, works.
Expected behavior
If the settable directive for onUpdate is set to false, the relationship should not be able to be changed. The generated schema should not offer this option at all.
System (please complete the following information):
Side question
If you set onUpdate to true via the settable directive, there is also the option of setting the relationship via the path
to do exactly the same thing. Are these simply two different ways of doing the same thing, or is there a difference here?
The text was updated successfully, but these errors were encountered: