Coverage increased (+0.4%) to 82.214% when pulling e8af672 on justlevine:feat/schema-customization-with-post-type-args into a2098ea on wp-graphql:develop.

Apologies for the status spam. commit justlevine@2d38d1f broke my git history.
Had to reset and reapply the commits manually.

On the bright side, its now rebased on the latest develop

@jasonbahl I dont see a clear path towards model/dataloader support as those are usually hard-coded and called devoid of the WP_Post_Type / WP_Taxonomy object.

I'm open to suggestions, but it might make sense to polish this off with the remaining tests, and leave that part for a future PR.

@jasonbahl I dont see a clear path towards model/dataloader support as those are usually hard-coded and called devoid of the WP_Post_Type / WP_Taxonomy object.

I'm open to suggestions, but it might make sense to polish this off with the remaining tests, and leave that part for a future PR.

@justlevine ya, let's hold off on that. I have some ideas for sure, but we can tackle it separately.

Ooops. Accidentally clicked "Close with comment" instead of "Comment" 🤦🏻‍♂️

@justlevine ok, so I'm running through some scenarios and finding some issues.

I'll document.

We'll start with this basic post_type registration:

add_action( 'init', function() {

  $args = [
    'label' => 'Test Type',
    'public' => true,
    'supports' => ['comments', 'title', 'author', 'editor', 'thumbnail', 'custom-fields' ],
    'show_in_graphql' => true,
    'graphql_single_name' => 'TestType',
    'graphql_plural_name' => 'TestTypes',
  ];

  register_post_type( 'test_type', $args );

} );

graphql_exclude_connection

Because the post type has support for "comments" and "author", this gives us connections: TestType -> Author and TestType -> Comments

I can query like so now:

{
  testTypes {
    nodes {
      title
      author {
        node {
          name
        }
      }
      comments {
        nodes {
          content
        }
      }
    }
  }
}

Let's say I want to "exclude" the Author and Comments connection.

According to this PR I should be able to add: 'graphql_exclude_connections' => [ 'author', 'comments' ]

If I do that, the comments connection is properly not exposed to the Schema, but the author connection is still present.

We can see that in the screenshot below, that the TestType does not have the comments field any longer, but does still have the author field:

I've not investigated the code, but I suspect this has something to do with one-to-one connections vs. plural connections?

graphql_exclude_interfaces

Using the registration above for the test_type post type, the TestType will apply the ContentNode interface (as expected).

If we want this type to not apply the ContentNode interface, we should be able to add:

'graphql_exclude_interfaces' => [ 'ContentNode' ],

If we do this, the TestType still implements the ContentNode interface:

(full registration here):

$args = [
	'label' => 'Test Type',
	'public' => true,
	'show_in_graphql' => true,
	'graphql_single_name' => 'TestType',
	'graphql_plural_name' => 'TestTypes',
	'supports' => ['comments', 'title', 'author', 'editor', 'thumbnail', 'custom-fields' ],
	'graphql_exclude_interfaces' => [ 'ContentNode' ],
];

register_post_type( 'test-type', $args );

If I remove the supports line altogether, then the ContentNode Interface is not applied.

If I do that, the comments connection is properly not exposed to the Schema, but the author connection is still present.

That's because the author isn't registered on the type, but on the NodeWithAuthor interface. If you exclude that interface it should work.

If we want this type to not apply the ContentNode interface, we should be able to add:
'graphql_exclude_interfaces' => [ 'ContentNode' ],
If we do this, the TestType still implements the ContentNode interface:

This should work, will investigate 🧐

That's because the author isn't registered on the type, but on the NodeWithAuthor interface. If [you exclude that interface]

Gah!

Yes, this is a documentation thing, not a code thing. 🤦🏻‍♂️

Thanks for reminding me 😄

If we want this type to not apply the ContentNode interface, we should be able to add:
'graphql_exclude_interfaces' => [ 'ContentNode' ],
If we do this, the TestType still implements the ContentNode interface:

This should work, will investigate 🧐

Not by my computer to test, but pretty sure the issue is that NodeWiithFeaturedImage includes ContentNode (and some others) as inherited interfaces.

Not sure why this inherits those interfaces when other NodeWith* types don't. An oversight, maybe?

Not by my computer to test, but pretty sure the issue is that NodeWiithFeaturedImage includes ContentNode (and some others) as inherited interfaces.

Not sure why this inherits those interfaces when other NodeWith* types don't. An oversight, maybe?

Ah! Yes! 💪🏻

This seems right.

And this is likely due to my identity crisis of "what is a content node?" (i.e. #2355)

I think sometimes in my mind ContentNode is synonomous with "Post of any post type" and then other times "ContentNode" is synonomous with "Posts of public post types" 🤔

I think we need to still solve that problem. 🤔

That said, I think this could be a documentation thing, then, too.

If you exclude_interfaces, but another interface that is not excluded includes it, it will still be included. As is the case here.

I excluded ContentNode, but because I did not exclude NodeWithFeaturedImage it was still included because NodeWithFeaturedImage included it.

Now. . .the question is, do we remove ContentNode from NodeWithFeaturedImage? My instinct is yes, we should remove that declaration 🤔 (although technically a breaking change at this point if we did, so perhaps leave it and cover in documentation?)

Now. . .the question is, do we remove ContentNode from NodeWithFeaturedImage? My instinct is yes, we should remove that declaration 🤔 (although technically a breaking change at this point if we did, so perhaps leave it and cover in documentation?)

I think I agree with your instincts.

• In WPGraphQL's codebase, NodeWithFeaturedImage is only used on PostObject types, all of which are required by default to have the 'Node', 'ContentNode', 'DatabaseIdentifier', and 'NodeWithTemplate' interfaces by default. So the GraphQL object will still have the same fields if we remove any of those from NodeWithFeaturedImage.
• Because PostObject GraphQL types are usually autogenerated, its unlikely someone is manually applying this interface to a GraphQL type with the intention to inherit those particular interfaces. It's possibly theyre applying NodeWithFeaturedImage to a TermNode, but if thats they case, they'd need to be filtering off ContentNode anyway, and the other extraneous interfaces are similarly included de facto in TermObject types.
• As I mentioned, none of the of the other NodeWith* interfaces inherit these additional types. Not just does this imply that the inclusion on NodeWithFeaturedImage is a bug, but it also means that even those schemas that are moving to a nodeByUri pattern of building up generic queries with interfaces are likely not including the fields in their ... on NodeWithFeaturedImage fragments. Definitely fewer than e.g. those that could have been using broken pagination as a feature (although to be fair, the necessity of this change is less severe).

True, we could address the extra interfaces on NodeWithFeaturedImage solely in the docs, as part of whatever disclaimer that says the API can only remove direct interfaces/connections/fields, but as with deregister_graphql_field() doesnt affect inherited types, but IMO educating users about the differences between the two is hard enough without throwing curveballs in the form of unnecessary inheritance.

Code Climate has analyzed commit e8af672 and detected 43 issues on this pull request.

Here's the issue category breakdown:

View more on Code Climate.

UPGRADE NOTICE:

This PR removes the ContentNode and DatabaseIdentifier interfaces from the NodeWithFeaturedImage interface.

This is considered a breaking change (see: https://github.com/wp-graphql/wp-graphql/actions/runs/3200436057/jobs/5227330295) affecting any queries that are using a ...on NodeWithFeaturedImage fragment referencing fields added by the ContentNode or DatabaseIdentifier interface.

The following filter can be used to add back the ContentNode and DatabaseIdentifier interface to the NodeWithFeaturedImage interface:

add_filter( 'graphql_wp_interface_type_config', function( $config ) {
	if ( $config['name'] === 'NodeWithFeaturedImage' ) {
		$config['interfaces'][] = 'ContentNode';
		$config['interfaces'][] = 'DatabaseIdentifier';
	}
	return $config;
}, 10, 1 );