diff --git a/Modules/Sources/Networking/Mapper/WooShippingConfigMapper.swift b/Modules/Sources/Networking/Mapper/WooShippingConfigMapper.swift index 628787eecd5..a9b39028c3e 100644 --- a/Modules/Sources/Networking/Mapper/WooShippingConfigMapper.swift +++ b/Modules/Sources/Networking/Mapper/WooShippingConfigMapper.swift @@ -44,5 +44,5 @@ private struct WooShippingConfigMapperEnvelope: Decodable { extension WooShippingConfigMapper { /// Load only the relevant fields from remote /// - static let fieldsToLoad = "config.shipments, config.shippingLabelData.currentOrderLabels" + static let fieldsToLoad = "config.shipments, config.shippingLabelData.currentOrderLabels, config.shippingLabelData.storedData.selected_destination" } diff --git a/Modules/Sources/Networking/Model/ShippingLabel/Packages/WooShippingPackagePurchase.swift b/Modules/Sources/Networking/Model/ShippingLabel/Packages/WooShippingPackagePurchase.swift index ac7eaa6e925..b2871e37600 100644 --- a/Modules/Sources/Networking/Model/ShippingLabel/Packages/WooShippingPackagePurchase.swift +++ b/Modules/Sources/Networking/Model/ShippingLabel/Packages/WooShippingPackagePurchase.swift @@ -76,7 +76,7 @@ extension WooShippingPackagePurchase { /// shipment ID to set for hazmat and customs form var formattedShipmentID: String { - Values.shipmentIDPrefix + shipmentID + return WooShippingShipmentIDFormatter.formattedShipmentID(shipmentID) } } @@ -219,6 +219,5 @@ extension WooShippingPackagePurchase: Encodable { static let adult = "adult" static let signatureRequired = "signatureRequired" static let adultSignatureRequired = "adultSignatureRequired" - static let shipmentIDPrefix = "shipment_" } } diff --git a/Modules/Sources/Networking/Model/ShippingLabel/Shipments/WooShippingConfigResponse.swift b/Modules/Sources/Networking/Model/ShippingLabel/Shipments/WooShippingConfigResponse.swift index 86e744471ed..02b3b45d1cb 100644 --- a/Modules/Sources/Networking/Model/ShippingLabel/Shipments/WooShippingConfigResponse.swift +++ b/Modules/Sources/Networking/Model/ShippingLabel/Shipments/WooShippingConfigResponse.swift @@ -60,18 +60,60 @@ public struct WooShippingLabelData: Decodable, Equatable { /// Labels purchased for the current order public let currentOrderLabels: [ShippingLabel] - public init(currentOrderLabels: [ShippingLabel]) { + /// Contains destination addresses + public let storedData: StoredData? + + public init( + currentOrderLabels: [ShippingLabel], + storedData: StoredData? = nil + ) { self.currentOrderLabels = currentOrderLabels + self.storedData = storedData } public init(from decoder: Decoder) throws { let container = try decoder.container(keyedBy: CodingKeys.self) - let currentOrderLabels = try container.decodeIfPresent([ShippingLabel].self, forKey: .currentOrderLabels) ?? [] - self.init(currentOrderLabels: currentOrderLabels) + + let storedData = try container.decodeIfPresent(StoredData.self, forKey: .storedData) + let decodedOrderLabels = try container.decodeIfPresent([ShippingLabel].self, forKey: .currentOrderLabels) ?? [] + + /// Inject destination addresses into labels if present + let orderLabels: [ShippingLabel] + if let destinations = storedData?.selectedDestinations, !destinations.isEmpty { + orderLabels = WooShippingLabelData.mapDestinations(destinations, into: decodedOrderLabels) + } else { + orderLabels = decodedOrderLabels + } + + self.init( + currentOrderLabels: orderLabels, + storedData: storedData + ) } private enum CodingKeys: String, CodingKey { case currentOrderLabels + case storedData + } +} + +public extension WooShippingLabelData { + typealias WooShippingLabelDestinations = [String: WooShippingAddress] + + struct StoredData: Decodable, Equatable { + let selectedDestinations: WooShippingLabelDestinations? + + public enum CodingKeys: String, CodingKey { + case selectedDestination = "selected_destination" + } + + public init(from decoder: any Decoder) throws { + let container = try decoder.container(keyedBy: CodingKeys.self) + self.selectedDestinations = try? container.decodeIfPresent( + WooShippingLabelDestinations.self, + forKey: CodingKeys.selectedDestination + ) + } } } diff --git a/Modules/Sources/Networking/Model/ShippingLabel/Shipments/WooShippingLabelData+mapDestinations.swift b/Modules/Sources/Networking/Model/ShippingLabel/Shipments/WooShippingLabelData+mapDestinations.swift new file mode 100644 index 00000000000..380ae9ee8b4 --- /dev/null +++ b/Modules/Sources/Networking/Model/ShippingLabel/Shipments/WooShippingLabelData+mapDestinations.swift @@ -0,0 +1,21 @@ +import Foundation + +extension WooShippingLabelData { + static func mapDestinations( + _ destinations: WooShippingLabelDestinations, + into labels: [ShippingLabel] + ) -> [ShippingLabel] { + return labels.map { label in + guard + let shipmentID = label.shipmentID, + let destinationAddress = destinations[ + WooShippingShipmentIDFormatter.formattedShipmentID(shipmentID) + ] ?? destinations[shipmentID] /// Fallback for ids previously submitted without `shipment_` formatting + else { + return label + } + + return label.copy(destinationAddress: destinationAddress.toShippingLabelAddress()) + } + } +} diff --git a/Modules/Sources/Networking/Model/ShippingLabel/WooShippingAddress.swift b/Modules/Sources/Networking/Model/ShippingLabel/WooShippingAddress.swift index 4c9ffe1ccdb..0cc0d04b766 100644 --- a/Modules/Sources/Networking/Model/ShippingLabel/WooShippingAddress.swift +++ b/Modules/Sources/Networking/Model/ShippingLabel/WooShippingAddress.swift @@ -59,12 +59,12 @@ extension WooShippingAddress: Codable { // If no name is sent to validation address request, no name will be received in response. // So make sure to decode it only if it's present. let name = try container.decodeIfPresent(String.self, forKey: .name) ?? "" - let company = try container.decode(String.self, forKey: .company) - let phone = try container.decode(String.self, forKey: .phone) + let company = try container.decodeIfPresent(String.self, forKey: .company) ?? "" + let phone = try container.decodeIfPresent(String.self, forKey: .phone) ?? "" let country = try container.decode(String.self, forKey: .country) let state = try container.decode(String.self, forKey: .state) let address1 = try container.decodeIfPresent(String.self, forKey: .address1) ?? container.decode(String.self, forKey: .alternateAddress1) - let address2 = try container.decode(String.self, forKey: .address2) + let address2 = try container.decodeIfPresent(String.self, forKey: .address2) ?? "" let city = try container.decode(String.self, forKey: .city) let postcode = try container.decode(String.self, forKey: .postcode) diff --git a/Modules/Sources/Networking/Model/ShippingLabel/WooShippingShipmentIDFormatter.swift b/Modules/Sources/Networking/Model/ShippingLabel/WooShippingShipmentIDFormatter.swift new file mode 100644 index 00000000000..8ca09cab557 --- /dev/null +++ b/Modules/Sources/Networking/Model/ShippingLabel/WooShippingShipmentIDFormatter.swift @@ -0,0 +1,23 @@ +import Foundation + +enum WooShippingShipmentIDFormatter { + /// Turns numeric shipment ID into formatted as `shipment_` + /// - Parameter shipmentID: numeric shipment id + /// - Returns: formated id string + static func formattedShipmentID(_ shipmentID: String) -> String { + return isArgumentIDValid(shipmentID) ? + Values.shipmentIDPrefix + shipmentID : + shipmentID + } +} + +private extension WooShippingShipmentIDFormatter { + private enum Values { + static let shipmentIDPrefix = "shipment_" + } + + /// Make sure we are formatting incoming numeric ID string + private static func isArgumentIDValid(_ argumentID: String) -> Bool { + return Int(argumentID) != nil + } +} diff --git a/Modules/Sources/NetworkingCore/Model/ShippingLabel/ShippingLabelAddress.swift b/Modules/Sources/NetworkingCore/Model/ShippingLabel/ShippingLabelAddress.swift index bfe44a91be9..703631aabf3 100644 --- a/Modules/Sources/NetworkingCore/Model/ShippingLabel/ShippingLabelAddress.swift +++ b/Modules/Sources/NetworkingCore/Model/ShippingLabel/ShippingLabelAddress.swift @@ -117,4 +117,16 @@ extension ShippingLabelAddress { init() { self.init(company: "", name: "", phone: "", country: "", state: "", address1: "", address2: "", city: "", postcode: "") } + + var isEmpty: Bool { + return company.isEmpty && + name.isEmpty && + phone.isEmpty && + country.isEmpty && + state.isEmpty && + address1.isEmpty && + address2.isEmpty && + city.isEmpty && + postcode.isEmpty + } } diff --git a/Modules/Tests/NetworkingTests/Model/WooShippingShipmentIDFormatterTests.swift b/Modules/Tests/NetworkingTests/Model/WooShippingShipmentIDFormatterTests.swift new file mode 100644 index 00000000000..de568055296 --- /dev/null +++ b/Modules/Tests/NetworkingTests/Model/WooShippingShipmentIDFormatterTests.swift @@ -0,0 +1,44 @@ +import Foundation +import XCTest +@testable import Networking + +final class WooShippingShipmentIDFormatterTests: XCTestCase { + + // MARK: - formattedShipmentID + + func test_formatted_shipment_id_when_id_is_numeric_should_return_formatted_id() { + // Given + let sut = WooShippingShipmentIDFormatter.self + let id = "123456" + + // When + let formattedID = sut.formattedShipmentID(id) + + // Then + XCTAssertEqual(formattedID, "shipment_123456") + } + + func test_formatted_shipment_id_when_id_is_already_formatted_should_return_same_id() { + // Given + let sut = WooShippingShipmentIDFormatter.self + let id = "shipment_123456" + + // When + let formattedID = sut.formattedShipmentID(id) + + // Then + XCTAssertEqual(formattedID, "shipment_123456") + } + + func test_formatted_shipment_id_when_non_numeric_should_return_same_id() { + // Given + let sut = WooShippingShipmentIDFormatter.self + let id = "non-numeric-id" + + // When + let formattedID = sut.formattedShipmentID(id) + + // Then + XCTAssertEqual(formattedID, id) + } +} diff --git a/Modules/Tests/NetworkingTests/Remote/WooShippingRemoteTests.swift b/Modules/Tests/NetworkingTests/Remote/WooShippingRemoteTests.swift index 7d5e36fd0a1..52eb73aaada 100644 --- a/Modules/Tests/NetworkingTests/Remote/WooShippingRemoteTests.swift +++ b/Modules/Tests/NetworkingTests/Remote/WooShippingRemoteTests.swift @@ -855,6 +855,63 @@ final class WooShippingRemoteTests: XCTestCase { XCTAssertNotNil(result.failure) } + // MARK: - Load Config With Destinations + + func test_loadConfig_withDestinations_parses_success_response() throws { + // Given + let remote = WooShippingRemote(network: network) + network.simulateResponse(requestUrlSuffix: "config/label-purchase/\(sampleOrderID)", filename: "wooshipping-config-with-destinations") + + // When + let result: Result = waitFor { promise in + remote.loadConfig(siteID: self.sampleSiteID, orderID: self.sampleOrderID) { result in + promise(result) + } + } + + // Then + let config = try XCTUnwrap(result.get()) + let label = try XCTUnwrap(config.shippingLabelData?.currentOrderLabels.first) + XCTAssertNotNil(label.destinationAddress) + XCTAssertEqual(label.destinationAddress.address1, "200 N SPRING ST") + } + + func test_loadConfig_withoutDestinations_parses_success_response() throws { + // Given + let remote = WooShippingRemote(network: network) + network.simulateResponse(requestUrlSuffix: "config/label-purchase/\(sampleOrderID)", filename: "wooshipping-config-without-destinations") + + // When + let result: Result = waitFor { promise in + remote.loadConfig(siteID: self.sampleSiteID, orderID: self.sampleOrderID) { result in + promise(result) + } + } + + // Then + let config = try XCTUnwrap(result.get()) + let label = try XCTUnwrap(config.shippingLabelData?.currentOrderLabels.first) + XCTAssertTrue(label.destinationAddress.isEmpty) + } + + func test_loadConfig_withInvalidDestinations_parses_success_response() throws { + // Given + let remote = WooShippingRemote(network: network) + network.simulateResponse(requestUrlSuffix: "config/label-purchase/\(sampleOrderID)", filename: "wooshipping-config-with-invalid-destinations") + + // When + let result: Result = waitFor { promise in + remote.loadConfig(siteID: self.sampleSiteID, orderID: self.sampleOrderID) { result in + promise(result) + } + } + + // Then + let config = try XCTUnwrap(result.get()) + let label = try XCTUnwrap(config.shippingLabelData?.currentOrderLabels.first) + XCTAssertTrue(label.destinationAddress.isEmpty) + } + // MARK: Update shipment func test_updateShipment_parses_success_response() throws { diff --git a/Modules/Tests/NetworkingTests/Responses/wooshipping-config-with-destinations.json b/Modules/Tests/NetworkingTests/Responses/wooshipping-config-with-destinations.json new file mode 100644 index 00000000000..11d0a18d215 --- /dev/null +++ b/Modules/Tests/NetworkingTests/Responses/wooshipping-config-with-destinations.json @@ -0,0 +1,43 @@ +{ + "data": { + "config": { + "shippingLabelData": { + "storedData": { + "selected_destination": { + "shipment_1": { + "address_1": "200 N SPRING ST", + "city": "LOS ANGELES", + "state": "CA", + "postcode": "90012-4801", + "country": "US" + } + } + }, + "currentOrderLabels": [ + { + "label_id": 4871, + "tracking": null, + "refundable_amount": 0, + "created": 1742292110723, + "carrier_id": "usps", + "service_name": "USPS - Priority Mail", + "status": "PURCHASE_IN_PROGRESS", + "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": "1", + "receipt_item_id": -1, + "created_date": 1742292110000 + } + ] + } + } + } +} diff --git a/Modules/Tests/NetworkingTests/Responses/wooshipping-config-with-invalid-destinations.json b/Modules/Tests/NetworkingTests/Responses/wooshipping-config-with-invalid-destinations.json new file mode 100644 index 00000000000..a73c78d8818 --- /dev/null +++ b/Modules/Tests/NetworkingTests/Responses/wooshipping-config-with-invalid-destinations.json @@ -0,0 +1,39 @@ +{ + "data": { + "config": { + "shippingLabelData": { + "storedData": { + "selected_destination": [ + { + "address_1": "200 N SPRING ST" + } + ] + }, + "currentOrderLabels": [ + { + "label_id": 4871, + "tracking": null, + "refundable_amount": 0, + "created": 1742292110723, + "carrier_id": "usps", + "service_name": "USPS - Priority Mail", + "status": "PURCHASE_IN_PROGRESS", + "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": "1", + "receipt_item_id": -1, + "created_date": 1742292110000 + } + ] + } + } + } +} diff --git a/Modules/Tests/NetworkingTests/Responses/wooshipping-config-without-destinations.json b/Modules/Tests/NetworkingTests/Responses/wooshipping-config-without-destinations.json new file mode 100644 index 00000000000..3f0d49468a2 --- /dev/null +++ b/Modules/Tests/NetworkingTests/Responses/wooshipping-config-without-destinations.json @@ -0,0 +1,32 @@ +{ + "data": { + "config": { + "shippingLabelData": { + "currentOrderLabels": [ + { + "label_id": 4871, + "tracking": null, + "refundable_amount": 0, + "created": 1742292110723, + "carrier_id": "usps", + "service_name": "USPS - Priority Mail", + "status": "PURCHASE_IN_PROGRESS", + "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": "1", + "receipt_item_id": -1, + "created_date": 1742292110000 + } + ] + } + } + } +} diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index 1850e274499..8942c70cd91 100644 --- a/RELEASE-NOTES.txt +++ b/RELEASE-NOTES.txt @@ -7,6 +7,7 @@ 22.7 ----- +- [*] Shipping Labels: Fixed an issue where the destination address was missing for previously purchased shipping labels. [https://github.com/woocommerce/woocommerce-ios/pull/15866] - [*] Shipping Labels: Fixed an issue where the purchase button would display a stale price after changing the origin address. [https://github.com/woocommerce/woocommerce-ios/pull/15795] - [*] Order Details: Fix crash when reloading data [https://github.com/woocommerce/woocommerce-ios/pull/15764] - [*] Shipping Labels: Improved shipment management UI by hiding remove/merge options instead of disabling them, hiding merge option for orders with 2 or fewer unfulfilled shipments, and hiding the ellipsis menu when no remove/merge actions are available [https://github.com/woocommerce/woocommerce-ios/pull/15760]