Skip to content
This repository was archived by the owner on Jul 23, 2024. It is now read-only.

"graphql_acf_relationship_post_model" hook added. #124

Closed
wants to merge 1 commit into from

Conversation

kidunot89
Copy link
Collaborator

Adds a hook so a custom post type model can be provided to ACF "relationship" group type resolvers.

This is related to an issue in WooGraphQL #253.

$post_object = get_post( $post_id );
if ( $post_object instanceof \WP_Post ) {
$post_model = new Post( $post_object );
}

$post_model = apply_filters( 'graphql_acf_relationship_post_model', $post_model, $post_id, $root, $acf_field );
Copy link
Contributor

@jasonbahl jasonbahl Apr 16, 2020

Choose a reason for hiding this comment

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

@kidunot89 So, this is a bit of a band-aid and doesn't address the underlying issue.

Any other plugin like Carbon Fields, CMB2, Metabox.io, etc, etc will have similar needs as this, and going around filtering all their fields to play nice with WooCommerce custom Models doesn't scale.

First, I think the ACF plugin needs to take advantage of Deferred loading and use the Loader. That's on me to get updated in this repo.

This would then mean that the Model wouldn't even be instantiated in this resolver, so this would need to be handled more centrally in the Loader level.

Perhaps a central filter somewhere in the PostObjectLoader might help us re-route posts of the "product" type to the Product Model 🤔 (maybe that's already even happening?)

Then I think it might solve some headaches if WPGraphQL for WooCommerce extended the WPGraphQL Model Layer.

From what I can tell, you extend Model class here: https://github.com/wp-graphql/wp-graphql-woocommerce/blob/develop/includes/model/class-crud-cpt.php#L20

And since the Crud_CPT Model is intended to deal with CPTs, so you should probably extend the Post model instead of just Model.

This would then mean that any field expecting a Post model would work with any instance of a Product Model, because they'd be an extension of the Post model.

That would, I believe, solve a lot of one-off headaches around this stuff?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I could try that, but it could definitely break some things.

@kidunot89
Copy link
Collaborator Author

@jasonbahl I was pretty sure we had resolved this in another PR, but I can't find that PR 🤦‍♂️

@jasonbahl
Copy link
Contributor

@kidunot89 oh gosh. This was the Model issue we discussed with @CodeProKid. Haha!

@kidunot89
Copy link
Collaborator Author

Yea, but I remember us having video call where we resolved it.

@jasonbahl
Copy link
Contributor

@kidunot89 ok, so I think this code needs to change to return $context->get_loader( 'post' )->load_deferred();

Then, in the loader we need to have a filter where you can return a Product model if the post_type == 'product'

Right now we have a check if it's a nav_menu_item post type and if so we return a MenuItem() model. This has no filter for other post types, and we need to support that.

I think we should add a new method to the Abstract Loader called get_model() or something like that which has a filter and all loaders use that method to return the Model and respect the same filter.

I can get a PR in shortly, then you can filter the loader to return whatever model(s) you need to.

@kidunot89
Copy link
Collaborator Author

Yea, thats the solution I kinda remember. I though we had opened a PR then 😅.

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

Successfully merging this pull request may close these issues.

2 participants