Launchpad.- Update mayberedirect launchpad option retrieval #74365

agrullon95 · 2023-03-13T03:31:53Z

Related to #73830

Time Estimate

Review: Short
Test: Short

Proposed Changes

Add useLaunchpad hook to maybeRedirect controller function. This hook will use the launchpad endpoint to fetch the latest launchpad to determine if the user needs to be redirected to the launchpad screen or /home

Testing Instructions

Checkout this branch
Create or use an existing launchpad enabled site (Test on an atomic site as well)
Navigate to /home/{siteSlug} and verify that you are redirected to the launchpad screen
Navigate to /home/{siteSlug}?launchpadComplete=true and verify you are not redirected to the launchpad screen
Disable the launchpad (change site option launchpad_screen to off or FALSE) or use a site that is not launchpad enable and navigate to /home. Verify that you are not navigated to the launchpad screen.

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 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-ajp-p2)?

This reverts commit ffbbd9a.

github-actions · 2023-03-13T03:32:05Z

Calypso Live (direct link)

	https://calypso.live?image=registry.a8c.com/calypso/app:build-60393

Jetpack Cloud live (direct link)

	https://calypso.live?image=registry.a8c.com/calypso/app:build-60393&env=jetpack

matticbot · 2023-03-13T03:32:27Z

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

Sections (~81 bytes added 📈 [gzipped])

name                  parsed_size           gzip_size
home                       +218 B  (+0.0%)      +69 B  (+0.0%)
write-flow                 +117 B  (+0.1%)      +12 B  (+0.0%)
videopress-flow            +117 B  (+0.0%)      +12 B  (+0.0%)
newsletter-flow            +117 B  (+0.0%)      +12 B  (+0.0%)
link-in-bio-tld-flow       +117 B  (+0.0%)      +12 B  (+0.0%)
link-in-bio-flow           +117 B  (+0.0%)      +12 B  (+0.0%)
free-flow                  +117 B  (+0.0%)      +12 B  (+0.0%)
build-flow                 +117 B  (+0.1%)      +12 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.

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.

edanzer · 2023-03-14T15:55:34Z

client/data/sites/use-launchpad.ts

+	} );
+};
+
+export const useLaunchpad = ( siteSlug: string, cache = true ) => {


Been a while since I used React Query, and I know it has a range of different options and settings around caching. Do we need to worry about caching this value, and not updating quickly after the setting has been updated on the backend?

Worth noting I tested this - by completing Launchpad, which turns the launchpad_screen option to off, and then immediately navigating to My Home. Seems to work fine. If I recall, RQ may automatticaly refetch data on window refreshes. Not sure how that interacts with the meta -> persist property here, but in any case, seems to be working just fine - no stale data when arriving on My Home.

Looking at the React Query docs, it seems like the query will refetch when its mounted again or the user refocuses on the window by default: https://react-query-v3.tanstack.com/guides/caching

For now we may be fine with the react query default values

edanzer · 2023-03-14T17:14:31Z

client/my-sites/customer-home/controller.jsx

 	const shouldRedirectToLaunchpad =
-		refetchedOptions?.launchpad_screen === 'full' &&
+		launchpad_screen === 'full' &&
 		// Temporary hack/band-aid to resolve a stale issue with atomic that the requestSite


Wondering if we need the 'temporary band-aid for atomic' part of this given the new endpoint?

We can remove the shouldRedirectToLaunchpad logic since const { launchpad_screen, site_intent } = await fetchLaunchpad( slug ); should always retrieve the latest data.

edanzer · 2023-03-14T17:41:17Z

This is testing as expected for me. Added a few comments. I'll also hold on approving as it's still in draft, and there's a convo happening about React Query vs using Redux data stores. Also some failing e2e tests.

It's nice to be able to delete all that extra code to handle stale state situations.

edanzer

Latest code changes look good. I tested this multiple times on Simple sites with different flows. I then tested multiple times on Atomic via different flows and upgrade paths. I also tested both prelaunch (when the home-to-launchpad redirect should work) and post-launch (when the redirect should not be active). In all cases, the My Home redirect worked when it should, and did not fire when it shouldn't.

Only small issue I had was unrelated to the PR. On two separate atomic sites, I'm noticing if you upgrade to an Ecommerce plan, we always seem to load the wp-admin version of My Home, not the calypso version. In such cases, obviously the redirect doesn't work. I would force the issue by manually loading the calypso home url, and the redirect would then work.

I usually test atomic via business plans (and force the atomic transfer by activating a plugin).
I can't recall testing an ecommerce plan. I only did so here because I wanted to force the atomic transfer right away without extra steps. But because I haven't tested this before, it could be that loading the wp-admin version of the dashboard is standard behavior for ecommerce plan sites. But to the extent that's expected, our redirect in the my home controller won't fire on such sites.

This is nothing we need to address in this PR. If we did somehow want to address it, I think we would need to discuss this and do it via a very different PR.

edanzer · 2023-03-15T19:40:37Z

Some e2e tests had been failing. I just re-ran them, and looks like they're good now.

sharpirate · 2023-03-16T17:20:39Z

Testing

✔️ Navigate to /home/{siteSlug} and verify that you are redirected to the launchpad screen
✔️ Navigate to /home/{siteSlug}?launchpadComplete=true and verify you are not redirected to the launchpad screen
✔️ Disable the launchpad (change site option launchpad_screen to off or FALSE) or use a site that is not launchpad enable and navigate to /home. Verify that you are not navigated to the launchpad screen.

…n-retrieval

agrullon95 added 3 commits March 12, 2023 18:06

Update maybeRedirect launchpad option retrieval

ffbbd9a

Revert "Update maybeRedirect launchpad option retrieval"

544ee28

This reverts commit ffbbd9a.

Update maybe redirect launchpad data retrieval

c1b4540

edanzer reviewed Mar 14, 2023

View reviewed changes

Remove launchpad stale data temp fix in controller maybeRedirect

06f475e

agrullon95 requested a review from a team March 14, 2023 20:09

matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Mar 14, 2023

agrullon95 self-assigned this Mar 14, 2023

agrullon95 added [Feature] Launchpad The onboarding Launchpad that guides users through setting up their site and getting it launched. and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Mar 14, 2023

matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Mar 14, 2023

agrullon95 marked this pull request as ready for review March 14, 2023 20:10

Update launchpad_screen logic in home controller maybeRedirect

622ae87

agrullon95 changed the title ~~Update/mayberedirect update launchpad option retrieval~~ Launchpad.- Update mayberedirect launchpad option retrieval Mar 15, 2023

edanzer approved these changes Mar 15, 2023

View reviewed changes

sharpirate approved these changes Mar 16, 2023

View reviewed changes

Merge branch 'trunk' into update/mayberedirect-update-launchpad-optio…

8fc8a58

…n-retrieval

agrullon95 merged commit aaaa7d4 into trunk Mar 16, 2023

agrullon95 deleted the update/mayberedirect-update-launchpad-option-retrieval branch March 16, 2023 21:38

github-actions bot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Mar 16, 2023

This was referenced Mar 17, 2023

My Home: use a try/catch on Launchpad fetching #74586

Merged

Launchpad: use dedicated endpoint to fix Atomic lag #73833

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Launchpad.- Update mayberedirect launchpad option retrieval #74365

Launchpad.- Update mayberedirect launchpad option retrieval #74365

agrullon95 commented Mar 13, 2023 •

edited

Loading

github-actions bot commented Mar 13, 2023 •

edited by matticbot

Loading

matticbot commented Mar 13, 2023 •

edited

Loading

edanzer Mar 14, 2023 •

edited

Loading

edanzer Mar 14, 2023

agrullon95 Mar 15, 2023

edanzer Mar 14, 2023

agrullon95 Mar 14, 2023 •

edited

Loading

edanzer commented Mar 14, 2023

edanzer left a comment

edanzer commented Mar 15, 2023

sharpirate commented Mar 16, 2023

Launchpad.- Update mayberedirect launchpad option retrieval #74365

Launchpad.- Update mayberedirect launchpad option retrieval #74365

Conversation

agrullon95 commented Mar 13, 2023 • edited Loading

Time Estimate

Proposed Changes

Testing Instructions

Pre-merge Checklist

github-actions bot commented Mar 13, 2023 • edited by matticbot Loading

matticbot commented Mar 13, 2023 • edited Loading

edanzer Mar 14, 2023 • edited Loading

Choose a reason for hiding this comment

edanzer Mar 14, 2023

Choose a reason for hiding this comment

agrullon95 Mar 15, 2023

Choose a reason for hiding this comment

edanzer Mar 14, 2023

Choose a reason for hiding this comment

agrullon95 Mar 14, 2023 • edited Loading

Choose a reason for hiding this comment

edanzer commented Mar 14, 2023

edanzer left a comment

Choose a reason for hiding this comment

edanzer commented Mar 15, 2023

sharpirate commented Mar 16, 2023

Testing

agrullon95 commented Mar 13, 2023 •

edited

Loading

github-actions bot commented Mar 13, 2023 •

edited by matticbot

Loading

matticbot commented Mar 13, 2023 •

edited

Loading

edanzer Mar 14, 2023 •

edited

Loading

agrullon95 Mar 14, 2023 •

edited

Loading