Merge pull request #4973 from woocommerce/issue/4971-shipping-label-address-validation-issues

Shipping Labels: Fix incorrect phone validation for non-US country and add instant local validation on value changes

Author: itsmeichigo

WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/Create Shipping Label Form/Shipping Address Validation/ShippingLabelAddressFormViewController.swift

@@ -142,16 +142,20 @@ private extension ShippingLabelAddressFormViewController {
     }
 
     func registerTableViewCells() {
-            tableView.registerNib(for: TitleAndTextFieldTableViewCell.self)
-            tableView.registerNib(for: BasicTableViewCell.self)
+        tableView.registerNib(for: TitleAndTextFieldTableViewCell.self)
+        tableView.registerNib(for: BasicTableViewCell.self)
     }
 
     func observeViewModel() {
-        viewModel.onChange = { [weak self] in
+        viewModel.onChange = { [weak self] focusedIndex in
             guard let self = self else { return }
             self.configureNavigationBar()
             self.updateTopBannerView()
             self.tableView.reloadData()
+            if let index = focusedIndex,
+               let cell = self.tableView.cellForRow(at: IndexPath(row: index, section: 0)) as? TitleAndTextFieldTableViewCell {
+                cell.textFieldBecomeFirstResponder()
+            }
         }
     }
 
@@ -459,8 +463,7 @@ private extension ShippingLabelAddressFormViewController {
                                                                      placeholder: placeholder,
                                                                      state: .normal,
                                                                      keyboardType: .default,
-                                                                     textFieldAlignment: .leading) { _ in
-        }
+                                                                     textFieldAlignment: .leading) { _ in }
         cell.configure(viewModel: cellViewModel)
         cell.enableTextField(viewModel.statesOfSelectedCountry.isEmpty)
     }
@@ -471,8 +474,7 @@ private extension ShippingLabelAddressFormViewController {
                                                                      placeholder: Localization.countryFieldPlaceholder,
                                                                      state: .normal,
                                                                      keyboardType: .default,
-                                                                     textFieldAlignment: .leading) { _ in
-        }
+                                                                     textFieldAlignment: .leading) { _ in }
         cell.configure(viewModel: cellViewModel)
         cell.enableTextField(false)
     }

WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/Create Shipping Label Form/Shipping Address Validation/ShippingLabelAddressFormViewModel.swift

@@ -25,15 +25,16 @@ final class ShippingLabelAddressFormViewModel {
     private let stores: StoresManager
 
     /// Closure to notify the `ViewController` when the view model properties change.
+    /// Accepts optional index for row to be focused after reloading the table view.
     ///
-    var onChange: (() -> (Void))?
+    var onChange: ((_ focusedRowIndex: Int?) -> (Void))?
 
     /// Current `ViewModel` state.
     ///
     private var state: State = State() {
         didSet {
             updateSections()
-            onChange?()
+            onChange?(nil)
         }
     }
 
@@ -106,10 +107,14 @@ final class ShippingLabelAddressFormViewModel {
         case .state:
             address = address?.copy(state: newValue)
         case .country:
-            address = address?.copy(country: newValue)
+            address = address?.copy(country: newValue, state: "")
         default:
             return
         }
+
+        updateSections()
+        let index: Int? = sections.first?.rows.firstIndex(where: { $0 == row })
+        onChange?(index)
     }
 
     func updateSections() {
@@ -181,23 +186,38 @@ extension ShippingLabelAddressFormViewModel {
         case invalidPhoneNumber
     }
 
-    /// Validates phone number for origin address.
+    /// Validates phone number for the address.
     /// This take into account whether phone is not empty,
     /// has length 10 with additional "1" area code for US.
     ///
-    /// Note: This logic may need to be updated if there is a need for validating other cases.
-    ///
     private var isPhoneNumberValid: Bool {
         guard let phone = address?.phone, phone.isNotEmpty else {
             return false
         }
+        guard address?.country == "US" else {
+            return true
+        }
         if phone.hasPrefix("1") {
             return phone.count == 11
         } else {
             return phone.count == 10
         }
     }
 
+    private var phoneValidationError: ValidationError? {
+        guard let addressToBeValidated = address else {
+            return nil
+        }
+        if phoneNumberRequired {
+            if addressToBeValidated.phone.isEmpty {
+                return .missingPhoneNumber
+            } else if !isPhoneNumberValid {
+                return .invalidPhoneNumber
+            }
+        }
+        return nil
+    }
+
     private func validateAddressLocally() -> [ValidationError] {
         var errors: [ValidationError] = []
 
@@ -220,12 +240,8 @@ extension ShippingLabelAddressFormViewModel {
             if addressToBeValidated.country.isEmpty {
                 errors.append(.country)
             }
-            if phoneNumberRequired {
-                if addressToBeValidated.phone.isEmpty {
-                    errors.append(.missingPhoneNumber)
-                } else if !isPhoneNumberValid {
-                    errors.append(.invalidPhoneNumber)
-                }
+            if let error = phoneValidationError {
+                errors.append(error)
             }
         }
 

WooCommerce/WooCommerceTests/ViewRelated/Shipping Label/Create Shipping Label/ShippingLabelAddressFormViewModelTests.swift

@@ -33,6 +33,19 @@ final class ShippingLabelAddressFormViewModelTests: XCTestCase {
         XCTAssertEqual(viewModel.address?.postcode, "94121-2303")
     }
 
+    func test_handleAddressValueChanges_empties_stateOfCountry_when_country_is_updated() {
+
+        // Given
+        let shippingAddress = MockShippingLabelAddress.sampleAddress(country: "US", state: "CA")
+        let viewModel = ShippingLabelAddressFormViewModel(siteID: 10, type: .origin, address: shippingAddress, validationError: nil, countries: [])
+
+        // When
+        viewModel.handleAddressValueChanges(row: .country, newValue: "AU")
+
+        // Then
+        XCTAssertEqual(viewModel.address?.state, "")
+    }
+
     func test_sections_are_returned_correctly_if_there_are_no_errors() {
         // Given
         let shippingAddress = ShippingLabelAddress(company: "Automattic Inc.",
@@ -223,7 +236,7 @@ final class ShippingLabelAddressFormViewModelTests: XCTestCase {
 
     func test_sections_are_returned_correctly_if_phone_is_required_and_invalid() {
         // Given
-        let shippingAddress = MockShippingLabelAddress.sampleAddress(phone: "0123", country: "VN")
+        let shippingAddress = MockShippingLabelAddress.sampleAddress(phone: "0123", country: "US")
         let stores = MockStoresManager(sessionManager: .testingInstance)
         let validationError = ShippingLabelAddressValidationError(addressError: "Error", generalError: nil)
 
@@ -264,6 +277,48 @@ final class ShippingLabelAddressFormViewModelTests: XCTestCase {
         XCTAssertEqual(viewModel.sections, [ShippingLabelAddressFormViewModel.Section(rows: expectedRows)])
     }
 
+    func test_sections_are_returned_correctly_if_phone_is_required_and_country_is_nonUS() {
+        // Given
+        let shippingAddress = MockShippingLabelAddress.sampleAddress(phone: "0123", country: "VN")
+        let stores = MockStoresManager(sessionManager: .testingInstance)
+        let validationError = ShippingLabelAddressValidationError(addressError: "Error", generalError: nil)
+
+        // When
+        stores.whenReceivingAction(ofType: ShippingLabelAction.self) { action in
+            switch action {
+            case let .validateAddress(_, _, onCompletion):
+                onCompletion(.failure(validationError))
+            default:
+                break
+            }
+        }
+
+        let viewModel = ShippingLabelAddressFormViewModel(siteID: 10,
+                                                          type: .origin,
+                                                          address: shippingAddress,
+                                                          phoneNumberRequired: true,
+                                                          stores: stores,
+                                                          validationError: nil,
+                                                          countries: [])
+        viewModel.validateAddress(onlyLocally: false) { _ in }
+
+        // Then
+        let expectedRows: [ShippingLabelAddressFormViewModel.Row] = [.name,
+                                                                     .fieldError(.name),
+                                                                     .company,
+                                                                     .phone,
+                                                                     .address,
+                                                                     .fieldError(.address),
+                                                                     .address2,
+                                                                     .city,
+                                                                     .fieldError(.city),
+                                                                     .postcode,
+                                                                     .fieldError(.postcode),
+                                                                     .state,
+                                                                     .country]
+        XCTAssertEqual(viewModel.sections, [ShippingLabelAddressFormViewModel.Section(rows: expectedRows)])
+    }
+
     func test_address_validation_returns_correct_values_if_succeeded() {
         // Given
         let shippingAddress = ShippingLabelAddress(company: "Automattic Inc.",