Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import Yosemite

/// Creates actions for product form bottom sheet.
struct ProductFormBottomSheetActionsFactory {
private let product: Product
private let isEditProductsRelease2Enabled: Bool
private let isEditProductsRelease3Enabled: Bool

init(product: Product, isEditProductsRelease2Enabled: Bool, isEditProductsRelease3Enabled: Bool) {
self.product = product
self.isEditProductsRelease2Enabled = isEditProductsRelease2Enabled
self.isEditProductsRelease3Enabled = isEditProductsRelease3Enabled
}

/// Retruns an array of actions that are visible in the product form bottom sheet.
func actions() -> [ProductFormBottomSheetAction] {
let shouldShowShippingSettingsRow = product.isShippingEnabled
let shouldShowCategoriesRow = isEditProductsRelease3Enabled
let shouldShowShortDescriptionRow = isEditProductsRelease2Enabled
let actions: [ProductFormBottomSheetAction?] = [
.editInventorySettings,
shouldShowShippingSettingsRow ? .editShippingSettings: nil,
shouldShowCategoriesRow ? .editCategories: nil,
shouldShowShortDescriptionRow ? .editBriefDescription: nil
]
return actions.compactMap({ $0 }).filter({ $0.isVisible(product: product) })
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should also encapsulate the isVisible logic inside this Factory. 🤔 Though that is also eventually used within DefaultProductFormTableViewModel.settingsRows. This makes me think that all these visibility logic should be merged too.

I wonder if we've outgrown the current architecture (expected and normal) and we need to do a review at some point in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yea I think it'd be nice if we can merge the logic for making the bottom sheet actions and settings rows since they are based on the same data (product editing categories), maybe having both created from the same factory (a different name like ProductFormActionsFactory). what do you think? I created an issue for tech debt #2317 to track 🙂

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,10 @@ final class ProductFormBottomSheetListSelectorCommand: BottomSheetListSelectorCo

private let onSelection: (ProductFormBottomSheetAction) -> Void

init(product: Product,
isEditProductsRelease3Enabled: Bool,
init(actions: [ProductFormBottomSheetAction],
onSelection: @escaping (ProductFormBottomSheetAction) -> Void) {
self.onSelection = onSelection

let shouldShowShippingSettingsRow = product.isShippingEnabled
let shouldShowCategoriesRow = isEditProductsRelease3Enabled
let actions: [ProductFormBottomSheetAction?] = [
.editInventorySettings,
shouldShowShippingSettingsRow ? .editShippingSettings: nil,
shouldShowCategoriesRow ? .editCategories: nil,
.editBriefDescription
]
self.data = actions.compactMap({ $0 }).filter({ $0.isVisible(product: product) })
self.data = actions
}

func configureCell(cell: ImageAndTitleAndTextTableViewCell, model: ProductFormBottomSheetAction) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -229,11 +229,6 @@ private extension ProductFormViewController {
}

func configureMoreDetailsContainerView() {
guard isEditProductsRelease2Enabled else {
moreDetailsContainerView.isHidden = true
return
}

let title = NSLocalizedString("Add more details", comment: "Title of the button at the bottom of the product form to add more product details.")
let viewModel = BottomButtonContainerView.ViewModel(style: .link,
title: title,
Expand All @@ -258,8 +253,8 @@ private extension ProductFormViewController {
let title = NSLocalizedString("Add more details",
comment: "Title of the bottom sheet from the product form to add more product details.")
let viewProperties = BottomSheetListSelectorViewProperties(title: title)
let dataSource = ProductFormBottomSheetListSelectorCommand(product: product,
isEditProductsRelease3Enabled: isEditProductsRelease3Enabled) { [weak self] action in
let actions = createBottomSheetActions()
let dataSource = ProductFormBottomSheetListSelectorCommand(actions: actions) { [weak self] action in
self?.dismiss(animated: true) { [weak self] in
switch action {
case .editInventorySettings:
Expand All @@ -282,16 +277,8 @@ private extension ProductFormViewController {
}

func updateMoreDetailsButtonVisibility(product: Product) {
guard isEditProductsRelease2Enabled else {
moreDetailsContainerView.isHidden = true
return
}

let moreDetailsActions: [ProductFormBottomSheetAction] = isEditProductsRelease3Enabled ?
[.editInventorySettings, .editShippingSettings, .editCategories, .editBriefDescription]:
[.editInventorySettings, .editShippingSettings, .editBriefDescription]
let hasVisibleActions = moreDetailsActions.map({ $0.isVisible(product: product) }).contains(true)
moreDetailsContainerView.isHidden = hasVisibleActions == false
let moreDetailsActions = createBottomSheetActions()
moreDetailsContainerView.isHidden = moreDetailsActions.isEmpty
}
}

Expand Down Expand Up @@ -477,6 +464,12 @@ private extension ProductFormViewController {
moreButton.accessibilityIdentifier = "edit-product-more-options-button"
return moreButton
}

private func createBottomSheetActions() -> [ProductFormBottomSheetAction] {
ProductFormBottomSheetActionsFactory(product: product,
isEditProductsRelease2Enabled: isEditProductsRelease2Enabled,
isEditProductsRelease3Enabled: isEditProductsRelease3Enabled).actions()
}
}

extension ProductFormViewController: UITableViewDelegate {
Expand Down
8 changes: 8 additions & 0 deletions WooCommerce/WooCommerce.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@
0235595B24496E88004BE2B8 /* BottomSheetListSelectorViewController+DrawerPresentable.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0235595A24496E88004BE2B8 /* BottomSheetListSelectorViewController+DrawerPresentable.swift */; };
02357B2A23CDB3E300147C2B /* ProductImageViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 02357B2823CDB3E300147C2B /* ProductImageViewController.swift */; };
02357B2B23CDB3E300147C2B /* ProductImageViewController.xib in Resources */ = {isa = PBXBuildFile; fileRef = 02357B2923CDB3E300147C2B /* ProductImageViewController.xib */; };
0235BFD9246E959500778909 /* ProductFormBottomSheetActionsFactory.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0235BFD8246E959500778909 /* ProductFormBottomSheetActionsFactory.swift */; };
0235BFDB246E99A700778909 /* ProductFormBottomSheetActionsFactoryTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0235BFDA246E99A700778909 /* ProductFormBottomSheetActionsFactoryTests.swift */; };
02396251239948470096F34C /* UIImage+TintColor.swift in Sources */ = {isa = PBXBuildFile; fileRef = 02396250239948470096F34C /* UIImage+TintColor.swift */; };
023A059A24135F2600E3FC99 /* ReviewsViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 023A059824135F2600E3FC99 /* ReviewsViewController.swift */; };
023A059B24135F2600E3FC99 /* ReviewsViewController.xib in Resources */ = {isa = PBXBuildFile; fileRef = 023A059924135F2600E3FC99 /* ReviewsViewController.xib */; };
Expand Down Expand Up @@ -938,6 +940,8 @@
0235595A24496E88004BE2B8 /* BottomSheetListSelectorViewController+DrawerPresentable.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "BottomSheetListSelectorViewController+DrawerPresentable.swift"; sourceTree = "<group>"; };
02357B2823CDB3E300147C2B /* ProductImageViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ProductImageViewController.swift; sourceTree = "<group>"; };
02357B2923CDB3E300147C2B /* ProductImageViewController.xib */ = {isa = PBXFileReference; lastKnownFileType = file.xib; path = ProductImageViewController.xib; sourceTree = "<group>"; };
0235BFD8246E959500778909 /* ProductFormBottomSheetActionsFactory.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ProductFormBottomSheetActionsFactory.swift; sourceTree = "<group>"; };
0235BFDA246E99A700778909 /* ProductFormBottomSheetActionsFactoryTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ProductFormBottomSheetActionsFactoryTests.swift; sourceTree = "<group>"; };
02396250239948470096F34C /* UIImage+TintColor.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "UIImage+TintColor.swift"; sourceTree = "<group>"; };
023A059824135F2600E3FC99 /* ReviewsViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ReviewsViewController.swift; sourceTree = "<group>"; };
023A059924135F2600E3FC99 /* ReviewsViewController.xib */ = {isa = PBXFileReference; lastKnownFileType = file.xib; path = ReviewsViewController.xib; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1764,6 +1768,7 @@
children = (
0212276024498A270042161F /* ProductFormBottomSheetListSelectorCommand.swift */,
0212276224498CDC0042161F /* ProductFormBottomSheetAction.swift */,
0235BFD8246E959500778909 /* ProductFormBottomSheetActionsFactory.swift */,
);
path = BottomSheetListSelector;
sourceTree = "<group>";
Expand Down Expand Up @@ -2199,6 +2204,7 @@
children = (
02A9A495244D84AB00757B99 /* ProductsSortOrderBottomSheetListSelectorCommandTests.swift */,
02E493EE245C1087000AEA9E /* ProductFormBottomSheetListSelectorCommandTests.swift */,
0235BFDA246E99A700778909 /* ProductFormBottomSheetActionsFactoryTests.swift */,
);
path = BottomSheetListSelector;
sourceTree = "<group>";
Expand Down Expand Up @@ -4727,6 +4733,7 @@
D8C251DB230D288A00F49782 /* PushNotesManager.swift in Sources */,
748D34E12148291E00E21A2F /* TopPerformerDataViewController.swift in Sources */,
02C887712450285100E4470F /* BottomButtonContainerView.swift in Sources */,
0235BFD9246E959500778909 /* ProductFormBottomSheetActionsFactory.swift in Sources */,
024EFA6923FCC10B00F36918 /* Product+Media.swift in Sources */,
5795F23023E26B5300F6707C /* OrderSearchStarterViewModel.swift in Sources */,
0202B68D23876BC100F3EBE0 /* ProductsTabProductViewModel+ProductVariation.swift in Sources */,
Expand Down Expand Up @@ -4950,6 +4957,7 @@
02BA23C022EE9DAF009539E7 /* AsyncDictionaryTests.swift in Sources */,
B555531121B57E6F00449E71 /* MockupApplicationAdapter.swift in Sources */,
021E2A2023AA274700B1DE07 /* ProductBackordersSettingListSelectorCommandTests.swift in Sources */,
0235BFDB246E99A700778909 /* ProductFormBottomSheetActionsFactoryTests.swift in Sources */,
020BE76723B49FE9007FE54C /* AztecBoldFormatBarCommandTests.swift in Sources */,
CE4DA5C821DD759400074607 /* CurrencyFormatterTests.swift in Sources */,
B57C745120F56EE900EEFC87 /* UITableViewCellHelpersTests.swift in Sources */,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,171 @@
import XCTest
@testable import WooCommerce
@testable import Yosemite

final class ProductFormBottomSheetActionsFactoryTests: XCTestCase {

// M3 feature flag off & M2 feature flag off

func testDataHasNoEditProductsRelease2And3ActionsForAPhysicalProductWhenBothFeatureFlagsAreOff() {
let product = Fixtures.physicalProduct
let isEditProductsRelease2Enabled = false
let isEditProductsRelease3Enabled = false
let actions = ProductFormBottomSheetActionsFactory(product: product,
isEditProductsRelease2Enabled: isEditProductsRelease2Enabled,
isEditProductsRelease3Enabled: isEditProductsRelease3Enabled).actions()

let expectedActions: [ProductFormBottomSheetAction] = [
.editInventorySettings,
.editShippingSettings
]
XCTAssertEqual(actions, expectedActions)
}

func testDataHasNoEditProductsRelease2And3AndShippingActionsForAVirtualProductWhenBothFeatureFlagsAreOff() {
let product = Fixtures.virtualProduct
let isEditProductsRelease2Enabled = false
let isEditProductsRelease3Enabled = false
let actions = ProductFormBottomSheetActionsFactory(product: product,
isEditProductsRelease2Enabled: isEditProductsRelease2Enabled,
isEditProductsRelease3Enabled: isEditProductsRelease3Enabled).actions()

let expectedActions: [ProductFormBottomSheetAction] = [
.editInventorySettings
]
XCTAssertEqual(actions, expectedActions)
}

func testDataHasNoEditProductsRelease2And3AndShippingActionsForADownloadableProductWhenBothFeatureFlagsAreOff() {
let product = Fixtures.downloadableProduct
let isEditProductsRelease2Enabled = false
let isEditProductsRelease3Enabled = false
let actions = ProductFormBottomSheetActionsFactory(product: product,
isEditProductsRelease2Enabled: isEditProductsRelease2Enabled,
isEditProductsRelease3Enabled: isEditProductsRelease3Enabled).actions()

let expectedActions: [ProductFormBottomSheetAction] = [
.editInventorySettings
]
XCTAssertEqual(actions, expectedActions)
}

// M3 feature flag off & M2 feature flag on

func testDataHasNoEditProductsRelease3ActionsForAPhysicalProductWhenM3FeatureFlagIsOff() {
let product = Fixtures.physicalProduct
let isEditProductsRelease2Enabled = true
let isEditProductsRelease3Enabled = false
let actions = ProductFormBottomSheetActionsFactory(product: product,
isEditProductsRelease2Enabled: isEditProductsRelease2Enabled,
isEditProductsRelease3Enabled: isEditProductsRelease3Enabled).actions()

let expectedActions: [ProductFormBottomSheetAction] = [
.editInventorySettings,
.editShippingSettings,
.editBriefDescription
]
XCTAssertEqual(actions, expectedActions)
}

func testDataHasNoEditProductsRelease3AndShippingActionsForAVirtualProductWhenM3FeatureFlagIsOff() {
let product = Fixtures.virtualProduct
let isEditProductsRelease2Enabled = true
let isEditProductsRelease3Enabled = false
let actions = ProductFormBottomSheetActionsFactory(product: product,
isEditProductsRelease2Enabled: isEditProductsRelease2Enabled,
isEditProductsRelease3Enabled: isEditProductsRelease3Enabled).actions()

let expectedActions: [ProductFormBottomSheetAction] = [
.editInventorySettings,
.editBriefDescription
]
XCTAssertEqual(actions, expectedActions)
}

func testDataHasNoEditProductsRelease3AndShippingActionsForADownloadableProductWhenM3FeatureFlagIsOff() {
let product = Fixtures.downloadableProduct
let isEditProductsRelease2Enabled = true
let isEditProductsRelease3Enabled = false
let actions = ProductFormBottomSheetActionsFactory(product: product,
isEditProductsRelease2Enabled: isEditProductsRelease2Enabled,
isEditProductsRelease3Enabled: isEditProductsRelease3Enabled).actions()

let expectedActions: [ProductFormBottomSheetAction] = [
.editInventorySettings,
.editBriefDescription
]
XCTAssertEqual(actions, expectedActions)
}

// M3 feature flag on & M2 feature flag is on

func testDataHasEditProductsRelease3ActionsForAPhysicalProductWhenBothFeatureFlagsAreOn() {
let product = Fixtures.physicalProduct
let isEditProductsRelease2Enabled = true
let isEditProductsRelease3Enabled = true
let actions = ProductFormBottomSheetActionsFactory(product: product,
isEditProductsRelease2Enabled: isEditProductsRelease2Enabled,
isEditProductsRelease3Enabled: isEditProductsRelease3Enabled).actions()

let expectedActions: [ProductFormBottomSheetAction] = [
.editInventorySettings,
.editShippingSettings,
.editCategories,
.editBriefDescription
]
XCTAssertEqual(actions, expectedActions)
}

func testDataHasEditProductsRelease3ButNoShippingActionsForAVirtualProductWhenBothFeatureFlagsAreOn() {
let product = Fixtures.virtualProduct
let isEditProductsRelease2Enabled = true
let isEditProductsRelease3Enabled = true
let actions = ProductFormBottomSheetActionsFactory(product: product,
isEditProductsRelease2Enabled: isEditProductsRelease2Enabled,
isEditProductsRelease3Enabled: isEditProductsRelease3Enabled).actions()

let expectedActions: [ProductFormBottomSheetAction] = [
.editInventorySettings,
.editCategories,
.editBriefDescription
]
XCTAssertEqual(actions, expectedActions)
}

func testDataHasEditProductsRelease3ButNoShippingActionsForADownloadableProductWhenBothFeatureFlagsAreOn() {
let product = Fixtures.downloadableProduct
let isEditProductsRelease2Enabled = true
let isEditProductsRelease3Enabled = true
let actions = ProductFormBottomSheetActionsFactory(product: product,
isEditProductsRelease2Enabled: isEditProductsRelease2Enabled,
isEditProductsRelease3Enabled: isEditProductsRelease3Enabled).actions()

let expectedActions: [ProductFormBottomSheetAction] = [
.editInventorySettings,
.editCategories,
.editBriefDescription
]
XCTAssertEqual(actions, expectedActions)
}

}

private extension ProductFormBottomSheetActionsFactoryTests {
enum Fixtures {
// downloadable: false, virtual: false, missing inventory/shipping/categories/brief description
static let physicalProduct = MockProduct().product(downloadable: false, briefDescription: "", manageStock: true, sku: nil, stockQuantity: nil,
dimensions: ProductDimensions(length: "", width: "", height: ""), weight: nil,
virtual: false,
categories: [])
// downloadable: false, virtual: true, missing inventory/shipping/categories/brief description
static let virtualProduct = MockProduct().product(downloadable: false, briefDescription: "", manageStock: true, sku: nil, stockQuantity: nil,
dimensions: ProductDimensions(length: "", width: "", height: ""), weight: nil,
virtual: true,
categories: [])
// downloadable: true, virtual: true, missing inventory/shipping/categories/brief description
static let downloadableProduct = MockProduct().product(downloadable: true, briefDescription: "", manageStock: true, sku: nil, stockQuantity: nil,
dimensions: ProductDimensions(length: "", width: "", height: ""), weight: nil,
virtual: true,
categories: [])
}
}
Loading