From 50c25852c83cc17da156fdda389cde4ee0245203 Mon Sep 17 00:00:00 2001 From: Huong Do Date: Fri, 15 Aug 2025 14:50:12 +0700 Subject: [PATCH 1/4] Send isVerified when updating destination address --- .../Networking/Remote/WooShippingRemote.swift | 10 +++----- .../Yosemite/Actions/WooShippingAction.swift | 1 + .../Yosemite/Stores/WooShippingStore.swift | 7 +++--- .../Remote/WooShippingRemoteTests.swift | 6 +++-- .../Remote/MockWooShippingRemote.swift | 1 + .../WooShippingEditAddressViewModel.swift | 24 ++++++++++++------- .../WooShippingCreateLabelsViewModel.swift | 4 ++-- ...WooShippingEditAddressViewModelTests.swift | 17 ++++++------- 8 files changed, 39 insertions(+), 31 deletions(-) diff --git a/Modules/Sources/Networking/Remote/WooShippingRemote.swift b/Modules/Sources/Networking/Remote/WooShippingRemote.swift index 6764fbd0a43..c1d27e4bab2 100644 --- a/Modules/Sources/Networking/Remote/WooShippingRemote.swift +++ b/Modules/Sources/Networking/Remote/WooShippingRemote.swift @@ -63,6 +63,7 @@ public protocol WooShippingRemoteProtocol { func updateDestinationAddress(siteID: Int64, orderID: Int64, address: WooShippingDestinationAddress, + isVerified: Bool, completion: @escaping (Result) -> Void) func loadConfig(siteID: Int64, orderID: Int64, @@ -465,17 +466,12 @@ public final class WooShippingRemote: Remote, WooShippingRemoteProtocol { public func updateDestinationAddress(siteID: Int64, orderID: Int64, address: WooShippingDestinationAddress, + isVerified: Bool, completion: @escaping (Result) -> Void) { do { let parameters: [String: Any] = [ ParameterKey.address: try address.toDictionary(), - /* - * Always gets saved as true, because this endpoint is called after the address is normalized - * and the normalized address is suggested to the user to confirm and use. - * The user may however choose to not use the normalized address, in which case the isVerified will - * still be true, as they've confirmed the address - */ - ParameterKey.isVerified: true + ParameterKey.isVerified: isVerified ] let path = Path.updateDestination(orderID: orderID) let request = JetpackRequest(wooApiVersion: .wooShipping, diff --git a/Modules/Sources/Yosemite/Actions/WooShippingAction.swift b/Modules/Sources/Yosemite/Actions/WooShippingAction.swift index 27c17ea721b..e8fb0e0a900 100644 --- a/Modules/Sources/Yosemite/Actions/WooShippingAction.swift +++ b/Modules/Sources/Yosemite/Actions/WooShippingAction.swift @@ -98,6 +98,7 @@ public enum WooShippingAction: Action { case updateDestinationAddress(siteID: Int64, orderID: Int64, address: WooShippingDestinationAddress, + isVerified: Bool, completion: (Result) -> Void) /// Loads label config for a given order diff --git a/Modules/Sources/Yosemite/Stores/WooShippingStore.swift b/Modules/Sources/Yosemite/Stores/WooShippingStore.swift index aa97ecb6b5a..bfef74b6df0 100644 --- a/Modules/Sources/Yosemite/Stores/WooShippingStore.swift +++ b/Modules/Sources/Yosemite/Stores/WooShippingStore.swift @@ -82,8 +82,8 @@ public final class WooShippingStore: Store { updateOriginAddress(siteID: siteID, address: address, isVerified: isVerified, completion: completion) case let .verifyDestinationAddress(siteID, orderID, completion): verifyDestinationAddress(siteID: siteID, orderID: orderID, completion: completion) - case let .updateDestinationAddress(siteID, orderID, address, completion): - updateDestinationAddress(siteID: siteID, orderID: orderID, address: address, completion: completion) + case let .updateDestinationAddress(siteID, orderID, address, isVerified, completion): + updateDestinationAddress(siteID: siteID, orderID: orderID, address: address, isVerified: isVerified, completion: completion) case let .loadConfig(siteID, orderID, completion): loadConfig(siteID: siteID, orderID: orderID, completion: completion) case let .syncShipments(siteID, orderID, completion): @@ -410,8 +410,9 @@ private extension WooShippingStore { func updateDestinationAddress(siteID: Int64, orderID: Int64, address: WooShippingDestinationAddress, + isVerified: Bool, completion: @escaping (Result) -> Void) { - remote.updateDestinationAddress(siteID: siteID, orderID: orderID, address: address) { [weak self] result in + remote.updateDestinationAddress(siteID: siteID, orderID: orderID, address: address, isVerified: isVerified) { [weak self] result in completion(result) guard let self, case .success = result else { return } diff --git a/Modules/Tests/NetworkingTests/Remote/WooShippingRemoteTests.swift b/Modules/Tests/NetworkingTests/Remote/WooShippingRemoteTests.swift index 73e6a09d919..0f2d1896301 100644 --- a/Modules/Tests/NetworkingTests/Remote/WooShippingRemoteTests.swift +++ b/Modules/Tests/NetworkingTests/Remote/WooShippingRemoteTests.swift @@ -793,7 +793,8 @@ final class WooShippingRemoteTests: XCTestCase { let result: Result = waitFor { promise in remote.updateDestinationAddress(siteID: self.sampleSiteID, orderID: self.sampleOrderID, - address: WooShippingDestinationAddress.fake()) { result in + address: WooShippingDestinationAddress.fake(), + isVerified: true) { result in promise(result) } } @@ -813,7 +814,8 @@ final class WooShippingRemoteTests: XCTestCase { let result: Result = waitFor { promise in remote.updateDestinationAddress(siteID: self.sampleSiteID, orderID: self.sampleOrderID, - address: WooShippingDestinationAddress.fake()) { result in + address: WooShippingDestinationAddress.fake(), + isVerified: true) { result in promise(result) } } diff --git a/Modules/Tests/YosemiteTests/Mocks/Networking/Remote/MockWooShippingRemote.swift b/Modules/Tests/YosemiteTests/Mocks/Networking/Remote/MockWooShippingRemote.swift index c1723e83bd1..4559ebab7b8 100644 --- a/Modules/Tests/YosemiteTests/Mocks/Networking/Remote/MockWooShippingRemote.swift +++ b/Modules/Tests/YosemiteTests/Mocks/Networking/Remote/MockWooShippingRemote.swift @@ -443,6 +443,7 @@ extension MockWooShippingRemote: WooShippingRemoteProtocol { func updateDestinationAddress(siteID: Int64, orderID: Int64, address: WooShippingDestinationAddress, + isVerified: Bool, completion: @escaping (Result) -> Void) { DispatchQueue.main.async { [weak self] in guard let self = self else { return } diff --git a/WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/WooShipping Create Shipping Labels/WooShippingAddresses/WooShippingEditAddressViewModel.swift b/WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/WooShipping Create Shipping Labels/WooShippingAddresses/WooShippingEditAddressViewModel.swift index 987f260840b..bb2ed851938 100644 --- a/WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/WooShipping Create Shipping Labels/WooShippingAddresses/WooShippingEditAddressViewModel.swift +++ b/WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/WooShipping Create Shipping Labels/WooShippingAddresses/WooShippingEditAddressViewModel.swift @@ -192,7 +192,8 @@ final class WooShippingEditAddressViewModel: ObservableObject, Identifiable { /// Closure called when a destination address is done being edited and the changes are confirmed. /// Returns the updated address and email address. - private(set) var onDestinationAddressEdited: ((WooShippingAddress, String?) -> Void)? + private(set) var onDestinationAddressEdited: ((_ addressUpdate: WooShippingDestinationAddressUpdate, + _ email: String?) -> Void)? init(type: AddressType, id: String, @@ -215,7 +216,7 @@ final class WooShippingEditAddressViewModel: ObservableObject, Identifiable { debounceDelayInSeconds: Double = 1, analytics: Analytics = ServiceLocator.analytics, onOriginAddressEdited: ((WooShippingOriginAddress) -> Void)? = nil, - onDestinationAddressEdited: ((WooShippingAddress, String?) -> Void)? = nil) { + onDestinationAddressEdited: ((WooShippingDestinationAddressUpdate, String?) -> Void)? = nil) { self.addressType = type self.id = id self.name = WooShippingAddressField(type: .name, value: name, required: company.isEmpty, validate: { _ in return nil }) @@ -327,7 +328,7 @@ final class WooShippingEditAddressViewModel: ObservableObject, Identifiable { originStateCode: String?, stores: StoresManager = ServiceLocator.stores, storageManager: StorageManagerType = ServiceLocator.storageManager, - onAddressEdited: ((WooShippingAddress, String?) -> Void)? = nil) { + onAddressEdited: ((WooShippingDestinationAddressUpdate, String?) -> Void)? = nil) { self.init(type: .destination(orderID: orderID), id: UUID().uuidString, name: address?.name ?? "", @@ -496,9 +497,12 @@ final class WooShippingEditAddressViewModel: ObservableObject, Identifiable { Task { @MainActor in do { - let updatedDestinationAddress = try await updateDestinationAddress(for: orderID, - with: destinationAddress) - onDestinationAddressEdited?(updatedDestinationAddress.toWooShippingAddress(), email.value) + let result = try await updateDestinationAddress( + for: orderID, + with: destinationAddress, + isVerified: !withoutVerification + ) + onDestinationAddressEdited?(result, email.value) analytics.track(event: .WooShipping.editingAddressStep( type: .destination, state: withoutVerification ? .confirmedWithoutVerification : .confirmed @@ -727,16 +731,18 @@ private extension WooShippingEditAddressViewModel { /// Updates a destination address remotely. @MainActor func updateDestinationAddress(for orderID: Int64, - with address: WooShippingDestinationAddress) async throws -> WooShippingDestinationAddress { + with address: WooShippingDestinationAddress, + isVerified: Bool) async throws -> WooShippingDestinationAddressUpdate { return try await withCheckedThrowingContinuation { continuation in isLoading = true let action = WooShippingAction.updateDestinationAddress(siteID: siteID, orderID: orderID, - address: address) { [weak self] result in + address: address, + isVerified: isVerified) { [weak self] result in guard let self else { return } switch result { case let .success(result): - continuation.resume(returning: result.address) + continuation.resume(returning: result) case .failure(let error): continuation.resume(throwing: error) } diff --git a/WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/WooShipping Create Shipping Labels/WooShippingCreateLabelsViewModel.swift b/WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/WooShipping Create Shipping Labels/WooShippingCreateLabelsViewModel.swift index 10feb24cc2e..7c7a0284ea4 100644 --- a/WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/WooShipping Create Shipping Labels/WooShippingCreateLabelsViewModel.swift +++ b/WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/WooShipping Create Shipping Labels/WooShippingCreateLabelsViewModel.swift @@ -423,9 +423,9 @@ final class WooShippingCreateLabelsViewModel: ObservableObject { guard let self else { return } - destinationAddress = editedAddress + destinationAddress = editedAddress.address.toWooShippingAddress() destinationEmail = editedEmail - destinationAddressStatus = .verified + destinationAddressStatus = editedAddress.isVerified ? .verified : .unverified addressToEdit = nil // Dismisses address edit screen }) } diff --git a/WooCommerce/WooCommerceTests/ViewRelated/Shipping Label/WooShipping Create Shipping Labels/WooShippingEditAddressViewModelTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/Shipping Label/WooShipping Create Shipping Labels/WooShippingEditAddressViewModelTests.swift index 5166e93412c..d7d15b55dc4 100644 --- a/WooCommerce/WooCommerceTests/ViewRelated/Shipping Label/WooShipping Create Shipping Labels/WooShippingEditAddressViewModelTests.swift +++ b/WooCommerce/WooCommerceTests/ViewRelated/Shipping Label/WooShipping Create Shipping Labels/WooShippingEditAddressViewModelTests.swift @@ -1042,7 +1042,7 @@ final class WooShippingEditAddressViewModelTests: XCTestCase { switch action { case let .validateAddress(_, _, completion): completion(.success(.init(normalizedAddress: suggestedAddress, originalAddress: .fake(), isTrivialNormalization: true))) - case let .updateDestinationAddress(_, _, address, completion): + case let .updateDestinationAddress(_, _, address, _, completion): promise(address) completion(.success(WooShippingDestinationAddressUpdate(address: address, isVerified: true))) default: @@ -1077,12 +1077,12 @@ final class WooShippingEditAddressViewModelTests: XCTestCase { lastName: "DOE", phone: "123-456-7890") let stores = MockStoresManager(sessionManager: .testingInstance) - let result: (WooShippingAddress, String?) = await waitForAsync { promise in + let result: (WooShippingDestinationAddressUpdate, String?) = await waitForAsync { promise in stores.whenReceivingAction(ofType: WooShippingAction.self) { action in switch action { case let .validateAddress(_, _, completion): completion(.success(.init(normalizedAddress: normalizedAddress, originalAddress: .fake(), isTrivialNormalization: true))) - case let .updateDestinationAddress(_, _, address, completion): + case let .updateDestinationAddress(_, _, address, _, completion): completion(.success(WooShippingDestinationAddressUpdate(address: address, isVerified: true))) default: XCTFail("Unexpected action received: \(action)") @@ -1096,8 +1096,8 @@ final class WooShippingEditAddressViewModelTests: XCTestCase { isVerified: false, originCountryCode: nil, originStateCode: nil, - stores: stores) { address, email in - promise((address, email)) + stores: stores) { addressUpdate, email in + promise((addressUpdate, email)) } viewModel.name.value = "JANE DOE" @@ -1106,8 +1106,9 @@ final class WooShippingEditAddressViewModelTests: XCTestCase { } // Then - XCTAssertEqual(result.0, normalizedAddress.toWooShippingAddress()) + XCTAssertEqual(result.0.address.toWooShippingAddress(), normalizedAddress.toWooShippingAddress()) XCTAssertEqual(result.1, "TEXT@EXAMPLE.COM") + XCTAssertEqual(result.0.isVerified, true) } // MARK: - Error Alert Tests @@ -1221,7 +1222,7 @@ final class WooShippingEditAddressViewModelTests: XCTestCase { switch action { case let .validateAddress(_, _, completion): completion(.success(.init(normalizedAddress: .fake(), originalAddress: .fake(), isTrivialNormalization: true))) - case let .updateDestinationAddress(_, _, _, completion): + case let .updateDestinationAddress(_, _, _, _, completion): completion(.failure(NSError(domain: "UpdateError", code: 500, userInfo: [NSLocalizedDescriptionKey: "Failed to update destination address"]))) default: XCTFail("Unexpected action received: \(action)") @@ -1367,7 +1368,7 @@ final class WooShippingEditAddressViewModelTests: XCTestCase { case let .validateAddress(_, _, completion): validationCallCount += 1 completion(.success(.init(normalizedAddress: .fake(), originalAddress: .fake(), isTrivialNormalization: true))) - case let .updateDestinationAddress(_, _, _, completion): + case let .updateDestinationAddress(_, _, _, _, completion): updateCallCount += 1 if updateCallCount == 1 { // First update fails From 28bd70272089e3a70a91090642ba247f59083957 Mon Sep 17 00:00:00 2001 From: Huong Do Date: Fri, 15 Aug 2025 14:54:04 +0700 Subject: [PATCH 2/4] Update WooShippingStoreTests --- .../Tests/YosemiteTests/Stores/WooShippingStoreTests.swift | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Modules/Tests/YosemiteTests/Stores/WooShippingStoreTests.swift b/Modules/Tests/YosemiteTests/Stores/WooShippingStoreTests.swift index 042de449617..7505e4ac2b7 100644 --- a/Modules/Tests/YosemiteTests/Stores/WooShippingStoreTests.swift +++ b/Modules/Tests/YosemiteTests/Stores/WooShippingStoreTests.swift @@ -1075,7 +1075,8 @@ final class WooShippingStoreTests: XCTestCase { let result: Result = waitFor { promise in let action = WooShippingAction.updateDestinationAddress(siteID: self.sampleSiteID, orderID: self.sampleOrderID, - address: WooShippingDestinationAddress.fake()) { result in + address: WooShippingDestinationAddress.fake(), + isVerified: true) { result in promise(result) } store.onAction(action) @@ -1098,7 +1099,8 @@ final class WooShippingStoreTests: XCTestCase { let result: Result = waitFor { promise in let action = WooShippingAction.updateDestinationAddress(siteID: self.sampleSiteID, orderID: self.sampleOrderID, - address: WooShippingDestinationAddress.fake()) { result in + address: WooShippingDestinationAddress.fake(), + isVerified: true) { result in promise(result) } store.onAction(action) From 59165d50bef4616e20af6be8480ac882ae8e5120 Mon Sep 17 00:00:00 2001 From: Huong Do Date: Fri, 15 Aug 2025 14:54:45 +0700 Subject: [PATCH 3/4] Update release notes --- RELEASE-NOTES.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index d0fa174424f..14630987a81 100644 --- a/RELEASE-NOTES.txt +++ b/RELEASE-NOTES.txt @@ -8,6 +8,7 @@ - [*] Payments: Updated the In-Person Payments `Learn More` redirection to display the correct page based on the selected payment provider [https://github.com/woocommerce/woocommerce-ios/pull/15998] - [*] Order details: Always show shipping labels section if order is eligible for label creation. [https://github.com/woocommerce/woocommerce-ios/pull/16010] - [*] Order details: Remove print buttons from shipment details [https://github.com/woocommerce/woocommerce-ios/pull/16012] +- [*] Shipping labels: Mark manually saved addresses as unverified [https://github.com/woocommerce/woocommerce-ios/pull/16013] 23.0 ----- From 260b35d822f2f781990b4c42b3fc08935827e39e Mon Sep 17 00:00:00 2001 From: Huong Do Date: Fri, 15 Aug 2025 15:07:07 +0700 Subject: [PATCH 4/4] Add test for updateDestinationAddress --- .../WooShippingEditAddressViewModel.swift | 2 +- .../WooShippingCreateLabelsViewModel.swift | 6 +-- ...WooShippingEditAddressViewModelTests.swift | 46 +++++++++++++++++++ 3 files changed, 50 insertions(+), 4 deletions(-) diff --git a/WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/WooShipping Create Shipping Labels/WooShippingAddresses/WooShippingEditAddressViewModel.swift b/WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/WooShipping Create Shipping Labels/WooShippingAddresses/WooShippingEditAddressViewModel.swift index bb2ed851938..b624a568cb3 100644 --- a/WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/WooShipping Create Shipping Labels/WooShippingAddresses/WooShippingEditAddressViewModel.swift +++ b/WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/WooShipping Create Shipping Labels/WooShippingAddresses/WooShippingEditAddressViewModel.swift @@ -192,7 +192,7 @@ final class WooShippingEditAddressViewModel: ObservableObject, Identifiable { /// Closure called when a destination address is done being edited and the changes are confirmed. /// Returns the updated address and email address. - private(set) var onDestinationAddressEdited: ((_ addressUpdate: WooShippingDestinationAddressUpdate, + private(set) var onDestinationAddressEdited: ((_ result: WooShippingDestinationAddressUpdate, _ email: String?) -> Void)? init(type: AddressType, diff --git a/WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/WooShipping Create Shipping Labels/WooShippingCreateLabelsViewModel.swift b/WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/WooShipping Create Shipping Labels/WooShippingCreateLabelsViewModel.swift index 7c7a0284ea4..2c4f19b240c 100644 --- a/WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/WooShipping Create Shipping Labels/WooShippingCreateLabelsViewModel.swift +++ b/WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/WooShipping Create Shipping Labels/WooShippingCreateLabelsViewModel.swift @@ -419,13 +419,13 @@ final class WooShippingCreateLabelsViewModel: ObservableObject { isVerified: destinationAddressStatus == .verified, originCountryCode: selectedOriginAddress?.country, originStateCode: selectedOriginAddress?.state, - onAddressEdited: { [weak self] editedAddress, editedEmail in + onAddressEdited: { [weak self] result, editedEmail in guard let self else { return } - destinationAddress = editedAddress.address.toWooShippingAddress() + destinationAddress = result.address.toWooShippingAddress() destinationEmail = editedEmail - destinationAddressStatus = editedAddress.isVerified ? .verified : .unverified + destinationAddressStatus = result.isVerified ? .verified : .unverified addressToEdit = nil // Dismisses address edit screen }) } diff --git a/WooCommerce/WooCommerceTests/ViewRelated/Shipping Label/WooShipping Create Shipping Labels/WooShippingEditAddressViewModelTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/Shipping Label/WooShipping Create Shipping Labels/WooShippingEditAddressViewModelTests.swift index d7d15b55dc4..9ba1330581d 100644 --- a/WooCommerce/WooCommerceTests/ViewRelated/Shipping Label/WooShipping Create Shipping Labels/WooShippingEditAddressViewModelTests.swift +++ b/WooCommerce/WooCommerceTests/ViewRelated/Shipping Label/WooShipping Create Shipping Labels/WooShippingEditAddressViewModelTests.swift @@ -1590,6 +1590,52 @@ final class WooShippingEditAddressViewModelTests: XCTestCase { XCTAssertFalse(viewModel.canConfirmWithoutVerification) XCTAssertEqual(viewModel.status, .missingInformation) } + + @MainActor + func test_updateConfirmedAddress_without_verification_triggers_updateDestinationAddress_with_isVerified_false() async { + // Given + let sampleOrderID: Int64 = 123 + let stores = MockStoresManager(sessionManager: .testingInstance) + let viewModel = WooShippingEditAddressViewModel(type: .destination(orderID: sampleOrderID), + id: "", + name: "JANE DOE", + company: "HEADQUARTERS", + country: "US", + address: "15 ALGONKIN ST", + city: "TICONDEROGA", + state: "NY", + postalCode: "12883-1487", + email: "test@example.com", + phone: "123-456-7890", + isDefaultAddress: false, + showCompanyField: true, + isVerified: false, + stores: stores) + + let testAddress = WooShippingAddress(company: "TEST COMPANY", + name: "TEST NAME", + phone: "555-123-4567", + country: "US", + state: "CA", + address1: "123 TEST ST", + address2: "", + city: "TEST CITY", + postcode: "90210") + + // When + let receivedIsVerified: Bool = await waitForAsync { promise in + stores.whenReceivingAction(ofType: WooShippingAction.self) { action in + if case let .updateDestinationAddress(_, _, _, isVerified, completion) = action { + promise(isVerified) + completion(.success(WooShippingDestinationAddressUpdate(address: .fake(), isVerified: isVerified))) + } + } + viewModel.updateConfirmedAddress(testAddress, withoutVerification: true) + } + + // Then + XCTAssertFalse(receivedIsVerified, "updateDestinationAddress should be called with isVerified: false when withoutVerification is true") + } } private extension WooShippingEditAddressViewModel {