Skip to content

Commit bdbc520

Browse files
authored
Increase decimal points in order POST-PUT requests to mitigate tax rounding issues (#15957)
2 parents 5865702 + 1c5234a commit bdbc520

File tree

7 files changed

+79
-9
lines changed

7 files changed

+79
-9
lines changed

Modules/Sources/NetworkingCore/Remote/OrdersRemote.swift

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,8 @@ public class OrdersRemote: Remote {
228228
params[Order.CodingKeys.createdVia.rawValue] = createdViaValue
229229
}
230230

231+
params[ParameterKeys.decimalPlaces] = OrdersRemote.Defaults.decimalPoints
232+
231233
return params
232234
}()
233235

@@ -253,7 +255,10 @@ public class OrdersRemote: Remote {
253255
///
254256
public func updateOrder(from siteID: Int64, orderID: Int64, statusKey: OrderStatusEnum, completion: @escaping (Order?, Error?) -> Void) {
255257
let path = "\(Constants.ordersPath)/" + String(orderID)
256-
let parameters = [ParameterKeys.statusKey: statusKey.rawValue]
258+
let parameters = [
259+
ParameterKeys.statusKey: statusKey.rawValue,
260+
ParameterKeys.decimalPlaces: OrdersRemote.Defaults.decimalPoints
261+
]
257262
let mapper = OrderMapper(siteID: siteID)
258263

259264
let request = JetpackRequest(wooApiVersion: .mark3,
@@ -328,6 +333,9 @@ public class OrdersRemote: Remote {
328333
value: cashPaymentChangeDueAmount).toDictionary()]
329334
}
330335

336+
// Add decimal places parameter for better precision
337+
params[ParameterKeys.decimalPlaces] = OrdersRemote.Defaults.decimalPoints
338+
331339
return params
332340
}()
333341

@@ -449,8 +457,9 @@ extension OrdersRemote: POSOrdersRemoteProtocol {
449457
//
450458
public extension OrdersRemote {
451459
enum Defaults {
452-
public static let pageSize: Int = 25
453-
public static let pageNumber: Int = 1
460+
public static let pageSize: Int = 25
461+
public static let pageNumber: Int = 1
462+
public static let decimalPoints: String = "8"
454463
public static let statusAny: String = "any"
455464
}
456465

@@ -477,6 +486,7 @@ public extension OrdersRemote {
477486
static let customer = "customer"
478487
static let product = "product"
479488
static let createdVia = "created_via"
489+
static let decimalPlaces = "dp"
480490
}
481491

482492
enum ParameterValues {

Modules/Tests/NetworkingTests/Remote/OrdersRemoteTests.swift

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,19 @@ final class OrdersRemoteTests: XCTestCase {
309309
wait(for: [expectation], timeout: Constants.expectationTimeout)
310310
}
311311

312+
func test_updateOrder_status_includes_decimal_places_parameter() throws {
313+
// Given
314+
let remote = OrdersRemote(network: network)
315+
316+
// When
317+
remote.updateOrder(from: sampleSiteID, orderID: sampleOrderID, statusKey: .pending) { (order, error) in }
318+
319+
// Then
320+
let request = try XCTUnwrap(network.requestsForResponseData.last as? JetpackRequest)
321+
let received = try XCTUnwrap(request.parameters["dp"] as? String)
322+
assertEqual(received, "8")
323+
}
324+
312325
func test_update_order_properly_encodes_shipping_lines_for_removal_from_order() throws {
313326
// Given
314327
let remote = OrdersRemote(network: network)
@@ -517,6 +530,20 @@ final class OrdersRemoteTests: XCTestCase {
517530
XCTAssertNil(request.parameters["meta_data"])
518531
}
519532

533+
func test_updateOrder_with_fields_includes_decimal_places_parameter() throws {
534+
// Given
535+
let remote = OrdersRemote(network: network)
536+
let order = Order.fake()
537+
538+
// When
539+
remote.updateOrder(from: sampleSiteID, order: order, giftCard: nil, fields: [.customerNote]) { result in }
540+
541+
// Then
542+
let request = try XCTUnwrap(network.requestsForResponseData.last as? JetpackRequest)
543+
let received = try XCTUnwrap(request.parameters["dp"] as? String)
544+
assertEqual(received, "8")
545+
}
546+
520547
// MARK: - Load Order Notes Tests
521548

522549
/// Verifies that loadOrderNotes properly parses the `order-notes` sample response.
@@ -806,6 +833,20 @@ final class OrdersRemoteTests: XCTestCase {
806833
XCTAssertNil(request.parameters["created_via"])
807834
}
808835

836+
func test_createOrder_includes_decimal_places_parameter() throws {
837+
// Given
838+
let remote = OrdersRemote(network: network)
839+
let order = Order.fake()
840+
841+
// When
842+
remote.createOrder(siteID: 123, order: order, giftCard: nil, fields: []) { result in }
843+
844+
// Then
845+
let request = try XCTUnwrap(network.requestsForResponseData.last as? JetpackRequest)
846+
let received = try XCTUnwrap(request.parameters["dp"] as? String)
847+
assertEqual(received, "8")
848+
}
849+
809850
// MARK: - Delete order tests
810851

811852
func test_delete_order_properly_returns_parsed_order() throws {

Modules/Tests/YosemiteTests/Stores/OrderStoreTests.swift

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1269,6 +1269,7 @@ final class OrderStoreTests: XCTestCase {
12691269
"currency",
12701270
"customer_id",
12711271
"customer_note",
1272+
"dp",
12721273
"fee_lines",
12731274
"line_items",
12741275
"meta_data",
@@ -1297,6 +1298,7 @@ final class OrderStoreTests: XCTestCase {
12971298
"currency",
12981299
"customer_id",
12991300
"customer_note",
1301+
"dp",
13001302
"fee_lines",
13011303
"gift_cards",
13021304
"line_items",
@@ -1359,7 +1361,8 @@ final class OrderStoreTests: XCTestCase {
13591361
let expectedKeys = [
13601362
"gift_cards"
13611363
]
1362-
assertEqual(expectedKeys, receivedKeys)
1364+
// Updating an order will also contain `dp` (decimal point), but in this case we're only interested in `gift_cards`
1365+
XCTAssertTrue(receivedKeys.contains(expectedKeys))
13631366
}
13641367

13651368
func test_update_order_with_gift_card_returns_notApplied_error_when_error_response_does_not_include_gift_card() throws {

RELEASE-NOTES.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
23.0
55
-----
6+
- [*] Increased decimal sensitivity in order creation to mitigate tax rounding issues [https://github.com/woocommerce/woocommerce-ios/pull/15957]
67
- [*] Fix initialization of authenticator to avoid crashes during login [https://github.com/woocommerce/woocommerce-ios/pull/15953]
78
- [*] Shipping Labels: Made HS tariff number field required in customs form for EU destinations [https://github.com/woocommerce/woocommerce-ios/pull/15946]
89

WooCommerce/Classes/ViewRelated/Orders/Order Creation/EditableOrderViewModel.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2603,7 +2603,7 @@ private extension EditableOrderViewModel {
26032603
}
26042604

26052605
extension TaxBasedOnSetting {
2606-
var displayString: String {
2606+
var displayTaxCalculationHint: String {
26072607
switch self {
26082608
case .customerBillingAddress:
26092609
return NSLocalizedString("editableOrderViewModel.taxBasedOnSetting.customerBillingAddress",

WooCommerce/Classes/ViewRelated/Orders/Order Creation/PaymentSection/OrderPaymentSection.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,7 @@ private extension OrderPaymentSection {
234234

235235
@ViewBuilder var taxBasedOnLine: some View {
236236
HStack(spacing: Constants.taxBasedOnLineTextPadding) {
237-
Text(viewModel.taxBasedOnSetting?.displayString ?? "")
237+
Text(viewModel.taxBasedOnSetting?.displayTaxCalculationHint ?? "")
238238
.footnoteStyle()
239239
.multilineTextAlignment(.leading)
240240
Text(Localization.taxInformationLearnMore)

WooCommerce/WooCommerceTests/ViewRelated/Orders/Order Creation/EditableOrderViewModelTests.swift

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2379,6 +2379,10 @@ final class EditableOrderViewModelTests: XCTestCase {
23792379
}
23802380

23812381
func test_order_created_when_tax_based_on_is_customer_billing_address_then_property_is_updated() {
2382+
// Given
2383+
let expectedString = NSLocalizedString("Calculated on billing address.", comment: "")
2384+
2385+
// When
23822386
stores.whenReceivingAction(ofType: SettingAction.self, thenCall: { action in
23832387
switch action {
23842388
case .retrieveTaxBasedOnSetting(_, let onCompletion):
@@ -2391,10 +2395,15 @@ final class EditableOrderViewModelTests: XCTestCase {
23912395
let viewModel = EditableOrderViewModel(siteID: sampleSiteID,
23922396
stores: stores)
23932397

2394-
XCTAssertEqual(viewModel.paymentDataViewModel.taxBasedOnSetting?.displayString, NSLocalizedString("Calculated on billing address.", comment: ""))
2398+
// Then
2399+
XCTAssertEqual(viewModel.paymentDataViewModel.taxBasedOnSetting?.displayTaxCalculationHint, expectedString)
23952400
}
23962401

23972402
func test_order_created_when_tax_based_on_is_shop_base_address_then_property_is_updated() {
2403+
// Given
2404+
let expectedString = NSLocalizedString("Calculated on shop base address.", comment: "")
2405+
2406+
// When
23982407
stores.whenReceivingAction(ofType: SettingAction.self, thenCall: { action in
23992408
switch action {
24002409
case .retrieveTaxBasedOnSetting(_, let onCompletion):
@@ -2407,10 +2416,15 @@ final class EditableOrderViewModelTests: XCTestCase {
24072416
let viewModel = EditableOrderViewModel(siteID: sampleSiteID,
24082417
stores: stores)
24092418

2410-
XCTAssertEqual(viewModel.paymentDataViewModel.taxBasedOnSetting?.displayString, NSLocalizedString("Calculated on shop base address.", comment: ""))
2419+
// Then
2420+
XCTAssertEqual(viewModel.paymentDataViewModel.taxBasedOnSetting?.displayTaxCalculationHint, expectedString)
24112421
}
24122422

24132423
func test_order_created_when_tax_based_on_is_customer_shipping_address_then_property_is_updated() {
2424+
// Given
2425+
let expectedString = NSLocalizedString("Calculated on shipping address.", comment: "")
2426+
2427+
// When
24142428
stores.whenReceivingAction(ofType: SettingAction.self, thenCall: { action in
24152429
switch action {
24162430
case .retrieveTaxBasedOnSetting(_, let onCompletion):
@@ -2423,7 +2437,8 @@ final class EditableOrderViewModelTests: XCTestCase {
24232437
let viewModel = EditableOrderViewModel(siteID: sampleSiteID,
24242438
stores: stores)
24252439

2426-
XCTAssertEqual(viewModel.paymentDataViewModel.taxBasedOnSetting?.displayString, NSLocalizedString("Calculated on shipping address.", comment: ""))
2440+
// Then
2441+
XCTAssertEqual(viewModel.paymentDataViewModel.taxBasedOnSetting?.displayTaxCalculationHint, expectedString)
24272442
}
24282443

24292444
func test_payment_data_view_model_when_calling_onDismissWpAdminWebViewClosure_then_calls_to_update_elements() {

0 commit comments

Comments
 (0)