diff --git a/Modules/Sources/NetworkingCore/Remote/OrdersRemote.swift b/Modules/Sources/NetworkingCore/Remote/OrdersRemote.swift index 67c666071db..a4069d516a6 100644 --- a/Modules/Sources/NetworkingCore/Remote/OrdersRemote.swift +++ b/Modules/Sources/NetworkingCore/Remote/OrdersRemote.swift @@ -228,6 +228,8 @@ public class OrdersRemote: Remote { params[Order.CodingKeys.createdVia.rawValue] = createdViaValue } + params[ParameterKeys.decimalPlaces] = OrdersRemote.Defaults.decimalPoints + return params }() @@ -253,7 +255,10 @@ public class OrdersRemote: Remote { /// public func updateOrder(from siteID: Int64, orderID: Int64, statusKey: OrderStatusEnum, completion: @escaping (Order?, Error?) -> Void) { let path = "\(Constants.ordersPath)/" + String(orderID) - let parameters = [ParameterKeys.statusKey: statusKey.rawValue] + let parameters = [ + ParameterKeys.statusKey: statusKey.rawValue, + ParameterKeys.decimalPlaces: OrdersRemote.Defaults.decimalPoints + ] let mapper = OrderMapper(siteID: siteID) let request = JetpackRequest(wooApiVersion: .mark3, @@ -328,6 +333,9 @@ public class OrdersRemote: Remote { value: cashPaymentChangeDueAmount).toDictionary()] } + // Add decimal places parameter for better precision + params[ParameterKeys.decimalPlaces] = OrdersRemote.Defaults.decimalPoints + return params }() @@ -449,8 +457,9 @@ extension OrdersRemote: POSOrdersRemoteProtocol { // public extension OrdersRemote { enum Defaults { - public static let pageSize: Int = 25 - public static let pageNumber: Int = 1 + public static let pageSize: Int = 25 + public static let pageNumber: Int = 1 + public static let decimalPoints: String = "8" public static let statusAny: String = "any" } @@ -477,6 +486,7 @@ public extension OrdersRemote { static let customer = "customer" static let product = "product" static let createdVia = "created_via" + static let decimalPlaces = "dp" } enum ParameterValues { diff --git a/Modules/Tests/NetworkingTests/Remote/OrdersRemoteTests.swift b/Modules/Tests/NetworkingTests/Remote/OrdersRemoteTests.swift index 499e7377116..0d45ea8489b 100644 --- a/Modules/Tests/NetworkingTests/Remote/OrdersRemoteTests.swift +++ b/Modules/Tests/NetworkingTests/Remote/OrdersRemoteTests.swift @@ -309,6 +309,19 @@ final class OrdersRemoteTests: XCTestCase { wait(for: [expectation], timeout: Constants.expectationTimeout) } + func test_updateOrder_status_includes_decimal_places_parameter() throws { + // Given + let remote = OrdersRemote(network: network) + + // When + remote.updateOrder(from: sampleSiteID, orderID: sampleOrderID, statusKey: .pending) { (order, error) in } + + // Then + let request = try XCTUnwrap(network.requestsForResponseData.last as? JetpackRequest) + let received = try XCTUnwrap(request.parameters["dp"] as? String) + assertEqual(received, "8") + } + func test_update_order_properly_encodes_shipping_lines_for_removal_from_order() throws { // Given let remote = OrdersRemote(network: network) @@ -517,6 +530,20 @@ final class OrdersRemoteTests: XCTestCase { XCTAssertNil(request.parameters["meta_data"]) } + func test_updateOrder_with_fields_includes_decimal_places_parameter() throws { + // Given + let remote = OrdersRemote(network: network) + let order = Order.fake() + + // When + remote.updateOrder(from: sampleSiteID, order: order, giftCard: nil, fields: [.customerNote]) { result in } + + // Then + let request = try XCTUnwrap(network.requestsForResponseData.last as? JetpackRequest) + let received = try XCTUnwrap(request.parameters["dp"] as? String) + assertEqual(received, "8") + } + // MARK: - Load Order Notes Tests /// Verifies that loadOrderNotes properly parses the `order-notes` sample response. @@ -806,6 +833,20 @@ final class OrdersRemoteTests: XCTestCase { XCTAssertNil(request.parameters["created_via"]) } + func test_createOrder_includes_decimal_places_parameter() throws { + // Given + let remote = OrdersRemote(network: network) + let order = Order.fake() + + // When + remote.createOrder(siteID: 123, order: order, giftCard: nil, fields: []) { result in } + + // Then + let request = try XCTUnwrap(network.requestsForResponseData.last as? JetpackRequest) + let received = try XCTUnwrap(request.parameters["dp"] as? String) + assertEqual(received, "8") + } + // MARK: - Delete order tests func test_delete_order_properly_returns_parsed_order() throws { diff --git a/Modules/Tests/YosemiteTests/Stores/OrderStoreTests.swift b/Modules/Tests/YosemiteTests/Stores/OrderStoreTests.swift index 1accea52b62..768c7687b33 100644 --- a/Modules/Tests/YosemiteTests/Stores/OrderStoreTests.swift +++ b/Modules/Tests/YosemiteTests/Stores/OrderStoreTests.swift @@ -1269,6 +1269,7 @@ final class OrderStoreTests: XCTestCase { "currency", "customer_id", "customer_note", + "dp", "fee_lines", "line_items", "meta_data", @@ -1297,6 +1298,7 @@ final class OrderStoreTests: XCTestCase { "currency", "customer_id", "customer_note", + "dp", "fee_lines", "gift_cards", "line_items", @@ -1359,7 +1361,8 @@ final class OrderStoreTests: XCTestCase { let expectedKeys = [ "gift_cards" ] - assertEqual(expectedKeys, receivedKeys) + // Updating an order will also contain `dp` (decimal point), but in this case we're only interested in `gift_cards` + XCTAssertTrue(receivedKeys.contains(expectedKeys)) } func test_update_order_with_gift_card_returns_notApplied_error_when_error_response_does_not_include_gift_card() throws { diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index 200d741221d..90593dcde3a 100644 --- a/RELEASE-NOTES.txt +++ b/RELEASE-NOTES.txt @@ -3,6 +3,7 @@ 23.0 ----- +- [*] Increased decimal sensitivity in order creation to mitigate tax rounding issues [https://github.com/woocommerce/woocommerce-ios/pull/15957] - [*] Fix initialization of authenticator to avoid crashes during login [https://github.com/woocommerce/woocommerce-ios/pull/15953] - [*] Shipping Labels: Made HS tariff number field required in customs form for EU destinations [https://github.com/woocommerce/woocommerce-ios/pull/15946] diff --git a/WooCommerce/Classes/ViewRelated/Orders/Order Creation/EditableOrderViewModel.swift b/WooCommerce/Classes/ViewRelated/Orders/Order Creation/EditableOrderViewModel.swift index 2ed8f3618ec..d94a43626e0 100644 --- a/WooCommerce/Classes/ViewRelated/Orders/Order Creation/EditableOrderViewModel.swift +++ b/WooCommerce/Classes/ViewRelated/Orders/Order Creation/EditableOrderViewModel.swift @@ -2603,7 +2603,7 @@ private extension EditableOrderViewModel { } extension TaxBasedOnSetting { - var displayString: String { + var displayTaxCalculationHint: String { switch self { case .customerBillingAddress: return NSLocalizedString("editableOrderViewModel.taxBasedOnSetting.customerBillingAddress", diff --git a/WooCommerce/Classes/ViewRelated/Orders/Order Creation/PaymentSection/OrderPaymentSection.swift b/WooCommerce/Classes/ViewRelated/Orders/Order Creation/PaymentSection/OrderPaymentSection.swift index e9b9e9cf2fe..dc87cadeccc 100644 --- a/WooCommerce/Classes/ViewRelated/Orders/Order Creation/PaymentSection/OrderPaymentSection.swift +++ b/WooCommerce/Classes/ViewRelated/Orders/Order Creation/PaymentSection/OrderPaymentSection.swift @@ -234,7 +234,7 @@ private extension OrderPaymentSection { @ViewBuilder var taxBasedOnLine: some View { HStack(spacing: Constants.taxBasedOnLineTextPadding) { - Text(viewModel.taxBasedOnSetting?.displayString ?? "") + Text(viewModel.taxBasedOnSetting?.displayTaxCalculationHint ?? "") .footnoteStyle() .multilineTextAlignment(.leading) Text(Localization.taxInformationLearnMore) diff --git a/WooCommerce/WooCommerceTests/ViewRelated/Orders/Order Creation/EditableOrderViewModelTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/Orders/Order Creation/EditableOrderViewModelTests.swift index 56a1eb58850..a29f2fed390 100644 --- a/WooCommerce/WooCommerceTests/ViewRelated/Orders/Order Creation/EditableOrderViewModelTests.swift +++ b/WooCommerce/WooCommerceTests/ViewRelated/Orders/Order Creation/EditableOrderViewModelTests.swift @@ -2379,6 +2379,10 @@ final class EditableOrderViewModelTests: XCTestCase { } func test_order_created_when_tax_based_on_is_customer_billing_address_then_property_is_updated() { + // Given + let expectedString = NSLocalizedString("Calculated on billing address.", comment: "") + + // When stores.whenReceivingAction(ofType: SettingAction.self, thenCall: { action in switch action { case .retrieveTaxBasedOnSetting(_, let onCompletion): @@ -2391,10 +2395,15 @@ final class EditableOrderViewModelTests: XCTestCase { let viewModel = EditableOrderViewModel(siteID: sampleSiteID, stores: stores) - XCTAssertEqual(viewModel.paymentDataViewModel.taxBasedOnSetting?.displayString, NSLocalizedString("Calculated on billing address.", comment: "")) + // Then + XCTAssertEqual(viewModel.paymentDataViewModel.taxBasedOnSetting?.displayTaxCalculationHint, expectedString) } func test_order_created_when_tax_based_on_is_shop_base_address_then_property_is_updated() { + // Given + let expectedString = NSLocalizedString("Calculated on shop base address.", comment: "") + + // When stores.whenReceivingAction(ofType: SettingAction.self, thenCall: { action in switch action { case .retrieveTaxBasedOnSetting(_, let onCompletion): @@ -2407,10 +2416,15 @@ final class EditableOrderViewModelTests: XCTestCase { let viewModel = EditableOrderViewModel(siteID: sampleSiteID, stores: stores) - XCTAssertEqual(viewModel.paymentDataViewModel.taxBasedOnSetting?.displayString, NSLocalizedString("Calculated on shop base address.", comment: "")) + // Then + XCTAssertEqual(viewModel.paymentDataViewModel.taxBasedOnSetting?.displayTaxCalculationHint, expectedString) } func test_order_created_when_tax_based_on_is_customer_shipping_address_then_property_is_updated() { + // Given + let expectedString = NSLocalizedString("Calculated on shipping address.", comment: "") + + // When stores.whenReceivingAction(ofType: SettingAction.self, thenCall: { action in switch action { case .retrieveTaxBasedOnSetting(_, let onCompletion): @@ -2423,7 +2437,8 @@ final class EditableOrderViewModelTests: XCTestCase { let viewModel = EditableOrderViewModel(siteID: sampleSiteID, stores: stores) - XCTAssertEqual(viewModel.paymentDataViewModel.taxBasedOnSetting?.displayString, NSLocalizedString("Calculated on shipping address.", comment: "")) + // Then + XCTAssertEqual(viewModel.paymentDataViewModel.taxBasedOnSetting?.displayTaxCalculationHint, expectedString) } func test_payment_data_view_model_when_calling_onDismissWpAdminWebViewClosure_then_calls_to_update_elements() {