From 9c2ae6314bce442b48298bf6b2e172cbb1d19cc1 Mon Sep 17 00:00:00 2001 From: Huong Do Date: Wed, 9 Jul 2025 12:32:21 +0700 Subject: [PATCH 01/13] Return shipments when syncing labels for woo shipping --- .../Yosemite/Actions/WooShippingAction.swift | 2 +- .../Yosemite/Stores/WooShippingStore.swift | 19 ++++++-- .../OrderDetailsDataSource.swift | 34 +++++++++++++ .../Order Details/OrderDetailsViewModel.swift | 48 ++++++++++++------- 4 files changed, 82 insertions(+), 21 deletions(-) diff --git a/Modules/Sources/Yosemite/Actions/WooShippingAction.swift b/Modules/Sources/Yosemite/Actions/WooShippingAction.swift index 5e51861785a..ce37a8dead7 100644 --- a/Modules/Sources/Yosemite/Actions/WooShippingAction.swift +++ b/Modules/Sources/Yosemite/Actions/WooShippingAction.swift @@ -111,7 +111,7 @@ public enum WooShippingAction: Action { /// case syncShippingLabels(siteID: Int64, orderID: Int64, - completion: (Result<[ShippingLabel], Error>) -> Void) + completion: (Result) -> Void) /// Updates shipments for given order /// diff --git a/Modules/Sources/Yosemite/Stores/WooShippingStore.swift b/Modules/Sources/Yosemite/Stores/WooShippingStore.swift index 8dc2bfc4e94..2e365272a06 100644 --- a/Modules/Sources/Yosemite/Stores/WooShippingStore.swift +++ b/Modules/Sources/Yosemite/Stores/WooShippingStore.swift @@ -2,6 +2,18 @@ import Foundation import Networking import Storage +public struct ShippingLabelSyncResult { + public let labels: [ShippingLabel] + public let shipments: WooShippingShipments + + public init(labels: [ShippingLabel], shipments: WooShippingShipments) { + self.labels = labels + self.shipments = shipments + } + + public static let none = ShippingLabelSyncResult(labels: [], shipments: [:]) +} + /// Implements `WooShippingAction` actions /// public final class WooShippingStore: Store { @@ -292,7 +304,7 @@ private extension WooShippingStore { func syncShippingLabels(siteID: Int64, orderID: Int64, - completion: @escaping (Result<[ShippingLabel], Error>) -> Void) { + completion: @escaping (Result) -> Void) { remote.loadConfig(siteID: siteID, orderID: orderID, completion: { [weak self] result in guard let self else { return } @@ -300,13 +312,14 @@ private extension WooShippingStore { case .failure(let error): completion(.failure(error)) case .success(let config): + let shipments = config.shipments guard let labels = config.shippingLabelData?.currentOrderLabels else { - return completion(.success([])) + return completion(.success(ShippingLabelSyncResult(labels: [], shipments: shipments))) } upsertShippingLabelsInBackground(siteID: siteID, orderID: orderID, shippingLabels: labels) { - completion(.success(labels)) + completion(.success(ShippingLabelSyncResult(labels: labels, shipments: shipments))) } } }) diff --git a/WooCommerce/Classes/ViewModels/Order Details/OrderDetailsDataSource.swift b/WooCommerce/Classes/ViewModels/Order Details/OrderDetailsDataSource.swift index cad8157d7a9..772f80b5589 100644 --- a/WooCommerce/Classes/ViewModels/Order Details/OrderDetailsDataSource.swift +++ b/WooCommerce/Classes/ViewModels/Order Details/OrderDetailsDataSource.swift @@ -61,6 +61,10 @@ final class OrderDetailsDataSource: NSObject { return true } + /// Shipments created for the order + /// + private var wooShippingShipments: [Shipment] = [] + /// Whether the button to create shipping labels should be visible. /// var shouldShowShippingLabelCreation: Bool { @@ -1696,6 +1700,36 @@ extension OrderDetailsDataSource { } } +// MARK: - Woo Shipping Shipments +extension OrderDetailsDataSource { + struct Shipment { + let index: String + let shippingLabel: ShippingLabel? + } + + func populateShipments(labels: [ShippingLabel], shipments: WooShippingShipments) { + var contents = [Shipment]() + + for key in shipments.keys.sorted(by: { $0.localizedStandardCompare($1) == .orderedAscending }) { + + let label: ShippingLabel? = { + let purchasedLabels = labels.filter { + $0.shipmentID == key && $0.status == .purchased + } + let sortedLabels = purchasedLabels.sorted { $0.dateCreated > $1.dateCreated } + if let completedLabel = sortedLabels.first(where: { $0.refund == nil }) { + return completedLabel + } else { + return sortedLabels.first + } + }() + + let shipment = Shipment(index: key, shippingLabel: label) + contents.append(shipment) + } + self.wooShippingShipments = contents + } +} // MARK: - Constants extension OrderDetailsDataSource { diff --git a/WooCommerce/Classes/ViewModels/Order Details/OrderDetailsViewModel.swift b/WooCommerce/Classes/ViewModels/Order Details/OrderDetailsViewModel.swift index d41ccfa2261..a5bef586ed5 100644 --- a/WooCommerce/Classes/ViewModels/Order Details/OrderDetailsViewModel.swift +++ b/WooCommerce/Classes/ViewModels/Order Details/OrderDetailsViewModel.swift @@ -283,9 +283,12 @@ extension OrderDetailsViewModel { taskGroup.addTask { [weak self] in guard let self else { return } // Sync shipping labels and update order with the result if available - let shippingLabels = await syncShippingLabels() + let syncResult = await syncShippingLabels() + dataSource.populateShipments(labels: syncResult.labels, + shipments: syncResult.shipments) + // Update the order with the newly synced shipping labels - let updatedOrder = order.copy(shippingLabels: shippingLabels) + let updatedOrder = order.copy(shippingLabels: syncResult.labels) update(order: updatedOrder) } } @@ -703,18 +706,18 @@ extension OrderDetailsViewModel { } @discardableResult - @MainActor func syncShippingLabels() async -> [ShippingLabel] { + @MainActor func syncShippingLabels() async -> ShippingLabelSyncResult { let isRevampedFlow = featureFlagService.isFeatureFlagEnabled(.revampedShippingLabelCreation) guard isRevampedFlow else { /// old logic for syncing labels if await localRequirementsForShippingLabelsAreFulfilled() { return await syncShippingLabelsForLegacyPlugin(isRevampedFlow: isRevampedFlow) } - return [] + return .none } guard !orderContainsOnlyVirtualProducts else { - return [] + return .none } if await isPluginActive(pluginPath: SitePlugin.SupportedPluginPath.WooShipping) { @@ -722,7 +725,7 @@ extension OrderDetailsViewModel { } else if await isPluginActive(pluginPath: SitePlugin.SupportedPluginPath.LegacyWCShip) { return await syncShippingLabelsForLegacyPlugin(isRevampedFlow: isRevampedFlow) } else { - return [] + return .none } } @@ -990,33 +993,45 @@ private extension OrderDetailsViewModel { } } - @MainActor func syncShippingLabelsForWooShipping() async -> [ShippingLabel] { + @MainActor func syncShippingLabelsForWooShipping() async -> ShippingLabelSyncResult { await withCheckedContinuation { continuation in stores.dispatch(WooShippingAction.syncShippingLabels(siteID: order.siteID, orderID: order.orderID) { [weak self] result in - let labels = self?.handleShippingLabelSyncingResult(result: result, isRevampedFlow: true) ?? [] - continuation.resume(returning: labels) + switch result { + case let .success(result): + self?.trackShippingLabelSyncingResult(result: .success, isRevampedFlow: true) + continuation.resume(returning: result) + case let .failure(error): + self?.trackShippingLabelSyncingResult(result: .failed(error: error), isRevampedFlow: true) + continuation.resume(returning: .none) + } }) } } - @MainActor func syncShippingLabelsForLegacyPlugin(isRevampedFlow: Bool) async -> [ShippingLabel] { + @MainActor func syncShippingLabelsForLegacyPlugin(isRevampedFlow: Bool) async -> ShippingLabelSyncResult { await withCheckedContinuation { continuation in stores.dispatch(ShippingLabelAction.synchronizeShippingLabels(siteID: order.siteID, orderID: order.orderID) { [weak self] result in - let labels = self?.handleShippingLabelSyncingResult(result: result, isRevampedFlow: isRevampedFlow) ?? [] - continuation.resume(returning: labels) + switch result { + case .success(let labels): + self?.trackShippingLabelSyncingResult(result: .success, isRevampedFlow: isRevampedFlow) + continuation.resume(returning: ShippingLabelSyncResult(labels: labels, shipments: [:])) + case .failure(let error): + self?.trackShippingLabelSyncingResult(result: .failed(error: error), isRevampedFlow: isRevampedFlow) + continuation.resume(returning: .none) + } }) } } - func handleShippingLabelSyncingResult(result: Result<[ShippingLabel], Error>, isRevampedFlow: Bool) -> [ShippingLabel] { + func trackShippingLabelSyncingResult(result: WooAnalyticsEvent.ShippingLabelsAPIRequestResult, + isRevampedFlow: Bool) { switch result { - case .success(let shippingLabels): + case .success: ServiceLocator.analytics.track(event: .shippingLabelsAPIRequest( result: .success, isRevampedFlow: isRevampedFlow )) - return shippingLabels - case .failure(let error): + case .failed(let error): ServiceLocator.analytics.track(event: .shippingLabelsAPIRequest( result: .failed(error: error), isRevampedFlow: isRevampedFlow @@ -1026,7 +1041,6 @@ private extension OrderDetailsViewModel { } else { DDLogError("⛔️ Error synchronizing shipping labels: \(error)") } - return [] } } From cb2e10f7dc1426c988b7a9a970b9a60aaaa03bf3 Mon Sep 17 00:00:00 2001 From: Huong Do Date: Wed, 9 Jul 2025 12:54:15 +0700 Subject: [PATCH 02/13] Update unit tests --- .../YosemiteTests/Stores/WooShippingStoreTests.swift | 8 ++++++-- .../ViewRelated/OrderDetailsViewModelTests.swift | 4 ++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/Modules/Tests/YosemiteTests/Stores/WooShippingStoreTests.swift b/Modules/Tests/YosemiteTests/Stores/WooShippingStoreTests.swift index 0e277b90640..fa605cbbd24 100644 --- a/Modules/Tests/YosemiteTests/Stores/WooShippingStoreTests.swift +++ b/Modules/Tests/YosemiteTests/Stores/WooShippingStoreTests.swift @@ -1083,8 +1083,9 @@ final class WooShippingStoreTests: XCTestCase { usedDate: nil, expiryDate: nil) }() + let shipments = ["0": [WooShippingShipmentItem.fake()]] let expectedResponse = WooShippingConfig.fake().copy( - shipments: ["0": [WooShippingShipmentItem.fake()]], + shipments: shipments, shippingLabelData: WooShippingLabelData(currentOrderLabels: [expectedShippingLabel]) ) remote.whenLoadingConfig(siteID: sampleSiteID, thenReturn: .success(expectedResponse)) @@ -1093,7 +1094,7 @@ final class WooShippingStoreTests: XCTestCase { insertOrder(siteID: sampleSiteID, orderID: orderID) // When - let result: Result<[Yosemite.ShippingLabel], Error> = waitFor { promise in + let result: Result = waitFor { promise in let action = WooShippingAction.syncShippingLabels(siteID: self.sampleSiteID, orderID: orderID) { result in promise(result) } @@ -1102,6 +1103,9 @@ final class WooShippingStoreTests: XCTestCase { // Then XCTAssertTrue(result.isSuccess) + let syncResult = try XCTUnwrap(result.get()) + XCTAssertEqual(syncResult.labels, [expectedShippingLabel]) + XCTAssertEqual(syncResult.shipments, shipments) let persistedOrder = try XCTUnwrap(viewStorage.loadOrder(siteID: sampleSiteID, orderID: orderID)) let persistedShippingLabels = try XCTUnwrap(viewStorage.loadAllShippingLabels(siteID: sampleSiteID, orderID: orderID)) diff --git a/WooCommerce/WooCommerceTests/ViewRelated/OrderDetailsViewModelTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/OrderDetailsViewModelTests.swift index a94cfacabdd..aca38858f3c 100644 --- a/WooCommerce/WooCommerceTests/ViewRelated/OrderDetailsViewModelTests.swift +++ b/WooCommerce/WooCommerceTests/ViewRelated/OrderDetailsViewModelTests.swift @@ -152,7 +152,7 @@ final class OrderDetailsViewModelTests: XCTestCase { let plugin = insertSystemPlugin(path: SitePlugin.SupportedPluginPath.WooShipping, siteID: order.siteID, isActive: true) whenFetchingSystemPlugin(path: SitePlugin.SupportedPluginPath.WooShipping, thenReturn: plugin) - whenSyncingShippingLabels(thenReturn: .success([])) + whenSyncingShippingLabels(thenReturn: .success(ShippingLabelSyncResult.none)) let featureFlagService = MockFeatureFlagService(revampedShippingLabelCreation: true) let viewModel = OrderDetailsViewModel(order: order, @@ -789,7 +789,7 @@ private extension OrderDetailsViewModelTests { } } - func whenSyncingShippingLabels(thenReturn result: Result<[ShippingLabel], Error>) { + func whenSyncingShippingLabels(thenReturn result: Result) { storesManager.whenReceivingAction(ofType: WooShippingAction.self) { action in switch action { case let .syncShippingLabels(_, _, completion): From 18d83c0a77abd41f4beb9468e65abd65c569947a Mon Sep 17 00:00:00 2001 From: Huong Do Date: Wed, 9 Jul 2025 13:12:01 +0700 Subject: [PATCH 03/13] Populate items for shipments --- .../OrderDetailsDataSource.swift | 21 +++++++++++++++++-- .../Order Details/OrderDetailsViewModel.swift | 2 +- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/WooCommerce/Classes/ViewModels/Order Details/OrderDetailsDataSource.swift b/WooCommerce/Classes/ViewModels/Order Details/OrderDetailsDataSource.swift index 772f80b5589..6d054e042a3 100644 --- a/WooCommerce/Classes/ViewModels/Order Details/OrderDetailsDataSource.swift +++ b/WooCommerce/Classes/ViewModels/Order Details/OrderDetailsDataSource.swift @@ -1705,13 +1705,18 @@ extension OrderDetailsDataSource { struct Shipment { let index: String let shippingLabel: ShippingLabel? + let items: [ShippingLabelPackageItem] } func populateShipments(labels: [ShippingLabel], shipments: WooShippingShipments) { + let itemsDataSource = DefaultWooShippingItemsDataSource(order: order) + let packageItems = itemsDataSource.items var contents = [Shipment]() for key in shipments.keys.sorted(by: { $0.localizedStandardCompare($1) == .orderedAscending }) { - + guard let shipmentItems = shipments[key] else { + continue + } let label: ShippingLabel? = { let purchasedLabels = labels.filter { $0.shipmentID == key && $0.status == .purchased @@ -1724,7 +1729,19 @@ extension OrderDetailsDataSource { } }() - let shipment = Shipment(index: key, shippingLabel: label) + var shipmentContents: [ShippingLabelPackageItem] = [] + for shipmentItem in shipmentItems { + guard let packageItem = packageItems.first(where: { $0.orderItemID == shipmentItem.id }), + let subItems = shipmentItem.subItems else { + continue + } + + let quantity = subItems.count > 0 ? subItems.count : 1 + let updatedItem = ShippingLabelPackageItem(copy: packageItem, quantity: Decimal(quantity)) + shipmentContents.append(updatedItem) + } + + let shipment = Shipment(index: key, shippingLabel: label, items: shipmentContents) contents.append(shipment) } self.wooShippingShipments = contents diff --git a/WooCommerce/Classes/ViewModels/Order Details/OrderDetailsViewModel.swift b/WooCommerce/Classes/ViewModels/Order Details/OrderDetailsViewModel.swift index a5bef586ed5..c279df00683 100644 --- a/WooCommerce/Classes/ViewModels/Order Details/OrderDetailsViewModel.swift +++ b/WooCommerce/Classes/ViewModels/Order Details/OrderDetailsViewModel.swift @@ -280,7 +280,7 @@ extension OrderDetailsViewModel { dataSource.isEligibleForShippingLabelCreation = isEligible } - taskGroup.addTask { [weak self] in + taskGroup.addTask { @MainActor [weak self] in guard let self else { return } // Sync shipping labels and update order with the result if available let syncResult = await syncShippingLabels() From 5b4836affa940679d0ed42169b178c8fe617123f Mon Sep 17 00:00:00 2001 From: Huong Do Date: Wed, 9 Jul 2025 13:58:36 +0700 Subject: [PATCH 04/13] Add tests for wooShippingShipments --- .../OrderDetailsDataSource.swift | 4 +- .../OrderDetailsDataSourceTests.swift | 214 +++++++++++++++++- 2 files changed, 215 insertions(+), 3 deletions(-) diff --git a/WooCommerce/Classes/ViewModels/Order Details/OrderDetailsDataSource.swift b/WooCommerce/Classes/ViewModels/Order Details/OrderDetailsDataSource.swift index 6d054e042a3..932f955f9ad 100644 --- a/WooCommerce/Classes/ViewModels/Order Details/OrderDetailsDataSource.swift +++ b/WooCommerce/Classes/ViewModels/Order Details/OrderDetailsDataSource.swift @@ -63,7 +63,7 @@ final class OrderDetailsDataSource: NSObject { /// Shipments created for the order /// - private var wooShippingShipments: [Shipment] = [] + private(set) var wooShippingShipments: [Shipment] = [] /// Whether the button to create shipping labels should be visible. /// @@ -1709,7 +1709,7 @@ extension OrderDetailsDataSource { } func populateShipments(labels: [ShippingLabel], shipments: WooShippingShipments) { - let itemsDataSource = DefaultWooShippingItemsDataSource(order: order) + let itemsDataSource = DefaultWooShippingItemsDataSource(order: order, storageManager: storageManager) let packageItems = itemsDataSource.items var contents = [Shipment]() diff --git a/WooCommerce/WooCommerceTests/ViewRelated/Orders/Order Details/OrderDetailsDataSourceTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/Orders/Order Details/OrderDetailsDataSourceTests.swift index f2daab1b770..93c679dd10a 100644 --- a/WooCommerce/WooCommerceTests/ViewRelated/Orders/Order Details/OrderDetailsDataSourceTests.swift +++ b/WooCommerce/WooCommerceTests/ViewRelated/Orders/Order Details/OrderDetailsDataSourceTests.swift @@ -1122,6 +1122,213 @@ final class OrderDetailsDataSourceTests: XCTestCase { XCTAssertNil(seeReceiptRow) XCTAssertNil(seeLegacyReceiptRow) } + + // MARK: - wooShippingShipments Tests + + func test_wooShippingShipments_is_initially_empty() { + // Given + let order = Order.fake() + let dataSource = OrderDetailsDataSource(order: order, + storageManager: storageManager, + cardPresentPaymentsConfiguration: Mocks.configuration, + receiptEligibilityUseCase: MockReceiptEligibilityUseCase()) + + // Then + XCTAssertTrue(dataSource.wooShippingShipments.isEmpty) + } + + func test_populateShipments_with_empty_data_results_in_empty_shipments() { + // Given + let order = Order.fake() + let dataSource = OrderDetailsDataSource(order: order, + storageManager: storageManager, + cardPresentPaymentsConfiguration: Mocks.configuration, + receiptEligibilityUseCase: MockReceiptEligibilityUseCase()) + let labels: [ShippingLabel] = [] + let shipments: WooShippingShipments = [:] + + // When + dataSource.populateShipments(labels: labels, shipments: shipments) + + // Then + XCTAssertTrue(dataSource.wooShippingShipments.isEmpty) + } + + func test_populateShipments_with_valid_data_creates_shipments() { + // Given + let order = makeOrder() + let dataSource = OrderDetailsDataSource(order: order, + storageManager: storageManager, + cardPresentPaymentsConfiguration: Mocks.configuration, + receiptEligibilityUseCase: MockReceiptEligibilityUseCase()) + + let shippingLabel1 = ShippingLabel.fake().copy(siteID: order.siteID, + orderID: order.orderID, + shipmentID: "1", + dateCreated: Date(), status: .purchased) + let shippingLabel2 = ShippingLabel.fake().copy(siteID: order.siteID, + orderID: order.orderID, + shipmentID: "0", + dateCreated: Date(timeIntervalSinceNow: -3600), status: .purchased) + let labels = [shippingLabel1, shippingLabel2] + + let orderItem0 = order.items[0] + let orderItem1 = order.items[1] + storageManager.insertSampleProduct(readOnlyProduct: Product.fake().copy(siteID: order.siteID, productID: orderItem0.productID)) + storageManager.insertSampleProduct(readOnlyProduct: Product.fake().copy(siteID: order.siteID, productID: orderItem1.productID)) + + let shipmentItem1 = WooShippingShipmentItem.fake().copy(id: orderItem0.itemID, subItems: ["sub1", "sub2"]) + let shipmentItem2 = WooShippingShipmentItem.fake().copy(id: orderItem1.itemID, subItems: ["sub3"]) + let shipments: WooShippingShipments = [ + "0": [shipmentItem1], + "1": [shipmentItem2] + ] + + // When + dataSource.populateShipments(labels: labels, shipments: shipments) + + // Then + XCTAssertEqual(dataSource.wooShippingShipments.count, 2) + + // First shipment ("0" comes before "1" in sorted order) + let firstShipment = dataSource.wooShippingShipments[0] + XCTAssertEqual(firstShipment.index, "0") + XCTAssertEqual(firstShipment.shippingLabel, shippingLabel2) + XCTAssertEqual(firstShipment.items.count, 1) + XCTAssertEqual(firstShipment.items[0].quantity, 2) // 2 subItems + + // Second shipment + let secondShipment = dataSource.wooShippingShipments[1] + XCTAssertEqual(secondShipment.index, "1") + XCTAssertEqual(secondShipment.shippingLabel, shippingLabel1) + XCTAssertEqual(secondShipment.items.count, 1) + XCTAssertEqual(secondShipment.items[0].quantity, 1) // 1 subItem + } + + func test_populateShipments_with_no_matching_shipping_labels_creates_shipments_without_labels() { + // Given + let order = makeOrder() + let dataSource = OrderDetailsDataSource(order: order, + storageManager: storageManager, + cardPresentPaymentsConfiguration: Mocks.configuration, + receiptEligibilityUseCase: MockReceiptEligibilityUseCase()) + + let labels: [ShippingLabel] = [] + let shipmentItem1 = WooShippingShipmentItem.fake().copy(id: order.items[0].itemID, subItems: ["sub1"]) + let shipments: WooShippingShipments = [ + "0": [shipmentItem1] + ] + + // When + dataSource.populateShipments(labels: labels, shipments: shipments) + + // Then + XCTAssertEqual(dataSource.wooShippingShipments.count, 1) + + let shipment = dataSource.wooShippingShipments[0] + XCTAssertEqual(shipment.index, "0") + XCTAssertNil(shipment.shippingLabel) + } + + func test_populateShipments_prioritizes_non_refunded_labels() { + // Given + let order = makeOrder() + let dataSource = OrderDetailsDataSource(order: order, + storageManager: storageManager, + cardPresentPaymentsConfiguration: Mocks.configuration, + receiptEligibilityUseCase: MockReceiptEligibilityUseCase()) + + let refundedLabel = ShippingLabel.fake().copy(siteID: order.siteID, + orderID: order.orderID, + shipmentID: "0", + dateCreated: Date(), status: .purchased, + refund: ShippingLabelRefund.fake()) + let nonRefundedLabel = ShippingLabel.fake().copy(siteID: order.siteID, + orderID: order.orderID, + shipmentID: "0", + dateCreated: Date(timeIntervalSinceNow: -3600), status: .purchased, + refund: nil) + let labels = [refundedLabel, nonRefundedLabel] + + let shipmentItem1 = WooShippingShipmentItem.fake().copy(id: order.items[0].itemID, subItems: ["sub1"]) + let shipments: WooShippingShipments = [ + "0": [shipmentItem1] + ] + + // When + dataSource.populateShipments(labels: labels, shipments: shipments) + + // Then + XCTAssertEqual(dataSource.wooShippingShipments.count, 1) + + let shipment = dataSource.wooShippingShipments[0] + XCTAssertEqual(shipment.index, "0") + XCTAssertEqual(shipment.shippingLabel, nonRefundedLabel) // Should prioritize non-refunded + } + + func test_populateShipments_uses_most_recent_label_when_all_refunded() { + // Given + let order = makeOrder() + let dataSource = OrderDetailsDataSource(order: order, + storageManager: storageManager, + cardPresentPaymentsConfiguration: Mocks.configuration, + receiptEligibilityUseCase: MockReceiptEligibilityUseCase()) + + let olderRefundedLabel = ShippingLabel.fake().copy(siteID: order.siteID, + orderID: order.orderID, + shipmentID: "0", + dateCreated: Date(timeIntervalSinceNow: -3600), status: .purchased, + refund: ShippingLabelRefund.fake()) + let newerRefundedLabel = ShippingLabel.fake().copy(siteID: order.siteID, + orderID: order.orderID, + shipmentID: "0", + dateCreated: Date(), status: .purchased, + refund: ShippingLabelRefund.fake()) + let labels = [olderRefundedLabel, newerRefundedLabel] + + let shipmentItem1 = WooShippingShipmentItem.fake().copy(id: order.items[0].itemID, subItems: []) + let shipments: WooShippingShipments = [ + "0": [shipmentItem1] + ] + + // When + dataSource.populateShipments(labels: labels, shipments: shipments) + + // Then + XCTAssertEqual(dataSource.wooShippingShipments.count, 1) + + let shipment = dataSource.wooShippingShipments[0] + XCTAssertEqual(shipment.index, "0") + XCTAssertEqual(shipment.shippingLabel, newerRefundedLabel) // Should use newer refunded label + } + + func test_populateShipments_handles_shipments_with_no_subitems() { + // Given + let order = makeOrder() + let dataSource = OrderDetailsDataSource(order: order, + storageManager: storageManager, + cardPresentPaymentsConfiguration: Mocks.configuration, + receiptEligibilityUseCase: MockReceiptEligibilityUseCase()) + + let labels: [ShippingLabel] = [] + let orderItem = order.items[0] + storageManager.insertSampleProduct(readOnlyProduct: Product.fake().copy(siteID: order.siteID, productID: orderItem.productID)) + let shipmentItemNoSubItems = WooShippingShipmentItem.fake().copy(id: orderItem.itemID, subItems: []) + let shipments: WooShippingShipments = [ + "0": [shipmentItemNoSubItems] + ] + + // When + dataSource.populateShipments(labels: labels, shipments: shipments) + + // Then + XCTAssertEqual(dataSource.wooShippingShipments.count, 1) + + let shipment = dataSource.wooShippingShipments[0] + XCTAssertEqual(shipment.index, "0") + XCTAssertEqual(shipment.items.count, 1) + XCTAssertEqual(shipment.items[0].quantity, 1) // Should default to 1 when no subItems + } } // MARK: - Test Data @@ -1131,6 +1338,12 @@ private extension OrderDetailsDataSourceTests { MockOrders().makeOrder(items: [makeOrderItem(), makeOrderItem()], fees: [OrderFeeLine.fake()]) } + func makeOrder(items: [OrderItem] = [], fees: [OrderFeeLine] = []) -> Order { + let defaultItems = items.isEmpty ? [makeOrderItem(), makeOrderItem()] : items + let defaultFees = fees.isEmpty ? [OrderFeeLine.fake()] : fees + return MockOrders().makeOrder(items: defaultItems, fees: defaultFees) + } + func makeOrderItem() -> OrderItem { OrderItem(itemID: 1, name: "Order Item Name", @@ -1217,7 +1430,6 @@ private extension OrderDetailsDataSourceTests { func insert(_ readOnlyPlugin: Yosemite.SitePlugin) { let plugin = storage.insertNewObject(ofType: StorageSitePlugin.self) plugin.update(with: readOnlyPlugin) - storage.saveIfNeeded() } /// Finds first section with a given title from the provided data source. From 3551788e9f86b4a96866272f128264806a8b0419 Mon Sep 17 00:00:00 2001 From: Huong Do Date: Thu, 10 Jul 2025 11:57:04 +0700 Subject: [PATCH 05/13] Add new type WooShippingShipment --- .../Sources/Fakes/Networking.generated.swift | 12 +++++++++ .../Copiable/Models+Copiable.generated.swift | 23 +++++++++++++++- .../Shipments/WooShippingConfigResponse.swift | 25 +++++++++++++----- .../Shipments/WooShippingShipmentItem.swift | 26 +++++++++++++++++-- Modules/Sources/Yosemite/Model/Model.swift | 1 + .../Yosemite/Stores/WooShippingStore.swift | 6 ++--- .../OrderDetailsDataSource.swift | 19 +++++++------- .../Order Details/OrderDetailsViewModel.swift | 2 +- .../ShippingLabelSampleData.swift | 7 ++++- .../WooShippingSplitShipmentsViewModel.swift | 15 ++++++----- 10 files changed, 106 insertions(+), 30 deletions(-) diff --git a/Modules/Sources/Fakes/Networking.generated.swift b/Modules/Sources/Fakes/Networking.generated.swift index c0b2ee4f8c7..670ed23f3f7 100644 --- a/Modules/Sources/Fakes/Networking.generated.swift +++ b/Modules/Sources/Fakes/Networking.generated.swift @@ -2483,6 +2483,18 @@ extension Networking.WooShippingSelectedRate { ) } } +extension Networking.WooShippingShipment { + /// Returns a "ready to use" type filled with fake values. + /// + public static func fake() -> Networking.WooShippingShipment { + .init( + siteID: .fake(), + orderID: .fake(), + index: .fake(), + items: .fake() + ) + } +} extension Networking.WooShippingShipmentItem { /// Returns a "ready to use" type filled with fake values. /// diff --git a/Modules/Sources/Networking/Model/Copiable/Models+Copiable.generated.swift b/Modules/Sources/Networking/Model/Copiable/Models+Copiable.generated.swift index d1b14fea4d3..1d4afa5358d 100644 --- a/Modules/Sources/Networking/Model/Copiable/Models+Copiable.generated.swift +++ b/Modules/Sources/Networking/Model/Copiable/Models+Copiable.generated.swift @@ -3481,7 +3481,7 @@ extension Networking.WooShippingAddress { extension Networking.WooShippingConfig { public func copy( siteID: CopiableProp = .copy, - shipments: CopiableProp<[String: [WooShippingShipmentItem]]> = .copy, + shipments: CopiableProp<[WooShippingShipment]> = .copy, shippingLabelData: NullableCopiableProp = .copy ) -> Networking.WooShippingConfig { let siteID = siteID ?? self.siteID @@ -3724,6 +3724,27 @@ extension Networking.WooShippingPackagesResponse { } } +extension Networking.WooShippingShipment { + public func copy( + siteID: CopiableProp = .copy, + orderID: CopiableProp = .copy, + index: CopiableProp = .copy, + items: CopiableProp<[WooShippingShipmentItem]> = .copy + ) -> Networking.WooShippingShipment { + let siteID = siteID ?? self.siteID + let orderID = orderID ?? self.orderID + let index = index ?? self.index + let items = items ?? self.items + + return Networking.WooShippingShipment( + siteID: siteID, + orderID: orderID, + index: index, + items: items + ) + } +} + extension Networking.WooShippingShipmentItem { public func copy( id: CopiableProp = .copy, diff --git a/Modules/Sources/Networking/Model/ShippingLabel/Shipments/WooShippingConfigResponse.swift b/Modules/Sources/Networking/Model/ShippingLabel/Shipments/WooShippingConfigResponse.swift index 86e744471ed..cd9ebe27083 100644 --- a/Modules/Sources/Networking/Model/ShippingLabel/Shipments/WooShippingConfigResponse.swift +++ b/Modules/Sources/Networking/Model/ShippingLabel/Shipments/WooShippingConfigResponse.swift @@ -15,14 +15,14 @@ public struct WooShippingConfig: Decodable, Equatable, GeneratedFakeable, Genera /// The remote ID of the site that owns this shipping label config info. public let siteID: Int64 - /// Shipments of this order. The keys are the ids of the shipment. - public let shipments: WooShippingShipments + /// Shipments of this order. + public let shipments: [WooShippingShipment] /// Holds info about the shipping labels public let shippingLabelData: WooShippingLabelData? public init(siteID: Int64, - shipments: WooShippingShipments, + shipments: [WooShippingShipment], shippingLabelData: WooShippingLabelData?) { self.siteID = siteID self.shipments = shipments @@ -39,14 +39,26 @@ public struct WooShippingConfig: Decodable, Equatable, GeneratedFakeable, Genera throw WooShippingConfigDecodingError.missingSiteID } + guard let orderID = decoder.userInfo[.orderID] as? Int64 else { + throw WooShippingConfigDecodingError.missingOrderID + } + let container = try decoder.container(keyedBy: CodingKeys.self) - let shipments: WooShippingShipments = { + let shipments: [WooShippingShipment] = { guard let shipmentsString = try? container.decodeIfPresent(String.self, forKey: .shipments), let data = shipmentsString.data(using: .utf8) else { - return [:] + return [] } - return (try? JSONDecoder().decode(WooShippingShipments.self, from: data)) ?? [:] + guard let contents = (try? JSONDecoder().decode(WooShippingShipments.self, from: data)) else { + return [] + } + + var shipments = [WooShippingShipment]() + for (index, items) in contents { + shipments.append(WooShippingShipment(siteID: siteID, orderID: orderID, index: index, items: items)) + } + return shipments }() let shippingLabelData = try container.decodeIfPresent(WooShippingLabelData.self, forKey: .shippingLabelData) @@ -79,4 +91,5 @@ public struct WooShippingLabelData: Decodable, Equatable { // enum WooShippingConfigDecodingError: Error { case missingSiteID + case missingOrderID } diff --git a/Modules/Sources/Networking/Model/ShippingLabel/Shipments/WooShippingShipmentItem.swift b/Modules/Sources/Networking/Model/ShippingLabel/Shipments/WooShippingShipmentItem.swift index 979371562c6..45fe4cb0d69 100644 --- a/Modules/Sources/Networking/Model/ShippingLabel/Shipments/WooShippingShipmentItem.swift +++ b/Modules/Sources/Networking/Model/ShippingLabel/Shipments/WooShippingShipmentItem.swift @@ -1,8 +1,30 @@ import Foundation import Codegen -import WooFoundation -/// Represents a shipment in Shipping Labels for the WooCommerce Shipping extension. +/// Represents a shipment from the WooCommerce Shipping extension. +/// +public struct WooShippingShipment: Equatable, GeneratedFakeable, GeneratedCopiable { + /// ID of the site that the shipment belongs to. + public let siteID: Int64 + + /// ID of the order that the shipment belongs to. + public let orderID: Int64 + + /// Index of the shipment + public let index: String + + /// Contents of the shipment + public let items: [WooShippingShipmentItem] + + public init(siteID: Int64, orderID: Int64, index: String, items: [WooShippingShipmentItem]) { + self.siteID = siteID + self.orderID = orderID + self.index = index + self.items = items + } +} + +/// Represents a shipment item from the WooCommerce Shipping extension. /// public struct WooShippingShipmentItem: Codable, Equatable, GeneratedFakeable, GeneratedCopiable { /// ID of the shipment diff --git a/Modules/Sources/Yosemite/Model/Model.swift b/Modules/Sources/Yosemite/Model/Model.swift index 78506115603..f22184700c1 100644 --- a/Modules/Sources/Yosemite/Model/Model.swift +++ b/Modules/Sources/Yosemite/Model/Model.swift @@ -200,6 +200,7 @@ public typealias WooShippingDestinationAddressUpdate = Networking.WooShippingDes public typealias WooShippingConfig = Networking.WooShippingConfig public typealias WooShippingUpdateShipment = Networking.WooShippingUpdateShipment public typealias WooShippingShipmentItem = Networking.WooShippingShipmentItem +public typealias WooShippingShipment = Networking.WooShippingShipment public typealias WooShippingShipments = Networking.WooShippingShipments public typealias WooShippingSelectedRate = Networking.WooShippingSelectedRate public typealias WPComPlan = Networking.WPComPlan diff --git a/Modules/Sources/Yosemite/Stores/WooShippingStore.swift b/Modules/Sources/Yosemite/Stores/WooShippingStore.swift index 2e365272a06..2e8e1cd9695 100644 --- a/Modules/Sources/Yosemite/Stores/WooShippingStore.swift +++ b/Modules/Sources/Yosemite/Stores/WooShippingStore.swift @@ -4,14 +4,14 @@ import Storage public struct ShippingLabelSyncResult { public let labels: [ShippingLabel] - public let shipments: WooShippingShipments + public let shipments: [WooShippingShipment] - public init(labels: [ShippingLabel], shipments: WooShippingShipments) { + public init(labels: [ShippingLabel], shipments: [WooShippingShipment]) { self.labels = labels self.shipments = shipments } - public static let none = ShippingLabelSyncResult(labels: [], shipments: [:]) + public static let none = ShippingLabelSyncResult(labels: [], shipments: []) } /// Implements `WooShippingAction` actions diff --git a/WooCommerce/Classes/ViewModels/Order Details/OrderDetailsDataSource.swift b/WooCommerce/Classes/ViewModels/Order Details/OrderDetailsDataSource.swift index 932f955f9ad..e997c25e90a 100644 --- a/WooCommerce/Classes/ViewModels/Order Details/OrderDetailsDataSource.swift +++ b/WooCommerce/Classes/ViewModels/Order Details/OrderDetailsDataSource.swift @@ -1708,18 +1708,19 @@ extension OrderDetailsDataSource { let items: [ShippingLabelPackageItem] } - func populateShipments(labels: [ShippingLabel], shipments: WooShippingShipments) { + func populateShipments(labels: [ShippingLabel], shipments: [WooShippingShipment]) { let itemsDataSource = DefaultWooShippingItemsDataSource(order: order, storageManager: storageManager) let packageItems = itemsDataSource.items var contents = [Shipment]() - for key in shipments.keys.sorted(by: { $0.localizedStandardCompare($1) == .orderedAscending }) { - guard let shipmentItems = shipments[key] else { - continue - } + let sortedShipments = shipments.sorted(by: { + $0.index.localizedStandardCompare($1.index) == .orderedAscending + }) + for shipment in sortedShipments { + let index = shipment.index let label: ShippingLabel? = { let purchasedLabels = labels.filter { - $0.shipmentID == key && $0.status == .purchased + $0.shipmentID == index && $0.status == .purchased } let sortedLabels = purchasedLabels.sorted { $0.dateCreated > $1.dateCreated } if let completedLabel = sortedLabels.first(where: { $0.refund == nil }) { @@ -1730,7 +1731,7 @@ extension OrderDetailsDataSource { }() var shipmentContents: [ShippingLabelPackageItem] = [] - for shipmentItem in shipmentItems { + for shipmentItem in shipment.items { guard let packageItem = packageItems.first(where: { $0.orderItemID == shipmentItem.id }), let subItems = shipmentItem.subItems else { continue @@ -1741,8 +1742,8 @@ extension OrderDetailsDataSource { shipmentContents.append(updatedItem) } - let shipment = Shipment(index: key, shippingLabel: label, items: shipmentContents) - contents.append(shipment) + let shipmentWithLabel = Shipment(index: index, shippingLabel: label, items: shipmentContents) + contents.append(shipmentWithLabel) } self.wooShippingShipments = contents } diff --git a/WooCommerce/Classes/ViewModels/Order Details/OrderDetailsViewModel.swift b/WooCommerce/Classes/ViewModels/Order Details/OrderDetailsViewModel.swift index c279df00683..355249ab38e 100644 --- a/WooCommerce/Classes/ViewModels/Order Details/OrderDetailsViewModel.swift +++ b/WooCommerce/Classes/ViewModels/Order Details/OrderDetailsViewModel.swift @@ -1014,7 +1014,7 @@ private extension OrderDetailsViewModel { switch result { case .success(let labels): self?.trackShippingLabelSyncingResult(result: .success, isRevampedFlow: isRevampedFlow) - continuation.resume(returning: ShippingLabelSyncResult(labels: labels, shipments: [:])) + continuation.resume(returning: ShippingLabelSyncResult(labels: labels, shipments: [])) case .failure(let error): self?.trackShippingLabelSyncingResult(result: .failed(error: error), isRevampedFlow: isRevampedFlow) continuation.resume(returning: .none) diff --git a/WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/Create Shipping Label Form/Package Details/ShippingLabelSampleData.swift b/WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/Create Shipping Label Form/Package Details/ShippingLabelSampleData.swift index ae0dc6e8917..213604ccb86 100644 --- a/WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/Create Shipping Label Form/Package Details/ShippingLabelSampleData.swift +++ b/WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/Create Shipping Label Form/Package Details/ShippingLabelSampleData.swift @@ -57,7 +57,12 @@ enum ShippingLabelSampleData { static func sampleWooShippingConfig() -> WooShippingConfig { WooShippingConfig(siteID: 123, - shipments: ["0": [sampleWooShippingShipmentItem()]], + shipments: [WooShippingShipment( + siteID: 1, + orderID: 2, + index: "0", + items: [sampleWooShippingShipmentItem()] + )], shippingLabelData: nil) } } diff --git a/WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/WooShipping Create Shipping Labels/WooShipping Split Shipments/WooShippingSplitShipmentsViewModel.swift b/WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/WooShipping Create Shipping Labels/WooShipping Split Shipments/WooShippingSplitShipmentsViewModel.swift index 5e4feaa198e..d14eca0dc05 100644 --- a/WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/WooShipping Create Shipping Labels/WooShipping Split Shipments/WooShippingSplitShipmentsViewModel.swift +++ b/WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/WooShipping Create Shipping Labels/WooShipping Split Shipments/WooShippingSplitShipmentsViewModel.swift @@ -545,18 +545,19 @@ extension WooShippingSplitShipmentsViewModel { let currentOrderLabels = config.shippingLabelData?.currentOrderLabels ?? [] var shipments = [Shipment]() - for key in config.shipments.keys.sorted() { - guard let shipmentItems = config.shipments[key] else { - continue - } + let sortedShipments = config.shipments.sorted(by: { + $0.index.localizedStandardCompare($1.index) == .orderedAscending + }) + for shipment in sortedShipments { + let index = shipment.index let purchasedLabel = currentOrderLabels - .first(where: { $0.shipmentID == key && $0.status == .purchased && $0.refund == nil }) + .first(where: { $0.shipmentID == index && $0.status == .purchased && $0.refund == nil }) let isPurchased = purchasedLabel != nil var shipmentContents = ShipmentContents() - for shipmentItem in shipmentItems { + for shipmentItem in shipment.items { guard let packageItem = packageItems.first(where: { $0.orderItemID == shipmentItem.id }), let subItems = shipmentItem.subItems else { continue @@ -570,7 +571,7 @@ extension WooShippingSplitShipmentsViewModel { shipmentContents.append(content) } - let shipment = Shipment(index: Int(key) ?? 0, + let shipment = Shipment(index: Int(index) ?? 0, contents: shipmentContents, purchasedLabelID: purchasedLabel?.shippingLabelID, currency: currency, From a3b1982dcf1bfd8ac2fc2bff052b6b3c60044cdf Mon Sep 17 00:00:00 2001 From: Huong Do Date: Thu, 10 Jul 2025 12:10:46 +0700 Subject: [PATCH 06/13] Add shipping label to WooShippingShipment --- .../Sources/Fakes/Networking.generated.swift | 3 ++- .../Copiable/Models+Copiable.generated.swift | 7 +++++-- .../Shipments/WooShippingConfigResponse.swift | 21 +++++++++++++++++-- .../Shipments/WooShippingShipmentItem.swift | 10 ++++++++- .../ShippingLabelSampleData.swift | 3 ++- 5 files changed, 37 insertions(+), 7 deletions(-) diff --git a/Modules/Sources/Fakes/Networking.generated.swift b/Modules/Sources/Fakes/Networking.generated.swift index 670ed23f3f7..1d5f76ca017 100644 --- a/Modules/Sources/Fakes/Networking.generated.swift +++ b/Modules/Sources/Fakes/Networking.generated.swift @@ -2491,7 +2491,8 @@ extension Networking.WooShippingShipment { siteID: .fake(), orderID: .fake(), index: .fake(), - items: .fake() + items: .fake(), + shippingLabel: .fake() ) } } diff --git a/Modules/Sources/Networking/Model/Copiable/Models+Copiable.generated.swift b/Modules/Sources/Networking/Model/Copiable/Models+Copiable.generated.swift index 1d4afa5358d..3d21b437725 100644 --- a/Modules/Sources/Networking/Model/Copiable/Models+Copiable.generated.swift +++ b/Modules/Sources/Networking/Model/Copiable/Models+Copiable.generated.swift @@ -3729,18 +3729,21 @@ extension Networking.WooShippingShipment { siteID: CopiableProp = .copy, orderID: CopiableProp = .copy, index: CopiableProp = .copy, - items: CopiableProp<[WooShippingShipmentItem]> = .copy + items: CopiableProp<[WooShippingShipmentItem]> = .copy, + shippingLabel: NullableCopiableProp = .copy ) -> Networking.WooShippingShipment { let siteID = siteID ?? self.siteID let orderID = orderID ?? self.orderID let index = index ?? self.index let items = items ?? self.items + let shippingLabel = shippingLabel ?? self.shippingLabel return Networking.WooShippingShipment( siteID: siteID, orderID: orderID, index: index, - items: items + items: items, + shippingLabel: shippingLabel ) } } diff --git a/Modules/Sources/Networking/Model/ShippingLabel/Shipments/WooShippingConfigResponse.swift b/Modules/Sources/Networking/Model/ShippingLabel/Shipments/WooShippingConfigResponse.swift index cd9ebe27083..e1344acacef 100644 --- a/Modules/Sources/Networking/Model/ShippingLabel/Shipments/WooShippingConfigResponse.swift +++ b/Modules/Sources/Networking/Model/ShippingLabel/Shipments/WooShippingConfigResponse.swift @@ -44,6 +44,8 @@ public struct WooShippingConfig: Decodable, Equatable, GeneratedFakeable, Genera } let container = try decoder.container(keyedBy: CodingKeys.self) + let shippingLabelData = try container.decodeIfPresent(WooShippingLabelData.self, forKey: .shippingLabelData) + let shipments: [WooShippingShipment] = { guard let shipmentsString = try? container.decodeIfPresent(String.self, forKey: .shipments), let data = shipmentsString.data(using: .utf8) else { @@ -54,14 +56,29 @@ public struct WooShippingConfig: Decodable, Equatable, GeneratedFakeable, Genera return [] } + let labels = shippingLabelData?.currentOrderLabels ?? [] var shipments = [WooShippingShipment]() for (index, items) in contents { - shipments.append(WooShippingShipment(siteID: siteID, orderID: orderID, index: index, items: items)) + let label: ShippingLabel? = { + let purchasedLabels = labels.filter { + $0.shipmentID == index && $0.status == .purchased + } + let sortedLabels = purchasedLabels.sorted { $0.dateCreated > $1.dateCreated } + if let completedLabel = sortedLabels.first(where: { $0.refund == nil }) { + return completedLabel + } else { + return sortedLabels.first + } + }() + shipments.append(WooShippingShipment(siteID: siteID, + orderID: orderID, + index: index, + items: items, + shippingLabel: label)) } return shipments }() - let shippingLabelData = try container.decodeIfPresent(WooShippingLabelData.self, forKey: .shippingLabelData) self.init(siteID: siteID, shipments: shipments, shippingLabelData: shippingLabelData) diff --git a/Modules/Sources/Networking/Model/ShippingLabel/Shipments/WooShippingShipmentItem.swift b/Modules/Sources/Networking/Model/ShippingLabel/Shipments/WooShippingShipmentItem.swift index 45fe4cb0d69..a57f71ef962 100644 --- a/Modules/Sources/Networking/Model/ShippingLabel/Shipments/WooShippingShipmentItem.swift +++ b/Modules/Sources/Networking/Model/ShippingLabel/Shipments/WooShippingShipmentItem.swift @@ -16,11 +16,19 @@ public struct WooShippingShipment: Equatable, GeneratedFakeable, GeneratedCopiab /// Contents of the shipment public let items: [WooShippingShipmentItem] - public init(siteID: Int64, orderID: Int64, index: String, items: [WooShippingShipmentItem]) { + /// The latest label purchased for the shipment + public let shippingLabel: ShippingLabel? + + public init(siteID: Int64, + orderID: Int64, + index: String, + items: [WooShippingShipmentItem], + shippingLabel: ShippingLabel?) { self.siteID = siteID self.orderID = orderID self.index = index self.items = items + self.shippingLabel = shippingLabel } } diff --git a/WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/Create Shipping Label Form/Package Details/ShippingLabelSampleData.swift b/WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/Create Shipping Label Form/Package Details/ShippingLabelSampleData.swift index 213604ccb86..b63e9ef0a35 100644 --- a/WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/Create Shipping Label Form/Package Details/ShippingLabelSampleData.swift +++ b/WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/Create Shipping Label Form/Package Details/ShippingLabelSampleData.swift @@ -61,7 +61,8 @@ enum ShippingLabelSampleData { siteID: 1, orderID: 2, index: "0", - items: [sampleWooShippingShipmentItem()] + items: [sampleWooShippingShipmentItem()], + shippingLabel: nil, )], shippingLabelData: nil) } From 9220066a7d687a86c9ab2ef23cbc63af2ee81efb Mon Sep 17 00:00:00 2001 From: Huong Do Date: Thu, 10 Jul 2025 12:51:48 +0700 Subject: [PATCH 07/13] Revert changes to wooshippingaction and store --- .../Yosemite/Actions/WooShippingAction.swift | 2 +- .../Yosemite/Stores/WooShippingStore.swift | 19 +- .../Stores/WooShippingStoreTests.swift | 10 +- .../OrderDetailsDataSource.swift | 53 ----- .../Order Details/OrderDetailsViewModel.swift | 48 ++-- .../OrderDetailsViewModelTests.swift | 4 +- .../OrderDetailsDataSourceTests.swift | 213 ------------------ 7 files changed, 26 insertions(+), 323 deletions(-) diff --git a/Modules/Sources/Yosemite/Actions/WooShippingAction.swift b/Modules/Sources/Yosemite/Actions/WooShippingAction.swift index ce37a8dead7..5e51861785a 100644 --- a/Modules/Sources/Yosemite/Actions/WooShippingAction.swift +++ b/Modules/Sources/Yosemite/Actions/WooShippingAction.swift @@ -111,7 +111,7 @@ public enum WooShippingAction: Action { /// case syncShippingLabels(siteID: Int64, orderID: Int64, - completion: (Result) -> Void) + completion: (Result<[ShippingLabel], Error>) -> Void) /// Updates shipments for given order /// diff --git a/Modules/Sources/Yosemite/Stores/WooShippingStore.swift b/Modules/Sources/Yosemite/Stores/WooShippingStore.swift index 2e8e1cd9695..8dc2bfc4e94 100644 --- a/Modules/Sources/Yosemite/Stores/WooShippingStore.swift +++ b/Modules/Sources/Yosemite/Stores/WooShippingStore.swift @@ -2,18 +2,6 @@ import Foundation import Networking import Storage -public struct ShippingLabelSyncResult { - public let labels: [ShippingLabel] - public let shipments: [WooShippingShipment] - - public init(labels: [ShippingLabel], shipments: [WooShippingShipment]) { - self.labels = labels - self.shipments = shipments - } - - public static let none = ShippingLabelSyncResult(labels: [], shipments: []) -} - /// Implements `WooShippingAction` actions /// public final class WooShippingStore: Store { @@ -304,7 +292,7 @@ private extension WooShippingStore { func syncShippingLabels(siteID: Int64, orderID: Int64, - completion: @escaping (Result) -> Void) { + completion: @escaping (Result<[ShippingLabel], Error>) -> Void) { remote.loadConfig(siteID: siteID, orderID: orderID, completion: { [weak self] result in guard let self else { return } @@ -312,14 +300,13 @@ private extension WooShippingStore { case .failure(let error): completion(.failure(error)) case .success(let config): - let shipments = config.shipments guard let labels = config.shippingLabelData?.currentOrderLabels else { - return completion(.success(ShippingLabelSyncResult(labels: [], shipments: shipments))) + return completion(.success([])) } upsertShippingLabelsInBackground(siteID: siteID, orderID: orderID, shippingLabels: labels) { - completion(.success(ShippingLabelSyncResult(labels: labels, shipments: shipments))) + completion(.success(labels)) } } }) diff --git a/Modules/Tests/YosemiteTests/Stores/WooShippingStoreTests.swift b/Modules/Tests/YosemiteTests/Stores/WooShippingStoreTests.swift index fa605cbbd24..e7f3493aa8d 100644 --- a/Modules/Tests/YosemiteTests/Stores/WooShippingStoreTests.swift +++ b/Modules/Tests/YosemiteTests/Stores/WooShippingStoreTests.swift @@ -996,7 +996,7 @@ final class WooShippingStoreTests: XCTestCase { func test_loadConfig_returns_success_response() throws { // Given let remote = MockWooShippingRemote() - let expectedConfig = WooShippingConfig.fake().copy(shipments: ["0": [WooShippingShipmentItem.fake()]]) + let expectedConfig = WooShippingConfig.fake().copy(shipments: [WooShippingShipment.fake()]) remote.whenLoadingConfig(siteID: sampleSiteID, thenReturn: .success(expectedConfig)) let store = WooShippingStore(dispatcher: dispatcher, storageManager: storageManager, network: network, remote: remote) @@ -1083,9 +1083,8 @@ final class WooShippingStoreTests: XCTestCase { usedDate: nil, expiryDate: nil) }() - let shipments = ["0": [WooShippingShipmentItem.fake()]] let expectedResponse = WooShippingConfig.fake().copy( - shipments: shipments, + shipments: [WooShippingShipment.fake()], shippingLabelData: WooShippingLabelData(currentOrderLabels: [expectedShippingLabel]) ) remote.whenLoadingConfig(siteID: sampleSiteID, thenReturn: .success(expectedResponse)) @@ -1094,7 +1093,7 @@ final class WooShippingStoreTests: XCTestCase { insertOrder(siteID: sampleSiteID, orderID: orderID) // When - let result: Result = waitFor { promise in + let result: Result<[Yosemite.ShippingLabel], Error> = waitFor { promise in let action = WooShippingAction.syncShippingLabels(siteID: self.sampleSiteID, orderID: orderID) { result in promise(result) } @@ -1103,9 +1102,6 @@ final class WooShippingStoreTests: XCTestCase { // Then XCTAssertTrue(result.isSuccess) - let syncResult = try XCTUnwrap(result.get()) - XCTAssertEqual(syncResult.labels, [expectedShippingLabel]) - XCTAssertEqual(syncResult.shipments, shipments) let persistedOrder = try XCTUnwrap(viewStorage.loadOrder(siteID: sampleSiteID, orderID: orderID)) let persistedShippingLabels = try XCTUnwrap(viewStorage.loadAllShippingLabels(siteID: sampleSiteID, orderID: orderID)) diff --git a/WooCommerce/Classes/ViewModels/Order Details/OrderDetailsDataSource.swift b/WooCommerce/Classes/ViewModels/Order Details/OrderDetailsDataSource.swift index e997c25e90a..63d0e786621 100644 --- a/WooCommerce/Classes/ViewModels/Order Details/OrderDetailsDataSource.swift +++ b/WooCommerce/Classes/ViewModels/Order Details/OrderDetailsDataSource.swift @@ -61,10 +61,6 @@ final class OrderDetailsDataSource: NSObject { return true } - /// Shipments created for the order - /// - private(set) var wooShippingShipments: [Shipment] = [] - /// Whether the button to create shipping labels should be visible. /// var shouldShowShippingLabelCreation: Bool { @@ -1700,55 +1696,6 @@ extension OrderDetailsDataSource { } } -// MARK: - Woo Shipping Shipments -extension OrderDetailsDataSource { - struct Shipment { - let index: String - let shippingLabel: ShippingLabel? - let items: [ShippingLabelPackageItem] - } - - func populateShipments(labels: [ShippingLabel], shipments: [WooShippingShipment]) { - let itemsDataSource = DefaultWooShippingItemsDataSource(order: order, storageManager: storageManager) - let packageItems = itemsDataSource.items - var contents = [Shipment]() - - let sortedShipments = shipments.sorted(by: { - $0.index.localizedStandardCompare($1.index) == .orderedAscending - }) - for shipment in sortedShipments { - let index = shipment.index - let label: ShippingLabel? = { - let purchasedLabels = labels.filter { - $0.shipmentID == index && $0.status == .purchased - } - let sortedLabels = purchasedLabels.sorted { $0.dateCreated > $1.dateCreated } - if let completedLabel = sortedLabels.first(where: { $0.refund == nil }) { - return completedLabel - } else { - return sortedLabels.first - } - }() - - var shipmentContents: [ShippingLabelPackageItem] = [] - for shipmentItem in shipment.items { - guard let packageItem = packageItems.first(where: { $0.orderItemID == shipmentItem.id }), - let subItems = shipmentItem.subItems else { - continue - } - - let quantity = subItems.count > 0 ? subItems.count : 1 - let updatedItem = ShippingLabelPackageItem(copy: packageItem, quantity: Decimal(quantity)) - shipmentContents.append(updatedItem) - } - - let shipmentWithLabel = Shipment(index: index, shippingLabel: label, items: shipmentContents) - contents.append(shipmentWithLabel) - } - self.wooShippingShipments = contents - } -} - // MARK: - Constants extension OrderDetailsDataSource { enum Localization { diff --git a/WooCommerce/Classes/ViewModels/Order Details/OrderDetailsViewModel.swift b/WooCommerce/Classes/ViewModels/Order Details/OrderDetailsViewModel.swift index 355249ab38e..d96aeb633f7 100644 --- a/WooCommerce/Classes/ViewModels/Order Details/OrderDetailsViewModel.swift +++ b/WooCommerce/Classes/ViewModels/Order Details/OrderDetailsViewModel.swift @@ -283,12 +283,9 @@ extension OrderDetailsViewModel { taskGroup.addTask { @MainActor [weak self] in guard let self else { return } // Sync shipping labels and update order with the result if available - let syncResult = await syncShippingLabels() - dataSource.populateShipments(labels: syncResult.labels, - shipments: syncResult.shipments) - + let shippingLabels = await syncShippingLabels() // Update the order with the newly synced shipping labels - let updatedOrder = order.copy(shippingLabels: syncResult.labels) + let updatedOrder = order.copy(shippingLabels: shippingLabels) update(order: updatedOrder) } } @@ -706,18 +703,18 @@ extension OrderDetailsViewModel { } @discardableResult - @MainActor func syncShippingLabels() async -> ShippingLabelSyncResult { + @MainActor func syncShippingLabels() async -> [ShippingLabel] { let isRevampedFlow = featureFlagService.isFeatureFlagEnabled(.revampedShippingLabelCreation) guard isRevampedFlow else { /// old logic for syncing labels if await localRequirementsForShippingLabelsAreFulfilled() { return await syncShippingLabelsForLegacyPlugin(isRevampedFlow: isRevampedFlow) } - return .none + return [] } guard !orderContainsOnlyVirtualProducts else { - return .none + return [] } if await isPluginActive(pluginPath: SitePlugin.SupportedPluginPath.WooShipping) { @@ -725,7 +722,7 @@ extension OrderDetailsViewModel { } else if await isPluginActive(pluginPath: SitePlugin.SupportedPluginPath.LegacyWCShip) { return await syncShippingLabelsForLegacyPlugin(isRevampedFlow: isRevampedFlow) } else { - return .none + return [] } } @@ -993,45 +990,33 @@ private extension OrderDetailsViewModel { } } - @MainActor func syncShippingLabelsForWooShipping() async -> ShippingLabelSyncResult { + @MainActor func syncShippingLabelsForWooShipping() async -> [ShippingLabel] { await withCheckedContinuation { continuation in stores.dispatch(WooShippingAction.syncShippingLabels(siteID: order.siteID, orderID: order.orderID) { [weak self] result in - switch result { - case let .success(result): - self?.trackShippingLabelSyncingResult(result: .success, isRevampedFlow: true) - continuation.resume(returning: result) - case let .failure(error): - self?.trackShippingLabelSyncingResult(result: .failed(error: error), isRevampedFlow: true) - continuation.resume(returning: .none) - } + let labels = self?.handleShippingLabelSyncingResult(result: result, isRevampedFlow: true) ?? [] + continuation.resume(returning: labels) }) } } - @MainActor func syncShippingLabelsForLegacyPlugin(isRevampedFlow: Bool) async -> ShippingLabelSyncResult { + @MainActor func syncShippingLabelsForLegacyPlugin(isRevampedFlow: Bool) async -> [ShippingLabel] { await withCheckedContinuation { continuation in stores.dispatch(ShippingLabelAction.synchronizeShippingLabels(siteID: order.siteID, orderID: order.orderID) { [weak self] result in - switch result { - case .success(let labels): - self?.trackShippingLabelSyncingResult(result: .success, isRevampedFlow: isRevampedFlow) - continuation.resume(returning: ShippingLabelSyncResult(labels: labels, shipments: [])) - case .failure(let error): - self?.trackShippingLabelSyncingResult(result: .failed(error: error), isRevampedFlow: isRevampedFlow) - continuation.resume(returning: .none) - } + let labels = self?.handleShippingLabelSyncingResult(result: result, isRevampedFlow: isRevampedFlow) ?? [] + continuation.resume(returning: labels) }) } } - func trackShippingLabelSyncingResult(result: WooAnalyticsEvent.ShippingLabelsAPIRequestResult, - isRevampedFlow: Bool) { + func handleShippingLabelSyncingResult(result: Result<[ShippingLabel], Error>, isRevampedFlow: Bool) -> [ShippingLabel] { switch result { - case .success: + case .success(let shippingLabels): ServiceLocator.analytics.track(event: .shippingLabelsAPIRequest( result: .success, isRevampedFlow: isRevampedFlow )) - case .failed(let error): + return shippingLabels + case .failure(let error): ServiceLocator.analytics.track(event: .shippingLabelsAPIRequest( result: .failed(error: error), isRevampedFlow: isRevampedFlow @@ -1041,6 +1026,7 @@ private extension OrderDetailsViewModel { } else { DDLogError("⛔️ Error synchronizing shipping labels: \(error)") } + return [] } } diff --git a/WooCommerce/WooCommerceTests/ViewRelated/OrderDetailsViewModelTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/OrderDetailsViewModelTests.swift index aca38858f3c..a94cfacabdd 100644 --- a/WooCommerce/WooCommerceTests/ViewRelated/OrderDetailsViewModelTests.swift +++ b/WooCommerce/WooCommerceTests/ViewRelated/OrderDetailsViewModelTests.swift @@ -152,7 +152,7 @@ final class OrderDetailsViewModelTests: XCTestCase { let plugin = insertSystemPlugin(path: SitePlugin.SupportedPluginPath.WooShipping, siteID: order.siteID, isActive: true) whenFetchingSystemPlugin(path: SitePlugin.SupportedPluginPath.WooShipping, thenReturn: plugin) - whenSyncingShippingLabels(thenReturn: .success(ShippingLabelSyncResult.none)) + whenSyncingShippingLabels(thenReturn: .success([])) let featureFlagService = MockFeatureFlagService(revampedShippingLabelCreation: true) let viewModel = OrderDetailsViewModel(order: order, @@ -789,7 +789,7 @@ private extension OrderDetailsViewModelTests { } } - func whenSyncingShippingLabels(thenReturn result: Result) { + func whenSyncingShippingLabels(thenReturn result: Result<[ShippingLabel], Error>) { storesManager.whenReceivingAction(ofType: WooShippingAction.self) { action in switch action { case let .syncShippingLabels(_, _, completion): diff --git a/WooCommerce/WooCommerceTests/ViewRelated/Orders/Order Details/OrderDetailsDataSourceTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/Orders/Order Details/OrderDetailsDataSourceTests.swift index 93c679dd10a..99d5ea530f9 100644 --- a/WooCommerce/WooCommerceTests/ViewRelated/Orders/Order Details/OrderDetailsDataSourceTests.swift +++ b/WooCommerce/WooCommerceTests/ViewRelated/Orders/Order Details/OrderDetailsDataSourceTests.swift @@ -1122,213 +1122,6 @@ final class OrderDetailsDataSourceTests: XCTestCase { XCTAssertNil(seeReceiptRow) XCTAssertNil(seeLegacyReceiptRow) } - - // MARK: - wooShippingShipments Tests - - func test_wooShippingShipments_is_initially_empty() { - // Given - let order = Order.fake() - let dataSource = OrderDetailsDataSource(order: order, - storageManager: storageManager, - cardPresentPaymentsConfiguration: Mocks.configuration, - receiptEligibilityUseCase: MockReceiptEligibilityUseCase()) - - // Then - XCTAssertTrue(dataSource.wooShippingShipments.isEmpty) - } - - func test_populateShipments_with_empty_data_results_in_empty_shipments() { - // Given - let order = Order.fake() - let dataSource = OrderDetailsDataSource(order: order, - storageManager: storageManager, - cardPresentPaymentsConfiguration: Mocks.configuration, - receiptEligibilityUseCase: MockReceiptEligibilityUseCase()) - let labels: [ShippingLabel] = [] - let shipments: WooShippingShipments = [:] - - // When - dataSource.populateShipments(labels: labels, shipments: shipments) - - // Then - XCTAssertTrue(dataSource.wooShippingShipments.isEmpty) - } - - func test_populateShipments_with_valid_data_creates_shipments() { - // Given - let order = makeOrder() - let dataSource = OrderDetailsDataSource(order: order, - storageManager: storageManager, - cardPresentPaymentsConfiguration: Mocks.configuration, - receiptEligibilityUseCase: MockReceiptEligibilityUseCase()) - - let shippingLabel1 = ShippingLabel.fake().copy(siteID: order.siteID, - orderID: order.orderID, - shipmentID: "1", - dateCreated: Date(), status: .purchased) - let shippingLabel2 = ShippingLabel.fake().copy(siteID: order.siteID, - orderID: order.orderID, - shipmentID: "0", - dateCreated: Date(timeIntervalSinceNow: -3600), status: .purchased) - let labels = [shippingLabel1, shippingLabel2] - - let orderItem0 = order.items[0] - let orderItem1 = order.items[1] - storageManager.insertSampleProduct(readOnlyProduct: Product.fake().copy(siteID: order.siteID, productID: orderItem0.productID)) - storageManager.insertSampleProduct(readOnlyProduct: Product.fake().copy(siteID: order.siteID, productID: orderItem1.productID)) - - let shipmentItem1 = WooShippingShipmentItem.fake().copy(id: orderItem0.itemID, subItems: ["sub1", "sub2"]) - let shipmentItem2 = WooShippingShipmentItem.fake().copy(id: orderItem1.itemID, subItems: ["sub3"]) - let shipments: WooShippingShipments = [ - "0": [shipmentItem1], - "1": [shipmentItem2] - ] - - // When - dataSource.populateShipments(labels: labels, shipments: shipments) - - // Then - XCTAssertEqual(dataSource.wooShippingShipments.count, 2) - - // First shipment ("0" comes before "1" in sorted order) - let firstShipment = dataSource.wooShippingShipments[0] - XCTAssertEqual(firstShipment.index, "0") - XCTAssertEqual(firstShipment.shippingLabel, shippingLabel2) - XCTAssertEqual(firstShipment.items.count, 1) - XCTAssertEqual(firstShipment.items[0].quantity, 2) // 2 subItems - - // Second shipment - let secondShipment = dataSource.wooShippingShipments[1] - XCTAssertEqual(secondShipment.index, "1") - XCTAssertEqual(secondShipment.shippingLabel, shippingLabel1) - XCTAssertEqual(secondShipment.items.count, 1) - XCTAssertEqual(secondShipment.items[0].quantity, 1) // 1 subItem - } - - func test_populateShipments_with_no_matching_shipping_labels_creates_shipments_without_labels() { - // Given - let order = makeOrder() - let dataSource = OrderDetailsDataSource(order: order, - storageManager: storageManager, - cardPresentPaymentsConfiguration: Mocks.configuration, - receiptEligibilityUseCase: MockReceiptEligibilityUseCase()) - - let labels: [ShippingLabel] = [] - let shipmentItem1 = WooShippingShipmentItem.fake().copy(id: order.items[0].itemID, subItems: ["sub1"]) - let shipments: WooShippingShipments = [ - "0": [shipmentItem1] - ] - - // When - dataSource.populateShipments(labels: labels, shipments: shipments) - - // Then - XCTAssertEqual(dataSource.wooShippingShipments.count, 1) - - let shipment = dataSource.wooShippingShipments[0] - XCTAssertEqual(shipment.index, "0") - XCTAssertNil(shipment.shippingLabel) - } - - func test_populateShipments_prioritizes_non_refunded_labels() { - // Given - let order = makeOrder() - let dataSource = OrderDetailsDataSource(order: order, - storageManager: storageManager, - cardPresentPaymentsConfiguration: Mocks.configuration, - receiptEligibilityUseCase: MockReceiptEligibilityUseCase()) - - let refundedLabel = ShippingLabel.fake().copy(siteID: order.siteID, - orderID: order.orderID, - shipmentID: "0", - dateCreated: Date(), status: .purchased, - refund: ShippingLabelRefund.fake()) - let nonRefundedLabel = ShippingLabel.fake().copy(siteID: order.siteID, - orderID: order.orderID, - shipmentID: "0", - dateCreated: Date(timeIntervalSinceNow: -3600), status: .purchased, - refund: nil) - let labels = [refundedLabel, nonRefundedLabel] - - let shipmentItem1 = WooShippingShipmentItem.fake().copy(id: order.items[0].itemID, subItems: ["sub1"]) - let shipments: WooShippingShipments = [ - "0": [shipmentItem1] - ] - - // When - dataSource.populateShipments(labels: labels, shipments: shipments) - - // Then - XCTAssertEqual(dataSource.wooShippingShipments.count, 1) - - let shipment = dataSource.wooShippingShipments[0] - XCTAssertEqual(shipment.index, "0") - XCTAssertEqual(shipment.shippingLabel, nonRefundedLabel) // Should prioritize non-refunded - } - - func test_populateShipments_uses_most_recent_label_when_all_refunded() { - // Given - let order = makeOrder() - let dataSource = OrderDetailsDataSource(order: order, - storageManager: storageManager, - cardPresentPaymentsConfiguration: Mocks.configuration, - receiptEligibilityUseCase: MockReceiptEligibilityUseCase()) - - let olderRefundedLabel = ShippingLabel.fake().copy(siteID: order.siteID, - orderID: order.orderID, - shipmentID: "0", - dateCreated: Date(timeIntervalSinceNow: -3600), status: .purchased, - refund: ShippingLabelRefund.fake()) - let newerRefundedLabel = ShippingLabel.fake().copy(siteID: order.siteID, - orderID: order.orderID, - shipmentID: "0", - dateCreated: Date(), status: .purchased, - refund: ShippingLabelRefund.fake()) - let labels = [olderRefundedLabel, newerRefundedLabel] - - let shipmentItem1 = WooShippingShipmentItem.fake().copy(id: order.items[0].itemID, subItems: []) - let shipments: WooShippingShipments = [ - "0": [shipmentItem1] - ] - - // When - dataSource.populateShipments(labels: labels, shipments: shipments) - - // Then - XCTAssertEqual(dataSource.wooShippingShipments.count, 1) - - let shipment = dataSource.wooShippingShipments[0] - XCTAssertEqual(shipment.index, "0") - XCTAssertEqual(shipment.shippingLabel, newerRefundedLabel) // Should use newer refunded label - } - - func test_populateShipments_handles_shipments_with_no_subitems() { - // Given - let order = makeOrder() - let dataSource = OrderDetailsDataSource(order: order, - storageManager: storageManager, - cardPresentPaymentsConfiguration: Mocks.configuration, - receiptEligibilityUseCase: MockReceiptEligibilityUseCase()) - - let labels: [ShippingLabel] = [] - let orderItem = order.items[0] - storageManager.insertSampleProduct(readOnlyProduct: Product.fake().copy(siteID: order.siteID, productID: orderItem.productID)) - let shipmentItemNoSubItems = WooShippingShipmentItem.fake().copy(id: orderItem.itemID, subItems: []) - let shipments: WooShippingShipments = [ - "0": [shipmentItemNoSubItems] - ] - - // When - dataSource.populateShipments(labels: labels, shipments: shipments) - - // Then - XCTAssertEqual(dataSource.wooShippingShipments.count, 1) - - let shipment = dataSource.wooShippingShipments[0] - XCTAssertEqual(shipment.index, "0") - XCTAssertEqual(shipment.items.count, 1) - XCTAssertEqual(shipment.items[0].quantity, 1) // Should default to 1 when no subItems - } } // MARK: - Test Data @@ -1338,12 +1131,6 @@ private extension OrderDetailsDataSourceTests { MockOrders().makeOrder(items: [makeOrderItem(), makeOrderItem()], fees: [OrderFeeLine.fake()]) } - func makeOrder(items: [OrderItem] = [], fees: [OrderFeeLine] = []) -> Order { - let defaultItems = items.isEmpty ? [makeOrderItem(), makeOrderItem()] : items - let defaultFees = fees.isEmpty ? [OrderFeeLine.fake()] : fees - return MockOrders().makeOrder(items: defaultItems, fees: defaultFees) - } - func makeOrderItem() -> OrderItem { OrderItem(itemID: 1, name: "Order Item Name", From 6ca503f7695331a4e53978474362c605b2409050 Mon Sep 17 00:00:00 2001 From: Huong Do Date: Thu, 10 Jul 2025 13:28:05 +0700 Subject: [PATCH 08/13] Update unit tests --- .../Mapper/WooShippingConfigMapperTests.swift | 31 +++- ...ing-label-config-success-without-data.json | 51 ++++++- .../shipping-label-config-success.json | 51 ++++++- .../WooShippingSplitShipmentsViewModel.swift | 56 +++----- ...ShippingSplitShipmentsViewModelTests.swift | 132 ++++++++---------- ...ooShippingCreateLabelsViewModelTests.swift | 12 +- 6 files changed, 219 insertions(+), 114 deletions(-) diff --git a/Modules/Tests/NetworkingTests/Mapper/WooShippingConfigMapperTests.swift b/Modules/Tests/NetworkingTests/Mapper/WooShippingConfigMapperTests.swift index ea47e994bbf..ea6d195169a 100644 --- a/Modules/Tests/NetworkingTests/Mapper/WooShippingConfigMapperTests.swift +++ b/Modules/Tests/NetworkingTests/Mapper/WooShippingConfigMapperTests.swift @@ -15,8 +15,21 @@ final class WooShippingConfigMapperTests: XCTestCase { } XCTAssertEqual(config.shipments.count, 3) + let shipment0 = try XCTUnwrap(config.shipments.first(where: { $0.index == "0" })) + XCTAssertNil(shipment0.shippingLabel) // non-purchased status means no label found + XCTAssertEqual(shipment0.items.count, 1) + + let shipment1 = try XCTUnwrap(config.shipments.first(where: { $0.index == "1" })) + XCTAssertEqual(shipment1.shippingLabel?.shippingLabelID, 4871) + XCTAssertEqual(shipment1.items.count, 1) + + let shipment2 = try XCTUnwrap(config.shipments.first(where: { $0.index == "2" })) + XCTAssertEqual(shipment2.shippingLabel?.shippingLabelID, 4873) + XCTAssertNotNil(shipment2.shippingLabel?.refund) + XCTAssertEqual(shipment2.items.count, 1) + let shippingLabelData = try XCTUnwrap(config.shippingLabelData?.currentOrderLabels) - XCTAssertEqual(shippingLabelData.count, 1) + XCTAssertEqual(shippingLabelData.count, 3) XCTAssertEqual(shippingLabelData.first?.shipmentID, "1") XCTAssertEqual(shippingLabelData.first?.shippingLabelID, 4871) } @@ -28,8 +41,22 @@ final class WooShippingConfigMapperTests: XCTestCase { } XCTAssertEqual(config.shipments.count, 3) + let shipment0 = try XCTUnwrap(config.shipments.first(where: { $0.index == "0" })) + XCTAssertNil(shipment0.shippingLabel) // non-purchased status means no label found + XCTAssertEqual(shipment0.items.count, 1) + + let shipment1 = try XCTUnwrap(config.shipments.first(where: { $0.index == "1" })) + XCTAssertEqual(shipment1.shippingLabel?.shippingLabelID, 4871) + XCTAssertNil(shipment1.shippingLabel?.refund) + XCTAssertEqual(shipment1.items.count, 1) + + let shipment2 = try XCTUnwrap(config.shipments.first(where: { $0.index == "2" })) + XCTAssertEqual(shipment2.shippingLabel?.shippingLabelID, 4873) + XCTAssertNotNil(shipment2.shippingLabel?.refund) + XCTAssertEqual(shipment2.items.count, 1) + let shippingLabelData = try XCTUnwrap(config.shippingLabelData?.currentOrderLabels) - XCTAssertEqual(shippingLabelData.count, 1) + XCTAssertEqual(shippingLabelData.count, 3) XCTAssertEqual(shippingLabelData.first?.shipmentID, "1") XCTAssertEqual(shippingLabelData.first?.shippingLabelID, 4871) } diff --git a/Modules/Tests/NetworkingTests/Responses/shipping-label-config-success-without-data.json b/Modules/Tests/NetworkingTests/Responses/shipping-label-config-success-without-data.json index 11f096785be..67b2aec7079 100644 --- a/Modules/Tests/NetworkingTests/Responses/shipping-label-config-success-without-data.json +++ b/Modules/Tests/NetworkingTests/Responses/shipping-label-config-success-without-data.json @@ -9,7 +9,7 @@ "created": 1742292110723, "carrier_id": "usps", "service_name": "USPS - Priority Mail", - "status": "PURCHASE_IN_PROGRESS", + "status": "PURCHASED", "commercial_invoice_url": "", "is_commercial_invoice_submitted_electronically": false, "package_name": "Small Flat Rate Box", @@ -23,6 +23,55 @@ "id": "1", "receipt_item_id": -1, "created_date": 1742292110000 + }, + { + "label_id": 4872, + "tracking": null, + "refundable_amount": 0, + "created": 1742292110723, + "carrier_id": "usps", + "service_name": "USPS - Priority Mail", + "status": "PURCHASE_ERROR", + "commercial_invoice_url": "", + "is_commercial_invoice_submitted_electronically": false, + "package_name": "Small Flat Rate Box", + "is_letter": false, + "product_names": [ + "BG upload" + ], + "product_ids": [ + 522 + ], + "id": "0", + "receipt_item_id": -1, + "created_date": 1742292110000 + }, + { + "label_id": 4873, + "tracking": null, + "refundable_amount": 0, + "created": 1742292110723, + "carrier_id": "usps", + "service_name": "USPS - Priority Mail", + "status": "PURCHASED", + "commercial_invoice_url": "", + "is_commercial_invoice_submitted_electronically": false, + "package_name": "Small Flat Rate Box", + "is_letter": false, + "product_names": [ + "BG upload" + ], + "product_ids": [ + 522 + ], + "refund": { + "request_date": 1603715617000, + "status": "pending", + "refund_date": 1 + }, + "id": "2", + "receipt_item_id": -1, + "created_date": 1742292110000 } ] }, diff --git a/Modules/Tests/NetworkingTests/Responses/shipping-label-config-success.json b/Modules/Tests/NetworkingTests/Responses/shipping-label-config-success.json index 29ddc601e92..e136752dfc9 100644 --- a/Modules/Tests/NetworkingTests/Responses/shipping-label-config-success.json +++ b/Modules/Tests/NetworkingTests/Responses/shipping-label-config-success.json @@ -10,7 +10,7 @@ "created": 1742292110723, "carrier_id": "usps", "service_name": "USPS - Priority Mail", - "status": "PURCHASE_IN_PROGRESS", + "status": "PURCHASED", "commercial_invoice_url": "", "is_commercial_invoice_submitted_electronically": false, "package_name": "Small Flat Rate Box", @@ -24,6 +24,55 @@ "id": "1", "receipt_item_id": -1, "created_date": 1742292110000 + }, + { + "label_id": 4872, + "tracking": null, + "refundable_amount": 0, + "created": 1742292110723, + "carrier_id": "usps", + "service_name": "USPS - Priority Mail", + "status": "PURCHASE_ERROR", + "commercial_invoice_url": "", + "is_commercial_invoice_submitted_electronically": false, + "package_name": "Small Flat Rate Box", + "is_letter": false, + "product_names": [ + "BG upload" + ], + "product_ids": [ + 522 + ], + "id": "0", + "receipt_item_id": -1, + "created_date": 1742292110000 + }, + { + "label_id": 4873, + "tracking": null, + "refundable_amount": 0, + "created": 1742292110723, + "carrier_id": "usps", + "service_name": "USPS - Priority Mail", + "status": "PURCHASED", + "commercial_invoice_url": "", + "is_commercial_invoice_submitted_electronically": false, + "package_name": "Small Flat Rate Box", + "is_letter": false, + "product_names": [ + "BG upload" + ], + "product_ids": [ + 522 + ], + "refund": { + "request_date": 1603715617000, + "status": "pending", + "refund_date": 1 + }, + "id": "2", + "receipt_item_id": -1, + "created_date": 1742292110000 } ] }, diff --git a/WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/WooShipping Create Shipping Labels/WooShipping Split Shipments/WooShippingSplitShipmentsViewModel.swift b/WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/WooShipping Create Shipping Labels/WooShipping Split Shipments/WooShippingSplitShipmentsViewModel.swift index d14eca0dc05..cebde794ee4 100644 --- a/WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/WooShipping Create Shipping Labels/WooShipping Split Shipments/WooShippingSplitShipmentsViewModel.swift +++ b/WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/WooShipping Create Shipping Labels/WooShipping Split Shipments/WooShippingSplitShipmentsViewModel.swift @@ -37,7 +37,7 @@ final class WooShippingSplitShipmentsViewModel: ObservableObject { /// private var editedShipmentsInfo: WooShippingShipments { var shipmentsForRemote = [String: [WooShippingShipmentItem]]() - for shipment in shipments { + for shipment in shipments.sorted(by: { $0.index < $1.index}) { var items = [WooShippingShipmentItem]() for item in shipment.contents { if let mainItemID = Int(item.mainItemRow.itemID) { @@ -542,43 +542,31 @@ extension WooShippingSplitShipmentsViewModel { return [shipment] } - let currentOrderLabels = config.shippingLabelData?.currentOrderLabels ?? [] - var shipments = [Shipment]() - - let sortedShipments = config.shipments.sorted(by: { - $0.index.localizedStandardCompare($1.index) == .orderedAscending - }) - for shipment in sortedShipments { - let index = shipment.index - - let purchasedLabel = currentOrderLabels - .first(where: { $0.shipmentID == index && $0.status == .purchased && $0.refund == nil }) - - let isPurchased = purchasedLabel != nil + let shipments = config.shipments + .sorted(by: { $0.index.localizedStandardCompare($1.index) == .orderedAscending }) + .map { shipment in + var shipmentContents = ShipmentContents() + for shipmentItem in shipment.items { + guard let packageItem = packageItems.first(where: { $0.orderItemID == shipmentItem.id }), + let subItems = shipmentItem.subItems else { + continue + } - var shipmentContents = ShipmentContents() - for shipmentItem in shipment.items { - guard let packageItem = packageItems.first(where: { $0.orderItemID == shipmentItem.id }), - let subItems = shipmentItem.subItems else { - continue + let quantity = subItems.count > 0 ? subItems.count : 1 + let updatedItem = ShippingLabelPackageItem(copy: packageItem, quantity: Decimal(quantity)) + let content = CollapsibleShipmentItemCardViewModel(item: updatedItem, + isSelectable: shipment.shippingLabel == nil, + currency: currency) + shipmentContents.append(content) } - let quantity = subItems.count > 0 ? subItems.count : 1 - let updatedItem = ShippingLabelPackageItem(copy: packageItem, quantity: Decimal(quantity)) - let content = CollapsibleShipmentItemCardViewModel(item: updatedItem, - isSelectable: !isPurchased, - currency: currency) - shipmentContents.append(content) + return Shipment(index: Int(shipment.index) ?? 0, + contents: shipmentContents, + purchasedLabelID: shipment.shippingLabel?.shippingLabelID, + currency: currency, + currencySettings: currencySettings, + shippingSettingsService: shippingSettingsService) } - - let shipment = Shipment(index: Int(index) ?? 0, - contents: shipmentContents, - purchasedLabelID: purchasedLabel?.shippingLabelID, - currency: currency, - currencySettings: currencySettings, - shippingSettingsService: shippingSettingsService) - shipments.append(shipment) - } return shipments } diff --git a/WooCommerce/WooCommerceTests/ViewRelated/Shipping Label/WooShipping Create Shipping Labels/Split shipments/WooShippingSplitShipmentsViewModelTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/Shipping Label/WooShipping Create Shipping Labels/Split shipments/WooShippingSplitShipmentsViewModelTests.swift index 2a7eb206f9a..ce8e2ea095f 100644 --- a/WooCommerce/WooCommerceTests/ViewRelated/Shipping Label/WooShipping Create Shipping Labels/Split shipments/WooShippingSplitShipmentsViewModelTests.swift +++ b/WooCommerce/WooCommerceTests/ViewRelated/Shipping Label/WooShipping Create Shipping Labels/Split shipments/WooShippingSplitShipmentsViewModelTests.swift @@ -105,10 +105,11 @@ final class WooShippingSplitShipmentsViewModelTests: XCTestCase { sampleItem(id: 2, weight: 3, value: 2.5, quantity: 1)] // When - let shippingLabelData = WooShippingLabelData(currentOrderLabels: [ShippingLabel.fake().copy(shipmentID: "1")]) + let label = ShippingLabel.fake().copy(shipmentID: "1") + let shippingLabelData = WooShippingLabelData(currentOrderLabels: [label]) let config = WooShippingConfig(siteID: 123, shipments: [ - "0": [WooShippingShipmentItem(id: 1, subItems: ["sub-1", "sub-2"])], - "1": [WooShippingShipmentItem(id: 2, subItems: [])] + .fake().copy(index: "0", items: [.init(id: 1, subItems: ["sub-1", "sub-2"])]), + .fake().copy(index: "1", items: [.init(id: 2, subItems: [])], shippingLabel: label) ], shippingLabelData: shippingLabelData) let viewModel = WooShippingSplitShipmentsViewModel(order: sampleOrder, config: config, @@ -237,10 +238,11 @@ final class WooShippingSplitShipmentsViewModelTests: XCTestCase { let items = [sampleItem(id: 1, weight: 5, value: 10, quantity: 2), sampleItem(id: 2, weight: 3, value: 2.5, quantity: 1)] - let shippingLabelData = WooShippingLabelData(currentOrderLabels: [ShippingLabel.fake().copy(shipmentID: "1")]) + let label = ShippingLabel.fake().copy(shipmentID: "1") + let shippingLabelData = WooShippingLabelData(currentOrderLabels: [label]) let config = WooShippingConfig(siteID: 123, shipments: [ - "0": [WooShippingShipmentItem(id: 1, subItems: ["sub-1", "sub-2"])], - "1": [WooShippingShipmentItem(id: 2, subItems: [])] + .fake().copy(index: "0", items: [.init(id: 1, subItems: ["sub-1", "sub-2"])]), + .fake().copy(index: "1", items: [.init(id: 2, subItems: [])], shippingLabel: label) ], shippingLabelData: shippingLabelData) let viewModel = WooShippingSplitShipmentsViewModel(order: sampleOrder, @@ -596,11 +598,12 @@ final class WooShippingSplitShipmentsViewModelTests: XCTestCase { sampleItem(id: 2, weight: 3, value: 2.5, quantity: 1), sampleItem(id: 3, weight: 4, value: 5, quantity: 3)] - let shippingLabelData = WooShippingLabelData(currentOrderLabels: [ShippingLabel.fake().copy(shipmentID: "2")]) + let label = ShippingLabel.fake().copy(shipmentID: "2") + let shippingLabelData = WooShippingLabelData(currentOrderLabels: [label]) let config = WooShippingConfig(siteID: 123, shipments: [ - "0": [WooShippingShipmentItem(id: 1, subItems: ["1-sub-0", "1-sub-1"])], - "1": [WooShippingShipmentItem(id: 2, subItems: [])], - "2": [WooShippingShipmentItem(id: 3, subItems: ["3-sub-0", "3-sub-1", "3-sub-2"])] + .fake().copy(index: "0", items: [.init(id: 1, subItems: ["1-sub-0", "1-sub-1"])]), + .fake().copy(index: "1", items: [.init(id: 2, subItems: [])]), + .fake().copy(index: "2", items: [.init(id: 3, subItems: ["3-sub-0", "3-sub-1", "3-sub-2"])], shippingLabel: label) ], shippingLabelData: shippingLabelData) var receivedShipmentToUpdate: WooShippingUpdateShipment? @@ -629,9 +632,9 @@ final class WooShippingSplitShipmentsViewModelTests: XCTestCase { // Then let expectedShipmentToUpdate = WooShippingUpdateShipment( shipmentIdsToUpdate: ["2": 1], - shipments: ["1": [WooShippingShipmentItem(id: 3, subItems: ["3-sub-0", "3-sub-1", "3-sub-2"])], - "0": [WooShippingShipmentItem(id: 1, subItems: ["1-sub-0", "1-sub-1"]), - WooShippingShipmentItem(id: 2, subItems: [])]] + shipments: ["0": [WooShippingShipmentItem(id: 1, subItems: ["1-sub-0", "1-sub-1"]), + WooShippingShipmentItem(id: 2, subItems: [])], + "1": [WooShippingShipmentItem(id: 3, subItems: ["3-sub-0", "3-sub-1", "3-sub-2"])]] ) XCTAssertEqual(receivedShipmentToUpdate, expectedShipmentToUpdate) } @@ -648,14 +651,14 @@ final class WooShippingSplitShipmentsViewModelTests: XCTestCase { let shippingLabel = ShippingLabel.fake().copy(shipmentID: "1") let config = WooShippingConfig.fake().copy( shipments: [ - "0": [ + .fake().copy(index: "0", items: [ WooShippingShipmentItem.fake().copy(id: 1, subItems: ["1-sub-0", "1-sub-1"]), WooShippingShipmentItem.fake().copy(id: 2, subItems: []), WooShippingShipmentItem.fake().copy(id: 3, subItems: ["3-sub-0", "3-sub-1", "3-sub-2"]) - ], - "1": [ + ]), + .fake().copy(index: "1", items: [ WooShippingShipmentItem.fake().copy(id: 4, subItems: nil) - ] + ], shippingLabel: shippingLabel) ], shippingLabelData: WooShippingLabelData(currentOrderLabels: [shippingLabel]) ) @@ -773,11 +776,12 @@ final class WooShippingSplitShipmentsViewModelTests: XCTestCase { sampleItem(id: 2, weight: 3, value: 2.5, quantity: 1), sampleItem(id: 3, weight: 4, value: 5, quantity: 3)] - let shippingLabelData = WooShippingLabelData(currentOrderLabels: [ShippingLabel.fake().copy(shipmentID: "1")]) + let label = ShippingLabel.fake().copy(shipmentID: "1") + let shippingLabelData = WooShippingLabelData(currentOrderLabels: [label]) let config = WooShippingConfig(siteID: 123, shipments: [ - "0": [WooShippingShipmentItem(id: 1, subItems: ["sub-1", "sub-2"])], - "1": [WooShippingShipmentItem(id: 2, subItems: [])], - "2": [WooShippingShipmentItem(id: 3, subItems: ["sub-1", "sub-2", "sub-3"])] + .fake().copy(index: "0", items: [.init(id: 1, subItems: ["sub-1", "sub-2"])]), + .fake().copy(index: "1", items: [.init(id: 2, subItems: [])], shippingLabel: label), + .fake().copy(index: "2", items: [.init(id: 3, subItems: ["sub-1", "sub-2", "sub-3"])]) ], shippingLabelData: shippingLabelData) let viewModel = WooShippingSplitShipmentsViewModel(order: sampleOrder, config: config, @@ -815,8 +819,8 @@ final class WooShippingSplitShipmentsViewModelTests: XCTestCase { let shippingLabelData = WooShippingLabelData(currentOrderLabels: []) // none purchased yet let config = WooShippingConfig(siteID: 123, shipments: [ - "0": [WooShippingShipmentItem(id: 1, subItems: ["sub-1", "sub-2"])], - "1": [WooShippingShipmentItem(id: 2, subItems: [])] + .fake().copy(index: "0", items: [.init(id: 1, subItems: ["sub-1", "sub-2"])]), + .fake().copy(index: "1", items: [.init(id: 2, subItems: [])]), ], shippingLabelData: shippingLabelData) let viewModel = WooShippingSplitShipmentsViewModel(order: sampleOrder, config: config, @@ -852,14 +856,11 @@ final class WooShippingSplitShipmentsViewModelTests: XCTestCase { sampleItem(id: 2, weight: 3, value: 2.5, quantity: 1), sampleItem(id: 3, weight: 4, value: 5, quantity: 3)] - let shippingLabelData = WooShippingLabelData( - currentOrderLabels: [ - ShippingLabel.fake().copy(shipmentID: "0") - ] - ) + let label = ShippingLabel.fake().copy(shipmentID: "0") + let shippingLabelData = WooShippingLabelData(currentOrderLabels: [label]) let config = WooShippingConfig(siteID: 123, shipments: [ - "0": [WooShippingShipmentItem(id: 1, subItems: ["sub-1", "sub-2"])], - "1": [WooShippingShipmentItem(id: 2, subItems: [])] + .fake().copy(index: "0", items: [.init(id: 1, subItems: ["sub-1", "sub-2"])], shippingLabel: label), + .fake().copy(index: "1", items: [.init(id: 2, subItems: [])]), ], shippingLabelData: shippingLabelData) let viewModel = WooShippingSplitShipmentsViewModel(order: sampleOrder, config: config, @@ -1008,17 +1009,14 @@ final class WooShippingSplitShipmentsViewModelTests: XCTestCase { sampleItem(id: 2, weight: 3, value: 2.5, quantity: 1) ] - let shippingLabelData = WooShippingLabelData( - currentOrderLabels: [ - ShippingLabel.fake().copy(shipmentID: "0") - ] - ) + let label = ShippingLabel.fake().copy(shipmentID: "0") + let shippingLabelData = WooShippingLabelData(currentOrderLabels: [label]) let config = WooShippingConfig( siteID: 123, shipments: [ - "0": [WooShippingShipmentItem(id: 1, subItems: ["sub-1", "sub-2"])], - "1": [WooShippingShipmentItem(id: 2, subItems: [])] + .fake().copy(index: "0", items: [.init(id: 1, subItems: ["sub-1", "sub-2"])], shippingLabel: label), + .fake().copy(index: "1", items: [.init(id: 2, subItems: [])]), ], shippingLabelData: shippingLabelData ) @@ -1070,17 +1068,14 @@ final class WooShippingSplitShipmentsViewModelTests: XCTestCase { sampleItem(id: 2, weight: 3, value: 2.5, quantity: 1) ] - let shippingLabelData = WooShippingLabelData( - currentOrderLabels: [ - ShippingLabel.fake().copy(shipmentID: "1") - ] - ) + let label = ShippingLabel.fake().copy(shipmentID: "1") + let shippingLabelData = WooShippingLabelData(currentOrderLabels: [label]) let config = WooShippingConfig( siteID: 123, shipments: [ - "0": [WooShippingShipmentItem(id: 1, subItems: ["sub-1", "sub-2"])], - "1": [WooShippingShipmentItem(id: 2, subItems: [])] + .fake().copy(index: "0", items: [.init(id: 1, subItems: ["sub-1", "sub-2"])], shippingLabel: label), + .fake().copy(index: "1", items: [.init(id: 2, subItems: [])]), ], shippingLabelData: shippingLabelData ) @@ -1110,8 +1105,8 @@ final class WooShippingSplitShipmentsViewModelTests: XCTestCase { let config = WooShippingConfig( siteID: 123, shipments: [ - "0": [WooShippingShipmentItem(id: 1, subItems: ["sub-1", "sub-2"])], - "1": [WooShippingShipmentItem(id: 2, subItems: [])] + .fake().copy(index: "0", items: [.init(id: 1, subItems: ["sub-1", "sub-2"])]), + .fake().copy(index: "1", items: [.init(id: 2, subItems: [])]), ], shippingLabelData: WooShippingLabelData(currentOrderLabels: []) ) @@ -1192,16 +1187,13 @@ final class WooShippingSplitShipmentsViewModelTests: XCTestCase { func test_isMergeAllUnfulfilledAvailable_returns_false_when_no_unfulfilled_shipments() throws { // Given - let shippingLabelData = WooShippingLabelData( - currentOrderLabels: [ - ShippingLabel.fake().copy(shipmentID: "0") - ] - ) + let label = ShippingLabel.fake().copy(shipmentID: "0") + let shippingLabelData = WooShippingLabelData(currentOrderLabels: [label]) let config = WooShippingConfig( siteID: 123, shipments: [ - "0": [WooShippingShipmentItem(id: 1, subItems: ["sub-1", "sub-2"])] + .fake().copy(index: "0", items: [.init(id: 1, subItems: ["sub-1", "sub-2"])], shippingLabel: label), ], shippingLabelData: shippingLabelData ) @@ -1315,18 +1307,18 @@ final class WooShippingSplitShipmentsViewModelTests: XCTestCase { ] let config = WooShippingConfig.fake().copy( shipments: [ - "0": [ + .fake().copy(index: "0", items: [ WooShippingShipmentItem.fake().copy( id: 1, subItems: nil ) - ], - "1": [ + ]), + .fake().copy(index: "1", items: [ WooShippingShipmentItem.fake().copy( id: 1, subItems: nil ) - ] + ]) ] ) let viewModel = WooShippingSplitShipmentsViewModel( @@ -1354,18 +1346,18 @@ final class WooShippingSplitShipmentsViewModelTests: XCTestCase { let shippingLabel = ShippingLabel.fake().copy(shipmentID: "0") let config = WooShippingConfig.fake().copy( shipments: [ - "0": [ + .fake().copy(index: "0", items: [ WooShippingShipmentItem.fake().copy( id: 1, subItems: nil ) - ], - "1": [ + ], shippingLabel: shippingLabel), + .fake().copy(index: "1", items: [ WooShippingShipmentItem.fake().copy( id: 1, subItems: nil ) - ] + ]) ], shippingLabelData: WooShippingLabelData( currentOrderLabels: [shippingLabel] @@ -1397,18 +1389,18 @@ final class WooShippingSplitShipmentsViewModelTests: XCTestCase { ] let config = WooShippingConfig.fake().copy( shipments: [ - "0": [ + .fake().copy(index: "0", items: [ WooShippingShipmentItem.fake().copy( id: 1, subItems: nil ) - ], - "1": [ + ]), + .fake().copy(index: "1", items: [ WooShippingShipmentItem.fake().copy( id: 1, subItems: nil ) - ] + ]) ] ) let viewModel = WooShippingSplitShipmentsViewModel( @@ -1435,24 +1427,24 @@ final class WooShippingSplitShipmentsViewModelTests: XCTestCase { ] let config = WooShippingConfig.fake().copy( shipments: [ - "0": [ + .fake().copy(index: "0", items: [ WooShippingShipmentItem.fake().copy( id: 1, subItems: ["sub-1"] ) - ], - "1": [ + ]), + .fake().copy(index: "1", items: [ WooShippingShipmentItem.fake().copy( id: 1, subItems: ["sub-2"] ) - ], - "2": [ + ]), + .fake().copy(index: "2", items: [ WooShippingShipmentItem.fake().copy( id: 1, subItems: ["sub-3"] ) - ] + ]) ] ) let viewModel = WooShippingSplitShipmentsViewModel( 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 b718d64c009..1d14c1e97a8 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 @@ -537,8 +537,8 @@ final class WooShippingCreateLabelsViewModelTests: XCTestCase { case .loadConfig(_, _, let completion): // There exist 2 shipments, one of which has been fulfilled. let shippingLabel = ShippingLabel.fake().copy(shippingLabelID: 134) - let shipments = ["shipment_0": [WooShippingShipmentItem.fake()], - "shipment_1": [WooShippingShipmentItem.fake()]] + let shipments = [WooShippingShipment.fake().copy(index: "shipment_0", items: [.fake()]), + WooShippingShipment.fake().copy(index: "shipment_1", items: [.fake()])] let shippingLabelData = WooShippingLabelData(currentOrderLabels: [ ShippingLabel.fake().copy(shippingLabelID: shippingLabel.shippingLabelID, shipmentID: "shipment_0") @@ -610,8 +610,8 @@ final class WooShippingCreateLabelsViewModelTests: XCTestCase { completion(.success(self.settings)) case .loadConfig(_, _, let completion): // There exist 2 shipments, one of which has been fulfilled. - let shipments = ["0": [WooShippingShipmentItem.fake()], - "1": [WooShippingShipmentItem.fake()]] + let shipments = [WooShippingShipment.fake().copy(index: "0", items: [.fake()]), + WooShippingShipment.fake().copy(index: "1", items: [.fake()])] let shippingLabelData = WooShippingLabelData(currentOrderLabels: [shippingLabel]) completion(.success(WooShippingConfig.fake().copy(shipments: shipments, shippingLabelData: shippingLabelData))) @@ -658,8 +658,8 @@ final class WooShippingCreateLabelsViewModelTests: XCTestCase { completion(.success(self.settings)) case .loadConfig(_, _, let completion): // There exist 2 shipments, one of which has been fulfilled. - let shipments = ["0": [WooShippingShipmentItem.fake()], - "1": [WooShippingShipmentItem.fake()]] + let shipments = [WooShippingShipment.fake().copy(index: "0", items: [.fake()]), + WooShippingShipment.fake().copy(index: "1", items: [.fake()])] let shippingLabelData = WooShippingLabelData(currentOrderLabels: [shippingLabel]) completion(.success(WooShippingConfig.fake().copy(shipments: shipments, shippingLabelData: shippingLabelData))) From 78a978107e04e0f348eae67a10dac21d25822f8b Mon Sep 17 00:00:00 2001 From: Huong Do Date: Thu, 10 Jul 2025 14:54:28 +0700 Subject: [PATCH 09/13] Fix failed tests --- .../WooShippingCreateLabelsViewModelTests.swift | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 1d14c1e97a8..e55e99ee0d6 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 @@ -537,7 +537,7 @@ final class WooShippingCreateLabelsViewModelTests: XCTestCase { case .loadConfig(_, _, let completion): // There exist 2 shipments, one of which has been fulfilled. let shippingLabel = ShippingLabel.fake().copy(shippingLabelID: 134) - let shipments = [WooShippingShipment.fake().copy(index: "shipment_0", items: [.fake()]), + let shipments = [WooShippingShipment.fake().copy(index: "shipment_0", items: [.fake()], shippingLabel: shippingLabel), WooShippingShipment.fake().copy(index: "shipment_1", items: [.fake()])] let shippingLabelData = WooShippingLabelData(currentOrderLabels: [ ShippingLabel.fake().copy(shippingLabelID: shippingLabel.shippingLabelID, @@ -610,7 +610,7 @@ final class WooShippingCreateLabelsViewModelTests: XCTestCase { completion(.success(self.settings)) case .loadConfig(_, _, let completion): // There exist 2 shipments, one of which has been fulfilled. - let shipments = [WooShippingShipment.fake().copy(index: "0", items: [.fake()]), + let shipments = [WooShippingShipment.fake().copy(index: "0", items: [.fake()], shippingLabel: shippingLabel), WooShippingShipment.fake().copy(index: "1", items: [.fake()])] let shippingLabelData = WooShippingLabelData(currentOrderLabels: [shippingLabel]) completion(.success(WooShippingConfig.fake().copy(shipments: shipments, @@ -658,7 +658,7 @@ final class WooShippingCreateLabelsViewModelTests: XCTestCase { completion(.success(self.settings)) case .loadConfig(_, _, let completion): // There exist 2 shipments, one of which has been fulfilled. - let shipments = [WooShippingShipment.fake().copy(index: "0", items: [.fake()]), + let shipments = [WooShippingShipment.fake().copy(index: "0", items: [.fake()], shippingLabel: shippingLabel), WooShippingShipment.fake().copy(index: "1", items: [.fake()])] let shippingLabelData = WooShippingLabelData(currentOrderLabels: [shippingLabel]) completion(.success(WooShippingConfig.fake().copy(shipments: shipments, From eebb828a403f0be2fa4aa9a0210f3acde05510ba Mon Sep 17 00:00:00 2001 From: Huong Do Date: Thu, 10 Jul 2025 15:19:08 +0700 Subject: [PATCH 10/13] Simplify managing shipping labels --- .../Order Details/OrderDetailsViewModel.swift | 2 +- .../WooShippingSplitShipmentsViewModel.swift | 18 ++++++------ .../WooShippingCreateLabelsViewModel.swift | 28 ++++++------------- ...ShippingSplitShipmentsViewModelTests.swift | 11 ++++---- ...ooShippingCreateLabelsViewModelTests.swift | 16 +++++------ 5 files changed, 32 insertions(+), 43 deletions(-) diff --git a/WooCommerce/Classes/ViewModels/Order Details/OrderDetailsViewModel.swift b/WooCommerce/Classes/ViewModels/Order Details/OrderDetailsViewModel.swift index d96aeb633f7..d41ccfa2261 100644 --- a/WooCommerce/Classes/ViewModels/Order Details/OrderDetailsViewModel.swift +++ b/WooCommerce/Classes/ViewModels/Order Details/OrderDetailsViewModel.swift @@ -280,7 +280,7 @@ extension OrderDetailsViewModel { dataSource.isEligibleForShippingLabelCreation = isEligible } - taskGroup.addTask { @MainActor [weak self] in + taskGroup.addTask { [weak self] in guard let self else { return } // Sync shipping labels and update order with the result if available let shippingLabels = await syncShippingLabels() diff --git a/WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/WooShipping Create Shipping Labels/WooShipping Split Shipments/WooShippingSplitShipmentsViewModel.swift b/WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/WooShipping Create Shipping Labels/WooShipping Split Shipments/WooShippingSplitShipmentsViewModel.swift index cebde794ee4..4c74c9b42f2 100644 --- a/WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/WooShipping Create Shipping Labels/WooShipping Split Shipments/WooShippingSplitShipmentsViewModel.swift +++ b/WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/WooShipping Create Shipping Labels/WooShipping Split Shipments/WooShippingSplitShipmentsViewModel.swift @@ -159,14 +159,14 @@ final class WooShippingSplitShipmentsViewModel: ObservableObject { selectedShipmentIndex = 0 } - func didPurchaseLabel(for shipmentIndex: Int, purchasedLabelID: Int64) { + func didPurchaseLabel(for shipmentIndex: Int, label: ShippingLabel) { let currentShipment = shipments[shipmentIndex] let updatedContents = currentShipment.contents.map { CollapsibleShipmentItemCardViewModel(item: $0.packageItem, isSelectable: false, currency: order.currency) } shipments[shipmentIndex] = Shipment(index: shipmentIndex, contents: updatedContents, - purchasedLabelID: purchasedLabelID, + purchasedLabel: label, currency: order.currency, currencySettings: currencySettings, shippingSettingsService: shippingSettingsService) @@ -179,7 +179,7 @@ final class WooShippingSplitShipmentsViewModel: ObservableObject { } shipments[shipmentIndex] = Shipment(index: shipmentIndex, contents: updatedContents, - purchasedLabelID: nil, + purchasedLabel: nil, currency: order.currency, currencySettings: currencySettings, shippingSettingsService: shippingSettingsService) @@ -562,7 +562,7 @@ extension WooShippingSplitShipmentsViewModel { return Shipment(index: Int(shipment.index) ?? 0, contents: shipmentContents, - purchasedLabelID: shipment.shippingLabel?.shippingLabelID, + purchasedLabel: shipment.shippingLabel, currency: currency, currencySettings: currencySettings, shippingSettingsService: shippingSettingsService) @@ -586,7 +586,7 @@ extension WooShippingSplitShipmentsViewModel { let index: Int let contents: [CollapsibleShipmentItemCardViewModel] - let purchasedLabelID: Int64? + let purchasedLabel: ShippingLabel? let quantity: String let weight: String @@ -603,7 +603,7 @@ extension WooShippingSplitShipmentsViewModel { } var isPurchased: Bool { - purchasedLabelID != nil + purchasedLabel != nil } private let currency: String @@ -613,14 +613,14 @@ extension WooShippingSplitShipmentsViewModel { init(id: String = UUID().uuidString, index: Int = 0, contents: [CollapsibleShipmentItemCardViewModel], - purchasedLabelID: Int64? = nil, + purchasedLabel: ShippingLabel? = nil, currency: String, currencySettings: CurrencySettings, shippingSettingsService: ShippingSettingsService) { self.id = id self.index = index self.contents = contents - self.purchasedLabelID = purchasedLabelID + self.purchasedLabel = purchasedLabel let items = contents.map(\.packageItem) let itemsCount = items.map(\.quantity).reduce(0, +) @@ -661,7 +661,7 @@ extension WooShippingSplitShipmentsViewModel { Shipment(id: id, index: newIndex, contents: contents, - purchasedLabelID: purchasedLabelID, + purchasedLabel: purchasedLabel, currency: currency, currencySettings: currencySettings, shippingSettingsService: shippingSettingsService) 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 df1efcb51f1..0739fe45d41 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 @@ -26,9 +26,6 @@ final class WooShippingCreateLabelsViewModel: ObservableObject { private(set) var orderItems: WooShippingItemsViewModel - /// The purchased shipping label. - @Published private var shippingLabels: [ShippingLabel] = [] - var canViewLabel: Bool { currentShipmentDetailsViewModel.canViewLabel } @@ -57,7 +54,7 @@ final class WooShippingCreateLabelsViewModel: ObservableObject { } var hasUnfulfilledShipments: Bool { - shipments.contains(where: { $0.purchasedLabelID == nil }) + shipments.contains(where: { $0.purchasedLabel == nil }) } @Published var labelPurchaseErrorNotice: Notice? @@ -245,7 +242,7 @@ final class WooShippingCreateLabelsViewModel: ObservableObject { // After shipment configs are updated, shipments are updated with purchased label details // Update the selected tab now by comparing the purchased labels with the initial selected label. if let selectedShippingLabel, - let matchingIndex = shipments.firstIndex(where: { $0.purchasedLabelID == selectedShippingLabel.shippingLabelID }) { + let matchingIndex = shipments.firstIndex(where: { $0.purchasedLabel?.shippingLabelID == selectedShippingLabel.shippingLabelID }) { selectedShipmentIndex = matchingIndex } } @@ -261,7 +258,7 @@ final class WooShippingCreateLabelsViewModel: ObservableObject { } } - if shipments.contains(where: { $0.purchasedLabelID == nil }) { + if shipments.contains(where: { $0.purchasedLabel == nil }) { group.addTask { await self.loadOriginAddresses() } @@ -277,7 +274,7 @@ final class WooShippingCreateLabelsViewModel: ObservableObject { state = .missingRequiredData } else { state = .ready - let count = shipments.count(where: { $0.purchasedLabelID == nil }) + let count = shipments.count(where: { $0.purchasedLabel == nil }) analytics.track(event: .WooShipping.createShippingLabelFormShown(unfulfilledShipmentsCount: count)) } } @@ -435,7 +432,6 @@ private extension WooShippingCreateLabelsViewModel { } if let config { - shippingLabels = config.shippingLabelData?.currentOrderLabels.filter { $0.refund == nil && $0.status == .purchased } ?? [] splitShipmentsViewModel = WooShippingSplitShipmentsViewModel(order: order, config: config, items: itemsDataSource.items, @@ -494,14 +490,13 @@ private extension WooShippingCreateLabelsViewModel { func updateShipmentDetailsViewModels() { shipmentDetailViewModels = shipments.map { shipment in - let matchingShippingLabel = shippingLabels.first(where: { $0.shippingLabelID == shipment.purchasedLabelID }) let originAddressPublisher = $selectedOriginAddress .map { $0?.toWooShippingAddress() } .eraseToAnyPublisher() return WooShippingShipmentDetailsViewModel( order: order, shipment: shipment, - shippingLabel: matchingShippingLabel, + shippingLabel: shipment.purchasedLabel, originAddress: originAddressPublisher, destinationAddress: $destinationAddress.eraseToAnyPublisher(), stores: stores, @@ -518,33 +513,26 @@ private extension WooShippingCreateLabelsViewModel { } func handleLabelPurchaseSuccess(newLabel: ShippingLabel, in shipment: Shipment) { - shippingLabels.append(newLabel) - let index = shipment.index shipments[index] = Shipment(index: index, contents: shipment.contents, - purchasedLabelID: newLabel.shippingLabelID, + purchasedLabel: newLabel, currency: order.currency, currencySettings: currencySettings, shippingSettingsService: shippingSettingsService) - splitShipmentsViewModel.didPurchaseLabel(for: index, - purchasedLabelID: newLabel.shippingLabelID) + splitShipmentsViewModel.didPurchaseLabel(for: index, label: newLabel) onLabelPurchase?(markOrderComplete) } func handleLabelRefundRequested(labelID: Int64, in shipment: Shipment) { - let labelIndex = shippingLabels.firstIndex(where: { $0.shippingLabelID == labelID }) - guard let labelIndex else { return } - let shipmentIndex = shipment.index shipments[shipmentIndex] = Shipment(index: shipmentIndex, contents: shipment.contents, - purchasedLabelID: nil, + purchasedLabel: nil, currency: order.currency, currencySettings: currencySettings, shippingSettingsService: shippingSettingsService) - shippingLabels.remove(at: labelIndex) splitShipmentsViewModel.didRequestRefund(for: shipmentIndex) refundNotice = Notice(message: Localization.refundNotice) } diff --git a/WooCommerce/WooCommerceTests/ViewRelated/Shipping Label/WooShipping Create Shipping Labels/Split shipments/WooShippingSplitShipmentsViewModelTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/Shipping Label/WooShipping Create Shipping Labels/Split shipments/WooShippingSplitShipmentsViewModelTests.swift index ce8e2ea095f..956184d790c 100644 --- a/WooCommerce/WooCommerceTests/ViewRelated/Shipping Label/WooShipping Create Shipping Labels/Split shipments/WooShippingSplitShipmentsViewModelTests.swift +++ b/WooCommerce/WooCommerceTests/ViewRelated/Shipping Label/WooShipping Create Shipping Labels/Split shipments/WooShippingSplitShipmentsViewModelTests.swift @@ -834,19 +834,20 @@ final class WooShippingSplitShipmentsViewModelTests: XCTestCase { // When let shipmentID = viewModel.shipments[0].index let purchasedLabelID: Int64 = 325 - viewModel.didPurchaseLabel(for: shipmentID, purchasedLabelID: purchasedLabelID) + let label = ShippingLabel.fake().copy(shippingLabelID: purchasedLabelID) + viewModel.didPurchaseLabel(for: shipmentID, label: label) // Then XCTAssertEqual(viewModel.shipments.count, 2) XCTAssertEqual(viewModel.shipments[0].index, 0) - XCTAssertEqual(viewModel.shipments[0].purchasedLabelID, purchasedLabelID) + XCTAssertEqual(viewModel.shipments[0].purchasedLabel, label) XCTAssertFalse(viewModel.shipments[0].contents[0].mainItemRow.isSelectable) XCTAssertTrue(viewModel.shipments[0].contents[0].childItemRows.allSatisfy({ !$0.isSelectable })) XCTAssertEqual(viewModel.shipments[1].index, 1) XCTAssertEqual(viewModel.shipments[1].contents.count, 1) XCTAssertTrue(viewModel.shipments[1].contents[0].mainItemRow.isSelectable) - XCTAssertNil(viewModel.shipments[1].purchasedLabelID) + XCTAssertNil(viewModel.shipments[1].purchasedLabel) } // MARK: - `didRequestRefund` @@ -877,14 +878,14 @@ final class WooShippingSplitShipmentsViewModelTests: XCTestCase { // Then XCTAssertEqual(viewModel.shipments.count, 2) - XCTAssertNil(viewModel.shipments[0].purchasedLabelID) + XCTAssertNil(viewModel.shipments[0].purchasedLabel) XCTAssertTrue(viewModel.shipments[0].contents[0].mainItemRow.isSelectable) XCTAssertTrue(viewModel.shipments[0].contents[0].childItemRows.allSatisfy({ $0.isSelectable })) XCTAssertEqual(viewModel.shipments[1].contents.count, 1) XCTAssertTrue(viewModel.shipments[1].contents[0].mainItemRow.isSelectable) - XCTAssertNil(viewModel.shipments[1].purchasedLabelID) + XCTAssertNil(viewModel.shipments[1].purchasedLabel) } // MARK: - `enableDoneButton` 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 e55e99ee0d6..672634f901a 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 @@ -627,14 +627,14 @@ final class WooShippingCreateLabelsViewModelTests: XCTestCase { } // Then - XCTAssertEqual(viewModel.currentShipment.purchasedLabelID, shippingLabel.shippingLabelID) + XCTAssertEqual(viewModel.currentShipment.purchasedLabel?.shippingLabelID, shippingLabel.shippingLabelID) XCTAssertEqual(viewModel.originAddressLines?.first, labelOriginAddress.address1) // When viewModel.selectedShipmentIndex = 1 // Then - XCTAssertNil(viewModel.currentShipment.purchasedLabelID) + XCTAssertNil(viewModel.currentShipment.purchasedLabel) XCTAssertEqual(viewModel.originAddressLines?.first, originAddress.address1) } @@ -679,14 +679,14 @@ final class WooShippingCreateLabelsViewModelTests: XCTestCase { } // Then - XCTAssertEqual(viewModel.currentShipment.purchasedLabelID, shippingLabel.shippingLabelID) + XCTAssertEqual(viewModel.currentShipment.purchasedLabel?.shippingLabelID, shippingLabel.shippingLabelID) XCTAssertEqual(viewModel.destinationAddressLines?.first, labelDestinationAddress.address1) // When viewModel.selectedShipmentIndex = 1 // Then - XCTAssertNil(viewModel.currentShipment.purchasedLabelID) + XCTAssertNil(viewModel.currentShipment.purchasedLabel) XCTAssertEqual(viewModel.destinationAddressLines?.first, destinationAddress.address1) } @@ -702,7 +702,7 @@ final class WooShippingCreateLabelsViewModelTests: XCTestCase { let order = Order.fake().copy(shippingLabels: [shippingLabel]) let shipment = Shipment( contents: [], - purchasedLabelID: shippingLabel.shippingLabelID, + purchasedLabel: shippingLabel, currency: "USD", currencySettings: ServiceLocator.currencySettings, shippingSettingsService: MockShippingSettingsService() @@ -746,7 +746,7 @@ final class WooShippingCreateLabelsViewModelTests: XCTestCase { ) let shipment = Shipment( contents: [], - purchasedLabelID: nil, + purchasedLabel: nil, currency: "USD", currencySettings: ServiceLocator.currencySettings, shippingSettingsService: MockShippingSettingsService() @@ -813,7 +813,7 @@ final class WooShippingCreateLabelsViewModelTests: XCTestCase { let shipment = Shipment( contents: [], - purchasedLabelID: nil, + purchasedLabel: nil, currency: "USD", currencySettings: ServiceLocator.currencySettings, shippingSettingsService: MockShippingSettingsService() @@ -895,7 +895,7 @@ final class WooShippingCreateLabelsViewModelTests: XCTestCase { let shipment = Shipment( contents: [], - purchasedLabelID: nil, + purchasedLabel: nil, currency: "USD", currencySettings: ServiceLocator.currencySettings, shippingSettingsService: MockShippingSettingsService() From b6408ebe17e3c8576dabcf81536deb532aa03bff Mon Sep 17 00:00:00 2001 From: Huong Do Date: Thu, 10 Jul 2025 15:36:50 +0700 Subject: [PATCH 11/13] Exclude refunded labels in split shipment --- .../WooShippingSplitShipmentsViewModel.swift | 8 +++++++- .../WooShippingSplitShipmentsViewModelTests.swift | 5 +++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/WooShipping Create Shipping Labels/WooShipping Split Shipments/WooShippingSplitShipmentsViewModel.swift b/WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/WooShipping Create Shipping Labels/WooShipping Split Shipments/WooShippingSplitShipmentsViewModel.swift index 4c74c9b42f2..3416652b9cd 100644 --- a/WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/WooShipping Create Shipping Labels/WooShipping Split Shipments/WooShippingSplitShipmentsViewModel.swift +++ b/WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/WooShipping Create Shipping Labels/WooShipping Split Shipments/WooShippingSplitShipmentsViewModel.swift @@ -560,9 +560,15 @@ extension WooShippingSplitShipmentsViewModel { shipmentContents.append(content) } + let purchasedLabel: ShippingLabel? = { + guard let label = shipment.shippingLabel, label.refund == nil else { + return nil + } + return label + }() return Shipment(index: Int(shipment.index) ?? 0, contents: shipmentContents, - purchasedLabel: shipment.shippingLabel, + purchasedLabel: purchasedLabel, currency: currency, currencySettings: currencySettings, shippingSettingsService: shippingSettingsService) diff --git a/WooCommerce/WooCommerceTests/ViewRelated/Shipping Label/WooShipping Create Shipping Labels/Split shipments/WooShippingSplitShipmentsViewModelTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/Shipping Label/WooShipping Create Shipping Labels/Split shipments/WooShippingSplitShipmentsViewModelTests.swift index 956184d790c..987b5cdc533 100644 --- a/WooCommerce/WooCommerceTests/ViewRelated/Shipping Label/WooShipping Create Shipping Labels/Split shipments/WooShippingSplitShipmentsViewModelTests.swift +++ b/WooCommerce/WooCommerceTests/ViewRelated/Shipping Label/WooShipping Create Shipping Labels/Split shipments/WooShippingSplitShipmentsViewModelTests.swift @@ -106,9 +106,10 @@ final class WooShippingSplitShipmentsViewModelTests: XCTestCase { // When let label = ShippingLabel.fake().copy(shipmentID: "1") - let shippingLabelData = WooShippingLabelData(currentOrderLabels: [label]) + let refundedLabel = ShippingLabel.fake().copy(shipmentID: "0", refund: .fake()) + let shippingLabelData = WooShippingLabelData(currentOrderLabels: [label, refundedLabel]) let config = WooShippingConfig(siteID: 123, shipments: [ - .fake().copy(index: "0", items: [.init(id: 1, subItems: ["sub-1", "sub-2"])]), + .fake().copy(index: "0", items: [.init(id: 1, subItems: ["sub-1", "sub-2"])], shippingLabel: refundedLabel), .fake().copy(index: "1", items: [.init(id: 2, subItems: [])], shippingLabel: label) ], shippingLabelData: shippingLabelData) let viewModel = WooShippingSplitShipmentsViewModel(order: sampleOrder, From 6085b5eb4ff433944cfb46e323f80bee8fcc3662 Mon Sep 17 00:00:00 2001 From: Huong Do Date: Thu, 10 Jul 2025 17:02:34 +0700 Subject: [PATCH 12/13] Update shipmentsSavedInRemote after purchasing/refunding label --- .../WooShippingSplitShipmentsViewModel.swift | 2 ++ .../WooShippingSplitShipmentsViewModelTests.swift | 2 ++ 2 files changed, 4 insertions(+) diff --git a/WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/WooShipping Create Shipping Labels/WooShipping Split Shipments/WooShippingSplitShipmentsViewModel.swift b/WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/WooShipping Create Shipping Labels/WooShipping Split Shipments/WooShippingSplitShipmentsViewModel.swift index 3416652b9cd..3176ef1ac83 100644 --- a/WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/WooShipping Create Shipping Labels/WooShipping Split Shipments/WooShippingSplitShipmentsViewModel.swift +++ b/WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/WooShipping Create Shipping Labels/WooShipping Split Shipments/WooShippingSplitShipmentsViewModel.swift @@ -170,6 +170,7 @@ final class WooShippingSplitShipmentsViewModel: ObservableObject { currency: order.currency, currencySettings: currencySettings, shippingSettingsService: shippingSettingsService) + shipmentsSavedInRemote = shipments } func didRequestRefund(for shipmentIndex: Int) { @@ -183,6 +184,7 @@ final class WooShippingSplitShipmentsViewModel: ObservableObject { currency: order.currency, currencySettings: currencySettings, shippingSettingsService: shippingSettingsService) + shipmentsSavedInRemote = shipments } func moveSelectedItems(to destination: MoveToShipmentNoticeViewModel.Destination) { diff --git a/WooCommerce/WooCommerceTests/ViewRelated/Shipping Label/WooShipping Create Shipping Labels/Split shipments/WooShippingSplitShipmentsViewModelTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/Shipping Label/WooShipping Create Shipping Labels/Split shipments/WooShippingSplitShipmentsViewModelTests.swift index 987b5cdc533..2a892f26ae8 100644 --- a/WooCommerce/WooCommerceTests/ViewRelated/Shipping Label/WooShipping Create Shipping Labels/Split shipments/WooShippingSplitShipmentsViewModelTests.swift +++ b/WooCommerce/WooCommerceTests/ViewRelated/Shipping Label/WooShipping Create Shipping Labels/Split shipments/WooShippingSplitShipmentsViewModelTests.swift @@ -839,6 +839,7 @@ final class WooShippingSplitShipmentsViewModelTests: XCTestCase { viewModel.didPurchaseLabel(for: shipmentID, label: label) // Then + XCTAssertFalse(viewModel.containsUnsavedChanges) XCTAssertEqual(viewModel.shipments.count, 2) XCTAssertEqual(viewModel.shipments[0].index, 0) XCTAssertEqual(viewModel.shipments[0].purchasedLabel, label) @@ -878,6 +879,7 @@ final class WooShippingSplitShipmentsViewModelTests: XCTestCase { viewModel.didRequestRefund(for: shipmentID) // Then + XCTAssertFalse(viewModel.containsUnsavedChanges) XCTAssertEqual(viewModel.shipments.count, 2) XCTAssertNil(viewModel.shipments[0].purchasedLabel) XCTAssertTrue(viewModel.shipments[0].contents[0].mainItemRow.isSelectable) From cc35a0397f334e7f2df9554445ab91ab96e373d1 Mon Sep 17 00:00:00 2001 From: Huong Do Date: Fri, 11 Jul 2025 09:34:08 +0700 Subject: [PATCH 13/13] Add comment explaining the index format --- .../ShippingLabel/Shipments/WooShippingShipmentItem.swift | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Modules/Sources/Networking/Model/ShippingLabel/Shipments/WooShippingShipmentItem.swift b/Modules/Sources/Networking/Model/ShippingLabel/Shipments/WooShippingShipmentItem.swift index a57f71ef962..c4c1b5f11c2 100644 --- a/Modules/Sources/Networking/Model/ShippingLabel/Shipments/WooShippingShipmentItem.swift +++ b/Modules/Sources/Networking/Model/ShippingLabel/Shipments/WooShippingShipmentItem.swift @@ -10,7 +10,8 @@ public struct WooShippingShipment: Equatable, GeneratedFakeable, GeneratedCopiab /// ID of the order that the shipment belongs to. public let orderID: Int64 - /// Index of the shipment + /// Index of the shipment. + /// The expected format is a numeric string, e.g: "0", "1", etc. public let index: String /// Contents of the shipment