Skip to content

Products Onboarding: Enable product previews for all stores #8058

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Nov 9, 2022
Merged

Products Onboarding: Enable product previews for all stores #8058

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ae352ae
Replace full authentication with frame nonce for product previews
rachelmcr Nov 7, 2022
7ed57ab
Differentiate between when Preview button should be visible versus en…
rachelmcr Nov 8, 2022
d947b78
Keep the bottom toolbar disabled for previews
rachelmcr Nov 9, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 10 additions & 14 deletions WooCommerce/Classes/ViewRelated/Products/Edit Product/ProductFormViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,24 +215,18 @@ final class ProductFormViewController<ViewModel: ProductFormViewModelProtocol>:
}

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)
let configuration = WebViewControllerConfiguration(url: permalink.url)
configuration.secureInteraction = true
configuration.authenticate(site: site, username: username, token: token)
let webKitVC = WebKitViewController(configuration: configuration)
let nc = WooNavigationController(rootViewController: webKitVC)
present(nc, animated: true)
Expand Down Expand Up @@ -978,7 +972,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 {
Expand Down
10 changes: 3 additions & 7 deletions WooCommerce/Classes/ViewRelated/Products/Edit Product/ProductFormViewModel.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,15 +153,11 @@ 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
!(formType == .add && !hasUnsavedChanges()) {
(canSaveAsDraft() || originalProductModel.status == .draft) {
buttons.insert(.preview, at: 0)
}

Expand Down
7 changes: 7 additions & 0 deletions WooCommerce/Classes/ViewRelated/Products/Edit Product/ProductFormViewModelProtocol.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
Copy link
Contributor

@ealeksandrov ealeksandrov Nov 9, 2022

Choose a reason for hiding this comment

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

Good idea about toggling the button state instead of hiding it!

Copy link
Contributor

@Ecarrion Ecarrion Nov 9, 2022

Choose a reason for hiding this comment

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

We may want to change this condition due to the template experiment, as a new template product would be considered as a new product without any changes.

I think we need to add a check for the "blank" state.

Copy link
Contributor Author

@rachelmcr rachelmcr Nov 9, 2022

Choose a reason for hiding this comment

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

I think we need to add a check for the "blank" state.

I agree, but I haven't come up with a good solution for it yet. This PR doesn't change whether you can preview new template products (it just makes the preview button visible but disabled, instead of not appearing at all), so I'm going to go ahead and merge it. I've opened a new issue #8071 so we can follow up with a fix for new template products.

}
47 changes: 16 additions & 31 deletions ...mmerce/WooCommerceTests/ViewRelated/Products/Edit Product/ProductFormViewModelTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -542,9 +542,9 @@ 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(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,
Expand All @@ -556,12 +556,13 @@ 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(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,
Expand All @@ -575,11 +576,12 @@ 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() {
// 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,
Expand All @@ -597,7 +599,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,
Expand All @@ -614,7 +616,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,
Expand All @@ -631,7 +633,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,
Expand All @@ -649,7 +651,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,
Expand All @@ -666,7 +668,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,
Expand All @@ -684,7 +686,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,
Expand All @@ -700,26 +702,9 @@ final class ProductFormViewModelTests: XCTestCase {
XCTAssertEqual(actionButtons, [.more])
}

func test_no_preview_button_for_existing_draft_product_on_self_hosted_store() {
// 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() {
func test_no_preview_button_for_existing_draft_product_on_site_with_no_frame_nonce() {
// 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,
Expand Down