Shipping Labels: Add check for dimensions when confirming custom package (#15925)

Author: itsmeichigo

RELEASE-NOTES.txt

@@ -10,6 +10,7 @@
 - [*] POS: a POS tab in the tab bar is now available in the app for stores in countries eligible for Point of Sale, instead of the tab is only shown when the store is eligible for POS. [https://github.com/woocommerce/woocommerce-ios/pull/15918]
 - [*] Shipping Labels: Display base rate on selected shipping service cards [https://github.com/woocommerce/woocommerce-ios/pull/15916]
 - [*] Shipping Labels: Update mark order completed toggle on purchase form [https://github.com/woocommerce/woocommerce-ios/pull/15917]
+- [*] Shipping Labels: Validate custom package dimensions [https://github.com/woocommerce/woocommerce-ios/pull/15925]
 - [*] Shipping Labels: Optimize data loading on purchase form [https://github.com/woocommerce/woocommerce-ios/pull/15919]
 - [internal] Optimized assets for app size reduction [https://github.com/woocommerce/woocommerce-ios/pull/15881]
 

WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/WooShipping Create Shipping Labels/WooShipping Package and Rate Selection/WooAddCustomPackageView.swift

@@ -14,6 +14,7 @@ struct WooAddCustomPackageView: View {
 
     @State private var isSavingPackage = false
     @State private var showingSavingError = false
+    @State private var foundInvalidDimensions = false
 
     @Environment(\.shippingDimensionsUnit) private var dimensionsUnit
     @Environment(\.shippingWeightUnit) private var weightUnit
@@ -68,6 +69,14 @@ struct WooAddCustomPackageView: View {
                                     unitInputView(for: dimensionUnit, unit: dimensionsUnit)
                                 }
                             }
+
+                            if foundInvalidDimensions {
+                                Text(Localization.invalidDimensions)
+                                    .font(.footnote)
+                                    .foregroundColor(Color(.error))
+                                    .frame(maxWidth: .infinity, alignment: .leading)
+                            }
+
                             // showing weight input only if we are saving the template
                             if viewModel.showSaveTemplate {
                                 unitInputView(for: WooShippingPackageUnitType.weight, unit: weightUnit)
@@ -172,19 +181,21 @@ struct WooAddCustomPackageView: View {
             }
         }
     }
+}
 
-    private var selectionButtonDisabled: Bool {
+private extension WooAddCustomPackageView {
+    var selectionButtonDisabled: Bool {
         !viewModel.validateCustomPackageInputFields()
     }
 
-    private var selectionButtonText: String {
+    var selectionButtonText: String {
         if selectionButtonDisabled {
             return WooShippingAddPackageView.Localization.addPackageDetails
         }
         return WooShippingAddPackageView.Localization.addPackage
     }
 
-    private func unitInputView(for unitType: WooShippingPackageUnitType, unit: String) -> some View {
+    func unitInputView(for unitType: WooShippingPackageUnitType, unit: String) -> some View {
         WooShippingAddPackageUnitInputView(unitType: unitType,
                                            unit: unit,
                                            fieldValue: Binding(get: {
@@ -196,15 +207,20 @@ struct WooAddCustomPackageView: View {
 
     // MARK: - actions
 
-    private func confirmPackage() {
-        guard let packageData = viewModel.packageData else {
+    func confirmPackage() {
+        foundInvalidDimensions = !viewModel.allDimensionsValid
+        guard !foundInvalidDimensions, let packageData = viewModel.packageData else {
             return
         }
         addPackageAction(packageData)
     }
 
     @MainActor
-    private func savePackageAsTemplateButtonTapped() async {
+    func savePackageAsTemplateButtonTapped() async {
+        foundInvalidDimensions = !viewModel.allDimensionsValid
+        guard !foundInvalidDimensions else {
+            return
+        }
         let packageDataResult = await viewModel.savePackageAsTemplateAction()
         // call addPackageAction with data
         switch packageDataResult {
@@ -216,7 +232,7 @@ struct WooAddCustomPackageView: View {
         }
     }
 
-    private func onBackwardButtonTapped() {
+    func onBackwardButtonTapped() {
         switch focusedField {
         case .length:
             return
@@ -231,7 +247,7 @@ struct WooAddCustomPackageView: View {
         }
     }
 
-    private func onForwardButtonTapped() {
+    func onForwardButtonTapped() {
         switch focusedField {
         case .length:
             focusedField = .width
@@ -246,7 +262,7 @@ struct WooAddCustomPackageView: View {
         }
     }
 
-    private func dismissKeyboard() {
+    func dismissKeyboard() {
         focusedField = nil
         packageTemplateNameFieldFocused = false
     }
@@ -269,6 +285,11 @@ extension WooAddCustomPackageView {
         static let savePackageTemplatePlaceholder = NSLocalizedString("wooShipping.createLabel.addPackage.savePackageTemplatePlaceholder",
                                                            value: "Enter a unique package name",
                                                            comment: "Placeholder text for package name field")
+        static let invalidDimensions = NSLocalizedString(
+            "wooShipping.createLabel.addPackage.invalidDimensions",
+            value: "Package dimensions should all be larger than 0",
+            comment: "Message when user attempts to confirm a package with invalid dimension in the shipping label creation flow"
+        )
         enum SavingPackageError {
             static let title = NSLocalizedString(
                 "wooShipping.createLabel.addPackage.savingPackageError.title",

WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/WooShipping Create Shipping Labels/WooShipping Package and Rate Selection/WooShippingAddCustomPackageViewModel.swift

@@ -43,10 +43,10 @@ final class WooShippingAddCustomPackageViewModel: ObservableObject {
         }
     }
 
-    // Field values are invalid if one of them is empty
+    // Field values are invalid if one of them is incomplete
     // - if we are saving template we check all field values
     // - if we are not saving template we check only dimensions
-    var areFieldValuesInvalid: Bool {
+    var areFieldValuesIncomplete: Bool {
         let keysToCheck: [WooShippingPackageUnitType] = showSaveTemplate ? WooShippingPackageUnitType.allCases : WooShippingPackageUnitType.dimensionUnits
 
         var validFieldsCount: Int = 0
@@ -61,6 +61,19 @@ final class WooShippingAddCustomPackageViewModel: ObservableObject {
         return validFieldsCount != keysToCheck.count
     }
 
+    /// Ensure that all dimensions are larger than 0
+    var allDimensionsValid: Bool {
+        let keysToCheck = WooShippingPackageUnitType.dimensionUnits
+        for (key, value) in fieldValues {
+            guard keysToCheck.contains(key) else { continue }
+            let doubleValue = Double(value)
+            guard let doubleValue, doubleValue > 0 else {
+                return false
+            }
+        }
+        return true
+    }
+
     private var packageDataFromCurrentData: WooShippingPackageDataRepresentable {
         return WooShippingPackageData(id: packageID,
                                       name: packageTemplateName,
@@ -117,7 +130,7 @@ final class WooShippingAddCustomPackageViewModel: ObservableObject {
                     continuation.resume(returning: .success(packageData))
                 case let .failure(error):
                     DDLogError("⛔️ Error saving custom package with WCShip: \(error)")
-                    continuation.resume(returning: .failure(WooShippingAddCustomPackageViewModel.Error.failedSavingTemplate))
+                    continuation.resume(returning: .failure(WooShippingAddCustomPackageViewModel.Error.failure(error)))
                 }
             }
             stores.dispatch(action)
@@ -134,7 +147,7 @@ final class WooShippingAddCustomPackageViewModel: ObservableObject {
     }
 
     func validateCustomPackageInputFields() -> Bool {
-        guard !areFieldValuesInvalid else {
+        if areFieldValuesIncomplete {
             return false
         }
         if showSaveTemplate {

WooCommerce/WooCommerceTests/ViewRelated/Shipping Label/WooShipping Create Shipping Labels/WooShippingAddCustomPackageViewModelTests.swift

@@ -29,7 +29,7 @@ final class WooShippingAddCustomPackageViewModelTests: XCTestCase {
 
         // Then
         XCTAssertEqual(viewModel.fieldValues.isEmpty, false)
-        XCTAssertEqual(viewModel.areFieldValuesInvalid, true)
+        XCTAssertEqual(viewModel.areFieldValuesIncomplete, true)
     }
 
     @MainActor
@@ -45,7 +45,7 @@ final class WooShippingAddCustomPackageViewModelTests: XCTestCase {
 
         // Then
         XCTAssertEqual(viewModel.fieldValues.isEmpty, false)
-        XCTAssertEqual(viewModel.areFieldValuesInvalid, false)
+        XCTAssertEqual(viewModel.areFieldValuesIncomplete, false)
     }
 
     @MainActor
@@ -62,7 +62,7 @@ final class WooShippingAddCustomPackageViewModelTests: XCTestCase {
 
         // Then
         XCTAssertEqual(viewModel.fieldValues.isEmpty, false)
-        XCTAssertEqual(viewModel.areFieldValuesInvalid, false)
+        XCTAssertEqual(viewModel.areFieldValuesIncomplete, false)
     }
 
     @MainActor
@@ -79,7 +79,7 @@ final class WooShippingAddCustomPackageViewModelTests: XCTestCase {
 
         // Then
         XCTAssertEqual(viewModel.fieldValues.isEmpty, false)
-        XCTAssertEqual(viewModel.areFieldValuesInvalid, true)
+        XCTAssertEqual(viewModel.areFieldValuesIncomplete, true)
     }
 
     @MainActor
@@ -96,7 +96,7 @@ final class WooShippingAddCustomPackageViewModelTests: XCTestCase {
         viewModel.fieldValues[.weight] = "1"
         // Then
         XCTAssertEqual(viewModel.fieldValues.isEmpty, false)
-        XCTAssertEqual(viewModel.areFieldValuesInvalid, false)
+        XCTAssertEqual(viewModel.areFieldValuesIncomplete, false)
     }
 
     @MainActor
@@ -278,6 +278,98 @@ final class WooShippingAddCustomPackageViewModelTests: XCTestCase {
         XCTAssertEqual(viewModel.fieldValues[.height], "1.27")
         XCTAssertEqual(viewModel.packageType, .envelope)
     }
+
+    // MARK: - allDimensionsValid tests
+
+    func test_allDimensionsValid_returns_false_when_dimension_is_zero() {
+        // Given
+        let siteID: Int64 = 1234
+        let mockStores = MockStoresManager(sessionManager: .testingInstance)
+        let viewModel = WooShippingAddCustomPackageViewModel(siteID: siteID, stores: mockStores)
+
+        // When
+        viewModel.fieldValues[.length] = "10"
+        viewModel.fieldValues[.width] = "0"
+        viewModel.fieldValues[.height] = "5"
+
+        // Then
+        XCTAssertFalse(viewModel.allDimensionsValid)
+    }
+
+    func test_allDimensionsValid_returns_false_when_dimension_is_empty_string() {
+        // Given
+        let siteID: Int64 = 1234
+        let mockStores = MockStoresManager(sessionManager: .testingInstance)
+        let viewModel = WooShippingAddCustomPackageViewModel(siteID: siteID, stores: mockStores)
+
+        // When
+        viewModel.fieldValues[.length] = "10"
+        viewModel.fieldValues[.width] = ""
+        viewModel.fieldValues[.height] = "5"
+
+        // Then
+        XCTAssertFalse(viewModel.allDimensionsValid)
+    }
+
+    func test_allDimensionsValid_returns_false_when_dimension_is_invalid_string() {
+        // Given
+        let siteID: Int64 = 1234
+        let mockStores = MockStoresManager(sessionManager: .testingInstance)
+        let viewModel = WooShippingAddCustomPackageViewModel(siteID: siteID, stores: mockStores)
+
+        // When
+        viewModel.fieldValues[.length] = "10"
+        viewModel.fieldValues[.width] = "abc"
+        viewModel.fieldValues[.height] = "5"
+
+        // Then
+        XCTAssertFalse(viewModel.allDimensionsValid)
+    }
+
+    func test_allDimensionsValid_returns_false_when_dimension_is_negative() {
+        // Given
+        let siteID: Int64 = 1234
+        let mockStores = MockStoresManager(sessionManager: .testingInstance)
+        let viewModel = WooShippingAddCustomPackageViewModel(siteID: siteID, stores: mockStores)
+
+        // When
+        viewModel.fieldValues[.length] = "10"
+        viewModel.fieldValues[.width] = "-5"
+        viewModel.fieldValues[.height] = "5"
+
+        // Then
+        XCTAssertFalse(viewModel.allDimensionsValid)
+    }
+
+    func test_allDimensionsValid_returns_true_when_all_dimensions_are_valid_positive_numbers() {
+        // Given
+        let siteID: Int64 = 1234
+        let mockStores = MockStoresManager(sessionManager: .testingInstance)
+        let viewModel = WooShippingAddCustomPackageViewModel(siteID: siteID, stores: mockStores)
+
+        // When
+        viewModel.fieldValues[.length] = "10"
+        viewModel.fieldValues[.width] = "5"
+        viewModel.fieldValues[.height] = "3"
+
+        // Then
+        XCTAssertTrue(viewModel.allDimensionsValid)
+    }
+
+    func test_allDimensionsValid_returns_true_when_all_dimensions_are_valid_decimals() {
+        // Given
+        let siteID: Int64 = 1234
+        let mockStores = MockStoresManager(sessionManager: .testingInstance)
+        let viewModel = WooShippingAddCustomPackageViewModel(siteID: siteID, stores: mockStores)
+
+        // When
+        viewModel.fieldValues[.length] = "10.5"
+        viewModel.fieldValues[.width] = "5.25"
+        viewModel.fieldValues[.height] = "3.75"
+
+        // Then
+        XCTAssertTrue(viewModel.allDimensionsValid)
+    }
 }
 
 extension WooShippingAddCustomPackageViewModel {
@@ -298,6 +390,6 @@ extension WooShippingAddCustomPackageViewModel {
         XCTAssertEqual(packageType, WooShippingPackageType.box)
         XCTAssertEqual(showSaveTemplate, false)
         XCTAssertEqual(packageTemplateName, "")
-        XCTAssertEqual(areFieldValuesInvalid, true)
+        XCTAssertEqual(areFieldValuesIncomplete, true)
     }
 }