Refactor the bundle installation to be generic #84886

renatho · 2023-12-05T21:19:11Z

First part of this refactor: #84483

Proposed Changes

Refactor the bundle installation to be generic.
- Everything that happens for WooCommerce should continue having exactly the same behavior.
- It now should support other bundles.
Remove Woo Verify Email, which was not used, always protected by a feature flag.
- PR 1 with the feature: Woo on Stepper: Add placeholder verify email step #64387
- PR 2 with the feature: Stepper - Seller Experience: Edit email step #64462
- Commit: ad225e6
- Slack conversation: p1701903126481889-slack-C0Q664T29
Remove unused code from site-setup-flow.
- Commit: d8ea69e
- Slack conversation: p1702302906398119-slack-C0Q664T29

Testing Instructions

The following tests are just to make sure everything continues working as previously. Nothing should change and anything new was introduced. Also, please test other different scenarios that you can think of.

Flow starting with free plan and selecting a WooCommerce theme

Navigate to http://calypso.localhost:3000/start.
Select the free plan.
On the "Pick a design" step, go to the "store" tab, and select the "Tsubaki" theme (WooCommerce theme).
Continue through the flow, upgrading the plan to be able to install the theme.
Make sure everything works in the same way as previously (a good way I used to compare was to navigate in wordpress.com in parallel), and when you complete all the steps you are redirected to the WooCommerce home with WC installed.

Flow starting with business plan and selecting a WooCommerce theme

Navigate to http://calypso.localhost:3000/start.
Select the business plan.
On the "Pick a design" step, go to the "store" tab, and select the "Tsubaki" theme (WooCommerce theme).
Continue through the flow.
Make sure everything works in the same way as previously (a good way I used to compare was to navigate in wordpress.com in parallel), and when you complete all the steps you are redirected to the WooCommerce home with WC installed.

Flow starting with free plan and free theme changing to WooCommerce theme later

Navigate to http://calypso.localhost:3000/start.
Select the free plan.
On the "Pick a design" step, select the Twenty Twenty-Four theme.
Continue through the flow until redirected to the site.
On the site, navigate to Appearance > Themes.
Select the "Tsubaki" theme.
Upgrade, and continue through the flow.
Make sure everything works in the same way as previously (a good way I used to compare was to navigate in wordpress.com in parallel), and when you complete all the steps you are redirected to the WooCommerce home with WC installed.

Flow starting with business plan and free theme, and changing to WooCommerce theme later

Navigate to http://calypso.localhost:3000/start.
Select the business plan.
On the "Pick a design" step, select the Twenty Twenty-Four theme.
Continue through the flow.
Make sure everything works in the same way as previously (a good way I used to compare was to navigate in wordpress.com in parallel). Notice that in this case, WooCommerce wasn't installed. Not sure if it's the ideal behavior, but it keeps the current behavior. If confirmed that it's not an intentional behavior, I can open an issue for that.

Known issues

When I started developing this PR, I noticed an intermittent error (also reproduced on trunk and staging). When I completed the flow with a WooCommerce bundle (woo-on-plans), the "Customize this design" button in http://calypso.localhost:3000/marketplace/thank-you/{DOMAIN} was always a link to the site editor. But sometimes when clicked, it was redirecting to WooCommerce home (/wp-admin/admin.php?page=wc-admin), and others it was just navigating to the site editor without the redirect. I didn't find where this redirect logic lives (I suspect it's not in Calypso), and I couldn't reproduce it anymore, so I'm ignoring it. If you are testing it, and see the same behavior, I can open an issue for that.

Pre-merge Checklist

Has the general commit checklist been followed? (PCYsg-hS-p2)
https://wpcalypso.wordpress.com/devdocs/docs/testing/index.md 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)?

github-actions · 2023-12-05T21:19:23Z

Calypso Live (direct link)

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

Jetpack Cloud live (direct link)

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

matticbot · 2023-12-05T21:28:01Z

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

App Entrypoints (~195 bytes removed 📉 [gzipped])

name                   parsed_size           gzip_size
entry-stepper               -986 B  (-0.1%)     -194 B  (-0.0%)
entry-subscriptions         -710 B  (-0.0%)     -106 B  (-0.0%)
entry-main                  -710 B  (-0.0%)     -107 B  (-0.0%)
entry-login                 -710 B  (-0.0%)     -107 B  (-0.0%)
entry-domains-landing       -710 B  (-0.1%)     -106 B  (-0.1%)
entry-browsehappy           -710 B  (-0.5%)     -106 B  (-0.3%)

Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used.

Sections (~8246 bytes added 📈 [gzipped])

name                       parsed_size           gzip_size
plugin-bundle-flow             +7534 B  (+4.0%)    +3369 B  (+5.3%)
site-setup-flow                -1440 B  (-1.9%)     -830 B  (-5.2%)
with-theme-assembler-flow       -437 B  (-0.6%)       +0 B
update-options-flow             -437 B  (-1.1%)       +7 B  (+0.1%)
update-design-flow              -437 B  (-0.0%)       +4 B  (+0.0%)
free-post-setup-flow            -437 B  (-1.1%)       +4 B  (+0.1%)
free-flow                       -437 B  (-0.9%)       +3 B  (+0.0%)
assembler-first-flow            -437 B  (-0.6%)       -6 B  (-0.0%)
ai-assembler-flow               -437 B  (-0.6%)       -8 B  (-0.0%)
subscribers                     -188 B  (-0.0%)      -55 B  (-0.0%)
sites-dashboard                 -188 B  (-0.0%)      -55 B  (-0.0%)
settings                        -188 B  (-0.0%)      -55 B  (-0.0%)
hosting                         -188 B  (-0.0%)      -55 B  (-0.0%)
home                            -188 B  (-0.0%)      -56 B  (-0.0%)
theme                           +128 B  (+0.0%)      +45 B  (+0.0%)
themes                           +20 B  (+0.0%)      +13 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 (~81 bytes removed 📉 [gzipped])

name                                                 parsed_size           gzip_size
async-load-design                                         -188 B  (-0.0%)      -55 B  (-0.0%)
async-load-signup-steps-woocommerce-install-confirm        +46 B  (+0.1%)      -26 B  (-0.2%)

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 · 2023-12-05T22:11:53Z

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

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

blaze-dashboard
odyssey-stats
wpcom-block-editor

To test WordPress.com changes, run install-plugin.sh $pluginSlug change/make-bundle-installation-generic on your sandbox.

renatho · 2023-12-06T22:10:55Z

...landing/stepper/declarative-flow/internals/steps-repository/bundle-install-plugins/index.tsx

 	const { submit } = navigation;
 	const { setPendingAction, setProgressTitle, setProgress } = useDispatch( ONBOARD_STORE );
 	const { initiateSoftwareInstall, requestAtomicSoftwareStatus } = useDispatch( SITE_STORE );
 	const { getAtomicSoftwareInstallError, getAtomicSoftwareStatus, getAtomicSoftwareError } =
 		useSelect( ( select ) => select( SITE_STORE ) as SiteSelect, [] );
 	const site = useSite();
-	const softwareSet = 'woo-on-plans';
+	const softwareSet = useSitePluginSlug();


In some cases, you will see softwareSet vs pluginSlug.
They are the same thing. They just have different names because in different places they were called differently, and making everything generic, they are getting mixed in some parts.
I didn't want to rename all functions and variables to not make it even bigger, but it's something that could be done in the future.

renatho · 2023-12-08T19:05:34Z

client/landing/stepper/declarative-flow/plugin-bundle-data.ts


-export default pluginBundleSteps;
+export type BundledPlugin = keyof typeof bundleStepsSettings & string;


I don't have much experience with TypeScript, so I'd appreciate help here if any reviewer has a suggestion.
BundledPlugin type is currently accepting any string.

If I remove the type BundleStepsSettings from the bundleStepsSettings it works properly, accepting only "woo-on-plans". The same happens with the previous version of this object (trunk).

Any idea on how to solve it? The best resource I could find about this is https://stackoverflow.com/questions/51808160/keyof-inferring-string-number-when-key-is-only-a-string

renatho · 2023-12-08T20:28:04Z

client/landing/stepper/declarative-flow/internals/index.tsx

+	const stepNavigation = flow.useStepNavigation(
+		currentStepRoute,
+		_navigate,
+		flowSteps.map( ( step ) => step.slug )


I added the steps argument here.

It seems when it was created, the type already had the steps argument, but it was never used.

renatho · 2023-12-11T15:12:25Z

client/landing/stepper/declarative-flow/internals/steps-repository/bundle-confirm/index.tsx

-								recordTracksEvent( 'calypso_woocommerce_dashboard_confirm_submit', {
+								let eventName = 'calypso_bundle_dashboard_confirm_submit';
+
+								if ( 'woo-on-plans' === pluginSlug ) {


This logic was applied in some different places as part of this commit: 4d32095

Do you think it makes sense? So we keep the same event and log when it's the woo-on-plans, but we use a generic name for the others. Or maybe should we register one event and log for woo-on-plans, and always register the generic one (having 2 registers for woo-on-plans)?

Personally, I thought the ideal would be to just rename it for every case (the calypso_woocommerce_dashboard_confirm_submit wouldn't exist anymore), but I'm afraid that it could break existing reports, and so on.

Also, should I document this somewhere?

There's a tool for documenting new tracks events, more info: PCYsg-mUj-p2

I'd be tempted to have both events, one for woo-on-plans specifically, so as not to break any existing reports (though it doesn't look oft used tbh) and the generic one that's more future-proof

Done in: 59878f2. I wasn't very sure about the logstash, so I did the same there, just in case.

I'll take a look about the documentation! Thank you!

Events are also registered.
The previous ones (WooCommerce events) weren't registered, and I didn't register them because we probably don't want them to be used anymore.

https://github.com/Automattic/tracks-events-registration/pull/2054
https://github.com/Automattic/tracks-events-registration/pull/2055

renatho · 2023-12-11T15:15:39Z

client/my-sites/theme/hooks/use-bundle-settings.tsx

- * Hook to get the bundle settings for a given theme.
- * If the theme doesn't have a sotfware set defined, it returns `null`.
- */
-const useBundleSettings = ( themeId: string ): BundleSettings | null => {


Now we have hooks to get the settings by software or theme (through useBundleSettingsByTheme) now.

renatho · 2023-12-11T19:12:28Z

client/landing/stepper/declarative-flow/internals/steps-repository/bundle-confirm/index.tsx

 import { eligibilityHolds as eligibilityHoldsConstants } from 'calypso/state/automated-transfer/constants';
 import SupportCard from '../store-address/support-card';
 import type { Step } from '../../types';
 import type { OnboardSelect, SiteSelect } from '@automattic/data-stores';
 import type { TransferEligibilityError } from '@automattic/data-stores/src/automated-transfer-eligibility/types';
 import './style.scss';

+const ActionSection = styled.div`


I moved these styles directly to this final instead of importing it from woocommerce-install to not have it tied to woocommerce components, and following the approach used in other components. Ideally, in the future, it could be in a central place and replace all the occurrences.

client/my-sites/theme/main.jsx

…make-bundle-installation-generic

Update to match changes from conflicting PR. See #85013 and #84886 (comment). Co-authored-by: okmttdhr <okmttdhr@users.noreply.github.com>

…mpatibility

…make-bundle-installation-generic

#85013 (comment)

…make-bundle-installation-generic

renatho · 2023-12-15T20:44:02Z

client/landing/stepper/declarative-flow/plugin-bundle-flow.ts

 						intent,
 						storeType,
 						adminUrl,
 						dispatch,
 						exitFlow,
 					} );
-					if ( false !== endReturn ) {
+					if ( settings?.endFlow && false !== endReturn ) {


While testing it with the Sensei bundle (#85309), I noticed an issue here. Fixed as part of this commit: b5fa4e0

It should return earlier only if the endFlow exists and it doesn't return false.

…make-bundle-installation-generic

* Refactor check for plugins to be generic * Update bundle settings hook to allow getting settings by theme software * Move array of plugins to check to the bundle settings * Update feature flag to a generic name * Rename woo confirm to bundle confirm * Add new hook to get site bundle settings * Add TODO comment to review later * Rename woo transfer to bundle transfer * Make bundle transfer step generic * Rename woo install plugins to bundle install plugins * Make install plugins generic * Translate message that was not translated * Get software name dynamically from bundle settings * Remove a not necessary hook * Refactor hooks to be named exports, following the standards of the existing ones * Use hook to get plugin slug * Remove Woo Verify Email not used feature * Reorganize bundle steps array * Add comments to make the steps more clear * Move custom back to settings * Move flow settings to bundleStepsSettings * Move steps to settings So only one object from this file needs to be customized for bundles. * Make end of flow generic * Make custom functions optional * Add comments to the custom functions * Rename the variable to follow the same standard of the other file * Force type to be a string * Add steps as argument to the useStepNavigation * Make the navigation to the next step generic for the submit * Refactor submit * Update log and event name * Update import to the new function * Remove unused code This logic is part of plugin-bundle-flow. * Move the styles to the file Following the approach applied in other files * Remove comment Keeping it as a customization inside the step, as exception. * Update logic to upgrade site when installing free theme with bundle * Make `ThemeUpgradeModal` Calypso-State-free Update to match changes from conflicting PR. See #85013 and #84886 (comment). Co-authored-by: okmttdhr <okmttdhr@users.noreply.github.com> * Add event for all the cases, and keep the current one for backward compatibility * Fix case when endFlow is not set * Add Sensei bundle settings * Update string to match new updates from rebase * Redirect to Sensei Setup Wizard after the WPCOM flow * Remove step from URL It automattically redirects to the first step of the wizard. * Update string for bundled plugin messages * Fix logic to display activate or upgrade button --------- Co-authored-by: okmttdhr <okmttdhr@users.noreply.github.com>

a8ci18n · 2023-12-21T18:27:13Z

This Pull Request is now available for translation here: https://translate.wordpress.com/deliverables/10448936

Some locales (Hebrew, Japanese) have been temporarily machine-translated due to translator availability. All other translations are usually ready within a few days. Untranslated and machine-translated strings will be sent for translation next Monday and are expected to be completed by the following Friday.

Hi @renatho, could you please edit the description of this PR and add a screenshot for our translators? Ideally it'd include this string: Installing %(softwareName)s

Thank you in advance!

a8ci18n · 2023-12-24T21:29:59Z

Translation for this Pull Request has now been finished.

ilyasfoo · 2024-01-02T07:03:17Z

client/my-sites/theme/main.jsx

+				( ( ! isThemeAllowedOnSite || isPremium ) &&
+					! isThemePurchased &&
+					! isExternallyManagedTheme ) ||
+				( isBundledSoftwareSet && ! canInstallPlugins )


Hey, @renatho!

This particular change may have unintentionally caused a bug in Woo Express, where themes like tsubaki and tazza now requires an upgrade. This is because a free trial site can't activate plugins, and I believe woocommerce themes are marked as "bundled software set".

May I know what was the intended themes to be matched with this logic?

To reproduce:

Create a Woo Express free trial site by going to https://woo.com/start and follow through the NUX process

Go to Appearance > Themes

Search for tazza

Select Tazza theme

Observe you can't activate it now

I've made a PR to fix this, #85890.

Appreciate if you can take a look as well, @renatho!

@ilyasfoo

Thank you for catching it and quickly raising a PR with the fix!

May I know what was the intended themes to be matched with this logic?

It was supposed to match the paid themes (previous logic) + bundled themes. But I was assuming that any bundled theme would need to be upgraded to a plan where the user can install plugins because it will install a plugin along with the theme. But I missed this use case that you mentioned.

I've made a PR to fix this, #85890.

I'll take a look now.

May I know what was the intended themes to be matched with this logic?

The new theme we are considering outside of WooCommerce is the Course theme, which installs the Sensei LMS.

renatho self-assigned this Dec 5, 2023

renatho force-pushed the change/make-bundle-installation-generic branch from 6755a5c to ed39003 Compare December 5, 2023 22:05

renatho force-pushed the change/make-bundle-installation-generic branch 3 times, most recently from daad2b8 to 0a4f76f Compare December 6, 2023 21:10

renatho commented Dec 6, 2023

View reviewed changes

renatho force-pushed the change/make-bundle-installation-generic branch 4 times, most recently from c6fd676 to 1b8a773 Compare December 8, 2023 19:02

renatho commented Dec 8, 2023

View reviewed changes

renatho changed the title ~~Refactor bundle installation to be generic~~ Refactor the bundle installation to be generic Dec 8, 2023

renatho force-pushed the change/make-bundle-installation-generic branch 2 times, most recently from c050e84 to d449f0b Compare December 8, 2023 22:25

renatho commented Dec 11, 2023

View reviewed changes

renatho force-pushed the change/make-bundle-installation-generic branch from 4d32095 to a87c1e0 Compare December 11, 2023 18:28

renatho commented Dec 11, 2023

View reviewed changes

renatho added 8 commits December 11, 2023 17:42

Refactor check for plugins to be generic

afafd12

Update bundle settings hook to allow getting settings by theme software

cb51bbe

Move array of plugins to check to the bundle settings

5efd51a

Update feature flag to a generic name

4575b6a

Rename woo confirm to bundle confirm

14a7768

Add new hook to get site bundle settings

3fc713d

Add TODO comment to review later

fa5ba4f

Rename woo transfer to bundle transfer

c666534

Update logic to upgrade site when installing free theme with bundle

cd6f5da

renatho mentioned this pull request Dec 14, 2023

Add Sensei bundle settings #85309

Merged

7 tasks

renatho commented Dec 14, 2023

View reviewed changes

client/my-sites/theme/main.jsx Outdated Show resolved Hide resolved

renatho and others added 2 commits December 14, 2023 15:26

Merge branch 'trunk' of github.com:Automattic/wp-calypso into change/…

f7791a0

…make-bundle-installation-generic

Make ThemeUpgradeModal Calypso-State-free

15cee75

Update to match changes from conflicting PR. See #85013 and #84886 (comment). Co-authored-by: okmttdhr <okmttdhr@users.noreply.github.com>

renatho force-pushed the change/make-bundle-installation-generic branch from 373f83e to 15cee75 Compare December 14, 2023 19:09

renatho added 2 commits December 14, 2023 17:25

Add event for all the cases, and keep the current one for backward co…

59878f2

…mpatibility

Merge branch 'trunk' of github.com:Automattic/wp-calypso into change/…

99dfe64

…make-bundle-installation-generic

renatho requested review from okmttdhr and dsas December 14, 2023 22:47

okmttdhr added a commit that referenced this pull request Dec 15, 2023

Apply #84886 to useBundleSetting

9358abf

#85013 (comment)

Merge branch 'trunk' of github.com:Automattic/wp-calypso into change/…

fb98dbe

…make-bundle-installation-generic

renatho mentioned this pull request Dec 15, 2023

Enable the "Preview & Customize" button for Premium and WooCommerce themes (on horizon) #85182

Merged

7 tasks

renatho added 2 commits December 15, 2023 17:40

Fix case when endFlow is not set

b5fa4e0

Merge branch 'trunk' of github.com:Automattic/wp-calypso into change/…

67f5a17

…make-bundle-installation-generic

renatho commented Dec 15, 2023

View reviewed changes

Fix logic to display activate or upgrade button

c08ac45

renatho force-pushed the change/make-bundle-installation-generic branch from 7b251e7 to c08ac45 Compare December 18, 2023 20:36

Merge branch 'trunk' of github.com:Automattic/wp-calypso into change/…

00a7551

…make-bundle-installation-generic

dsas approved these changes Dec 20, 2023

View reviewed changes

renatho added 2 commits December 20, 2023 22:11

Merge branch 'trunk' of github.com:Automattic/wp-calypso into change/…

77bca35

…make-bundle-installation-generic

Merge branch 'trunk' of github.com:Automattic/wp-calypso into change/…

6698d00

…make-bundle-installation-generic

renatho merged commit 973a2b5 into trunk Dec 21, 2023
11 checks passed

renatho deleted the change/make-bundle-installation-generic branch December 21, 2023 17:49

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 Dec 21, 2023

ilyasfoo reviewed Jan 2, 2024

View reviewed changes

ilyasfoo mentioned this pull request Jan 2, 2024

Fix Woo Express free trial activate WooCommerce themes #85890

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor the bundle installation to be generic #84886

Refactor the bundle installation to be generic #84886

renatho commented Dec 5, 2023 •

edited

Loading

github-actions bot commented Dec 5, 2023 •

edited by matticbot

Loading

matticbot commented Dec 5, 2023 •

edited

Loading

matticbot commented Dec 5, 2023 •

edited

Loading

renatho Dec 6, 2023 •

edited

Loading

renatho Dec 8, 2023 •

edited

Loading

renatho Dec 8, 2023

renatho Dec 11, 2023

dsas Dec 13, 2023

dsas Dec 13, 2023

renatho Dec 14, 2023

renatho Dec 14, 2023 •

edited

Loading

renatho Dec 11, 2023

renatho Dec 11, 2023

renatho Dec 15, 2023

a8ci18n commented Dec 21, 2023

a8ci18n commented Dec 24, 2023

ilyasfoo Jan 2, 2024 •

edited

Loading

ilyasfoo Jan 2, 2024

renatho Jan 2, 2024

renatho Jan 2, 2024


		export default pluginBundleSteps;
		export type BundledPlugin = keyof typeof bundleStepsSettings & string;

Refactor the bundle installation to be generic #84886

Refactor the bundle installation to be generic #84886

Conversation

renatho commented Dec 5, 2023 • edited Loading

Proposed Changes

Testing Instructions

Known issues

Pre-merge Checklist

github-actions bot commented Dec 5, 2023 • edited by matticbot Loading

matticbot commented Dec 5, 2023 • edited Loading

matticbot commented Dec 5, 2023 • edited Loading

renatho Dec 6, 2023 • edited Loading

Choose a reason for hiding this comment

renatho Dec 8, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

renatho Dec 14, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

a8ci18n commented Dec 21, 2023

a8ci18n commented Dec 24, 2023

ilyasfoo Jan 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

renatho commented Dec 5, 2023 •

edited

Loading

github-actions bot commented Dec 5, 2023 •

edited by matticbot

Loading

matticbot commented Dec 5, 2023 •

edited

Loading

matticbot commented Dec 5, 2023 •

edited

Loading

renatho Dec 6, 2023 •

edited

Loading

renatho Dec 8, 2023 •

edited

Loading

renatho Dec 14, 2023 •

edited

Loading

ilyasfoo Jan 2, 2024 •

edited

Loading