New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
REST API: Add variation product type to response #47377
Conversation
Test using WordPress PlaygroundThe changes in this pull request can be previewed and tested using a WordPress Playground instance. Test this pull request with WordPress Playground. Note that this URL is valid for 30 days from when this comment was last updated. You can update it by closing/reopening the PR or pushing a new commit. |
Hi @jorgeatorres, @woocommerce/mothra Apart from reviewing the code changes, please make sure to review the testing instructions as well. You can follow this guide to find out what good testing instructions should look like: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've explored a very similar approach locally and it works, I didn't see any regressions and it meets expectations, which is to have a property type
which will always be variation
for product variations.
aee7f4d
to
5ee1cc8
Compare
@woocommerce/proton It would be helpful if somebody on your team could also review this to make sure we aren't overlooking a reason why this wasn't in place originally! Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mattsherman: Thanks for submitting the PR. We also don't know the context on why this wasn't in place originally, but the change looks good to us.
Would you please mind expanding a bit on the testing instructions before merging? I think it'd be best to explicitly indicate the URL that should be used for the REST part of the test (i.e. /wp-json/wc/v3/products/<product_id>/variations
) and confirm that for regular variable products you'll get variation
but for Subscriptions variable products you should get subscription_variation
.
Thanks again!
Thanks for asking for more detailed testing instructions... they were a bit lacking! |
Submission Review Guidelines:
Changes proposed in this Pull Request:
This PR address the need to be able to distinguish the product type of a product variation by adding the following
two propertiesproperty to the product variation REST API responses:type
-- the product type for the variationparent_type
-- the product type of the variation's parentThis needs has been identified by third party extension developers when considering how to distinguish product variation types in the context of the new product editor:
Prior to this PR, no
type
property was returned for product variations, which made it impossible to determine the specific type of variation.Questions:
type
property was not originally included with product variation responses?Is theparent_type
property addition necessary?In theory, I see no inherent restriction in WooCommerce that would limit all variations of a variable product to be the exact same type, or a variation of a particular type to always have the same parent type, so this seems like a reasonable addition to help further cover different ways of distinguishing variations.But, it does have a potential performance implication...Is theparent_type
property addition a performance concern?How can I quantify the performance hit from this?Update: I removed the
parent_type
property for now because we do not have an immediate need for it.How to test the changes in this Pull Request:
Add some variable products with variations.
Install a plugin such as WooCommerce Subscriptions that adds additional variable/variation types.
Using a tool such as Postman, make REST API requests to get variations (REST API doc)
/wp-json/wc/v3/products/<product_id>/variations
Verify that the
type
andproperty is correctparent_type
propertiesvariation
subscription_variation
Changelog entry
Significance
Type
Message
Comment