Skip to content

Commit 0758361

Browse files
authored
Merge pull request #8565 from woocommerce/issue/8492-inform-variations-alert
Variations: Ask for user confirmation before creating variations
2 parents 2c44962 + cbc110b commit 0758361

File tree

3 files changed

+144
-20
lines changed

3 files changed

+144
-20
lines changed

WooCommerce/Classes/ViewRelated/Products/Variations/ProductVariationsViewController.swift

Lines changed: 42 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -626,6 +626,21 @@ private extension ProductVariationsViewController {
626626
let notice = Notice(title: error.errorTitle, message: error.errorDescription)
627627
noticePresenter.enqueue(notice: notice)
628628
}
629+
630+
/// Asks the merchant for confirmation before generating all variations.
631+
///
632+
private func presentGenerationConfirmation(numberOfVariations: Int, onCompletion: @escaping (_ confirmed: Bool) -> Void) {
633+
let controller = UIAlertController(title: Localization.confirmationTitle,
634+
message: Localization.confirmationDescription(variationCount: numberOfVariations),
635+
preferredStyle: .alert)
636+
controller.addDefaultActionWithTitle(Localization.ok) { _ in
637+
onCompletion(true)
638+
}
639+
controller.addCancelActionWithTitle(Localization.cancel) { _ in
640+
onCompletion(false)
641+
}
642+
present(controller, animated: true)
643+
}
629644
}
630645

631646
// MARK: - Placeholders
@@ -702,18 +717,25 @@ extension ProductVariationsViewController: SyncingCoordinatorDelegate {
702717
/// Generates all possible variations for the product attributes.
703718
///
704719
private func generateAllVariations() {
705-
viewModel.generateAllVariations(for: product) { [weak self] result in
706-
guard let self else { return }
707-
switch result {
708-
case .success:
720+
viewModel.generateAllVariations(for: product) { [weak self] currentState in
721+
switch currentState {
722+
case .fetching:
723+
break // TODO: Show fetching loading Indicator
724+
case .confirmation(let variationCount, let onCompletion):
725+
self?.presentGenerationConfirmation(numberOfVariations: variationCount, onCompletion: onCompletion)
726+
case .creating:
727+
break // TODO: Show creating loading Indicator
728+
case .canceled:
729+
break // TODO: Remove loading indicator
730+
case .finished(let variationsCreated):
731+
// TODO: Remove loading indicator
732+
// TODO: Inform about created variations
709733
break
710-
case .failure(let error):
711-
self.presentGenerationError(error)
734+
case .error(let error):
735+
// TODO: Remove loading indicator
736+
self?.presentGenerationError(error)
712737
}
713738
}
714-
// TODO:
715-
// - Show Loading Indicator
716-
// - Hide Loading Indicator
717739
}
718740
}
719741

@@ -820,6 +842,17 @@ private extension ProductVariationsViewController {
820842
static let generateVariationError = NSLocalizedString("The variation couldn't be generated.",
821843
comment: "Error title when failing to generate a variation.")
822844
static let variationCreated = NSLocalizedString("Variation created", comment: "Text for the notice after creating the first variation.")
845+
846+
static let confirmationTitle = NSLocalizedString("Generate all variations?",
847+
comment: "Alert title to allow the user confirm if they want to generate all variations")
848+
static func confirmationDescription(variationCount: Int) -> String {
849+
let format = NSLocalizedString("This will create a variation for each and every possible combination of variation attributes (%d variations).",
850+
comment: "Alert description to allow the user confirm if they want to generate all variations")
851+
return String.localizedStringWithFormat(format, variationCount)
852+
}
853+
static let ok = NSLocalizedString("OK", comment: "Button text to confirm that we want to generate all variations")
854+
static let cancel = NSLocalizedString("Cancel", comment: "Button text to confirm that we don't want to generate all variations")
855+
823856
}
824857

825858
/// Localizated strings for the action sheet options

WooCommerce/Classes/ViewRelated/Products/Variations/ProductVariationsViewModel.swift

Lines changed: 57 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,27 +26,49 @@ final class ProductVariationsViewModel {
2626
}
2727

2828
/// Generates all missing variations for a product. Up to 100 variations.
29+
/// Parameters:
30+
/// - `Product`: Product on which we will be creating the variations
31+
/// - `onStateChanged`: Closure invoked every time there is a significant state change in the generation process.
2932
///
30-
func generateAllVariations(for product: Product, onCompletion: @escaping (Result<Void, GenerationError>) -> Void) {
33+
func generateAllVariations(for product: Product, onStateChanged: @escaping (GenerationState) -> Void) {
3134

35+
// Fetch Previous variations
36+
onStateChanged(.fetching)
3237
fetchAllVariations(of: product) { [weak self] result in
33-
guard let self else { return }
3438
switch result {
3539
case .success(let existingVariations):
3640

41+
// Generate variations locally
3742
let variationsToGenerate = ProductVariationGenerator.generateVariations(for: product, excluding: existingVariations)
3843

3944
// Guard for 100 variation limit
4045
guard variationsToGenerate.count <= 100 else {
41-
return onCompletion(.failure(.tooManyVariations(variationCount: variationsToGenerate.count)))
46+
return onStateChanged(.error(.tooManyVariations(variationCount: variationsToGenerate.count)))
4247
}
4348

49+
// Guard for no variations to generate
4450
guard variationsToGenerate.count > 0 else {
45-
// TODO: Inform user that no variation will be created
46-
return onCompletion(.success(()))
51+
return onStateChanged(.finished(false))
4752
}
4853

49-
self.createVariationsRemotely(for: product, variations: variationsToGenerate, onCompletion: onCompletion)
54+
// Confirm generation with merchant
55+
onStateChanged(.confirmation(variationsToGenerate.count, { confirmed in
56+
57+
guard confirmed else {
58+
return onStateChanged(.canceled)
59+
}
60+
61+
// Create variations remotely
62+
onStateChanged(.creating)
63+
self?.createVariationsRemotely(for: product, variations: variationsToGenerate) { result in
64+
switch result {
65+
case .success:
66+
onStateChanged(.finished(true))
67+
case .failure(let error):
68+
onStateChanged(.error(error))
69+
}
70+
}
71+
}))
5072

5173
case .failure:
5274
// TODO: Log and inform error
@@ -106,7 +128,36 @@ extension ProductVariationsViewModel {
106128
}
107129
}
108130

131+
// MARK: Definitions for Generate All Variations
109132
extension ProductVariationsViewModel {
133+
/// Type that represents the possible states while all variations are being created.
134+
///
135+
enum GenerationState {
136+
/// State while previous variations are being fetched
137+
///
138+
case fetching
139+
140+
/// State to allow merchant to confirm the variation generation
141+
///
142+
case confirmation(_ numberOfVariations: Int, _ onCompletion: (_ confirmed: Bool) -> Void)
143+
144+
/// State while the variations are being created remotely
145+
///
146+
case creating
147+
148+
///State when the merchant decides to not continue with the generation process.
149+
///
150+
case canceled
151+
152+
/// State when the the process is finished. `variationsCreated` indicates if variations were created or not.
153+
///
154+
case finished(_ variationsCreated: Bool)
155+
156+
/// Error state in any part of the process.
157+
///
158+
case error(GenerationError)
159+
}
160+
110161
/// Type to represent known generation errors
111162
///
112163
enum GenerationError: LocalizedError, Equatable {

WooCommerce/WooCommerceTests/ViewRelated/Products/Variations/ProductVariationsViewModelTests.swift

Lines changed: 45 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -137,8 +137,8 @@ final class ProductVariationsViewModelTests: XCTestCase {
137137

138138
// When
139139
let error = waitFor { promise in
140-
viewModel.generateAllVariations(for: product) { result in
141-
if case let .failure(error) = result {
140+
viewModel.generateAllVariations(for: product) { state in
141+
if case let .error(error) = state {
142142
promise(error)
143143
}
144144
}
@@ -148,7 +148,7 @@ final class ProductVariationsViewModelTests: XCTestCase {
148148
XCTAssertEqual(error, .tooManyVariations(variationCount: 125))
149149
}
150150

151-
func test_generating_less_than_100_variations_invokes_create_action() {
151+
func test_generating_less_than_100_variations_ask_for_confirmation_and_creates_variations() {
152152
// Given
153153
let product = Product.fake().copy(attributes: [
154154
ProductAttribute.fake().copy(attributeID: 1, name: "Size", options: ["XS", "S", "M", "L", "XL"]),
@@ -171,8 +171,11 @@ final class ProductVariationsViewModelTests: XCTestCase {
171171

172172
// When
173173
let succeeded = waitFor { promise in
174-
viewModel.generateAllVariations(for: product) { result in
175-
if case .success = result {
174+
viewModel.generateAllVariations(for: product) { state in
175+
if case let .confirmation(_, onCompletion) = state {
176+
onCompletion(true)
177+
}
178+
if case .finished = state {
176179
promise(true)
177180
}
178181
}
@@ -181,4 +184,41 @@ final class ProductVariationsViewModelTests: XCTestCase {
181184
// Then
182185
XCTAssertTrue(succeeded)
183186
}
187+
188+
func test_generating_less_than_100_variations_ask_for_confirmation_and_sends_cancel_state() {
189+
// Given
190+
let product = Product.fake().copy(attributes: [
191+
ProductAttribute.fake().copy(attributeID: 1, name: "Size", options: ["XS", "S", "M", "L", "XL"]),
192+
ProductAttribute.fake().copy(attributeID: 2, name: "Color", options: ["Red", "Green", "Blue", "White", "Black"]),
193+
])
194+
195+
let stores = MockStoresManager(sessionManager: SessionManager.makeForTesting())
196+
stores.whenReceivingAction(ofType: ProductVariationAction.self) { action in
197+
switch action {
198+
case .synchronizeAllProductVariations(_, _, let onCompletion):
199+
onCompletion(.success([]))
200+
case .createProductVariations(_, _, _, let onCompletion):
201+
onCompletion(.success([]))
202+
default:
203+
break
204+
}
205+
}
206+
207+
let viewModel = ProductVariationsViewModel(stores: stores, formType: .edit)
208+
209+
// When
210+
let canceled = waitFor { promise in
211+
viewModel.generateAllVariations(for: product) { state in
212+
if case let .confirmation(_, onCompletion) = state {
213+
onCompletion(false)
214+
}
215+
if case .canceled = state {
216+
promise(true)
217+
}
218+
}
219+
}
220+
221+
// Then
222+
XCTAssertTrue(canceled)
223+
}
184224
}

0 commit comments

Comments
 (0)