diff --git a/Modules/Sources/Storage/Model/MIGRATIONS.md b/Modules/Sources/Storage/Model/MIGRATIONS.md index 46d7c2ecc7b..eb5347c7636 100644 --- a/Modules/Sources/Storage/Model/MIGRATIONS.md +++ b/Modules/Sources/Storage/Model/MIGRATIONS.md @@ -10,6 +10,7 @@ This file documents changes in the WCiOS Storage data model. Please explain any - Added `shipments` relationship to `Order` entity. - @itsmeichigo 2025-07-22 - Added `WooShippingOriginAddress` entity. + - Added attributes `lastOrderCompleted` and `addPaymentMethodURL` to `ShippingLabelAccountSettings` entity. ## Model 123 (Release 22.8.0.0) - @iamgabrielma 2025-06-30 diff --git a/Modules/Sources/Storage/Model/ShippingLabelAccountSettings+CoreDataProperties.swift b/Modules/Sources/Storage/Model/ShippingLabelAccountSettings+CoreDataProperties.swift index 983bd16b4e4..33cf89eba94 100644 --- a/Modules/Sources/Storage/Model/ShippingLabelAccountSettings+CoreDataProperties.swift +++ b/Modules/Sources/Storage/Model/ShippingLabelAccountSettings+CoreDataProperties.swift @@ -12,6 +12,7 @@ extension ShippingLabelAccountSettings { @NSManaged public var canManagePayments: Bool @NSManaged public var isEmailReceiptsEnabled: Bool @NSManaged public var lastSelectedPackageID: String? + @NSManaged public var lastOrderCompleted: Bool @NSManaged public var paperSize: String? @NSManaged public var selectedPaymentMethodID: Int64 @NSManaged public var siteID: Int64 @@ -19,6 +20,7 @@ extension ShippingLabelAccountSettings { @NSManaged public var storeOwnerUsername: String? @NSManaged public var storeOwnerWpcomEmail: String? @NSManaged public var storeOwnerWpcomUsername: String? + @NSManaged public var addPaymentMethodURL: String? @NSManaged public var paymentMethods: Set? } diff --git a/Modules/Sources/Storage/Resources/WooCommerce.xcdatamodeld/Model 124.xcdatamodel/contents b/Modules/Sources/Storage/Resources/WooCommerce.xcdatamodeld/Model 124.xcdatamodel/contents index db135cc6dc7..6677bf46c4e 100644 --- a/Modules/Sources/Storage/Resources/WooCommerce.xcdatamodeld/Model 124.xcdatamodel/contents +++ b/Modules/Sources/Storage/Resources/WooCommerce.xcdatamodeld/Model 124.xcdatamodel/contents @@ -778,9 +778,11 @@ + + diff --git a/Modules/Sources/Yosemite/Model/Storage/ShippingLabelAccountSettings+ReadOnlyConvertible.swift b/Modules/Sources/Yosemite/Model/Storage/ShippingLabelAccountSettings+ReadOnlyConvertible.swift index d4d6cd0288b..dbaebd48a1d 100644 --- a/Modules/Sources/Yosemite/Model/Storage/ShippingLabelAccountSettings+ReadOnlyConvertible.swift +++ b/Modules/Sources/Yosemite/Model/Storage/ShippingLabelAccountSettings+ReadOnlyConvertible.swift @@ -18,6 +18,8 @@ extension Storage.ShippingLabelAccountSettings: ReadOnlyConvertible { isEmailReceiptsEnabled = settings.isEmailReceiptsEnabled paperSize = settings.paperSize.rawValue lastSelectedPackageID = settings.lastSelectedPackageID + lastOrderCompleted = settings.lastOrderCompleted + addPaymentMethodURL = settings.addPaymentMethodURL?.absoluteString } /// Returns a ReadOnly version of the receiver. @@ -26,7 +28,6 @@ extension Storage.ShippingLabelAccountSettings: ReadOnlyConvertible { let paymentMethodItems = paymentMethods?.map { $0.toReadOnly() } ?? [] /// Since account settings are not persisted for the new shipping label flow, - /// the conversion for the new properties `addPaymentMethodURL` & `lastOrderCompleted` is ignored. /// This avoids the complication of unnecessary Core Data migration for the new properties. return ShippingLabelAccountSettings(siteID: siteID, canManagePayments: canManagePayments, @@ -40,7 +41,7 @@ extension Storage.ShippingLabelAccountSettings: ReadOnlyConvertible { isEmailReceiptsEnabled: isEmailReceiptsEnabled, paperSize: .init(rawValue: paperSize ?? ""), lastSelectedPackageID: lastSelectedPackageID ?? "", - lastOrderCompleted: false, - addPaymentMethodURL: nil) + lastOrderCompleted: lastOrderCompleted, + addPaymentMethodURL: URL(string: addPaymentMethodURL ?? "")) } } diff --git a/Modules/Tests/StorageTests/CoreData/MigrationTests.swift b/Modules/Tests/StorageTests/CoreData/MigrationTests.swift index d21df4518e0..b892b1e470a 100644 --- a/Modules/Tests/StorageTests/CoreData/MigrationTests.swift +++ b/Modules/Tests/StorageTests/CoreData/MigrationTests.swift @@ -3518,6 +3518,57 @@ final class MigrationTests: XCTestCase { let insertedAddress = try XCTUnwrap(targetContext.firstObject(ofType: WooShippingOriginAddress.self)) XCTAssertEqual(insertedAddress, address) } + + func test_migrating_from_123_to_124_adds_new_attributes_lastOrderCompleted_and_addPaymentMethodURL_to_ShippingLabelAccountSettings() throws { + // Given + let sourceContainer = try startPersistentContainer("Model 123") + let sourceContext = sourceContainer.viewContext + + let object = sourceContext.insert(entityName: "ShippingLabelAccountSettings", properties: [ + "siteID": 123, + "canEditSettings": false, + "canManagePayments": false, + "isEmailReceiptsEnabled": false, + "lastSelectedPackageID": "", + "paperSize": "", + "selectedPaymentMethodID": 0, + "storeOwnerDisplayName": "", + "storeOwnerUsername": "", + "storeOwnerWpcomEmail": "", + "storeOwnerWpcomUsername": "" + ]) + try sourceContext.save() + + // `lastOrderCompleted` and `addPaymentMethodURL` should not be present in model 122 + XCTAssertNil(object.entity.attributesByName["lastOrderCompleted"], "Precondition. Attribute does not exist.") + XCTAssertNil(object.entity.attributesByName["addPaymentMethodURL"], "Precondition. Attribute does not exist.") + + // When + let targetContainer = try migrate(sourceContainer, to: "Model 124") + + // Then + let targetContext = targetContainer.viewContext + let migratedObject = try XCTUnwrap(targetContext.first(entityName: "ShippingLabelAccountSettings")) + + // `lastOrderCompleted` should be present in model 124 + XCTAssertNotNil(migratedObject.entity.attributesByName["lastOrderCompleted"]) + + // `addPaymentMethodURL` value should default as nil in model 124 + let value = migratedObject.value(forKey: "addPaymentMethodURL") as? String + XCTAssertNil(value) + + // `lastOrderCompleted` must be settable + migratedObject.setValue(true, forKey: "lastOrderCompleted") + try targetContext.save() + let updatedValue = migratedObject.value(forKey: "lastOrderCompleted") as? Bool + XCTAssertEqual(updatedValue, true) + + // `addPaymentMethodURL` must be settable + migratedObject.setValue("https://example.com", forKey: "addPaymentMethodURL") + try targetContext.save() + let urlValue = migratedObject.value(forKey: "addPaymentMethodURL") as? String + XCTAssertEqual(urlValue, "https://example.com") + } } // MARK: - Persistent Store Setup and Migrations diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index def30da7bb4..9cfd2567c22 100644 --- a/RELEASE-NOTES.txt +++ b/RELEASE-NOTES.txt @@ -13,6 +13,7 @@ - [*] Shipping Labels: Validate custom package dimensions [https://github.com/woocommerce/woocommerce-ios/pull/15925] - [*] Shipping Labels: Show UPS TOS modal in full length for better accessibility. [https://github.com/woocommerce/woocommerce-ios/pull/15926] - [*] Shipping Labels: Optimize data loading on purchase form [https://github.com/woocommerce/woocommerce-ios/pull/15919] +- [*] Shipping Labels: Cache settings and origin addresses to improve loading experience for purchase form [https://github.com/woocommerce/woocommerce-ios/pull/15935] - [internal] Optimized assets for app size reduction [https://github.com/woocommerce/woocommerce-ios/pull/15881] 22.8 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 59164e0c801..10feb24cc2e 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 @@ -220,6 +220,27 @@ final class WooShippingCreateLabelsViewModel: ObservableObject { return ResultsController(storageManager: storageManager, matching: predicate, sortedBy: [descriptor]) }() + /// Origin addresses Results Controller. + /// + private lazy var originAddressResultsController: ResultsController = { + let predicate = NSPredicate(format: "siteID = %ld", self.order.siteID) + let descriptor = NSSortDescriptor(keyPath: \StorageWooShippingOriginAddress.id, ascending: true) + + return ResultsController(storageManager: storageManager, matching: predicate, sortedBy: [descriptor]) + }() + + /// Shipping Label Account Settings ResultsController + /// + private lazy var accountSettingsResultsController: ResultsController = { + let predicate = NSPredicate(format: "siteID == %lld", order.siteID) + return ResultsController( + storageManager: storageManager, + matching: predicate, + fetchLimit: 1, + sortedBy: [] + ) + }() + /// Initialize the view model with or without an existing shipping label. init(order: Order, preselection: WooShippingCreateLabelSelection? = nil, @@ -241,6 +262,8 @@ final class WooShippingCreateLabelsViewModel: ObservableObject { self.storageManager = storageManager self.analytics = analytics self.shippingSettingsService = shippingSettingsService + self.weightUnit = shippingSettingsService.weightUnit ?? "" + self.dimensionsUnit = shippingSettingsService.dimensionUnit ?? "" self.initialNoticeDelay = initialNoticeDelay self.isOrderCompleted = order.status == .completed @@ -267,6 +290,8 @@ final class WooShippingCreateLabelsViewModel: ObservableObject { observeViewStates() observePaymentMethod() configureShipmentResultsController() + configureOriginAddressResultsController() + configureAccountSettingsResultsController() Task { @MainActor in await loadRequiredData() @@ -291,16 +316,24 @@ final class WooShippingCreateLabelsViewModel: ObservableObject { func loadRequiredData() async { state = .loading await withTaskGroup(of: Void.self) { group in + /// Only load store options synchronously if no settings have been saved in storage yet. if isMissingStoreSettings { group.addTask { await self.loadStoreOptions() } + } else { + /// load asynchronously to update the local storage and unblock UI + stores.dispatch(WooShippingAction.loadAccountSettings(siteID: order.siteID, completion: { _ in })) } - if hasUnfulfilledShipments { + /// Only load origin addresses synchronously if no addresses have been saved in storage yet. + if hasUnfulfilledShipments, originAddress.isEmpty { group.addTask { await self.loadOriginAddresses() } + } else if hasUnfulfilledShipments { + /// load asynchronously to update the local storage and unblock UI + stores.dispatch(WooShippingAction.loadOriginAddresses(siteID: order.siteID, completion: { _ in })) } } @@ -418,12 +451,7 @@ private extension WooShippingCreateLabelsViewModel { } weightUnit = settings?.storeOptions.weightUnit ?? shippingSettingsService.weightUnit ?? "" dimensionsUnit = settings?.storeOptions.dimensionUnit ?? shippingSettingsService.dimensionUnit ?? "" - markOrderComplete = settings?.accountSettings.lastOrderCompleted ?? false - - if let accountSettings = settings?.accountSettings { - paymentMethodsViewModel = ShippingLabelPaymentMethodsViewModel(accountSettings: accountSettings) - } - setupPaymentMethod(accountSettings: settings?.accountSettings) + updateAccountSettings(accountSettings: settings?.accountSettings) } /// Syncs origin addresses to use for shipping label from remote. @@ -442,12 +470,7 @@ private extension WooShippingCreateLabelsViewModel { } stores.dispatch(action) } - selectedOriginAddress = addresses.first(where: \.defaultAddress) - originAddresses = WooShippingOriginAddressListViewModel(addresses: addresses, - selectedAddressID: selectedOriginAddress?.id) - originAddresses.onSelect = { [weak self] selectedAddress in - self?.selectedOriginAddress = selectedAddress - } + updateSelectedOriginAddress(addresses: addresses) } /// Loads destination address of the order from remote. @@ -673,6 +696,70 @@ private extension WooShippingCreateLabelsViewModel { DDLogError("⛔️ Unable to fetch shipments: \(error)") } } + + func configureOriginAddressResultsController() { + let updateSelectedAddress = { [weak self] in + guard let self else { return } + let addresses = originAddressResultsController.fetchedObjects + updateSelectedOriginAddress(addresses: addresses) + } + originAddressResultsController.onDidChangeContent = { + updateSelectedAddress() + } + + originAddressResultsController.onDidResetContent = { + updateSelectedAddress() + } + + do { + try originAddressResultsController.performFetch() + updateSelectedAddress() + } catch { + DDLogError("⛔️ Unable to fetch origin addresses: \(error)") + } + } + + func updateSelectedOriginAddress(addresses: [WooShippingOriginAddress]) { + selectedOriginAddress = addresses.first(where: \.defaultAddress) + originAddresses = WooShippingOriginAddressListViewModel(addresses: addresses, + selectedAddressID: selectedOriginAddress?.id) + originAddresses.onSelect = { [weak self] selectedAddress in + self?.selectedOriginAddress = selectedAddress + } + } + + /// Shipping Label Account Settings ResultsController monitoring + /// + func configureAccountSettingsResultsController() { + let updateSettings = { [weak self] in + guard let self else { return } + let settings = accountSettingsResultsController.fetchedObjects.first + updateAccountSettings(accountSettings: settings) + } + accountSettingsResultsController.onDidChangeContent = { + updateSettings() + } + + accountSettingsResultsController.onDidResetContent = { + updateSettings() + } + + do { + try accountSettingsResultsController.performFetch() + updateSettings() + } catch { + DDLogError("⛔️ Unable to fetch woo shipping account settings: \(error)") + } + } + + func updateAccountSettings(accountSettings: ShippingLabelAccountSettings?) { + markOrderComplete = accountSettings?.lastOrderCompleted ?? false + + if let accountSettings { + paymentMethodsViewModel = ShippingLabelPaymentMethodsViewModel(accountSettings: accountSettings) + } + setupPaymentMethod(accountSettings: accountSettings) + } } private extension WooShippingCreateLabelsViewModel { diff --git a/WooCommerce/WooCommerceTests/ViewRelated/Shipping Label/WooShipping Create Shipping Labels/WooShippingCreateLabelsViewModelTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/Shipping Label/WooShipping Create Shipping Labels/WooShippingCreateLabelsViewModelTests.swift index 02ff97ef76a..2549abd298c 100644 --- a/WooCommerce/WooCommerceTests/ViewRelated/Shipping Label/WooShipping Create Shipping Labels/WooShippingCreateLabelsViewModelTests.swift +++ b/WooCommerce/WooCommerceTests/ViewRelated/Shipping Label/WooShipping Create Shipping Labels/WooShippingCreateLabelsViewModelTests.swift @@ -778,7 +778,9 @@ final class WooShippingCreateLabelsViewModelTests: XCTestCase { } // When - let viewModel = WooShippingCreateLabelsViewModel(order: Order.fake(), stores: stores) + let viewModel = WooShippingCreateLabelsViewModel(order: Order.fake(), + shippingSettingsService: MockShippingSettingsService(dimensionUnit: "", weightUnit: ""), + stores: stores) waitUntil { viewModel.state == .ready @@ -847,6 +849,7 @@ final class WooShippingCreateLabelsViewModelTests: XCTestCase { case .loadOriginAddresses(_, let completion): completion(.success([originAddress])) case .loadAccountSettings(_, let completion): + self.insert(accountSettings: accountSettings) completion(.success(settings)) case .loadPackages, .verifyDestinationAddress: break @@ -856,7 +859,9 @@ final class WooShippingCreateLabelsViewModelTests: XCTestCase { } - let viewModel = WooShippingCreateLabelsViewModel(order: Order.fake(), stores: stores) + let viewModel = WooShippingCreateLabelsViewModel(order: Order.fake(), + shippingSettingsService: MockShippingSettingsService(dimensionUnit: "", weightUnit: ""), + stores: stores) waitUntil { viewModel.state == .ready @@ -885,6 +890,154 @@ final class WooShippingCreateLabelsViewModelTests: XCTestCase { isEditable: true) )) } + + // MARK: - loadRequiredData with stored data tests + + func test_loadRequiredData_state_becomes_ready_immediately_when_origin_addresses_and_settings_are_stored() { + // Given + let originAddress = WooShippingOriginAddress(siteID: siteID, + id: "stored_address", + company: "Stored Company", + address1: "123 Stored St", + address2: "", + city: "San Francisco", + state: "CA", + postcode: "94102", + country: "US", + phone: "555-0123", + firstName: "John", + lastName: "Doe", + email: "john@stored.com", + defaultAddress: true, + isVerified: true) + + let accountSettings = ShippingLabelAccountSettings.fake().copy(siteID: siteID) + + // Pre-populate storage with data + insert(originAddress: originAddress) + insert(accountSettings: accountSettings) + + let stores = MockStoresManager(sessionManager: .testingInstance) + let shippingSettingsService = MockShippingSettingsService(dimensionUnit: "cm", weightUnit: "g") + let viewModel = WooShippingCreateLabelsViewModel(order: Order.fake().copy(siteID: siteID), + shippingSettingsService: shippingSettingsService, + stores: stores, + storageManager: storageManager) + + waitUntil { + viewModel.state == .ready + } + + // Then + XCTAssertEqual(viewModel.originAddress, "123 Stored St, San Francisco CA 94102, US") + XCTAssertEqual(viewModel.dimensionsUnit, "cm") + XCTAssertEqual(viewModel.weightUnit, "g") + } + + func test_loadRequiredData_state_becomes_ready_immediately_when_only_origin_addresses_are_stored() { + // Given + let originAddress = WooShippingOriginAddress(siteID: siteID, + id: "stored_address", + company: "Stored Company", + address1: "456 Another St", + address2: "", + city: "Los Angeles", + state: "CA", + postcode: "90210", + country: "US", + phone: "555-0456", + firstName: "Jane", + lastName: "Smith", + email: "jane@stored.com", + defaultAddress: true, + isVerified: false) + + // Pre-populate storage with origin addresses only + insert(originAddress: originAddress) + + let stores = MockStoresManager(sessionManager: .testingInstance) + var accountSettingsLoaded = false + + stores.whenReceivingAction(ofType: WooShippingAction.self) { action in + switch action { + case .loadAccountSettings(_, let completion): + accountSettingsLoaded = true + completion(.success(self.settings)) + case .loadPackages, .verifyDestinationAddress, .loadOriginAddresses: + break + default: + XCTFail("Unexpected action: \(action)") + } + } + + let shippingSettingsService = MockShippingSettingsService(dimensionUnit: nil, weightUnit: nil) + let viewModel = WooShippingCreateLabelsViewModel(order: Order.fake().copy(siteID: siteID), + shippingSettingsService: shippingSettingsService, + stores: stores, + storageManager: storageManager) + + waitUntil { + viewModel.state == .ready + } + + // Then + XCTAssertEqual(viewModel.originAddress, "456 Another St, Los Angeles CA 90210, US") + XCTAssertTrue(accountSettingsLoaded, "Account settings should be loaded from remote when missing") + XCTAssertEqual(viewModel.dimensionsUnit, settings.storeOptions.dimensionUnit) + XCTAssertEqual(viewModel.weightUnit, settings.storeOptions.weightUnit) + } + + func test_loadRequiredData_state_becomes_ready_immediately_when_only_account_settings_are_stored() { + // Given + let accountSettings = ShippingLabelAccountSettings.fake().copy(siteID: siteID) + + // Pre-populate storage with account settings only + insert(accountSettings: accountSettings) + + let originAddress = WooShippingOriginAddress(siteID: siteID, + id: "stored_address", + company: "Stored Company", + address1: "456 Another St", + address2: "", + city: "Los Angeles", + state: "CA", + postcode: "90210", + country: "US", + phone: "555-0456", + firstName: "Jane", + lastName: "Smith", + email: "jane@stored.com", + defaultAddress: true, + isVerified: false) + let stores = MockStoresManager(sessionManager: .testingInstance) + var originAddressesLoaded = false + + stores.whenReceivingAction(ofType: WooShippingAction.self) { action in + switch action { + case .loadOriginAddresses(_, let completion): + originAddressesLoaded = true + completion(.success([originAddress])) + case .loadPackages, .verifyDestinationAddress, .loadAccountSettings: + break + default: + XCTFail("Unexpected action: \(action)") + } + } + + let shippingSettingsService = MockShippingSettingsService(dimensionUnit: "cm", weightUnit: "g") + let viewModel = WooShippingCreateLabelsViewModel(order: Order.fake().copy(siteID: siteID), + shippingSettingsService: shippingSettingsService, + stores: stores, + storageManager: storageManager) + + waitUntil { + viewModel.state == .ready + } + + // Then + XCTAssertTrue(originAddressesLoaded, "Origin addresses should be loaded from remote when missing") + XCTAssertFalse(viewModel.originAddress.isEmpty, "Origin address should be set from loaded data") + } } private extension WooShippingCreateLabelsViewModelTests { @@ -934,4 +1087,14 @@ private extension WooShippingCreateLabelsViewModelTests { } } } + + func insert(originAddress: WooShippingOriginAddress) { + let storageAddress = storage.insertNewObject(ofType: StorageWooShippingOriginAddress.self) + storageAddress.update(with: originAddress) + } + + func insert(accountSettings: ShippingLabelAccountSettings) { + let storageSettings = storage.insertNewObject(ofType: StorageShippingLabelAccountSettings.self) + storageSettings.update(with: accountSettings) + } }