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

Model lists as Lists of NonNull elements #4

Merged
merged 1 commit into from
Aug 20, 2018

Conversation

cysp
Copy link
Member

@cysp cysp commented Feb 16, 2018

contentful#48 but for our fork. 🙃

@petronbot
Copy link

LGTM

@cysp cysp force-pushed the scentregroup/nonnull-list-elements branch from d2c407e to 82b830a Compare August 6, 2018 06:28
Copy link

@ianhowe76 ianhowe76 left a comment

Choose a reason for hiding this comment

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

👍

@ghost
Copy link

ghost commented Aug 16, 2018

How is this handled in the native GraphQL implementation?

@cysp
Copy link
Member Author

cysp commented Aug 17, 2018

It's a schema definition concern rather than an implementation difference, this is changing the definition of lists of entries from [EntryType] to [EntryType!], indicating that while the list can itself be null or empty, none of its elements will be null.

I've done some testing in the development space with the lists of occurrences for an event and it looks like the existing implementation filters out unpublished entries already, it just doesn't reflect that in the schema definition. So I think that this won't result in breakage.

@cysp
Copy link
Member Author

cysp commented Aug 17, 2018

Concretely, querying the occurrences for this event x74vyp9vlegq:1wGth51Ko4C8EOeGU0wSoo results in a list of occurrences two elements long, despite the entry referencing multiple unpublished occurrences.

@cysp
Copy link
Member Author

cysp commented Aug 19, 2018

Haha I just went to merge this but realised that I can't because I don't have write access. ☹️

@cysp cysp merged commit f3529cb into ScentreGroup:master Aug 20, 2018
@cysp cysp deleted the scentregroup/nonnull-list-elements branch August 20, 2018 00:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants