Filter is mandatory for update und delete mutation #73
Conversation
| ne: [Decimal] | ||
| } | ||
|
|
||
| """ |
There was a problem hiding this comment.
@schwma I am not sure why npm run test:generate-schemas changed this. May be I should revert it manually ?
There was a problem hiding this comment.
Yes, could you please revert bookshop-graphql.gql entirely? The schema has been adjusted to use the upcoming minorUnit from the not yet released cds@6.8.0. (see #70)
There was a problem hiding this comment.
The schema generation test currently skips bookshop-graphql.gql for cds<6.8.0
| ne: [Decimal] | ||
| } | ||
|
|
||
| """ |
There was a problem hiding this comment.
The schema generation test currently skips bookshop-graphql.gql for cds<6.8.0
Co-authored-by: Marcel Schwarz <marcel.schwarz@sap.com>
| AdminService { | ||
| Books { | ||
| update(input: $input) { | ||
| update(filter: {}, input: $input) { |
There was a problem hiding this comment.
But isn't it required to include the keys in the filter?
There was a problem hiding this comment.
No, filters are basically where conditions with limited operators. Keys don't necessarily need to be specified.
There was a problem hiding this comment.
According to the Changelog :
Empty filter lists resolve to false and empty filter objects resolve to true
There was a problem hiding this comment.
So in this case it would update all entries in the database, do we really want to allow that? Or are there checks in our runtime?
There was a problem hiding this comment.
Yes it would update all entries and there are no checks in the runtime. We discussed this with @ralfhandl and determined that there will always be ways to cause a filter to affect most/all entries, since it is essentially a where condition. Making the filter argument mandatory is a step against accidentally affecting all entries by forgetting the argument.
…ap-js/graphql into mandatory-filter-update-delete
No description provided.