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

Focused Launch: Only launch site if checkout is complete #48452

Merged
merged 11 commits into from Feb 17, 2021

Conversation

tjcafferkey
Copy link
Contributor

@tjcafferkey tjcafferkey commented Dec 17, 2020

Demo for the full paid flow: https://cloudup.com/cZCigAVkgu0

WBE diff [WIP] to be approved and merged before merging this PR: D56875-code
[Note] WBE diff also contains several undeployed changes

Changes proposed in this Pull Request

  • If Launch flow is in Calypso (iframed editor), don't launch the site if there are selected paid items and just open checkout modal
  • Show a loading state while the products are added to cart. Disable and animate the Launch button and make the modal non-dismissible. Demo: https://cloudup.com/cxGliq5vaWL (quickly approved by @ollierozdarz on Slack)
  • Don't redirect to a thank-you upsell page when checkout is completed using Editor Checkout Modal
  • After purchasing paid items and checkout modal is closed, continue with launching inside the editor and show success dialog with options to Continue editing and go to Home.
  • Update editor status to reflect the newly purchased plan (corresponding premium blocks should be available without refresh). Currently, this is done by refreshing the editor when clicking Continue Editing
  • Clean-up shouldDisplaySuccessView and enablePersistentSuccessView
  • Fix plan showing up in cart without being displayed as selected during flow in all cases reported in Focused Launch: existing paid plan is interrupting launch #49958

[Update] New changes proposed in the context of #48452 (comment)

Follow up ideas:

Testing instructions

Setup:

  1. Checkout this branch or go to https://calypso.live/?branch=update/focused-launch/launch-after-checkout
  2. Run yarn start in Claypso
  3. Run yarn dev --sync in apps/editing-toolkit
  4. Run yarn dev --sync in apps/wpcom-block-editor after you make sure wpcom-sandbox is aliased (28543-pb)
  5. Sandbox an unlaunched site created via /start to test changes and via /new for regression testing

Focused launch

  • Specific examples of test cases: p1613379716423600/1613375164.421500-slack-C0KDTA48Y
  1. Check Focused Launch (free and paid) in Calypso (inside iframe)
  • Go to /page/{UNLAUNCHED_SITE_ID_CREATED_WITH_START}.wordpress.com/home
  • Free flow should be unchanged and paid flow should match the changes described above
  1. Check Focused Launch (free and paid) in wp-admin (no iframe)
  • Go to /page/{UNLAUNCHED_SITE_ID_CREATED_WITH_START}.wordpress.com/wp-admin/post-new.php
  • Both flows should be unchanged. For paid flow, you'll get redirected to /checkout after the site is launched

Gutenboarding launch (regression testing)

  1. Check Gutenboarding launch (free and paid) in Calypso
  • Go to /page/{UNLAUNCHED_SITE_ID_CREATED_WITH_GUTENBOARDING}.wordpress.com/home
  1. Check Gutenboarding launch (free and paid) in wp-admin
  • Go to {UNLAUNCHED_SITE_ID_CREATED_WITH_GUTENBOARDING}.wordpress.com/wp-admin/post-new.php

Fixes #48081
Fixes #47798
Fixes #49958

@matticbot
Copy link
Contributor

@matticbot
Copy link
Contributor

Caution: This PR affects files in the Editing Toolkit Plugin on WordPress.com
Please ensure your changes work on WordPress.com before merging.

D54439-code has been created so you can easily test it on your sandbox. See this FieldGuide page about developing the Editing Toolkit Plugin for more info: PCYsg-ly5-p2

@matticbot
Copy link
Contributor

matticbot commented Dec 17, 2020

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

Sections (~179 bytes added 📈 [gzipped])

name              parsed_size           gzip_size
gutenberg-editor       +424 B  (+0.1%)     +108 B  (+0.1%)
checkout               +167 B  (+0.0%)      +71 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 (~130 bytes added 📈 [gzipped])

name                                             parsed_size           gzip_size
async-load-calypso-blocks-editor-checkout-modal       +291 B  (+0.0%)     +130 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.

@razvanpapadopol razvanpapadopol force-pushed the update/focused-launch/launch-after-checkout branch from 6c6f0e2 to b173649 Compare December 17, 2020 15:12
@razvanpapadopol razvanpapadopol self-assigned this Dec 22, 2020
@razvanpapadopol razvanpapadopol added the Focused Launch Issues and PRs related to Focused Launch label Jan 6, 2021
@razvanpapadopol razvanpapadopol force-pushed the update/focused-launch/launch-after-checkout branch 3 times, most recently from 1bab0d7 to 389da7e Compare February 10, 2021 12:36
@razvanpapadopol razvanpapadopol marked this pull request as ready for review February 10, 2021 13:01
@razvanpapadopol razvanpapadopol requested a review from a team as a code owner February 10, 2021 13:01
@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 Feb 10, 2021
@razvanpapadopol razvanpapadopol requested a review from a team February 10, 2021 13:01
@razvanpapadopol razvanpapadopol added this to Needs review in Focused Launch (inactive) via automation Feb 10, 2021
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 fine from a checkout perspective. I just left a few minor comments.

Copy link
Contributor

@StefanNieuwenhuis StefanNieuwenhuis left a comment

Choose a reason for hiding this comment

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

I left some comments. Can you please address them? Thank you so much and thank you so much for working on this!

packages/launch/src/hooks/use-cart.ts Outdated Show resolved Hide resolved
packages/launch/src/hooks/use-cart.ts Show resolved Hide resolved
packages/launch/src/launch/index.tsx Show resolved Hide resolved
packages/onboarding/styles/mixins.scss Show resolved Hide resolved
} );

const handlePaymentComplete = useCallback(
Copy link
Contributor

Choose a reason for hiding this comment

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

Note to myself: There might be something wrong here preventing a site from being launched after the payment is complete.

Copy link
Member

Choose a reason for hiding this comment

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

🤔 I was just thinking about this and it might not do what you want in cases where the payment method is a redirect. For example, if you pay via PayPal, you're redirected away from checkout when you click to pay, then redirected directly to the thank-you page when you come back from PayPal.

Copy link

Choose a reason for hiding this comment

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

Very good point, @sirbrillig 🙏🏼

Handling external payment processing fixed in a495601

Here is a demo: https://cloudup.com/cWO7-K6SAEH

Now I guess we need to add exceptions for eCommerce plan and registered domains where we should use the standard Checkout redirect to thank-you pages for their functionality. Do you know other cases like these?

Copy link
Member

Choose a reason for hiding this comment

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

Now I guess we need to add exceptions for eCommerce plan and registered domains where we should use the standard Checkout redirect to thank-you pages for their functionality. Do you know other cases like these?

Hm... not off the top of my head. You could look through the tests that cover the thank-you url function which in theory document all the possible conditions but while I try to shepherd the thank-you functionality, the actual logic is made by other teams (largely @Automattic/martech).

@alshakero
Copy link
Member

Regarding this point

After purchasing paid items and checkout modal is closed, continue with launching inside the editor and show success dialog with options to Continue editing and go to Home.

I'm getting a success view with "Continue editing" CTA for a second, then I'm getting redirected to the checkout on wordpress.com. When I pay, I land at Calypso home and not in the editor anymore.

@yansern
Copy link
Contributor

yansern commented Feb 12, 2021

image

Unrelated: I'm getting 404 double stroke when checking domain availability.

@razvanpapadopol
Copy link

image

Unrelated: I'm getting 404 double stroke when checking domain availability.

Yup, that was fixed in #49872. It will be gone after a rebase.

@razvanpapadopol razvanpapadopol force-pushed the update/focused-launch/launch-after-checkout branch from c97095a to a9ace01 Compare February 15, 2021 08:04
@StefanNieuwenhuis
Copy link
Contributor

After the payment processing is redirecting the user away to complete the purchase (eg: Paypal), they are being redirected back to the editor where their site is launched automatically.

Not sure if it's desired behavior, but after checking out a Premium Plan w/ PayPal, I'm redirected to a thank-you upsell page. What do you think @razvanpapadopol?

@StefanNieuwenhuis
Copy link
Contributor

StefanNieuwenhuis commented Feb 15, 2021

As an exception to the rule above, if the user selected eCommerce plan, their site will be launched before the Checkout step and after completing purchase they will be redirected to the default Checkout thank-you page.

The site is launched before payment but I briefly noticed the success dialog with options to Continue editing and go to Home before I was redirected to the checkout.

@razvanpapadopol
Copy link

FYI it looks like the related phab patch (D54439-code) failed to build for the last 3 times

It seems to work fine now. The unit test that was failing seem to be unrelated anyway.

Unfortunately, I can still reproduce the behavior described in the issue. I'm still redirected to the checkout with a plan that I already purchased.

I'm not able to reproduce this anymore. Did the purchased plan showed up as "Purchased" at the top of Summary view when this happened? Like this:
Screenshot 2021-02-16 at 13 18 20
Note: when using store sandbox to test checkout, the purchased items don't apply when you're using the non-sandboxed store and vice-versa.
If this still happens after double-checking the setup (sanboxed site, ETK synced, plan active and picked up by Launch store), could you share a recording?

Not sure if it's desired behavior, but after checking out a Premium Plan w/ PayPal, I'm redirected to a thank-you upsell page.

Were you on calypso.localhost or calypso.live with ETK synced when this happened? The redirect should be to the editor link with ?should_launch arg as we are setting it here.

The site is launched before payment but I briefly noticed the success dialog with options to Continue editing and go to Home before I was redirected to the checkout.

This is expected 👌🏼 We'll probably remove those buttons and add a little delay to make the experience more smooth or come up with a different solution. Opened #50122 to track this.

@StefanNieuwenhuis
Copy link
Contributor

StefanNieuwenhuis commented Feb 16, 2021

I'm not able to reproduce this anymore. Did the purchased plan showed up as "Purchased" at the top of Summary view when this happened? Like this:
Screenshot 2021-02-16 at 13 18 20
Note: when using store sandbox to test checkout, the purchased items don't apply when you're using the non-sandboxed store and vice-versa.
If this still happens after double-checking the setup (sanboxed site, ETK synced, plan active and picked up by Launch store), could you share a recording?

After double-checking the setup (sandboxed site, ETK synced, plan active, and picked up by Launch store) and another round of testing, I couldn't reproduce it anymore so consider this resolved.

Were you on calypso.localhost or calypso.live with ETK synced when this happened? The redirect should be to the editor link with ?should_launch arg as we are setting it here.

After double-checking the setup (sandboxed site, ETK synced, plan active, and picked up by Launch store) and another round of testing, I noticed the ?should_launch query param and the after checkout flow redirected, as expected, to the success dialog with options to Continue editing and go to Home. Consider this resolved!

This is expected 👌🏼 We'll probably remove those buttons and add a little delay to make the experience more smooth or come up with a different solution. Opened #50122 to track this.

Awesome 🤙

tjcafferkey and others added 11 commits February 17, 2021 09:47
- Add isInIframe to context and use it in use-cart hook
- Add goToCheckoutAndLaunch method on use-cart hook to be used in Focused Launch
- Extract addProductsToCart to a function to prepare further extraction
- Disable and show a pulsating effect on Launch button
- Make the modal non-dismissible
- Add onboarding-loading mixin and re-use it in onboarding-placeholder mixin in onboarding styles
…inue Editing in order for Jetpack premium blocks to pick up the new plan
* Open a new MessageChannel to listen for checkout success on each modal opening.
* add handleCheckoutModalOpened function
* Wrap handlePaymentComplete in a useCallback
* Use destructuring to cleanly remove callback from data before postMessage
* rename isPaidPlan to hasPaidPlan

Prevent paid plan to show up in cart.
* if site is on a paid plan, don't pre-select plan from cart
* if there is a selected plan in Focused Launch, clear selection if the site is on a paid plan

Refresh page after purchase only if a paid product is selected to enable Premium blocks
* Use getPaidPlanProductId selector in Summary view
* Handle redirecting back to editor after processing payment externally like with Paypal.
* Handle auto-launch when landing in editor by using 'should_launch' param and isLaunchImmediately prop on FocusedLaunchModal
* update useSelect for PLANS_STORE
* extract LaunchCart to a type
@razvanpapadopol razvanpapadopol force-pushed the update/focused-launch/launch-after-checkout branch from 9fe0757 to d53f569 Compare February 17, 2021 08:14
Copy link
Contributor

@yansern yansern left a comment

Choose a reason for hiding this comment

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

Apart from the code comments below, here are few more things to add:

When the user completes the checkout flow without redirection, a route is pushed to display the Success view.

When the user completes the checkout flow with redirection, the ?should_launch query arg is used to mark the isLaunchImmediately prop as true, which subsequently display the Success view. (Except eCommerce plan - see PR description.)

The composite-checkout component was also slightly modified with the isFocusedLaunch prop which prevents the component from showing the thank you page. This is so we can display our own Success view with confettis.

This PR is good to go.

Comment on lines -570 to +583
launchSite( siteId );
if ( selectedDomain || ! isSelectedPlanFree ) {
goToCheckout();
// Launch the site directly if Free plan and subdomain are selected.
// Otherwise, show checkout as the next step.
if ( ! selectedDomain && ! isSelectedPlanPaid ) {
launchSite( siteId );
} else {
goToCheckoutAndLaunch();
Copy link
Contributor

@yansern yansern Feb 17, 2021

Choose a reason for hiding this comment

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

Context:

  • The launchSite() function makes an API call to mark the site as launched.
  • In goToCheckoutAndLaunch(), it will also call launchSite() when launch is opened in wp-admin and when the plan chosen is eCommerce plan. This means that the site is launched first before the redirecting to the checkout page.
  • !isSelectedPlanPaid is used instead of !isSelectedPlanFree is because user may be entering the launch flow after they have paid for a plan.

Comment on lines +38 to +42
export const openCheckout = (
siteSlug: string = window?.currentSiteId?.toString() || '',
isEcommerce = false,
onSuccessCallback?: () => void
): void => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Context: siteSlug & isEcommerce param is used to construct the redirect url after checkout.


const goToCheckout = async () => {
await addProductsToCart();
openCheckout( siteSubdomain?.domain, isEcommercePlan, onSuccess );
Copy link
Contributor

Choose a reason for hiding this comment

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

Context: onSuccess and launchSite is synonymous. Maybe can clean this up in the future.

Choose a reason for hiding this comment

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

it's just that onSuccess takes no argument and launchSite takes siteId 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah yes right, almost synonymous.

Comment on lines -52 to +66
'launch__focused-modal-overlay--delay-animation-in': shouldDisplaySuccessView,
'launch__focused-modal-overlay--delay-animation-in': isLaunchImmediately,
Copy link
Contributor

Choose a reason for hiding this comment

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

Context: As to why it has 1 sec delay, easier on the eyes. See: #47808 (comment)

Comment on lines +55 to +61
// If there is a purchased plan, remove any selected plan from Launch store
const { unsetPlanProductId } = useDispatch( LAUNCH_STORE );
React.useEffect( () => {
if ( hasPaidPlan ) {
unsetPlanProductId();
}
}, [ hasPaidPlan, unsetPlanProductId ] );
Copy link
Contributor

Choose a reason for hiding this comment

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

Context: unsetPlanProductId() is called in situations where user may have previously used the launch flow to pick a paid plan, but then goes back customizer to actually purchase a paid plan. At this stage, the site is has a paid plan but it is not launched yet. So when launch modal reopens, we need to unset the selected plan product id. @razvanpapadopol Can you verify if this is one of the situations?

Choose a reason for hiding this comment

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

Yes, exactly that's the case here. It's also one of the cases mentioned in #49958

Comment on lines +48 to +55
React.useEffect( () => {
if ( isLaunchImmediately ) {
// if there was a plan in cart before redirect to payment processing,
// remove it now since we don't need to reload the page when dismissing Success view
unsetPlanProductId();
launchSite( siteId );
}
}, [ isLaunchImmediately, unsetPlanProductId, launchSite, siteId ] );
Copy link
Contributor

Choose a reason for hiding this comment

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

Context: This one happens when user enters the block editor again after checkout redirection (e.g. wp-admin or paypal).

Copy link

Choose a reason for hiding this comment

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

At the moment we're using this only in Calypso (iframed editor) for cases of external payment (eg: Paypal). If we'll be doing something like this also from wp-admin we need to control via some query argument also where the user lands from /checkout if they abandon which right now is on /plans and should be the editor with launch modal open but not launching immediately.

I've linked this comment to #50122 to track this case in the future.

Focused Launch (inactive) automation moved this from Needs review to Ready to merge Feb 17, 2021
@yansern yansern merged commit b5d9d3d into trunk Feb 17, 2021
@yansern yansern deleted the update/focused-launch/launch-after-checkout branch February 17, 2021 11:38
Focused Launch (inactive) automation moved this from Ready to merge to Done Feb 17, 2021
@matticbot matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Feb 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Focused Launch Issues and PRs related to Focused Launch
Projects
Focused Launch (inactive)
  
🎉 Done (yay us!)
8 participants