From fc0fa076821fcea6af1a428827dc3328699ac3c1 Mon Sep 17 00:00:00 2001 From: Adam Borbas Date: Wed, 15 Oct 2025 14:37:52 +0200 Subject: [PATCH 01/15] Marked code need to change presenting product detail --- .../ViewModels/Order Details/OrderDetailsViewModel.swift | 1 + .../Refund Details/RefundDetailsViewModel.swift | 1 + .../Dashboard/ProductStock/ProductStockDashboardCard.swift | 1 + .../TopPerformers/TopPerformerDataViewController.swift | 1 + .../TopPerformers/TopPerformersDashboardView.swift | 1 + WooCommerce/Classes/ViewRelated/MainTabBarController.swift | 1 + .../AggregatedProductListViewController.swift | 1 + .../Products/Product Loader/ProductLoaderView.swift | 1 + .../Product Loader/ProductLoaderViewController.swift | 1 + .../ViewRelated/Products/ProductsSplitViewCoordinator.swift | 1 + .../ViewRelated/Products/ProductsViewController.swift | 2 ++ .../ViewRelated/Products/ProductDetailsFactoryTests.swift | 6 ++++++ 12 files changed, 18 insertions(+) diff --git a/WooCommerce/Classes/ViewModels/Order Details/OrderDetailsViewModel.swift b/WooCommerce/Classes/ViewModels/Order Details/OrderDetailsViewModel.swift index 401717b425f..b4030f5c4db 100644 --- a/WooCommerce/Classes/ViewModels/Order Details/OrderDetailsViewModel.swift +++ b/WooCommerce/Classes/ViewModels/Order Details/OrderDetailsViewModel.swift @@ -510,6 +510,7 @@ extension OrderDetailsViewModel { viewController.present(navController, animated: true, completion: nil) case .aggregateOrderItem: let item = dataSource.aggregateOrderItems[indexPath.row] + // TODO: Replace with ProductDetailRouter let loaderViewController = ProductLoaderViewController(model: .init(aggregateOrderItem: item), siteID: order.siteID, forceReadOnly: false) diff --git a/WooCommerce/Classes/ViewModels/Order Details/Refund Details/RefundDetailsViewModel.swift b/WooCommerce/Classes/ViewModels/Order Details/Refund Details/RefundDetailsViewModel.swift index 3af7ee687eb..5880355ebe5 100644 --- a/WooCommerce/Classes/ViewModels/Order Details/Refund Details/RefundDetailsViewModel.swift +++ b/WooCommerce/Classes/ViewModels/Order Details/Refund Details/RefundDetailsViewModel.swift @@ -128,6 +128,7 @@ extension RefundDetailsViewModel { case .orderItem: let item = refund.items[indexPath.row] + // TODO: Replace with ProductDetailRouter, native let loaderViewController = ProductLoaderViewController(model: .init(orderItemRefund: item), siteID: refund.siteID, forceReadOnly: true) diff --git a/WooCommerce/Classes/ViewRelated/Dashboard/ProductStock/ProductStockDashboardCard.swift b/WooCommerce/Classes/ViewRelated/Dashboard/ProductStock/ProductStockDashboardCard.swift index 18f90d21809..12017a96201 100644 --- a/WooCommerce/Classes/ViewRelated/Dashboard/ProductStock/ProductStockDashboardCard.swift +++ b/WooCommerce/Classes/ViewRelated/Dashboard/ProductStock/ProductStockDashboardCard.swift @@ -189,6 +189,7 @@ private extension ProductStockDashboardCard { .product(productID: item.productID) } }() + // TODO: Replace with ProductDetailRouter let loaderViewController = ProductLoaderViewController(model: model, siteID: viewModel.siteID, forceReadOnly: false) diff --git a/WooCommerce/Classes/ViewRelated/Dashboard/TopPerformers/TopPerformerDataViewController.swift b/WooCommerce/Classes/ViewRelated/Dashboard/TopPerformers/TopPerformerDataViewController.swift index 0b71b6ee5e5..71988e16989 100644 --- a/WooCommerce/Classes/ViewRelated/Dashboard/TopPerformers/TopPerformerDataViewController.swift +++ b/WooCommerce/Classes/ViewRelated/Dashboard/TopPerformers/TopPerformerDataViewController.swift @@ -176,6 +176,7 @@ private extension TopPerformerDataViewController { /// Presents the product details for a given TopEarnerStatsItem. /// func presentProductDetails(statsItem: TopEarnerStatsItem) { + // TODO: Replace with ProductDetailRouter let loaderViewController = ProductLoaderViewController(model: .init(topEarnerStatsItem: statsItem), siteID: siteID, forceReadOnly: false) diff --git a/WooCommerce/Classes/ViewRelated/Dashboard/TopPerformers/TopPerformersDashboardView.swift b/WooCommerce/Classes/ViewRelated/Dashboard/TopPerformers/TopPerformersDashboardView.swift index 3c2c2e77b89..2a25c26c023 100644 --- a/WooCommerce/Classes/ViewRelated/Dashboard/TopPerformers/TopPerformersDashboardView.swift +++ b/WooCommerce/Classes/ViewRelated/Dashboard/TopPerformers/TopPerformersDashboardView.swift @@ -181,6 +181,7 @@ private extension TopPerformersDashboardView { } func productDetailView(for item: TopEarnerStatsItem) -> UIViewController { + // TODO: Replace with ProductDetailRouter let loaderViewController = ProductLoaderViewController(model: .init(topEarnerStatsItem: item), siteID: viewModel.siteID, forceReadOnly: false) diff --git a/WooCommerce/Classes/ViewRelated/MainTabBarController.swift b/WooCommerce/Classes/ViewRelated/MainTabBarController.swift index f0ffdaabbca..d79dca97815 100644 --- a/WooCommerce/Classes/ViewRelated/MainTabBarController.swift +++ b/WooCommerce/Classes/ViewRelated/MainTabBarController.swift @@ -1036,6 +1036,7 @@ private extension MainTabBarController { return .productVariation(productID: productID, variationID: variationID) } }() + // TODO: Replace with ProductDetailRouter let productViewController = ProductLoaderViewController(model: model, siteID: error.siteID, forceReadOnly: false) diff --git a/WooCommerce/Classes/ViewRelated/Orders/Order Details/Product List Section/Product Details/AggregatedProductListViewController.swift b/WooCommerce/Classes/ViewRelated/Orders/Order Details/Product List Section/Product Details/AggregatedProductListViewController.swift index f1f861186b0..2c574104878 100644 --- a/WooCommerce/Classes/ViewRelated/Orders/Order Details/Product List Section/Product Details/AggregatedProductListViewController.swift +++ b/WooCommerce/Classes/ViewRelated/Orders/Order Details/Product List Section/Product Details/AggregatedProductListViewController.swift @@ -122,6 +122,7 @@ private extension AggregatedProductListViewController { /// Displays the product details screen for the provided AggregateOrderItem /// func productWasPressed(orderItem: AggregateOrderItem) { + // TODO: Replace with ProductDetailRouter, native let loaderViewController = ProductLoaderViewController(model: .init(aggregateOrderItem: orderItem), siteID: viewModel.order.siteID, forceReadOnly: true) diff --git a/WooCommerce/Classes/ViewRelated/Products/Product Loader/ProductLoaderView.swift b/WooCommerce/Classes/ViewRelated/Products/Product Loader/ProductLoaderView.swift index 3f177527318..d22a4369500 100644 --- a/WooCommerce/Classes/ViewRelated/Products/Product Loader/ProductLoaderView.swift +++ b/WooCommerce/Classes/ViewRelated/Products/Product Loader/ProductLoaderView.swift @@ -9,6 +9,7 @@ struct ProductLoaderView: UIViewControllerRepresentable { let forceReadOnly: Bool func makeUIViewController(context: Context) -> UINavigationController { + // TODO: Replace with ProductDetailRouter let viewController = ProductLoaderViewController(model: model, siteID: siteID, forceReadOnly: forceReadOnly) diff --git a/WooCommerce/Classes/ViewRelated/Products/Product Loader/ProductLoaderViewController.swift b/WooCommerce/Classes/ViewRelated/Products/Product Loader/ProductLoaderViewController.swift index b8e43e60ff9..2c1338d6c7d 100644 --- a/WooCommerce/Classes/ViewRelated/Products/Product Loader/ProductLoaderViewController.swift +++ b/WooCommerce/Classes/ViewRelated/Products/Product Loader/ProductLoaderViewController.swift @@ -227,6 +227,7 @@ private extension ProductLoaderViewController { /// Presents the ProductFormViewController, as a childViewController, for a given Product. /// func presentProductDetails(for product: Product) { + // TODO: Replace with ProductDetailRouter ProductDetailsFactory.productDetails(product: product, presentationStyle: .contained(containerViewController: { [weak self] in self }), forceReadOnly: forceReadOnly) { [weak self] viewController in diff --git a/WooCommerce/Classes/ViewRelated/Products/ProductsSplitViewCoordinator.swift b/WooCommerce/Classes/ViewRelated/Products/ProductsSplitViewCoordinator.swift index a8373b4f59b..78a7c96500d 100644 --- a/WooCommerce/Classes/ViewRelated/Products/ProductsSplitViewCoordinator.swift +++ b/WooCommerce/Classes/ViewRelated/Products/ProductsSplitViewCoordinator.swift @@ -122,6 +122,7 @@ private extension ProductsSplitViewCoordinator { } func showProductForm(product: Product) { + // TODO: Replace with ProductDetailRouter ProductDetailsFactory.productDetails(product: product, presentationStyle: .navigationStack, forceReadOnly: false, diff --git a/WooCommerce/Classes/ViewRelated/Products/ProductsViewController.swift b/WooCommerce/Classes/ViewRelated/Products/ProductsViewController.swift index d27b02315bb..915c9a407fe 100644 --- a/WooCommerce/Classes/ViewRelated/Products/ProductsViewController.swift +++ b/WooCommerce/Classes/ViewRelated/Products/ProductsViewController.swift @@ -1214,6 +1214,7 @@ extension ProductsViewController: UITableViewDelegate { private extension ProductsViewController { func didSelectProduct(product: Product) { guard isSplitViewEnabled else { + // TODO: Replace with ProductDetailRouter ProductDetailsFactory.productDetails(product: product, presentationStyle: .navigationStack, forceReadOnly: false) { [weak self] viewController in @@ -1221,6 +1222,7 @@ private extension ProductsViewController { } return } + // TODO: Replace with ProductDetailRouter navigateToContent(.productForm(product: product)) } } diff --git a/WooCommerce/WooCommerceTests/ViewRelated/Products/ProductDetailsFactoryTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/Products/ProductDetailsFactoryTests.swift index c035dd480b4..38d9ec511c4 100644 --- a/WooCommerce/WooCommerceTests/ViewRelated/Products/ProductDetailsFactoryTests.swift +++ b/WooCommerce/WooCommerceTests/ViewRelated/Products/ProductDetailsFactoryTests.swift @@ -12,6 +12,7 @@ final class ProductDetailsFactoryTests: XCTestCase { let exp = expectation(description: #function) // Action + // TODO: Replace with ProductDetailRouter ProductDetailsFactory.productDetails(product: product, presentationStyle: .navigationStack, forceReadOnly: false) { viewController in @@ -30,6 +31,7 @@ final class ProductDetailsFactoryTests: XCTestCase { let exp = expectation(description: #function) // Action + // TODO: Replace with ProductDetailRouter ProductDetailsFactory.productDetails(product: product, presentationStyle: .navigationStack, forceReadOnly: false) { viewController in @@ -48,6 +50,7 @@ final class ProductDetailsFactoryTests: XCTestCase { let exp = expectation(description: #function) // Action + // TODO: Replace with ProductDetailRouter ProductDetailsFactory.productDetails(product: product, presentationStyle: .navigationStack, forceReadOnly: false) { viewController in @@ -66,6 +69,7 @@ final class ProductDetailsFactoryTests: XCTestCase { let exp = expectation(description: #function) // Action + // TODO: Replace with ProductDetailRouter ProductDetailsFactory.productDetails(product: product, presentationStyle: .navigationStack, forceReadOnly: false) { viewController in @@ -84,6 +88,7 @@ final class ProductDetailsFactoryTests: XCTestCase { // Action waitForExpectation { expectation in + // TODO: Replace with ProductDetailRouter ProductDetailsFactory.productDetails(product: product, presentationStyle: .navigationStack, forceReadOnly: false) { viewController in @@ -100,6 +105,7 @@ final class ProductDetailsFactoryTests: XCTestCase { // Action waitForExpectation { expectation in + // TODO: Replace with ProductDetailRouter ProductDetailsFactory.productDetails(product: product, presentationStyle: .navigationStack, forceReadOnly: true) { viewController in From f3c83fd74afb44482a642ed55cd8561955371952 Mon Sep 17 00:00:00 2001 From: Adam Borbas Date: Thu, 16 Oct 2025 10:29:12 +0200 Subject: [PATCH 02/15] POC put to use --- .../Classes/Routing/ProductDetailRouter.swift | 136 ++++++++++++++++++ .../Order Details/OrderDetailsViewModel.swift | 1 - .../ProductLoaderViewController.swift | 10 +- .../Products/ProductDetailsFactory.swift | 15 ++ .../ProductsSplitViewCoordinator.swift | 16 +-- .../Products/ProductsViewController.swift | 1 - .../WooCommerce.xcodeproj/project.pbxproj | 4 + 7 files changed, 167 insertions(+), 16 deletions(-) create mode 100644 WooCommerce/Classes/Routing/ProductDetailRouter.swift diff --git a/WooCommerce/Classes/Routing/ProductDetailRouter.swift b/WooCommerce/Classes/Routing/ProductDetailRouter.swift new file mode 100644 index 00000000000..e372614e9f1 --- /dev/null +++ b/WooCommerce/Classes/Routing/ProductDetailRouter.swift @@ -0,0 +1,136 @@ +import UIKit +import Yosemite + +final class ProductDetailRouter { + enum PresentationStyle { + case undefined + case push + case modal + case contained(containerViewController: () -> UIViewController?) + + var asProductFormPresentationStyle: ProductFormPresentationStyle { + switch self { + case .push: + .navigationStack + case .modal: + .navigationStack + case .contained(let containerViewController): + .contained(containerViewController: containerViewController) + case .undefined: + .navigationStack + } + } + } + + static var shared = ProductDetailRouter(productURLProvider: ProductURLProvider()) + + private let productURLProvider: ProductURLProvider + + init(productURLProvider: ProductURLProvider) { + self.productURLProvider = productURLProvider + } + + func open(product: Product, + presenter: UIViewController, + presentationStyle: PresentationStyle, + onDismiss: (() -> Void)? = nil) { + if product.productType == .booking { + let coordinator = WebViewProductDetailCoordinator(product: product, + productURLProvider: productURLProvider) + coordinator.start(presenter: presenter, onDismiss: onDismiss) + } else { + let coordinator = NativeProductDetailCoordinator(product: product) + coordinator.start(presenter: presenter, + presentationStyle: presentationStyle, + onDismiss: onDismiss) + } + } + + func viewController(product: Product, + presentationStyle: PresentationStyle = .undefined, + forceReadOnly: Bool, + onDeleteCompletion: (() -> Void)? = nil) -> UIViewController { + if product.productType == .booking { + let coordinator = WebViewProductDetailCoordinator(product: product, + productURLProvider: productURLProvider) + return coordinator.viewController() + + } else { + let coordinator = NativeProductDetailCoordinator(product: product) + return coordinator.viewController(presentationStyle: presentationStyle, + forceReadOnly: forceReadOnly) + } + } +} + +final class ProductURLProvider { + func adminURL(for product: Product) -> URL? { + return URL(string: "https://wordpress.com") + } +} + +final class WebViewProductDetailCoordinator: NSObject { + private let product: Product + private let productURLProvider: ProductURLProvider + + private var onDismiss: (() -> Void)? + + init(product: Product, productURLProvider: ProductURLProvider) { + self.product = product + self.productURLProvider = productURLProvider + } + + func viewController(onDeleteCompletion: (() -> Void)? = nil) -> UIViewController { + guard let url = productURLProvider.adminURL(for: product) else { + return UIViewController() // TODO: What do we do in this case? + } + + let viewModel = WPAdminWebViewModel(title: product.name, initialURL: url) + let webViewController = AuthenticatedWebViewController(viewModel: viewModel) + + return webViewController + } + + func start(presenter: UIViewController, + onDismiss: (() -> Void)? = nil) { + let webViewController = viewController() + webViewController.navigationItem.leftBarButtonItem = UIBarButtonItem( + barButtonSystemItem: .done, + target: self, + action: #selector(dismissWebView) + ) + + presenter.navigationController?.present(webViewController, animated: true) + } + + @objc + private func dismissWebView() { + let completion = onDismiss + onDismiss = nil + completion?() + } +} + +final class NativeProductDetailCoordinator { + private let product: Product + + init(product: Product) { + self.product = product + } + + func start(presenter: UIViewController, + presentationStyle: ProductDetailRouter.PresentationStyle, + onDismiss: (() -> Void)? = nil) { + + } + + func viewController( + presentationStyle: ProductDetailRouter.PresentationStyle, + forceReadOnly: Bool, + onDeleteCompletion: (() -> Void)? = nil) -> UIViewController { + return ProductDetailsFactory.productDetails(product: product, + presentationStyle: presentationStyle.asProductFormPresentationStyle, + forceReadOnly: false, + onDeleteCompletion: onDeleteCompletion ?? {}) + } +} diff --git a/WooCommerce/Classes/ViewModels/Order Details/OrderDetailsViewModel.swift b/WooCommerce/Classes/ViewModels/Order Details/OrderDetailsViewModel.swift index b4030f5c4db..401717b425f 100644 --- a/WooCommerce/Classes/ViewModels/Order Details/OrderDetailsViewModel.swift +++ b/WooCommerce/Classes/ViewModels/Order Details/OrderDetailsViewModel.swift @@ -510,7 +510,6 @@ extension OrderDetailsViewModel { viewController.present(navController, animated: true, completion: nil) case .aggregateOrderItem: let item = dataSource.aggregateOrderItems[indexPath.row] - // TODO: Replace with ProductDetailRouter let loaderViewController = ProductLoaderViewController(model: .init(aggregateOrderItem: item), siteID: order.siteID, forceReadOnly: false) diff --git a/WooCommerce/Classes/ViewRelated/Products/Product Loader/ProductLoaderViewController.swift b/WooCommerce/Classes/ViewRelated/Products/Product Loader/ProductLoaderViewController.swift index 2c1338d6c7d..d5d3f9bd3f6 100644 --- a/WooCommerce/Classes/ViewRelated/Products/Product Loader/ProductLoaderViewController.swift +++ b/WooCommerce/Classes/ViewRelated/Products/Product Loader/ProductLoaderViewController.swift @@ -227,12 +227,10 @@ private extension ProductLoaderViewController { /// Presents the ProductFormViewController, as a childViewController, for a given Product. /// func presentProductDetails(for product: Product) { - // TODO: Replace with ProductDetailRouter - ProductDetailsFactory.productDetails(product: product, - presentationStyle: .contained(containerViewController: { [weak self] in self }), - forceReadOnly: forceReadOnly) { [weak self] viewController in - self?.attachProductDetailsChildViewController(viewController) - } + let viewController = ProductDetailRouter.shared.viewController(product: product, + presentationStyle: .contained(containerViewController: { [weak self] in self }), + forceReadOnly: forceReadOnly) + attachProductDetailsChildViewController(viewController) } /// Presents the product variation details for a given ProductVariation and its parent Product. diff --git a/WooCommerce/Classes/ViewRelated/Products/ProductDetailsFactory.swift b/WooCommerce/Classes/ViewRelated/Products/ProductDetailsFactory.swift index 743bc080594..f110fe62686 100644 --- a/WooCommerce/Classes/ViewRelated/Products/ProductDetailsFactory.swift +++ b/WooCommerce/Classes/ViewRelated/Products/ProductDetailsFactory.swift @@ -26,6 +26,21 @@ struct ProductDetailsFactory { onDeleteCompletion: onDeleteCompletion) onCompletion(vc) } + + static func productDetails(product: Product, + presentationStyle: ProductFormPresentationStyle, + currencySettings: CurrencySettings = ServiceLocator.currencySettings, + forceReadOnly: Bool, + productImageUploader: ProductImageUploaderProtocol = ServiceLocator.productImageUploader, + onDeleteCompletion: @escaping () -> Void = {}) -> UIViewController { + let vc = productDetails(product: product, + presentationStyle: presentationStyle, + currencySettings: currencySettings, + isEditProductsEnabled: forceReadOnly ? false: true, + productImageUploader: productImageUploader, + onDeleteCompletion: onDeleteCompletion) + return vc + } } private extension ProductDetailsFactory { diff --git a/WooCommerce/Classes/ViewRelated/Products/ProductsSplitViewCoordinator.swift b/WooCommerce/Classes/ViewRelated/Products/ProductsSplitViewCoordinator.swift index 78a7c96500d..45b9e62947f 100644 --- a/WooCommerce/Classes/ViewRelated/Products/ProductsSplitViewCoordinator.swift +++ b/WooCommerce/Classes/ViewRelated/Products/ProductsSplitViewCoordinator.swift @@ -122,15 +122,15 @@ private extension ProductsSplitViewCoordinator { } func showProductForm(product: Product) { - // TODO: Replace with ProductDetailRouter - ProductDetailsFactory.productDetails(product: product, - presentationStyle: .navigationStack, - forceReadOnly: false, - onDeleteCompletion: { [weak self] in + let viewController = ProductDetailRouter.shared.viewController(product: product, + forceReadOnly: false, + onDeleteCompletion: { [weak self] in self?.onSecondaryProductFormDeletion() - }) { [weak self] viewController in - self?.showSecondaryView(contentType: .productForm(product: product), viewController: viewController, replacesNavigationStack: true) - } + }) + + showSecondaryView(contentType: .productForm(product: product), + viewController: viewController, + replacesNavigationStack: true) } func startProductCreationIfNoUnsavedChanges(sourceView: AddProductCoordinator.SourceView, isFirstProduct: Bool) { diff --git a/WooCommerce/Classes/ViewRelated/Products/ProductsViewController.swift b/WooCommerce/Classes/ViewRelated/Products/ProductsViewController.swift index 915c9a407fe..622b4e6f1cc 100644 --- a/WooCommerce/Classes/ViewRelated/Products/ProductsViewController.swift +++ b/WooCommerce/Classes/ViewRelated/Products/ProductsViewController.swift @@ -1222,7 +1222,6 @@ private extension ProductsViewController { } return } - // TODO: Replace with ProductDetailRouter navigateToContent(.productForm(product: product)) } } diff --git a/WooCommerce/WooCommerce.xcodeproj/project.pbxproj b/WooCommerce/WooCommerce.xcodeproj/project.pbxproj index 75f6968ef19..e993a92124f 100644 --- a/WooCommerce/WooCommerce.xcodeproj/project.pbxproj +++ b/WooCommerce/WooCommerce.xcodeproj/project.pbxproj @@ -1340,6 +1340,7 @@ 640DA3482E97DE4F00317FB2 /* SceneDelegate.swift in Sources */ = {isa = PBXBuildFile; fileRef = 640DA3472E97DE4F00317FB2 /* SceneDelegate.swift */; }; 64D355A52E99048E005F53F7 /* TestingSceneDelegate.swift in Sources */ = {isa = PBXBuildFile; fileRef = 64D355A42E99048E005F53F7 /* TestingSceneDelegate.swift */; }; 68051E1E2E9DFE5500228196 /* POSNotificationSchedulerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 68051E1D2E9DFE5100228196 /* POSNotificationSchedulerTests.swift */; }; + 680BA59A2A4C377900F5559D /* UpgradeViewState.swift in Sources */ = {isa = PBXBuildFile; fileRef = 680BA5992A4C377900F5559D /* UpgradeViewState.swift */; }; 680E36B52BD8B9B900E8BCEA /* OrderSubscriptionTableViewCell.xib in Resources */ = {isa = PBXBuildFile; fileRef = 680E36B42BD8B9B900E8BCEA /* OrderSubscriptionTableViewCell.xib */; }; 680E36B72BD8C49F00E8BCEA /* OrderSubscriptionTableViewCellViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 680E36B62BD8C49F00E8BCEA /* OrderSubscriptionTableViewCellViewModel.swift */; }; 682140AF2E125437005E86AB /* UILabel+SalesChannel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 682140AE2E125430005E86AB /* UILabel+SalesChannel.swift */; }; @@ -5777,6 +5778,7 @@ 3F0904022D26A40800D8ACCE /* WordPressAuthenticator */ = {isa = PBXFileSystemSynchronizedRootGroup; exceptions = (3F09041C2D26A40800D8ACCE /* PBXFileSystemSynchronizedBuildFileExceptionSet */, ); explicitFileTypes = {}; explicitFolders = (); path = WordPressAuthenticator; sourceTree = ""; }; 3F09040E2D26A40800D8ACCE /* WordPressAuthenticatorTests */ = {isa = PBXFileSystemSynchronizedRootGroup; exceptions = (3FD28D5E2D271391002EBB3D /* PBXFileSystemSynchronizedBuildFileExceptionSet */, ); explicitFileTypes = {}; explicitFolders = (); path = WordPressAuthenticatorTests; sourceTree = ""; }; 3FD9BFBE2E0A2533004A8DC8 /* WooCommerceScreenshots */ = {isa = PBXFileSystemSynchronizedRootGroup; exceptions = (3FD9BFC42E0A2534004A8DC8 /* PBXFileSystemSynchronizedBuildFileExceptionSet */, ); explicitFileTypes = {}; explicitFolders = (); path = WooCommerceScreenshots; sourceTree = ""; }; + 646A2C682E9FCD7E003A32A1 /* Routing */ = {isa = PBXFileSystemSynchronizedRootGroup; explicitFileTypes = {}; explicitFolders = (); path = Routing; sourceTree = ""; }; DEDB5D342E7A68950022E5A1 /* Bookings */ = {isa = PBXFileSystemSynchronizedRootGroup; explicitFileTypes = {}; explicitFolders = (); path = Bookings; sourceTree = ""; }; /* End PBXFileSystemSynchronizedRootGroup section */ @@ -9727,6 +9729,7 @@ B56DB3F12049C0B800D4AA8E /* Classes */ = { isa = PBXGroup; children = ( + 646A2C682E9FCD7E003A32A1 /* Routing */, 640DA3472E97DE4F00317FB2 /* SceneDelegate.swift */, DEDB5D342E7A68950022E5A1 /* Bookings */, 2DCB54F82E6AE8C900621F90 /* CIAB */, @@ -13248,6 +13251,7 @@ ); fileSystemSynchronizedGroups = ( 2D33E6B02DD1453E000C7198 /* WooShippingPaymentMethod */, + 646A2C682E9FCD7E003A32A1 /* Routing */, DEDB5D342E7A68950022E5A1 /* Bookings */, ); name = WooCommerce; From 5d7bfde976f90afc4d25a783c3b167d9a3f0576f Mon Sep 17 00:00:00 2001 From: Adam Borbas Date: Thu, 16 Oct 2025 12:39:59 +0200 Subject: [PATCH 03/15] Remove comments where we do not need migration --- .../Refund Details/RefundDetailsViewModel.swift | 1 - .../Dashboard/ProductStock/ProductStockDashboardCard.swift | 1 - .../TopPerformers/TopPerformerDataViewController.swift | 1 - .../TopPerformers/TopPerformersDashboardView.swift | 1 - WooCommerce/Classes/ViewRelated/MainTabBarController.swift | 1 - .../AggregatedProductListViewController.swift | 1 - .../Products/Product Loader/ProductLoaderView.swift | 1 - .../ViewRelated/Products/ProductDetailsFactoryTests.swift | 6 ------ 8 files changed, 13 deletions(-) diff --git a/WooCommerce/Classes/ViewModels/Order Details/Refund Details/RefundDetailsViewModel.swift b/WooCommerce/Classes/ViewModels/Order Details/Refund Details/RefundDetailsViewModel.swift index 5880355ebe5..3af7ee687eb 100644 --- a/WooCommerce/Classes/ViewModels/Order Details/Refund Details/RefundDetailsViewModel.swift +++ b/WooCommerce/Classes/ViewModels/Order Details/Refund Details/RefundDetailsViewModel.swift @@ -128,7 +128,6 @@ extension RefundDetailsViewModel { case .orderItem: let item = refund.items[indexPath.row] - // TODO: Replace with ProductDetailRouter, native let loaderViewController = ProductLoaderViewController(model: .init(orderItemRefund: item), siteID: refund.siteID, forceReadOnly: true) diff --git a/WooCommerce/Classes/ViewRelated/Dashboard/ProductStock/ProductStockDashboardCard.swift b/WooCommerce/Classes/ViewRelated/Dashboard/ProductStock/ProductStockDashboardCard.swift index 12017a96201..18f90d21809 100644 --- a/WooCommerce/Classes/ViewRelated/Dashboard/ProductStock/ProductStockDashboardCard.swift +++ b/WooCommerce/Classes/ViewRelated/Dashboard/ProductStock/ProductStockDashboardCard.swift @@ -189,7 +189,6 @@ private extension ProductStockDashboardCard { .product(productID: item.productID) } }() - // TODO: Replace with ProductDetailRouter let loaderViewController = ProductLoaderViewController(model: model, siteID: viewModel.siteID, forceReadOnly: false) diff --git a/WooCommerce/Classes/ViewRelated/Dashboard/TopPerformers/TopPerformerDataViewController.swift b/WooCommerce/Classes/ViewRelated/Dashboard/TopPerformers/TopPerformerDataViewController.swift index 71988e16989..0b71b6ee5e5 100644 --- a/WooCommerce/Classes/ViewRelated/Dashboard/TopPerformers/TopPerformerDataViewController.swift +++ b/WooCommerce/Classes/ViewRelated/Dashboard/TopPerformers/TopPerformerDataViewController.swift @@ -176,7 +176,6 @@ private extension TopPerformerDataViewController { /// Presents the product details for a given TopEarnerStatsItem. /// func presentProductDetails(statsItem: TopEarnerStatsItem) { - // TODO: Replace with ProductDetailRouter let loaderViewController = ProductLoaderViewController(model: .init(topEarnerStatsItem: statsItem), siteID: siteID, forceReadOnly: false) diff --git a/WooCommerce/Classes/ViewRelated/Dashboard/TopPerformers/TopPerformersDashboardView.swift b/WooCommerce/Classes/ViewRelated/Dashboard/TopPerformers/TopPerformersDashboardView.swift index 2a25c26c023..3c2c2e77b89 100644 --- a/WooCommerce/Classes/ViewRelated/Dashboard/TopPerformers/TopPerformersDashboardView.swift +++ b/WooCommerce/Classes/ViewRelated/Dashboard/TopPerformers/TopPerformersDashboardView.swift @@ -181,7 +181,6 @@ private extension TopPerformersDashboardView { } func productDetailView(for item: TopEarnerStatsItem) -> UIViewController { - // TODO: Replace with ProductDetailRouter let loaderViewController = ProductLoaderViewController(model: .init(topEarnerStatsItem: item), siteID: viewModel.siteID, forceReadOnly: false) diff --git a/WooCommerce/Classes/ViewRelated/MainTabBarController.swift b/WooCommerce/Classes/ViewRelated/MainTabBarController.swift index d79dca97815..f0ffdaabbca 100644 --- a/WooCommerce/Classes/ViewRelated/MainTabBarController.swift +++ b/WooCommerce/Classes/ViewRelated/MainTabBarController.swift @@ -1036,7 +1036,6 @@ private extension MainTabBarController { return .productVariation(productID: productID, variationID: variationID) } }() - // TODO: Replace with ProductDetailRouter let productViewController = ProductLoaderViewController(model: model, siteID: error.siteID, forceReadOnly: false) diff --git a/WooCommerce/Classes/ViewRelated/Orders/Order Details/Product List Section/Product Details/AggregatedProductListViewController.swift b/WooCommerce/Classes/ViewRelated/Orders/Order Details/Product List Section/Product Details/AggregatedProductListViewController.swift index 2c574104878..f1f861186b0 100644 --- a/WooCommerce/Classes/ViewRelated/Orders/Order Details/Product List Section/Product Details/AggregatedProductListViewController.swift +++ b/WooCommerce/Classes/ViewRelated/Orders/Order Details/Product List Section/Product Details/AggregatedProductListViewController.swift @@ -122,7 +122,6 @@ private extension AggregatedProductListViewController { /// Displays the product details screen for the provided AggregateOrderItem /// func productWasPressed(orderItem: AggregateOrderItem) { - // TODO: Replace with ProductDetailRouter, native let loaderViewController = ProductLoaderViewController(model: .init(aggregateOrderItem: orderItem), siteID: viewModel.order.siteID, forceReadOnly: true) diff --git a/WooCommerce/Classes/ViewRelated/Products/Product Loader/ProductLoaderView.swift b/WooCommerce/Classes/ViewRelated/Products/Product Loader/ProductLoaderView.swift index d22a4369500..3f177527318 100644 --- a/WooCommerce/Classes/ViewRelated/Products/Product Loader/ProductLoaderView.swift +++ b/WooCommerce/Classes/ViewRelated/Products/Product Loader/ProductLoaderView.swift @@ -9,7 +9,6 @@ struct ProductLoaderView: UIViewControllerRepresentable { let forceReadOnly: Bool func makeUIViewController(context: Context) -> UINavigationController { - // TODO: Replace with ProductDetailRouter let viewController = ProductLoaderViewController(model: model, siteID: siteID, forceReadOnly: forceReadOnly) diff --git a/WooCommerce/WooCommerceTests/ViewRelated/Products/ProductDetailsFactoryTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/Products/ProductDetailsFactoryTests.swift index 38d9ec511c4..c035dd480b4 100644 --- a/WooCommerce/WooCommerceTests/ViewRelated/Products/ProductDetailsFactoryTests.swift +++ b/WooCommerce/WooCommerceTests/ViewRelated/Products/ProductDetailsFactoryTests.swift @@ -12,7 +12,6 @@ final class ProductDetailsFactoryTests: XCTestCase { let exp = expectation(description: #function) // Action - // TODO: Replace with ProductDetailRouter ProductDetailsFactory.productDetails(product: product, presentationStyle: .navigationStack, forceReadOnly: false) { viewController in @@ -31,7 +30,6 @@ final class ProductDetailsFactoryTests: XCTestCase { let exp = expectation(description: #function) // Action - // TODO: Replace with ProductDetailRouter ProductDetailsFactory.productDetails(product: product, presentationStyle: .navigationStack, forceReadOnly: false) { viewController in @@ -50,7 +48,6 @@ final class ProductDetailsFactoryTests: XCTestCase { let exp = expectation(description: #function) // Action - // TODO: Replace with ProductDetailRouter ProductDetailsFactory.productDetails(product: product, presentationStyle: .navigationStack, forceReadOnly: false) { viewController in @@ -69,7 +66,6 @@ final class ProductDetailsFactoryTests: XCTestCase { let exp = expectation(description: #function) // Action - // TODO: Replace with ProductDetailRouter ProductDetailsFactory.productDetails(product: product, presentationStyle: .navigationStack, forceReadOnly: false) { viewController in @@ -88,7 +84,6 @@ final class ProductDetailsFactoryTests: XCTestCase { // Action waitForExpectation { expectation in - // TODO: Replace with ProductDetailRouter ProductDetailsFactory.productDetails(product: product, presentationStyle: .navigationStack, forceReadOnly: false) { viewController in @@ -105,7 +100,6 @@ final class ProductDetailsFactoryTests: XCTestCase { // Action waitForExpectation { expectation in - // TODO: Replace with ProductDetailRouter ProductDetailsFactory.productDetails(product: product, presentationStyle: .navigationStack, forceReadOnly: true) { viewController in From 231c58773ce57473ba92ad520da0ec2d2e14add7 Mon Sep 17 00:00:00 2001 From: Adam Borbas Date: Thu, 16 Oct 2025 12:41:26 +0200 Subject: [PATCH 04/15] POC of ProductDetailRouter --- .../Classes/Routing/ProductDetailRouter.swift | 58 ++++--------------- .../Products/ProductDetailsFactory.swift | 7 +++ .../Products/ProductsViewController.swift | 9 +-- 3 files changed, 21 insertions(+), 53 deletions(-) diff --git a/WooCommerce/Classes/Routing/ProductDetailRouter.swift b/WooCommerce/Classes/Routing/ProductDetailRouter.swift index e372614e9f1..beb402ca846 100644 --- a/WooCommerce/Classes/Routing/ProductDetailRouter.swift +++ b/WooCommerce/Classes/Routing/ProductDetailRouter.swift @@ -4,16 +4,10 @@ import Yosemite final class ProductDetailRouter { enum PresentationStyle { case undefined - case push - case modal case contained(containerViewController: () -> UIViewController?) var asProductFormPresentationStyle: ProductFormPresentationStyle { switch self { - case .push: - .navigationStack - case .modal: - .navigationStack case .contained(let containerViewController): .contained(containerViewController: containerViewController) case .undefined: @@ -30,42 +24,24 @@ final class ProductDetailRouter { self.productURLProvider = productURLProvider } - func open(product: Product, - presenter: UIViewController, - presentationStyle: PresentationStyle, - onDismiss: (() -> Void)? = nil) { - if product.productType == .booking { - let coordinator = WebViewProductDetailCoordinator(product: product, - productURLProvider: productURLProvider) - coordinator.start(presenter: presenter, onDismiss: onDismiss) - } else { - let coordinator = NativeProductDetailCoordinator(product: product) - coordinator.start(presenter: presenter, - presentationStyle: presentationStyle, - onDismiss: onDismiss) - } - } - func viewController(product: Product, presentationStyle: PresentationStyle = .undefined, forceReadOnly: Bool, onDeleteCompletion: (() -> Void)? = nil) -> UIViewController { + + let viewController: UIViewController if product.productType == .booking { let coordinator = WebViewProductDetailCoordinator(product: product, productURLProvider: productURLProvider) - return coordinator.viewController() + viewController = coordinator.viewController() } else { let coordinator = NativeProductDetailCoordinator(product: product) - return coordinator.viewController(presentationStyle: presentationStyle, + viewController = coordinator.viewController(presentationStyle: presentationStyle, forceReadOnly: forceReadOnly) } - } -} -final class ProductURLProvider { - func adminURL(for product: Product) -> URL? { - return URL(string: "https://wordpress.com") + return viewController } } @@ -91,18 +67,6 @@ final class WebViewProductDetailCoordinator: NSObject { return webViewController } - func start(presenter: UIViewController, - onDismiss: (() -> Void)? = nil) { - let webViewController = viewController() - webViewController.navigationItem.leftBarButtonItem = UIBarButtonItem( - barButtonSystemItem: .done, - target: self, - action: #selector(dismissWebView) - ) - - presenter.navigationController?.present(webViewController, animated: true) - } - @objc private func dismissWebView() { let completion = onDismiss @@ -118,12 +82,6 @@ final class NativeProductDetailCoordinator { self.product = product } - func start(presenter: UIViewController, - presentationStyle: ProductDetailRouter.PresentationStyle, - onDismiss: (() -> Void)? = nil) { - - } - func viewController( presentationStyle: ProductDetailRouter.PresentationStyle, forceReadOnly: Bool, @@ -134,3 +92,9 @@ final class NativeProductDetailCoordinator { onDeleteCompletion: onDeleteCompletion ?? {}) } } + +final class ProductURLProvider { + func adminURL(for product: Product) -> URL? { + return URL(string: "https://wordpress.com") + } +} diff --git a/WooCommerce/Classes/ViewRelated/Products/ProductDetailsFactory.swift b/WooCommerce/Classes/ViewRelated/Products/ProductDetailsFactory.swift index f110fe62686..1df9b8eb3d3 100644 --- a/WooCommerce/Classes/ViewRelated/Products/ProductDetailsFactory.swift +++ b/WooCommerce/Classes/ViewRelated/Products/ProductDetailsFactory.swift @@ -27,6 +27,13 @@ struct ProductDetailsFactory { onCompletion(vc) } + /// Creates a product details view controller asynchronously based on the app settings. + /// - Parameters: + /// - product: product model. + /// - presentationStyle: how the product details are presented. + /// - currencySettings: site currency settings. + /// - forceReadOnly: force the product detail to be presented in read only mode + /// - onDeleteCompletion: called when the product deletion completes in the product form. static func productDetails(product: Product, presentationStyle: ProductFormPresentationStyle, currencySettings: CurrencySettings = ServiceLocator.currencySettings, diff --git a/WooCommerce/Classes/ViewRelated/Products/ProductsViewController.swift b/WooCommerce/Classes/ViewRelated/Products/ProductsViewController.swift index 622b4e6f1cc..2530931ade8 100644 --- a/WooCommerce/Classes/ViewRelated/Products/ProductsViewController.swift +++ b/WooCommerce/Classes/ViewRelated/Products/ProductsViewController.swift @@ -1214,12 +1214,9 @@ extension ProductsViewController: UITableViewDelegate { private extension ProductsViewController { func didSelectProduct(product: Product) { guard isSplitViewEnabled else { - // TODO: Replace with ProductDetailRouter - ProductDetailsFactory.productDetails(product: product, - presentationStyle: .navigationStack, - forceReadOnly: false) { [weak self] viewController in - self?.navigationController?.pushViewController(viewController, animated: true) - } + let viewController = ProductDetailRouter.shared.viewController(product: product, + forceReadOnly: false) + navigationController?.pushViewController(viewController, animated: true) return } navigateToContent(.productForm(product: product)) From c3f26279fe62eab203e749072546c17fe39a47e0 Mon Sep 17 00:00:00 2001 From: Adam Borbas Date: Mon, 20 Oct 2025 09:37:00 +0200 Subject: [PATCH 05/15] These are actually used --- WooCommerce/Classes/CIAB/CIABEligibilityChecker.swift | 2 -- WooCommerce/Classes/CIAB/CIABEligibilityCheckerProtocol.swift | 1 - 2 files changed, 3 deletions(-) diff --git a/WooCommerce/Classes/CIAB/CIABEligibilityChecker.swift b/WooCommerce/Classes/CIAB/CIABEligibilityChecker.swift index b3627647e1c..955af2262b0 100644 --- a/WooCommerce/Classes/CIAB/CIABEligibilityChecker.swift +++ b/WooCommerce/Classes/CIAB/CIABEligibilityChecker.swift @@ -1,5 +1,3 @@ -/// periphery: ignore:all - Will be used in upcoming PRs - import Foundation import Yosemite diff --git a/WooCommerce/Classes/CIAB/CIABEligibilityCheckerProtocol.swift b/WooCommerce/Classes/CIAB/CIABEligibilityCheckerProtocol.swift index 6c7cd0aac92..54c82d7f844 100644 --- a/WooCommerce/Classes/CIAB/CIABEligibilityCheckerProtocol.swift +++ b/WooCommerce/Classes/CIAB/CIABEligibilityCheckerProtocol.swift @@ -1,7 +1,6 @@ import Foundation import Yosemite -/// periphery: ignore - Will be used in upcoming changes for app feature gating protocol CIABEligibilityCheckerProtocol { var isCurrentSiteCIAB: Bool { get } From 6f92ebc593f919f2164526548b3c50baeb491489 Mon Sep 17 00:00:00 2001 From: Adam Borbas Date: Mon, 20 Oct 2025 09:37:11 +0200 Subject: [PATCH 06/15] Constructiong URL --- .../Classes/Routing/ProductDetailRouter.swift | 38 ++++++++++++------- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/WooCommerce/Classes/Routing/ProductDetailRouter.swift b/WooCommerce/Classes/Routing/ProductDetailRouter.swift index beb402ca846..a0fbc11fd2e 100644 --- a/WooCommerce/Classes/Routing/ProductDetailRouter.swift +++ b/WooCommerce/Classes/Routing/ProductDetailRouter.swift @@ -19,9 +19,15 @@ final class ProductDetailRouter { static var shared = ProductDetailRouter(productURLProvider: ProductURLProvider()) private let productURLProvider: ProductURLProvider + private let ciabChecker: CIABEligibilityCheckerProtocol + private let stores: StoresManager - init(productURLProvider: ProductURLProvider) { + init(productURLProvider: ProductURLProvider, + ciabChecker: CIABEligibilityCheckerProtocol = CIABEligibilityChecker(), + stores: StoresManager = ServiceLocator.stores) { self.productURLProvider = productURLProvider + self.ciabChecker = ciabChecker + self.stores = stores } func viewController(product: Product, @@ -30,35 +36,39 @@ final class ProductDetailRouter { onDeleteCompletion: (() -> Void)? = nil) -> UIViewController { let viewController: UIViewController - if product.productType == .booking { - let coordinator = WebViewProductDetailCoordinator(product: product, - productURLProvider: productURLProvider) - viewController = coordinator.viewController() + if shouldOpenInWeb(product: product) { + let coordinator = WebViewProductDetailCoordinator(productURLProvider: productURLProvider) + viewController = coordinator.viewController(product: product, + site: stores.sessionManager.defaultSite!) } else { let coordinator = NativeProductDetailCoordinator(product: product) viewController = coordinator.viewController(presentationStyle: presentationStyle, - forceReadOnly: forceReadOnly) + forceReadOnly: forceReadOnly) } return viewController } + + private func shouldOpenInWeb(product: Product) -> Bool { + return ciabChecker.isCurrentSiteCIAB && product.productType == .booking + } } final class WebViewProductDetailCoordinator: NSObject { - private let product: Product private let productURLProvider: ProductURLProvider private var onDismiss: (() -> Void)? - init(product: Product, productURLProvider: ProductURLProvider) { - self.product = product + init(productURLProvider: ProductURLProvider) { self.productURLProvider = productURLProvider } - func viewController(onDeleteCompletion: (() -> Void)? = nil) -> UIViewController { - guard let url = productURLProvider.adminURL(for: product) else { - return UIViewController() // TODO: What do we do in this case? + func viewController(product: Product, + site: Site, + onDeleteCompletion: (() -> Void)? = nil) -> UIViewController { + guard let url = productURLProvider.adminURL(for: product, site: site) else { + return UIViewController() } let viewModel = WPAdminWebViewModel(title: product.name, initialURL: url) @@ -94,7 +104,7 @@ final class NativeProductDetailCoordinator { } final class ProductURLProvider { - func adminURL(for product: Product) -> URL? { - return URL(string: "https://wordpress.com") + func adminURL(for product: Product, site: Site) -> URL? { + return site.adminURLWithFallback()?.appendingPathComponent("?page=next-admin&p=/woocommerce/products/edit/\(product.productID)") } } From 04e9cb6fd12d59b4501338380f20293ee6a3c4b6 Mon Sep 17 00:00:00 2001 From: Adam Borbas Date: Mon, 20 Oct 2025 13:34:35 +0200 Subject: [PATCH 07/15] Use correct URL --- WooCommerce/Classes/Routing/ProductDetailRouter.swift | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/WooCommerce/Classes/Routing/ProductDetailRouter.swift b/WooCommerce/Classes/Routing/ProductDetailRouter.swift index a0fbc11fd2e..969c7fcd90c 100644 --- a/WooCommerce/Classes/Routing/ProductDetailRouter.swift +++ b/WooCommerce/Classes/Routing/ProductDetailRouter.swift @@ -105,6 +105,15 @@ final class NativeProductDetailCoordinator { final class ProductURLProvider { func adminURL(for product: Product, site: Site) -> URL? { - return site.adminURLWithFallback()?.appendingPathComponent("?page=next-admin&p=/woocommerce/products/edit/\(product.productID)") + guard let base = site.adminURLWithFallback() else { return nil } + let admin = base.appendingPathComponent("wp-admin") + + var components = URLComponents(url: admin, resolvingAgainstBaseURL: false)! + components.queryItems = [ + URLQueryItem(name: "page", value: "next-admin"), + URLQueryItem(name: "p", value: "/woocommerce/products/edit/\(product.productID)") + ] + + return components.url } } From a2b5a8a1a56323ed1da0777941f65b89f2164dc3 Mon Sep 17 00:00:00 2001 From: Adam Borbas Date: Tue, 21 Oct 2025 07:50:23 +0200 Subject: [PATCH 08/15] Adding tests --- .../NativeProductDetailCoordinator.swift | 15 +++ .../ProductDetail/ProductDetailRouter.swift | 84 +++++++++++++ .../ProductDetail/ProductURLProvider.swift | 16 +++ .../WebViewProductDetailCoordinator.swift | 33 +++++ .../Classes/Routing/ProductDetailRouter.swift | 119 ------------------ .../ProductLoaderViewController.swift | 2 +- .../ProductsSplitViewCoordinator.swift | 2 +- .../Products/ProductsViewController.swift | 2 +- .../WooCommerce.xcodeproj/project.pbxproj | 6 +- .../ProductURLProviderTests.swift | 77 ++++++++++++ 10 files changed, 233 insertions(+), 123 deletions(-) create mode 100644 WooCommerce/Classes/Routing/ProductDetail/NativeProductDetailCoordinator.swift create mode 100644 WooCommerce/Classes/Routing/ProductDetail/ProductDetailRouter.swift create mode 100644 WooCommerce/Classes/Routing/ProductDetail/ProductURLProvider.swift create mode 100644 WooCommerce/Classes/Routing/ProductDetail/WebViewProductDetailCoordinator.swift delete mode 100644 WooCommerce/Classes/Routing/ProductDetailRouter.swift create mode 100644 WooCommerce/WooCommerceTests/Routing/ProductDetail/ProductURLProviderTests.swift diff --git a/WooCommerce/Classes/Routing/ProductDetail/NativeProductDetailCoordinator.swift b/WooCommerce/Classes/Routing/ProductDetail/NativeProductDetailCoordinator.swift new file mode 100644 index 00000000000..4923dd7822f --- /dev/null +++ b/WooCommerce/Classes/Routing/ProductDetail/NativeProductDetailCoordinator.swift @@ -0,0 +1,15 @@ +import Yosemite +import UIKit + +class NativeProductDetailCoordinator: ProductDetailCoordinator { + func viewController( + product: Product, + presentationStyle: ProductDetailPresenter.PresentationStyle, + forceReadOnly: Bool, + onDeleteCompletion: (() -> Void)? = nil) -> UIViewController { + return ProductDetailsFactory.productDetails(product: product, + presentationStyle: presentationStyle.asProductFormPresentationStyle, + forceReadOnly: false, + onDeleteCompletion: onDeleteCompletion ?? {}) + } +} diff --git a/WooCommerce/Classes/Routing/ProductDetail/ProductDetailRouter.swift b/WooCommerce/Classes/Routing/ProductDetail/ProductDetailRouter.swift new file mode 100644 index 00000000000..6130951d724 --- /dev/null +++ b/WooCommerce/Classes/Routing/ProductDetail/ProductDetailRouter.swift @@ -0,0 +1,84 @@ +import UIKit +import Yosemite + +final class ProductDetailPresenter { + enum PresentationStyle { + case undefined + case contained(containerViewController: () -> UIViewController?) + + var asProductFormPresentationStyle: ProductFormPresentationStyle { + switch self { + case .contained(let containerViewController): + .contained(containerViewController: containerViewController) + case .undefined: + .navigationStack + } + } + } + + static var shared = ProductDetailPresenter() + + private let ciabChecker: CIABEligibilityCheckerProtocol + private let coordinatorFactory: ProductDetailCoordinatorFactoryProtocol + + init(ciabChecker: CIABEligibilityCheckerProtocol = CIABEligibilityChecker(), + coordinatorFactory: ProductDetailCoordinatorFactoryProtocol = ProductDetailCoordinatorFactory.default) { + self.ciabChecker = ciabChecker + self.coordinatorFactory = coordinatorFactory + } + + func viewController(product: Product, + presentationStyle: PresentationStyle = .undefined, + forceReadOnly: Bool, + onDeleteCompletion: (() -> Void)? = nil) -> UIViewController { + + let coordinator: ProductDetailCoordinator + if shouldOpenInWeb(product: product) { + coordinator = coordinatorFactory.webCoordinator() + } else { + coordinator = coordinatorFactory.nativeCoordinator() + } + + let viewController = coordinator.viewController(product: product, + presentationStyle: presentationStyle, + forceReadOnly: forceReadOnly, + onDeleteCompletion: onDeleteCompletion) + + return viewController + } + + private func shouldOpenInWeb(product: Product) -> Bool { + return ciabChecker.isCurrentSiteCIAB && product.productType == .booking + } +} + +protocol ProductDetailCoordinator { + func viewController( + product: Product, + presentationStyle: ProductDetailPresenter.PresentationStyle, + forceReadOnly: Bool, + onDeleteCompletion: (() -> Void)?) -> UIViewController +} + +protocol ProductDetailCoordinatorFactoryProtocol { + func webCoordinator() -> ProductDetailCoordinator + func nativeCoordinator() -> ProductDetailCoordinator +} + +class ProductDetailCoordinatorFactory: ProductDetailCoordinatorFactoryProtocol { + static let `default` = ProductDetailCoordinatorFactory() + + private let stores: StoresManager + + init(stores: StoresManager = ServiceLocator.stores) { + self.stores = stores + } + + func webCoordinator() -> ProductDetailCoordinator { + return WebViewProductDetailCoordinator(site: stores.sessionManager.defaultSite!) + } + + func nativeCoordinator() -> ProductDetailCoordinator { + return NativeProductDetailCoordinator() + } +} diff --git a/WooCommerce/Classes/Routing/ProductDetail/ProductURLProvider.swift b/WooCommerce/Classes/Routing/ProductDetail/ProductURLProvider.swift new file mode 100644 index 00000000000..7648d1f4984 --- /dev/null +++ b/WooCommerce/Classes/Routing/ProductDetail/ProductURLProvider.swift @@ -0,0 +1,16 @@ +import Yosemite +import Foundation + +enum ProductURLProvider { + static func editAdminURL(for product: Product, site: Site) -> URL? { + guard let base = site.adminURLWithFallback() else { return nil } + + var components = URLComponents(url: base, resolvingAgainstBaseURL: false)! + components.queryItems = [ + URLQueryItem(name: "page", value: "next-admin"), + URLQueryItem(name: "p", value: "/woocommerce/products/edit/\(product.productID)") + ] + + return components.url + } +} diff --git a/WooCommerce/Classes/Routing/ProductDetail/WebViewProductDetailCoordinator.swift b/WooCommerce/Classes/Routing/ProductDetail/WebViewProductDetailCoordinator.swift new file mode 100644 index 00000000000..abb50efff50 --- /dev/null +++ b/WooCommerce/Classes/Routing/ProductDetail/WebViewProductDetailCoordinator.swift @@ -0,0 +1,33 @@ +import UIKit +import Yosemite + +final class WebViewProductDetailCoordinator: NSObject, ProductDetailCoordinator { + private var onDismiss: (() -> Void)? + + private let site: Site + + init(site: Site) { + self.site = site + } + + func viewController(product: Product, + presentationStyle: ProductDetailPresenter.PresentationStyle, + forceReadOnly: Bool, + onDeleteCompletion: (() -> Void)? = nil) -> UIViewController { + guard let url = ProductURLProvider.editAdminURL(for: product, site: site) else { + return UIViewController() + } + + let viewModel = WPAdminWebViewModel(title: product.name, initialURL: url) + let webViewController = AuthenticatedWebViewController(viewModel: viewModel) + + return webViewController + } + + @objc + private func dismissWebView() { + let completion = onDismiss + onDismiss = nil + completion?() + } +} diff --git a/WooCommerce/Classes/Routing/ProductDetailRouter.swift b/WooCommerce/Classes/Routing/ProductDetailRouter.swift deleted file mode 100644 index 969c7fcd90c..00000000000 --- a/WooCommerce/Classes/Routing/ProductDetailRouter.swift +++ /dev/null @@ -1,119 +0,0 @@ -import UIKit -import Yosemite - -final class ProductDetailRouter { - enum PresentationStyle { - case undefined - case contained(containerViewController: () -> UIViewController?) - - var asProductFormPresentationStyle: ProductFormPresentationStyle { - switch self { - case .contained(let containerViewController): - .contained(containerViewController: containerViewController) - case .undefined: - .navigationStack - } - } - } - - static var shared = ProductDetailRouter(productURLProvider: ProductURLProvider()) - - private let productURLProvider: ProductURLProvider - private let ciabChecker: CIABEligibilityCheckerProtocol - private let stores: StoresManager - - init(productURLProvider: ProductURLProvider, - ciabChecker: CIABEligibilityCheckerProtocol = CIABEligibilityChecker(), - stores: StoresManager = ServiceLocator.stores) { - self.productURLProvider = productURLProvider - self.ciabChecker = ciabChecker - self.stores = stores - } - - func viewController(product: Product, - presentationStyle: PresentationStyle = .undefined, - forceReadOnly: Bool, - onDeleteCompletion: (() -> Void)? = nil) -> UIViewController { - - let viewController: UIViewController - if shouldOpenInWeb(product: product) { - let coordinator = WebViewProductDetailCoordinator(productURLProvider: productURLProvider) - viewController = coordinator.viewController(product: product, - site: stores.sessionManager.defaultSite!) - - } else { - let coordinator = NativeProductDetailCoordinator(product: product) - viewController = coordinator.viewController(presentationStyle: presentationStyle, - forceReadOnly: forceReadOnly) - } - - return viewController - } - - private func shouldOpenInWeb(product: Product) -> Bool { - return ciabChecker.isCurrentSiteCIAB && product.productType == .booking - } -} - -final class WebViewProductDetailCoordinator: NSObject { - private let productURLProvider: ProductURLProvider - - private var onDismiss: (() -> Void)? - - init(productURLProvider: ProductURLProvider) { - self.productURLProvider = productURLProvider - } - - func viewController(product: Product, - site: Site, - onDeleteCompletion: (() -> Void)? = nil) -> UIViewController { - guard let url = productURLProvider.adminURL(for: product, site: site) else { - return UIViewController() - } - - let viewModel = WPAdminWebViewModel(title: product.name, initialURL: url) - let webViewController = AuthenticatedWebViewController(viewModel: viewModel) - - return webViewController - } - - @objc - private func dismissWebView() { - let completion = onDismiss - onDismiss = nil - completion?() - } -} - -final class NativeProductDetailCoordinator { - private let product: Product - - init(product: Product) { - self.product = product - } - - func viewController( - presentationStyle: ProductDetailRouter.PresentationStyle, - forceReadOnly: Bool, - onDeleteCompletion: (() -> Void)? = nil) -> UIViewController { - return ProductDetailsFactory.productDetails(product: product, - presentationStyle: presentationStyle.asProductFormPresentationStyle, - forceReadOnly: false, - onDeleteCompletion: onDeleteCompletion ?? {}) - } -} - -final class ProductURLProvider { - func adminURL(for product: Product, site: Site) -> URL? { - guard let base = site.adminURLWithFallback() else { return nil } - let admin = base.appendingPathComponent("wp-admin") - - var components = URLComponents(url: admin, resolvingAgainstBaseURL: false)! - components.queryItems = [ - URLQueryItem(name: "page", value: "next-admin"), - URLQueryItem(name: "p", value: "/woocommerce/products/edit/\(product.productID)") - ] - - return components.url - } -} diff --git a/WooCommerce/Classes/ViewRelated/Products/Product Loader/ProductLoaderViewController.swift b/WooCommerce/Classes/ViewRelated/Products/Product Loader/ProductLoaderViewController.swift index d5d3f9bd3f6..2cfcf35b0be 100644 --- a/WooCommerce/Classes/ViewRelated/Products/Product Loader/ProductLoaderViewController.swift +++ b/WooCommerce/Classes/ViewRelated/Products/Product Loader/ProductLoaderViewController.swift @@ -227,7 +227,7 @@ private extension ProductLoaderViewController { /// Presents the ProductFormViewController, as a childViewController, for a given Product. /// func presentProductDetails(for product: Product) { - let viewController = ProductDetailRouter.shared.viewController(product: product, + let viewController = ProductDetailPresenter.shared.viewController(product: product, presentationStyle: .contained(containerViewController: { [weak self] in self }), forceReadOnly: forceReadOnly) attachProductDetailsChildViewController(viewController) diff --git a/WooCommerce/Classes/ViewRelated/Products/ProductsSplitViewCoordinator.swift b/WooCommerce/Classes/ViewRelated/Products/ProductsSplitViewCoordinator.swift index 45b9e62947f..1a3be5be4d0 100644 --- a/WooCommerce/Classes/ViewRelated/Products/ProductsSplitViewCoordinator.swift +++ b/WooCommerce/Classes/ViewRelated/Products/ProductsSplitViewCoordinator.swift @@ -122,7 +122,7 @@ private extension ProductsSplitViewCoordinator { } func showProductForm(product: Product) { - let viewController = ProductDetailRouter.shared.viewController(product: product, + let viewController = ProductDetailPresenter.shared.viewController(product: product, forceReadOnly: false, onDeleteCompletion: { [weak self] in self?.onSecondaryProductFormDeletion() diff --git a/WooCommerce/Classes/ViewRelated/Products/ProductsViewController.swift b/WooCommerce/Classes/ViewRelated/Products/ProductsViewController.swift index 2530931ade8..8e30c2abd78 100644 --- a/WooCommerce/Classes/ViewRelated/Products/ProductsViewController.swift +++ b/WooCommerce/Classes/ViewRelated/Products/ProductsViewController.swift @@ -1214,7 +1214,7 @@ extension ProductsViewController: UITableViewDelegate { private extension ProductsViewController { func didSelectProduct(product: Product) { guard isSplitViewEnabled else { - let viewController = ProductDetailRouter.shared.viewController(product: product, + let viewController = ProductDetailPresenter.shared.viewController(product: product, forceReadOnly: false) navigationController?.pushViewController(viewController, animated: true) return diff --git a/WooCommerce/WooCommerce.xcodeproj/project.pbxproj b/WooCommerce/WooCommerce.xcodeproj/project.pbxproj index e993a92124f..6f4a3c8f87a 100644 --- a/WooCommerce/WooCommerce.xcodeproj/project.pbxproj +++ b/WooCommerce/WooCommerce.xcodeproj/project.pbxproj @@ -1340,7 +1340,6 @@ 640DA3482E97DE4F00317FB2 /* SceneDelegate.swift in Sources */ = {isa = PBXBuildFile; fileRef = 640DA3472E97DE4F00317FB2 /* SceneDelegate.swift */; }; 64D355A52E99048E005F53F7 /* TestingSceneDelegate.swift in Sources */ = {isa = PBXBuildFile; fileRef = 64D355A42E99048E005F53F7 /* TestingSceneDelegate.swift */; }; 68051E1E2E9DFE5500228196 /* POSNotificationSchedulerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 68051E1D2E9DFE5100228196 /* POSNotificationSchedulerTests.swift */; }; - 680BA59A2A4C377900F5559D /* UpgradeViewState.swift in Sources */ = {isa = PBXBuildFile; fileRef = 680BA5992A4C377900F5559D /* UpgradeViewState.swift */; }; 680E36B52BD8B9B900E8BCEA /* OrderSubscriptionTableViewCell.xib in Resources */ = {isa = PBXBuildFile; fileRef = 680E36B42BD8B9B900E8BCEA /* OrderSubscriptionTableViewCell.xib */; }; 680E36B72BD8C49F00E8BCEA /* OrderSubscriptionTableViewCellViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 680E36B62BD8C49F00E8BCEA /* OrderSubscriptionTableViewCellViewModel.swift */; }; 682140AF2E125437005E86AB /* UILabel+SalesChannel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 682140AE2E125430005E86AB /* UILabel+SalesChannel.swift */; }; @@ -5779,6 +5778,7 @@ 3F09040E2D26A40800D8ACCE /* WordPressAuthenticatorTests */ = {isa = PBXFileSystemSynchronizedRootGroup; exceptions = (3FD28D5E2D271391002EBB3D /* PBXFileSystemSynchronizedBuildFileExceptionSet */, ); explicitFileTypes = {}; explicitFolders = (); path = WordPressAuthenticatorTests; sourceTree = ""; }; 3FD9BFBE2E0A2533004A8DC8 /* WooCommerceScreenshots */ = {isa = PBXFileSystemSynchronizedRootGroup; exceptions = (3FD9BFC42E0A2534004A8DC8 /* PBXFileSystemSynchronizedBuildFileExceptionSet */, ); explicitFileTypes = {}; explicitFolders = (); path = WooCommerceScreenshots; sourceTree = ""; }; 646A2C682E9FCD7E003A32A1 /* Routing */ = {isa = PBXFileSystemSynchronizedRootGroup; explicitFileTypes = {}; explicitFolders = (); path = Routing; sourceTree = ""; }; + 6489D8522EA667AC00D96802 /* Routing */ = {isa = PBXFileSystemSynchronizedRootGroup; explicitFileTypes = {}; explicitFolders = (); path = Routing; sourceTree = ""; }; DEDB5D342E7A68950022E5A1 /* Bookings */ = {isa = PBXFileSystemSynchronizedRootGroup; explicitFileTypes = {}; explicitFolders = (); path = Bookings; sourceTree = ""; }; /* End PBXFileSystemSynchronizedRootGroup section */ @@ -9649,6 +9649,7 @@ B56DB3E02049BFAA00D4AA8E /* WooCommerceTests */ = { isa = PBXGroup; children = ( + 6489D8522EA667AC00D96802 /* Routing */, DEB387962C2E80060025256E /* GoogleAds */, DABF35242C11B40C006AF826 /* POS */, B958A7CF28B527FB00823EEF /* Universal Links */, @@ -13276,6 +13277,9 @@ dependencies = ( B56DB3DF2049BFAA00D4AA8E /* PBXTargetDependency */, ); + fileSystemSynchronizedGroups = ( + 6489D8522EA667AC00D96802 /* Routing */, + ); name = WooCommerceTests; packageProductDependencies = ( 3F2B4AEB2DDC319800E5E49C /* XcodeTarget_WooCommerceTests */, diff --git a/WooCommerce/WooCommerceTests/Routing/ProductDetail/ProductURLProviderTests.swift b/WooCommerce/WooCommerceTests/Routing/ProductDetail/ProductURLProviderTests.swift new file mode 100644 index 00000000000..3261a232f1d --- /dev/null +++ b/WooCommerce/WooCommerceTests/Routing/ProductDetail/ProductURLProviderTests.swift @@ -0,0 +1,77 @@ +@testable import WooCommerce +import Testing +import Yosemite + +final class ProductURLProviderTests { + + private let storeURL = "https://nicestore.com" + private let productID: Int64 = 1234567 + private lazy var aProduct = Product.fake().copy(productID: productID) + private lazy var aSite = Site.fake().copy(url: storeURL) + + @Test + private func validateEditAdminURL() async throws { + let url = try #require(ProductURLProvider.editAdminURL(for: aProduct, site: aSite)) + #expect(url.absoluteString == + "\(storeURL)/wp-admin?page=next-admin&p=/woocommerce/products/edit/\(productID)") + } +} + +@MainActor +final class ProductDetailPresenterTests { + private static let aNonBookingProduct = Product.fake().copy(productTypeKey: "simple") + private static let aBookingProduct = Product.fake().copy(productTypeKey: "booking") + private lazy var coordinatorFactory = MockProductDetailCoordinatorFactory() + + @Test(arguments: [ + (isCIABSite: true, product: aNonBookingProduct), + (isCIABSite: false, product: aBookingProduct), + (isCIABSite: false, product: aNonBookingProduct), + ]) + private func regardlessOfCIABSiteWeShouldDirectToNative(isCIABSite: Bool, product: Product) { + let router = ProductDetailPresenter( + ciabChecker: MockCIABEligibilityChecker(mockedIsCurrentSiteCIAB: isCIABSite), + coordinatorFactory: coordinatorFactory + ) + _ = router.viewController(product: product, forceReadOnly: false) + #expect(coordinatorFactory.createdNativeCoordiantor) + #expect(!coordinatorFactory.createdWebCoordiantor) + } + + @Test + private func bookableProductOnCIABSiteWeShouldDirectToWeb() { + let router = ProductDetailPresenter( + ciabChecker: MockCIABEligibilityChecker(mockedIsCurrentSiteCIAB: true), + coordinatorFactory: coordinatorFactory + ) + _ = router.viewController(product: Self.aBookingProduct, forceReadOnly: false) + #expect(coordinatorFactory.createdWebCoordiantor) + #expect(!coordinatorFactory.createdNativeCoordiantor) + } +} + +class MockProductDetailCoordinatorFactory: ProductDetailCoordinatorFactoryProtocol { + private(set) var createdWebCoordiantor = false + private(set) var createdNativeCoordiantor = false + + func webCoordinator() -> ProductDetailCoordinator { + createdWebCoordiantor = true + return MockProductDetailCoordinator() + } + + func nativeCoordinator() -> ProductDetailCoordinator { + createdNativeCoordiantor = true + return MockProductDetailCoordinator() + } +} + +import UIKit + +struct MockProductDetailCoordinator: ProductDetailCoordinator { + func viewController(product: Product, + presentationStyle: ProductDetailPresenter.PresentationStyle, + forceReadOnly: Bool, + onDeleteCompletion: (() -> Void)?) -> UIViewController { + UIViewController() + } +} From 8c2da98205a870bfd5c91434b5f11eba72811a99 Mon Sep 17 00:00:00 2001 From: Adam Borbas Date: Tue, 21 Oct 2025 11:51:36 +0200 Subject: [PATCH 09/15] Improved naming and split to multiple files --- .../ProductDetailCoordinator.swift | 11 +++ .../ProductDetailNativeCoordinator.swift} | 13 +-- .../ProductDetailWebCoordinator.swift} | 11 +-- ...er.swift => ProductAdminURLProvider.swift} | 6 +- ...ductDetailCoordinatorFactoryProtocol.swift | 26 ++++++ .../ProductDetailNavigator.swift | 64 ++++++++++++++ .../ProductDetail/ProductDetailRouter.swift | 84 ------------------- .../ProductLoaderViewController.swift | 6 +- .../ProductsSplitViewCoordinator.swift | 6 +- .../Products/ProductsViewController.swift | 4 +- .../WooCommerce.xcodeproj/project.pbxproj | 8 ++ .../Mocks/MockProductDetailCoordinator.swift | 12 +++ .../MockProductDetailCoordinatorFactory.swift | 16 ++++ .../ProductAdminURLProviderTests.swift | 18 ++++ .../ProductDetailNavigatorTests.swift | 36 ++++++++ .../ProductURLProviderTests.swift | 77 ----------------- 16 files changed, 217 insertions(+), 181 deletions(-) create mode 100644 WooCommerce/Classes/Routing/ProductDetail/Coordinator/ProductDetailCoordinator.swift rename WooCommerce/Classes/Routing/ProductDetail/{NativeProductDetailCoordinator.swift => Coordinator/ProductDetailNativeCoordinator.swift} (51%) rename WooCommerce/Classes/Routing/ProductDetail/{WebViewProductDetailCoordinator.swift => Coordinator/ProductDetailWebCoordinator.swift} (60%) rename WooCommerce/Classes/Routing/ProductDetail/{ProductURLProvider.swift => ProductAdminURLProvider.swift} (70%) create mode 100644 WooCommerce/Classes/Routing/ProductDetail/ProductDetailCoordinatorFactoryProtocol.swift create mode 100644 WooCommerce/Classes/Routing/ProductDetail/ProductDetailNavigator.swift delete mode 100644 WooCommerce/Classes/Routing/ProductDetail/ProductDetailRouter.swift create mode 100644 WooCommerce/WooCommerceTests/Mocks/MockProductDetailCoordinator.swift create mode 100644 WooCommerce/WooCommerceTests/Mocks/MockProductDetailCoordinatorFactory.swift create mode 100644 WooCommerce/WooCommerceTests/Routing/ProductDetail/ProductAdminURLProviderTests.swift create mode 100644 WooCommerce/WooCommerceTests/Routing/ProductDetail/ProductDetailNavigatorTests.swift delete mode 100644 WooCommerce/WooCommerceTests/Routing/ProductDetail/ProductURLProviderTests.swift diff --git a/WooCommerce/Classes/Routing/ProductDetail/Coordinator/ProductDetailCoordinator.swift b/WooCommerce/Classes/Routing/ProductDetail/Coordinator/ProductDetailCoordinator.swift new file mode 100644 index 00000000000..4fee8308a61 --- /dev/null +++ b/WooCommerce/Classes/Routing/ProductDetail/Coordinator/ProductDetailCoordinator.swift @@ -0,0 +1,11 @@ +import UIKit +import Yosemite + +/// Abstraction for building the concrete destination VC for a product detail flow. +protocol ProductDetailCoordinator { + func viewController( + product: Product, + presentationStyle: ProductDetailNavigator.Presentation, + isReadOnly: Bool, + onDelete: (() -> Void)?) -> UIViewController +} diff --git a/WooCommerce/Classes/Routing/ProductDetail/NativeProductDetailCoordinator.swift b/WooCommerce/Classes/Routing/ProductDetail/Coordinator/ProductDetailNativeCoordinator.swift similarity index 51% rename from WooCommerce/Classes/Routing/ProductDetail/NativeProductDetailCoordinator.swift rename to WooCommerce/Classes/Routing/ProductDetail/Coordinator/ProductDetailNativeCoordinator.swift index 4923dd7822f..7aa07c76c35 100644 --- a/WooCommerce/Classes/Routing/ProductDetail/NativeProductDetailCoordinator.swift +++ b/WooCommerce/Classes/Routing/ProductDetail/Coordinator/ProductDetailNativeCoordinator.swift @@ -1,15 +1,18 @@ import Yosemite import UIKit -class NativeProductDetailCoordinator: ProductDetailCoordinator { +/// Coordinator for the **native** product detail/editor flow. +/// Delegates VC construction to `ProductDetailsFactory` and applies the requested presentation style. +class ProductDetailNativeCoordinator: ProductDetailCoordinator { + func viewController( product: Product, - presentationStyle: ProductDetailPresenter.PresentationStyle, - forceReadOnly: Bool, - onDeleteCompletion: (() -> Void)? = nil) -> UIViewController { + presentationStyle: ProductDetailNavigator.Presentation, + isReadOnly: Bool, + onDelete: (() -> Void)? = nil) -> UIViewController { return ProductDetailsFactory.productDetails(product: product, presentationStyle: presentationStyle.asProductFormPresentationStyle, forceReadOnly: false, - onDeleteCompletion: onDeleteCompletion ?? {}) + onDeleteCompletion: onDelete ?? {}) } } diff --git a/WooCommerce/Classes/Routing/ProductDetail/WebViewProductDetailCoordinator.swift b/WooCommerce/Classes/Routing/ProductDetail/Coordinator/ProductDetailWebCoordinator.swift similarity index 60% rename from WooCommerce/Classes/Routing/ProductDetail/WebViewProductDetailCoordinator.swift rename to WooCommerce/Classes/Routing/ProductDetail/Coordinator/ProductDetailWebCoordinator.swift index abb50efff50..e0a74d38a70 100644 --- a/WooCommerce/Classes/Routing/ProductDetail/WebViewProductDetailCoordinator.swift +++ b/WooCommerce/Classes/Routing/ProductDetail/Coordinator/ProductDetailWebCoordinator.swift @@ -1,7 +1,8 @@ import UIKit import Yosemite -final class WebViewProductDetailCoordinator: NSObject, ProductDetailCoordinator { +/// Coordinator for the **admin web** product detail/editor flow. +final class ProductDetailWebCoordinator: NSObject, ProductDetailCoordinator { private var onDismiss: (() -> Void)? private let site: Site @@ -11,10 +12,10 @@ final class WebViewProductDetailCoordinator: NSObject, ProductDetailCoordinator } func viewController(product: Product, - presentationStyle: ProductDetailPresenter.PresentationStyle, - forceReadOnly: Bool, - onDeleteCompletion: (() -> Void)? = nil) -> UIViewController { - guard let url = ProductURLProvider.editAdminURL(for: product, site: site) else { + presentationStyle: ProductDetailNavigator.Presentation, + isReadOnly: Bool, + onDelete: (() -> Void)? = nil) -> UIViewController { + guard let url = ProductAdminURLProvider.editURL(for: product, site: site) else { return UIViewController() } diff --git a/WooCommerce/Classes/Routing/ProductDetail/ProductURLProvider.swift b/WooCommerce/Classes/Routing/ProductDetail/ProductAdminURLProvider.swift similarity index 70% rename from WooCommerce/Classes/Routing/ProductDetail/ProductURLProvider.swift rename to WooCommerce/Classes/Routing/ProductDetail/ProductAdminURLProvider.swift index 7648d1f4984..ee45e52a27c 100644 --- a/WooCommerce/Classes/Routing/ProductDetail/ProductURLProvider.swift +++ b/WooCommerce/Classes/Routing/ProductDetail/ProductAdminURLProvider.swift @@ -1,8 +1,10 @@ import Yosemite import Foundation -enum ProductURLProvider { - static func editAdminURL(for product: Product, site: Site) -> URL? { +/// Builds canonical admin edit URLs for products. Treat returned URLs as opaque. +enum ProductAdminURLProvider { + + static func editURL(for product: Product, site: Site) -> URL? { guard let base = site.adminURLWithFallback() else { return nil } var components = URLComponents(url: base, resolvingAgainstBaseURL: false)! diff --git a/WooCommerce/Classes/Routing/ProductDetail/ProductDetailCoordinatorFactoryProtocol.swift b/WooCommerce/Classes/Routing/ProductDetail/ProductDetailCoordinatorFactoryProtocol.swift new file mode 100644 index 00000000000..fb7993fb15c --- /dev/null +++ b/WooCommerce/Classes/Routing/ProductDetail/ProductDetailCoordinatorFactoryProtocol.swift @@ -0,0 +1,26 @@ +import Yosemite + +/// Factory for producing coordinators used by the navigator. +protocol ProductDetailCoordinatorFactoryProtocol { + func webCoordinator() -> ProductDetailCoordinator + func nativeCoordinator() -> ProductDetailCoordinator +} + +/// Default coordinator factory that wires production dependencies. +class ProductDetailCoordinatorFactory: ProductDetailCoordinatorFactoryProtocol { + static let `default` = ProductDetailCoordinatorFactory() + + private let stores: StoresManager + + init(stores: StoresManager = ServiceLocator.stores) { + self.stores = stores + } + + func webCoordinator() -> ProductDetailCoordinator { + return ProductDetailWebCoordinator(site: stores.sessionManager.defaultSite!) + } + + func nativeCoordinator() -> ProductDetailCoordinator { + return ProductDetailNativeCoordinator() + } +} diff --git a/WooCommerce/Classes/Routing/ProductDetail/ProductDetailNavigator.swift b/WooCommerce/Classes/Routing/ProductDetail/ProductDetailNavigator.swift new file mode 100644 index 00000000000..fc316947a87 --- /dev/null +++ b/WooCommerce/Classes/Routing/ProductDetail/ProductDetailNavigator.swift @@ -0,0 +1,64 @@ +import UIKit +import Yosemite + +/// Decides between **native** and **admin web** product detail flows and builds the destination VC. +final class ProductDetailNavigator { + + /// Describes how the **native** destination should be presented. + enum Presentation { + case push + case contained(in: () -> UIViewController?) + + var asProductFormPresentationStyle: ProductFormPresentationStyle { + switch self { + case .contained(let inVC): + .contained(containerViewController: inVC) + case .push: + .navigationStack + } + } + } + + /// Convenience singleton for places that cannot inject dependencies easily. + static var shared = ProductDetailNavigator() + + init(ciabChecker: CIABEligibilityCheckerProtocol = CIABEligibilityChecker(), + coordinatorFactory: ProductDetailCoordinatorFactoryProtocol = ProductDetailCoordinatorFactory.default) { + self.ciabChecker = ciabChecker + self.coordinatorFactory = coordinatorFactory + } + + /// Builds the destination `UIViewController` for the given product. + /// - Parameters: + /// - product: The product to display. + /// - presentationStyle: How to present **native** detail (ignored for web). + /// - isReadOnly: Whether the native screen should be read-only. + /// - onDelete: Optional callback invoked after a successful product delete. + /// - Returns: A ready-to-present view controller (native or web). + func makeDestination(product: Product, + presentationStyle: Presentation = .push, + isReadOnly: Bool, + onDelete: (() -> Void)? = nil) -> UIViewController { + + let coordinator: ProductDetailCoordinator + if shouldOpenInWeb(product: product) { + coordinator = coordinatorFactory.webCoordinator() + } else { + coordinator = coordinatorFactory.nativeCoordinator() + } + + let viewController = coordinator.viewController(product: product, + presentationStyle: presentationStyle, + isReadOnly: isReadOnly, + onDelete: onDelete) + + return viewController + } + + private func shouldOpenInWeb(product: Product) -> Bool { + return ciabChecker.isCurrentSiteCIAB && product.productType == .booking + } + + private let ciabChecker: CIABEligibilityCheckerProtocol + private let coordinatorFactory: ProductDetailCoordinatorFactoryProtocol +} diff --git a/WooCommerce/Classes/Routing/ProductDetail/ProductDetailRouter.swift b/WooCommerce/Classes/Routing/ProductDetail/ProductDetailRouter.swift deleted file mode 100644 index 6130951d724..00000000000 --- a/WooCommerce/Classes/Routing/ProductDetail/ProductDetailRouter.swift +++ /dev/null @@ -1,84 +0,0 @@ -import UIKit -import Yosemite - -final class ProductDetailPresenter { - enum PresentationStyle { - case undefined - case contained(containerViewController: () -> UIViewController?) - - var asProductFormPresentationStyle: ProductFormPresentationStyle { - switch self { - case .contained(let containerViewController): - .contained(containerViewController: containerViewController) - case .undefined: - .navigationStack - } - } - } - - static var shared = ProductDetailPresenter() - - private let ciabChecker: CIABEligibilityCheckerProtocol - private let coordinatorFactory: ProductDetailCoordinatorFactoryProtocol - - init(ciabChecker: CIABEligibilityCheckerProtocol = CIABEligibilityChecker(), - coordinatorFactory: ProductDetailCoordinatorFactoryProtocol = ProductDetailCoordinatorFactory.default) { - self.ciabChecker = ciabChecker - self.coordinatorFactory = coordinatorFactory - } - - func viewController(product: Product, - presentationStyle: PresentationStyle = .undefined, - forceReadOnly: Bool, - onDeleteCompletion: (() -> Void)? = nil) -> UIViewController { - - let coordinator: ProductDetailCoordinator - if shouldOpenInWeb(product: product) { - coordinator = coordinatorFactory.webCoordinator() - } else { - coordinator = coordinatorFactory.nativeCoordinator() - } - - let viewController = coordinator.viewController(product: product, - presentationStyle: presentationStyle, - forceReadOnly: forceReadOnly, - onDeleteCompletion: onDeleteCompletion) - - return viewController - } - - private func shouldOpenInWeb(product: Product) -> Bool { - return ciabChecker.isCurrentSiteCIAB && product.productType == .booking - } -} - -protocol ProductDetailCoordinator { - func viewController( - product: Product, - presentationStyle: ProductDetailPresenter.PresentationStyle, - forceReadOnly: Bool, - onDeleteCompletion: (() -> Void)?) -> UIViewController -} - -protocol ProductDetailCoordinatorFactoryProtocol { - func webCoordinator() -> ProductDetailCoordinator - func nativeCoordinator() -> ProductDetailCoordinator -} - -class ProductDetailCoordinatorFactory: ProductDetailCoordinatorFactoryProtocol { - static let `default` = ProductDetailCoordinatorFactory() - - private let stores: StoresManager - - init(stores: StoresManager = ServiceLocator.stores) { - self.stores = stores - } - - func webCoordinator() -> ProductDetailCoordinator { - return WebViewProductDetailCoordinator(site: stores.sessionManager.defaultSite!) - } - - func nativeCoordinator() -> ProductDetailCoordinator { - return NativeProductDetailCoordinator() - } -} diff --git a/WooCommerce/Classes/ViewRelated/Products/Product Loader/ProductLoaderViewController.swift b/WooCommerce/Classes/ViewRelated/Products/Product Loader/ProductLoaderViewController.swift index 2cfcf35b0be..6a1ba22e9bb 100644 --- a/WooCommerce/Classes/ViewRelated/Products/Product Loader/ProductLoaderViewController.swift +++ b/WooCommerce/Classes/ViewRelated/Products/Product Loader/ProductLoaderViewController.swift @@ -227,9 +227,9 @@ private extension ProductLoaderViewController { /// Presents the ProductFormViewController, as a childViewController, for a given Product. /// func presentProductDetails(for product: Product) { - let viewController = ProductDetailPresenter.shared.viewController(product: product, - presentationStyle: .contained(containerViewController: { [weak self] in self }), - forceReadOnly: forceReadOnly) + let viewController = ProductDetailNavigator.shared.makeDestination(product: product, + presentationStyle: .contained(in: { [weak self] in self }), + isReadOnly: forceReadOnly) attachProductDetailsChildViewController(viewController) } diff --git a/WooCommerce/Classes/ViewRelated/Products/ProductsSplitViewCoordinator.swift b/WooCommerce/Classes/ViewRelated/Products/ProductsSplitViewCoordinator.swift index 1a3be5be4d0..1e769371d70 100644 --- a/WooCommerce/Classes/ViewRelated/Products/ProductsSplitViewCoordinator.swift +++ b/WooCommerce/Classes/ViewRelated/Products/ProductsSplitViewCoordinator.swift @@ -122,9 +122,9 @@ private extension ProductsSplitViewCoordinator { } func showProductForm(product: Product) { - let viewController = ProductDetailPresenter.shared.viewController(product: product, - forceReadOnly: false, - onDeleteCompletion: { [weak self] in + let viewController = ProductDetailNavigator.shared.makeDestination(product: product, + isReadOnly: false, + onDelete: { [weak self] in self?.onSecondaryProductFormDeletion() }) diff --git a/WooCommerce/Classes/ViewRelated/Products/ProductsViewController.swift b/WooCommerce/Classes/ViewRelated/Products/ProductsViewController.swift index 8e30c2abd78..4966799c5a2 100644 --- a/WooCommerce/Classes/ViewRelated/Products/ProductsViewController.swift +++ b/WooCommerce/Classes/ViewRelated/Products/ProductsViewController.swift @@ -1214,8 +1214,8 @@ extension ProductsViewController: UITableViewDelegate { private extension ProductsViewController { func didSelectProduct(product: Product) { guard isSplitViewEnabled else { - let viewController = ProductDetailPresenter.shared.viewController(product: product, - forceReadOnly: false) + let viewController = ProductDetailNavigator.shared.makeDestination(product: product, + isReadOnly: false) navigationController?.pushViewController(viewController, animated: true) return } diff --git a/WooCommerce/WooCommerce.xcodeproj/project.pbxproj b/WooCommerce/WooCommerce.xcodeproj/project.pbxproj index 6f4a3c8f87a..c22239ec58d 100644 --- a/WooCommerce/WooCommerce.xcodeproj/project.pbxproj +++ b/WooCommerce/WooCommerce.xcodeproj/project.pbxproj @@ -1338,6 +1338,8 @@ 57F2C6CD246DECC10074063B /* SummaryTableViewCellViewModelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 57F2C6CC246DECC10074063B /* SummaryTableViewCellViewModelTests.swift */; }; 57F42E40253768D600EA87F7 /* TitleAndEditableValueTableViewCellViewModelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 57F42E3F253768D600EA87F7 /* TitleAndEditableValueTableViewCellViewModelTests.swift */; }; 640DA3482E97DE4F00317FB2 /* SceneDelegate.swift in Sources */ = {isa = PBXBuildFile; fileRef = 640DA3472E97DE4F00317FB2 /* SceneDelegate.swift */; }; + 6489DFFF2EA78E0D00D96802 /* MockProductDetailCoordinatorFactory.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6489DFFE2EA78E0D00D96802 /* MockProductDetailCoordinatorFactory.swift */; }; + 6489E0012EA78E2D00D96802 /* MockProductDetailCoordinator.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6489E0002EA78E2D00D96802 /* MockProductDetailCoordinator.swift */; }; 64D355A52E99048E005F53F7 /* TestingSceneDelegate.swift in Sources */ = {isa = PBXBuildFile; fileRef = 64D355A42E99048E005F53F7 /* TestingSceneDelegate.swift */; }; 68051E1E2E9DFE5500228196 /* POSNotificationSchedulerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 68051E1D2E9DFE5100228196 /* POSNotificationSchedulerTests.swift */; }; 680E36B52BD8B9B900E8BCEA /* OrderSubscriptionTableViewCell.xib in Resources */ = {isa = PBXBuildFile; fileRef = 680E36B42BD8B9B900E8BCEA /* OrderSubscriptionTableViewCell.xib */; }; @@ -4227,6 +4229,8 @@ 57F2C6CC246DECC10074063B /* SummaryTableViewCellViewModelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SummaryTableViewCellViewModelTests.swift; sourceTree = ""; }; 57F42E3F253768D600EA87F7 /* TitleAndEditableValueTableViewCellViewModelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TitleAndEditableValueTableViewCellViewModelTests.swift; sourceTree = ""; }; 640DA3472E97DE4F00317FB2 /* SceneDelegate.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SceneDelegate.swift; sourceTree = ""; }; + 6489DFFE2EA78E0D00D96802 /* MockProductDetailCoordinatorFactory.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockProductDetailCoordinatorFactory.swift; sourceTree = ""; }; + 6489E0002EA78E2D00D96802 /* MockProductDetailCoordinator.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockProductDetailCoordinator.swift; sourceTree = ""; }; 64D355A42E99048E005F53F7 /* TestingSceneDelegate.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TestingSceneDelegate.swift; sourceTree = ""; }; 68051E1D2E9DFE5100228196 /* POSNotificationSchedulerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = POSNotificationSchedulerTests.swift; sourceTree = ""; }; 680BA5992A4C377900F5559D /* UpgradeViewState.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UpgradeViewState.swift; sourceTree = ""; }; @@ -8941,6 +8945,8 @@ 746791642108D853007CF1DC /* Mocks */ = { isa = PBXGroup; children = ( + 6489E0002EA78E2D00D96802 /* MockProductDetailCoordinator.swift */, + 6489DFFE2EA78E0D00D96802 /* MockProductDetailCoordinatorFactory.swift */, 02F5DE602E852909002DEE24 /* MockPOSTabVisibilityChecker.swift */, 2DE9DDFA2E6EF4A300155408 /* MockCIABEligibilityChecker.swift */, 022900892E3019020028F6D7 /* MockPluginsService.swift */, @@ -15919,6 +15925,7 @@ 02038C612AF222D600CD36D9 /* ConfigurableVariableBundleAttributePickerViewModelTests.swift in Sources */, CE56C01E2D2431C000EBDE24 /* WooShippingOriginAddressListViewModelTests.swift in Sources */, DA3F99BA2C92F6D30034BDA5 /* MarkOrderAsReadUseCaseTests.swift in Sources */, + 6489E0012EA78E2D00D96802 /* MockProductDetailCoordinator.swift in Sources */, 26C0D1E32B460E5700F6EDA5 /* OrderNotificationViewModel.swift in Sources */, 86E40AED2B597DEC00990365 /* BlazeCampaignCreationCoordinatorTests.swift in Sources */, 20A3AFE12B0F750B0033AF2D /* MockInPersonPaymentsCashOnDeliveryToggleRowViewModel.swift in Sources */, @@ -16172,6 +16179,7 @@ 03F8D87D2A7A76DE00DD6D2F /* MockCardPresentPaymentPreflightController.swift in Sources */, 02524A5D252ED5C60033E7BD /* ProductVariationLoadUseCaseTests.swift in Sources */, 028AFFB62484EDA000693C09 /* Dictionary+LoggingTests.swift in Sources */, + 6489DFFF2EA78E0D00D96802 /* MockProductDetailCoordinatorFactory.swift in Sources */, AEA3F91527BEC96B00B9F555 /* PriceFieldFormatterTests.swift in Sources */, 6879B8DB287AFFA100A0F9A8 /* CardReaderManualsViewModelTests.swift in Sources */, DE58D0042D8A6F85005914DF /* NotificationSettingsViewModelTests.swift in Sources */, diff --git a/WooCommerce/WooCommerceTests/Mocks/MockProductDetailCoordinator.swift b/WooCommerce/WooCommerceTests/Mocks/MockProductDetailCoordinator.swift new file mode 100644 index 00000000000..ab35a16f9b2 --- /dev/null +++ b/WooCommerce/WooCommerceTests/Mocks/MockProductDetailCoordinator.swift @@ -0,0 +1,12 @@ +import UIKit +import Yosemite +@testable import WooCommerce + +struct MockProductDetailCoordinator: ProductDetailCoordinator { + func viewController(product: Product, + presentationStyle: ProductDetailNavigator.Presentation, + isReadOnly: Bool, + onDelete: (() -> Void)?) -> UIViewController { + UIViewController() + } +} diff --git a/WooCommerce/WooCommerceTests/Mocks/MockProductDetailCoordinatorFactory.swift b/WooCommerce/WooCommerceTests/Mocks/MockProductDetailCoordinatorFactory.swift new file mode 100644 index 00000000000..770edbe3a6c --- /dev/null +++ b/WooCommerce/WooCommerceTests/Mocks/MockProductDetailCoordinatorFactory.swift @@ -0,0 +1,16 @@ +@testable import WooCommerce + +class MockProductDetailCoordinatorFactory: ProductDetailCoordinatorFactoryProtocol { + private(set) var createdWebCoordiantor = false + private(set) var createdNativeCoordiantor = false + + func webCoordinator() -> ProductDetailCoordinator { + createdWebCoordiantor = true + return MockProductDetailCoordinator() + } + + func nativeCoordinator() -> ProductDetailCoordinator { + createdNativeCoordiantor = true + return MockProductDetailCoordinator() + } +} diff --git a/WooCommerce/WooCommerceTests/Routing/ProductDetail/ProductAdminURLProviderTests.swift b/WooCommerce/WooCommerceTests/Routing/ProductDetail/ProductAdminURLProviderTests.swift new file mode 100644 index 00000000000..702a7a9e1a2 --- /dev/null +++ b/WooCommerce/WooCommerceTests/Routing/ProductDetail/ProductAdminURLProviderTests.swift @@ -0,0 +1,18 @@ +@testable import WooCommerce +import Testing +import Yosemite + +final class ProductAdminURLProviderTests { + + private let storeURL = "https://nicestore.com" + private let productID: Int64 = 1234567 + private lazy var aProduct = Product.fake().copy(productID: productID) + private lazy var aSite = Site.fake().copy(url: storeURL) + + @Test + private func validateEditAdminURL() async throws { + let url = try #require(ProductAdminURLProvider.editURL(for: aProduct, site: aSite)) + #expect(url.absoluteString == + "\(storeURL)/wp-admin?page=next-admin&p=/woocommerce/products/edit/\(productID)") + } +} diff --git a/WooCommerce/WooCommerceTests/Routing/ProductDetail/ProductDetailNavigatorTests.swift b/WooCommerce/WooCommerceTests/Routing/ProductDetail/ProductDetailNavigatorTests.swift new file mode 100644 index 00000000000..38136918dbc --- /dev/null +++ b/WooCommerce/WooCommerceTests/Routing/ProductDetail/ProductDetailNavigatorTests.swift @@ -0,0 +1,36 @@ +@testable import WooCommerce +import Testing +import Yosemite + +@MainActor +final class ProductDetailNavigatorTests { + private static let aNonBookingProduct = Product.fake().copy(productTypeKey: "simple") + private static let aBookingProduct = Product.fake().copy(productTypeKey: "booking") + private lazy var coordinatorFactory = MockProductDetailCoordinatorFactory() + + @Test(arguments: [ + (isCIABSite: true, product: aNonBookingProduct), + (isCIABSite: false, product: aBookingProduct), + (isCIABSite: false, product: aNonBookingProduct), + ]) + private func regardlessOfCIABSiteWeShouldDirectToNative(isCIABSite: Bool, product: Product) { + let navigator = ProductDetailNavigator( + ciabChecker: MockCIABEligibilityChecker(mockedIsCurrentSiteCIAB: isCIABSite), + coordinatorFactory: coordinatorFactory + ) + _ = navigator.makeDestination(product: product, isReadOnly: false) + #expect(coordinatorFactory.createdNativeCoordiantor) + #expect(!coordinatorFactory.createdWebCoordiantor) + } + + @Test + private func bookableProductOnCIABSiteWeShouldDirectToWeb() { + let navigator = ProductDetailNavigator( + ciabChecker: MockCIABEligibilityChecker(mockedIsCurrentSiteCIAB: true), + coordinatorFactory: coordinatorFactory + ) + _ = navigator.makeDestination(product: Self.aBookingProduct, isReadOnly: false) + #expect(coordinatorFactory.createdWebCoordiantor) + #expect(!coordinatorFactory.createdNativeCoordiantor) + } +} diff --git a/WooCommerce/WooCommerceTests/Routing/ProductDetail/ProductURLProviderTests.swift b/WooCommerce/WooCommerceTests/Routing/ProductDetail/ProductURLProviderTests.swift deleted file mode 100644 index 3261a232f1d..00000000000 --- a/WooCommerce/WooCommerceTests/Routing/ProductDetail/ProductURLProviderTests.swift +++ /dev/null @@ -1,77 +0,0 @@ -@testable import WooCommerce -import Testing -import Yosemite - -final class ProductURLProviderTests { - - private let storeURL = "https://nicestore.com" - private let productID: Int64 = 1234567 - private lazy var aProduct = Product.fake().copy(productID: productID) - private lazy var aSite = Site.fake().copy(url: storeURL) - - @Test - private func validateEditAdminURL() async throws { - let url = try #require(ProductURLProvider.editAdminURL(for: aProduct, site: aSite)) - #expect(url.absoluteString == - "\(storeURL)/wp-admin?page=next-admin&p=/woocommerce/products/edit/\(productID)") - } -} - -@MainActor -final class ProductDetailPresenterTests { - private static let aNonBookingProduct = Product.fake().copy(productTypeKey: "simple") - private static let aBookingProduct = Product.fake().copy(productTypeKey: "booking") - private lazy var coordinatorFactory = MockProductDetailCoordinatorFactory() - - @Test(arguments: [ - (isCIABSite: true, product: aNonBookingProduct), - (isCIABSite: false, product: aBookingProduct), - (isCIABSite: false, product: aNonBookingProduct), - ]) - private func regardlessOfCIABSiteWeShouldDirectToNative(isCIABSite: Bool, product: Product) { - let router = ProductDetailPresenter( - ciabChecker: MockCIABEligibilityChecker(mockedIsCurrentSiteCIAB: isCIABSite), - coordinatorFactory: coordinatorFactory - ) - _ = router.viewController(product: product, forceReadOnly: false) - #expect(coordinatorFactory.createdNativeCoordiantor) - #expect(!coordinatorFactory.createdWebCoordiantor) - } - - @Test - private func bookableProductOnCIABSiteWeShouldDirectToWeb() { - let router = ProductDetailPresenter( - ciabChecker: MockCIABEligibilityChecker(mockedIsCurrentSiteCIAB: true), - coordinatorFactory: coordinatorFactory - ) - _ = router.viewController(product: Self.aBookingProduct, forceReadOnly: false) - #expect(coordinatorFactory.createdWebCoordiantor) - #expect(!coordinatorFactory.createdNativeCoordiantor) - } -} - -class MockProductDetailCoordinatorFactory: ProductDetailCoordinatorFactoryProtocol { - private(set) var createdWebCoordiantor = false - private(set) var createdNativeCoordiantor = false - - func webCoordinator() -> ProductDetailCoordinator { - createdWebCoordiantor = true - return MockProductDetailCoordinator() - } - - func nativeCoordinator() -> ProductDetailCoordinator { - createdNativeCoordiantor = true - return MockProductDetailCoordinator() - } -} - -import UIKit - -struct MockProductDetailCoordinator: ProductDetailCoordinator { - func viewController(product: Product, - presentationStyle: ProductDetailPresenter.PresentationStyle, - forceReadOnly: Bool, - onDeleteCompletion: (() -> Void)?) -> UIViewController { - UIViewController() - } -} From 0a92483710d2393d05ebcf6b379cf059b924bafb Mon Sep 17 00:00:00 2001 From: Adam Borbas Date: Tue, 21 Oct 2025 12:32:42 +0200 Subject: [PATCH 10/15] Removed unused code --- .../ProductAdminURLProvider.swift | 2 +- .../ProductDetailNavigator.swift | 8 +- .../Products/ProductDetailsFactory.swift | 24 ----- .../Products/ProductDetailsFactoryTests.swift | 87 +++++++------------ 4 files changed, 37 insertions(+), 84 deletions(-) diff --git a/WooCommerce/Classes/Routing/ProductDetail/ProductAdminURLProvider.swift b/WooCommerce/Classes/Routing/ProductDetail/ProductAdminURLProvider.swift index ee45e52a27c..abfa356489c 100644 --- a/WooCommerce/Classes/Routing/ProductDetail/ProductAdminURLProvider.swift +++ b/WooCommerce/Classes/Routing/ProductDetail/ProductAdminURLProvider.swift @@ -1,7 +1,7 @@ import Yosemite import Foundation -/// Builds canonical admin edit URLs for products. Treat returned URLs as opaque. +/// Builds canonical admin edit URLs for products. enum ProductAdminURLProvider { static func editURL(for product: Product, site: Site) -> URL? { diff --git a/WooCommerce/Classes/Routing/ProductDetail/ProductDetailNavigator.swift b/WooCommerce/Classes/Routing/ProductDetail/ProductDetailNavigator.swift index fc316947a87..fd9f4541c2d 100644 --- a/WooCommerce/Classes/Routing/ProductDetail/ProductDetailNavigator.swift +++ b/WooCommerce/Classes/Routing/ProductDetail/ProductDetailNavigator.swift @@ -3,7 +3,6 @@ import Yosemite /// Decides between **native** and **admin web** product detail flows and builds the destination VC. final class ProductDetailNavigator { - /// Describes how the **native** destination should be presented. enum Presentation { case push @@ -19,9 +18,11 @@ final class ProductDetailNavigator { } } - /// Convenience singleton for places that cannot inject dependencies easily. static var shared = ProductDetailNavigator() + private let ciabChecker: CIABEligibilityCheckerProtocol + private let coordinatorFactory: ProductDetailCoordinatorFactoryProtocol + init(ciabChecker: CIABEligibilityCheckerProtocol = CIABEligibilityChecker(), coordinatorFactory: ProductDetailCoordinatorFactoryProtocol = ProductDetailCoordinatorFactory.default) { self.ciabChecker = ciabChecker @@ -58,7 +59,4 @@ final class ProductDetailNavigator { private func shouldOpenInWeb(product: Product) -> Bool { return ciabChecker.isCurrentSiteCIAB && product.productType == .booking } - - private let ciabChecker: CIABEligibilityCheckerProtocol - private let coordinatorFactory: ProductDetailCoordinatorFactoryProtocol } diff --git a/WooCommerce/Classes/ViewRelated/Products/ProductDetailsFactory.swift b/WooCommerce/Classes/ViewRelated/Products/ProductDetailsFactory.swift index 1df9b8eb3d3..420c834e05f 100644 --- a/WooCommerce/Classes/ViewRelated/Products/ProductDetailsFactory.swift +++ b/WooCommerce/Classes/ViewRelated/Products/ProductDetailsFactory.swift @@ -3,30 +3,6 @@ import Yosemite import WooFoundation struct ProductDetailsFactory { - /// Creates a product details view controller asynchronously based on the app settings. - /// - Parameters: - /// - product: product model. - /// - presentationStyle: how the product details are presented. - /// - currencySettings: site currency settings. - /// - forceReadOnly: force the product detail to be presented in read only mode - /// - onDeleteCompletion: called when the product deletion completes in the product form. - /// - onCompletion: called when the view controller is created and ready for display. - static func productDetails(product: Product, - presentationStyle: ProductFormPresentationStyle, - currencySettings: CurrencySettings = ServiceLocator.currencySettings, - forceReadOnly: Bool, - productImageUploader: ProductImageUploaderProtocol = ServiceLocator.productImageUploader, - onDeleteCompletion: @escaping () -> Void = {}, - onCompletion: @escaping (UIViewController) -> Void) { - let vc = productDetails(product: product, - presentationStyle: presentationStyle, - currencySettings: currencySettings, - isEditProductsEnabled: forceReadOnly ? false: true, - productImageUploader: productImageUploader, - onDeleteCompletion: onDeleteCompletion) - onCompletion(vc) - } - /// Creates a product details view controller asynchronously based on the app settings. /// - Parameters: /// - product: product model. diff --git a/WooCommerce/WooCommerceTests/ViewRelated/Products/ProductDetailsFactoryTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/Products/ProductDetailsFactoryTests.swift index c035dd480b4..29ede5711dc 100644 --- a/WooCommerce/WooCommerceTests/ViewRelated/Products/ProductDetailsFactoryTests.swift +++ b/WooCommerce/WooCommerceTests/ViewRelated/Products/ProductDetailsFactoryTests.swift @@ -10,16 +10,13 @@ final class ProductDetailsFactoryTests: XCTestCase { // Arrange let product = Product.fake().copy(productTypeKey: ProductType.simple.rawValue) - let exp = expectation(description: #function) // Action - ProductDetailsFactory.productDetails(product: product, - presentationStyle: .navigationStack, - forceReadOnly: false) { viewController in - // Assert - XCTAssertTrue(viewController is ProductFormViewController) - exp.fulfill() - } - waitForExpectations(timeout: Constants.expectationTimeout, handler: nil) + let viewController = ProductDetailsFactory.productDetails(product: product, + presentationStyle: .navigationStack, + forceReadOnly: false) + + // Assert + XCTAssertTrue(viewController is ProductFormViewController) } // MARK: External/affiliate product type @@ -27,17 +24,14 @@ final class ProductDetailsFactoryTests: XCTestCase { func test_factory_creates_product_form_for_affiliate_product() { // Arrange let product = Product.fake().copy(productTypeKey: ProductType.affiliate.rawValue) - let exp = expectation(description: #function) // Action - ProductDetailsFactory.productDetails(product: product, - presentationStyle: .navigationStack, - forceReadOnly: false) { viewController in - // Assert - XCTAssertTrue(viewController is ProductFormViewController) - exp.fulfill() - } - waitForExpectations(timeout: Constants.expectationTimeout, handler: nil) + let viewController = ProductDetailsFactory.productDetails(product: product, + presentationStyle: .navigationStack, + forceReadOnly: false) + + // Assert + XCTAssertTrue(viewController is ProductFormViewController) } // MARK: Grouped product type @@ -45,17 +39,13 @@ final class ProductDetailsFactoryTests: XCTestCase { func test_factory_creates_product_form_for_grouped_product() { // Arrange let product = Product.fake().copy(productTypeKey: ProductType.grouped.rawValue) - let exp = expectation(description: #function) // Action - ProductDetailsFactory.productDetails(product: product, - presentationStyle: .navigationStack, - forceReadOnly: false) { viewController in - // Assert - XCTAssertTrue(viewController is ProductFormViewController) - exp.fulfill() - } - waitForExpectations(timeout: Constants.expectationTimeout, handler: nil) + let viewController = ProductDetailsFactory.productDetails(product: product, + presentationStyle: .navigationStack, + forceReadOnly: false) + // Assert + XCTAssertTrue(viewController is ProductFormViewController) } // MARK: Variable product type @@ -63,17 +53,14 @@ final class ProductDetailsFactoryTests: XCTestCase { func test_factory_creates_product_form_for_variable_product() { // Arrange let product = Product.fake().copy(productTypeKey: ProductType.variable.rawValue) - let exp = expectation(description: #function) // Action - ProductDetailsFactory.productDetails(product: product, - presentationStyle: .navigationStack, - forceReadOnly: false) { viewController in - // Assert - XCTAssertTrue(viewController is ProductFormViewController) - exp.fulfill() - } - waitForExpectations(timeout: Constants.expectationTimeout, handler: nil) + let viewController = ProductDetailsFactory.productDetails(product: product, + presentationStyle: .navigationStack, + forceReadOnly: false) + + // Assert + XCTAssertTrue(viewController is ProductFormViewController) } // MARK: Non-core product type @@ -83,15 +70,11 @@ final class ProductDetailsFactoryTests: XCTestCase { let product = Product.fake().copy(productTypeKey: "other") // Action - waitForExpectation { expectation in - ProductDetailsFactory.productDetails(product: product, - presentationStyle: .navigationStack, - forceReadOnly: false) { viewController in - // Assert - XCTAssertTrue(viewController is ProductFormViewController) - expectation.fulfill() - } - } + let viewController = ProductDetailsFactory.productDetails(product: product, + presentationStyle: .navigationStack, + forceReadOnly: false) + // Assert + XCTAssertTrue(viewController is ProductFormViewController) } func test_factory_creates_readonly_product_details_for_product_when_forceReadOnly_is_on() { @@ -99,14 +82,10 @@ final class ProductDetailsFactoryTests: XCTestCase { let product = Product.fake().copy(productTypeKey: ProductType.simple.rawValue) // Action - waitForExpectation { expectation in - ProductDetailsFactory.productDetails(product: product, - presentationStyle: .navigationStack, - forceReadOnly: true) { viewController in - // Assert - XCTAssertTrue(viewController is ProductFormViewController) - expectation.fulfill() - } - } + let viewController = ProductDetailsFactory.productDetails(product: product, + presentationStyle: .navigationStack, + forceReadOnly: true) + // Assert + XCTAssertTrue(viewController is ProductFormViewController) } } From 99663cc056550e2915a9c3e03487c2b8a314c023 Mon Sep 17 00:00:00 2001 From: Adam Borbas Date: Wed, 22 Oct 2025 10:58:52 +0200 Subject: [PATCH 11/15] Handle dismiss --- .../UIViewController+Navigation.swift | 5 +++- .../ProductDetailWebCoordinator.swift | 24 ++++++++++++------- ...ductDetailCoordinatorFactoryProtocol.swift | 16 ++++--------- .../ProductDetailNavigator.swift | 13 +++++++--- .../ProductsSplitViewCoordinator.swift | 19 +++++++++++---- .../Products/ProductsViewController.swift | 5 ++++ 6 files changed, 54 insertions(+), 28 deletions(-) diff --git a/WooCommerce/Classes/Extensions/UIViewController+Navigation.swift b/WooCommerce/Classes/Extensions/UIViewController+Navigation.swift index 58fe7eaf011..a67c27a763e 100644 --- a/WooCommerce/Classes/Extensions/UIViewController+Navigation.swift +++ b/WooCommerce/Classes/Extensions/UIViewController+Navigation.swift @@ -6,7 +6,10 @@ extension UIViewController { /// A VC added to an existing navigation controller is dismissed when `isMovingFromParent` is `true`. /// For any other scenario `isBeingDismissed` will do. var isBeingDismissedInAnyWay: Bool { - isMovingFromParent || isBeingDismissed || navigationController?.isBeingDismissed == true + isMovingFromParent || + isBeingDismissed || + navigationController?.isBeingDismissed == true || + navigationController?.isMovingFromParent == true } /// Async/await version of the UIKit `dismiss(animated:)`. diff --git a/WooCommerce/Classes/Routing/ProductDetail/Coordinator/ProductDetailWebCoordinator.swift b/WooCommerce/Classes/Routing/ProductDetail/Coordinator/ProductDetailWebCoordinator.swift index e0a74d38a70..a0310482bf2 100644 --- a/WooCommerce/Classes/Routing/ProductDetail/Coordinator/ProductDetailWebCoordinator.swift +++ b/WooCommerce/Classes/Routing/ProductDetail/Coordinator/ProductDetailWebCoordinator.swift @@ -3,8 +3,7 @@ import Yosemite /// Coordinator for the **admin web** product detail/editor flow. final class ProductDetailWebCoordinator: NSObject, ProductDetailCoordinator { - private var onDismiss: (() -> Void)? - + var onDismiss: (() -> Void)? private let site: Site init(site: Site) { @@ -19,16 +18,25 @@ final class ProductDetailWebCoordinator: NSObject, ProductDetailCoordinator { return UIViewController() } - let viewModel = WPAdminWebViewModel(title: product.name, initialURL: url) + let viewModel = AdminWebViewModel(title: product.name, initialURL: url) { [onDismiss] in + onDismiss?() + } let webViewController = AuthenticatedWebViewController(viewModel: viewModel) return webViewController } +} + +fileprivate class AdminWebViewModel: WPAdminWebViewModel { + var onDismiss: (() -> Void)? + + init(title: String, initialURL: URL, onDismiss: (() -> Void)?) { + self.onDismiss = onDismiss + super.init(title: title, initialURL: initialURL) + } - @objc - private func dismissWebView() { - let completion = onDismiss - onDismiss = nil - completion?() + override func handleDismissal() { + onDismiss?() + super.handleDismissal() } } diff --git a/WooCommerce/Classes/Routing/ProductDetail/ProductDetailCoordinatorFactoryProtocol.swift b/WooCommerce/Classes/Routing/ProductDetail/ProductDetailCoordinatorFactoryProtocol.swift index fb7993fb15c..5b130b5a740 100644 --- a/WooCommerce/Classes/Routing/ProductDetail/ProductDetailCoordinatorFactoryProtocol.swift +++ b/WooCommerce/Classes/Routing/ProductDetail/ProductDetailCoordinatorFactoryProtocol.swift @@ -2,25 +2,19 @@ import Yosemite /// Factory for producing coordinators used by the navigator. protocol ProductDetailCoordinatorFactoryProtocol { - func webCoordinator() -> ProductDetailCoordinator - func nativeCoordinator() -> ProductDetailCoordinator + func webCoordinator(site: Site) -> ProductDetailWebCoordinator + func nativeCoordinator() -> ProductDetailNativeCoordinator } /// Default coordinator factory that wires production dependencies. class ProductDetailCoordinatorFactory: ProductDetailCoordinatorFactoryProtocol { static let `default` = ProductDetailCoordinatorFactory() - private let stores: StoresManager - - init(stores: StoresManager = ServiceLocator.stores) { - self.stores = stores - } - - func webCoordinator() -> ProductDetailCoordinator { - return ProductDetailWebCoordinator(site: stores.sessionManager.defaultSite!) + func webCoordinator(site: Site) -> ProductDetailWebCoordinator { + return ProductDetailWebCoordinator(site: site) } - func nativeCoordinator() -> ProductDetailCoordinator { + func nativeCoordinator() -> ProductDetailNativeCoordinator { return ProductDetailNativeCoordinator() } } diff --git a/WooCommerce/Classes/Routing/ProductDetail/ProductDetailNavigator.swift b/WooCommerce/Classes/Routing/ProductDetail/ProductDetailNavigator.swift index fd9f4541c2d..9e58b5a5999 100644 --- a/WooCommerce/Classes/Routing/ProductDetail/ProductDetailNavigator.swift +++ b/WooCommerce/Classes/Routing/ProductDetail/ProductDetailNavigator.swift @@ -22,11 +22,15 @@ final class ProductDetailNavigator { private let ciabChecker: CIABEligibilityCheckerProtocol private let coordinatorFactory: ProductDetailCoordinatorFactoryProtocol + private let stores: StoresManager init(ciabChecker: CIABEligibilityCheckerProtocol = CIABEligibilityChecker(), - coordinatorFactory: ProductDetailCoordinatorFactoryProtocol = ProductDetailCoordinatorFactory.default) { + coordinatorFactory: ProductDetailCoordinatorFactoryProtocol = ProductDetailCoordinatorFactory.default, + stores: StoresManager = ServiceLocator.stores, + ) { self.ciabChecker = ciabChecker self.coordinatorFactory = coordinatorFactory + self.stores = stores } /// Builds the destination `UIViewController` for the given product. @@ -39,11 +43,14 @@ final class ProductDetailNavigator { func makeDestination(product: Product, presentationStyle: Presentation = .push, isReadOnly: Bool, + onDismissWeb: (() -> Void)? = nil, onDelete: (() -> Void)? = nil) -> UIViewController { let coordinator: ProductDetailCoordinator if shouldOpenInWeb(product: product) { - coordinator = coordinatorFactory.webCoordinator() + let webCoordinator = coordinatorFactory.webCoordinator(site: stores.sessionManager.defaultSite!) + webCoordinator.onDismiss = onDismissWeb + coordinator = webCoordinator } else { coordinator = coordinatorFactory.nativeCoordinator() } @@ -57,6 +64,6 @@ final class ProductDetailNavigator { } private func shouldOpenInWeb(product: Product) -> Bool { - return ciabChecker.isCurrentSiteCIAB && product.productType == .booking + return /*ciabChecker.isCurrentSiteCIAB &&*/ product.productType == .booking } } diff --git a/WooCommerce/Classes/ViewRelated/Products/ProductsSplitViewCoordinator.swift b/WooCommerce/Classes/ViewRelated/Products/ProductsSplitViewCoordinator.swift index 1e769371d70..3662500245f 100644 --- a/WooCommerce/Classes/ViewRelated/Products/ProductsSplitViewCoordinator.swift +++ b/WooCommerce/Classes/ViewRelated/Products/ProductsSplitViewCoordinator.swift @@ -122,11 +122,15 @@ private extension ProductsSplitViewCoordinator { } func showProductForm(product: Product) { - let viewController = ProductDetailNavigator.shared.makeDestination(product: product, - isReadOnly: false, - onDelete: { [weak self] in - self?.onSecondaryProductFormDeletion() - }) + let viewController = ProductDetailNavigator.shared.makeDestination( + product: product, + isReadOnly: false, + onDismissWeb: { [weak self] in + self?.resyncProducts() + }, + onDelete: { [weak self] in + self?.onSecondaryProductFormDeletion() + }) showSecondaryView(contentType: .productForm(product: product), viewController: viewController, @@ -195,6 +199,11 @@ private extension ProductsSplitViewCoordinator { } } + func resyncProducts() { + guard let productsViewController = primaryNavigationController.topViewController as? ProductsViewController else { return } + productsViewController.resync() + } + func showEmptyViewOrFirstProduct() { showEmptyView() switch primaryNavigationController.topViewController { diff --git a/WooCommerce/Classes/ViewRelated/Products/ProductsViewController.swift b/WooCommerce/Classes/ViewRelated/Products/ProductsViewController.swift index 4966799c5a2..c7884b1a2c7 100644 --- a/WooCommerce/Classes/ViewRelated/Products/ProductsViewController.swift +++ b/WooCommerce/Classes/ViewRelated/Products/ProductsViewController.swift @@ -303,6 +303,11 @@ final class ProductsViewController: UIViewController, GhostableViewController { func startProductCreation() { addProduct(sourceBarButtonItem: addProductButton, isFirstProduct: false) } + + func resync() { + tableView.reloadData() + paginationTracker.resync() + } } // MARK: - Navigation Bar Actions From 3448c7af337e0f624a765936b778e83051898c9a Mon Sep 17 00:00:00 2001 From: Adam Borbas Date: Wed, 22 Oct 2025 12:02:59 +0200 Subject: [PATCH 12/15] Split web and native coordinator --- .../ProductDetailCoordinator.swift | 11 ---------- .../ProductDetailNativeCoordinator.swift | 2 +- .../ProductDetailWebCoordinator.swift | 20 ++++++++----------- .../ProductAdminURLProvider.swift | 4 ++-- ...ductDetailCoordinatorFactoryProtocol.swift | 4 ++-- .../ProductDetailNavigator.swift | 19 +++++++++--------- .../Mocks/MockProductDetailCoordinator.swift | 10 ++++++++-- .../MockProductDetailCoordinatorFactory.swift | 9 +++++---- 8 files changed, 36 insertions(+), 43 deletions(-) delete mode 100644 WooCommerce/Classes/Routing/ProductDetail/Coordinator/ProductDetailCoordinator.swift diff --git a/WooCommerce/Classes/Routing/ProductDetail/Coordinator/ProductDetailCoordinator.swift b/WooCommerce/Classes/Routing/ProductDetail/Coordinator/ProductDetailCoordinator.swift deleted file mode 100644 index 4fee8308a61..00000000000 --- a/WooCommerce/Classes/Routing/ProductDetail/Coordinator/ProductDetailCoordinator.swift +++ /dev/null @@ -1,11 +0,0 @@ -import UIKit -import Yosemite - -/// Abstraction for building the concrete destination VC for a product detail flow. -protocol ProductDetailCoordinator { - func viewController( - product: Product, - presentationStyle: ProductDetailNavigator.Presentation, - isReadOnly: Bool, - onDelete: (() -> Void)?) -> UIViewController -} diff --git a/WooCommerce/Classes/Routing/ProductDetail/Coordinator/ProductDetailNativeCoordinator.swift b/WooCommerce/Classes/Routing/ProductDetail/Coordinator/ProductDetailNativeCoordinator.swift index 7aa07c76c35..3d6e9e857c4 100644 --- a/WooCommerce/Classes/Routing/ProductDetail/Coordinator/ProductDetailNativeCoordinator.swift +++ b/WooCommerce/Classes/Routing/ProductDetail/Coordinator/ProductDetailNativeCoordinator.swift @@ -3,7 +3,7 @@ import UIKit /// Coordinator for the **native** product detail/editor flow. /// Delegates VC construction to `ProductDetailsFactory` and applies the requested presentation style. -class ProductDetailNativeCoordinator: ProductDetailCoordinator { +class ProductDetailNativeCoordinator { func viewController( product: Product, diff --git a/WooCommerce/Classes/Routing/ProductDetail/Coordinator/ProductDetailWebCoordinator.swift b/WooCommerce/Classes/Routing/ProductDetail/Coordinator/ProductDetailWebCoordinator.swift index a0310482bf2..5b75603e172 100644 --- a/WooCommerce/Classes/Routing/ProductDetail/Coordinator/ProductDetailWebCoordinator.swift +++ b/WooCommerce/Classes/Routing/ProductDetail/Coordinator/ProductDetailWebCoordinator.swift @@ -2,24 +2,20 @@ import UIKit import Yosemite /// Coordinator for the **admin web** product detail/editor flow. -final class ProductDetailWebCoordinator: NSObject, ProductDetailCoordinator { - var onDismiss: (() -> Void)? - private let site: Site +class ProductDetailWebCoordinator: NSObject { + private let site: Site? - init(site: Site) { + init(site: Site?) { self.site = site } - func viewController(product: Product, - presentationStyle: ProductDetailNavigator.Presentation, - isReadOnly: Bool, - onDelete: (() -> Void)? = nil) -> UIViewController { + func viewController(product: Product, onDismiss: @escaping () -> Void) -> UIViewController { guard let url = ProductAdminURLProvider.editURL(for: product, site: site) else { return UIViewController() } let viewModel = AdminWebViewModel(title: product.name, initialURL: url) { [onDismiss] in - onDismiss?() + onDismiss() } let webViewController = AuthenticatedWebViewController(viewModel: viewModel) @@ -28,15 +24,15 @@ final class ProductDetailWebCoordinator: NSObject, ProductDetailCoordinator { } fileprivate class AdminWebViewModel: WPAdminWebViewModel { - var onDismiss: (() -> Void)? + let onDismiss: (() -> Void) - init(title: String, initialURL: URL, onDismiss: (() -> Void)?) { + init(title: String, initialURL: URL, onDismiss: @escaping () -> Void) { self.onDismiss = onDismiss super.init(title: title, initialURL: initialURL) } override func handleDismissal() { - onDismiss?() + onDismiss() super.handleDismissal() } } diff --git a/WooCommerce/Classes/Routing/ProductDetail/ProductAdminURLProvider.swift b/WooCommerce/Classes/Routing/ProductDetail/ProductAdminURLProvider.swift index abfa356489c..dad8a14588a 100644 --- a/WooCommerce/Classes/Routing/ProductDetail/ProductAdminURLProvider.swift +++ b/WooCommerce/Classes/Routing/ProductDetail/ProductAdminURLProvider.swift @@ -4,8 +4,8 @@ import Foundation /// Builds canonical admin edit URLs for products. enum ProductAdminURLProvider { - static func editURL(for product: Product, site: Site) -> URL? { - guard let base = site.adminURLWithFallback() else { return nil } + static func editURL(for product: Product, site: Site?) -> URL? { + guard let base = site?.adminURLWithFallback() else { return nil } var components = URLComponents(url: base, resolvingAgainstBaseURL: false)! components.queryItems = [ diff --git a/WooCommerce/Classes/Routing/ProductDetail/ProductDetailCoordinatorFactoryProtocol.swift b/WooCommerce/Classes/Routing/ProductDetail/ProductDetailCoordinatorFactoryProtocol.swift index 5b130b5a740..81ccf02ed29 100644 --- a/WooCommerce/Classes/Routing/ProductDetail/ProductDetailCoordinatorFactoryProtocol.swift +++ b/WooCommerce/Classes/Routing/ProductDetail/ProductDetailCoordinatorFactoryProtocol.swift @@ -2,7 +2,7 @@ import Yosemite /// Factory for producing coordinators used by the navigator. protocol ProductDetailCoordinatorFactoryProtocol { - func webCoordinator(site: Site) -> ProductDetailWebCoordinator + func webCoordinator(site: Site?) -> ProductDetailWebCoordinator func nativeCoordinator() -> ProductDetailNativeCoordinator } @@ -10,7 +10,7 @@ protocol ProductDetailCoordinatorFactoryProtocol { class ProductDetailCoordinatorFactory: ProductDetailCoordinatorFactoryProtocol { static let `default` = ProductDetailCoordinatorFactory() - func webCoordinator(site: Site) -> ProductDetailWebCoordinator { + func webCoordinator(site: Site?) -> ProductDetailWebCoordinator { return ProductDetailWebCoordinator(site: site) } diff --git a/WooCommerce/Classes/Routing/ProductDetail/ProductDetailNavigator.swift b/WooCommerce/Classes/Routing/ProductDetail/ProductDetailNavigator.swift index 9e58b5a5999..26360d94587 100644 --- a/WooCommerce/Classes/Routing/ProductDetail/ProductDetailNavigator.swift +++ b/WooCommerce/Classes/Routing/ProductDetail/ProductDetailNavigator.swift @@ -46,24 +46,25 @@ final class ProductDetailNavigator { onDismissWeb: (() -> Void)? = nil, onDelete: (() -> Void)? = nil) -> UIViewController { - let coordinator: ProductDetailCoordinator + let viewController: UIViewController if shouldOpenInWeb(product: product) { - let webCoordinator = coordinatorFactory.webCoordinator(site: stores.sessionManager.defaultSite!) - webCoordinator.onDismiss = onDismissWeb - coordinator = webCoordinator - } else { - coordinator = coordinatorFactory.nativeCoordinator() - } + let coordinator = coordinatorFactory.webCoordinator(site: stores.sessionManager.defaultSite) + viewController = coordinator.viewController(product: product) { + onDismissWeb?() + } - let viewController = coordinator.viewController(product: product, + } else { + let coordinator = coordinatorFactory.nativeCoordinator() + viewController = coordinator.viewController(product: product, presentationStyle: presentationStyle, isReadOnly: isReadOnly, onDelete: onDelete) + } return viewController } private func shouldOpenInWeb(product: Product) -> Bool { - return /*ciabChecker.isCurrentSiteCIAB &&*/ product.productType == .booking + return ciabChecker.isCurrentSiteCIAB && product.productType == .booking } } diff --git a/WooCommerce/WooCommerceTests/Mocks/MockProductDetailCoordinator.swift b/WooCommerce/WooCommerceTests/Mocks/MockProductDetailCoordinator.swift index ab35a16f9b2..6e30b82c7ad 100644 --- a/WooCommerce/WooCommerceTests/Mocks/MockProductDetailCoordinator.swift +++ b/WooCommerce/WooCommerceTests/Mocks/MockProductDetailCoordinator.swift @@ -2,8 +2,14 @@ import UIKit import Yosemite @testable import WooCommerce -struct MockProductDetailCoordinator: ProductDetailCoordinator { - func viewController(product: Product, +class MockProductDetailWebCoordinator: ProductDetailWebCoordinator { + override func viewController(product: Product, onDismiss: @escaping () -> Void) -> UIViewController { + UIViewController() + } +} + +class MockProductDetailNativeCoordinator: ProductDetailNativeCoordinator { + override func viewController(product: Product, presentationStyle: ProductDetailNavigator.Presentation, isReadOnly: Bool, onDelete: (() -> Void)?) -> UIViewController { diff --git a/WooCommerce/WooCommerceTests/Mocks/MockProductDetailCoordinatorFactory.swift b/WooCommerce/WooCommerceTests/Mocks/MockProductDetailCoordinatorFactory.swift index 770edbe3a6c..bc7dc336ada 100644 --- a/WooCommerce/WooCommerceTests/Mocks/MockProductDetailCoordinatorFactory.swift +++ b/WooCommerce/WooCommerceTests/Mocks/MockProductDetailCoordinatorFactory.swift @@ -1,16 +1,17 @@ @testable import WooCommerce +import Yosemite class MockProductDetailCoordinatorFactory: ProductDetailCoordinatorFactoryProtocol { private(set) var createdWebCoordiantor = false private(set) var createdNativeCoordiantor = false - func webCoordinator() -> ProductDetailCoordinator { + func webCoordinator(site: Site?) -> ProductDetailWebCoordinator { createdWebCoordiantor = true - return MockProductDetailCoordinator() + return MockProductDetailWebCoordinator(site: Site.fake()) } - func nativeCoordinator() -> ProductDetailCoordinator { + func nativeCoordinator() -> ProductDetailNativeCoordinator { createdNativeCoordiantor = true - return MockProductDetailCoordinator() + return MockProductDetailNativeCoordinator() } } From 257d8ebea5cc3febe50dda32c1ca311484d63f08 Mon Sep 17 00:00:00 2001 From: Adam Borbas Date: Wed, 22 Oct 2025 12:26:45 +0200 Subject: [PATCH 13/15] Use param --- .../Coordinator/ProductDetailNativeCoordinator.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WooCommerce/Classes/Routing/ProductDetail/Coordinator/ProductDetailNativeCoordinator.swift b/WooCommerce/Classes/Routing/ProductDetail/Coordinator/ProductDetailNativeCoordinator.swift index 3d6e9e857c4..ba1a1977c86 100644 --- a/WooCommerce/Classes/Routing/ProductDetail/Coordinator/ProductDetailNativeCoordinator.swift +++ b/WooCommerce/Classes/Routing/ProductDetail/Coordinator/ProductDetailNativeCoordinator.swift @@ -12,7 +12,7 @@ class ProductDetailNativeCoordinator { onDelete: (() -> Void)? = nil) -> UIViewController { return ProductDetailsFactory.productDetails(product: product, presentationStyle: presentationStyle.asProductFormPresentationStyle, - forceReadOnly: false, + forceReadOnly: isReadOnly, onDeleteCompletion: onDelete ?? {}) } } From 3317d495e3a132407852f60f4ae0ab826987d652 Mon Sep 17 00:00:00 2001 From: Adam Borbas Date: Wed, 22 Oct 2025 14:48:43 +0200 Subject: [PATCH 14/15] Rather handle in didDisappear --- .../Authentication/AuthenticatedWebViewController.swift | 5 +++++ .../Classes/Authentication/AuthenticatedWebViewModel.swift | 7 +++++++ .../Classes/Authentication/WPAdminWebViewModel.swift | 4 ++++ .../Classes/Extensions/UIViewController+Navigation.swift | 3 +-- .../Coordinator/ProductDetailWebCoordinator.swift | 5 ++--- 5 files changed, 19 insertions(+), 5 deletions(-) diff --git a/WooCommerce/Classes/Authentication/AuthenticatedWebViewController.swift b/WooCommerce/Classes/Authentication/AuthenticatedWebViewController.swift index 0a8ebed7a68..c00fc5974bd 100644 --- a/WooCommerce/Classes/Authentication/AuthenticatedWebViewController.swift +++ b/WooCommerce/Classes/Authentication/AuthenticatedWebViewController.swift @@ -123,6 +123,11 @@ final class AuthenticatedWebViewController: UIViewController { viewModel.handleDismissal() } } + + override func viewDidDisappear(_ animated: Bool) { + viewModel.handleDisappear() + super.viewDidDisappear(animated) + } } private extension AuthenticatedWebViewController { diff --git a/WooCommerce/Classes/Authentication/AuthenticatedWebViewModel.swift b/WooCommerce/Classes/Authentication/AuthenticatedWebViewModel.swift index 41864638dc6..48f982b7a0b 100644 --- a/WooCommerce/Classes/Authentication/AuthenticatedWebViewModel.swift +++ b/WooCommerce/Classes/Authentication/AuthenticatedWebViewModel.swift @@ -22,6 +22,9 @@ protocol AuthenticatedWebViewModel { /// Triggered when the web view is dismissed func handleDismissal() + /// Triggered when the web view is disappeared + func handleDisappear() + /// Triggered when the web view redirects to a new URL func handleRedirect(for url: URL?) @@ -53,6 +56,10 @@ extension AuthenticatedWebViewModel { func didFailProvisionalNavigation(with error: Error) { // NO-OP } + + func handleDisappear () { + // NO-OP + } } // MARK: - Helper methods diff --git a/WooCommerce/Classes/Authentication/WPAdminWebViewModel.swift b/WooCommerce/Classes/Authentication/WPAdminWebViewModel.swift index df727133515..658388b8696 100644 --- a/WooCommerce/Classes/Authentication/WPAdminWebViewModel.swift +++ b/WooCommerce/Classes/Authentication/WPAdminWebViewModel.swift @@ -20,6 +20,10 @@ class WPAdminWebViewModel: AuthenticatedWebViewModel, WebviewReloadable { // no-op } + func handleDisappear() { + // no-op + } + func handleRedirect(for url: URL?) { guard let url else { return diff --git a/WooCommerce/Classes/Extensions/UIViewController+Navigation.swift b/WooCommerce/Classes/Extensions/UIViewController+Navigation.swift index a67c27a763e..9c379fb7d53 100644 --- a/WooCommerce/Classes/Extensions/UIViewController+Navigation.swift +++ b/WooCommerce/Classes/Extensions/UIViewController+Navigation.swift @@ -8,8 +8,7 @@ extension UIViewController { var isBeingDismissedInAnyWay: Bool { isMovingFromParent || isBeingDismissed || - navigationController?.isBeingDismissed == true || - navigationController?.isMovingFromParent == true + navigationController?.isBeingDismissed == true } /// Async/await version of the UIKit `dismiss(animated:)`. diff --git a/WooCommerce/Classes/Routing/ProductDetail/Coordinator/ProductDetailWebCoordinator.swift b/WooCommerce/Classes/Routing/ProductDetail/Coordinator/ProductDetailWebCoordinator.swift index 5b75603e172..78ced3128ff 100644 --- a/WooCommerce/Classes/Routing/ProductDetail/Coordinator/ProductDetailWebCoordinator.swift +++ b/WooCommerce/Classes/Routing/ProductDetail/Coordinator/ProductDetailWebCoordinator.swift @@ -18,7 +18,6 @@ class ProductDetailWebCoordinator: NSObject { onDismiss() } let webViewController = AuthenticatedWebViewController(viewModel: viewModel) - return webViewController } } @@ -31,8 +30,8 @@ fileprivate class AdminWebViewModel: WPAdminWebViewModel { super.init(title: title, initialURL: initialURL) } - override func handleDismissal() { + override func handleDisappear() { onDismiss() - super.handleDismissal() + super.handleDisappear() } } From 5002b37c0a86a6ac6d844fbd650eef0f41186433 Mon Sep 17 00:00:00 2001 From: Adam Borbas Date: Wed, 22 Oct 2025 14:50:31 +0200 Subject: [PATCH 15/15] Revert isBeingDismissedInAnyWay changes --- .../Classes/Extensions/UIViewController+Navigation.swift | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/WooCommerce/Classes/Extensions/UIViewController+Navigation.swift b/WooCommerce/Classes/Extensions/UIViewController+Navigation.swift index 9c379fb7d53..58fe7eaf011 100644 --- a/WooCommerce/Classes/Extensions/UIViewController+Navigation.swift +++ b/WooCommerce/Classes/Extensions/UIViewController+Navigation.swift @@ -6,9 +6,7 @@ extension UIViewController { /// A VC added to an existing navigation controller is dismissed when `isMovingFromParent` is `true`. /// For any other scenario `isBeingDismissed` will do. var isBeingDismissedInAnyWay: Bool { - isMovingFromParent || - isBeingDismissed || - navigationController?.isBeingDismissed == true + isMovingFromParent || isBeingDismissed || navigationController?.isBeingDismissed == true } /// Async/await version of the UIKit `dismiss(animated:)`.