Skip to content

Commit e1e5d34

Browse files
Merge pull request #7126 from woocommerce/feat/7021-add-product-flow
Background image uploads: Add product flow.
2 parents 3a67956 + 79a477c commit e1e5d34

File tree

7 files changed

+213
-14
lines changed

7 files changed

+213
-14
lines changed

WooCommerce/Classes/ServiceLocator/LegacyProductImageUploader.swift

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@ final class LegacyProductImageUploader: ProductImageUploaderProtocol {
66
ProductImageActionHandler(siteID: siteID, productID: productID, imageStatuses: originalStatuses)
77
}
88

9+
func replaceLocalID(siteID: Int64, localProductID: Int64, remoteProductID: Int64) {
10+
// no-op
11+
}
12+
913
func saveProductImagesWhenNoneIsPendingUploadAnymore(siteID: Int64,
1014
productID: Int64,
1115
isLocalID: Bool,

WooCommerce/Classes/ServiceLocator/ProductImageUploader.swift

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,19 @@ protocol ProductImageUploaderProtocol {
1212
/// - originalStatuses: the current image statuses of the product for initialization.
1313
func actionHandler(siteID: Int64, productID: Int64, isLocalID: Bool, originalStatuses: [ProductImageStatus]) -> ProductImageActionHandler
1414

15+
/// Replaces the local ID of the product with the remote ID from API.
16+
///
17+
/// Called in "Add product" flow as soon as the product is saved in the API.
18+
///
19+
/// Replacing product ID is necessary to update the product with the images that are already uploaded without product ID.
20+
/// Note that the images start uploading even before the product is created in API.
21+
///
22+
/// - Parameters:
23+
/// - siteID: The ID of the site to which images are uploaded to.
24+
/// - localProductID: A temporary local ID of the product.
25+
/// - remoteProductID: Remote product ID received from API.
26+
func replaceLocalID(siteID: Int64, localProductID: Int64, remoteProductID: Int64)
27+
1528
/// Saves the product remotely with the images after none is pending upload.
1629
/// - Parameters:
1730
/// - siteID: the ID of the site where images are uploaded to.
@@ -62,6 +75,16 @@ final class ProductImageUploader: ProductImageUploaderProtocol {
6275
return actionHandler
6376
}
6477

78+
func replaceLocalID(siteID: Int64, localProductID: Int64, remoteProductID: Int64) {
79+
let key = ProductKey(siteID: siteID, productID: localProductID, isLocalID: true)
80+
guard let handler = actionHandlersByProduct[key] else {
81+
return
82+
}
83+
actionHandlersByProduct.removeValue(forKey: key)
84+
let keyWithRemoteProductID = ProductKey(siteID: siteID, productID: remoteProductID, isLocalID: false)
85+
actionHandlersByProduct[keyWithRemoteProductID] = handler
86+
}
87+
6588
func hasUnsavedChangesOnImages(siteID: Int64, productID: Int64, isLocalID: Bool, originalImages: [ProductImage]) -> Bool {
6689
let key = ProductKey(siteID: siteID, productID: productID, isLocalID: isLocalID)
6790
guard let handler = actionHandlersByProduct[key] else {

WooCommerce/Classes/ViewRelated/Products/Edit Product/ProductFormViewModel.swift

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -417,6 +417,7 @@ extension ProductFormViewModel {
417417
let remoteActionUseCase = ProductFormRemoteActionUseCase()
418418
switch formType {
419419
case .add:
420+
let productIDBeforeSave = productModel.productID
420421
remoteActionUseCase.addProduct(product: productModelToSave, password: password) { [weak self] result in
421422
switch result {
422423
case .failure(let error):
@@ -429,6 +430,7 @@ extension ProductFormViewModel {
429430
self.resetPassword(data.password)
430431
onCompletion(.success(data.product))
431432
if self.isBackgroundImageUploadEnabled {
433+
self.replaceProductID(productIDBeforeSave: productIDBeforeSave)
432434
self.saveProductImagesWhenNoneIsPendingUploadAnymore()
433435
}
434436
}
@@ -458,6 +460,28 @@ extension ProductFormViewModel {
458460
}
459461
}
460462

463+
func deleteProductRemotely(onCompletion: @escaping (Result<Void, ProductUpdateError>) -> Void) {
464+
let remoteActionUseCase = ProductFormRemoteActionUseCase()
465+
remoteActionUseCase.deleteProduct(product: product) { result in
466+
switch result {
467+
case .success:
468+
onCompletion(.success(()))
469+
case .failure(let error):
470+
onCompletion(.failure(error))
471+
}
472+
}
473+
}
474+
}
475+
476+
// MARK: Background image upload
477+
//
478+
private extension ProductFormViewModel {
479+
func replaceProductID(productIDBeforeSave: Int64) {
480+
productImagesUploader.replaceLocalID(siteID: product.siteID,
481+
localProductID: productIDBeforeSave,
482+
remoteProductID: product.productID)
483+
}
484+
461485
func saveProductImagesWhenNoneIsPendingUploadAnymore() {
462486
productImagesUploader.saveProductImagesWhenNoneIsPendingUploadAnymore(siteID: product.siteID,
463487
productID: product.productID,
@@ -481,18 +505,6 @@ extension ProductFormViewModel {
481505
// The save request keeps track of the latest image statuses at the time of the call and their upload progress over time.
482506
isUpdateEnabledSubject.send(hasUnsavedChanges())
483507
}
484-
485-
func deleteProductRemotely(onCompletion: @escaping (Result<Void, ProductUpdateError>) -> Void) {
486-
let remoteActionUseCase = ProductFormRemoteActionUseCase()
487-
remoteActionUseCase.deleteProduct(product: product) { result in
488-
switch result {
489-
case .success:
490-
onCompletion(.success(()))
491-
case .failure(let error):
492-
onCompletion(.failure(error))
493-
}
494-
}
495-
}
496508
}
497509

498510
// MARK: Reset actions

WooCommerce/WooCommerce.xcodeproj/project.pbxproj

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1690,6 +1690,7 @@
16901690
EEADF622281A40CB001B40F1 /* ShippingValueLocalizer.swift in Sources */ = {isa = PBXBuildFile; fileRef = EEADF621281A40CB001B40F1 /* ShippingValueLocalizer.swift */; };
16911691
EEADF624281A421A001B40F1 /* DefaultShippingValueLocalizer.swift in Sources */ = {isa = PBXBuildFile; fileRef = EEADF623281A421A001B40F1 /* DefaultShippingValueLocalizer.swift */; };
16921692
EEADF626281A65A9001B40F1 /* DefaultShippingValueLocalizerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = EEADF625281A65A9001B40F1 /* DefaultShippingValueLocalizerTests.swift */; };
1693+
EECB7EE02862115C0028C888 /* MockProductImageUploader.swift in Sources */ = {isa = PBXBuildFile; fileRef = EECB7EDF2862115C0028C888 /* MockProductImageUploader.swift */; };
16931694
F997170523DBB97500592D8E /* WooCommerceScreenshots.swift in Sources */ = {isa = PBXBuildFile; fileRef = F997170423DBB97500592D8E /* WooCommerceScreenshots.swift */; };
16941695
F997174523DC068500592D8E /* XLPagerStrip+AccessibilityIdentifier.swift in Sources */ = {isa = PBXBuildFile; fileRef = F997174323DC065900592D8E /* XLPagerStrip+AccessibilityIdentifier.swift */; };
16951696
F997174723DC070D00592D8E /* XLPagerStrip+AccessibilityIdentifierTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = F997174623DC070C00592D8E /* XLPagerStrip+AccessibilityIdentifierTests.swift */; };
@@ -3471,6 +3472,7 @@
34713472
EEADF621281A40CB001B40F1 /* ShippingValueLocalizer.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ShippingValueLocalizer.swift; sourceTree = "<group>"; };
34723473
EEADF623281A421A001B40F1 /* DefaultShippingValueLocalizer.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DefaultShippingValueLocalizer.swift; sourceTree = "<group>"; };
34733474
EEADF625281A65A9001B40F1 /* DefaultShippingValueLocalizerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DefaultShippingValueLocalizerTests.swift; sourceTree = "<group>"; };
3475+
EECB7EDF2862115C0028C888 /* MockProductImageUploader.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockProductImageUploader.swift; sourceTree = "<group>"; };
34743476
F93E8E5124087FDA0057FF21 /* BetaFeaturesScreen.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = BetaFeaturesScreen.swift; sourceTree = "<group>"; };
34753477
F93E8E5224087FDA0057FF21 /* SettingsScreen.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = SettingsScreen.swift; sourceTree = "<group>"; };
34763478
F93E8E5724087FE10057FF21 /* ProductsScreen.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ProductsScreen.swift; sourceTree = "<group>"; };
@@ -5816,6 +5818,7 @@
58165818
B9C4AB2A28003481007008B8 /* MockPaymentsPluginsDataProvider.swift */,
58175819
0375799C2822F9040083F2E1 /* MockCardPresentPaymentsOnboardingPresenter.swift */,
58185820
02BF9BAE2851E7EA008CE2DD /* MockAppleIDCredentialChecker.swift */,
5821+
EECB7EDF2862115C0028C888 /* MockProductImageUploader.swift */,
58195822
);
58205823
path = Mocks;
58215824
sourceTree = "<group>";
@@ -10107,6 +10110,7 @@
1010710110
262A0999262908A60033AD20 /* OrderAddOnListI1Tests.swift in Sources */,
1010810111
0234680A282CEA5F00CFC503 /* ReceiptViewModelTests.swift in Sources */,
1010910112
B96B536B2816ECFC00F753E6 /* CardPresentPluginsDataProviderTests.swift in Sources */,
10113+
EECB7EE02862115C0028C888 /* MockProductImageUploader.swift in Sources */,
1011010114
022A45EE237BADA6001417F0 /* Product+ProductFormTests.swift in Sources */,
1011110115
B57C5C9921B80E7100FF82B2 /* DictionaryWooTests.swift in Sources */,
1011210116
26F94E34267AA42F00DB6CCF /* ProductAddOnViewModelTests.swift in Sources */,
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
@testable import Yosemite
2+
@testable import WooCommerce
3+
4+
final class MockProductImageUploader: ProductImageUploaderProtocol {
5+
var replaceLocalIDWasCalled = false
6+
var saveProductImagesWhenNoneIsPendingUploadAnymoreWasCalled = false
7+
8+
func replaceLocalID(siteID: Int64, localProductID: Int64, remoteProductID: Int64) {
9+
replaceLocalIDWasCalled = true
10+
}
11+
12+
func saveProductImagesWhenNoneIsPendingUploadAnymore(siteID: Int64,
13+
productID: Int64,
14+
isLocalID: Bool,
15+
onProductSave: @escaping (Result<[ProductImage], Error>) -> Void) {
16+
saveProductImagesWhenNoneIsPendingUploadAnymoreWasCalled = true
17+
}
18+
19+
func actionHandler(siteID: Int64, productID: Int64, isLocalID: Bool, originalStatuses: [ProductImageStatus]) -> ProductImageActionHandler {
20+
ProductImageActionHandler(siteID: 0, productID: 0, imageStatuses: [])
21+
}
22+
23+
func hasUnsavedChangesOnImages(siteID: Int64, productID: Int64, isLocalID: Bool, originalImages: [ProductImage]) -> Bool {
24+
false
25+
}
26+
}

WooCommerce/WooCommerceTests/ViewRelated/Products/Edit Product/ProductFormViewModel+SaveTests.swift

Lines changed: 74 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,49 @@ final class ProductFormViewModel_SaveTests: XCTestCase {
6868
XCTAssertEqual(savedProduct, EditableProductModel(product: product.copy(statusKey: ProductStatus.pending.rawValue)))
6969
}
7070

71+
func test_adding_a_product_remotely_fires_replaceLocalID_in_productImagesUploader() throws {
72+
// Given
73+
let product = Product.fake().copy(statusKey: ProductStatus.published.rawValue)
74+
let productImagesUploader = MockProductImageUploader()
75+
let viewModel = createViewModel(product: product, formType: .add, productImagesUploader: productImagesUploader, isBackgroundImageUploadEnabled: true)
76+
storesManager.whenReceivingAction(ofType: ProductAction.self) { action in
77+
if case let ProductAction.addProduct(product, onCompletion) = action {
78+
onCompletion(.success(product))
79+
}
80+
}
81+
82+
// When
83+
waitForExpectation { expectation in
84+
viewModel.saveProductRemotely(status: .pending) { result in
85+
expectation.fulfill()
86+
}
87+
}
88+
// Then
89+
XCTAssertTrue(productImagesUploader.replaceLocalIDWasCalled)
90+
}
91+
92+
func test_adding_a_product_remotely_fires_method_to_save_images_in_background_using_productImagesUploader() throws {
93+
// Given
94+
let product = Product.fake().copy(statusKey: ProductStatus.published.rawValue)
95+
let productImagesUploader = MockProductImageUploader()
96+
let viewModel = createViewModel(product: product, formType: .add, productImagesUploader: productImagesUploader, isBackgroundImageUploadEnabled: true)
97+
storesManager.whenReceivingAction(ofType: ProductAction.self) { action in
98+
if case let ProductAction.addProduct(product, onCompletion) = action {
99+
onCompletion(.success(product))
100+
}
101+
}
102+
103+
// When
104+
waitForExpectation { expectation in
105+
viewModel.saveProductRemotely(status: .pending) { result in
106+
expectation.fulfill()
107+
}
108+
}
109+
110+
// Then
111+
XCTAssertTrue(productImagesUploader.saveProductImagesWhenNoneIsPendingUploadAnymoreWasCalled)
112+
}
113+
71114
// MARK: `saveProductRemotely` for editing a product
72115

73116
func test_editing_a_product_remotely_with_nil_status_uses_the_original_product() throws {
@@ -115,14 +158,43 @@ final class ProductFormViewModel_SaveTests: XCTestCase {
115158
// Assert
116159
XCTAssertEqual(savedProduct, EditableProductModel(product: product.copy(statusKey: ProductStatus.pending.rawValue)))
117160
}
161+
162+
func test_editing_a_product_remotely_fires_method_to_save_images_in_background_using_productImagesUploader() throws {
163+
// Given
164+
let product = Product.fake().copy(statusKey: ProductStatus.published.rawValue)
165+
let productImagesUploader = MockProductImageUploader()
166+
let viewModel = createViewModel(product: product, formType: .edit, productImagesUploader: productImagesUploader, isBackgroundImageUploadEnabled: true)
167+
storesManager.whenReceivingAction(ofType: ProductAction.self) { action in
168+
if case let ProductAction.updateProduct(product, onCompletion) = action {
169+
onCompletion(.success(product))
170+
}
171+
}
172+
173+
// When
174+
waitForExpectation { expectation in
175+
viewModel.saveProductRemotely(status: .pending) { result in
176+
expectation.fulfill()
177+
}
178+
}
179+
180+
// Then
181+
XCTAssertTrue(productImagesUploader.saveProductImagesWhenNoneIsPendingUploadAnymoreWasCalled)
182+
}
118183
}
119184

120185
private extension ProductFormViewModel_SaveTests {
121-
func createViewModel(product: Product, formType: ProductFormType) -> ProductFormViewModel {
186+
func createViewModel(
187+
product: Product,
188+
formType: ProductFormType,
189+
productImagesUploader: ProductImageUploaderProtocol = ServiceLocator.productImageUploader,
190+
isBackgroundImageUploadEnabled: Bool = ServiceLocator.featureFlagService.isFeatureFlagEnabled(.backgroundProductImageUpload)
191+
) -> ProductFormViewModel {
122192
let model = EditableProductModel(product: product)
123193
let productImageActionHandler = ProductImageActionHandler(siteID: 0, product: model)
124194
return ProductFormViewModel(product: model,
125195
formType: formType,
126-
productImageActionHandler: productImageActionHandler)
196+
productImageActionHandler: productImageActionHandler,
197+
productImagesUploader: productImagesUploader,
198+
isBackgroundImageUploadEnabled: isBackgroundImageUploadEnabled)
127199
}
128200
}

WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImageUploaderTests.swift

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,4 +135,62 @@ final class ProductImageUploaderTests: XCTestCase {
135135
let images = try XCTUnwrap(resultOfSavedImages.get())
136136
XCTAssertEqual(images.map { $0.imageID }, [606, 645])
137137
}
138+
139+
func test_replaceLocalID_replaces_productID_properly() {
140+
// Given
141+
let imageUploader = ProductImageUploader()
142+
let localProductID: Int64 = 0
143+
let remoteProductID = productID
144+
let originalStatuses: [ProductImageStatus] = [.remote(image: ProductImage.fake()),
145+
.uploading(asset: PHAsset()),
146+
.uploading(asset: PHAsset())]
147+
_ = imageUploader.actionHandler(siteID: siteID,
148+
productID: localProductID,
149+
isLocalID: true,
150+
originalStatuses: originalStatuses)
151+
152+
// Before replacing product ID
153+
154+
// Pass empty statuses to get the `actionHandler`, and validate that `actionHandler` with `originalStatuses` is returned.
155+
XCTAssertEqual(originalStatuses, imageUploader.actionHandler(siteID: siteID,
156+
productID: localProductID,
157+
isLocalID: true,
158+
originalStatuses: []).productImageStatuses)
159+
160+
// When
161+
imageUploader.replaceLocalID(siteID: siteID, localProductID: localProductID, remoteProductID: remoteProductID)
162+
163+
// After replacing local product ID with remote product ID
164+
165+
// Pass empty statuses and `remoteProductID` to get the `actionHandler`, and validate that `actionHandler` with `originalStatuses` is returned.
166+
XCTAssertEqual(originalStatuses, imageUploader.actionHandler(siteID: siteID,
167+
productID: remoteProductID,
168+
isLocalID: false,
169+
originalStatuses: []).productImageStatuses)
170+
}
171+
172+
func test_calling_replaceLocalID_with_nonExistent_localProductID_does_nothing() {
173+
// Given
174+
let imageUploader = ProductImageUploader()
175+
let localProductID: Int64 = 0
176+
let nonExistentProductID: Int64 = 999
177+
let remoteProductID = productID
178+
let originalStatuses: [ProductImageStatus] = [.remote(image: ProductImage.fake()),
179+
.uploading(asset: PHAsset()),
180+
.uploading(asset: PHAsset())]
181+
_ = imageUploader.actionHandler(siteID: siteID,
182+
productID: localProductID,
183+
isLocalID: true,
184+
originalStatuses: originalStatuses)
185+
186+
// When
187+
imageUploader.replaceLocalID(siteID: siteID, localProductID: nonExistentProductID, remoteProductID: remoteProductID)
188+
189+
// Then
190+
// Ensure that trying to replace a non-existent product ID does nothing.
191+
XCTAssertEqual(originalStatuses, imageUploader.actionHandler(siteID: siteID,
192+
productID: localProductID,
193+
isLocalID: true,
194+
originalStatuses: []).productImageStatuses)
195+
}
138196
}

0 commit comments

Comments
 (0)