Skip to content
This repository has been archived by the owner on Sep 3, 2021. It is now read-only.

Schema Transforms are not applied when using graphql-constraint-directive #562

Open
gpaluk opened this issue Dec 31, 2020 · 9 comments
Open

Comments

@gpaluk
Copy link

gpaluk commented Dec 31, 2020

I am getting back no errors when I apply constraints to my new Neo4J Apollo project. I have created a minimal project to illustrate the point at:

https://github.com/gpaluk/apollo-graphql-mfp

When using the following GraphQL query in the playground, the entity gets created which is undesired:

mutation {
  CreateUser(userRole:"admin", username:"a", email:"a")
  {
    userRole
    username
    email
  }
}

I filed an issue at: confuser/graphql-constraint-directive#52 and they redirected me here.

@michaeldgraham
Copy link
Collaborator

Hey @gpaluk, thanks for bringing this here, and for sharing your code! It looks like you're using the most recent version of neo4j-graphql-js, 2.18.0. We updated graphql and graphql-tools to the most recent versions, so something was perhaps missed in assessing the differences. I'll try to use the graphql-constraint-directive and get back to you! We probably need to add testing for schema transforms~

@gpaluk
Copy link
Author

gpaluk commented Jan 3, 2021

@michaeldgraham I wonder if there might be an ETA on this or if I can assist with anything? I'm reviewing the viability of this stack but have a close deadline. It seems like this could possibly a minor issue and don't want to have to rule it out. Thank you.

@michaeldgraham
Copy link
Collaborator

michaeldgraham commented Jan 4, 2021

Hey @gpaluk, so it looks like the upgrade of graphql-tools needed to be fixed, as it was an upgrade to a version that included support for the schemaTransforms argument. The makeExecutableSchema used internally wasn't exposing the schemaTransforms argument, despite it being available in makeAugmentedSchema from neo4j-graphql-js.

So, we've updated the library today to v2.19.0, which internally uses the most recent versions of graphql and graphql-tools - please let me know if this works for you after upgrading :)

Edit: It looks like there's still a problem :( I'll be working on trying to figure out what's going on with the dependencies, we should have it fixed up shortly

Edit 2: Alrighty! It should work with the current version of neo4j-graphql-js v2.19.1, in which graphql-tools is now updated :)

@gpaluk
Copy link
Author

gpaluk commented Jan 5, 2021

@michaeldgraham Stellar start, it seems close for some fields however when I add the @unique generated annotation to the username, that is still ignored also the email constraint doesn't seem to being applied to my example field so it feels like there is some flakiness still. So many thanks for taking a look at this though, like I say, it seems close.

[Edit]
I pushed a small update to the original example.

@michaeldgraham
Copy link
Collaborator

Heyy @gpaluk, so to use the @unique directive to set a property uniqueness constraint enforced in Neo4j, we use the APOC procedure apoc.schema.assert(), which would be called in a transaction sent to Neo4j by the assertSchema export of neo4j-graphql-js. It looks like you have assertSchema commented out on this line, so if you uncomment it and import assertSchema from neo4j-graphql-js, then the uniqueness constraint should be set in Neo4j.

It looks like the @constraint directive can be used as it's intended, on object type fields for validating queries and on input type fields for validating argument values. But since the neo4j-graphql-js schema augmentation process does not know about the @constraint directive, it isn't carried over onto mutation argument input schema from its use on object type fields. This seems like it could be a nice integration! I'll take a look into applying @constraint directives when generating arguments and input objects used in the Mutation API~

@gpaluk
Copy link
Author

gpaluk commented Jan 9, 2021

That's correct, I'd commented that line out due to the previous version crashing. I can confirm that both the length and unique annotations being applied to username are now working however validation doesn't seem to be picked up at all by the email field.

using the following GraphQL:

mutation {
  CreateUser(
    userRole:"admin",
    username:"admin3",
    email:"bob")
  {
    username
  }
}

email validates without issue, even though the following annotation is applied:

type User {
  userId: ID! @id

  userRole: String!
  username: String! @constraint(minLength: 6, maxLength: 30) @unique
  email: String! @constraint(minLength: 5, format: "email")
}

As can be seen above, bob is neither longer than the minimum string length nor a valid email pattern. Many thanks again for your attention on this.

@gpaluk
Copy link
Author

gpaluk commented Jan 14, 2021

@michaeldgraham - After re-reading your post, I think that you might be alluding to the functionality that I am referring. Am I correct in assuming that there needs to be a check on the data validity earlier in the entity processing system?

@J-Krush
Copy link

J-Krush commented Apr 7, 2021

Bumping this thread. Would love to be able to use the @constraint directive as you said @michaeldgraham: carrying over the "mutation argument input schema from its use on object type fields."

@michaeldgraham
Copy link
Collaborator

#608

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants