From ae352ae5db27cd9d2bb78a1156240757d908cccb Mon Sep 17 00:00:00 2001 From: rachelmcr Date: Mon, 7 Nov 2022 15:32:20 +0000 Subject: [PATCH 1/3] Replace full authentication with frame nonce for product previews --- .../ProductFormViewController.swift | 21 ++++------ .../Edit Product/ProductFormViewModel.swift | 6 +-- .../ProductFormViewModelTests.swift | 39 ++++++------------- 3 files changed, 20 insertions(+), 46 deletions(-) diff --git a/WooCommerce/Classes/ViewRelated/Products/Edit Product/ProductFormViewController.swift b/WooCommerce/Classes/ViewRelated/Products/Edit Product/ProductFormViewController.swift index d4fff5d75fb..42dd8d115f8 100644 --- a/WooCommerce/Classes/ViewRelated/Products/Edit Product/ProductFormViewController.swift +++ b/WooCommerce/Classes/ViewRelated/Products/Edit Product/ProductFormViewController.swift @@ -215,24 +215,17 @@ final class ProductFormViewController: } private func displayProductPreview() { - var permalink = URLComponents(string: product.permalink) - var updatedQueryItems = permalink?.queryItems ?? [] - updatedQueryItems.append(.init(name: "preview", value: "true")) - permalink?.queryItems = updatedQueryItems - guard let url = permalink?.url else { + guard var permalink = URLComponents(string: product.permalink), + let nonce = ServiceLocator.stores.sessionManager.defaultSite?.frameNonce else { return } - let credentials = ServiceLocator.stores.sessionManager.defaultCredentials - guard let username = credentials?.username, - let token = credentials?.authToken, - let site = ServiceLocator.stores.sessionManager.defaultSite else { - return - } + var updatedQueryItems = permalink.queryItems ?? [] + updatedQueryItems.append(.init(name: "preview", value: "true")) + updatedQueryItems.append(.init(name: "frame-nonce", value: nonce)) + permalink.queryItems = updatedQueryItems - let configuration = WebViewControllerConfiguration(url: url) - configuration.secureInteraction = true - configuration.authenticate(site: site, username: username, token: token) + let configuration = WebViewControllerConfiguration(url: permalink.url) let webKitVC = WebKitViewController(configuration: configuration) let nc = WooNavigationController(rootViewController: webKitVC) present(nc, animated: true) diff --git a/WooCommerce/Classes/ViewRelated/Products/Edit Product/ProductFormViewModel.swift b/WooCommerce/Classes/ViewRelated/Products/Edit Product/ProductFormViewModel.swift index 7caa4e7e37a..0e6042de51c 100644 --- a/WooCommerce/Classes/ViewRelated/Products/Edit Product/ProductFormViewModel.swift +++ b/WooCommerce/Classes/ViewRelated/Products/Edit Product/ProductFormViewModel.swift @@ -153,11 +153,9 @@ final class ProductFormViewModel: ProductFormViewModelProtocol { }() if featureFlagService.isFeatureFlagEnabled(.productsOnboarding), - // The store is hosted on WP.com + // The `frame_nonce` value must be stored for the preview to be displayed let site = stores.sessionManager.defaultSite, - site.isWordPressComStore, - // In some edge cases loginURL can be empty preventing successful login flow - site.loginURL.isNotEmpty, + site.frameNonce.isNotEmpty, // Preview existing drafts or new products, that can be saved as a draft (canSaveAsDraft() || originalProductModel.status == .draft), // Do not preview new blank products without any changes diff --git a/WooCommerce/WooCommerceTests/ViewRelated/Products/Edit Product/ProductFormViewModelTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/Products/Edit Product/ProductFormViewModelTests.swift index b07144b3557..0a879008723 100644 --- a/WooCommerce/WooCommerceTests/ViewRelated/Products/Edit Product/ProductFormViewModelTests.swift +++ b/WooCommerce/WooCommerceTests/ViewRelated/Products/Edit Product/ProductFormViewModelTests.swift @@ -544,7 +544,7 @@ final class ProductFormViewModelTests: XCTestCase { func test_no_preview_button_for_new_blank_product_without_any_changes() { // Given - sessionManager.defaultSite = Site.fake().copy(loginURL: "http://test.com/wp-login.php", isWordPressComStore: true) + sessionManager.defaultSite = Site.fake().copy(frameNonce: "abc123") let product = Product.fake().copy(statusKey: ProductStatus.published.rawValue) let viewModel = createViewModel(product: product, @@ -561,7 +561,7 @@ final class ProductFormViewModelTests: XCTestCase { func test_preview_button_for_new_product_with_pending_changes() { // Given - sessionManager.defaultSite = Site.fake().copy(loginURL: "http://test.com/wp-login.php", isWordPressComStore: true) + sessionManager.defaultSite = Site.fake().copy(frameNonce: "abc123") let product = Product.fake().copy(statusKey: ProductStatus.published.rawValue) let viewModel = createViewModel(product: product, @@ -579,7 +579,7 @@ final class ProductFormViewModelTests: XCTestCase { func test_no_preview_button_for_existing_published_product_without_any_changes() { // Given - sessionManager.defaultSite = Site.fake().copy(loginURL: "http://test.com/wp-login.php", isWordPressComStore: true) + sessionManager.defaultSite = Site.fake().copy(frameNonce: "abc123") let product = Product.fake().copy(productID: 123, statusKey: ProductStatus.published.rawValue) let viewModel = createViewModel(product: product, @@ -597,7 +597,7 @@ final class ProductFormViewModelTests: XCTestCase { func test_no_preview_button_for_existing_published_product_with_pending_changes() { // Given - sessionManager.defaultSite = Site.fake().copy(loginURL: "http://test.com/wp-login.php", isWordPressComStore: true) + sessionManager.defaultSite = Site.fake().copy(frameNonce: "abc123") let product = Product.fake().copy(productID: 123, statusKey: ProductStatus.published.rawValue) let viewModel = createViewModel(product: product, @@ -614,7 +614,7 @@ final class ProductFormViewModelTests: XCTestCase { func test_preview_button_for_existing_draft_product_without_any_changes() { // Given - sessionManager.defaultSite = Site.fake().copy(loginURL: "http://test.com/wp-login.php", isWordPressComStore: true) + sessionManager.defaultSite = Site.fake().copy(frameNonce: "abc123") let product = Product.fake().copy(productID: 123, statusKey: ProductStatus.draft.rawValue) let viewModel = createViewModel(product: product, @@ -631,7 +631,7 @@ final class ProductFormViewModelTests: XCTestCase { func test_preview_button_for_existing_draft_product_with_pending_changes() { // Given - sessionManager.defaultSite = Site.fake().copy(loginURL: "http://test.com/wp-login.php", isWordPressComStore: true) + sessionManager.defaultSite = Site.fake().copy(frameNonce: "abc123") let product = Product.fake().copy(productID: 123, statusKey: ProductStatus.draft.rawValue) let viewModel = createViewModel(product: product, @@ -649,7 +649,7 @@ final class ProductFormViewModelTests: XCTestCase { func test_no_preview_button_for_existing_product_with_other_status_and_without_any_changes() { // Given - sessionManager.defaultSite = Site.fake().copy(loginURL: "http://test.com/wp-login.php", isWordPressComStore: true) + sessionManager.defaultSite = Site.fake().copy(frameNonce: "abc123") let product = Product.fake().copy(productID: 123, statusKey: "other") let viewModel = createViewModel(product: product, @@ -666,7 +666,7 @@ final class ProductFormViewModelTests: XCTestCase { func test_no_preview_button_for_existing_product_with_other_status_and_pending_changes() { // Given - sessionManager.defaultSite = Site.fake().copy(loginURL: "http://test.com/wp-login.php", isWordPressComStore: true) + sessionManager.defaultSite = Site.fake().copy(frameNonce: "abc123") let product = Product.fake().copy(productID: 123, statusKey: "other") let viewModel = createViewModel(product: product, @@ -684,7 +684,7 @@ final class ProductFormViewModelTests: XCTestCase { func test_no_preview_button_for_any_product_in_read_only_mode() { // Given - sessionManager.defaultSite = Site.fake().copy(loginURL: "http://test.com/wp-login.php", isWordPressComStore: true) + sessionManager.defaultSite = Site.fake().copy(frameNonce: "abc123") let product = Product.fake().copy(productID: 123, statusKey: ProductStatus.published.rawValue) let viewModel = createViewModel(product: product, @@ -700,26 +700,9 @@ final class ProductFormViewModelTests: XCTestCase { XCTAssertEqual(actionButtons, [.more]) } - func test_no_preview_button_for_existing_draft_product_on_self_hosted_store() { + func test_no_preview_button_for_existing_draft_product_on_site_with_no_frame_nonce() { // Given - sessionManager.defaultSite = Site.fake().copy(loginURL: "http://test.com/wp-login.php", isWordPressComStore: false) - - let product = Product.fake().copy(productID: 123, statusKey: ProductStatus.draft.rawValue) - let viewModel = createViewModel(product: product, - formType: .edit, - stores: stores, - featureFlagService: MockFeatureFlagService(isProductsOnboardingEnabled: true)) - - // When - let actionButtons = viewModel.actionButtons - - // Then - XCTAssertEqual(actionButtons, [.publish, .more]) - } - - func test_no_preview_button_for_existing_draft_product_on_site_with_no_preview_url() { - // Given - sessionManager.defaultSite = Site.fake().copy(loginURL: "", isWordPressComStore: true) + sessionManager.defaultSite = Site.fake().copy(frameNonce: "") let product = Product.fake().copy(productID: 123, statusKey: ProductStatus.draft.rawValue) let viewModel = createViewModel(product: product, From 7ed57abe7791844944c83c6197fbb5468df6e075 Mon Sep 17 00:00:00 2001 From: rachelmcr Date: Tue, 8 Nov 2022 17:36:37 +0000 Subject: [PATCH 2/3] Differentiate between when Preview button should be visible versus enabled --- .../Products/Edit Product/ProductFormViewController.swift | 4 +++- .../Products/Edit Product/ProductFormViewModel.swift | 4 +--- .../Edit Product/ProductFormViewModelProtocol.swift | 7 +++++++ .../Products/Edit Product/ProductFormViewModelTests.swift | 8 +++++--- 4 files changed, 16 insertions(+), 7 deletions(-) diff --git a/WooCommerce/Classes/ViewRelated/Products/Edit Product/ProductFormViewController.swift b/WooCommerce/Classes/ViewRelated/Products/Edit Product/ProductFormViewController.swift index 42dd8d115f8..d19b7fc4f3b 100644 --- a/WooCommerce/Classes/ViewRelated/Products/Edit Product/ProductFormViewController.swift +++ b/WooCommerce/Classes/ViewRelated/Products/Edit Product/ProductFormViewController.swift @@ -971,7 +971,9 @@ private extension ProductFormViewController { } func createPreviewBarButtonItem() -> UIBarButtonItem { - return UIBarButtonItem(title: Localization.previewTitle, style: .done, target: self, action: #selector(saveDraftAndDisplayProductPreview)) + let previewButton = UIBarButtonItem(title: Localization.previewTitle, style: .done, target: self, action: #selector(saveDraftAndDisplayProductPreview)) + previewButton.isEnabled = viewModel.shouldEnablePreviewButton() + return previewButton } func createMoreOptionsBarButtonItem() -> UIBarButtonItem { diff --git a/WooCommerce/Classes/ViewRelated/Products/Edit Product/ProductFormViewModel.swift b/WooCommerce/Classes/ViewRelated/Products/Edit Product/ProductFormViewModel.swift index 0e6042de51c..d8730d16de1 100644 --- a/WooCommerce/Classes/ViewRelated/Products/Edit Product/ProductFormViewModel.swift +++ b/WooCommerce/Classes/ViewRelated/Products/Edit Product/ProductFormViewModel.swift @@ -157,9 +157,7 @@ final class ProductFormViewModel: ProductFormViewModelProtocol { let site = stores.sessionManager.defaultSite, site.frameNonce.isNotEmpty, // Preview existing drafts or new products, that can be saved as a draft - (canSaveAsDraft() || originalProductModel.status == .draft), - // Do not preview new blank products without any changes - !(formType == .add && !hasUnsavedChanges()) { + (canSaveAsDraft() || originalProductModel.status == .draft) { buttons.insert(.preview, at: 0) } diff --git a/WooCommerce/Classes/ViewRelated/Products/Edit Product/ProductFormViewModelProtocol.swift b/WooCommerce/Classes/ViewRelated/Products/Edit Product/ProductFormViewModelProtocol.swift index d70563fd392..8db690e950f 100644 --- a/WooCommerce/Classes/ViewRelated/Products/Edit Product/ProductFormViewModelProtocol.swift +++ b/WooCommerce/Classes/ViewRelated/Products/Edit Product/ProductFormViewModelProtocol.swift @@ -180,4 +180,11 @@ extension ProductFormViewModelProtocol { } } } + + /// Whether the Preview button should be enabled, when it's available in the navigation bar. + /// Returns `false` when it's a new blank product without any changes. + /// + func shouldEnablePreviewButton() -> Bool { + !(formType == .add && !hasUnsavedChanges()) + } } diff --git a/WooCommerce/WooCommerceTests/ViewRelated/Products/Edit Product/ProductFormViewModelTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/Products/Edit Product/ProductFormViewModelTests.swift index 0a879008723..c3aa2d2eba0 100644 --- a/WooCommerce/WooCommerceTests/ViewRelated/Products/Edit Product/ProductFormViewModelTests.swift +++ b/WooCommerce/WooCommerceTests/ViewRelated/Products/Edit Product/ProductFormViewModelTests.swift @@ -542,7 +542,7 @@ final class ProductFormViewModelTests: XCTestCase { // MARK: Preview button tests (with enabled Product Onboarding feature flag) - func test_no_preview_button_for_new_blank_product_without_any_changes() { + func test_disabled_preview_button_for_new_blank_product_without_any_changes() { // Given sessionManager.defaultSite = Site.fake().copy(frameNonce: "abc123") @@ -556,10 +556,11 @@ final class ProductFormViewModelTests: XCTestCase { let actionButtons = viewModel.actionButtons // Then - XCTAssertEqual(actionButtons, [.publish, .more]) + XCTAssertEqual(actionButtons, [.preview, .publish, .more]) + XCTAssertFalse(viewModel.shouldEnablePreviewButton()) } - func test_preview_button_for_new_product_with_pending_changes() { + func test_enabled_preview_button_for_new_product_with_pending_changes() { // Given sessionManager.defaultSite = Site.fake().copy(frameNonce: "abc123") @@ -575,6 +576,7 @@ final class ProductFormViewModelTests: XCTestCase { // Then XCTAssertEqual(actionButtons, [.preview, .publish, .more]) + XCTAssertTrue(viewModel.shouldEnablePreviewButton()) } func test_no_preview_button_for_existing_published_product_without_any_changes() { From d947b78ec31f3ffca9891b4a22755ada907839ff Mon Sep 17 00:00:00 2001 From: rachelmcr Date: Wed, 9 Nov 2022 14:28:36 +0000 Subject: [PATCH 3/3] Keep the bottom toolbar disabled for previews --- .../Products/Edit Product/ProductFormViewController.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/WooCommerce/Classes/ViewRelated/Products/Edit Product/ProductFormViewController.swift b/WooCommerce/Classes/ViewRelated/Products/Edit Product/ProductFormViewController.swift index d19b7fc4f3b..11031faac90 100644 --- a/WooCommerce/Classes/ViewRelated/Products/Edit Product/ProductFormViewController.swift +++ b/WooCommerce/Classes/ViewRelated/Products/Edit Product/ProductFormViewController.swift @@ -226,6 +226,7 @@ final class ProductFormViewController: permalink.queryItems = updatedQueryItems let configuration = WebViewControllerConfiguration(url: permalink.url) + configuration.secureInteraction = true let webKitVC = WebKitViewController(configuration: configuration) let nc = WooNavigationController(rootViewController: webKitVC) present(nc, animated: true)