Only retrieve the pages that are registered in the settings #226
Conversation
|
Hey @jffr, thank you for this. |
|
@zerolab Thank you for the feedback! I have fixed the failing tests and introduced two new tests for the app setting:
I think we're missing one last test, which is a query where we get more pages from different apps. But in order to write that test we have to include a new demo app within the example project. Do you think it is worth it or are the test cases explained above good enough? |
| # Retrieve only the pages that are registered in the settings | ||
| content_types = ContentType.objects.filter( | ||
| app_label__in=grapple_settings.APPS | ||
| ) | ||
| pages = pages.filter(content_type__in=content_types) |
There was a problem hiding this comment.
this could be simplified as:
pages = pages.filter(content_type__app_label__in=grapple_settings.APPS)
There was a problem hiding this comment.
additionally, we still support the old dict-style declaration. https://cs.github.com/GrappleGQL/wagtail-grapple/blob/9b8a49c05ddc74673e980e6a359c9f642aae8877/grapple/actions.py?q=grapple_settings.APPS#L55-L63
Perhaps use registry.apps - https://cs.github.com/GrappleGQL/wagtail-grapple/blob/9b8a49c05ddc74673e980e6a359c9f642aae8877/grapple/registry.py#L62 for this
|
|
||
| pages_data = executed["data"]["pages"] | ||
| self.assertEquals(pages_data[0]["contentType"], "home.HomePage") | ||
| self.assertEquals(pages_data[1]["contentType"], "home.BlogPage") |
There was a problem hiding this comment.
I suggest adding news_post = NewsPageFactory(parent=self.home) before the execute and add a check to ensure the news page doesn't appear in the results
| @@ -0,0 +1,22 @@ | |||
| # Generated by Django 3.2.13 on 2022-04-24 07:18 | |||
There was a problem hiding this comment.
I can't see the changes that triggered this migration 🤔
|
@jffr do you still have any inclination to work on this PR? |
A small fix for removing pages that are not registered in the GRAPPLE settings.
Having a project with two apps (for example home and blog). You'll get an error when we only expose the 'blog' app. Which returns the following output:
{ "errors": [ { "message": "Expected value of type \"Page\" but got: HomePage.", "locations": [ { "line": 2, "column": 3 } ] } ], "data": null }Where we have the following settings:
And the query:
{ pages { id title pageType slug } }