Closed shiki closed 4 years ago
Thanks for creating the PR, great job (the description is 🥇 )!
cc @vedanshujain as we're going to move the API to the core repo (https://github.com/woocommerce/woocommerce/pull/27100), so we might need to move this one over to the woo/woo repo.
Hi @peterfabian! Thank you. 🙇
I didn't know that we were merging this back into Core. 😱 Should I wait until woocommerce/woocommerce#27100 is merged then and submit the PR in Core?
Also, don't worry about opening the API core, I will move when the PR is approved and ready to be merged :+1:
Looks good and testing well, I am going to move this in a PR in core repo and close this PR.
Thank you, @vedanshujain! 🙏
Closing in favor of https://github.com/woocommerce/woocommerce/pull/27492
Fixes #9.
Ref: p91TBi-2Lt-p2
This will also allow us to fix these issues for the WooCommerce iOS and Android apps:
Background
For both the iOS and Android apps, we regularly receive feedback from users that they could not view the details of the variable products in the orders. Example: C013AAPA4G0-p1596070344020400-slack. To find a solution, @amandariu investigated what the iOS and Android apps can do using the REST API and posted her findings at p91TBi-2Lt-p2.
The apps use the
line_items
name
in theGET /v3/orders
response. However, thename
does not include all the selected variations. Consider an order for a variable product with color "Blue" and size "Medium". The API endpoint would return something like this (simplified result):As shown above, the size was not included in the
name
. This is how it would look like in the iOS app:WCAdmin shows the attribute that was omitted:
Why Not Use Metadata?
We could use the
meta_data
array to find the missing attributes. However, this may be unreliable because:name
to figure out which attribute is already filled in. This can include finding the dash separator, which I think can be customized too.value
inmeta_data
is the sanitized value. In the example above,"medium"
should be"Medium"
(capital M).Solution
The
name
not showing all the attributes is probably attributed to thegenerate_product_title
function in WooCommerce. It's probably possible to solve the issue by making sure that all attributes are included. However, this won't solve one of our other design requirements (https://github.com/woocommerce/woocommerce-ios/issues/2280) which is to present the attributes separately:We would still have to do a string operation to split out the attribute values.
In this PR, I'm proposing adding the displayable attribute name and values in the
meta_data
instead:With these available properties, the API clients (e.g. iOS and Android apps) would be able to easily decide how to present the attributes.
There is still the problem with the
name
possibly showing one of the attributes. I'm planning to submit another PR to fix that after this one.Schema
I added the new properties above in the schema.
Changes
These new properties are only applied to Orders API V3 since we are hoping to get fixed in the mobile apps soon. I did this by overriding the
get_order_item_data
inControllers\Version3\ WC_REST_Orders_Controller
.Please let me know if I should include this in V4 as well.
Testing
This is just a suggestion but please feel free to test other scenarios. 🙂
GET /wp-json/wc/v3/orders/{OrderID}
.line_items
meta_data
has thedisplay_key
anddisplay_value
properties and they are correct.Unit Test
I have also added a unit test that I think covers the manual testing scenario above.