Skip to content

Conversation

@bummytime
Copy link
Contributor

@bummytime bummytime commented Jan 8, 2019

This PR adds the price field to the order item entities across the Networking, Storage, and Yosemite frameworks. Additionally it fixes the item price display issue on the order details and fulfillment screens:

item_price_fix

Notes:

  • There is a data model update to the version 9 model — however it has not been released to the wild yet (it will be in version 0.15)
  • The Networking OrderItem model includes price as a NSDecimalNumber. There are 2 reasons for this: 1) It makes formatting easier in the main Woo app and 2) even though the documentation claims the item price is a string, in reality, the response JSON returns price as a Number
  • I updated all of the appropriate Networking, Storage, and Yosemite framework unit tests to test the new price field
  • I overloaded CurrencyFormatter's formatAmount() func to accept a NSDecimalNumber as well as a String (and added some unit tests as well)

Fixes: #358
Ref: #552

Testing

  • Build and run all the unit tests — verify everything is ✅

Here are some suggested testing steps, but feel free to test this however you want.

  1. Build and run the app

  2. Open the orders tab

  3. Find an order with Processing status where the line items have a quantity of 1

  4. Open the fullfillment screen

    • Verify the item price and quantity details looks good
  5. Find an order with Completed status where the line items have a quantity of 1

  6. Open the item details screen

    • Verify the item price and quantity details looks good
  7. Find an order with Processing status where the line items have a quantity of 2 or more

  8. Open the fullfillment screen

    • Verify the item price and quantity details looks good
  9. Find an order with Completed status where the line items have a quantity of 2 or more

  10. Open the item details screen

    • Verify the item price and quantity details looks good

@mindgraffiti and/or @astralbodies Could either of you give this PR a 👀 ? Thanks!!

@bummytime bummytime added type: bug A confirmed bug. feature: order details Related to order details. labels Jan 8, 2019
@bummytime bummytime added this to the 0.15 milestone Jan 8, 2019
@bummytime bummytime self-assigned this Jan 8, 2019
Copy link
Contributor

@mindgraffiti mindgraffiti left a comment

Choose a reason for hiding this comment

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

✅ unit tests
✅ code review
✅ UI testing

Thanks Matt!!

:shipit:

/// - currency: a 3-letter country code for currencies that are supported in the API. e.g. "USD"
///
func formatAmount(_ decimalAmount: NSDecimalNumber, with currency: String = CurrencySettings.shared.currencyCode.rawValue) -> String? {

Copy link
Contributor

Choose a reason for hiding this comment

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

yesss thank you 👍

@bummytime
Copy link
Contributor Author

Thanks for the review @mindgraffiti !!

@wpmobilebot
Copy link
Collaborator

1 Warning
⚠️ Core Data: Do not edit an existing model in a release branch unless it hasn’t been released to testers yet. Instead create a new model version and merge back to develop soon.

Generated by 🚫 Danger

@bummytime
Copy link
Contributor Author

Take it easy wpmobilebot!

@bummytime bummytime merged commit bd01f6e into develop Jan 8, 2019
@bummytime bummytime deleted the issue/358-item-price branch January 8, 2019 21:06
@astralbodies astralbodies modified the milestones: 0.15, App Store release Jan 11, 2019
bummytime added a commit that referenced this pull request Jan 11, 2019
bummytime added a commit that referenced this pull request Jan 11, 2019
Reapply updates from "item price" PR (#577)
bummytime added a commit that referenced this pull request Jan 11, 2019
…s into fix/563-mark-as-read-notice

* 'release/1.0.0' of github.com:woocommerce/woocommerce-ios:
  Updates from "item price" PR (#577)
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. type: bug A confirmed bug.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Product Details: Item Price Inconsistency

5 participants