Merge pull request #4350 from woocommerce/issue/4312-show-attributes-row

Ecarrion · web-flow · commit 8754f6d89d4a · 2021-06-04T17:11:50.000-05:00
diff --git a/Networking/Networking/Model/Copiable/Models+Copiable.generated.swift b/Networking/Networking/Model/Copiable/Models+Copiable.generated.swift
@@ -391,6 +391,36 @@ extension ProductAddOnOption {
     }
 }
 
+extension ProductAttribute {
+    public func copy(
+        siteID: CopiableProp<Int64> = .copy,
+        attributeID: CopiableProp<Int64> = .copy,
+        name: CopiableProp<String> = .copy,
+        position: CopiableProp<Int> = .copy,
+        visible: CopiableProp<Bool> = .copy,
+        variation: CopiableProp<Bool> = .copy,
+        options: CopiableProp<[String]> = .copy
+    ) -> ProductAttribute {
+        let siteID = siteID ?? self.siteID
+        let attributeID = attributeID ?? self.attributeID
+        let name = name ?? self.name
+        let position = position ?? self.position
+        let visible = visible ?? self.visible
+        let variation = variation ?? self.variation
+        let options = options ?? self.options
+
+        return ProductAttribute(
+            siteID: siteID,
+            attributeID: attributeID,
+            name: name,
+            position: position,
+            visible: visible,
+            variation: variation,
+            options: options
+        )
+    }
+}
+
 extension ProductImage {
     public func copy(
         imageID: CopiableProp<Int64> = .copy,
diff --git a/Networking/Networking/Model/Product/ProductAttribute.swift b/Networking/Networking/Model/Product/ProductAttribute.swift
@@ -3,7 +3,7 @@ import Codegen
 
 /// Represents a ProductAttribute entity.
 ///
-public struct ProductAttribute: Codable, GeneratedFakeable {
+public struct ProductAttribute: Codable, GeneratedFakeable, GeneratedCopiable {
     public let siteID: Int64
     public let attributeID: Int64
     public let name: String
diff --git a/WooCommerce/Classes/ViewRelated/Products/Edit Product/DefaultProductFormTableViewModel.swift b/WooCommerce/Classes/ViewRelated/Products/Edit Product/DefaultProductFormTableViewModel.swift
@@ -92,6 +92,8 @@ private extension DefaultProductFormTableViewModel {
                 return .linkedProducts(viewModel: linkedProductsRow(product: product, isEditable: editable), isEditable: editable)
             case .noPriceWarning:
                 return .noPriceWarning(viewModel: noPriceWarningRow())
+            case .attributes(let editable):
+                return .attributes(viewModel: productVariationsAttributesRow(product: product.product, isEditable: editable), isEditable: editable)
             default:
                 assertionFailure("Unexpected action in the settings section: \(action)")
                 return nil
@@ -382,23 +384,8 @@ private extension DefaultProductFormTableViewModel {
     func variationsRow(product: Product) -> ProductFormSection.SettingsRow.ViewModel {
         let icon = UIImage.variationsImage
         let title = product.variations.isEmpty ? Localization.addVariationsTitle : Localization.variationsTitle
-
-        let details: String
-        let format = NSLocalizedString("%1$@ (%2$ld options)", comment: "Format for each Product attribute")
-
-        switch product.variations.count {
-        case 1...:
-            details = product.attributesForVariations
-                .map({ String.localizedStringWithFormat(format, $0.name, $0.options.count) })
-                .joined(separator: "\n")
-        default:
-            details = ""
-        }
-
-        return ProductFormSection.SettingsRow.ViewModel(icon: icon,
-                                                        title: title,
-                                                        details: details,
-                                                        isActionable: true)
+        let details = Localization.variationsDetail(count: product.variations.count)
+        return ProductFormSection.SettingsRow.ViewModel(icon: icon, title: title, details: details, isActionable: true)
     }
 
     // MARK: Product variation only
@@ -420,6 +407,20 @@ private extension DefaultProductFormTableViewModel {
         return ProductFormSection.SettingsRow.WarningViewModel(icon: icon, title: title)
     }
 
+    func productVariationsAttributesRow(product: Product, isEditable: Bool) -> ProductFormSection.SettingsRow.ViewModel {
+        let icon = UIImage.customizeImage
+        let title = Localization.productVariationAttributesTitle
+
+        let details = product.attributesForVariations
+            .map {
+                let format = Localization.variationAttributesDetailFormat(optionCount: $0.options.count)
+                return String.localizedStringWithFormat(format, $0.name, $0.options.count)
+            }
+            .joined(separator: "\n")
+
+        return ProductFormSection.SettingsRow.ViewModel(icon: icon, title: title, details: details, isActionable: isEditable)
+    }
+
     func variationAttributesRow(productVariation: EditableProductVariationModel, isEditable: Bool) -> ProductFormSection.SettingsRow.ViewModel {
         let icon = UIImage.customizeImage
         let title = Localization.variationAttributesTitle
@@ -569,14 +570,46 @@ private extension DefaultProductFormTableViewModel {
             NSLocalizedString("Variations",
                               comment: "Title of the Product Variations row on Product main screen for a variable product")
 
+        static func variationsDetail(count: Int) -> String {
+            let format: String = {
+                switch count {
+                case 0:
+                    return ""
+                case 1:
+                    return NSLocalizedString("%1$ld variation",
+                                             comment: "Format for the variations detail row in singular form. Reads, `1 variation`")
+                default:
+                    return NSLocalizedString("%1$ld variations",
+                                             comment: "Format for the variations detail row in plural form. Reads, `2 variations`")
+                }
+            }()
+
+            return String.localizedStringWithFormat(format, count)
+        }
+
         // Variation status
         static let variationStatusTitle =
             NSLocalizedString("Enabled",
                               comment: "Title of the status row on Product Variation main screen to enable/disable a variation")
 
+        // Product Variations Attributes
+        static let productVariationAttributesTitle = NSLocalizedString("Variations Attributes",
+                                                                       comment: "Title of the variations attributes row on Product screen")
+
         // Variation attributes
         static let variationAttributesTitle = NSLocalizedString("Attributes", comment: "Title of the attributes row on Product Variation main screen")
 
+        static func variationAttributesDetailFormat(optionCount: Int) -> String {
+            switch optionCount {
+            case 0:
+                return ""
+            case 1:
+                return NSLocalizedString("%1$@ (%2$ld option)", comment: "Format for each Product attribute in singular form")
+            default:
+                return NSLocalizedString("%1$@ (%2$ld options)", comment: "Format for each Product attribute in plural form")
+            }
+        }
+
         // No price warning row
         static let noPriceWarningTitle =
             NSLocalizedString("Variations without price won’t be shown in your store",
diff --git a/WooCommerce/Classes/ViewRelated/Products/Edit Product/ProductFormActionsFactory.swift b/WooCommerce/Classes/ViewRelated/Products/Edit Product/ProductFormActionsFactory.swift
@@ -168,6 +168,7 @@ private extension ProductFormActionsFactory {
         let shouldShowReviewsRow = product.reviewsAllowed
         let canEditProductType = formType != .add && editable
         let canEditInventorySettingsRow = editable && product.hasIntegerStockQuantity
+        let shouldShowAttributesRow = product.product.attributesForVariations.isNotEmpty
         let shouldShowNoPriceWarningRow: Bool = {
             let variationsHaveNoPriceSet = variationsPrice == .notSet
             let productHasNoPriceSet = variationsPrice == .unknown && product.product.variations.isNotEmpty && product.product.price.isEmpty
@@ -177,6 +178,7 @@ private extension ProductFormActionsFactory {
         let actions: [ProductFormEditAction?] = [
             .variations,
             shouldShowNoPriceWarningRow ? .noPriceWarning : nil,
+            shouldShowAttributesRow ? .attributes(editable: editable) : nil,
             shouldShowReviewsRow ? .reviews: nil,
             .shippingSettings(editable: editable),
             .inventorySettings(editable: canEditInventorySettingsRow),
@@ -265,6 +267,9 @@ private extension ProductFormActionsFactory {
         case .noPriceWarning:
             // Always visible when available
             return true
+        case .attributes:
+            // Always visible when available
+            return true
         default:
             return false
         }
diff --git a/WooCommerce/Classes/ViewRelated/Products/Edit Product/ProductFormViewController.swift b/WooCommerce/Classes/ViewRelated/Products/Edit Product/ProductFormViewController.swift
@@ -370,8 +370,7 @@ final class ProductFormViewController<ViewModel: ProductFormViewModelProtocol>:
                 guard isEditable else {
                     return
                 }
-                editVariationAttributes()
-                trackEditVariationAttributesRowTapped()
+                editAttributes()
             case .status, .noPriceWarning:
                 break
             }
@@ -781,6 +780,10 @@ private extension ProductFormViewController {
         ServiceLocator.analytics.track(event: WooAnalyticsEvent.Variations.editVariationAttributeOptionsRowTapped(productID: variation.productID,
                                                                                                                   variationID: variation.productVariationID))
     }
+
+    func trackEditProductAttributeRowTapped() {
+        ServiceLocator.analytics.track(event: WooAnalyticsEvent.Variations.editAttributesButtonTapped(productID: product.productID))
+    }
 }
 
 // MARK: Navigation Bar Items
@@ -1323,6 +1326,35 @@ private extension ProductFormViewController {
 // MARK: Action - Edit Product Variation Attributes
 //
 private extension ProductFormViewController {
+    /// Edit the product attributes or the variation attributes depending on the product model type.
+    ///
+    func editAttributes() {
+        switch product {
+        case is EditableProductModel:
+            editProductAttributes()
+            trackEditProductAttributeRowTapped()
+        case is EditableProductVariationModel:
+            editVariationAttributes()
+            trackEditVariationAttributesRowTapped()
+        default:
+            break
+        }
+    }
+
+    /// Navigate to edit product attributes
+    ///
+    func editProductAttributes() {
+        guard let productModel = product as? EditableProductModel else {
+            return
+        }
+        let attributesViewModel = EditAttributesViewModel(product: productModel.product, allowVariationCreation: false)
+        let attributesViewController = EditAttributesViewController(viewModel: attributesViewModel)
+        attributesViewController.onAttributesUpdate = { [weak self] updatedProduct in
+            self?.onAttributeUpdated(attributesViewController: attributesViewController, updatedProduct: updatedProduct)
+        }
+        show(attributesViewController, sender: self)
+    }
+
     func editVariationAttributes() {
         guard let productVariationModel = product as? EditableProductVariationModel else {
             return
@@ -1349,6 +1381,13 @@ private extension ProductFormViewController {
         }
         viewModel.updateVariationAttributes(attributes)
     }
+
+    /// Perform necessary actions when an attribute is created or updated.
+    ///
+    func onAttributeUpdated(attributesViewController: UIViewController, updatedProduct: Product) {
+        viewModel.updateProductVariations(from: updatedProduct)
+        navigationController?.popToViewController(attributesViewController, animated: true)
+    }
 }
 
 // MARK: Constants
diff --git a/WooCommerce/Classes/ViewRelated/Products/Variations/ProductVariationsViewController.swift b/WooCommerce/Classes/ViewRelated/Products/Variations/ProductVariationsViewController.swift
@@ -82,7 +82,6 @@ final class ProductVariationsViewController: UIViewController {
         didSet {
             viewModel.updatedFormTypeIfNeeded(newProduct: product)
 
-            configureRightButtonItem()
             resetResultsController(oldProduct: oldValue)
             updateEmptyState()
             onProductUpdate?(product)
@@ -166,22 +165,6 @@ private extension ProductVariationsViewController {
             "Variations",
             comment: "Title that appears on top of the Product Variation List screen."
         )
-        configureRightButtonItem()
-    }
-
-    /// Configure right button item.
-    ///
-    func configureRightButtonItem() {
-        guard viewModel.shouldShowMoreButton(for: product) else {
-            return navigationItem.rightBarButtonItem = nil
-        }
-
-        let moreButton = UIBarButtonItem(image: .moreImage,
-                                         style: .plain,
-                                         target: self,
-                                         action: #selector(presentMoreOptionsActionSheet(_:)))
-        moreButton.accessibilityLabel = Localization.moreButtonLabel
-        navigationItem.setRightBarButton(moreButton, animated: false)
     }
 
     /// Apply Woo styles.
@@ -283,13 +266,8 @@ private extension ProductVariationsViewController {
 
         addTopButton(title: Localization.generateVariationAction,
                      insets: .init(top: 16, left: 16, bottom: 8, right: 16),
-                     actionSelector: #selector(addButtonTapped),
-                     stylingHandler: { $0.applyPrimaryButtonStyle() })
-
-        addTopButton(title: Localization.editAttributesAction,
-                     insets: .init(top: 8, left: 16, bottom: 16, right: 16),
                      hasBottomBorder: true,
-                     actionSelector: #selector(editAttributesTapped),
+                     actionSelector: #selector(addButtonTapped),
                      stylingHandler: { $0.applySecondaryButtonStyle() })
 
         topStackView.addArrangedSubview(topBannerView)
@@ -563,37 +541,6 @@ private extension ProductVariationsViewController {
         analytics.track(event: WooAnalyticsEvent.Variations.addMoreVariationsButtonTapped(productID: product.productID))
         createVariation()
     }
-
-    @objc func editAttributesTapped() {
-        navigateToEditAttributeViewController(allowVariationCreation: false)
-        trackEditAttributesButtonPressed()
-    }
-
-    func trackEditAttributesButtonPressed() {
-        analytics.track(event: WooAnalyticsEvent.Variations.editAttributesButtonTapped(productID: product.productID))
-    }
-}
-
-// MARK: - Action sheet
-//
-private extension ProductVariationsViewController {
-    @objc private func presentMoreOptionsActionSheet(_ sender: UIBarButtonItem) {
-        let actionSheet = UIAlertController(title: nil, message: nil, preferredStyle: .actionSheet)
-        actionSheet.view.tintColor = .text
-
-        let editAttributesAction = UIAlertAction(title: Localization.editAttributesAction, style: .default) { [weak self] _ in
-            self?.editAttributesTapped()
-        }
-        actionSheet.addAction(editAttributesAction)
-
-        let cancelAction = UIAlertAction(title: Localization.cancelAction, style: .cancel)
-        actionSheet.addAction(cancelAction)
-
-        let popoverController = actionSheet.popoverPresentationController
-        popoverController?.barButtonItem = sender
-
-        present(actionSheet, animated: true)
-    }
 }
 
 // MARK: - Placeholders
diff --git a/WooCommerce/Classes/ViewRelated/Products/Variations/ProductVariationsViewModel.swift b/WooCommerce/Classes/ViewRelated/Products/Variations/ProductVariationsViewModel.swift
@@ -43,12 +43,6 @@ extension ProductVariationsViewModel {
         product.variations.isEmpty || product.attributesForVariations.isEmpty
     }
 
-    /// Defines if the More Options button should be shown
-    ///
-    func shouldShowMoreButton(for product: Product) -> Bool {
-        product.variations.isEmpty && product.attributesForVariations.isNotEmpty
-    }
-
     /// Defines if empty state screen should show guide for creating attributes
     ///
     func shouldShowAttributeGuide(for product: Product) -> Bool {
diff --git a/WooCommerce/WooCommerceTests/ViewRelated/Products/Actions Factory/ProductFormActionsFactoryTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/Products/Actions Factory/ProductFormActionsFactoryTests.swift
@@ -452,6 +452,21 @@ final class ProductFormActionsFactoryTests: XCTestCase {
         let containsWarningAction = factory.settingsSectionActions().contains(ProductFormEditAction.noPriceWarning)
         XCTAssertFalse(containsWarningAction)
     }
+
+    func test_actions_for_variable_product_with_attributes_contains_attributes_action() {
+        // Given
+        let product = Fixtures.variableProductWithVariations.copy(attributes: [
+            ProductAttribute.fake().copy(variation: true)
+        ])
+        let model = EditableProductModel(product: product)
+
+        // When
+        let factory = ProductFormActionsFactory(product: model, formType: .edit, variationsPrice: .unknown)
+
+        // Then
+        let containsAttributeAction = factory.settingsSectionActions().contains(ProductFormEditAction.attributes(editable: true))
+        XCTAssertTrue(containsAttributeAction)
+    }
 }
 
 private extension ProductFormActionsFactoryTests {
diff --git a/WooCommerce/WooCommerceTests/ViewRelated/Products/Variations/ProductVariationsViewModelTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/Products/Variations/ProductVariationsViewModelTests.swift
