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

Return null for organizers when none are associated with the event. #30

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

justlevine
Copy link
Contributor

This PR fixes organizers so it returns null if no organizers are set for the event.
Fixes #13

@justlevine justlevine changed the title Bugfix/return null if no organizers Return null for organizers when none are associated with the event. Jan 24, 2021
Copy link
Member

@bordoni bordoni left a comment

Choose a reason for hiding this comment

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

One small performance change.

@@ -29,6 +34,12 @@ public static function register_connections() {
'fromType' => 'Event',
'toType' => 'Organizer',
'fromFieldName' => 'organizers',
'resolve' => function( $source, array $args, AppContext $context, ResolveInfo $info ) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
'resolve' => function( $source, array $args, AppContext $context, ResolveInfo $info ) {
'resolve' => static function( $source, array $args, AppContext $context, ResolveInfo $info ) {

@justlevine
Copy link
Contributor Author

justlevine commented Apr 9, 2021

@bordoni What is the performance benefit of using static in this case?
I ask because I've never seen resolve return as static in WPGraphQL or any of its extensions I've worked with, and I'm under the (possibly false) impression that static methods are actually slower than instantiated ones in php 7.2+ (but no idea if that holds true on a callable )

@kidunot89
Copy link
Collaborator

kidunot89 commented Sep 1, 2022

@justlevine @bordoni I don't think there are any here since most resolvers don't get called on each request and this would automatically allocate a small amount of CPU/RAM to this static closure on each request, even the ones where it doesn't get called.

Copy link
Collaborator

@kidunot89 kidunot89 left a comment

Choose a reason for hiding this comment

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

@justlevine The recommended method for resolving your initial issue is to default the empty $organizer_id array to ["0"]. This will force the desired empty result from the connection resolver.

@justlevine
Copy link
Contributor Author

@kidunot89 ultimately I even took it further, and went with setting the IDs on the model for use on the connection interface.

Feel free to change whatever you want on this PR or even just close it.

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

Successfully merging this pull request may close these issues.

When an event has no organizer selected, graphql result returns all existing organizers
3 participants