From b945188aa7c018293d9ae6540d77bcce095fa8dd Mon Sep 17 00:00:00 2001 From: Huong Do Date: Wed, 25 Jun 2025 10:56:58 +0700 Subject: [PATCH 1/2] Fix incorrect height for predefined packages when unavailable --- .../WooShippingPredefinedPackage.swift | 4 +- .../WooSavedPackagesSelectionView.swift | 3 ++ ...ippingAddCustomPackageViewModelTests.swift | 37 +++++++++++++++++++ 3 files changed, 42 insertions(+), 2 deletions(-) diff --git a/Networking/Sources/Extended/Model/ShippingLabel/Packages/PredefinedPackage/WooShippingPredefinedPackage.swift b/Networking/Sources/Extended/Model/ShippingLabel/Packages/PredefinedPackage/WooShippingPredefinedPackage.swift index 5d6961dcc9b..b8ad58f5397 100644 --- a/Networking/Sources/Extended/Model/ShippingLabel/Packages/PredefinedPackage/WooShippingPredefinedPackage.swift +++ b/Networking/Sources/Extended/Model/ShippingLabel/Packages/PredefinedPackage/WooShippingPredefinedPackage.swift @@ -38,7 +38,7 @@ public struct WooShippingPredefinedPackage: Equatable, GeneratedFakeable, Identi } public func getLength() -> Double { - let firstComponent = dimensions.components(separatedBy: " x ").first ?? "" + let firstComponent = dimensions.components(separatedBy: " x ")[safe: 0] ?? "" return Double(firstComponent) ?? 0 } @@ -48,7 +48,7 @@ public struct WooShippingPredefinedPackage: Equatable, GeneratedFakeable, Identi } public func getHeight() -> Double { - let lastComponent = dimensions.components(separatedBy: " x ").last ?? "" + let lastComponent = dimensions.components(separatedBy: " x ")[safe: 2] ?? "" return Double(lastComponent) ?? 0 } } diff --git a/WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/WooShipping Create Shipping Labels/WooShipping Package and Rate Selection/WooSavedPackagesSelectionView.swift b/WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/WooShipping Create Shipping Labels/WooShipping Package and Rate Selection/WooSavedPackagesSelectionView.swift index 2dd8c91efcb..70076ed16e2 100644 --- a/WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/WooShipping Create Shipping Labels/WooShipping Package and Rate Selection/WooSavedPackagesSelectionView.swift +++ b/WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/WooShipping Create Shipping Labels/WooShipping Package and Rate Selection/WooSavedPackagesSelectionView.swift @@ -92,6 +92,9 @@ struct WooShippingPackageData: WooShippingPackageDataRepresentable { extension WooShippingPackageDataRepresentable { func dimensionsDescription(unit: String) -> String { + guard height.isNotEmpty, let numericHeight = Int(height), numericHeight > 0 else { + return "\(length) x \(width) \( unit)" + } return "\(length) x \(width) x \(height) \( unit)" } diff --git a/WooCommerce/WooCommerceTests/ViewRelated/Shipping Label/WooShipping Create Shipping Labels/WooShippingAddCustomPackageViewModelTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/Shipping Label/WooShipping Create Shipping Labels/WooShippingAddCustomPackageViewModelTests.swift index 4a749f5f999..c855584dcaf 100644 --- a/WooCommerce/WooCommerceTests/ViewRelated/Shipping Label/WooShipping Create Shipping Labels/WooShippingAddCustomPackageViewModelTests.swift +++ b/WooCommerce/WooCommerceTests/ViewRelated/Shipping Label/WooShipping Create Shipping Labels/WooShippingAddCustomPackageViewModelTests.swift @@ -187,6 +187,43 @@ final class WooShippingAddCustomPackageViewModelTests: XCTestCase { XCTAssertEqual(updatedPackageData.id, "a") } + func test_packageData_does_not_show_height_if_height_is_unavailable() throws { + // Given + let dimensionUnit = "cm" + let weightUnit = "kg" + let siteID: Int64 = 1234 + let mockStores = MockStoresManager(sessionManager: .testingInstance) + let viewModel = WooShippingAddCustomPackageViewModel(siteID: siteID, + stores: mockStores) + let length = "1" + let width = "2" + let height = "0" + let weight = "4" + + let expectedDimensions = "\(length) x \(width) \(dimensionUnit)" + let expectedWeight = "\(weight) \(weightUnit)" + + // When + viewModel.fieldValues[.length] = length + viewModel.fieldValues[.width] = width + viewModel.fieldValues[.height] = height + viewModel.fieldValues[.weight] = weight + + // Then + let packageData = try XCTUnwrap(viewModel.packageData) + XCTAssertEqual(packageData.dimensionsDescription(unit: dimensionUnit), expectedDimensions) + XCTAssertEqual(packageData.weightDescription(unit: weightUnit), expectedWeight) + XCTAssertEqual(packageData.id, "custom_box") + + // When: selecting a template + viewModel.showSaveTemplate = true + viewModel.packageTemplateName = "a" + + // Then + let updatedPackageData = try XCTUnwrap(viewModel.packageData) + XCTAssertEqual(updatedPackageData.id, "a") + } + @MainActor func test_save_package_as_template_action() async { // Given From ad889737b003b6e24ecfaf08a3f1ca8f71a9ba9d Mon Sep 17 00:00:00 2001 From: Huong Do Date: Thu, 26 Jun 2025 15:08:31 +0700 Subject: [PATCH 2/2] Set 0.25 as default height when height is 0 --- .../NetworkingTests/Remote/WooShippingRemoteTests.swift | 6 +++++- .../CarriersAndRates/ShippingLabelPackageSelected.swift | 5 ++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/Networking/NetworkingTests/Remote/WooShippingRemoteTests.swift b/Networking/NetworkingTests/Remote/WooShippingRemoteTests.swift index 700e202912d..788046c8c9a 100644 --- a/Networking/NetworkingTests/Remote/WooShippingRemoteTests.swift +++ b/Networking/NetworkingTests/Remote/WooShippingRemoteTests.swift @@ -132,6 +132,7 @@ final class WooShippingRemoteTests: XCTestCase { let remote = WooShippingRemote(network: network) network.simulateResponse(requestUrlSuffix: "label/rate", filename: "wooshipping-get-label-rates-success") let expectedDefaultRate = sampleLabelRate() + let expectedPackage = ShippingLabelPackageSelected.fake().copy(length: 12, width: 20, height: 0) // When let result: Result<[ShippingLabelCarriersAndRates], Error> = waitFor { promise in @@ -139,7 +140,7 @@ final class WooShippingRemoteTests: XCTestCase { orderID: self.sampleOrderID, originAddress: WooShippingAddress.fake(), destinationAddress: WooShippingAddress.fake(), - packages: [ShippingLabelPackageSelected.fake()]) { (result) in + packages: [expectedPackage]) { (result) in promise(result) } } @@ -148,6 +149,9 @@ final class WooShippingRemoteTests: XCTestCase { let request = try XCTUnwrap(network.requestsForResponseData.last as? JetpackRequest) let featuresParam = try XCTUnwrap(request.parameters["features_supported_by_client"] as? [String]) XCTAssertEqual(featuresParam, ["upsdap"]) + let packagesParam = try XCTUnwrap(request.parameters["packages"] as? [[String: Any]]) + let package = try XCTUnwrap(packagesParam.first) + XCTAssertEqual(package["height"] as? Double, 0.25) let successResponse = try XCTUnwrap(result.get()) XCTAssertEqual(successResponse.first?.defaultRates.first, expectedDefaultRate) diff --git a/Networking/Sources/Extended/Model/ShippingLabel/Packages/CarriersAndRates/ShippingLabelPackageSelected.swift b/Networking/Sources/Extended/Model/ShippingLabel/Packages/CarriersAndRates/ShippingLabelPackageSelected.swift index 5a40bdc9a52..969fbf2f4f5 100644 --- a/Networking/Sources/Extended/Model/ShippingLabel/Packages/CarriersAndRates/ShippingLabelPackageSelected.swift +++ b/Networking/Sources/Extended/Model/ShippingLabel/Packages/CarriersAndRates/ShippingLabelPackageSelected.swift @@ -46,7 +46,10 @@ extension ShippingLabelPackageSelected: Encodable { try container.encode(boxID.isEmpty ? "0" : boxID, forKey: .boxID) try container.encode(length, forKey: .length) try container.encode(width, forKey: .width) - try container.encode(height, forKey: .height) + + // workaround because 0 would cause an error for the API request + try container.encode(height > 0 ? height : 0.25, forKey: .height) + try container.encode(weight, forKey: .weight) try container.encode(isLetter, forKey: .isLetter) try container.encodeIfPresent(hazmatCategory, forKey: .hazmatCategory)