From 2153187749f23ee1b8165a3c74f14b83cba26008 Mon Sep 17 00:00:00 2001 From: Rachel McRoberts Date: Wed, 31 May 2023 12:52:40 +0100 Subject: [PATCH 1/2] Add fallback when stockQuantity field value is a string --- .../Extensions/KeyedDecodingContainer+Woo.swift | 9 +++++++++ Networking/Networking/Model/Product/Product.swift | 6 +++++- .../Networking/Model/Product/ProductVariation.swift | 6 +++++- .../NetworkingTests/Mapper/ProductMapperTests.swift | 1 + .../Mapper/ProductVariationMapperTests.swift | 1 + .../Responses/product-alternative-types.json | 2 +- .../Responses/product-variation-alternative-types.json | 2 +- 7 files changed, 23 insertions(+), 4 deletions(-) diff --git a/Networking/Networking/Extensions/KeyedDecodingContainer+Woo.swift b/Networking/Networking/Extensions/KeyedDecodingContainer+Woo.swift index 481301e3377..28dd44c4fb2 100644 --- a/Networking/Networking/Extensions/KeyedDecodingContainer+Woo.swift +++ b/Networking/Networking/Extensions/KeyedDecodingContainer+Woo.swift @@ -116,6 +116,11 @@ extension KeyedDecodingContainer { return nil } + /// Decodes a Decimal for the specified key. Supported Encodings = [Decimal / Int / String] + /// + /// This method *does NOT throw*. We want this behavior so that if a malformed entity is received, we just skip it, rather + /// than breaking the entire parsing chain. + /// func failsafeDecodeIfPresent(decimalForKey key: KeyedDecodingContainer.Key) -> Decimal? { if let decimal = failsafeDecodeIfPresent(Decimal.self, forKey: key) { return decimal @@ -125,6 +130,10 @@ extension KeyedDecodingContainer { return Decimal(integerLiteral: integerAsDecimal) } + if let stringAsDecimal = failsafeDecodeIfPresent(String.self, forKey: key) { + return Decimal(string: stringAsDecimal) + } + return nil } diff --git a/Networking/Networking/Model/Product/Product.swift b/Networking/Networking/Model/Product/Product.swift index 7c325a749ec..9ab579df8c0 100644 --- a/Networking/Networking/Model/Product/Product.swift +++ b/Networking/Networking/Model/Product/Product.swift @@ -416,7 +416,11 @@ public struct Product: Codable, GeneratedCopiable, Equatable, GeneratedFakeable }) ]) ?? false - let stockQuantity = try container.decodeIfPresent(Decimal.self, forKey: .stockQuantity) + // Even though WooCommerce Core returns Int or null values, + // some plugins alter the field value from Int to Decimal or String. + // We handle this as an optional Decimal value. + let stockQuantity = container.failsafeDecodeIfPresent(decimalForKey: .stockQuantity) + let stockStatusKey = try container.decode(String.self, forKey: .stockStatusKey) let backordersKey = try container.decode(String.self, forKey: .backordersKey) diff --git a/Networking/Networking/Model/Product/ProductVariation.swift b/Networking/Networking/Model/Product/ProductVariation.swift index d45c05265f4..5d9f4e8901b 100644 --- a/Networking/Networking/Model/Product/ProductVariation.swift +++ b/Networking/Networking/Model/Product/ProductVariation.swift @@ -264,7 +264,11 @@ public struct ProductVariation: Codable, GeneratedCopiable, Equatable, Generated }) ]) ?? false - let stockQuantity = try container.decodeIfPresent(Decimal.self, forKey: .stockQuantity) + // Even though WooCommerce Core returns Int or null values, + // some plugins alter the field value from Int to Decimal or String. + // We handle this as an optional Decimal value. + let stockQuantity = container.failsafeDecodeIfPresent(decimalForKey: .stockQuantity) + let stockStatusKey = try container.decode(String.self, forKey: .stockStatusKey) let stockStatus = ProductStockStatus(rawValue: stockStatusKey) let backordersKey = try container.decode(String.self, forKey: .backordersKey) diff --git a/Networking/NetworkingTests/Mapper/ProductMapperTests.swift b/Networking/NetworkingTests/Mapper/ProductMapperTests.swift index 22ba41eb71a..384a090044c 100644 --- a/Networking/NetworkingTests/Mapper/ProductMapperTests.swift +++ b/Networking/NetworkingTests/Mapper/ProductMapperTests.swift @@ -117,6 +117,7 @@ final class ProductMapperTests: XCTestCase { XCTAssertFalse(product.soldIndividually) XCTAssertTrue(product.purchasable) XCTAssertEqual(product.permalink, "") + XCTAssertNil(product.stockQuantity) } /// Verifies that the `salePrice` field of the Product are parsed correctly when the product is on sale, and the sale price is an empty string diff --git a/Networking/NetworkingTests/Mapper/ProductVariationMapperTests.swift b/Networking/NetworkingTests/Mapper/ProductVariationMapperTests.swift index 64640924e1a..c3818ef8fc2 100644 --- a/Networking/NetworkingTests/Mapper/ProductVariationMapperTests.swift +++ b/Networking/NetworkingTests/Mapper/ProductVariationMapperTests.swift @@ -40,6 +40,7 @@ final class ProductVariationMapperTests: XCTestCase { XCTAssertFalse(productVariation.manageStock) XCTAssertTrue(productVariation.purchasable) XCTAssertEqual(productVariation.permalink, "") + XCTAssertEqual(productVariation.stockQuantity, 16) } /// Test that the fields for variations of a subscription product are properly parsed. diff --git a/Networking/NetworkingTests/Responses/product-alternative-types.json b/Networking/NetworkingTests/Responses/product-alternative-types.json index 08f6d14fba5..230e6b13d2b 100644 --- a/Networking/NetworkingTests/Responses/product-alternative-types.json +++ b/Networking/NetworkingTests/Responses/product-alternative-types.json @@ -36,7 +36,7 @@ "tax_status": "taxable", "tax_class": "", "manage_stock": "parent", - "stock_quantity": null, + "stock_quantity": "", "stock_status": "instock", "backorders": "no", "backorders_allowed": false, diff --git a/Networking/NetworkingTests/Responses/product-variation-alternative-types.json b/Networking/NetworkingTests/Responses/product-variation-alternative-types.json index 638b186d876..b9696c9be91 100644 --- a/Networking/NetworkingTests/Responses/product-variation-alternative-types.json +++ b/Networking/NetworkingTests/Responses/product-variation-alternative-types.json @@ -27,7 +27,7 @@ "tax_status": "taxable", "tax_class": "", "manage_stock": "parent", - "stock_quantity": 16, + "stock_quantity": "16", "stock_status": "instock", "backorders": "notify", "backorders_allowed": true, From 0fbf42d504ba0acee3f1c6ee47eb2fba054e5b9a Mon Sep 17 00:00:00 2001 From: Rachel McRoberts Date: Wed, 31 May 2023 12:59:50 +0100 Subject: [PATCH 2/2] Add stockQuantity decoding fallbacks to 13.9 release notes --- RELEASE-NOTES.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index e4e3bb9b543..7e096f2aa6e 100644 --- a/RELEASE-NOTES.txt +++ b/RELEASE-NOTES.txt @@ -3,6 +3,7 @@ 13.9 ----- - [*] Payments: Location permissions request is not shown to TTP users who grant "Allow once" permission on first foregrounding the app any more [https://github.com/woocommerce/woocommerce-ios/pull/9821] +- [*] Products: Allow alternative types for `stockQuantity` in `Product` and `ProductVariation`, as some third-party plugins alter the type in the API. This helps with the product list not loading due to product decoding errors. [https://github.com/woocommerce/woocommerce-ios/pull/9850] 13.8 -----