Fix Product Variation Shipping Class #895
Conversation
There was a problem hiding this comment.
@robbiebel Sorry for the slow review of this. It's failing CI and I can't merge it as long as it is.
|
Hi @kidunot89, I'm also sorry for slowness on my side. I have tried before create first PR and now again. But I have not succeed testing on my local although I tried a few things.
|
There was a problem hiding this comment.
I think I understand what you're trying to achieve here but there is some work to be done. See my comments and also take a look at CONTRIBUTING.md
| @@ -413,7 +413,7 @@ protected function init() { | |||
| return ! empty( $this->wc_data->get_height() ) ? $this->wc_data->get_height() : null; | |||
| }, | |||
| 'shippingClassId' => function () { | |||
| return ! empty( $this->wc_data->get_image_id() ) ? $this->wc_data->get_shipping_class_id() : null; | |||
| return ! empty( $this->wc_data->get_shipping_class_id() ) ? $this->wc_data->get_shipping_class_id() : null; | |||
| 'shippingClassId' => function () { | ||
| return ! empty( $this->wc_data->get_shipping_class_id() ) ? $this->wc_data->get_shipping_class_id() : null; | ||
| }, |
There was a problem hiding this comment.
Is this necessary, why can't we continue to used the snake case shipping_class_id
| 'shippingClass' => [ | ||
| 'type' => 'String', | ||
| 'type' => 'ShippingClass', |
There was a problem hiding this comment.
This type ShippingClass does not exist.
There was a problem hiding this comment.
I thought this is the type:
https://woographql.com/schema/definition/-shipping-class
There was a problem hiding this comment.
That means this needs to be provided with a resolver that resolves to the TermNode. The resolver should looks something like this.
'resolve' => static function ( $source, array $args, $context ) {
$id = $source->shipping_class_id;
if ( $id ) {
return $context->get_loader( "term" )->load_deferred( $id );
}
return null;
},
I already followed CONTRIBUTING.md but not succeeded even I have good experience in Docker. The CONTRIBUTING.md has many missing parts for fresh install. But I achieved to run docker applications by tracing the code and filling the missing parts. I recommend that updating the CONTRIBUTING.md |
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.
What does this implement/fix? Explain your changes.
…
Product Variations Shipping Classes was not implemented correctly.
Does this close any currently open issues?
…
No
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?
…
It's a small fix to understand easily to look changes.