torchbox / wagtail-grapple

A Wagtail app that makes building GraphQL endpoints a breeze!
https://wagtail-grapple.readthedocs.io/en/latest/
Other
152 stars 57 forks source link

GraphQLForeignKey doesn't work with "wagtailcore.Page" for content_type #116

Closed zerolab closed 3 years ago

zerolab commented 3 years ago

trying to add GraphQLForeignKey pointing to wagtailcore.Page results in Could not import 'grapple.schema.schema' for Graphene setting 'SCHEMA'. AttributeError: 'NoneType' object has no attribute '_meta'.

Use case:

As far as I can see, the issue is around https://github.com/GrappleGQL/wagtail-grapple/blob/e84622a8c67a0a684a59c1ba0d3448baf541a305/grapple/models.py#L81-L87 and the fact that the core page is not added to models. instead it is added as a DjangoObjectType

tbrlpld commented 3 years ago

I just ran into the same issue yesterday. What worked for me is using the GraphQLPage type for the field that holds the backward relation of a foreign key relationship.

The model creating the foreign key relationship (in my case a page) does know the class it is pointing to, while the pointed to model (in my case a form) does not know what kind of model is pointing to it, or more specifically it does not know which kind of page. In the schema, the field with the backward relation ship shows up to be of type PageInterface.

Maybe this would work for your use case, but it is probably still a good idea to implement your suggested changes to make the behavior more consistent.

zerolab commented 3 years ago

Yeah, GraphQLPage("field_name", source="method_that_returns_queryset", is_list=True) did the trick.

I am tempted to close this as working as designed 🤔

tbrlpld commented 3 years ago

Awesome! Glad this "workaround" is working for your case too.

If using the GraphQLPage field is the designed approach for this kind of relationship, we should maybe make that explicit in the docs too . Currently there is no mention of the GraphQLPage field model (not sure how I even found it in the first place 😅). I can also imagine that this is a common use case for foreign keys. It might make sense to explicitly reference that in the GraphQLForeignKey documentation also.

zerolab commented 3 years ago

@tbrlpld opened #119 with additions to docs. Would you be able to have a look/suggest improvements? Thanks

tbrlpld commented 3 years ago

@zerolab Of course, I will have a look!