Skip to content
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

Show extension metadata in order item details #4753

Merged
merged 5 commits into from
Aug 9, 2021

Conversation

Ecarrion
Copy link
Contributor

@Ecarrion Ecarrion commented Aug 9, 2021

fix #4708

Why

It was reported(4182704-zd-woothemes) that merchants were able to see some order metadata in Android but not in iOS.

After some investigation, this happens because some extensions(like Measurement Price Calculator) adds some extra metadata that does not have the format we expect, hence making the whole metadata/attributes decoding to fail.

How

This PR solves the problem by making the value field from OrderItemAttribute to be decoded safely when an unexpected format is found.

Screenshots

Core Before After
core bad good

Testing Steps

Follow testings steps on #4708

Update release notes:

  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@Ecarrion Ecarrion added the feature: order details Related to order details. label Aug 9, 2021
@Ecarrion Ecarrion added this to the 7.4 milestone Aug 9, 2021
@Ecarrion Ecarrion requested a review from a team August 9, 2021 15:01
@peril-woocommerce
Copy link

peril-woocommerce bot commented Aug 9, 2021

You can trigger an installable build for these changes by visiting CircleCI here.

@peril-woocommerce
Copy link

peril-woocommerce bot commented Aug 9, 2021

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

@@ -21,7 +21,10 @@ public struct OrderItemAttribute: Decodable, Hashable, Equatable, GeneratedFakea
let container = try decoder.container(keyedBy: CodingKeys.self)
let metaID = try container.decode(Int64.self, forKey: .metaID)
let name = try container.decode(String.self, forKey: .name)
let value = try container.decode(String.self, forKey: .value)

/// Some extensions sent the `value` field with non-string format.
Copy link
Contributor

Choose a reason for hiding this comment

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

sent -> send

@@ -325,6 +337,12 @@ private extension OrderMapperTests {
return mapOrder(from: "order-with-line-item-attributes")
}

/// Returns the OrderMapper output upon receiving `order-with-faulty-attributes`
///
func mapLoadOrderWithFaultyAttributesResponse() -> Order? {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please consider mentioning in a comment that the _measurement_data value having object format (and not a string) is what is "faulty" here. I had to fire up the debugger to understand what in the order was the problem.

Copy link
Contributor

@allendav allendav left a comment

Choose a reason for hiding this comment

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

Tests fine. Two suggestions to consider. :shipit:

@Ecarrion Ecarrion enabled auto-merge August 9, 2021 19:16
@Ecarrion
Copy link
Contributor Author

Ecarrion commented Aug 9, 2021

Thanks for the review and suggestions @allendav!

@Ecarrion Ecarrion merged commit 6751832 into develop Aug 9, 2021
@Ecarrion Ecarrion deleted the issue/4708-order-metadata branch August 9, 2021 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature: order details Related to order details.
Projects
None yet
2 participants