Post Type schema settings implemented by kidunot89 · Pull Request #1486 · wp-graphql/wp-graphql

kidunot89 · 2020-10-03T08:19:15Z

Your checklist for this pull request

Thanks for sending a pull request! Please make sure you click the link above to view the contribution guidelines, then fill out the blanks below.

🚨Please review the guidelines for contributing to this repository.

Make sure you are making a pull request against the develop branch (left side). Also you should start your branch off our develop.
Make sure you are requesting to pull request from a topic/feature/bugfix branch (right side). Don't pull request from your master!

What does this implement/fix? Explain your changes.

Adds more Post-type settings related to WPGraphQL and using them to allows post-types to opt-in or out of special points of registration in the GraphQL tree. See the example below to see how this works with the Product type of WooGraphQL.

/**
 * Registers WooCommerce post-types to be used in GraphQL schema
 *
 * @param array  $args      - allowed post-types.
 * @param string $post_type - name of taxonomy being checked.
 *
 * @return array
 */
function register_post_types( $args, $post_type ) {
	if ( 'product' === $post_type ) {
		$args['show_in_graphql']                 = true;
		$args['graphql_single_name']             = 'Product';
		$args['graphql_plural_name']             = 'Products';
		$args['register_schema_type']            = false;
		$args['register_root_query']             = false;
		$args['register_root_connection']        = false;
		$args['connect_to_comments']             = false;
		$args['connect_to_taxonomies']           = false;
		$args['use_post_object_loader']          = false;
	}

        return $args;
}
add_filter( 'register_post_type_args', 'register_post_types', 10, 2 );

Does this close any currently open issues?

Closes #1443

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?

…

Where has this been tested?

Operating System: …

WordPress Version: …

netlify · 2020-10-03T08:19:17Z

Deploy request for wpgraphql-docs pending review.

Review with commit 3c49760

https://app.netlify.com/sites/wpgraphql-docs/deploys

codecov · 2020-10-03T08:40:44Z

Codecov Report

Merging #1486 into develop will decrease coverage by 0.10%.
The diff coverage is 50.00%.

@@             Coverage Diff             @@
##           develop    #1486      +/-   ##
===========================================
- Coverage    59.04%   58.93%   -0.11%     
===========================================
  Files          192      192              
  Lines        11270    11299      +29     
===========================================
+ Hits          6654     6659       +5     
- Misses        4616     4640      +24

Impacted Files	Coverage Δ
src/Type/Object/Taxonomy.php	`50.00% <ø> (-0.61%)`	⬇️
wp-graphql.php	`31.55% <29.62%> (-0.30%)`	⬇️
src/Type/Union/MenuItemObjectUnion.php	`64.28% <50.00%> (-13.50%)`	⬇️
src/Connection/PostObjects.php	`62.75% <75.00%> (-0.08%)`	⬇️
src/Connection/Comments.php	`43.84% <100.00%> (ø)`
src/Connection/TermObjects.php	`58.62% <100.00%> (ø)`
src/Data/Loader/PostObjectLoader.php	`84.31% <100.00%> (ø)`
src/Data/NodeResolver.php	`77.90% <100.00%> (ø)`
src/Registry/TypeRegistry.php	`79.61% <100.00%> (ø)`
src/Type/Enum/ContentNodeIdTypeEnum.php	`77.27% <100.00%> (ø)`
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5f4efd0...2a349fd. Read the comment docs.

jasonbahl · 2020-10-05T19:22:23Z

src/Connection/Comments.php

 		 * Register Connections from all existing PostObject Types to Comments
 		 */
-		$allowed_post_types = \WPGraphQL::get_allowed_post_types();
+		$allowed_post_types = \WPGraphQL::get_allowed_post_types( [ 'connect_to_comments' => true ] );


How is this different than what we're already doing here:

wp-graphql/src/Connection/Comments.php

Line 80 in 2a349fd

if ( post_type_supports( $post_type_object->name, 'comments' ) ) {

What if you set connect_to_comments => true but the post type doesn't register support for comments?

Seems kinda like an akward rule when we already have: show_in_graphql and supports => [ 'comments' ]

@jasonbahl Not necessarily, currently I manual register the comments connection for the Product type in WooGraphQL so I can add the product comment (review) meta rating as an edge field. Although, I plan to change this in the near future, so that one won't be necessary for long, but I can see a need this when you want to defer to an alternate source for comments like Disqus 🤔

The Disqus use case is interesting. 🤔

You would want the post_type to support comments in general, but would want to be able to disable the connection in GraphQL because the actual comments are handled outside of WordPress. 🤔

that or in a custom table that might use a custom query class or unique $wpdb functions.

jasonbahl · 2020-10-05T19:24:21Z

wp-graphql.php

+		public static function add_post_type_settings( $args, $post_type ) {
+			global $wp_post_types;
+
+			$post_type_settings = [


I think we should nest these rules under a graphql key.

like:

'graphql' => [ 'use_post_object_loader' => true, ...other args ]

jasonbahl · 2020-10-05T19:26:32Z

wp-graphql.php

+				'supports_post_object_union'      => true,
+				'supports_menu_item_object_union' => true,
+				'resolve_from_uri'                => true,
+				'use_post_object_loader'          => true,


I wonder if instead of a true/false here it should just be the name of the loader to use?

Good idea 👍 I'll see what I can wire up.

jasonbahl · 2020-10-05T19:28:11Z

wp-graphql.php

+				'supports_content_node'           => true,
+				'supports_post_object_union'      => true,
+				'supports_menu_item_object_union' => true,
+				'resolve_from_uri'                => true,


WordPress already has rules for when objects can/cannot resolve from a URI. If it's a public post type and the post object is published or in some other publicly visible state, it has a URI. The goal is to be able to use queries like nodeByUri( $uri: '/same-url-you-could-use-in-browser )` and be able to get the same node the browser would return.

So this should be determined by if the post-type's public property is set to true?

jasonbahl · 2020-10-05T19:29:37Z

wp-graphql.php

+				'connect_to_taxonomies'           => true,
+				'supports_content_node'           => true,
+				'supports_post_object_union'      => true,
+				'supports_menu_item_object_union' => true,


WordPress also has rules for this.

show_in_nav_menus: https://developer.wordpress.org/reference/functions/register_post_type/#show_in_nav_menus

If a CPT is set to show in Nav Menus, WPGraphQL should support Nav Menu Items returning that type.

I know, but this is just determine if it's displayed that way in the schema tree.

jasonbahl · 2020-10-05T19:30:59Z

wp-graphql.php

+				'connect_to_comments'             => true,
+				'connect_to_taxonomies'           => true,
+				'supports_content_node'           => true,
+				'supports_post_object_union'      => true,


Hmm. If you use register_post_type() and show_in_graphql your post type is already agreeing to this. 🤔

Not necessarily, I'm forced to inject the PostObjectUnion using the graphql_union_resolve_type filter just to make it work with Product child types, if this is toggled to true.

jasonbahl · 2020-10-05T19:32:36Z

wp-graphql.php

+				'register_root_query'             => true,
+				'register_root_connection'        => true,
+				'connect_to_comments'             => true,
+				'connect_to_taxonomies'           => true,


This case is already handled by registering a post type and taxonomy to be connected to each other and having both set to show_in_graphql. If you set one or the other, or both to not show_in_graphql that decision is respected.

jasonbahl · 2020-10-05T19:33:03Z

wp-graphql.php

+				'register_schema_type'            => true,
+				'register_root_query'             => true,
+				'register_root_connection'        => true,
+				'connect_to_comments'             => true,


See: https://github.com/wp-graphql/wp-graphql/pull/1486/files#r499819148

jasonbahl · 2020-10-05T19:34:16Z

wp-graphql.php

+				'graphql_plural_name'             => str_replace( '_', '', ucwords( strtolower( $post_type ), '_' ) ) . 's',
+				'register_schema_type'            => true,
+				'register_root_query'             => true,
+				'register_root_connection'        => true,


what's the use case for this so I can understand more?

Same thing as the register_root_query. Most of WooGraphQL connections resolve using custom AbstractConnectionResolver classes and not the PostObjectConnectionResolver class. This is due to the fact that they use a WooCommerce Query class and not WP_Query

jasonbahl · 2020-10-05T19:35:10Z

wp-graphql.php

+				'show_in_graphql'                 => true,
+				'graphql_single_name'             => str_replace( '_', '', ucwords( strtolower( $post_type ), '_' ) ),
+				'graphql_plural_name'             => str_replace( '_', '', ucwords( strtolower( $post_type ), '_' ) ) . 's',
+				'register_schema_type'            => true,


what's the use case for this so I can understand more?

This would replace the primary use of the skip-post-type-registry WooGraphQL has been using to opt out of using the core PostObject types definition for WooCommerce post-type so it can roll it's own.

See the example in the PR summary above to see how it will be use for Products in WooGraphQL. ☝️

jasonbahl · 2020-10-05T19:35:14Z

wp-graphql.php

+				'graphql_single_name'             => str_replace( '_', '', ucwords( strtolower( $post_type ), '_' ) ),
+				'graphql_plural_name'             => str_replace( '_', '', ucwords( strtolower( $post_type ), '_' ) ) . 's',
+				'register_schema_type'            => true,
+				'register_root_query'             => true,


what's the use case for this so I can understand more?

The root queries in WooGraphQL are actually rolled for the time being actually 😅.

stale · 2022-08-02T21:20:46Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale · 2022-09-01T21:42:27Z

This issue has been automatically closed because it has not had recent activity. If you believe this issue is still valid, please open a new issue and mark this as a related issue.

justlevine · 2022-09-02T16:53:43Z

superseded by #2399

Post Type schema settings implemented

ae9da33

kidunot89 requested a review from jasonbahl October 3, 2020 08:19

reverts change made to the CommentObjectQueriesTest class.

2a349fd

jasonbahl reviewed Oct 5, 2020

View reviewed changes

jasonbahl requested changes Oct 5, 2020

View reviewed changes

WPGraphQLAllowedPostTypesFilterTest added.

3c49760

kidunot89 requested a review from jasonbahl October 6, 2020 23:18

This was referenced Oct 12, 2020

Bugfix/node resolver target public cpts #1503

Merged

Incompatibility with GraphQL Meta Query wp-graphql/wp-graphql-woocommerce#264

Open

This was referenced May 27, 2022

NodeByUri will not query WooCommerce products since v1.8.1 #2394

Closed

feat: schema customization with post type args #2399

Merged

stale bot added the stale label Aug 2, 2022

stale bot closed this Sep 1, 2022

Conversation

kidunot89 commented Oct 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Your checklist for this pull request

What does this implement/fix? Explain your changes.

Does this close any currently open issues?

Any relevant logs, error output, GraphiQL screenshots, etc?

Any other comments?

Where has this been tested?

Uh oh!

netlify bot commented Oct 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Oct 3, 2020

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kidunot89 Oct 5, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kidunot89 Oct 6, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stale bot commented Aug 2, 2022

Uh oh!

stale bot commented Sep 1, 2022

Uh oh!

justlevine commented Sep 2, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

kidunot89 commented Oct 3, 2020 •

edited

Loading

netlify bot commented Oct 3, 2020 •

edited

Loading

kidunot89 Oct 5, 2020 •

edited

Loading

kidunot89 Oct 6, 2020 •

edited

Loading