Skip to content
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

Add support for custom scalars #99

Merged
merged 2 commits into from
Aug 6, 2020

Conversation

Andy2003
Copy link
Collaborator

@Andy2003 Andy2003 commented Aug 3, 2020

This PR add support for custom scalars

For the given schema:

scalar Date
type Movie {
  _id: ID!
  title: String!
  released: Date
}

you can now run queries like:

{
  movie(filter:{released_gte: 1994}) {
    title
    released
  }
}

resolves #9

@Andy2003 Andy2003 requested a review from jexp August 3, 2020 13:58
@Andy2003 Andy2003 force-pushed the feature/GH9-custom-scalars branch from d76a3f4 to e801a8e Compare August 3, 2020 14:56
----
MATCH (updateMovie: Movie)
WHERE ID(updateMovie) = toInteger($updateMovie_id)
SET updateMovie = { released: $updateMovieReleased }
Copy link
Contributor

Choose a reason for hiding this comment

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

this should probably be +=
as it should not override other existing properties.

Copy link
Contributor

Choose a reason for hiding this comment

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

Or useREMOVE updateMovie.released for null fields
or just use SET updateMovie.released = $updateMovieReleased

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This has nothing to do with this PR, since its common setter logic, that update is SET x = and merge is SET x += If we want to change this behavior, we should create an extra ticket for.

Copy link
Contributor

Choose a reason for hiding this comment

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

then that logic is odd, as SET x = {} should only be there for create.

For update (as you might leave off properties) it should always be +=, never =

if that's not the case it's a bug and we need to fix it please create the GH issue.

Copy link
Collaborator Author

@Andy2003 Andy2003 Aug 5, 2020

Choose a reason for hiding this comment

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

So what is than the difference between the merge and update-fields? The SET x = {} was also the problem of #95

Copy link
Contributor

Choose a reason for hiding this comment

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

Usually you merge on a single field (in exceptional cases on a set of fields). (they keys that are used to look-up nodes for uniqueness)
And update fields are the fields that are added/overriden.

As MERGE is get-or-create you don't want to destroy existing additional data that might have been added in between by other means.

And if you leave off fields it doesn't mean you want to remove them, just that you don't have data (or updates) for them at the moment.

Copy link
Contributor

@jexp jexp left a comment

Choose a reason for hiding this comment

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

Looks good, please have a look at my one comment.

Thanks a lot Andreas!!

@jexp jexp merged commit 9602a55 into neo4j-graphql:master Aug 6, 2020
@Andy2003 Andy2003 deleted the feature/GH9-custom-scalars branch August 27, 2020 09:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

support custom scalars
2 participants