feat: Refactor NodeResolver::resolve_uri() to use WP_Query#2680
Conversation
This is a codeception quick we need to work around.
| $page_on_front = get_option( 'page_on_front', 0 ); | ||
| $post = get_post( absint( $page_on_front ) ); | ||
| return ! empty( $post ) ? $this->context->get_loader( 'post' )->load_deferred( $post->ID ) : null; | ||
| return ! empty( $post_type_object->name ) ? $this->context->get_loader( 'post_type' )->load_deferred( $post_type_object->name ) : null; |
There was a problem hiding this comment.
Avoid too many return statements within this method.
| $post = isset( $post->ID ) ? $this->validate_post( $post ) : null; | ||
| return isset( $post->ID ) ? $this->context->get_loader( 'post' )->load_deferred( $post->ID ) : null; | ||
| } | ||
| return ! empty( $queried_object->ID ) ? $this->context->get_loader( 'post' )->load_deferred( $queried_object->ID ) : null; |
There was a problem hiding this comment.
Avoid too many return statements within this method.
| $post_type_object = get_post_type_object( $this->wp->query_vars['post_type'] ); | ||
| // Resolve Terms. | ||
| if ( $queried_object instanceof \WP_Term ) { | ||
| return ! empty( $queried_object->term_id ) ? $this->context->get_loader( 'term' )->load_deferred( $queried_object->term_id ) : null; |
There was a problem hiding this comment.
Avoid too many return statements within this method.
| return ! empty( $post_type_object ) ? $this->context->get_loader( 'post_type' )->load_deferred( $post_type_object->name ) : null; | ||
| // Resolve Post Types. | ||
| if ( $queried_object instanceof \WP_Post_Type ) { | ||
| return ! empty( $queried_object->name ) ? $this->context->get_loader( 'post_type' )->load_deferred( $queried_object->name ) : null; |
There was a problem hiding this comment.
Avoid too many return statements within this method.
| } | ||
| // Resolve Users | ||
| if ( $queried_object instanceof \WP_User ) { | ||
| return ! empty( $queried_object->ID ) ? $this->context->get_loader( 'user' )->load_deferred( $queried_object->ID ) : null; |
There was a problem hiding this comment.
Avoid too many return statements within this method.
| } | ||
|
|
||
| return $node; | ||
| return apply_filters( 'graphql_post_resolve_uri', $node, $uri, $queried_object, $query, $this->context, $this->wp, $extra_query_vars ); |
There was a problem hiding this comment.
Avoid too many return statements within this method.
| switch ( $idType ) { | ||
| case 'uri': | ||
| return $context->node_resolver->resolve_uri( $args['id'] ); | ||
| return $context->node_resolver->resolve_uri( |
There was a problem hiding this comment.
Avoid too many return statements within this method.
| break; | ||
| case 'uri': | ||
| return $context->node_resolver->resolve_uri( $args['id'] ); | ||
| return $context->node_resolver->resolve_uri( |
There was a problem hiding this comment.
Avoid too many return statements within this method.
| break; | ||
| case 'uri': | ||
| return $context->node_resolver->resolve_uri( $args['id'] ); | ||
| return $context->node_resolver->resolve_uri( |
There was a problem hiding this comment.
Avoid too many return statements within this method.
| * | ||
| * @return \WP_Term|null | ||
| */ | ||
| public function validate_term( \WP_Term $term ) { |
There was a problem hiding this comment.
Function validate_term has a Cognitive Complexity of 8 (exceeds 5 allowed). Consider refactoring.
|
Code Climate has analyzed commit 47fe8ab and detected 10 issues on this pull request. Here's the issue category breakdown:
View more on Code Climate. |
|
Verifying #1910 is resolved:
{
nodeByUri(uri: "/shop") {
__typename
... on ContentType {
name
contentNodes {
nodes {
__typename
databaseId
}
}
}
}
}Results:
add_filter( 'register_post_type_args',
function( array $args, string $post_type ) {
if ( 'product' === $post_type ) {
$args['show_in_graphql'] = true;
$args['graphql_single_name'] = 'product';
$args['graphql_plural_name'] = 'products';
}
return $args;
},
10, 2 );
{
page(id: "/shop", idType: URI) {
__typename
databaseId
}
}Result: |
What does this implement/fix? Explain your changes.
This pr refactors
NodeResolver::resolve_uri()to useWP_Query, instead of the current approach where we try to replicate the class logic locally.This brings the resolution process more in line with
WP::main(), mitigating edge cases and reducing code complexity.In addition, it adds the following WP filters to the method:
graphql_resolve_uri_query_class. Used to swap out WP_Query with a specialized class, e.g.WC_Queryfrom woo.graphql_resolve_uri. Used to short-circuitresolve_uri()after the query has been run, but before our own attempts to resolve it to a WPGraphQLnode, e.g. if we want a CPT to use a special dataloader.graphql_post_resolve_uri. Used to provide a WPGraphQLnodeifresolve_uri()is unable to do so, e.g. a plugin-specific Edge case.Does this close any currently open issues?
fixes #2178
fixes #2190
closes #1910
(possibly others)
Part of #2366
Any relevant logs, error output, GraphiQL screenshots, etc?
(If it’s long, please paste to https://ghostbin.com/ and insert the link here.)
Any other comments?
nodeByUriquery testing #2659, which should be merged first.$extra_query_vars['nodeType']and$extra_query_vars['taxonomy']are now set in more places to let us validate the type we're receiving is compatible with the GraphQL type we resolve to.nodeByUritests for when pretty permalinks are disabled for now. The reason is thatparse_request()(from before this PR) marks those as a 404, which we were able to ignore when we were using our own logic before calling WP_Query, but no longer. I believe this is a quirk with Codeception, and not an issue with the code itself, and I am looking into workarounds to reinstate those tests.WP_Queryresults are usually cached, and we're hitting it earlier in the lifecycle), but we should do some profiling to be 100% sure. This is beyond my skillset.graphql_debug()saying theyre not currently supported, and fallback to theContentType, but there isnt precedent for that in the codebase.Where has this been tested?
Operating System: Ubuntu 20.04 (wsl2 + devilbox + php8.0.19)
WordPress Version: 6.1.1