From 358daa887fd0aecc94d26cbf72de67969f173547 Mon Sep 17 00:00:00 2001 From: Paolo Musolino Date: Tue, 18 Feb 2025 15:48:40 +0100 Subject: [PATCH 01/25] refactor: moved `ProductImageStatus` into Networking layer and related changes --- .../Networking.xcodeproj/project.pbxproj | 12 ++++++ .../ProductImageStatus.swift | 40 +++++++++++++++++++ .../ServiceLocator/ProductImageUploader.swift | 2 + ...ift => ProductImageStatus+Extension.swift} | 38 ------------------ .../WooCommerce.xcodeproj/project.pbxproj | 16 ++++---- .../Mocks/MockProductImageActionHandler.swift | 2 + Yosemite/Yosemite/Model/Model.swift | 2 + 7 files changed, 66 insertions(+), 46 deletions(-) create mode 100644 Networking/Networking/ProductImageInBackground/ProductImageStatus.swift rename WooCommerce/Classes/ViewRelated/Products/Media/{ProductImageStatus.swift => ProductImageStatus+Extension.swift} (58%) diff --git a/Networking/Networking.xcodeproj/project.pbxproj b/Networking/Networking.xcodeproj/project.pbxproj index e42cbfa4778..8e929383b05 100644 --- a/Networking/Networking.xcodeproj/project.pbxproj +++ b/Networking/Networking.xcodeproj/project.pbxproj @@ -479,6 +479,7 @@ 457453E725A72C9700276508 /* CreateProductVariation.swift in Sources */ = {isa = PBXBuildFile; fileRef = 457453E625A72C9700276508 /* CreateProductVariation.swift */; }; 457A574025D1817E000797AD /* ShippingLabelAddressVerification.swift in Sources */ = {isa = PBXBuildFile; fileRef = 457A573F25D1817E000797AD /* ShippingLabelAddressVerification.swift */; }; 457FC68C2382B2FD00B41B02 /* product-update.json in Resources */ = {isa = PBXBuildFile; fileRef = 457FC68B2382B2FD00B41B02 /* product-update.json */; }; + 4587D1152D64CBC2001971E4 /* ProductImageStatus.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4587D1142D64CBC2001971E4 /* ProductImageStatus.swift */; }; 458C6DE425AC72A1009B300D /* StoredProductSettings.swift in Sources */ = {isa = PBXBuildFile; fileRef = 458C6DE325AC72A1009B300D /* StoredProductSettings.swift */; }; 4595827C264D5B3F000A4413 /* ShippingLabelPackageSelected.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4595827B264D5B3F000A4413 /* ShippingLabelPackageSelected.swift */; }; 4599FC5824A624BD0056157A /* ProductTagListMapper.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4599FC5724A624BD0056157A /* ProductTagListMapper.swift */; }; @@ -1681,6 +1682,7 @@ 457453E625A72C9700276508 /* CreateProductVariation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CreateProductVariation.swift; sourceTree = ""; }; 457A573F25D1817E000797AD /* ShippingLabelAddressVerification.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ShippingLabelAddressVerification.swift; sourceTree = ""; }; 457FC68B2382B2FD00B41B02 /* product-update.json */ = {isa = PBXFileReference; lastKnownFileType = text.json; path = "product-update.json"; sourceTree = ""; }; + 4587D1142D64CBC2001971E4 /* ProductImageStatus.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ProductImageStatus.swift; sourceTree = ""; }; 458C6DE325AC72A1009B300D /* StoredProductSettings.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = StoredProductSettings.swift; sourceTree = ""; }; 4595827B264D5B3F000A4413 /* ShippingLabelPackageSelected.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ShippingLabelPackageSelected.swift; sourceTree = ""; }; 4599FC5724A624BD0056157A /* ProductTagListMapper.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ProductTagListMapper.swift; sourceTree = ""; }; @@ -2671,6 +2673,14 @@ path = "Carriers and Rates"; sourceTree = ""; }; + 4587D1132D64CB70001971E4 /* ProductImageInBackground */ = { + isa = PBXGroup; + children = ( + 4587D1142D64CBC2001971E4 /* ProductImageStatus.swift */, + ); + path = ProductImageInBackground; + sourceTree = ""; + }; 45AE58142306ADA8001901E3 /* Refund */ = { isa = PBXGroup; children = ( @@ -2904,6 +2914,7 @@ B557DA1020975F60005962F4 /* Settings */, B505F6EB20BEFD9100BB1B69 /* Tools */, B53EF539218138EB003E146F /* Validators */, + 4587D1132D64CB70001971E4 /* ProductImageInBackground */, B557D9E6209753AA005962F4 /* Networking.h */, B557D9E7209753AA005962F4 /* Info.plist */, 3FA7D9FA2D547DC700CE5611 /* Networking.xcconfig */, @@ -5315,6 +5326,7 @@ B518662420A099BF00037A38 /* AlamofireNetwork.swift in Sources */, CE070A3A2BBC56CC00017578 /* GiftCardStatsRemote.swift in Sources */, 311D412E2783C07D00052F64 /* StripeAccountMapper.swift in Sources */, + 4587D1152D64CBC2001971E4 /* ProductImageStatus.swift in Sources */, DE78DE482B2AEBEC002E58DE /* WordPressPageMapper.swift in Sources */, CE430676234BA7920073CBFF /* RefundListMapper.swift in Sources */, 45551F122523E7F1007EF104 /* UserAgent.swift in Sources */, diff --git a/Networking/Networking/ProductImageInBackground/ProductImageStatus.swift b/Networking/Networking/ProductImageInBackground/ProductImageStatus.swift new file mode 100644 index 00000000000..9eafb556998 --- /dev/null +++ b/Networking/Networking/ProductImageInBackground/ProductImageStatus.swift @@ -0,0 +1,40 @@ +import Foundation +import Photos + +/// The status of a Product image. +/// +public enum ProductImageStatus: Equatable { + /// An image asset is being uploaded. + /// + case uploading(asset: ProductImageAssetType) + + /// The Product image exists remotely. + /// + case remote(image: ProductImage) + + /// An image asset upload failed. + /// + case uploadFailure(asset: ProductImageAssetType, error: Error) + + public static func == (lhs: Self, rhs: Self) -> Bool { + switch (lhs, rhs) { + case let (.uploading(lAsset), .uploading(rAsset)): + lAsset == rAsset + case let (.remote(lImage), .remote(image: rImage)): + lImage == rImage + case let (.uploadFailure(lAsset, lError), .uploadFailure(rAsset, rError)): + lAsset == rAsset && (lError as NSError) == (rError as NSError) + default: + false + } + } +} + +/// The type of product image asset. +public enum ProductImageAssetType: Equatable { + /// `PHAsset` from device photo library or camera capture. + case phAsset(asset: PHAsset) + + /// `UIImage` from image processing. The filename and alt text need to be provided separately. + case uiImage(image: UIImage, filename: String?, altText: String?) +} diff --git a/WooCommerce/Classes/ServiceLocator/ProductImageUploader.swift b/WooCommerce/Classes/ServiceLocator/ProductImageUploader.swift index bd43ef75733..7f08c119834 100644 --- a/WooCommerce/Classes/ServiceLocator/ProductImageUploader.swift +++ b/WooCommerce/Classes/ServiceLocator/ProductImageUploader.swift @@ -3,6 +3,8 @@ import Foundation import struct Yosemite.ProductImage import enum Yosemite.ProductAction import protocol Yosemite.StoresManager +import enum Yosemite.ProductImageStatus +import enum Yosemite.ProductImageAssetType /// Information about a background product image upload error. struct ProductImageUploadErrorInfo { diff --git a/WooCommerce/Classes/ViewRelated/Products/Media/ProductImageStatus.swift b/WooCommerce/Classes/ViewRelated/Products/Media/ProductImageStatus+Extension.swift similarity index 58% rename from WooCommerce/Classes/ViewRelated/Products/Media/ProductImageStatus.swift rename to WooCommerce/Classes/ViewRelated/Products/Media/ProductImageStatus+Extension.swift index 44c1ac357c4..0c5c2bccf81 100644 --- a/WooCommerce/Classes/ViewRelated/Products/Media/ProductImageStatus.swift +++ b/WooCommerce/Classes/ViewRelated/Products/Media/ProductImageStatus+Extension.swift @@ -1,44 +1,6 @@ import Photos import Yosemite -/// The status of a Product image. -/// -enum ProductImageStatus: Equatable { - /// An image asset is being uploaded. - /// - case uploading(asset: ProductImageAssetType) - - /// The Product image exists remotely. - /// - case remote(image: ProductImage) - - /// An image asset upload failed. - /// - case uploadFailure(asset: ProductImageAssetType, error: Error) - - static func == (lhs: Self, rhs: Self) -> Bool { - switch (lhs, rhs) { - case let (.uploading(lAsset), .uploading(rAsset)): - lAsset == rAsset - case let (.remote(lImage), .remote(image: rImage)): - lImage == rImage - case let (.uploadFailure(lAsset, lError), .uploadFailure(rAsset, rError)): - lAsset == rAsset && (lError as NSError) == (rError as NSError) - default: - false - } - } -} - -/// The type of product image asset. -enum ProductImageAssetType: Equatable { - /// `PHAsset` from device photo library or camera capture. - case phAsset(asset: PHAsset) - - /// `UIImage` from image processing. The filename and alt text need to be provided separately. - case uiImage(image: UIImage, filename: String?, altText: String?) -} - extension Collection where Element == ProductImageStatus { var images: [ProductImage] { compactMap { status in diff --git a/WooCommerce/WooCommerce.xcodeproj/project.pbxproj b/WooCommerce/WooCommerce.xcodeproj/project.pbxproj index 9e27f58ca8f..19ed9f91dc9 100644 --- a/WooCommerce/WooCommerce.xcodeproj/project.pbxproj +++ b/WooCommerce/WooCommerce.xcodeproj/project.pbxproj @@ -236,7 +236,6 @@ 0245465D24EE779D004F531C /* ProductFormEventLogger.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0245465C24EE779D004F531C /* ProductFormEventLogger.swift */; }; 0245465F24EE9106004F531C /* ProductVariationFormEventLogger.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0245465E24EE9106004F531C /* ProductVariationFormEventLogger.swift */; }; 0246405F258B122100C10A7D /* PrintShippingLabelCoordinatorTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0246405E258B122100C10A7D /* PrintShippingLabelCoordinatorTests.swift */; }; - 0247F50E286E6CCD009C177E /* MockProductImageActionHandler.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0247F50D286E6CCD009C177E /* MockProductImageActionHandler.swift */; }; 0247F510286E7D26009C177E /* ProductVariationFormViewModel+ImageUploaderTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0247F50F286E7D26009C177E /* ProductVariationFormViewModel+ImageUploaderTests.swift */; }; 0247F512286F73EA009C177E /* WooAnalyticsEvent+ImageUpload.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0247F511286F73EA009C177E /* WooAnalyticsEvent+ImageUpload.swift */; }; 0248042D2887C92A00991319 /* MockLoggedOutAppSettings.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0248042C2887C92A00991319 /* MockLoggedOutAppSettings.swift */; }; @@ -602,7 +601,7 @@ 02E8B17723E2C49000A43403 /* InProgressProductImageCollectionViewCell.swift in Sources */ = {isa = PBXBuildFile; fileRef = 02E8B17523E2C49000A43403 /* InProgressProductImageCollectionViewCell.swift */; }; 02E8B17823E2C49000A43403 /* InProgressProductImageCollectionViewCell.xib in Resources */ = {isa = PBXBuildFile; fileRef = 02E8B17623E2C49000A43403 /* InProgressProductImageCollectionViewCell.xib */; }; 02E8B17A23E2C4BD00A43403 /* CircleSpinnerView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 02E8B17923E2C4BD00A43403 /* CircleSpinnerView.swift */; }; - 02E8B17C23E2C78A00A43403 /* ProductImageStatus.swift in Sources */ = {isa = PBXBuildFile; fileRef = 02E8B17B23E2C78A00A43403 /* ProductImageStatus.swift */; }; + 02E8B17C23E2C78A00A43403 /* ProductImageStatus+Extension.swift in Sources */ = {isa = PBXBuildFile; fileRef = 02E8B17B23E2C78A00A43403 /* ProductImageStatus+Extension.swift */; }; 02E8B17E23E2C8D900A43403 /* ProductImageActionHandler.swift in Sources */ = {isa = PBXBuildFile; fileRef = 02E8B17D23E2C8D900A43403 /* ProductImageActionHandler.swift */; }; 02EA6BF82435E80600FFF90A /* ImageDownloader.swift in Sources */ = {isa = PBXBuildFile; fileRef = 02EA6BF72435E80600FFF90A /* ImageDownloader.swift */; }; 02EA6BFA2435E92600FFF90A /* KingfisherImageDownloader+ImageDownloadable.swift in Sources */ = {isa = PBXBuildFile; fileRef = 02EA6BF92435E92600FFF90A /* KingfisherImageDownloader+ImageDownloadable.swift */; }; @@ -1409,6 +1408,7 @@ 4580BA7423F192D400B5F764 /* ProductSettingsViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4580BA7223F192D400B5F764 /* ProductSettingsViewController.swift */; }; 4580BA7523F192D400B5F764 /* ProductSettingsViewController.xib in Resources */ = {isa = PBXBuildFile; fileRef = 4580BA7323F192D400B5F764 /* ProductSettingsViewController.xib */; }; 4580BA7723F19D4A00B5F764 /* ProductSettingsViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4580BA7623F19D4A00B5F764 /* ProductSettingsViewModel.swift */; }; + 4587D11B2D64D2F0001971E4 /* MockProductImageActionHandler.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4587D11A2D64D2F0001971E4 /* MockProductImageActionHandler.swift */; }; 458BAC6E2C57CDA6009440EA /* ProductPasswordEligibilityUseCase.swift in Sources */ = {isa = PBXBuildFile; fileRef = 458BAC6D2C57CDA6009440EA /* ProductPasswordEligibilityUseCase.swift */; }; 458BAC702C57E38F009440EA /* ProductPasswordEligibilityUseCaseTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 458BAC6F2C57E38F009440EA /* ProductPasswordEligibilityUseCaseTests.swift */; }; 459097F823CDE47F00DEA9E0 /* UIAlertController+Helpers.swift in Sources */ = {isa = PBXBuildFile; fileRef = 459097F723CDE47F00DEA9E0 /* UIAlertController+Helpers.swift */; }; @@ -3444,7 +3444,6 @@ 0245465C24EE779D004F531C /* ProductFormEventLogger.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ProductFormEventLogger.swift; sourceTree = ""; }; 0245465E24EE9106004F531C /* ProductVariationFormEventLogger.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ProductVariationFormEventLogger.swift; sourceTree = ""; }; 0246405E258B122100C10A7D /* PrintShippingLabelCoordinatorTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PrintShippingLabelCoordinatorTests.swift; sourceTree = ""; }; - 0247F50D286E6CCD009C177E /* MockProductImageActionHandler.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockProductImageActionHandler.swift; sourceTree = ""; }; 0247F50F286E7D26009C177E /* ProductVariationFormViewModel+ImageUploaderTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "ProductVariationFormViewModel+ImageUploaderTests.swift"; sourceTree = ""; }; 0247F511286F73EA009C177E /* WooAnalyticsEvent+ImageUpload.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "WooAnalyticsEvent+ImageUpload.swift"; sourceTree = ""; }; 0248042C2887C92A00991319 /* MockLoggedOutAppSettings.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockLoggedOutAppSettings.swift; sourceTree = ""; }; @@ -3811,7 +3810,7 @@ 02E8B17523E2C49000A43403 /* InProgressProductImageCollectionViewCell.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = InProgressProductImageCollectionViewCell.swift; sourceTree = ""; }; 02E8B17623E2C49000A43403 /* InProgressProductImageCollectionViewCell.xib */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = file.xib; path = InProgressProductImageCollectionViewCell.xib; sourceTree = ""; }; 02E8B17923E2C4BD00A43403 /* CircleSpinnerView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CircleSpinnerView.swift; sourceTree = ""; }; - 02E8B17B23E2C78A00A43403 /* ProductImageStatus.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ProductImageStatus.swift; sourceTree = ""; }; + 02E8B17B23E2C78A00A43403 /* ProductImageStatus+Extension.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "ProductImageStatus+Extension.swift"; sourceTree = ""; }; 02E8B17D23E2C8D900A43403 /* ProductImageActionHandler.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ProductImageActionHandler.swift; sourceTree = ""; }; 02EA6BF72435E80600FFF90A /* ImageDownloader.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ImageDownloader.swift; sourceTree = ""; }; 02EA6BF92435E92600FFF90A /* KingfisherImageDownloader+ImageDownloadable.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "KingfisherImageDownloader+ImageDownloadable.swift"; sourceTree = ""; }; @@ -4562,6 +4561,7 @@ 4580BA7223F192D400B5F764 /* ProductSettingsViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ProductSettingsViewController.swift; sourceTree = ""; }; 4580BA7323F192D400B5F764 /* ProductSettingsViewController.xib */ = {isa = PBXFileReference; lastKnownFileType = file.xib; path = ProductSettingsViewController.xib; sourceTree = ""; }; 4580BA7623F19D4A00B5F764 /* ProductSettingsViewModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ProductSettingsViewModel.swift; sourceTree = ""; }; + 4587D11A2D64D2F0001971E4 /* MockProductImageActionHandler.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockProductImageActionHandler.swift; sourceTree = ""; }; 458BAC6D2C57CDA6009440EA /* ProductPasswordEligibilityUseCase.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ProductPasswordEligibilityUseCase.swift; sourceTree = ""; }; 458BAC6F2C57E38F009440EA /* ProductPasswordEligibilityUseCaseTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ProductPasswordEligibilityUseCaseTests.swift; sourceTree = ""; }; 459097F723CDE47F00DEA9E0 /* UIAlertController+Helpers.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "UIAlertController+Helpers.swift"; sourceTree = ""; }; @@ -7438,7 +7438,7 @@ 0286B27823C7051F003D784B /* ProductImagesViewController.xib */, 450C2CB424D1ABB100D570DD /* ProductImagesGalleryViewController.swift */, 450C2CB524D1ABB200D570DD /* ProductImagesGalleryViewController.xib */, - 02E8B17B23E2C78A00A43403 /* ProductImageStatus.swift */, + 02E8B17B23E2C78A00A43403 /* ProductImageStatus+Extension.swift */, 02E8B17D23E2C8D900A43403 /* ProductImageActionHandler.swift */, 024EFA6823FCC10B00F36918 /* Product+Media.swift */, 027B8BB723FE0CB30040944E /* DefaultProductUIImageLoader.swift */, @@ -9940,6 +9940,7 @@ 02A275BF23FE58F6005C560F /* MockImageCache.swift */, 02A275C123FE590A005C560F /* MockKingfisherImageDownloader.swift */, 02A275C323FE5B64005C560F /* MockPHAssetImageLoader.swift */, + 4587D11A2D64D2F0001971E4 /* MockProductImageActionHandler.swift */, 02A275C523FE9EFC005C560F /* MockFeatureFlagService.swift */, 02B653AB2429F7BF00A9C839 /* MockTaxClassStoresManager.swift */, 02EA6BFB2435EC3500FFF90A /* MockImageDownloader.swift */, @@ -9969,7 +9970,6 @@ 02BF9BAE2851E7EA008CE2DD /* MockAppleIDCredentialChecker.swift */, EECB7EDF2862115C0028C888 /* MockProductImageUploader.swift */, EEEA41F12869A5F400AEFC4B /* MockProductImagesProductIDUpdater.swift */, - 0247F50D286E6CCD009C177E /* MockProductImageActionHandler.swift */, 0248042C2887C92A00991319 /* MockLoggedOutAppSettings.swift */, 02829BA9288FA8B300951E1E /* MockUserNotification.swift */, B958A7D228B52A2300823EEF /* MockRoute.swift */, @@ -16228,7 +16228,7 @@ DA1D68C22C36F0980097859A /* PointOfSaleAssets.swift in Sources */, 0210D8692A7BEEF700846F8C /* WooAnalyticsEvent+ProductListFilter.swift in Sources */, B958A7C728B3D44A00823EEF /* UniversalLinkRouter.swift in Sources */, - 02E8B17C23E2C78A00A43403 /* ProductImageStatus.swift in Sources */, + 02E8B17C23E2C78A00A43403 /* ProductImageStatus+Extension.swift in Sources */, 03F5CB832A0C3A1A0026877A /* AnimatedPlaceholder.swift in Sources */, 0259D5FF2581F3FA003B1CD6 /* ShippingLabelPaperSizeOptionsViewController.swift in Sources */, 02EA6BFA2435E92600FFF90A /* KingfisherImageDownloader+ImageDownloadable.swift in Sources */, @@ -17753,7 +17753,6 @@ 20FCBCE12CE24CE70082DCA3 /* MockPOSItemProvider.swift in Sources */, 02524A5D252ED5C60033E7BD /* ProductVariationLoadUseCaseTests.swift in Sources */, 028AFFB62484EDA000693C09 /* Dictionary+LoggingTests.swift in Sources */, - 0247F50E286E6CCD009C177E /* MockProductImageActionHandler.swift in Sources */, AEA3F91527BEC96B00B9F555 /* PriceFieldFormatterTests.swift in Sources */, 6879B8DB287AFFA100A0F9A8 /* CardReaderManualsViewModelTests.swift in Sources */, 02BC5AA824D2802B00C43326 /* MockProductVariationStoresManager.swift in Sources */, @@ -17763,6 +17762,7 @@ E1C6535C27BD1D0A003E87D4 /* CardPresentConfigurationLoaderTests.swift in Sources */, 2688644325D471C700821BA5 /* EditAttributesViewModelTests.swift in Sources */, 8697AFBF2B622DEA00EFAF21 /* BlazeAddParameterViewModelTests.swift in Sources */, + 4587D11B2D64D2F0001971E4 /* MockProductImageActionHandler.swift in Sources */, 023BD5882BFDCF3100A10D7B /* MockInMemoryStorage.swift in Sources */, DE001323279A793A00EB0350 /* CouponWooTests.swift in Sources */, 45B98E1F25DECC1C00A1232B /* ShippingLabelAddressFormViewModelTests.swift in Sources */, diff --git a/WooCommerce/WooCommerceTests/Mocks/MockProductImageActionHandler.swift b/WooCommerce/WooCommerceTests/Mocks/MockProductImageActionHandler.swift index 0e5bafcd366..9211643bd70 100644 --- a/WooCommerce/WooCommerceTests/Mocks/MockProductImageActionHandler.swift +++ b/WooCommerce/WooCommerceTests/Mocks/MockProductImageActionHandler.swift @@ -3,6 +3,8 @@ import Photos @testable import WooCommerce import struct Yosemite.Media import struct Yosemite.ProductImage +import enum Yosemite.ProductImageStatus +import enum Yosemite.ProductImageAssetType final class MockProductImageActionHandler: ProductImageActionHandlerProtocol { typealias AllStatuses = [ProductImageStatus] diff --git a/Yosemite/Yosemite/Model/Model.swift b/Yosemite/Yosemite/Model/Model.swift index ac596b756e3..d55c2d061fc 100644 --- a/Yosemite/Yosemite/Model/Model.swift +++ b/Yosemite/Yosemite/Model/Model.swift @@ -241,6 +241,8 @@ public typealias WCAnalyticsStatsTotals = Networking.WCAnalyticsStatsTotals public typealias WordPressPage = Networking.WordPressPage public typealias WordPressTheme = Networking.WordPressTheme public typealias MetaData = Networking.MetaData +public typealias ProductImageStatus = Networking.ProductImageStatus +public typealias ProductImageAssetType = Networking.ProductImageAssetType // MARK: - Exported Storage Symbols From 992cc5c0c19c9c84e044751306fe834947574a79 Mon Sep 17 00:00:00 2001 From: Paolo Musolino Date: Tue, 18 Feb 2025 16:32:00 +0100 Subject: [PATCH 02/25] Add Codable conformance and new file for storing product image statuses in UserDefaults - Modify `ProductImageStatus.swift` to add Codable conformance and additional properties - Create `ProductImagesUserDefaultsStatuses.swift` for managing product image statuses in User Defaults - Move `ProductOrVariationID` in a separate file to define a type-safe identifier for products and variations - Update `Model.swift` to include `ProductOrVariationID` typealias --- .../Networking.xcodeproj/project.pbxproj | 8 + .../ProductImageStatus.swift | 153 ++++++++++++++++-- .../ProductImagesUserDefaultsStatuses.swift | 81 ++++++++++ .../ProductOrVariationID.swift | 54 +++++++ .../ServiceLocator/ProductImageUploader.swift | 17 +- Yosemite/Yosemite/Model/Model.swift | 1 + 6 files changed, 283 insertions(+), 31 deletions(-) create mode 100644 Networking/Networking/ProductImageInBackground/ProductImagesUserDefaultsStatuses.swift create mode 100644 Networking/Networking/ProductImageInBackground/ProductOrVariationID.swift diff --git a/Networking/Networking.xcodeproj/project.pbxproj b/Networking/Networking.xcodeproj/project.pbxproj index 8e929383b05..1a535bd4051 100644 --- a/Networking/Networking.xcodeproj/project.pbxproj +++ b/Networking/Networking.xcodeproj/project.pbxproj @@ -480,6 +480,8 @@ 457A574025D1817E000797AD /* ShippingLabelAddressVerification.swift in Sources */ = {isa = PBXBuildFile; fileRef = 457A573F25D1817E000797AD /* ShippingLabelAddressVerification.swift */; }; 457FC68C2382B2FD00B41B02 /* product-update.json in Resources */ = {isa = PBXBuildFile; fileRef = 457FC68B2382B2FD00B41B02 /* product-update.json */; }; 4587D1152D64CBC2001971E4 /* ProductImageStatus.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4587D1142D64CBC2001971E4 /* ProductImageStatus.swift */; }; + 4587D11D2D64D559001971E4 /* ProductImagesUserDefaultsStatuses.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4587D11C2D64D556001971E4 /* ProductImagesUserDefaultsStatuses.swift */; }; + 4587D11F2D64D887001971E4 /* ProductOrVariationID.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4587D11E2D64D886001971E4 /* ProductOrVariationID.swift */; }; 458C6DE425AC72A1009B300D /* StoredProductSettings.swift in Sources */ = {isa = PBXBuildFile; fileRef = 458C6DE325AC72A1009B300D /* StoredProductSettings.swift */; }; 4595827C264D5B3F000A4413 /* ShippingLabelPackageSelected.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4595827B264D5B3F000A4413 /* ShippingLabelPackageSelected.swift */; }; 4599FC5824A624BD0056157A /* ProductTagListMapper.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4599FC5724A624BD0056157A /* ProductTagListMapper.swift */; }; @@ -1683,6 +1685,8 @@ 457A573F25D1817E000797AD /* ShippingLabelAddressVerification.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ShippingLabelAddressVerification.swift; sourceTree = ""; }; 457FC68B2382B2FD00B41B02 /* product-update.json */ = {isa = PBXFileReference; lastKnownFileType = text.json; path = "product-update.json"; sourceTree = ""; }; 4587D1142D64CBC2001971E4 /* ProductImageStatus.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ProductImageStatus.swift; sourceTree = ""; }; + 4587D11C2D64D556001971E4 /* ProductImagesUserDefaultsStatuses.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ProductImagesUserDefaultsStatuses.swift; sourceTree = ""; }; + 4587D11E2D64D886001971E4 /* ProductOrVariationID.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ProductOrVariationID.swift; sourceTree = ""; }; 458C6DE325AC72A1009B300D /* StoredProductSettings.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = StoredProductSettings.swift; sourceTree = ""; }; 4595827B264D5B3F000A4413 /* ShippingLabelPackageSelected.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ShippingLabelPackageSelected.swift; sourceTree = ""; }; 4599FC5724A624BD0056157A /* ProductTagListMapper.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ProductTagListMapper.swift; sourceTree = ""; }; @@ -2677,6 +2681,8 @@ isa = PBXGroup; children = ( 4587D1142D64CBC2001971E4 /* ProductImageStatus.swift */, + 4587D11E2D64D886001971E4 /* ProductOrVariationID.swift */, + 4587D11C2D64D556001971E4 /* ProductImagesUserDefaultsStatuses.swift */, ); path = ProductImageInBackground; sourceTree = ""; @@ -5283,6 +5289,7 @@ 02C254B925637BA000A04423 /* OrderShippingLabelListMapper.swift in Sources */, 261CF1B8255AE62D0090D8D3 /* PaymentGatewayRemote.swift in Sources */, CE0A0F1522396BF00075ED8D /* ProductAttribute.swift in Sources */, + 4587D11F2D64D887001971E4 /* ProductOrVariationID.swift in Sources */, 026CF620237D69D6009563D4 /* ProductVariationsRemote.swift in Sources */, 03DCB7402624AD7D00C8953D /* CouponListMapper.swift in Sources */, 077F39D626A58E4500ABEADC /* SystemStatusRemote.swift in Sources */, @@ -5333,6 +5340,7 @@ CED9BCC12D3EAF7B00C063B8 /* WooShippingAddressValidationError.swift in Sources */, 45CDAFAB2434CA9300F83C22 /* ProductCatalogVisibility.swift in Sources */, 3148977027232982007A86BD /* SystemStatus.swift in Sources */, + 4587D11D2D64D559001971E4 /* ProductImagesUserDefaultsStatuses.swift in Sources */, B557DA1820979D51005962F4 /* Credentials.swift in Sources */, DEC51AF72769A15B009F3DF4 /* SystemStatusReport+Security.swift in Sources */, EE1D9A9F2ACD6BA60020D817 /* AIProduct.swift in Sources */, diff --git a/Networking/Networking/ProductImageInBackground/ProductImageStatus.swift b/Networking/Networking/ProductImageInBackground/ProductImageStatus.swift index 9eafb556998..a404027e38e 100644 --- a/Networking/Networking/ProductImageInBackground/ProductImageStatus.swift +++ b/Networking/Networking/ProductImageInBackground/ProductImageStatus.swift @@ -1,40 +1,163 @@ import Foundation import Photos +import UIKit /// The status of a Product image. /// -public enum ProductImageStatus: Equatable { + +public enum ProductImageStatus: Equatable, Codable { /// An image asset is being uploaded. /// - case uploading(asset: ProductImageAssetType) - + case uploading(asset: ProductImageAssetType, siteID: Int64, productID: ProductOrVariationID) + /// The Product image exists remotely. /// - case remote(image: ProductImage) - + case remote(image: ProductImage, siteID: Int64, productID: ProductOrVariationID) + /// An image asset upload failed. /// - case uploadFailure(asset: ProductImageAssetType, error: Error) - + case uploadFailure(asset: ProductImageAssetType, error: Error, siteID: Int64, productID: ProductOrVariationID) + + private enum CodingKeys: String, CodingKey { + case type + case asset + case image + case error + case siteID + case productID + } + + public init(from decoder: Decoder) throws { + let container = try decoder.container(keyedBy: CodingKeys.self) + let typeString = try container.decode(String.self, forKey: .type) + switch typeString { + case "uploading": + let asset = try container.decode(ProductImageAssetType.self, forKey: .asset) + let sID = try container.decode(Int64.self, forKey: .siteID) + let pID = try container.decode(ProductOrVariationID.self, forKey: .productID) + self = .uploading(asset: asset, siteID: sID, productID: pID) + case "remote": + let image = try container.decode(ProductImage.self, forKey: .image) + let sID = try container.decode(Int64.self, forKey: .siteID) + let pID = try container.decode(ProductOrVariationID.self, forKey: .productID) + self = .remote(image: image, siteID: sID, productID: pID) + case "uploadFailure": + let asset = try container.decode(ProductImageAssetType.self, forKey: .asset) + let errorMessage = try container.decode(String.self, forKey: .error) + let error = NSError(domain: "ProductImageStatus", code: 0, userInfo: [NSLocalizedDescriptionKey: errorMessage]) + let sID = try container.decode(Int64.self, forKey: .siteID) + let pID = try container.decode(ProductOrVariationID.self, forKey: .productID) + self = .uploadFailure(asset: asset, error: error, siteID: sID, productID: pID) + default: + throw DecodingError.dataCorruptedError(forKey: .type, + in: container, + debugDescription: "Invalid type value: \(typeString)") + } + } + + public func encode(to encoder: Encoder) throws { + var container = encoder.container(keyedBy: CodingKeys.self) + switch self { + case .uploading(let asset, let siteID, let productID): + try container.encode("uploading", forKey: .type) + try container.encode(asset, forKey: .asset) + try container.encode(siteID, forKey: .siteID) + try container.encode(productID, forKey: .productID) + case .remote(let image, let siteID, let productID): + try container.encode("remote", forKey: .type) + try container.encode(image, forKey: .image) + try container.encode(siteID, forKey: .siteID) + try container.encode(productID, forKey: .productID) + case .uploadFailure(let asset, let error, let siteID, let productID): + try container.encode("uploadFailure", forKey: .type) + try container.encode(asset, forKey: .asset) + let errorMessage = (error as NSError).localizedDescription + try container.encode(errorMessage, forKey: .error) + try container.encode(siteID, forKey: .siteID) + try container.encode(productID, forKey: .productID) + } + } + public static func == (lhs: Self, rhs: Self) -> Bool { switch (lhs, rhs) { - case let (.uploading(lAsset), .uploading(rAsset)): - lAsset == rAsset - case let (.remote(lImage), .remote(image: rImage)): - lImage == rImage - case let (.uploadFailure(lAsset, lError), .uploadFailure(rAsset, rError)): - lAsset == rAsset && (lError as NSError) == (rError as NSError) + case let (.uploading(lAsset, lSiteID, lProductID), .uploading(rAsset, rSiteID, rProductID)): + return lAsset == rAsset && lSiteID == rSiteID && lProductID == rProductID + case let (.remote(lImage, lSiteID, lProductID), .remote(rImage, rSiteID, rProductID)): + return lImage == rImage && lSiteID == rSiteID && lProductID == rProductID + case let (.uploadFailure(lAsset, lError, lSiteID, lProductID), .uploadFailure(rAsset, rError, rSiteID, rProductID)): + return lAsset == rAsset && + (lError as NSError) == (rError as NSError) && + lSiteID == rSiteID && + lProductID == rProductID default: - false + return false } } } /// The type of product image asset. -public enum ProductImageAssetType: Equatable { +public enum ProductImageAssetType: Equatable, Codable { /// `PHAsset` from device photo library or camera capture. case phAsset(asset: PHAsset) /// `UIImage` from image processing. The filename and alt text need to be provided separately. case uiImage(image: UIImage, filename: String?, altText: String?) + + private enum CodingKeys: String, CodingKey { + case type + case asset // For phAsset: localIdentifier string + case imageData // For uiImage: base64 encoded image data + case filename + case altText + } + + public init(from decoder: Decoder) throws { + let container = try decoder.container(keyedBy: CodingKeys.self) + let typeString = try container.decode(String.self, forKey: .type) + switch typeString { + case "phAsset": + let localIdentifier = try container.decode(String.self, forKey: .asset) + let fetchResult = PHAsset.fetchAssets(withLocalIdentifiers: [localIdentifier], options: nil) + guard let asset = fetchResult.firstObject else { + throw DecodingError.dataCorruptedError(forKey: .asset, + in: container, + debugDescription: "No PHAsset found with localIdentifier \(localIdentifier)") + } + self = .phAsset(asset: asset) + case "uiImage": + let base64String = try container.decode(String.self, forKey: .imageData) + guard let imageData = Data(base64Encoded: base64String), + let image = UIImage(data: imageData) else { + throw DecodingError.dataCorruptedError(forKey: .imageData, + in: container, + debugDescription: "Invalid image data") + } + let filename = try container.decodeIfPresent(String.self, forKey: .filename) + let altText = try container.decodeIfPresent(String.self, forKey: .altText) + self = .uiImage(image: image, filename: filename, altText: altText) + default: + throw DecodingError.dataCorruptedError(forKey: .type, + in: container, + debugDescription: "Unknown type \(typeString)") + } + } + + public func encode(to encoder: Encoder) throws { + var container = encoder.container(keyedBy: CodingKeys.self) + switch self { + case .phAsset(let asset): + try container.encode("phAsset", forKey: .type) + try container.encode(asset.localIdentifier, forKey: .asset) + case .uiImage(let image, let filename, let altText): + try container.encode("uiImage", forKey: .type) + guard let imageData = image.pngData() else { + let context = EncodingError.Context(codingPath: container.codingPath, debugDescription: "Unable to encode UIImage as PNG") + throw EncodingError.invalidValue(image, context) + } + let base64String = imageData.base64EncodedString() + try container.encode(base64String, forKey: .imageData) + try container.encode(filename, forKey: .filename) + try container.encode(altText, forKey: .altText) + } + } } diff --git a/Networking/Networking/ProductImageInBackground/ProductImagesUserDefaultsStatuses.swift b/Networking/Networking/ProductImageInBackground/ProductImagesUserDefaultsStatuses.swift new file mode 100644 index 00000000000..43e3739a5b4 --- /dev/null +++ b/Networking/Networking/ProductImageInBackground/ProductImagesUserDefaultsStatuses.swift @@ -0,0 +1,81 @@ +import Foundation + +/// Save product image upload statuses in User Defaults. +/// This class is declared in the Networking layer because it will also be accessed by the background URLSession operations. +/// +final class ProductImagesUserDefaultsStatuses { + private static let key = "savedProductUploadImageStatuses" + + static func addStatus(_ status: ProductImageStatus) { + var statuses = getAllStatuses() + statuses.append(status) + saveAllStatuses(statuses) + } + + static func removeStatus(_ status: ProductImageStatus) { + var statuses = getAllStatuses() + statuses.removeAll(where: { $0 == status }) + saveAllStatuses(statuses) + } + + static func findStatus(where predicate: (ProductImageStatus) -> Bool) -> ProductImageStatus? { + return getAllStatuses().first(where: predicate) + } + + static func getAllStatuses() -> [ProductImageStatus] { + guard let data = UserDefaults.standard.data(forKey: key) else { + return [] + } + do { + let statuses = try JSONDecoder().decode([ProductImageStatus].self, from: data) + return statuses + } catch { + DDLogError("Error decoding saved product image statuses: \(error)") + return [] + } + } + + static func getAllStatuses(for siteID: Int64, productID: ProductOrVariationID) -> [ProductImageStatus] { + return getAllStatuses().filter { status in + switch status { + case .uploading(_, let sID, let pID), + .remote(_, let sID, let pID), + .uploadFailure(_, _, let sID, let pID): + return sID == siteID && pID == productID + } + } + } + + static func clearAllStatuses() { + UserDefaults.standard.removeObject(forKey: key) + } + + private static func saveAllStatuses(_ statuses: [ProductImageStatus]) { + do { + let data = try JSONEncoder().encode(statuses) + UserDefaults.standard.set(data, forKey: key) + } catch { + DDLogError("Error encoding saved product image statuses: \(error)") + } + } +} + +extension ProductImagesUserDefaultsStatuses { + static func setAllStatuses(_ statuses: [ProductImageStatus]) { + saveAllStatuses(statuses) + } + + static func setAllStatuses(_ statuses: [ProductImageStatus], for siteID: Int64, productID: ProductOrVariationID) { + // Merge with existing, removing any old statuses for this site/product + var all = getAllStatuses().filter { st in + switch st { + case .uploading(_, let sID, let pID), + .remote(_, let sID, let pID), + .uploadFailure(_, _, let sID, let pID): + return !(sID == siteID && pID == productID) + } + } + all.append(contentsOf: statuses) + saveAllStatuses(all) + } +} diff --git a/Networking/Networking/ProductImageInBackground/ProductOrVariationID.swift b/Networking/Networking/ProductImageInBackground/ProductOrVariationID.swift new file mode 100644 index 00000000000..f335f8c231d --- /dev/null +++ b/Networking/Networking/ProductImageInBackground/ProductOrVariationID.swift @@ -0,0 +1,54 @@ +/// Identifiable data about a product or product variation. +public enum ProductOrVariationID: Equatable, Hashable, Codable { + case product(id: Int64) + case variation(productID: Int64, variationID: Int64) + + /// Returns the product ID for product type and variation ID for variation type. + public var id: Int64 { + switch self { + case .product(let id): + return id + case .variation(_, let variationID): + return variationID + } + } + + private enum CodingKeys: String, CodingKey { + case type + case id + case productID + case variationID + } + + private enum CaseType: String, Codable { + case product + case variation + } + + public init(from decoder: Decoder) throws { + let container = try decoder.container(keyedBy: CodingKeys.self) + let caseType = try container.decode(CaseType.self, forKey: .type) + switch caseType { + case .product: + let id = try container.decode(Int64.self, forKey: .id) + self = .product(id: id) + case .variation: + let productID = try container.decode(Int64.self, forKey: .productID) + let variationID = try container.decode(Int64.self, forKey: .variationID) + self = .variation(productID: productID, variationID: variationID) + } + } + + public func encode(to encoder: Encoder) throws { + var container = encoder.container(keyedBy: CodingKeys.self) + switch self { + case .product(let id): + try container.encode(CaseType.product, forKey: .type) + try container.encode(id, forKey: .id) + case .variation(let productID, let variationID): + try container.encode(CaseType.variation, forKey: .type) + try container.encode(productID, forKey: .productID) + try container.encode(variationID, forKey: .variationID) + } + } +} diff --git a/WooCommerce/Classes/ServiceLocator/ProductImageUploader.swift b/WooCommerce/Classes/ServiceLocator/ProductImageUploader.swift index 7f08c119834..acb78da8008 100644 --- a/WooCommerce/Classes/ServiceLocator/ProductImageUploader.swift +++ b/WooCommerce/Classes/ServiceLocator/ProductImageUploader.swift @@ -5,6 +5,7 @@ import enum Yosemite.ProductAction import protocol Yosemite.StoresManager import enum Yosemite.ProductImageStatus import enum Yosemite.ProductImageAssetType +import enum Yosemite.ProductOrVariationID /// Information about a background product image upload error. struct ProductImageUploadErrorInfo { @@ -13,22 +14,6 @@ struct ProductImageUploadErrorInfo { let error: ProductImageUploaderError } -/// Identifiable data about a product or product variation. -enum ProductOrVariationID: Equatable, Hashable { - case product(id: Int64) - case variation(productID: Int64, variationID: Int64) - - /// Returns the product ID for product type and variation ID for variation type. - var id: Int64 { - switch self { - case .product(let id): - return id - case .variation(_, let variationID): - return variationID - } - } -} - /// Identifiable information about a specific product or product variation of different sites for image upload. struct ProductImageUploaderKey: Equatable, Hashable { let siteID: Int64 diff --git a/Yosemite/Yosemite/Model/Model.swift b/Yosemite/Yosemite/Model/Model.swift index d55c2d061fc..f02a1a87ae7 100644 --- a/Yosemite/Yosemite/Model/Model.swift +++ b/Yosemite/Yosemite/Model/Model.swift @@ -243,6 +243,7 @@ public typealias WordPressTheme = Networking.WordPressTheme public typealias MetaData = Networking.MetaData public typealias ProductImageStatus = Networking.ProductImageStatus public typealias ProductImageAssetType = Networking.ProductImageAssetType +public typealias ProductOrVariationID = Networking.ProductOrVariationID // MARK: - Exported Storage Symbols From b7b2078073bf85bded0d68490906f4bad43fce5a Mon Sep 17 00:00:00 2001 From: Paolo Musolino Date: Tue, 18 Feb 2025 16:33:32 +0100 Subject: [PATCH 03/25] fix: spaces in `ProductImageStatus` --- .../ProductImageStatus.swift | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/Networking/Networking/ProductImageInBackground/ProductImageStatus.swift b/Networking/Networking/ProductImageInBackground/ProductImageStatus.swift index a404027e38e..460796632e2 100644 --- a/Networking/Networking/ProductImageInBackground/ProductImageStatus.swift +++ b/Networking/Networking/ProductImageInBackground/ProductImageStatus.swift @@ -9,15 +9,15 @@ public enum ProductImageStatus: Equatable, Codable { /// An image asset is being uploaded. /// case uploading(asset: ProductImageAssetType, siteID: Int64, productID: ProductOrVariationID) - + /// The Product image exists remotely. /// case remote(image: ProductImage, siteID: Int64, productID: ProductOrVariationID) - + /// An image asset upload failed. /// case uploadFailure(asset: ProductImageAssetType, error: Error, siteID: Int64, productID: ProductOrVariationID) - + private enum CodingKeys: String, CodingKey { case type case asset @@ -26,7 +26,7 @@ public enum ProductImageStatus: Equatable, Codable { case siteID case productID } - + public init(from decoder: Decoder) throws { let container = try decoder.container(keyedBy: CodingKeys.self) let typeString = try container.decode(String.self, forKey: .type) @@ -54,7 +54,7 @@ public enum ProductImageStatus: Equatable, Codable { debugDescription: "Invalid type value: \(typeString)") } } - + public func encode(to encoder: Encoder) throws { var container = encoder.container(keyedBy: CodingKeys.self) switch self { @@ -77,7 +77,7 @@ public enum ProductImageStatus: Equatable, Codable { try container.encode(productID, forKey: .productID) } } - + public static func == (lhs: Self, rhs: Self) -> Bool { switch (lhs, rhs) { case let (.uploading(lAsset, lSiteID, lProductID), .uploading(rAsset, rSiteID, rProductID)): @@ -141,7 +141,7 @@ public enum ProductImageAssetType: Equatable, Codable { debugDescription: "Unknown type \(typeString)") } } - + public func encode(to encoder: Encoder) throws { var container = encoder.container(keyedBy: CodingKeys.self) switch self { From a804b1eb05451e184224aa46e6439e874addd84d Mon Sep 17 00:00:00 2001 From: Paolo Musolino Date: Tue, 18 Feb 2025 17:52:16 +0100 Subject: [PATCH 04/25] Add siteID and productID to ProductImageStatus - Update `ProductImagesCollectionViewDataSource.swift` to include siteID and productID in `ProductImageStatus` cases. - Modify `ProductImagesHeaderTableViewCell.swift` to handle additional parameters in `ProductImageStatus`. - Adjust `Product+Media.swift` to pass siteID and productID when mapping images to `ProductImageStatus`. - Change `ProductImageActionHandler.swift` to incorporate siteID and productID in `ProductImageStatus` handling and updating. - Update `ProductImageStatus+Extension.swift` to manage new parameters in `ProductImageStatus`. - Amend `ProductImagesCollectionViewController.swift` to handle siteID and productID in status cases. - Modify `ProductImagesSaver.swift` to use siteID and productID in status comparison and updates. - Adjust `ProductImagesViewController.swift` to manage `ProductImageStatus` with siteID and productID. --- ...roductImagesCollectionViewDataSource.swift | 6 ++--- .../ProductImagesHeaderTableViewCell.swift | 4 ++-- .../Products/Media/Product+Media.swift | 4 ++-- .../Media/ProductImageActionHandler.swift | 24 ++++++++++--------- .../Media/ProductImageStatus+Extension.swift | 6 ++--- ...roductImagesCollectionViewController.swift | 8 +++---- .../Products/Media/ProductImagesSaver.swift | 6 ++--- .../Media/ProductImagesViewController.swift | 2 +- 8 files changed, 31 insertions(+), 29 deletions(-) diff --git a/WooCommerce/Classes/ViewRelated/Products/Cells/Product Images/ProductImagesCollectionViewDataSource.swift b/WooCommerce/Classes/ViewRelated/Products/Cells/Product Images/ProductImagesCollectionViewDataSource.swift index c849280893a..ff96307717b 100644 --- a/WooCommerce/Classes/ViewRelated/Products/Cells/Product Images/ProductImagesCollectionViewDataSource.swift +++ b/WooCommerce/Classes/ViewRelated/Products/Cells/Product Images/ProductImagesCollectionViewDataSource.swift @@ -51,16 +51,16 @@ private extension ProductImagesCollectionViewDataSource { func configureImageCell(_ cell: UICollectionViewCell, productImageStatus: ProductImageStatus, isFirstImage: Bool) { switch productImageStatus { - case .remote(let image): + case .remote(let image, let siteID, let productID): configureRemoteImageCell(cell, productImage: image, isFirstImage: isFirstImage) - case .uploading(let asset): + case .uploading(let asset, let siteID, let productID): switch asset { case .phAsset(let asset): configureUploadingImageCell(cell, asset: asset) case .uiImage(let image, _, _): configureUploadingImageCell(cell, image: image) } - case let .uploadFailure(asset, _): + case let .uploadFailure(asset, _, _, _): switch asset { case .phAsset(let asset): configureFailedImageCell(cell, asset: asset) diff --git a/WooCommerce/Classes/ViewRelated/Products/Cells/Product Images/ProductImagesHeaderTableViewCell.swift b/WooCommerce/Classes/ViewRelated/Products/Cells/Product Images/ProductImagesHeaderTableViewCell.swift index 8ad6abba0f3..49dd2879b02 100644 --- a/WooCommerce/Classes/ViewRelated/Products/Cells/Product Images/ProductImagesHeaderTableViewCell.swift +++ b/WooCommerce/Classes/ViewRelated/Products/Cells/Product Images/ProductImagesHeaderTableViewCell.swift @@ -72,11 +72,11 @@ extension ProductImagesHeaderTableViewCell: UICollectionViewDelegate { switch viewModel?.items[indexPath.item] { case .image(let status): switch status { - case .remote(let image): + case .remote(let image, _, _): onImageSelected?(image, indexPath) case .uploading: onImageSelected?(nil, indexPath) - case let .uploadFailure(asset, error): + case let .uploadFailure(asset, error, _, _): onFailedUploadSelected?(asset, error) } case .addImage: diff --git a/WooCommerce/Classes/ViewRelated/Products/Media/Product+Media.swift b/WooCommerce/Classes/ViewRelated/Products/Media/Product+Media.swift index 6d76a3b1214..76d2c750329 100644 --- a/WooCommerce/Classes/ViewRelated/Products/Media/Product+Media.swift +++ b/WooCommerce/Classes/ViewRelated/Products/Media/Product+Media.swift @@ -3,13 +3,13 @@ import Yosemite extension ProductFormDataModel { var imageStatuses: [ProductImageStatus] { - return images.map({ ProductImageStatus.remote(image: $0) }) + return images.map({ ProductImageStatus.remote(image: $0, siteID: siteID, productID: .product(id: productID)) }) } } extension Product { var imageStatuses: [ProductImageStatus] { - return images.map({ ProductImageStatus.remote(image: $0) }) + return images.map({ ProductImageStatus.remote(image: $0, siteID: siteID, productID: .product(id: productID)) }) } /// Returns the URL of the first image, if available. Otherwise, nil is returned. diff --git a/WooCommerce/Classes/ViewRelated/Products/Media/ProductImageActionHandler.swift b/WooCommerce/Classes/ViewRelated/Products/Media/ProductImageActionHandler.swift index bd8af7331bb..7c4187c13d6 100644 --- a/WooCommerce/Classes/ViewRelated/Products/Media/ProductImageActionHandler.swift +++ b/WooCommerce/Classes/ViewRelated/Products/Media/ProductImageActionHandler.swift @@ -162,7 +162,9 @@ final class ProductImageActionHandler: ProductImageActionHandlerProtocol { return } - let newProductImageStatuses = mediaItems.map { ProductImageStatus.remote(image: $0.toProductImage) } + let newProductImageStatuses = mediaItems.map { ProductImageStatus.remote(image: $0.toProductImage, + siteID: self.siteID, + productID: self.productOrVariationID) } let imageStatuses = newProductImageStatuses + self.productImageStatuses self.productImageStatuses = imageStatuses } @@ -174,7 +176,7 @@ final class ProductImageActionHandler: ProductImageActionHandlerProtocol { return } - productImageStatuses = [.uploading(asset: asset)] + self.productImageStatuses + productImageStatuses = [.uploading(asset: asset, siteID: self.siteID, productID: self.productOrVariationID)] + self.productImageStatuses self.uploadMediaAssetToSiteMediaLibrary(asset: asset) { [weak self] result in self?.queue.async { [weak self] in @@ -257,10 +259,10 @@ final class ProductImageActionHandler: ProductImageActionHandlerProtocol { var imageStatuses = self.productImageStatuses imageStatuses.removeAll { status -> Bool in - guard case .remote(let image) = status else { + guard case .remote(let image, let siteID, let productID) = status else { return false } - return image.imageID == productImage.imageID + return image.imageID == productImage.imageID && siteID == self.siteID && productID == self.productOrVariationID } self.productImageStatuses = imageStatuses } @@ -295,9 +297,9 @@ private extension ProductImageActionHandler { func index(of asset: ProductImageAssetType) -> Int? { return productImageStatuses.firstIndex(where: { status -> Bool in switch status { - case .uploading(let uploadingAsset): - return uploadingAsset == asset - case let .uploadFailure(failedAsset, _): + case .uploading(let uploadingAsset, let siteID, let productID): + return uploadingAsset == asset && siteID == self.siteID && productID == self.productOrVariationID + case let .uploadFailure(failedAsset, _, _, _): return failedAsset == asset case .remote: return false @@ -306,21 +308,21 @@ private extension ProductImageActionHandler { } func updateProductImageStatus(at index: Int, productImage: ProductImage) { - if case .uploading(let asset) = productImageStatuses[safe: index] { + if case .uploading(let asset, _, _) = productImageStatuses[safe: index] { observations.assetUploaded.values.forEach { closure in closure(asset, .success(productImage)) } } - productImageStatuses[index] = .remote(image: productImage) + productImageStatuses[index] = .remote(image: productImage, siteID: siteID, productID: productOrVariationID) } func updateProductImageStatus(at index: Int, error: Error) { - if case .uploading(let asset) = productImageStatuses[safe: index] { + if case .uploading(let asset, let siteID, let productID) = productImageStatuses[safe: index] { observations.assetUploaded.values.forEach { closure in closure(asset, .failure(error)) } - productImageStatuses[index] = .uploadFailure(asset: asset, error: error) + productImageStatuses[index] = .uploadFailure(asset: asset, error: error, siteID: siteID, productID: productID) } else { productImageStatuses.remove(at: index) } diff --git a/WooCommerce/Classes/ViewRelated/Products/Media/ProductImageStatus+Extension.swift b/WooCommerce/Classes/ViewRelated/Products/Media/ProductImageStatus+Extension.swift index 0c5c2bccf81..ffb9f6e8876 100644 --- a/WooCommerce/Classes/ViewRelated/Products/Media/ProductImageStatus+Extension.swift +++ b/WooCommerce/Classes/ViewRelated/Products/Media/ProductImageStatus+Extension.swift @@ -5,7 +5,7 @@ extension Collection where Element == ProductImageStatus { var images: [ProductImage] { compactMap { status in switch status { - case .remote(let productImage): + case .remote(let productImage, let siteID, let productID): return productImage default: return nil @@ -48,7 +48,7 @@ extension ProductImageStatus { /// var dragItemIdentifier: String { switch self { - case .uploading(let asset): + case .uploading(let asset, let siteID, let productID): switch asset { case let .phAsset(asset): return asset.identifier() @@ -57,7 +57,7 @@ extension ProductImageStatus { } case .uploadFailure: return UUID().uuidString - case .remote(let image): + case .remote(let image, let siteID, let productID): return "\(image.imageID)" } } diff --git a/WooCommerce/Classes/ViewRelated/Products/Media/ProductImagesCollectionViewController.swift b/WooCommerce/Classes/ViewRelated/Products/Media/ProductImagesCollectionViewController.swift index bc524798859..c5b5c35703c 100644 --- a/WooCommerce/Classes/ViewRelated/Products/Media/ProductImagesCollectionViewController.swift +++ b/WooCommerce/Classes/ViewRelated/Products/Media/ProductImagesCollectionViewController.swift @@ -87,16 +87,16 @@ extension ProductImagesCollectionViewController { private extension ProductImagesCollectionViewController { func configureCell(_ cell: UICollectionViewCell, productImageStatus: ProductImageStatus, isFirstImage: Bool) { switch productImageStatus { - case .remote(let image): + case .remote(let image, _, _): configureRemoteImageCell(cell, productImage: image, isFirstImage: isFirstImage) - case .uploading(let asset): + case .uploading(let asset, _, _): switch asset { case .phAsset(let asset): configureUploadingImageCell(cell, asset: asset) case .uiImage(let image, _, _): configureUploadingImageCell(cell, image: image) } - case let .uploadFailure(asset, _): + case let .uploadFailure(asset, _, _, _): switch asset { case .phAsset(let asset): configureFailedImageCell(cell, asset: asset) @@ -186,7 +186,7 @@ extension ProductImagesCollectionViewController { switch status { case .remote: break - case let .uploadFailure(asset, error): + case let .uploadFailure(asset, error, siteID, productID): return onFailedUploadSelected(asset, error) case .uploading: return diff --git a/WooCommerce/Classes/ViewRelated/Products/Media/ProductImagesSaver.swift b/WooCommerce/Classes/ViewRelated/Products/Media/ProductImagesSaver.swift index fee1f4f8395..72e20067a1a 100644 --- a/WooCommerce/Classes/ViewRelated/Products/Media/ProductImagesSaver.swift +++ b/WooCommerce/Classes/ViewRelated/Products/Media/ProductImagesSaver.swift @@ -129,8 +129,8 @@ private extension ProductImagesSaver { guard let self = self else { return } guard let index = self.imageStatusesToSave.firstIndex(where: { status -> Bool in switch status { - case .uploading(let uploadingAsset): - return uploadingAsset == asset + case .uploading(let uploadingAsset, let siteID, let productID): + return uploadingAsset == asset && siteID == self.siteID && productID == self.productOrVariationID default: return false } @@ -148,7 +148,7 @@ private extension ProductImagesSaver { } func updateProductImageStatus(at index: Int, productImage: ProductImage) { - imageStatusesToSave[index] = .remote(image: productImage) + imageStatusesToSave[index] = .remote(image: productImage, siteID: siteID, productID: productOrVariationID) } func updateProductImageStatus(at index: Int, error: Error?) { diff --git a/WooCommerce/Classes/ViewRelated/Products/Media/ProductImagesViewController.swift b/WooCommerce/Classes/ViewRelated/Products/Media/ProductImagesViewController.swift index d1e66484cc1..4a788796141 100644 --- a/WooCommerce/Classes/ViewRelated/Products/Media/ProductImagesViewController.swift +++ b/WooCommerce/Classes/ViewRelated/Products/Media/ProductImagesViewController.swift @@ -26,7 +26,7 @@ final class ProductImagesViewController: UIViewController { private var productImages: [ProductImage] { return productImageStatuses.compactMap { status in switch status { - case .remote(let productImage): + case .remote(let productImage, let siteID, let productID): return productImage default: return nil From e3c87b8267f4fa54db575945c0569087a143bb01 Mon Sep 17 00:00:00 2001 From: Paolo Musolino Date: Tue, 18 Feb 2025 22:18:55 +0100 Subject: [PATCH 05/25] fix: when a product is published and `updateProductID(_:)` is called, all statuses without the correct value are now updated. This ensures that even if the statuses were initially created with a nil or outdated productID, they will be updated to the new remoteProductID. --- .../Products/Media/ProductImageActionHandler.swift | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/WooCommerce/Classes/ViewRelated/Products/Media/ProductImageActionHandler.swift b/WooCommerce/Classes/ViewRelated/Products/Media/ProductImageActionHandler.swift index 7c4187c13d6..f2165adc5b0 100644 --- a/WooCommerce/Classes/ViewRelated/Products/Media/ProductImageActionHandler.swift +++ b/WooCommerce/Classes/ViewRelated/Products/Media/ProductImageActionHandler.swift @@ -249,6 +249,16 @@ final class ProductImageActionHandler: ProductImageActionHandlerProtocol { /// func updateProductID(_ remoteProductID: ProductOrVariationID) { self.productOrVariationID = remoteProductID + self.productImageStatuses = self.productImageStatuses.map { status in + switch status { + case .uploading(let asset, let siteID, _): + return .uploading(asset: asset, siteID: siteID, productID: remoteProductID) + case .uploadFailure(let asset, let error, let siteID, _): + return .uploadFailure(asset: asset, error: error, siteID: siteID, productID: remoteProductID) + default: + return status + } + } } func deleteProductImage(_ productImage: ProductImage) { From 66c6694748f053a9e044b8e0352fa4d9392f76b8 Mon Sep 17 00:00:00 2001 From: Paolo Musolino Date: Tue, 18 Feb 2025 22:19:51 +0100 Subject: [PATCH 06/25] - Update `ProductImageStatus.swift` to make `productID` optional in `uploading` and `uploadFailure` cases - Modify `ProductImagesUserDefaultsStatuses.swift` to handle optional `productID` in filtering statuses --- .../ProductImageStatus.swift | 12 ++++---- .../ProductImagesUserDefaultsStatuses.swift | 30 +++++++++++++++---- 2 files changed, 30 insertions(+), 12 deletions(-) diff --git a/Networking/Networking/ProductImageInBackground/ProductImageStatus.swift b/Networking/Networking/ProductImageInBackground/ProductImageStatus.swift index 460796632e2..cf20b51b121 100644 --- a/Networking/Networking/ProductImageInBackground/ProductImageStatus.swift +++ b/Networking/Networking/ProductImageInBackground/ProductImageStatus.swift @@ -8,7 +8,7 @@ import UIKit public enum ProductImageStatus: Equatable, Codable { /// An image asset is being uploaded. /// - case uploading(asset: ProductImageAssetType, siteID: Int64, productID: ProductOrVariationID) + case uploading(asset: ProductImageAssetType, siteID: Int64, productID: ProductOrVariationID?) /// The Product image exists remotely. /// @@ -16,7 +16,7 @@ public enum ProductImageStatus: Equatable, Codable { /// An image asset upload failed. /// - case uploadFailure(asset: ProductImageAssetType, error: Error, siteID: Int64, productID: ProductOrVariationID) + case uploadFailure(asset: ProductImageAssetType, error: Error, siteID: Int64, productID: ProductOrVariationID?) private enum CodingKeys: String, CodingKey { case type @@ -34,7 +34,7 @@ public enum ProductImageStatus: Equatable, Codable { case "uploading": let asset = try container.decode(ProductImageAssetType.self, forKey: .asset) let sID = try container.decode(Int64.self, forKey: .siteID) - let pID = try container.decode(ProductOrVariationID.self, forKey: .productID) + let pID = try container.decodeIfPresent(ProductOrVariationID.self, forKey: .productID) self = .uploading(asset: asset, siteID: sID, productID: pID) case "remote": let image = try container.decode(ProductImage.self, forKey: .image) @@ -46,7 +46,7 @@ public enum ProductImageStatus: Equatable, Codable { let errorMessage = try container.decode(String.self, forKey: .error) let error = NSError(domain: "ProductImageStatus", code: 0, userInfo: [NSLocalizedDescriptionKey: errorMessage]) let sID = try container.decode(Int64.self, forKey: .siteID) - let pID = try container.decode(ProductOrVariationID.self, forKey: .productID) + let pID = try container.decodeIfPresent(ProductOrVariationID.self, forKey: .productID) self = .uploadFailure(asset: asset, error: error, siteID: sID, productID: pID) default: throw DecodingError.dataCorruptedError(forKey: .type, @@ -62,7 +62,7 @@ public enum ProductImageStatus: Equatable, Codable { try container.encode("uploading", forKey: .type) try container.encode(asset, forKey: .asset) try container.encode(siteID, forKey: .siteID) - try container.encode(productID, forKey: .productID) + try container.encodeIfPresent(productID, forKey: .productID) case .remote(let image, let siteID, let productID): try container.encode("remote", forKey: .type) try container.encode(image, forKey: .image) @@ -74,7 +74,7 @@ public enum ProductImageStatus: Equatable, Codable { let errorMessage = (error as NSError).localizedDescription try container.encode(errorMessage, forKey: .error) try container.encode(siteID, forKey: .siteID) - try container.encode(productID, forKey: .productID) + try container.encodeIfPresent(productID, forKey: .productID) } } diff --git a/Networking/Networking/ProductImageInBackground/ProductImagesUserDefaultsStatuses.swift b/Networking/Networking/ProductImageInBackground/ProductImagesUserDefaultsStatuses.swift index 43e3739a5b4..790df97490e 100644 --- a/Networking/Networking/ProductImageInBackground/ProductImagesUserDefaultsStatuses.swift +++ b/Networking/Networking/ProductImageInBackground/ProductImagesUserDefaultsStatuses.swift @@ -35,13 +35,22 @@ final class ProductImagesUserDefaultsStatuses { } } - static func getAllStatuses(for siteID: Int64, productID: ProductOrVariationID) -> [ProductImageStatus] { + static func getAllStatuses(for siteID: Int64, productID: ProductOrVariationID?) -> [ProductImageStatus] { return getAllStatuses().filter { status in switch status { + case .remote(_, let sID, let pID): + if let filterProductID = productID { + return sID == siteID && pID == filterProductID + } else { + return false + } case .uploading(_, let sID, let pID), - .remote(_, let sID, let pID), .uploadFailure(_, _, let sID, let pID): - return sID == siteID && pID == productID + if let filterProductID = productID { + return sID == siteID && pID == filterProductID + } else { + return sID == siteID && pID == nil + } } } } @@ -65,14 +74,23 @@ extension ProductImagesUserDefaultsStatuses { saveAllStatuses(statuses) } - static func setAllStatuses(_ statuses: [ProductImageStatus], for siteID: Int64, productID: ProductOrVariationID) { + static func setAllStatuses(_ statuses: [ProductImageStatus], for siteID: Int64, productID: ProductOrVariationID?) { // Merge with existing, removing any old statuses for this site/product var all = getAllStatuses().filter { st in switch st { + case .remote(_, let sID, let pID): + if let filterProductID = productID { + return !(sID == siteID && pID == filterProductID) + } else { + return true + } case .uploading(_, let sID, let pID), - .remote(_, let sID, let pID), .uploadFailure(_, _, let sID, let pID): - return !(sID == siteID && pID == productID) + if let filterProductID = productID { + return !(sID == siteID && pID == filterProductID) + } else { + return !(sID == siteID && pID == nil) + } } } all.append(contentsOf: statuses) From 6c678cf86c2fd0eca703e01780b76bf2cc7d4634 Mon Sep 17 00:00:00 2001 From: Paolo Musolino Date: Tue, 18 Feb 2025 23:42:39 +0100 Subject: [PATCH 07/25] update: unit tests regarding product image upload classes --- .../Mocks/MockProductImageActionHandler.swift | 1 + ...tionFormViewModel+ImageUploaderTests.swift | 6 +- .../ProductImageActionHandlerTests.swift | 134 +++++++++++++----- .../ProductImageStatus+HelpersTests.swift | 21 +-- .../Media/ProductImageUploaderTests.swift | 94 ++++++------ .../Media/ProductImagesSaverTests.swift | 16 ++- 6 files changed, 173 insertions(+), 99 deletions(-) diff --git a/WooCommerce/WooCommerceTests/Mocks/MockProductImageActionHandler.swift b/WooCommerce/WooCommerceTests/Mocks/MockProductImageActionHandler.swift index 9211643bd70..1c7fef2ff14 100644 --- a/WooCommerce/WooCommerceTests/Mocks/MockProductImageActionHandler.swift +++ b/WooCommerce/WooCommerceTests/Mocks/MockProductImageActionHandler.swift @@ -5,6 +5,7 @@ import struct Yosemite.Media import struct Yosemite.ProductImage import enum Yosemite.ProductImageStatus import enum Yosemite.ProductImageAssetType +import enum Yosemite.ProductOrVariationID final class MockProductImageActionHandler: ProductImageActionHandlerProtocol { typealias AllStatuses = [ProductImageStatus] diff --git a/WooCommerce/WooCommerceTests/ViewRelated/Products/Edit Product/ProductVariationFormViewModel+ImageUploaderTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/Products/Edit Product/ProductVariationFormViewModel+ImageUploaderTests.swift index 405a65c1b65..24dacafd077 100644 --- a/WooCommerce/WooCommerceTests/ViewRelated/Products/Edit Product/ProductVariationFormViewModel+ImageUploaderTests.swift +++ b/WooCommerce/WooCommerceTests/ViewRelated/Products/Edit Product/ProductVariationFormViewModel+ImageUploaderTests.swift @@ -8,6 +8,8 @@ import Yosemite final class ProductVariationFormViewModel_ImageUploaderTests: XCTestCase { private var storesManager: MockStoresManager! private var subscriptions: Set = [] + private let siteID: Int64 = 1234 + private let productID = ProductOrVariationID.product(id: 5678) override func setUp() { super.setUp() @@ -23,7 +25,9 @@ final class ProductVariationFormViewModel_ImageUploaderTests: XCTestCase { // Given let productVariation = ProductVariation.fake().copy(status: .published) let model = EditableProductVariationModel(productVariation: productVariation) - let productImageActionHandler = MockProductImageActionHandler(productImageStatuses: [.uploading(asset: .phAsset(asset: PHAsset()))]) + let productImageActionHandler = MockProductImageActionHandler(productImageStatuses: [.uploading(asset: .phAsset(asset: PHAsset()), + siteID: siteID, + productID: productID)]) let viewModel = ProductVariationFormViewModel(productVariation: model, productImageActionHandler: productImageActionHandler, storesManager: storesManager) diff --git a/WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImageActionHandlerTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImageActionHandlerTests.swift index 9a9fadeecc6..fb8cfd616fc 100644 --- a/WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImageActionHandlerTests.swift +++ b/WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImageActionHandlerTests.swift @@ -8,8 +8,10 @@ import XCTest final class ProductImageActionHandlerTests: XCTestCase { private var productImageStatusesSubscription: AnyCancellable? private var assetUploadSubscription: AnyCancellable? + private let siteID: Int64 = 1234 + private let productID = ProductOrVariationID.product(id: 5678) - func testUploadingMediaSuccessfully() { + func test_uploading_media_successfully() { let mockMedia = createMockMedia() let mockUploadedProductImage = ProductImage(imageID: mockMedia.mediaID, dateCreated: mockMedia.date, @@ -24,18 +26,20 @@ final class ProductImageActionHandlerTests: XCTestCase { ProductImage(imageID: 1, dateCreated: Date(), dateModified: Date(), src: "", name: "", alt: ""), ProductImage(imageID: 2, dateCreated: Date(), dateModified: Date(), src: "", name: "", alt: "") ] - let mockRemoteProductImageStatuses = mockProductImages.map { ProductImageStatus.remote(image: $0) } - let mockProduct = Product.fake().copy(images: mockProductImages) + let mockRemoteProductImageStatuses = mockProductImages.map { ProductImageStatus.remote(image: $0, siteID: 0, productID: productID) } + let mockProduct = Product.fake().copy(siteID: 0, productID: productID.id, images: mockProductImages) let model = EditableProductModel(product: mockProduct) - let productImageActionHandler = ProductImageActionHandler(siteID: 123, + let productImageActionHandler = ProductImageActionHandler(siteID: siteID, product: model) let mockAsset = PHAsset() let expectedStatusUpdates: [[ProductImageStatus]] = [ mockRemoteProductImageStatuses, - [.uploading(asset: .phAsset(asset: mockAsset))] + mockRemoteProductImageStatuses, - [.remote(image: mockUploadedProductImage)] + mockRemoteProductImageStatuses + [.uploading(asset: .phAsset(asset: mockAsset), siteID: siteID, productID: productID)] + mockRemoteProductImageStatuses, + [.remote(image: mockUploadedProductImage, + siteID: siteID, + productID: productID)] + mockRemoteProductImageStatuses ] let waitForStatusUpdates = self.expectation(description: "Wait for status updates from image upload") @@ -70,7 +74,7 @@ final class ProductImageActionHandlerTests: XCTestCase { XCTAssertEqual(observedProductImageStatusChanges, expectedStatusUpdates) } - func testUploadingMediaUnsuccessfully() { + func test_uploading_media_unsuccessfully() { let mockStoresManager = MockMediaStoresManager(media: nil, sessionManager: SessionManager.testingInstance) ServiceLocator.setStores(mockStoresManager) @@ -78,18 +82,23 @@ final class ProductImageActionHandlerTests: XCTestCase { ProductImage(imageID: 1, dateCreated: Date(), dateModified: Date(), src: "", name: "", alt: ""), ProductImage(imageID: 2, dateCreated: Date(), dateModified: Date(), src: "", name: "", alt: "") ] - let mockRemoteProductImageStatuses = mockProductImages.map { ProductImageStatus.remote(image: $0) } - let mockProduct = Product.fake().copy(images: mockProductImages) + let mockRemoteProductImageStatuses = mockProductImages.map { ProductImageStatus.remote(image: $0, siteID: siteID, productID: productID) } + let mockProduct = Product.fake().copy(siteID: siteID, productID: productID.id, images: mockProductImages) let model = EditableProductModel(product: mockProduct) - let productImageActionHandler = ProductImageActionHandler(siteID: 123, + let productImageActionHandler = ProductImageActionHandler(siteID: siteID, product: model) let mockAsset = PHAsset() let expectedStatusUpdates: [[ProductImageStatus]] = [ mockRemoteProductImageStatuses, - [.uploading(asset: .phAsset(asset: mockAsset))] + mockRemoteProductImageStatuses, - [.uploadFailure(asset: .phAsset(asset: mockAsset), error: MediaActionError.unknown)] + mockRemoteProductImageStatuses + [.uploading(asset: .phAsset(asset: mockAsset), + siteID: siteID, + productID: productID)] + mockRemoteProductImageStatuses, + [.uploadFailure(asset: .phAsset(asset: mockAsset), + error: MediaActionError.unknown, + siteID: siteID, + productID: productID)] + mockRemoteProductImageStatuses ] let expectation = self.expectation(description: "Wait for image upload") @@ -119,7 +128,7 @@ final class ProductImageActionHandlerTests: XCTestCase { ServiceLocator.setStores(mockStoresManager) let model = EditableProductModel(product: .fake()) - let productImageActionHandler = ProductImageActionHandler(siteID: 123, + let productImageActionHandler = ProductImageActionHandler(siteID: siteID, product: model) // When @@ -145,8 +154,8 @@ final class ProductImageActionHandlerTests: XCTestCase { let mockStoresManager = MockStoresManager(sessionManager: .testingInstance) ServiceLocator.setStores(mockStoresManager) - let model = EditableProductModel(product: .fake()) - let productImageActionHandler = ProductImageActionHandler(siteID: 123, + let model = EditableProductModel(product: Product.fake().copy(siteID: siteID, productID: productID.id)) + let productImageActionHandler = ProductImageActionHandler(siteID: siteID, product: model) let mockImage = UIImage() @@ -160,19 +169,21 @@ final class ProductImageActionHandlerTests: XCTestCase { } // Then - assertEqual(statuses, [.uploading(asset: .uiImage(image: mockImage, filename: "woocommerce.jpg", altText: "cool product"))]) + assertEqual(statuses, [.uploading(asset: .uiImage(image: mockImage, filename: "woocommerce.jpg", altText: "cool product"), + siteID: siteID, + productID: productID)]) } - func testDeletingProductImage() { + func test_deleting_product_image() { let mockProductImages = [ ProductImage(imageID: 1, dateCreated: Date(), dateModified: Date(), src: "", name: "", alt: ""), ProductImage(imageID: 2, dateCreated: Date(), dateModified: Date(), src: "", name: "", alt: "") ] - let mockRemoteProductImageStatuses = mockProductImages.map { ProductImageStatus.remote(image: $0) } - let mockProduct = Product.fake().copy(images: mockProductImages) + let mockRemoteProductImageStatuses = mockProductImages.map { ProductImageStatus.remote(image: $0, siteID: siteID, productID: productID) } + let mockProduct = Product.fake().copy(siteID: siteID, productID: productID.id, images: mockProductImages) let model = EditableProductModel(product: mockProduct) - let productImageActionHandler = ProductImageActionHandler(siteID: 123, + let productImageActionHandler = ProductImageActionHandler(siteID: siteID, product: model) let expectedStatusUpdates: [[ProductImageStatus]] = [ @@ -203,17 +214,17 @@ final class ProductImageActionHandlerTests: XCTestCase { // MARK: - `addSiteMediaLibraryImagesToProduct(mediaItems:)` - func testAddingProductImagesFromSiteMediaLibrary() { + func test_adding_product_images_from_site_media_library() { // Arrange let mockProductImages = [ ProductImage(imageID: 1, dateCreated: Date(), dateModified: Date(), src: "", name: "", alt: ""), ProductImage(imageID: 2, dateCreated: Date(), dateModified: Date(), src: "", name: "", alt: "") ] - let mockRemoteProductImageStatuses = mockProductImages.map { ProductImageStatus.remote(image: $0) } - let mockProduct = Product.fake().copy(images: mockProductImages) + let mockRemoteProductImageStatuses = mockProductImages.map { ProductImageStatus.remote(image: $0, siteID: siteID, productID: productID) } + let mockProduct = Product.fake().copy(siteID: siteID, productID: productID.id, images: mockProductImages) let model = EditableProductModel(product: mockProduct) - let productImageActionHandler = ProductImageActionHandler(siteID: 123, + let productImageActionHandler = ProductImageActionHandler(siteID: siteID, product: model) // Media items to upload to site media library. @@ -229,7 +240,9 @@ final class ProductImageActionHandlerTests: XCTestCase { height: 320, width: 776) let mockMediaItems = [mockMedia1, mockMedia2] - let expectedImageStatusesFromSiteMediaLibrary = mockMediaItems.map { ProductImageStatus.remote(image: $0.toProductImage) } + let expectedImageStatusesFromSiteMediaLibrary = mockMediaItems.map { ProductImageStatus.remote(image: $0.toProductImage, + siteID: siteID, + productID: productID) } let expectedStatusUpdates: [[ProductImageStatus]] = [ mockRemoteProductImageStatuses, expectedImageStatusesFromSiteMediaLibrary + mockRemoteProductImageStatuses @@ -257,20 +270,20 @@ final class ProductImageActionHandlerTests: XCTestCase { // MARK: - `resetProductImages(to:)` - func testResettingProductImagesToAProduct() { + func test_resetting_product_images_to_a_product() { // Arrange let mockProduct = Product.fake().copy(images: []) let model = EditableProductModel(product: mockProduct) - let productImageActionHandler = ProductImageActionHandler(siteID: 123, + let productImageActionHandler = ProductImageActionHandler(siteID: siteID, product: model) let mockProductImages = [ ProductImage(imageID: 1, dateCreated: Date(), dateModified: Date(), src: "", name: "", alt: ""), ProductImage(imageID: 2, dateCreated: Date(), dateModified: Date(), src: "", name: "", alt: "") ] - let anotherMockProduct = Product.fake().copy(images: mockProductImages) + let anotherMockProduct = Product.fake().copy(siteID: siteID, productID: productID.id, images: mockProductImages) let anotherModel = EditableProductModel(product: anotherMockProduct) - let expectedProductImageStatuses = mockProductImages.map { ProductImageStatus.remote(image: $0) } + let expectedProductImageStatuses = mockProductImages.map { ProductImageStatus.remote(image: $0, siteID: siteID, productID: productID) } let expectation = self.expectation(description: "Wait for reset product images") expectation.expectedFulfillmentCount = 1 @@ -292,20 +305,20 @@ final class ProductImageActionHandlerTests: XCTestCase { XCTAssertEqual(productImageActionHandler.productImageStatuses, expectedProductImageStatuses) } - // MARK: - `updateProductImageStatusesAfterReordering + // MARK: - `updateProductImageStatusesAfterReordering` func test_productImageStatuses_are_updated_correctly_after_reordering() { // Given let mockProduct = Product.fake().copy(images: []) let model = EditableProductModel(product: mockProduct) - let productImageActionHandler = ProductImageActionHandler(siteID: 123, + let productImageActionHandler = ProductImageActionHandler(siteID: siteID, product: model) let mockProductImages = [ ProductImage(imageID: 1, dateCreated: Date(), dateModified: Date(), src: "", name: "", alt: ""), ProductImage(imageID: 2, dateCreated: Date(), dateModified: Date(), src: "", name: "", alt: "") ] - let anotherMockProduct = Product.fake().copy(images: mockProductImages) - let expectedProductImageStatuses = mockProductImages.map { ProductImageStatus.remote(image: $0) } + let anotherMockProduct = Product.fake().copy(siteID: siteID, productID: productID.id, images: mockProductImages) + let expectedProductImageStatuses = mockProductImages.map { ProductImageStatus.remote(image: $0, siteID: siteID, productID: productID) } let expectation = self.expectation(description: "Wait for update product images") expectation.expectedFulfillmentCount = 1 @@ -342,8 +355,8 @@ final class ProductImageActionHandlerTests: XCTestCase { completion(.failure(uploadError)) } - let model = EditableProductModel(product: .fake()) - let productImageActionHandler = ProductImageActionHandler(siteID: 123, + let model = EditableProductModel(product: Product.fake().copy(siteID: siteID, productID: productID.id)) + let productImageActionHandler = ProductImageActionHandler(siteID: siteID, product: model) let expectation = self.expectation(description: "Wait for status update") @@ -352,8 +365,11 @@ final class ProductImageActionHandlerTests: XCTestCase { let mockAsset = PHAsset() let expectedStatusUpdates: [[ProductImageStatus]] = [ [], - [.uploading(asset: .phAsset(asset: mockAsset))], - [.uploadFailure(asset: .phAsset(asset: mockAsset), error: MediaActionError.unknown)], + [.uploading(asset: .phAsset(asset: mockAsset), siteID: siteID, productID: productID)], + [.uploadFailure(asset: .phAsset(asset: mockAsset), + error: MediaActionError.unknown, + siteID: siteID, + productID: productID)], ] var observedProductImageStatusChanges: [[ProductImageStatus]] = [] @@ -381,6 +397,50 @@ final class ProductImageActionHandlerTests: XCTestCase { productImageActionHandler.productImageStatuses == [] } } + + func test_updateProductID_propagates_to_existing_upload_statuses() { + // Given + let localProductID = ProductOrVariationID.product(id: 0) + let remoteProductID = productID + let dummyImage = ProductImage(imageID: 1, dateCreated: Date(), dateModified: Date(), src: "", name: "", alt: "") + let mockProduct = Product.fake().copy(siteID: siteID, productID: localProductID.id, images: [dummyImage]) + let model = EditableProductModel(product: mockProduct) + let handler = ProductImageActionHandler(siteID: siteID, product: model) + + // Simulate an upload with a status .uploading, with the current productID (localProductID) + let mockAsset = PHAsset() + handler.uploadMediaAssetToSiteMediaLibrary(asset: .phAsset(asset: mockAsset)) + + // Aspetta brevemente l'aggiornamento asincrono + // (usando XCTestExpectation o waitUntil helper se presente) + waitUntil { + handler.productImageStatuses.first.map { + switch $0 { + case .uploading(_, let site, let productID): + return site == self.siteID && productID == localProductID + default: + return false + } + } ?? false + } + + // When + // update of productID from locale to remote one + handler.updateProductID(remoteProductID) + + // Then + // the new state should contain the remoteProductID + waitUntil(timeout: Constants.expectationTimeout) { + handler.productImageStatuses.first.map { + switch $0 { + case .uploading(_, _, let productID): + return productID == remoteProductID + default: + return false + } + } ?? false + } + } } private extension ProductImageActionHandlerTests { diff --git a/WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImageStatus+HelpersTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImageStatus+HelpersTests.swift index 2b03de9dd23..aae14138d07 100644 --- a/WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImageStatus+HelpersTests.swift +++ b/WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImageStatus+HelpersTests.swift @@ -5,11 +5,15 @@ import XCTest @testable import Yosemite final class ProductImageStatus_HelpersTests: XCTestCase { + + private let siteID: Int64 = 1234 + private let productID = ProductOrVariationID.product(id: 5678) + // MARK: - `images` func testImagesReturnsEmptyIfThereAreNoRemoteImages() { let statuses: [ProductImageStatus] = [ - .uploading(asset: .phAsset(asset: PHAsset())) + .uploading(asset: .phAsset(asset: PHAsset()), siteID: siteID, productID: productID) ] XCTAssertEqual(statuses.images, []) } @@ -18,8 +22,8 @@ final class ProductImageStatus_HelpersTests: XCTestCase { let productImage = ProductImage(imageID: 17, dateCreated: Date(), dateModified: Date(), src: "", name: nil, alt: nil) let statuses: [ProductImageStatus] = [ - .uploading(asset: .phAsset(asset: PHAsset())), - .remote(image: productImage) + .uploading(asset: .phAsset(asset: PHAsset()), siteID: siteID, productID: productID), + .remote(image: productImage, siteID: siteID, productID: productID) ] XCTAssertEqual(statuses.images, [productImage]) } @@ -34,7 +38,8 @@ final class ProductImageStatus_HelpersTests: XCTestCase { func testHasPendingUploadWithAllRemoteImages() { let productImage = ProductImage(imageID: 17, dateCreated: Date(), dateModified: Date(), src: "", name: nil, alt: nil) - let statuses: [ProductImageStatus] = [.remote(image: productImage), .remote(image: productImage)] + let statuses: [ProductImageStatus] = [.remote(image: productImage, siteID: siteID, productID: productID), + .remote(image: productImage, siteID: siteID, productID: productID)] XCTAssertFalse(statuses.hasPendingUpload) } @@ -42,8 +47,8 @@ final class ProductImageStatus_HelpersTests: XCTestCase { let productImage = ProductImage(imageID: 17, dateCreated: Date(), dateModified: Date(), src: "", name: nil, alt: nil) let statuses: [ProductImageStatus] = [ - .uploading(asset: .phAsset(asset: PHAsset())), - .remote(image: productImage) + .uploading(asset: .phAsset(asset: PHAsset()), siteID: siteID, productID: productID), + .remote(image: productImage, siteID: siteID, productID: productID) ] XCTAssertTrue(statuses.hasPendingUpload) } @@ -53,7 +58,7 @@ final class ProductImageStatus_HelpersTests: XCTestCase { func test_dragItemIdentifier_is_correct_for_remote_image() { // Given let productImage = ProductImage(imageID: 17, dateCreated: Date(), dateModified: Date(), src: "", name: nil, alt: nil) - let status = ProductImageStatus.remote(image: productImage) + let status = ProductImageStatus.remote(image: productImage, siteID: siteID, productID: productID) let expectedIdentifier = "\(17)" // When @@ -66,7 +71,7 @@ final class ProductImageStatus_HelpersTests: XCTestCase { func test_dragItemIdentifier_is_correct_for_uploading_asset() { // Given let asset = PHAsset() - let status = ProductImageStatus.uploading(asset: .phAsset(asset: asset)) + let status = ProductImageStatus.uploading(asset: .phAsset(asset: asset), siteID: siteID, productID: productID) let expectedIdentifier = asset.identifier() // When diff --git a/WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImageUploaderTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImageUploaderTests.swift index cb362f27ce5..17b0150dc79 100644 --- a/WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImageUploaderTests.swift +++ b/WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImageUploaderTests.swift @@ -6,7 +6,7 @@ import Yosemite final class ProductImageUploaderTests: XCTestCase { private let siteID: Int64 = 134 - private let productID: Int64 = 606 + private let productID = ProductOrVariationID.product(id: 606) private var errorsSubscription: AnyCancellable? private var assetUploadSubscription: AnyCancellable? private var activeUploadsSubscription: AnyCancellable? @@ -15,13 +15,13 @@ final class ProductImageUploaderTests: XCTestCase { // Given let imageUploader = ProductImageUploader() let actionHandler = imageUploader.actionHandler(key: .init(siteID: siteID, - productOrVariationID: .product(id: productID), + productOrVariationID: .product(id: productID.id), isLocalID: false), originalStatuses: []) let asset = PHAsset() XCTAssertFalse(imageUploader.hasUnsavedChangesOnImages(key: .init(siteID: siteID, - productOrVariationID: .product(id: productID), + productOrVariationID: .product(id: productID.id), isLocalID: false), originalImages: [])) @@ -34,16 +34,16 @@ final class ProductImageUploaderTests: XCTestCase { } XCTAssertTrue(statuses.hasPendingUpload) XCTAssertTrue(imageUploader.hasUnsavedChangesOnImages(key: .init(siteID: siteID, - productOrVariationID: .product(id: productID), + productOrVariationID: .product(id: productID.id), isLocalID: false), originalImages: [])) imageUploader.saveProductImagesWhenNoneIsPendingUploadAnymore(key: .init(siteID: siteID, - productOrVariationID: .product(id: productID), + productOrVariationID: .product(id: productID.id), isLocalID: false)) { _ in } // Then XCTAssertFalse(imageUploader.hasUnsavedChangesOnImages(key: .init(siteID: siteID, - productOrVariationID: .product(id: productID), + productOrVariationID: .product(id: productID.id), isLocalID: false), originalImages: [])) } @@ -53,7 +53,7 @@ final class ProductImageUploaderTests: XCTestCase { let stores = MockStoresManager(sessionManager: .testingInstance) let imageUploader = ProductImageUploader(stores: stores) let actionHandler = imageUploader.actionHandler(key: .init(siteID: siteID, - productOrVariationID: .product(id: productID), + productOrVariationID: .product(id: productID.id), isLocalID: false), originalStatuses: []) let asset = PHAsset() @@ -71,7 +71,7 @@ final class ProductImageUploaderTests: XCTestCase { } XCTAssertFalse(imageUploader.hasUnsavedChangesOnImages(key: .init(siteID: siteID, - productOrVariationID: .product(id: productID), + productOrVariationID: .product(id: productID.id), isLocalID: false), originalImages: [])) @@ -84,12 +84,12 @@ final class ProductImageUploaderTests: XCTestCase { } XCTAssertTrue(statuses.hasPendingUpload) XCTAssertTrue(imageUploader.hasUnsavedChangesOnImages(key: .init(siteID: siteID, - productOrVariationID: .product(id: productID), + productOrVariationID: productID, isLocalID: false), originalImages: [])) let resultOfSavedImages = waitFor { promise in imageUploader.saveProductImagesWhenNoneIsPendingUploadAnymore(key: .init(siteID: self.siteID, - productOrVariationID: .product(id: self.productID), + productOrVariationID: self.productID, isLocalID: false)) { result in promise(result) } @@ -97,7 +97,7 @@ final class ProductImageUploaderTests: XCTestCase { // Then XCTAssertFalse(imageUploader.hasUnsavedChangesOnImages(key: .init(siteID: siteID, - productOrVariationID: .product(id: productID), + productOrVariationID: productID, isLocalID: false), originalImages: [.fake().copy(imageID: 645)])) XCTAssertTrue(resultOfSavedImages.isSuccess) @@ -110,7 +110,7 @@ final class ProductImageUploaderTests: XCTestCase { let stores = MockStoresManager(sessionManager: .testingInstance) let imageUploader = ProductImageUploader(stores: stores) let actionHandler = imageUploader.actionHandler(key: .init(siteID: siteID, - productOrVariationID: .product(id: productID), + productOrVariationID: productID, isLocalID: false), originalStatuses: []) let asset = PHAsset() @@ -122,7 +122,7 @@ final class ProductImageUploaderTests: XCTestCase { } XCTAssertFalse(imageUploader.hasUnsavedChangesOnImages(key: .init(siteID: siteID, - productOrVariationID: .product(id: productID), + productOrVariationID: productID, isLocalID: false), originalImages: [])) @@ -138,13 +138,13 @@ final class ProductImageUploaderTests: XCTestCase { } XCTAssertTrue(imageUploader.hasUnsavedChangesOnImages(key: .init(siteID: siteID, - productOrVariationID: .product(id: productID), + productOrVariationID: productID, isLocalID: false), originalImages: [])) // The first save. imageUploader.saveProductImagesWhenNoneIsPendingUploadAnymore(key: .init(siteID: self.siteID, - productOrVariationID: .product(id: self.productID), + productOrVariationID: self.productID, isLocalID: false)) { result in XCTFail("The product save callback should not be triggered after another save request.") } @@ -161,7 +161,7 @@ final class ProductImageUploaderTests: XCTestCase { // The second save. imageUploader.saveProductImagesWhenNoneIsPendingUploadAnymore(key: .init(siteID: self.siteID, - productOrVariationID: .product(id: self.productID), + productOrVariationID: self.productID, isLocalID: false)) { result in promise(result) } @@ -171,7 +171,7 @@ final class ProductImageUploaderTests: XCTestCase { // Then XCTAssertFalse(imageUploader.hasUnsavedChangesOnImages(key: .init(siteID: siteID, - productOrVariationID: .product(id: productID), + productOrVariationID: productID, isLocalID: false), originalImages: [.fake().copy(imageID: 606), .fake().copy(imageID: 645)])) XCTAssertTrue(resultOfSavedImages.isSuccess) @@ -183,10 +183,10 @@ final class ProductImageUploaderTests: XCTestCase { // Given let imageUploader = ProductImageUploader() let localProductID: Int64 = 0 - let remoteProductID = productID - let originalStatuses: [ProductImageStatus] = [.remote(image: ProductImage.fake()), - .uploading(asset: .phAsset(asset: PHAsset())), - .uploading(asset: .phAsset(asset: PHAsset()))] + let remoteProductID = productID.id + let originalStatuses: [ProductImageStatus] = [.remote(image: ProductImage.fake(), siteID: siteID, productID: productID), + .uploading(asset: .phAsset(asset: PHAsset()), siteID: siteID, productID: productID), + .uploading(asset: .phAsset(asset: PHAsset()), siteID: siteID, productID: productID)] _ = imageUploader.actionHandler(key: .init(siteID: siteID, productOrVariationID: .product(id: localProductID), isLocalID: true), @@ -218,16 +218,16 @@ final class ProductImageUploaderTests: XCTestCase { let localProductID: Int64 = 0 let nonExistentProductID: Int64 = 999 let remoteProductID = productID - let originalStatuses: [ProductImageStatus] = [.remote(image: ProductImage.fake()), - .uploading(asset: .phAsset(asset: PHAsset())), - .uploading(asset: .phAsset(asset: PHAsset()))] + let originalStatuses: [ProductImageStatus] = [.remote(image: ProductImage.fake(), siteID: siteID, productID: productID), + .uploading(asset: .phAsset(asset: PHAsset()), siteID: siteID, productID: productID), + .uploading(asset: .phAsset(asset: PHAsset()), siteID: siteID, productID: productID)] _ = imageUploader.actionHandler(key: .init(siteID: siteID, productOrVariationID: .product(id: localProductID), isLocalID: true), originalStatuses: originalStatuses) // When - imageUploader.replaceLocalID(siteID: siteID, localID: .product(id: nonExistentProductID), remoteID: remoteProductID) + imageUploader.replaceLocalID(siteID: siteID, localID: .product(id: nonExistentProductID), remoteID: remoteProductID.id) // Then // Ensure that trying to replace a non-existent product ID does nothing. @@ -244,7 +244,7 @@ final class ProductImageUploaderTests: XCTestCase { let imageUploader = ProductImageUploader(stores: stores, imagesProductIDUpdater: mockProductIDUpdater) let actionHandler = imageUploader.actionHandler(key: .init(siteID: siteID, - productOrVariationID: .product(id: productID), + productOrVariationID: productID, isLocalID: false), originalStatuses: []) @@ -270,7 +270,7 @@ final class ProductImageUploaderTests: XCTestCase { } imageUploader.saveProductImagesWhenNoneIsPendingUploadAnymore(key: .init(siteID: siteID, - productOrVariationID: .product(id: productID), + productOrVariationID: productID, isLocalID: false)) { result in } // Then @@ -286,7 +286,7 @@ final class ProductImageUploaderTests: XCTestCase { let stores = MockStoresManager(sessionManager: .testingInstance) let imageUploader = ProductImageUploader(stores: stores) let actionHandler = imageUploader.actionHandler(key: .init(siteID: siteID, - productOrVariationID: .product(id: productID), + productOrVariationID: productID, isLocalID: true), originalStatuses: []) let error = NSError(domain: "", code: 6) @@ -309,7 +309,7 @@ final class ProductImageUploaderTests: XCTestCase { // Then assertEqual([.init(siteID: siteID, - productOrVariationID: .product(id: productID), + productOrVariationID: productID, error: ProductImageUploaderError.failedUploadingImage(asset: asset, error: error))], errors) } @@ -319,7 +319,7 @@ final class ProductImageUploaderTests: XCTestCase { let stores = MockStoresManager(sessionManager: .testingInstance) let imageUploader = ProductImageUploader(stores: stores) let actionHandler = imageUploader.actionHandler(key: .init(siteID: siteID, - productOrVariationID: .product(id: productID), + productOrVariationID: productID, isLocalID: false), originalStatuses: []) @@ -343,7 +343,7 @@ final class ProductImageUploaderTests: XCTestCase { } } imageUploader.saveProductImagesWhenNoneIsPendingUploadAnymore(key: .init(siteID: siteID, - productOrVariationID: .product(id: productID), + productOrVariationID: productID, isLocalID: false)) { result in } var errors: [ProductImageUploadErrorInfo] = [] let _: Void = waitFor { promise in @@ -355,7 +355,7 @@ final class ProductImageUploaderTests: XCTestCase { // Then assertEqual([.init(siteID: siteID, - productOrVariationID: .product(id: productID), + productOrVariationID: productID, error: .failedSavingProductAfterImageUpload(error: ProductUpdateError.unexpected))], errors) } @@ -365,7 +365,7 @@ final class ProductImageUploaderTests: XCTestCase { let stores = MockStoresManager(sessionManager: .testingInstance) let imageUploader = ProductImageUploader(stores: stores) let actionHandler = imageUploader.actionHandler(key: .init(siteID: siteID, - productOrVariationID: .product(id: productID), + productOrVariationID: productID, isLocalID: true), originalStatuses: []) stores.whenReceivingAction(ofType: MediaAction.self) { action in @@ -393,7 +393,7 @@ final class ProductImageUploaderTests: XCTestCase { let stores = MockStoresManager(sessionManager: .testingInstance) let imageUploader = ProductImageUploader(stores: stores) let actionHandler = imageUploader.actionHandler(key: .init(siteID: siteID, - productOrVariationID: .product(id: productID), + productOrVariationID: productID, isLocalID: true), originalStatuses: []) let error = NSError(domain: "", code: 6) @@ -420,7 +420,7 @@ final class ProductImageUploaderTests: XCTestCase { // Then assertEqual([.init(siteID: siteID, - productOrVariationID: .product(id: productID), + productOrVariationID: productID, error: .failedUploadingImage(asset: asset, error: error))], errors) } @@ -430,7 +430,7 @@ final class ProductImageUploaderTests: XCTestCase { let stores = MockStoresManager(sessionManager: .testingInstance) let imageUploader = ProductImageUploader(stores: stores) let actionHandler = imageUploader.actionHandler(key: .init(siteID: siteID, - productOrVariationID: .product(id: productID), + productOrVariationID: productID, isLocalID: true), originalStatuses: []) let error = NSError(domain: "", code: 6) @@ -442,7 +442,7 @@ final class ProductImageUploaderTests: XCTestCase { // When imageUploader.stopEmittingErrors(key: .init(siteID: siteID, - productOrVariationID: .product(id: productID), + productOrVariationID: productID, isLocalID: true)) var errors: [ProductImageUploadErrorInfo] = [] @@ -474,7 +474,7 @@ final class ProductImageUploaderTests: XCTestCase { isLocalID: true)) imageUploader.replaceLocalID(siteID: siteID, localID: .product(id: nonExistentProductID), - remoteID: remoteProductID) + remoteID: remoteProductID.id) var errors: [ProductImageUploadErrorInfo] = [] _ = imageUploader.errors.sink { error in @@ -499,7 +499,7 @@ final class ProductImageUploaderTests: XCTestCase { let stores = MockStoresManager(sessionManager: .testingInstance) let imageUploader = ProductImageUploader(stores: stores) let actionHandler = imageUploader.actionHandler(key: .init(siteID: siteID, - productOrVariationID: .product(id: productID), + productOrVariationID: productID, isLocalID: true), originalStatuses: []) let error = NSError(domain: "", code: 6) @@ -511,10 +511,10 @@ final class ProductImageUploaderTests: XCTestCase { // When imageUploader.stopEmittingErrors(key: .init(siteID: siteID, - productOrVariationID: .product(id: productID), + productOrVariationID: productID, isLocalID: true)) imageUploader.startEmittingErrors(key: .init(siteID: siteID, - productOrVariationID: .product(id: productID), + productOrVariationID: productID, isLocalID: true)) var errors: [ProductImageUploadErrorInfo] = [] @@ -529,7 +529,7 @@ final class ProductImageUploaderTests: XCTestCase { // Then assertEqual([.init(siteID: siteID, - productOrVariationID: .product(id: productID), + productOrVariationID: productID, error: ProductImageUploaderError.failedUploadingImage(asset: asset, error: error))], errors) } @@ -541,7 +541,7 @@ final class ProductImageUploaderTests: XCTestCase { let stores = MockStoresManager(sessionManager: .testingInstance) let imageUploader = ProductImageUploader(stores: stores) let actionHandler = imageUploader.actionHandler(key: .init(siteID: siteID, - productOrVariationID: .product(id: productID), + productOrVariationID: productID, isLocalID: true), originalStatuses: []) stores.whenReceivingAction(ofType: MediaAction.self) { action in @@ -583,7 +583,7 @@ final class ProductImageUploaderTests: XCTestCase { let stores = MockStoresManager(sessionManager: .testingInstance) let imageUploader = ProductImageUploader(stores: stores) let key = ProductImageUploaderKey(siteID: siteID, - productOrVariationID: .product(id: productID), + productOrVariationID: productID, isLocalID: false) let actionHandler = imageUploader.actionHandler(key: key, originalStatuses: []) @@ -615,10 +615,10 @@ final class ProductImageUploaderTests: XCTestCase { let stores = MockStoresManager(sessionManager: .testingInstance) let imageUploader = ProductImageUploader(stores: stores) let key = ProductImageUploaderKey(siteID: siteID, - productOrVariationID: .product(id: productID), + productOrVariationID: productID, isLocalID: false) let actionHandler = imageUploader.actionHandler(key: key, originalStatuses: []) - let productFormDataModel = EditableProductModel(product: .fake().copy(siteID: siteID, productID: productID, images: [])) + let productFormDataModel = EditableProductModel(product: .fake().copy(siteID: siteID, productID: productID.id, images: [])) var activeUploads: [ProductImageUploaderKey] = [] activeUploadsSubscription = imageUploader.activeUploads @@ -649,7 +649,7 @@ final class ProductImageUploaderTests: XCTestCase { let stores = MockStoresManager(sessionManager: .testingInstance) let imageUploader = ProductImageUploader(stores: stores) let key = ProductImageUploaderKey(siteID: siteID, - productOrVariationID: .product(id: productID), + productOrVariationID: productID, isLocalID: false) let actionHandler = imageUploader.actionHandler(key: key, originalStatuses: []) diff --git a/WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImagesSaverTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImagesSaverTests.swift index 4fcca0b133d..1611897ebd2 100644 --- a/WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImagesSaverTests.swift +++ b/WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImagesSaverTests.swift @@ -44,7 +44,7 @@ final class ProductImagesSaverTests: XCTestCase { // Saves product images. imagesSaver.saveProductImagesWhenNoneIsPendingUploadAnymore(imageActionHandler: actionHandler) { _ in } - XCTAssertEqual(imagesSaver.imageStatusesToSave, [.uploading(asset: asset)]) + XCTAssertEqual(imagesSaver.imageStatusesToSave, [.uploading(asset: asset, siteID: siteID, productID: .product(id: productID))]) // When imageUploadCompletion(.failure(MediaActionError.unknown)) @@ -84,7 +84,7 @@ final class ProductImagesSaverTests: XCTestCase { imagesSaver.saveProductImagesWhenNoneIsPendingUploadAnymore(imageActionHandler: actionHandler) { _ in promise(()) } - XCTAssertEqual(imagesSaver.imageStatusesToSave, [.uploading(asset: asset)]) + XCTAssertEqual(imagesSaver.imageStatusesToSave, [.uploading(asset: asset, siteID: self.siteID, productID: .product(id: self.productID))]) // When // Mocks successful image upload. @@ -140,7 +140,7 @@ final class ProductImagesSaverTests: XCTestCase { imagesSaver.saveProductImagesWhenNoneIsPendingUploadAnymore(imageActionHandler: actionHandler) { _ in promise(()) } - XCTAssertEqual(imagesSaver.imageStatusesToSave, [.uploading(asset: asset)]) + XCTAssertEqual(imagesSaver.imageStatusesToSave, [.uploading(asset: asset, siteID: self.siteID, productID: .product(id: self.productID))]) // Mocks successful image upload. imageUploadCompletion(.success(.fake().copy(mediaID: 645))) @@ -156,7 +156,9 @@ final class ProductImagesSaverTests: XCTestCase { let variationID: ProductOrVariationID = .variation(productID: productID, variationID: 134) let imagesSaver = ProductImagesSaver(siteID: siteID, productOrVariationID: variationID, stores: stores) let asset: ProductImageAssetType = .phAsset(asset: PHAsset()) - let actionHandler = MockProductImageActionHandler(productImageStatuses: [.uploading(asset: asset)]) + let actionHandler = MockProductImageActionHandler(productImageStatuses: [.uploading(asset: asset, + siteID: siteID, + productID: .product(id: productID))]) let image = ProductImage.fake() actionHandler.assetUploadResults = (asset: asset, result: .success(image)) @@ -246,7 +248,9 @@ final class ProductImagesSaverTests: XCTestCase { let variationID: ProductOrVariationID = .variation(productID: productID, variationID: 134) let imagesSaver = ProductImagesSaver(siteID: siteID, productOrVariationID: variationID, stores: stores, analytics: analytics) let asset: ProductImageAssetType = .phAsset(asset: PHAsset()) - let actionHandler = MockProductImageActionHandler(productImageStatuses: [.uploading(asset: asset)]) + let actionHandler = MockProductImageActionHandler(productImageStatuses: [.uploading(asset: asset, + siteID: siteID, + productID: .variation(productID: productID, variationID: 134))]) let image = ProductImage.fake() actionHandler.assetUploadResults = (asset: asset, result: .success(image)) @@ -275,7 +279,7 @@ final class ProductImagesSaverTests: XCTestCase { let stores = MockStoresManager(sessionManager: .testingInstance) let imagesSaver = ProductImagesSaver(siteID: siteID, productOrVariationID: .product(id: 648), stores: stores, analytics: analytics) let asset: ProductImageAssetType = .phAsset(asset: PHAsset()) - let actionHandler = MockProductImageActionHandler(productImageStatuses: [.uploading(asset: asset)]) + let actionHandler = MockProductImageActionHandler(productImageStatuses: [.uploading(asset: asset, siteID: siteID, productID: .product(id: productID))]) let image = ProductImage.fake() actionHandler.assetUploadResults = (asset: asset, result: .success(image)) From d5069463a320f9d39a6b7117ef03ab8e19df51e1 Mon Sep 17 00:00:00 2001 From: Paolo Musolino Date: Wed, 19 Feb 2025 00:00:45 +0100 Subject: [PATCH 08/25] update: removed superfluous comment --- .../Products/Media/ProductImageActionHandlerTests.swift | 2 -- 1 file changed, 2 deletions(-) diff --git a/WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImageActionHandlerTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImageActionHandlerTests.swift index fb8cfd616fc..5dbba4b3889 100644 --- a/WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImageActionHandlerTests.swift +++ b/WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImageActionHandlerTests.swift @@ -411,8 +411,6 @@ final class ProductImageActionHandlerTests: XCTestCase { let mockAsset = PHAsset() handler.uploadMediaAssetToSiteMediaLibrary(asset: .phAsset(asset: mockAsset)) - // Aspetta brevemente l'aggiornamento asincrono - // (usando XCTestExpectation o waitUntil helper se presente) waitUntil { handler.productImageStatuses.first.map { switch $0 { From 51a1cfd73079f60a85bd727ee352a19b23a05fc2 Mon Sep 17 00:00:00 2001 From: Paolo Musolino Date: Wed, 19 Feb 2025 00:56:06 +0100 Subject: [PATCH 09/25] fix: product image variation management, which allow maximum one image for variations. --- .../Products/Media/ProductImageActionHandler.swift | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/WooCommerce/Classes/ViewRelated/Products/Media/ProductImageActionHandler.swift b/WooCommerce/Classes/ViewRelated/Products/Media/ProductImageActionHandler.swift index f2165adc5b0..d9e8ab9db08 100644 --- a/WooCommerce/Classes/ViewRelated/Products/Media/ProductImageActionHandler.swift +++ b/WooCommerce/Classes/ViewRelated/Products/Media/ProductImageActionHandler.swift @@ -175,8 +175,15 @@ final class ProductImageActionHandler: ProductImageActionHandlerProtocol { guard let self = self else { return } - - productImageStatuses = [.uploading(asset: asset, siteID: self.siteID, productID: self.productOrVariationID)] + self.productImageStatuses + let uploadingStatus = ProductImageStatus.uploading(asset: asset, siteID: self.siteID, productID: self.productOrVariationID) + + // If the product is a variation, substitute the existing status with the new uploading status. + // Otherwise, if a standard product, append a new status. + if case .variation = self.productOrVariationID { + self.productImageStatuses = [uploadingStatus] + } else { + self.productImageStatuses = [uploadingStatus] + self.productImageStatuses + } self.uploadMediaAssetToSiteMediaLibrary(asset: asset) { [weak self] result in self?.queue.async { [weak self] in From 70e0fa949baea864cddf98b10f49fc8b06157fb4 Mon Sep 17 00:00:00 2001 From: Paolo Musolino Date: Wed, 19 Feb 2025 01:16:11 +0100 Subject: [PATCH 10/25] update: release-notes 21.8 --- RELEASE-NOTES.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index 499f8562280..88ddf10a6b8 100644 --- a/RELEASE-NOTES.txt +++ b/RELEASE-NOTES.txt @@ -12,6 +12,7 @@ - [*] Background image upload: Show a notice when the user leaves product details while uploads are pending [https://github.com/woocommerce/woocommerce-ios/pull/15134] - [*] Filters applied in product selector no longer affect the main product list screen. [https://github.com/woocommerce/woocommerce-ios/pull/14764] - [**] Product Images: Update error handling [https://github.com/woocommerce/woocommerce-ios/pull/15105] +- [internal] Assign `siteID` and `productID` to image product upload statuses. [https://github.com/woocommerce/woocommerce-ios/pull/15196] 21.7 ----- From 27a11460dfcf45e2f4b4495dbdd2e826200b1d7a Mon Sep 17 00:00:00 2001 From: Paolo Musolino Date: Wed, 19 Feb 2025 15:21:24 +0100 Subject: [PATCH 11/25] fix: failing unit tests --- .../ViewRelated/Products/Media/ProductImagesSaver.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/WooCommerce/Classes/ViewRelated/Products/Media/ProductImagesSaver.swift b/WooCommerce/Classes/ViewRelated/Products/Media/ProductImagesSaver.swift index 72e20067a1a..b468d296340 100644 --- a/WooCommerce/Classes/ViewRelated/Products/Media/ProductImagesSaver.swift +++ b/WooCommerce/Classes/ViewRelated/Products/Media/ProductImagesSaver.swift @@ -129,8 +129,8 @@ private extension ProductImagesSaver { guard let self = self else { return } guard let index = self.imageStatusesToSave.firstIndex(where: { status -> Bool in switch status { - case .uploading(let uploadingAsset, let siteID, let productID): - return uploadingAsset == asset && siteID == self.siteID && productID == self.productOrVariationID + case .uploading(let uploadingAsset, _, _): + return uploadingAsset == asset default: return false } From 561582adcf0eb26a52cb4460aa197b63318b057e Mon Sep 17 00:00:00 2001 From: Paolo Musolino Date: Sun, 23 Feb 2025 22:25:42 +0100 Subject: [PATCH 12/25] update: release-notes --- RELEASE-NOTES.txt | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index fe319773eee..1f87c3dbab1 100644 --- a/RELEASE-NOTES.txt +++ b/RELEASE-NOTES.txt @@ -1,6 +1,10 @@ *** PLEASE FOLLOW THIS FORMAT: [] [] *** Use [*****] to indicate smoke tests of all critical flows should be run on the final IPA before release (e.g. major library or OS update). +21.9 +----- +- [internal] Assign `siteID` and `productID` to image product upload statuses. [https://github.com/woocommerce/woocommerce-ios/pull/15196] + 21.8 ----- - [**] Shipping Labels: resolved issue preventing shipping labels from displaying in some orders when an error occurred during label creation. [https://github.com/woocommerce/woocommerce-ios/pull/15101] @@ -14,7 +18,6 @@ - [*] Better error messages if Application Password login is disabled on user's website. [https://github.com/woocommerce/woocommerce-ios/pull/15031] - [**] Product Images: Update error handling [https://github.com/woocommerce/woocommerce-ios/pull/15105] - [*] Merchants can mark and filter favorite products for quicker access. [https://github.com/woocommerce/woocommerce-ios/pull/14597] -- [internal] Assign `siteID` and `productID` to image product upload statuses. [https://github.com/woocommerce/woocommerce-ios/pull/15196] 21.7 ----- From c84cef64a6117dc381568569ce2d55f7fb15af83 Mon Sep 17 00:00:00 2001 From: Paolo Musolino Date: Sun, 23 Feb 2025 23:47:09 +0100 Subject: [PATCH 13/25] - Update `ProductImageStatus.swift` to use non-optional `ProductOrVariationID` in `uploading` and `uploadFailure` cases - Update unit tests --- .../ProductImageInBackground/ProductImageStatus.swift | 8 ++++---- .../Products/Media/ProductImageUploaderTests.swift | 8 +++++--- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/Networking/Networking/ProductImageInBackground/ProductImageStatus.swift b/Networking/Networking/ProductImageInBackground/ProductImageStatus.swift index cf20b51b121..f5d9cac8895 100644 --- a/Networking/Networking/ProductImageInBackground/ProductImageStatus.swift +++ b/Networking/Networking/ProductImageInBackground/ProductImageStatus.swift @@ -8,7 +8,7 @@ import UIKit public enum ProductImageStatus: Equatable, Codable { /// An image asset is being uploaded. /// - case uploading(asset: ProductImageAssetType, siteID: Int64, productID: ProductOrVariationID?) + case uploading(asset: ProductImageAssetType, siteID: Int64, productID: ProductOrVariationID) /// The Product image exists remotely. /// @@ -16,7 +16,7 @@ public enum ProductImageStatus: Equatable, Codable { /// An image asset upload failed. /// - case uploadFailure(asset: ProductImageAssetType, error: Error, siteID: Int64, productID: ProductOrVariationID?) + case uploadFailure(asset: ProductImageAssetType, error: Error, siteID: Int64, productID: ProductOrVariationID) private enum CodingKeys: String, CodingKey { case type @@ -34,7 +34,7 @@ public enum ProductImageStatus: Equatable, Codable { case "uploading": let asset = try container.decode(ProductImageAssetType.self, forKey: .asset) let sID = try container.decode(Int64.self, forKey: .siteID) - let pID = try container.decodeIfPresent(ProductOrVariationID.self, forKey: .productID) + let pID = try container.decode(ProductOrVariationID.self, forKey: .productID) self = .uploading(asset: asset, siteID: sID, productID: pID) case "remote": let image = try container.decode(ProductImage.self, forKey: .image) @@ -46,7 +46,7 @@ public enum ProductImageStatus: Equatable, Codable { let errorMessage = try container.decode(String.self, forKey: .error) let error = NSError(domain: "ProductImageStatus", code: 0, userInfo: [NSLocalizedDescriptionKey: errorMessage]) let sID = try container.decode(Int64.self, forKey: .siteID) - let pID = try container.decodeIfPresent(ProductOrVariationID.self, forKey: .productID) + let pID = try container.decode(ProductOrVariationID.self, forKey: .productID) self = .uploadFailure(asset: asset, error: error, siteID: sID, productID: pID) default: throw DecodingError.dataCorruptedError(forKey: .type, diff --git a/WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImageUploaderTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImageUploaderTests.swift index 17b0150dc79..a1156541428 100644 --- a/WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImageUploaderTests.swift +++ b/WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImageUploaderTests.swift @@ -53,7 +53,7 @@ final class ProductImageUploaderTests: XCTestCase { let stores = MockStoresManager(sessionManager: .testingInstance) let imageUploader = ProductImageUploader(stores: stores) let actionHandler = imageUploader.actionHandler(key: .init(siteID: siteID, - productOrVariationID: .product(id: productID.id), + productOrVariationID: productID, isLocalID: false), originalStatuses: []) let asset = PHAsset() @@ -71,7 +71,7 @@ final class ProductImageUploaderTests: XCTestCase { } XCTAssertFalse(imageUploader.hasUnsavedChangesOnImages(key: .init(siteID: siteID, - productOrVariationID: .product(id: productID.id), + productOrVariationID: productID, isLocalID: false), originalImages: [])) @@ -79,7 +79,9 @@ final class ProductImageUploaderTests: XCTestCase { actionHandler.uploadMediaAssetToSiteMediaLibrary(asset: .phAsset(asset: asset)) let statuses = waitFor { promise in actionHandler.addUpdateObserver(self) { statuses in - promise(statuses) + if statuses.hasPendingUpload { + promise(statuses) + } } } XCTAssertTrue(statuses.hasPendingUpload) From 1344637b87e22a2ca0a4358f30dfd7f7f359b2ac Mon Sep 17 00:00:00 2001 From: Paolo Musolino Date: Mon, 24 Feb 2025 11:38:27 +0100 Subject: [PATCH 14/25] fix: test_hasUnsavedChangesOnImages_stays_false_after_uploading_and_saving_successfully --- .../ViewRelated/Products/Media/ProductImageUploaderTests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImageUploaderTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImageUploaderTests.swift index a1156541428..cf99ea18e49 100644 --- a/WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImageUploaderTests.swift +++ b/WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImageUploaderTests.swift @@ -66,7 +66,7 @@ final class ProductImageUploaderTests: XCTestCase { } stores.whenReceivingAction(ofType: ProductAction.self) { action in if case let .updateProductImages(_, _, images, onCompletion) = action { - onCompletion(.success(.fake().copy(images: images))) + onCompletion(.success(.fake().copy(siteID: self.siteID, productID: self.productID.id, images: images))) } } From f5803bba4d371f3ba82c58f65f030bead54eafc8 Mon Sep 17 00:00:00 2001 From: Paolo Musolino Date: Mon, 24 Feb 2025 14:39:14 +0100 Subject: [PATCH 15/25] refactor: Set up the observer first to wait for the status change to pending upload, then trigger the media upload after adding the observer. --- .../ViewRelated/Products/Media/ProductImageUploaderTests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImageUploaderTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImageUploaderTests.swift index cf99ea18e49..c3fee77ef63 100644 --- a/WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImageUploaderTests.swift +++ b/WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImageUploaderTests.swift @@ -76,7 +76,6 @@ final class ProductImageUploaderTests: XCTestCase { originalImages: [])) // When - actionHandler.uploadMediaAssetToSiteMediaLibrary(asset: .phAsset(asset: asset)) let statuses = waitFor { promise in actionHandler.addUpdateObserver(self) { statuses in if statuses.hasPendingUpload { @@ -84,6 +83,7 @@ final class ProductImageUploaderTests: XCTestCase { } } } + actionHandler.uploadMediaAssetToSiteMediaLibrary(asset: .phAsset(asset: asset)) XCTAssertTrue(statuses.hasPendingUpload) XCTAssertTrue(imageUploader.hasUnsavedChangesOnImages(key: .init(siteID: siteID, productOrVariationID: productID, From 1d85fd204dc4b9b83b6db68c9d894b7768de5bb2 Mon Sep 17 00:00:00 2001 From: Paolo Musolino Date: Mon, 24 Feb 2025 15:34:35 +0100 Subject: [PATCH 16/25] Fix: asset state transition issue by updating .phAsset comparison logic. Instead of direct PHAsset comparison, compare localIdentifier to ensure accurate detection and state updates from "uploading" to "remote." This resolves the issue where hasUnsavedChangesOnImages(...) incorrectly returns true after saving, fixing related test failures. --- .../ProductImageStatus.swift | 14 ++++++++++++++ .../Products/Media/ProductImageUploaderTests.swift | 2 +- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/Networking/Networking/ProductImageInBackground/ProductImageStatus.swift b/Networking/Networking/ProductImageInBackground/ProductImageStatus.swift index f5d9cac8895..3a3e650e92a 100644 --- a/Networking/Networking/ProductImageInBackground/ProductImageStatus.swift +++ b/Networking/Networking/ProductImageInBackground/ProductImageStatus.swift @@ -160,4 +160,18 @@ public enum ProductImageAssetType: Equatable, Codable { try container.encode(altText, forKey: .altText) } } + + public static func == (lhs: ProductImageAssetType, rhs: ProductImageAssetType) -> Bool { + switch (lhs, rhs) { + case (.phAsset(let lAsset), .phAsset(let rAsset)): + return lAsset.localIdentifier == rAsset.localIdentifier + case (.uiImage(let lImage, let lFilename, let lAltText), + .uiImage(let rImage, let rFilename, let rAltText)): + return lImage.pngData() == rImage.pngData() && + lFilename == rFilename && + lAltText == rAltText + default: + return false + } + } } diff --git a/WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImageUploaderTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImageUploaderTests.swift index c3fee77ef63..cf99ea18e49 100644 --- a/WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImageUploaderTests.swift +++ b/WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImageUploaderTests.swift @@ -76,6 +76,7 @@ final class ProductImageUploaderTests: XCTestCase { originalImages: [])) // When + actionHandler.uploadMediaAssetToSiteMediaLibrary(asset: .phAsset(asset: asset)) let statuses = waitFor { promise in actionHandler.addUpdateObserver(self) { statuses in if statuses.hasPendingUpload { @@ -83,7 +84,6 @@ final class ProductImageUploaderTests: XCTestCase { } } } - actionHandler.uploadMediaAssetToSiteMediaLibrary(asset: .phAsset(asset: asset)) XCTAssertTrue(statuses.hasPendingUpload) XCTAssertTrue(imageUploader.hasUnsavedChangesOnImages(key: .init(siteID: siteID, productOrVariationID: productID, From e0120967668f1783556304bc500de9691846a959 Mon Sep 17 00:00:00 2001 From: Paolo Musolino Date: Mon, 24 Feb 2025 17:28:58 +0100 Subject: [PATCH 17/25] update: explicit injection of mock store dependencies in tests to avoid default `ServiceLocator` usage. --- .../Media/ProductImageUploaderTests.swift | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImageUploaderTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImageUploaderTests.swift index cf99ea18e49..457b17ad94c 100644 --- a/WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImageUploaderTests.swift +++ b/WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImageUploaderTests.swift @@ -13,7 +13,10 @@ final class ProductImageUploaderTests: XCTestCase { func test_hasUnsavedChangesOnImages_becomes_false_after_uploading_and_saving() throws { // Given - let imageUploader = ProductImageUploader() + let stores = MockStoresManager(sessionManager: .testingInstance) + let mockProductIDUpdater = MockProductImagesProductIDUpdater() + let imageUploader = ProductImageUploader(stores: stores, + imagesProductIDUpdater: mockProductIDUpdater) let actionHandler = imageUploader.actionHandler(key: .init(siteID: siteID, productOrVariationID: .product(id: productID.id), isLocalID: false), @@ -183,7 +186,10 @@ final class ProductImageUploaderTests: XCTestCase { func test_replaceLocalID_replaces_productID_properly() { // Given - let imageUploader = ProductImageUploader() + let stores = MockStoresManager(sessionManager: .testingInstance) + let mockProductIDUpdater = MockProductImagesProductIDUpdater() + let imageUploader = ProductImageUploader(stores: stores, + imagesProductIDUpdater: mockProductIDUpdater) let localProductID: Int64 = 0 let remoteProductID = productID.id let originalStatuses: [ProductImageStatus] = [.remote(image: ProductImage.fake(), siteID: siteID, productID: productID), @@ -216,7 +222,10 @@ final class ProductImageUploaderTests: XCTestCase { func test_calling_replaceLocalID_with_nonExistent_localProductID_does_nothing() { // Given - let imageUploader = ProductImageUploader() + let stores = MockStoresManager(sessionManager: .testingInstance) + let mockProductIDUpdater = MockProductImagesProductIDUpdater() + let imageUploader = ProductImageUploader(stores: stores, + imagesProductIDUpdater: mockProductIDUpdater) let localProductID: Int64 = 0 let nonExistentProductID: Int64 = 999 let remoteProductID = productID From 67dd20a7cd540230960f7c8b468cd5e84acf0187 Mon Sep 17 00:00:00 2001 From: Paolo Musolino Date: Tue, 25 Feb 2025 11:14:38 +0100 Subject: [PATCH 18/25] update: omit the unused siteID and productID to avoid confusion --- .../ProductImagesCollectionViewDataSource.swift | 4 ++-- .../Products/Media/ProductImageStatus+Extension.swift | 6 +++--- .../Media/ProductImagesCollectionViewController.swift | 2 +- .../Products/Media/ProductImagesViewController.swift | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/WooCommerce/Classes/ViewRelated/Products/Cells/Product Images/ProductImagesCollectionViewDataSource.swift b/WooCommerce/Classes/ViewRelated/Products/Cells/Product Images/ProductImagesCollectionViewDataSource.swift index ff96307717b..d41f9725ff1 100644 --- a/WooCommerce/Classes/ViewRelated/Products/Cells/Product Images/ProductImagesCollectionViewDataSource.swift +++ b/WooCommerce/Classes/ViewRelated/Products/Cells/Product Images/ProductImagesCollectionViewDataSource.swift @@ -51,9 +51,9 @@ private extension ProductImagesCollectionViewDataSource { func configureImageCell(_ cell: UICollectionViewCell, productImageStatus: ProductImageStatus, isFirstImage: Bool) { switch productImageStatus { - case .remote(let image, let siteID, let productID): + case .remote(let image, _, _): configureRemoteImageCell(cell, productImage: image, isFirstImage: isFirstImage) - case .uploading(let asset, let siteID, let productID): + case .uploading(let asset, _, _): switch asset { case .phAsset(let asset): configureUploadingImageCell(cell, asset: asset) diff --git a/WooCommerce/Classes/ViewRelated/Products/Media/ProductImageStatus+Extension.swift b/WooCommerce/Classes/ViewRelated/Products/Media/ProductImageStatus+Extension.swift index ffb9f6e8876..24ee96a7886 100644 --- a/WooCommerce/Classes/ViewRelated/Products/Media/ProductImageStatus+Extension.swift +++ b/WooCommerce/Classes/ViewRelated/Products/Media/ProductImageStatus+Extension.swift @@ -5,7 +5,7 @@ extension Collection where Element == ProductImageStatus { var images: [ProductImage] { compactMap { status in switch status { - case .remote(let productImage, let siteID, let productID): + case .remote(let productImage, _, _): return productImage default: return nil @@ -48,7 +48,7 @@ extension ProductImageStatus { /// var dragItemIdentifier: String { switch self { - case .uploading(let asset, let siteID, let productID): + case .uploading(let asset, _, _): switch asset { case let .phAsset(asset): return asset.identifier() @@ -57,7 +57,7 @@ extension ProductImageStatus { } case .uploadFailure: return UUID().uuidString - case .remote(let image, let siteID, let productID): + case .remote(let image, _, _): return "\(image.imageID)" } } diff --git a/WooCommerce/Classes/ViewRelated/Products/Media/ProductImagesCollectionViewController.swift b/WooCommerce/Classes/ViewRelated/Products/Media/ProductImagesCollectionViewController.swift index c5b5c35703c..c4a237b8e39 100644 --- a/WooCommerce/Classes/ViewRelated/Products/Media/ProductImagesCollectionViewController.swift +++ b/WooCommerce/Classes/ViewRelated/Products/Media/ProductImagesCollectionViewController.swift @@ -186,7 +186,7 @@ extension ProductImagesCollectionViewController { switch status { case .remote: break - case let .uploadFailure(asset, error, siteID, productID): + case let .uploadFailure(asset, error, _, _): return onFailedUploadSelected(asset, error) case .uploading: return diff --git a/WooCommerce/Classes/ViewRelated/Products/Media/ProductImagesViewController.swift b/WooCommerce/Classes/ViewRelated/Products/Media/ProductImagesViewController.swift index 4a788796141..fde156eaf71 100644 --- a/WooCommerce/Classes/ViewRelated/Products/Media/ProductImagesViewController.swift +++ b/WooCommerce/Classes/ViewRelated/Products/Media/ProductImagesViewController.swift @@ -26,7 +26,7 @@ final class ProductImagesViewController: UIViewController { private var productImages: [ProductImage] { return productImageStatuses.compactMap { status in switch status { - case .remote(let productImage, let siteID, let productID): + case .remote(let productImage, _, _): return productImage default: return nil From aafaa19947561de71163f78230f57b87f1cf5cbc Mon Sep 17 00:00:00 2001 From: Paolo Musolino Date: Tue, 25 Feb 2025 11:18:17 +0100 Subject: [PATCH 19/25] Update method parameter from `.product(id: productID.id)` to `.productID` in `ProductImageUploaderTests.swift` --- .../ViewRelated/Products/Media/ProductImageUploaderTests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImageUploaderTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImageUploaderTests.swift index 457b17ad94c..b7b64374a59 100644 --- a/WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImageUploaderTests.swift +++ b/WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImageUploaderTests.swift @@ -18,7 +18,7 @@ final class ProductImageUploaderTests: XCTestCase { let imageUploader = ProductImageUploader(stores: stores, imagesProductIDUpdater: mockProductIDUpdater) let actionHandler = imageUploader.actionHandler(key: .init(siteID: siteID, - productOrVariationID: .product(id: productID.id), + productOrVariationID: .productID, isLocalID: false), originalStatuses: []) let asset = PHAsset() From 0c362dcf9ef0485fab34bfcd6f1d42b2fb941ed2 Mon Sep 17 00:00:00 2001 From: Paolo Musolino Date: Tue, 25 Feb 2025 11:35:40 +0100 Subject: [PATCH 20/25] Enhance error handling in ProductImageStatus - Add new cases for `errorDomain`, `errorCode`, and `errorUserInfo` in `ProductImageStatus.swift`. - Modify decoding logic to handle detailed error information. - Update encoding logic to include `errorDomain`, `errorCode`, and `errorUserInfo`. - Change `productID` to be always encoded in `uploading` and `uploadFailure` cases. --- .../ProductImageStatus.swift | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/Networking/Networking/ProductImageInBackground/ProductImageStatus.swift b/Networking/Networking/ProductImageInBackground/ProductImageStatus.swift index 3a3e650e92a..2303e66193a 100644 --- a/Networking/Networking/ProductImageInBackground/ProductImageStatus.swift +++ b/Networking/Networking/ProductImageInBackground/ProductImageStatus.swift @@ -25,11 +25,15 @@ public enum ProductImageStatus: Equatable, Codable { case error case siteID case productID + case errorDomain + case errorCode + case errorUserInfo } public init(from decoder: Decoder) throws { let container = try decoder.container(keyedBy: CodingKeys.self) let typeString = try container.decode(String.self, forKey: .type) + switch typeString { case "uploading": let asset = try container.decode(ProductImageAssetType.self, forKey: .asset) @@ -43,11 +47,12 @@ public enum ProductImageStatus: Equatable, Codable { self = .remote(image: image, siteID: sID, productID: pID) case "uploadFailure": let asset = try container.decode(ProductImageAssetType.self, forKey: .asset) - let errorMessage = try container.decode(String.self, forKey: .error) - let error = NSError(domain: "ProductImageStatus", code: 0, userInfo: [NSLocalizedDescriptionKey: errorMessage]) let sID = try container.decode(Int64.self, forKey: .siteID) let pID = try container.decode(ProductOrVariationID.self, forKey: .productID) - self = .uploadFailure(asset: asset, error: error, siteID: sID, productID: pID) + let domain = try container.decode(String.self, forKey: .errorDomain) + let code = try container.decode(Int.self, forKey: .errorCode) + let userInfo = try container.decode([String: String]?.self, forKey: .errorUserInfo) + self = .uploadFailure(asset: asset, error: NSError(domain: domain, code: code, userInfo: userInfo), siteID: sID, productID: pID) default: throw DecodingError.dataCorruptedError(forKey: .type, in: container, @@ -62,7 +67,7 @@ public enum ProductImageStatus: Equatable, Codable { try container.encode("uploading", forKey: .type) try container.encode(asset, forKey: .asset) try container.encode(siteID, forKey: .siteID) - try container.encodeIfPresent(productID, forKey: .productID) + try container.encode(productID, forKey: .productID) case .remote(let image, let siteID, let productID): try container.encode("remote", forKey: .type) try container.encode(image, forKey: .image) @@ -71,10 +76,13 @@ public enum ProductImageStatus: Equatable, Codable { case .uploadFailure(let asset, let error, let siteID, let productID): try container.encode("uploadFailure", forKey: .type) try container.encode(asset, forKey: .asset) - let errorMessage = (error as NSError).localizedDescription - try container.encode(errorMessage, forKey: .error) try container.encode(siteID, forKey: .siteID) - try container.encodeIfPresent(productID, forKey: .productID) + try container.encode(productID, forKey: .productID) + + let errorMessage = error as NSError + try container.encode(errorMessage.domain, forKey: .errorDomain) + try container.encode(errorMessage.code, forKey: .errorCode) + try container.encode(errorMessage.userInfo as? [String: String], forKey: .errorUserInfo) } } From 6df709a0c6c29a6a13f54c6233c332821bab33da Mon Sep 17 00:00:00 2001 From: Paolo Musolino Date: Tue, 25 Feb 2025 11:49:35 +0100 Subject: [PATCH 21/25] Update the upload failure condition to include siteID and productID comparison in ProductImageActionHandler --- .../ViewRelated/Products/Media/ProductImageActionHandler.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WooCommerce/Classes/ViewRelated/Products/Media/ProductImageActionHandler.swift b/WooCommerce/Classes/ViewRelated/Products/Media/ProductImageActionHandler.swift index d9e8ab9db08..74bf1792765 100644 --- a/WooCommerce/Classes/ViewRelated/Products/Media/ProductImageActionHandler.swift +++ b/WooCommerce/Classes/ViewRelated/Products/Media/ProductImageActionHandler.swift @@ -317,7 +317,7 @@ private extension ProductImageActionHandler { case .uploading(let uploadingAsset, let siteID, let productID): return uploadingAsset == asset && siteID == self.siteID && productID == self.productOrVariationID case let .uploadFailure(failedAsset, _, _, _): - return failedAsset == asset + return failedAsset == asset && siteID == self.siteID && productID == self.productOrVariationID case .remote: return false } From 666adbe37112a0e0980d1dc4d78667c668b660e6 Mon Sep 17 00:00:00 2001 From: Paolo Musolino Date: Tue, 25 Feb 2025 15:04:47 +0100 Subject: [PATCH 22/25] fix: passing siteID and productID in uploadFailure --- .../ViewRelated/Products/Media/ProductImageActionHandler.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WooCommerce/Classes/ViewRelated/Products/Media/ProductImageActionHandler.swift b/WooCommerce/Classes/ViewRelated/Products/Media/ProductImageActionHandler.swift index 74bf1792765..01ad80d8490 100644 --- a/WooCommerce/Classes/ViewRelated/Products/Media/ProductImageActionHandler.swift +++ b/WooCommerce/Classes/ViewRelated/Products/Media/ProductImageActionHandler.swift @@ -316,7 +316,7 @@ private extension ProductImageActionHandler { switch status { case .uploading(let uploadingAsset, let siteID, let productID): return uploadingAsset == asset && siteID == self.siteID && productID == self.productOrVariationID - case let .uploadFailure(failedAsset, _, _, _): + case .uploadFailure(let failedAsset, _, let siteID, let productID): return failedAsset == asset && siteID == self.siteID && productID == self.productOrVariationID case .remote: return false From a66c8008baf08b0ff316048048ae7cc7af1e15c4 Mon Sep 17 00:00:00 2001 From: Paolo Musolino Date: Tue, 25 Feb 2025 23:32:29 +0100 Subject: [PATCH 23/25] fix: typo --- .../ViewRelated/Products/Media/ProductImageUploaderTests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImageUploaderTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImageUploaderTests.swift index b7b64374a59..3e112b3bb4c 100644 --- a/WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImageUploaderTests.swift +++ b/WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImageUploaderTests.swift @@ -18,7 +18,7 @@ final class ProductImageUploaderTests: XCTestCase { let imageUploader = ProductImageUploader(stores: stores, imagesProductIDUpdater: mockProductIDUpdater) let actionHandler = imageUploader.actionHandler(key: .init(siteID: siteID, - productOrVariationID: .productID, + productOrVariationID: productID, isLocalID: false), originalStatuses: []) let asset = PHAsset() From 9659be101a339ee174ae3d20f7b5e122deda30a9 Mon Sep 17 00:00:00 2001 From: Paolo Musolino Date: Wed, 26 Feb 2025 00:18:21 +0100 Subject: [PATCH 24/25] - Modify `ProductImageActionHandlerTests.swift` to use `siteID` instead of a hardcoded value. - Change `ProductImageUploaderTests.swift` to correctly assign `productID.id` to `remoteProductID`. --- .../Products/Media/ProductImageActionHandlerTests.swift | 4 ++-- .../Products/Media/ProductImageUploaderTests.swift | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImageActionHandlerTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImageActionHandlerTests.swift index 5dbba4b3889..fb6e8cf0c8b 100644 --- a/WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImageActionHandlerTests.swift +++ b/WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImageActionHandlerTests.swift @@ -26,8 +26,8 @@ final class ProductImageActionHandlerTests: XCTestCase { ProductImage(imageID: 1, dateCreated: Date(), dateModified: Date(), src: "", name: "", alt: ""), ProductImage(imageID: 2, dateCreated: Date(), dateModified: Date(), src: "", name: "", alt: "") ] - let mockRemoteProductImageStatuses = mockProductImages.map { ProductImageStatus.remote(image: $0, siteID: 0, productID: productID) } - let mockProduct = Product.fake().copy(siteID: 0, productID: productID.id, images: mockProductImages) + let mockRemoteProductImageStatuses = mockProductImages.map { ProductImageStatus.remote(image: $0, siteID: siteID, productID: productID) } + let mockProduct = Product.fake().copy(siteID: siteID, productID: productID.id, images: mockProductImages) let model = EditableProductModel(product: mockProduct) let productImageActionHandler = ProductImageActionHandler(siteID: siteID, diff --git a/WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImageUploaderTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImageUploaderTests.swift index 3e112b3bb4c..138d5e027f3 100644 --- a/WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImageUploaderTests.swift +++ b/WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImageUploaderTests.swift @@ -228,7 +228,7 @@ final class ProductImageUploaderTests: XCTestCase { imagesProductIDUpdater: mockProductIDUpdater) let localProductID: Int64 = 0 let nonExistentProductID: Int64 = 999 - let remoteProductID = productID + let remoteProductID = productID.id let originalStatuses: [ProductImageStatus] = [.remote(image: ProductImage.fake(), siteID: siteID, productID: productID), .uploading(asset: .phAsset(asset: PHAsset()), siteID: siteID, productID: productID), .uploading(asset: .phAsset(asset: PHAsset()), siteID: siteID, productID: productID)] @@ -238,7 +238,7 @@ final class ProductImageUploaderTests: XCTestCase { originalStatuses: originalStatuses) // When - imageUploader.replaceLocalID(siteID: siteID, localID: .product(id: nonExistentProductID), remoteID: remoteProductID.id) + imageUploader.replaceLocalID(siteID: siteID, localID: .product(id: nonExistentProductID), remoteID: remoteProductID) // Then // Ensure that trying to replace a non-existent product ID does nothing. From 6ea99b33e9873bd8b8a72dad3c465e964e9cc5c8 Mon Sep 17 00:00:00 2001 From: Paolo Musolino Date: Wed, 26 Feb 2025 11:35:29 +0100 Subject: [PATCH 25/25] Update: Removed `ProductImagesUserDefaultsStatuses` since it has been extensively updated later on. --- .../Networking.xcodeproj/project.pbxproj | 4 - .../ProductImagesUserDefaultsStatuses.swift | 99 ------------------- 2 files changed, 103 deletions(-) delete mode 100644 Networking/Networking/ProductImageInBackground/ProductImagesUserDefaultsStatuses.swift diff --git a/Networking/Networking.xcodeproj/project.pbxproj b/Networking/Networking.xcodeproj/project.pbxproj index b5e5d24b383..e3ce95d7cc4 100644 --- a/Networking/Networking.xcodeproj/project.pbxproj +++ b/Networking/Networking.xcodeproj/project.pbxproj @@ -480,7 +480,6 @@ 457A574025D1817E000797AD /* ShippingLabelAddressVerification.swift in Sources */ = {isa = PBXBuildFile; fileRef = 457A573F25D1817E000797AD /* ShippingLabelAddressVerification.swift */; }; 457FC68C2382B2FD00B41B02 /* product-update.json in Resources */ = {isa = PBXBuildFile; fileRef = 457FC68B2382B2FD00B41B02 /* product-update.json */; }; 4587D1152D64CBC2001971E4 /* ProductImageStatus.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4587D1142D64CBC2001971E4 /* ProductImageStatus.swift */; }; - 4587D11D2D64D559001971E4 /* ProductImagesUserDefaultsStatuses.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4587D11C2D64D556001971E4 /* ProductImagesUserDefaultsStatuses.swift */; }; 4587D11F2D64D887001971E4 /* ProductOrVariationID.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4587D11E2D64D886001971E4 /* ProductOrVariationID.swift */; }; 458C6DE425AC72A1009B300D /* StoredProductSettings.swift in Sources */ = {isa = PBXBuildFile; fileRef = 458C6DE325AC72A1009B300D /* StoredProductSettings.swift */; }; 4595827C264D5B3F000A4413 /* ShippingLabelPackageSelected.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4595827B264D5B3F000A4413 /* ShippingLabelPackageSelected.swift */; }; @@ -1694,7 +1693,6 @@ 457A573F25D1817E000797AD /* ShippingLabelAddressVerification.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ShippingLabelAddressVerification.swift; sourceTree = ""; }; 457FC68B2382B2FD00B41B02 /* product-update.json */ = {isa = PBXFileReference; lastKnownFileType = text.json; path = "product-update.json"; sourceTree = ""; }; 4587D1142D64CBC2001971E4 /* ProductImageStatus.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ProductImageStatus.swift; sourceTree = ""; }; - 4587D11C2D64D556001971E4 /* ProductImagesUserDefaultsStatuses.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ProductImagesUserDefaultsStatuses.swift; sourceTree = ""; }; 4587D11E2D64D886001971E4 /* ProductOrVariationID.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ProductOrVariationID.swift; sourceTree = ""; }; 458C6DE325AC72A1009B300D /* StoredProductSettings.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = StoredProductSettings.swift; sourceTree = ""; }; 4595827B264D5B3F000A4413 /* ShippingLabelPackageSelected.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ShippingLabelPackageSelected.swift; sourceTree = ""; }; @@ -2703,7 +2701,6 @@ children = ( 4587D1142D64CBC2001971E4 /* ProductImageStatus.swift */, 4587D11E2D64D886001971E4 /* ProductOrVariationID.swift */, - 4587D11C2D64D556001971E4 /* ProductImagesUserDefaultsStatuses.swift */, ); path = ProductImageInBackground; sourceTree = ""; @@ -5382,7 +5379,6 @@ CED9BCC12D3EAF7B00C063B8 /* WooShippingAddressValidationError.swift in Sources */, 45CDAFAB2434CA9300F83C22 /* ProductCatalogVisibility.swift in Sources */, 3148977027232982007A86BD /* SystemStatus.swift in Sources */, - 4587D11D2D64D559001971E4 /* ProductImagesUserDefaultsStatuses.swift in Sources */, B557DA1820979D51005962F4 /* Credentials.swift in Sources */, DEC51AF72769A15B009F3DF4 /* SystemStatusReport+Security.swift in Sources */, EE1D9A9F2ACD6BA60020D817 /* AIProduct.swift in Sources */, diff --git a/Networking/Networking/ProductImageInBackground/ProductImagesUserDefaultsStatuses.swift b/Networking/Networking/ProductImageInBackground/ProductImagesUserDefaultsStatuses.swift deleted file mode 100644 index 790df97490e..00000000000 --- a/Networking/Networking/ProductImageInBackground/ProductImagesUserDefaultsStatuses.swift +++ /dev/null @@ -1,99 +0,0 @@ -import Foundation - -/// Save product image upload statuses in User Defaults. -/// This class is declared in the Networking layer because it will also be accessed by the background URLSession operations. -/// -final class ProductImagesUserDefaultsStatuses { - private static let key = "savedProductUploadImageStatuses" - - static func addStatus(_ status: ProductImageStatus) { - var statuses = getAllStatuses() - statuses.append(status) - saveAllStatuses(statuses) - } - - static func removeStatus(_ status: ProductImageStatus) { - var statuses = getAllStatuses() - statuses.removeAll(where: { $0 == status }) - saveAllStatuses(statuses) - } - - static func findStatus(where predicate: (ProductImageStatus) -> Bool) -> ProductImageStatus? { - return getAllStatuses().first(where: predicate) - } - - static func getAllStatuses() -> [ProductImageStatus] { - guard let data = UserDefaults.standard.data(forKey: key) else { - return [] - } - do { - let statuses = try JSONDecoder().decode([ProductImageStatus].self, from: data) - return statuses - } catch { - DDLogError("Error decoding saved product image statuses: \(error)") - return [] - } - } - - static func getAllStatuses(for siteID: Int64, productID: ProductOrVariationID?) -> [ProductImageStatus] { - return getAllStatuses().filter { status in - switch status { - case .remote(_, let sID, let pID): - if let filterProductID = productID { - return sID == siteID && pID == filterProductID - } else { - return false - } - case .uploading(_, let sID, let pID), - .uploadFailure(_, _, let sID, let pID): - if let filterProductID = productID { - return sID == siteID && pID == filterProductID - } else { - return sID == siteID && pID == nil - } - } - } - } - - static func clearAllStatuses() { - UserDefaults.standard.removeObject(forKey: key) - } - - private static func saveAllStatuses(_ statuses: [ProductImageStatus]) { - do { - let data = try JSONEncoder().encode(statuses) - UserDefaults.standard.set(data, forKey: key) - } catch { - DDLogError("Error encoding saved product image statuses: \(error)") - } - } -} - -extension ProductImagesUserDefaultsStatuses { - static func setAllStatuses(_ statuses: [ProductImageStatus]) { - saveAllStatuses(statuses) - } - - static func setAllStatuses(_ statuses: [ProductImageStatus], for siteID: Int64, productID: ProductOrVariationID?) { - // Merge with existing, removing any old statuses for this site/product - var all = getAllStatuses().filter { st in - switch st { - case .remote(_, let sID, let pID): - if let filterProductID = productID { - return !(sID == siteID && pID == filterProductID) - } else { - return true - } - case .uploading(_, let sID, let pID), - .uploadFailure(_, _, let sID, let pID): - if let filterProductID = productID { - return !(sID == siteID && pID == filterProductID) - } else { - return !(sID == siteID && pID == nil) - } - } - } - all.append(contentsOf: statuses) - saveAllStatuses(all) - } -}