Skip to content

Filter is mandatory for update und delete mutation #73

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

Merged
mariayord merged 5 commits into from
Apr 28, 2023
Merged

Filter is mandatory for update und delete mutation #73

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
cdc21a5
Filter is mandatory for update und delete mutation
mariayord Apr 28, 2023
d8e1b77
revert
mariayord Apr 28, 2023
be3ab41
Update CHANGELOG.md
mariayord Apr 28, 2023
a9e8b9b
fix
mariayord Apr 28, 2023
945850a
Merge branch 'mandatory-filter-update-delete' of https://github.com/c…
mariayord Apr 28, 2023
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
+ Wrap only objects (i.e. not primitive types or arrays) returned by custom handlers in arrays in create, read, and update resolvers
+ Delete mutations return the length of an array that is returned by a `DELETE` custom handler or 1 if a single object is returned
- Don't generate fields for key elements in update input objects
- Update and delete mutations have mandatory `filter` argument

### Fixed

Expand Down
4 changes: 2 additions & 2 deletions lib/schema/mutation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ module.exports = cache => {
if (!filterInputObjectType || !updateInputObjectType) return

const args = {
[ARGS.filter]: { type: filterInputObjectType },
[ARGS.filter]: { type: new GraphQLNonNull(filterInputObjectType) },
[ARGS.input]: { type: new GraphQLNonNull(updateInputObjectType) }
}
return { type: new GraphQLList(entityObjectType), args }
Expand All @@ -89,7 +89,7 @@ module.exports = cache => {
if (!filterInputObjectType) return

const args = {
[ARGS.filter]: { type: filterInputObjectType }
[ARGS.filter]: { type: new GraphQLNonNull(filterInputObjectType) }
}
return { type: GraphQLInt, args }
}
Expand Down
8 changes: 4 additions & 4 deletions test/schemas/edge-cases/field-named-localized.gql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,10 @@ type FieldNamedLocalizedService_Root_input {
input: [FieldNamedLocalizedService_Root_C]!
): [FieldNamedLocalizedService_Root]
delete(
filter: [FieldNamedLocalizedService_Root_filter]
filter: [FieldNamedLocalizedService_Root_filter]!
): Int
update(
filter: [FieldNamedLocalizedService_Root_filter]
filter: [FieldNamedLocalizedService_Root_filter]!
input: FieldNamedLocalizedService_Root_U!
): [FieldNamedLocalizedService_Root]
}
Expand Down Expand Up @@ -99,10 +99,10 @@ type FieldNamedLocalizedService_localized_input {
input: [FieldNamedLocalizedService_localized_C]!
): [FieldNamedLocalizedService_localized]
delete(
filter: [FieldNamedLocalizedService_localized_filter]
filter: [FieldNamedLocalizedService_localized_filter]!
): Int
update(
filter: [FieldNamedLocalizedService_localized_filter]
filter: [FieldNamedLocalizedService_localized_filter]!
input: FieldNamedLocalizedService_localized_U!
): [FieldNamedLocalizedService_localized]
}
Expand Down
8 changes: 4 additions & 4 deletions test/schemas/model-structure/composition-of-aspect.gql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ type CompositionOfAspectService_Books_chapters_input {
input: [CompositionOfAspectService_Books_chapters_C]!
): [CompositionOfAspectService_Books_chapters]
delete(
filter: [CompositionOfAspectService_Books_chapters_filter]
filter: [CompositionOfAspectService_Books_chapters_filter]!
): Int
}

Expand All @@ -78,10 +78,10 @@ type CompositionOfAspectService_Books_input {
input: [CompositionOfAspectService_Books_C]!
): [CompositionOfAspectService_Books]
delete(
filter: [CompositionOfAspectService_Books_filter]
filter: [CompositionOfAspectService_Books_filter]!
): Int
update(
filter: [CompositionOfAspectService_Books_filter]
filter: [CompositionOfAspectService_Books_filter]!
input: CompositionOfAspectService_Books_U!
): [CompositionOfAspectService_Books]
}
Expand Down Expand Up @@ -112,7 +112,7 @@ type CompositionOfAspectService_Books_reviews_input {
input: [CompositionOfAspectService_Books_reviews_C]!
): [CompositionOfAspectService_Books_reviews]
delete(
filter: [CompositionOfAspectService_Books_reviews_filter]
filter: [CompositionOfAspectService_Books_reviews_filter]!
): Int
}

Expand Down
4 changes: 2 additions & 2 deletions test/schemas/types.gql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,10 +269,10 @@ type TypesService_MyEntity_input {
input: [TypesService_MyEntity_C]!
): [TypesService_MyEntity]
delete(
filter: [TypesService_MyEntity_filter]
filter: [TypesService_MyEntity_filter]!
): Int
update(
filter: [TypesService_MyEntity_filter]
filter: [TypesService_MyEntity_filter]!
input: TypesService_MyEntity_U!
): [TypesService_MyEntity]
}
Expand Down
2 changes: 1 addition & 1 deletion test/tests/mutations/delete.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ describe('graphql - delete mutations', () => {
mutation {
AdminService {
Books {
delete
delete(filter: {})
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion test/tests/mutations/update.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ describe('graphql - update mutations', () => {
mutation ($input: AdminService_Books_U!) {
AdminService {
Books {
update(input: $input) {
update(filter: {}, input: $input) {
Copy link
Contributor

@David-Kunz David-Kunz Apr 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But isn't it required to include the keys in the filter?

Copy link
Member

@schwma schwma Apr 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, filters are basically where conditions with limited operators. Keys don't necessarily need to be specified.

Copy link
Contributor Author

@mariayord mariayord Apr 28, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the Changelog :
Empty filter lists resolve to false and empty filter objects resolve to true

Copy link
Contributor

@David-Kunz David-Kunz Apr 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member

@schwma schwma Apr 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

title
stock
}
Expand Down