From be3f65014325d3cfb3530acef695d9aec8c05aff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ernesto=20Carri=C3=B3n?= Date: Thu, 10 Nov 2022 17:34:48 -0500 Subject: [PATCH 1/5] Allow new templates to be considered as product with unsaved changes --- .../Products/Edit Product/EditableProductModel.swift | 10 ++++++++++ .../Products/Edit Product/ProductFormViewModel.swift | 11 ++++++++++- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/WooCommerce/Classes/ViewRelated/Products/Edit Product/EditableProductModel.swift b/WooCommerce/Classes/ViewRelated/Products/Edit Product/EditableProductModel.swift index b94d3f13491..c04366879ec 100644 --- a/WooCommerce/Classes/ViewRelated/Products/Edit Product/EditableProductModel.swift +++ b/WooCommerce/Classes/ViewRelated/Products/Edit Product/EditableProductModel.swift @@ -181,6 +181,16 @@ extension EditableProductModel: ProductFormDataModel, TaxClassRequestable { var existsRemotely: Bool { product.existsRemotely } + + /// Helper to determine if a product model is empty. + /// We consider it as empty if its underlying product matches the `ProductFactory.createNewProduct` output. + /// + func isEmpty() -> Bool { + guard let emptyProduct = ProductFactory().createNewProduct(type: productType, isVirtual: virtual, siteID: siteID) else { + return false + } + return emptyProduct == product + } } extension EditableProductModel: Equatable { diff --git a/WooCommerce/Classes/ViewRelated/Products/Edit Product/ProductFormViewModel.swift b/WooCommerce/Classes/ViewRelated/Products/Edit Product/ProductFormViewModel.swift index d8730d16de1..52ffb0daf7c 100644 --- a/WooCommerce/Classes/ViewRelated/Products/Edit Product/ProductFormViewModel.swift +++ b/WooCommerce/Classes/ViewRelated/Products/Edit Product/ProductFormViewModel.swift @@ -221,7 +221,7 @@ final class ProductFormViewModel: ProductFormViewModelProtocol { productOrVariationID: .product(id: product.productID), isLocalID: !product.existsRemotely), originalImages: originalProduct.images) - return hasProductChangesExcludingImages || hasImageChanges || password != originalPassword + return hasProductChangesExcludingImages || hasImageChanges || password != originalPassword || isNewTemplateProduct() } } @@ -620,6 +620,15 @@ private extension ProductFormViewModel { return controller } + + /// Helper to determine if the added/editted product comes as a new template product. + /// We assume that a new template product is a product that: + /// - Doesn't have an `id` - has not been saved remotely + /// - Is not empty. + /// + private func isNewTemplateProduct() -> Bool { + originalProduct.productID == .zero && !originalProduct.isEmpty() + } } // MARK: Beta feature handling From f1b6c83ae67ea8f6d0fd0a4a45ea149e5f7cb2f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ernesto=20Carri=C3=B3n?= Date: Thu, 10 Nov 2022 17:35:11 -0500 Subject: [PATCH 2/5] Refactor shouldEnablePreviewButton method --- .../Edit Product/ProductFormViewModelProtocol.swift | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/WooCommerce/Classes/ViewRelated/Products/Edit Product/ProductFormViewModelProtocol.swift b/WooCommerce/Classes/ViewRelated/Products/Edit Product/ProductFormViewModelProtocol.swift index 8db690e950f..13dc19b0b48 100644 --- a/WooCommerce/Classes/ViewRelated/Products/Edit Product/ProductFormViewModelProtocol.swift +++ b/WooCommerce/Classes/ViewRelated/Products/Edit Product/ProductFormViewModelProtocol.swift @@ -185,6 +185,11 @@ extension ProductFormViewModelProtocol { /// Returns `false` when it's a new blank product without any changes. /// func shouldEnablePreviewButton() -> Bool { - !(formType == .add && !hasUnsavedChanges()) + switch formType { + case .add: + return hasUnsavedChanges() + case .edit, .readonly: + return true + } } } From c6b0475d08550a7a2a5e6837e281caec6ff8295e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ernesto=20Carri=C3=B3n?= Date: Thu, 10 Nov 2022 19:09:24 -0500 Subject: [PATCH 3/5] Make sure dates are not taken into consideration when determining if a product is empty --- .../Products/Edit Product/EditableProductModel.swift | 6 +++++- .../Products/Edit Product/ProductFormViewModelTests.swift | 4 ++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/WooCommerce/Classes/ViewRelated/Products/Edit Product/EditableProductModel.swift b/WooCommerce/Classes/ViewRelated/Products/Edit Product/EditableProductModel.swift index c04366879ec..efa3e69d0be 100644 --- a/WooCommerce/Classes/ViewRelated/Products/Edit Product/EditableProductModel.swift +++ b/WooCommerce/Classes/ViewRelated/Products/Edit Product/EditableProductModel.swift @@ -184,12 +184,16 @@ extension EditableProductModel: ProductFormDataModel, TaxClassRequestable { /// Helper to determine if a product model is empty. /// We consider it as empty if its underlying product matches the `ProductFactory.createNewProduct` output. + /// Additionally we don't take dates into consideration as we don't control their value when creating a product. /// func isEmpty() -> Bool { guard let emptyProduct = ProductFactory().createNewProduct(type: productType, isVirtual: virtual, siteID: siteID) else { return false } - return emptyProduct == product + + let commonDate = Date() + return emptyProduct.copy(date: commonDate, dateCreated: commonDate, dateModified: commonDate, dateOnSaleStart: commonDate, dateOnSaleEnd: commonDate) == + product.copy(date: commonDate, dateCreated: commonDate, dateModified: commonDate, dateOnSaleStart: commonDate, dateOnSaleEnd: commonDate) } } diff --git a/WooCommerce/WooCommerceTests/ViewRelated/Products/Edit Product/ProductFormViewModelTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/Products/Edit Product/ProductFormViewModelTests.swift index c3aa2d2eba0..c5d3c5e5689 100644 --- a/WooCommerce/WooCommerceTests/ViewRelated/Products/Edit Product/ProductFormViewModelTests.swift +++ b/WooCommerce/WooCommerceTests/ViewRelated/Products/Edit Product/ProductFormViewModelTests.swift @@ -542,11 +542,11 @@ final class ProductFormViewModelTests: XCTestCase { // MARK: Preview button tests (with enabled Product Onboarding feature flag) - func test_disabled_preview_button_for_new_blank_product_without_any_changes() { + func test_disabled_preview_button_for_new_blank_product_without_any_changes() throws { // Given sessionManager.defaultSite = Site.fake().copy(frameNonce: "abc123") - let product = Product.fake().copy(statusKey: ProductStatus.published.rawValue) + let product = try XCTUnwrap(ProductFactory().createNewProduct(type: .simple, isVirtual: false, siteID: 123)) let viewModel = createViewModel(product: product, formType: .add, stores: stores, From d883e0f51521691dc369b7634f72b13a834bdcff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ernesto=20Carri=C3=B3n?= Date: Thu, 10 Nov 2022 19:16:35 -0500 Subject: [PATCH 4/5] Adds unit tests --- .../ProductFormViewModelTests.swift | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/WooCommerce/WooCommerceTests/ViewRelated/Products/Edit Product/ProductFormViewModelTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/Products/Edit Product/ProductFormViewModelTests.swift index c5d3c5e5689..e0500734386 100644 --- a/WooCommerce/WooCommerceTests/ViewRelated/Products/Edit Product/ProductFormViewModelTests.swift +++ b/WooCommerce/WooCommerceTests/ViewRelated/Products/Edit Product/ProductFormViewModelTests.swift @@ -560,6 +560,25 @@ final class ProductFormViewModelTests: XCTestCase { XCTAssertFalse(viewModel.shouldEnablePreviewButton()) } + func test_enabled_preview_button_for_new_template_product() throws { + // Given + sessionManager.defaultSite = Site.fake().copy(frameNonce: "abc123") + + // Adding some value to simulate a template product + let product = try XCTUnwrap(ProductFactory().createNewProduct(type: .simple, isVirtual: false, siteID: 123)?.copy(price: "10.00")) + let viewModel = createViewModel(product: product, + formType: .add, + stores: stores, + featureFlagService: MockFeatureFlagService(isProductsOnboardingEnabled: true)) + + // When + let actionButtons = viewModel.actionButtons + + // Then + XCTAssertEqual(actionButtons, [.preview, .publish, .more]) + XCTAssertTrue(viewModel.shouldEnablePreviewButton()) + } + func test_enabled_preview_button_for_new_product_with_pending_changes() { // Given sessionManager.defaultSite = Site.fake().copy(frameNonce: "abc123") From 5b90ccbcf8a488e0445333661689f449baf7b2be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ernesto=20Carri=C3=B3n?= Date: Fri, 11 Nov 2022 08:17:37 -0500 Subject: [PATCH 5/5] Fix unit tests --- Fakes/Fakes/Products/ProductFactory.swift | 3 ++- .../Edit Product/ProductFormViewModel+ObservablesTests.swift | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/Fakes/Fakes/Products/ProductFactory.swift b/Fakes/Fakes/Products/ProductFactory.swift index 64df5917ad9..097a6d74665 100644 --- a/Fakes/Fakes/Products/ProductFactory.swift +++ b/Fakes/Fakes/Products/ProductFactory.swift @@ -24,7 +24,8 @@ public enum ProductFactory { /// Returns a fake product filled with data can be edited by the merchants /// public static func productWithEditableDataFilled() -> Product { - Product.fake().copy(name: "name", + Product.fake().copy(productID: 123, + name: "name", dateOnSaleStart: Date(), dateOnSaleEnd: Date(), fullDescription: "description", diff --git a/WooCommerce/WooCommerceTests/ViewRelated/Products/Edit Product/ProductFormViewModel+ObservablesTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/Products/Edit Product/ProductFormViewModel+ObservablesTests.swift index 3381e605f8b..27f750cc3ea 100644 --- a/WooCommerce/WooCommerceTests/ViewRelated/Products/Edit Product/ProductFormViewModel+ObservablesTests.swift +++ b/WooCommerce/WooCommerceTests/ViewRelated/Products/Edit Product/ProductFormViewModel+ObservablesTests.swift @@ -159,7 +159,7 @@ final class ProductFormViewModel_ObservablesTests: XCTestCase { func testObservablesFromUpdatingProductPasswordRemotely() { // Arrange - let product = Product.fake() + let product = Product.fake().copy(productID: 123) let model = EditableProductModel(product: product) let productImageActionHandler = ProductImageActionHandler(siteID: defaultSiteID, product: model) let viewModel = ProductFormViewModel(product: model,