Skip to content

Commit d5b81b1

Browse files
authored
Merge pull request #8541 from woocommerce/issue/8489-abort-if-multiple-variations
Variations: Stop generation if there are more than 100 variations to generate
2 parents 77ff590 + 5636c23 commit d5b81b1

File tree

4 files changed

+97
-12
lines changed

4 files changed

+97
-12
lines changed

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

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,17 +5,21 @@ import Yosemite
55
///
66
struct ProductVariationGenerator {
77

8-
/// Group a colection of attribute options.
8+
/// Group a collection of attribute options.
99
/// EG: [Size: Large, Color: Black, Fabric: Cotton]
1010
///
11-
private struct Combination: Hashable {
11+
private struct Combination: Hashable, Equatable {
1212
let options: [Option]
13+
14+
static func == (lhs: Combination, rhs: Combination) -> Bool {
15+
Set(lhs.options) == Set(rhs.options)
16+
}
1317
}
1418

1519
/// Represents an attribute option.
1620
/// EG: Size: Large
1721
///
18-
private struct Option: Hashable {
22+
private struct Option: Hashable, Equatable {
1923
let attributeID: Int64
2024
let attributeName: String
2125
let value: String
@@ -33,7 +37,7 @@ struct ProductVariationGenerator {
3337
/// Generates all posible combination for a product attributes.
3438
///
3539
private static func getCombinations(from product: Product) -> [Combination] {
36-
// Iterates through attributes while eceiving the previous combinations list.
40+
// Iterates through attributes while receiving the previous combinations list.
3741
product.attributes.reduce([Combination(options: [])]) { combinations, attribute in
3842
combinations.flatMap { combination in
3943
// When receiving a previous combination list, we add each attribute to each previous combination util we finish with them.
@@ -49,17 +53,17 @@ struct ProductVariationGenerator {
4953
private static func filterExistingCombinations(_ combinations: [Combination], existing variations: [ProductVariation]) -> [Combination] {
5054
// Convert variations into combinations
5155
let existingCombinations = variations.map { existingVariation in
52-
let options = existingVariation.attributes.map { attibute in
53-
Option(attributeID: attibute.id, attributeName: attibute.name, value: attibute.option)
56+
let options = existingVariation.attributes.map { attribute in
57+
Option(attributeID: attribute.id, attributeName: attribute.name, value: attribute.option)
5458
}
5559
return Combination(options: options)
5660
}
5761

5862
// Filter existing combinations.
59-
let existingSet = Set(existingCombinations)
60-
return combinations.filter { combination in
61-
!existingSet.contains(combination)
63+
let unique = combinations.filter { combination in
64+
!existingCombinations.contains(combination)
6265
}
66+
return unique
6367
}
6468

6569
/// Convert the provided combinations into `[CreateProductVariation]` types that are consumed by our Yosemite stores.

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

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -619,6 +619,13 @@ private extension ProductVariationsViewController {
619619
let bottomSheetPresenter = BottomSheetListSelectorPresenter(viewProperties: viewProperties, command: command)
620620
bottomSheetPresenter.show(from: self, sourceView: topStackView)
621621
}
622+
623+
/// Informs the merchant about errors that happen during the variation generation
624+
///
625+
private func presentGenerationError(_ error: ProductVariationsViewModel.GenerationError) {
626+
let notice = Notice(title: error.errorTitle, message: error.errorDescription)
627+
noticePresenter.enqueue(notice: notice)
628+
}
622629
}
623630

624631
// MARK: - Placeholders
@@ -695,10 +702,17 @@ extension ProductVariationsViewController: SyncingCoordinatorDelegate {
695702
/// Generates all possible variations for the product attibutes.
696703
///
697704
private func generateAllVariations() {
698-
viewModel.generateAllVariations(for: product)
705+
viewModel.generateAllVariations(for: product) { [weak self] result in
706+
guard let self else { return }
707+
switch result {
708+
case .success:
709+
break
710+
case .failure(let error):
711+
self.presentGenerationError(error)
712+
}
713+
}
699714
// TODO:
700715
// - Show Loading Indicator
701-
// - Alert if there are more than 100 variations to create
702716
// - Hide Loading Indicator
703717
}
704718
}

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

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ final class ProductVariationsViewModel {
2727

2828
/// Generates all missing variations for a product. Up to 100 variations.
2929
///
30-
func generateAllVariations(for product: Product) {
30+
func generateAllVariations(for product: Product, onCompletion: @escaping (Result<Void, GenerationError>) -> Void) {
3131
let action = ProductVariationAction.synchronizeAllProductVariations(siteID: product.siteID, productID: product.productID) { result in
3232
// TODO: Fetch this via a results controller
3333
let existingVariations = ServiceLocator.storageManager.viewStorage.loadProductVariations(siteID: product.siteID, productID: product.productID)?
@@ -39,6 +39,13 @@ final class ProductVariationsViewModel {
3939
let variationsToGenerate = ProductVariationGenerator.generateVariations(for: product, excluding: existingVariations)
4040
print("Variations to Generate: \(variationsToGenerate.count)")
4141

42+
// Guard for 100 variation limit
43+
guard variationsToGenerate.count <= 100 else {
44+
return onCompletion(.failure(.tooManyVariations(variationCount: variationsToGenerate.count)))
45+
}
46+
47+
onCompletion(.success(()))
48+
4249
}
4350
stores.dispatch(action)
4451

@@ -71,3 +78,29 @@ extension ProductVariationsViewModel {
7178
product.attributesForVariations.isEmpty
7279
}
7380
}
81+
82+
extension ProductVariationsViewModel {
83+
/// Type to represent known generation errors
84+
///
85+
enum GenerationError: LocalizedError, Equatable {
86+
case tooManyVariations(variationCount: Int)
87+
88+
var errorTitle: String {
89+
switch self {
90+
case .tooManyVariations:
91+
return NSLocalizedString("Generation limit exceeded", comment: "Error title for for when there are too many variations to generate.")
92+
}
93+
}
94+
95+
var errorDescription: String? {
96+
switch self {
97+
case .tooManyVariations(let variationCount):
98+
let format = NSLocalizedString(
99+
"Currently creation is supported for 100 variations maximum. Generating variations for this product would create %d variations.",
100+
comment: "Error description for when there are too many variations to generate."
101+
)
102+
return String.localizedStringWithFormat(format, variationCount)
103+
}
104+
}
105+
}
106+
}

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

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,4 +114,38 @@ final class ProductVariationsViewModelTests: XCTestCase {
114114
XCTAssertTrue(product.existsRemotely)
115115
XCTAssertEqual(viewModel.formType, .readonly)
116116
}
117+
118+
func test_trying_to_generate_more_than_100_variations_will_return_error() {
119+
// Given
120+
let product = Product.fake().copy(attributes: [
121+
ProductAttribute.fake().copy(attributeID: 1, name: "Size", options: ["XS", "S", "M", "L", "XL"]),
122+
ProductAttribute.fake().copy(attributeID: 2, name: "Color", options: ["Red", "Green", "Blue", "White", "Black"]),
123+
ProductAttribute.fake().copy(attributeID: 3, name: "Fabric", options: ["Cotton", "Nylon", "Polyester", "Silk", "Linen"]),
124+
])
125+
126+
let stores = MockStoresManager(sessionManager: SessionManager.makeForTesting())
127+
stores.whenReceivingAction(ofType: ProductVariationAction.self) { action in
128+
switch action {
129+
case .synchronizeAllProductVariations(_, _, let onCompletion):
130+
onCompletion(.success(()))
131+
default:
132+
break
133+
}
134+
}
135+
136+
let viewModel = ProductVariationsViewModel(stores: stores, formType: .edit)
137+
138+
// When
139+
let error = waitFor { promise in
140+
viewModel.generateAllVariations(for: product) { result in
141+
if case let .failure(error) = result {
142+
promise(error)
143+
}
144+
}
145+
}
146+
147+
// Then
148+
XCTAssertEqual(error, .tooManyVariations(variationCount: 125))
149+
150+
}
117151
}

0 commit comments

Comments
 (0)