Skip to content
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.

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

Redirect users to /domains/add when cart is cleared in EmailUpsell flow. #98519

Open
wants to merge 6 commits into
base: trunk
Choose a base branch
from

Conversation

StevenDufresne
Copy link
Contributor

@StevenDufresne StevenDufresne commented Jan 17, 2025

Fixes: #84228

Proposed Changes

When a user clears their cart at checkout after coming from the domainEmail upsell flow, they encounter an error when attempting to add an email after being redirected there.

This PR updates leaveCheckout to return users to /domain/add if they have cleared their cart and are coming from the Email Upsell flow.

Changes:

  • Pass a new optional property userHasClearedCart to leaveCheckout to determine whether the user has chosen to clear their cart.
  • Add getCloseURL function to encapsulate closeUrl logic.
    • Add regex to determine if previousPath was the email upsell path and return to /domain/add if users have cleared their cart.

Why are these changes being made?

Ideally, we wouldn't need to add flow-specific logic to leaveCheckout. It would be best to redirect users who arrive on /domains/add/{domain}/email/{site} before rendering the page.

Currently when users clear their cart, we call replaceProductsInCart and immediately redirect the user. Often, users arrive at their next route before the cart has fully cleared. We therefore can't simply check the cart early and redirect. They EmailUpsell view will flash.

We therefore have three choices:

  • Run replaceProductsInCart synchronously, then check cart on upsell page load.
  • Suspend rendering the email upsell view until cart actions have all resolved.
  • Direct users in leaveCheckout.

Run replaceProductsInCart synchronously

Unfortunately, this doesn't work because users will see the empty cart state flash before they are redirected.

Suspend rendering the email upsell

This approach is more predictable and how other pages in upsells flows work. However, this will introduce more delays in rendering the page. As it is, the email upsell page loads fast and is snappy. This is a much better experience for upsells. We don't want users to wait for an upsell they may not want.

Direct users in leaveCheckout [Implemented in this PR]

I went with this approach. If feels like the quickest win that doesn't add delays in the purchase flow or introduce any large refactors.

It isn't without some compromise though.

I decided to use a regex to determine if the users came from the domain flow. I considered using domainAddEmailUpsell but since we don't have access to context.params to determine the domain the were adding, I didn't see an easy way to compare the URLs. There may be a way to do it with the flow name but that also requires access to context or maybe there's a way to get the context?

There's also one use case that isn't handled. If a user clears their cart but makes their way back to the upsell page, they will get an error if they add an email. I don't think that use case would be common and I'm not convinced we need to handle it.

Testing Instructions

Using an account with no domain yet.

Test Emptying Cart

  1. From Upgrade->Domains click "Just search for a domain"
  2. Add Domain
  3. Skip email upsell
  4. At checkout, click "X" and empty cart
  5. Confirm you are redirected to /domains/add

Test Leave Items in Cart

  1. From Upgrade->Domains click "Just search for a domain"
  2. Add Domain
  3. Skip email upsell
  4. At checkout, click "X" and leave items
  5. Confirm you are on the email upsell page.

Test Add Plan flow, empty cart

  1. From Upgrade->Plans click "Upgrade" on a plan
  2. At checkout, click "X" and empty cart
  3. Expect to be back on the plan page

Re-run the test but choose "Leave Items" in cart, expect the same result.

Pre-merge Checklist

  • Has the general commit checklist been followed? (PCYsg-hS-p2)
  • Have you written new tests for your changes?
  • Have you tested the feature in Simple (P9HQHe-k8-p2), Atomic (P9HQHe-jW-p2), and self-hosted Jetpack sites (PCYsg-g6b-p2)?
  • Have you checked for TypeScript, React or other console errors?
  • Have you used memoizing on expensive computations? More info in Memoizing with create-selector and Using memoizing selectors and Our Approach to Data
  • Have we added the "[Status] String Freeze" label as soon as any new strings were ready for translation (p4TIVU-5Jq-p2)?
    • For UI changes, have we tested the change in various languages (for example, ES, PT, FR, or DE)? The length of text and words vary significantly between languages.
  • For changes affecting Jetpack: Have we added the "[Status] Needs Privacy Updates" label if this pull request changes what data or activity we track or use (p4TIVU-aUh-p2)?

@matticbot
Copy link
Contributor

matticbot commented Jan 17, 2025

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

Sections (~98 bytes added 📈 [gzipped])

name      parsed_size           gzip_size
checkout       +223 B  (+0.0%)      +98 B  (+0.0%)

Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to.

Async-loaded Components (~225 bytes added 📈 [gzipped])

name                                             parsed_size           gzip_size
async-load-calypso-my-sites-checkout-modal            +290 B  (+0.0%)     +120 B  (+0.0%)
async-load-calypso-layout-masterbar-checkout          +268 B  (+0.4%)     +105 B  (+0.4%)
async-load-calypso-blocks-editor-checkout-modal       +223 B  (+0.0%)      +98 B  (+0.0%)

React components that are loaded lazily, when a certain part of UI is displayed for the first time.

Legend

What is parsed and gzip size?

Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory.
Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

@matticbot
Copy link
Contributor

matticbot commented Jan 17, 2025

This PR modifies the release build for the following Calypso Apps:

For info about this notification, see here: PCYsg-OT6-p2

  • notifications
  • wpcom-block-editor

To test WordPress.com changes, run install-plugin.sh $pluginSlug fix/empty-cart-back-to-domains on your sandbox.

@StevenDufresne StevenDufresne changed the title Fix/empty cart back to domains Redirect users to /domains/add when cart is cleared in EmailUpsell flow. Jan 17, 2025
@StevenDufresne StevenDufresne marked this pull request as ready for review January 17, 2025 03:08
@StevenDufresne StevenDufresne requested a review from a team as a code owner January 17, 2025 03:08
@matticbot matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Jan 17, 2025
Copy link
Member

@sirbrillig sirbrillig left a comment

Choose a reason for hiding this comment

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

This looks like an elegant solution! Thanks for all the research on the different alternatives. I was wondering, since this seems to focus on leaving checkout using the masterbar close button, does this same bug exist if you remove the product from your cart manually in checkout (which also redirects away if it's the last cart item)?

@StevenDufresne
Copy link
Contributor Author

Aha! Thanks for catching that. I've fixed that in 7ee9365.

Copy link
Member

@sirbrillig sirbrillig left a comment

Choose a reason for hiding this comment

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

Looks good! I made one request for the API of closeAndLeave but it's not a blocker.

client/layout/masterbar/checkout.tsx Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Going back from checkout empty cart assumes you still have items in your cart.
3 participants