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

Provide global $post in resolvers that apply filters. #195

Closed

Conversation

chriszarate
Copy link
Collaborator

Also refines descriptions.

@jasonbahl
Copy link
Collaborator

@chriszarate thanks for the PR!

Looks like a recent change I made to the composer files is causing Travis CI not to run the unit tests correctly. Fixing that now, then will run the tests on this again. . . then, will provide feedback.

Thanks!

@coveralls
Copy link

coveralls commented Aug 18, 2017

Coverage Status

Changes Unknown when pulling c0167b9 on Quartz:fix/global-post-in-resolver into ** on wp-graphql:develop**.

@jasonbahl
Copy link
Collaborator

jasonbahl commented Aug 18, 2017

@chriszarate would you be interested in writing some unit tests for this change? Would be good to make sure anything we change here works in the future as well as the project continues to evolve.

Also, can you elaborate with an example of what the output is now when you query for the content on a post vs. what happens with this change in place (a unit test would serve as this example if you have time/interest in writing a test for this)

Thanks!

Jason

@chriszarate
Copy link
Collaborator Author

Sure I'll work up a test. This is all subject to change given the comments in #160, so I'll try to write something that can be built upon.

@jasonbahl
Copy link
Collaborator

@chris I think whatever we do for #160 would likely still respect the default output if there are no args passed. So if you just ask for the content you get a default output like you do now. If you do pass args to apply certain filters, then the output would be customized to match the applied filters.

@jasonbahl
Copy link
Collaborator

@chriszarate it might be good to investigate further whether other fields rely on the $GLOBALS['post'] in the same way as the content and excerpt do. We might want to consider setting that global when the $post object is prepared to be sent down the resolve tree, so it can happen once. Or should every field be responsible for setting the global if they need it, like you have now? 🤔

Also, I'm thinking we probably need to capture the current state of the global before setting it, so that we can set it back to it's original state after the resolving is complete. While WPGraphQL is likely going to be used in a decoupled environment, it can be used within the standard WordPress environment as well, so if someone wanted to use a GraphQL query to populate a shortcode in a page, or something to that tune, not resetting the global to what it was before could cause interesting behavior.

ex:

add_shortcode( 'graphql_basic_post_list', function( $atts ) {
	$query = '
	query basicPostList($first:Int){
		posts(first:$first){
			edges{
				node{
					id
					title
					date
				}
			}
		}
	}
	';

	$variables = [
		'first' => ! empty( $atts['first'] ) ? absint( $atts['first'] ) : 5,
	];

	$data = do_graphql_request( $query, 'basicPostList', $variables );
	$edges = ! empty( $data['data']['posts']['edges'] ) ? $data['data']['posts']['edges'] : [];

	if ( ! empty( $edges ) && is_array( $edges ) ) {
		$output = '<ul>';
		foreach ( $edges as $edge ) {
			$node = ! empty( $edge['node'] ) ? $edge['node'] : '';
			if ( ! empty( $node ) && is_array( $node ) ) {
				$output .= '<li id="' . $node['id'] . '">' . $node['title'] . ' ' . $node['date'] . '</li>';
			}
		}
		$output .= '</ul>';
	}

	return ! empty( $output ) ? $output : '';
});

If we don't reset the $global, anything on the page below the shortcode, like related posts, comments, etc would get funky. So whenever we override the $GLOBALS['post'] we need to make sure we reset it.

FWIW, this shortcode works, and an example screenshot is attached:

wpgraphql-basic-shortcode-example

@jasonbahl
Copy link
Collaborator

@chriszarate any thoughts on my feedback?

@chriszarate
Copy link
Collaborator Author

Closing in favor of #230.

@chriszarate chriszarate deleted the fix/global-post-in-resolver branch September 21, 2017 00:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants