Add one-click installation for recommended extensions in multichannel Marketing page #35542

ecgan · 2022-11-10T14:42:58Z

All Submissions:

Have you followed the WooCommerce Contributing guideline?
Does your code follow the WordPress' coding standards?
Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changes proposed in this Pull Request:

This PR depends on woocommerce.com API changes in PR https://github.com/Automattic/woocommerce.com/pull/14729.

In this PR, we add one-click installation feature to recommended extensions in Marketing page (beta multichannel view only; this PR does not affect the normal Marketing page).

In the API response, recommended extensions will have a property direct_install to indicate whether it supports one-click installation. direct_install is true if the extension is available in the WordPress.org plugin listings; false otherwise.

If direct_install is true, we will display a "Install plugin" button. Clicking it will automatically install and activate the plugin. The plugin will then be removed from the "Discover more marketing tools" card and will appear in the "Installed extensions" card.

If direct_install is false, the plugin will have a "View details" button. Clicking it will navigate to the product page on WooCommerce.com. This is same as existing behavior; we just changed the button text from "Get started" to "View details".

The code for "automatic install and activate" is based on existing prior art in the code base, e.g.:

woocommerce/plugins/woocommerce-admin/client/tasks/fills/Marketing/index.tsx

Lines 131 to 156 in 0b52411

    
           const installAndActivate = ( slug: string ) => { 
        
           	setCurrentPlugin( slug ); 
        
           	actionTask( 'marketing' ); 
        
           	installAndActivatePlugins( [ slug ] ) 
        
           		.then( ( response ) => { 
        
           			recordEvent( 'tasklist_marketing_install', { 
        
           				selected_extension: slug, 
        
           				installed_extensions: installedExtensions.map( 
        
           					( extension ) => extension.slug 
        
           				), 
        
           				section_order: pluginLists 
        
           					.map( ( list ) => list.key ) 
        
           					.join( ', ' ), 
        
           			} ); 
        
           			createNoticesFromResponse( response ); 
        
           			setCurrentPlugin( null ); 
        
           			onComplete( { 
        
           				redirectPath: getNewPath( { task: 'marketing' } ), 
        
           			} ); 
        
           		} ) 
        
           		.catch( ( response: { errors: Record< string, string > } ) => { 
        
           			createNoticesFromResponse( response ); 
        
           			setCurrentPlugin( null ); 
        
           		} ); 
        
           };

Other technical notes:

I introduced a new action called removeRecommendedPlugin. This is called after "automatic install and activate", to remove the extension from "Discover more marketing tools".
I added new folders hooks and types for the shared hooks and types.
usePlugins has been renamed to useInstalledPlugins for better clarity.

📹 Demo video of the feature with my voice annotation:

demo-pr-one-click-installation.mp4

How to test the changes in this Pull Request:

Pre-condition:

Make sure you have the changes in PR https://github.com/Automattic/woocommerce.com/pull/14729.
Enable WooCommerce Multichannel Marketing experience in WooCommerce Settings > Advanced > Features (/wp-admin/admin.php?page=wc-settings&tab=advanced&section=features).

Test instructions:

Go to Marketing page at /wp-admin/admin.php?page=wc-admin&path=%2Fmarketing.
In the "Discover more marketing tools", find a plugin with "Install plugin" button, and click on the button.
- The button should become busy.
- All the other "Install plugin" and "View details" buttons should become disabled.
- The plugin should be installed and activated automatically.
- There should be a successful notice message in the bottom left corner.
  - If there is an error, there should be an error notice message in the bottom left corner. You can try this out by hardcoding a non-existing plugin slug in the code.
- The plugin should then be removed from "Discover more marketing tools" card.
- The plugin should appear in the "Installed extensions" card.
In the "Discover more marketing tools", find a plugin with "View details" button, and click on the button.
- The browser tab should navigate to the product page on WooCommerce.com

Other information:

Have you added an explanation of what your changes do and why you'd like us to include them?
Have you written new tests for your changes, as applicable?
Have you created a changelog file for each project being changed, ie pnpm --filter=<project> changelog add?

FOR PR REVIEWER ONLY:

I have reviewed that everything is sanitized/escaped appropriately for any SQL or XSS injection possibilities. I made sure Linting is not ignored or disabled.

The approach implemented here is similar to plugins/woocommerce-admin/client/tasks/fills/Marketing/index.tsx.

…sion.

This is so that we don't need to make network request and display loading progress.

github-actions · 2022-11-10T15:04:33Z

Test Results Summary

Commit SHA: 90677fd

Test 🧪	Passed ✅	Failed 🚨	Broken 🚧	Skipped ⏭️	Unknown ❔	Total 📊	Duration ⏱️
API Tests	259	0	0	2	0	261	1m 0s
E2E Tests	186	0	0	6	0	192	13m 59s

To view the full API test report, click here.
To view the full E2E test report, click here.
To view all test reports, visit the WooCommerce Test Reports Dashboard.

ecgan · 2022-11-11T18:05:57Z

About the failing GitHub check https://github.com/woocommerce/woocommerce/actions/runs/3439480519/jobs/5736827762, the failed e2e tests are not related to the changes in this PR.

joshuatf

Thanks for the changes here, @ecgan! Code looks good overall. Left a few comments and questions, but nothing too major.

I think skipping the introduction of removeRecommendedPlugin might be a good idea if we can rely on installed plugins to hide this item from the bottom list.

joshuatf · 2022-11-15T18:16:00Z

plugins/woocommerce-admin/client/marketing/hooks/useInstalledPlugins.ts

 };

-export const usePlugins = (): UsePluginsType => {
+export const useInstalledPlugins = (): UsePluginsType => {


Any particular reason for this name change? It looks like this returns more than just installed plugins and the previous name may be more accurate.

In this one I'm going to agree with @ecgan solution since for me it seems that the return is only related to the installed plugins. usePlugins seems more generic for me and is not really clear.

💅 If we are going to refactor it tho... besides the name it would be even more helpful to add a JSDOC at the top :)

I think in this case, the logic in here does not just deal with "installed" plugins. It also deals with plugins that are in the process of activation, activating plugins, and "loading" plugins after installation. While this semantically nitpicky, the semantics here are important.

I don't want to dig into this too much in this PR since I know it was not the intention of this PR to open this can of worms, but I don't think a unique data store should have been created around plugin activation specific to marketing. We have a data store for plugins in WooCommerce that would work well here. We should strive to not reinvent the wheel and consolidate efforts behind existing packages.

Any particular reason for this name change?

The main reason is because we already have a useRecommendedPlugins for the "Discover more marketing tools" card. If we continue to have usePlugin, it feels confusing.

I renamed it to useInstalledPlugins because it is used in the "Installed extensions" card.

I think in this case, the logic in here does not just deal with "installed" plugins. It also deals with plugins that are in the process of activation, activating plugins, and "loading" plugins after installation. While this semantically nitpicky, the semantics here are important.

Yeah, I thought about that too, but I couldn't think of a nice hook name for it.

In the end, I named it useInstalledPlugins because it is mainly used in the "Installed extensions" card (although it is also used in the DiscoverTools component).

I don't think a unique data store should have been created around plugin activation specific to marketing. We have a data store for plugins in WooCommerce that would work well here. We should strive to not reinvent the wheel and consolidate efforts behind existing packages.

@joshuatf , I agree with you on this, that's why in this PR, I looked into the data store for plugins and I am using the installAndActivatePlugins method, to use the more standardized approach in WooCommerce.

To give a bit more context, the "unique data store specific to marketing" was created in March 2020 or earlier (see woocommerce/woocommerce-admin#3953), while the @woocommerce/data package was first released in April 2020, so I guess there probably wasn't a choice back then.

...ins/woocommerce-admin/client/marketing/overview-multichannel/DiscoverTools/DiscoverTools.tsx

...commerce-admin/client/marketing/overview-multichannel/DiscoverTools/useRecommendedPlugins.ts

...ins/woocommerce-admin/client/marketing/overview-multichannel/DiscoverTools/DiscoverTools.tsx

...commerce-admin/client/marketing/overview-multichannel/DiscoverTools/useRecommendedPlugins.ts

...ins/woocommerce-admin/client/marketing/overview-multichannel/DiscoverTools/DiscoverTools.tsx

plugins/woocommerce-admin/client/marketing/data/reducer.js

plugins/woocommerce-admin/client/marketing/data/actions.js

...ins/woocommerce-admin/client/marketing/overview-multichannel/DiscoverTools/DiscoverTools.tsx

isResolving is not accurate for the use of isLoading.

For consistency with the hook name.

ecgan · 2022-11-16T18:01:34Z

I think skipping the introduction of removeRecommendedPlugin might be a good idea if we can rely on installed plugins to hide this item from the bottom list.

@joshuatf , I'm not very keen of the idea. If we have that, then imagine the following scenario:

We first load the page. There are 8 items in the wp-data store, and 8 items in the recommended plugin list UI.
We install and activate a recommended extension. We rely on installed plugins to do the filtering without removeRecommendedPlugin, so there are still 8 items in the wp-data store, but 7 items in the recommended plugin list UI.
We reload the page. There will be 7 items in the wp-data store, and 7 items in the recommended plugin list UI.

I think it makes things a bit more complicated, and it would make maintainers to take more time to figure out:

why the numbers do not match in Step 2.
why the wp-data store item count is different in Step 2 and Step 3.

By having removeRecommendedPlugin, it makes the wp-data store match with the UI one to one. I think it is simpler and more straightforward.

joshuatf · 2022-11-16T18:57:11Z

so there are still 8 items in the wp-data store, but 7 items in the recommended plugin list UI.

I don't see anything particularly wrong with this scenario and there are plenty of cases where we don't display all items returned from an endpoint in the UI.

We reload the page. There will be 7 items in the wp-data store, and 7 items in the recommended plugin list UI.

If this is a real issue, what you're referring to here is your data store becoming out of sync with the endpoint, in which case, you can solve by 1 of 2 ways:

Invalidate the resolution for your endpoint after install so that it's refetched from the endpoint and updates the UI with the newly corrected values
Optimistically remove the item from your data store, assuming the install went well and the endpoint response will match in the future

What you appear to be attempting is option 2, however you've added the removal logic in your client-side instead of your data store. Dispatching actions in Redux should be akin to dispatching an event.

[client] Dispatches install event
[data] Receives event and begins resolution
[data] Optimistically removes the item from the data store
[client] Receives any data changes from the data store

By using removeRecommendedPlugin in the consumer directly, we make the consumer directly responsible for manipulating state inside of the data store. We should avoid creating actions that are purely setters.

There are quite a few cases of incorrectly using setters in this data store and across WooCommerce, but one reason I think this one may be more confusing is that someone calling removeRecommendedPlugin would expect that calling this continues to persist the removal upon refresh. This is obviously not the case and the plugin would have to be installed prior to calling this in order for this to work as intended.

ecgan · 2022-11-17T07:31:46Z

@joshuatf , thanks for the thorough explanation! 😃

one reason I think this one may be more confusing is that someone calling removeRecommendedPlugin would expect that calling this continues to persist the removal upon refresh. This is obviously not the case and the plugin would have to be installed prior to calling this in order for this to work as intended.

Got it, that makes sense. 👍

Invalidate the resolution for your endpoint after install so that it's refetched from the endpoint and updates the UI with the newly corrected values

That was actually what I did in the first place, but I removed that because I thought it would be good not to make a network request just for that, and I didn't want to display the loading indicator after invalidate resolution but I couldn't figure out how to make it work at that time.

I have removed removeRecommendedPlugin and used invalidateResolution approach in 38abe25 (#35542).

joshuatf

Thanks for making all the changes on this, @ecgan! This is testing well for me. Left one optional suggestion, but will leave that up to your discretion and pre-approving 😄

I didn't want to display the loading indicator after invalidate resolution but I couldn't figure out how to make it work at that time.

I think the suggestion I made on isLoading may help to discern between the initialization and refetching states. But let me know if I misunderstood this!

That was actually what I did in the first place, but I removed that because I thought it would be good not to make a network request just for that

I think the answer is that it kind of depends. If you need the local data store to be in sync with the network (which I think you suggested by wanting the UI and data store to match) then we need to invalidate.

But you could both invalidate and remove (or filter the item based on "active" plugin status) so that it both gets removed immediately and that the data store is in sync with the server changes.

joshuatf · 2022-11-17T16:49:22Z

...commerce-admin/client/marketing/overview-multichannel/DiscoverTools/useRecommendedPlugins.ts

-		[ category ]
-	);
+		return {
+			isLoading:


After install there's a very brief moment where the activated plugin remains in the list and buttons are no longer disabled.

Optionally, I think that creating separate isLoading and isInitializing states may help differentiate and disable the UI while being refetched. Something like:

isInitializing: ! hasFinishedResolution( selector, [ category ] ) && ! plugins.length, isLoading: ! hasFinishedResolution( selector, [ category ] );

Thanks @joshuatf , I think that's a good idea, I have made the change in 4db827e (#35542).

This is to have different loading state when first loading and when refetching after installing an extension.

This is so that the installed recommended plugin gets removed immediately and the data store is in sync with the server changes. This is similar to the previous removeRecommendedPlugin but with different naming following redux style guide.

ecgan · 2022-11-22T16:53:02Z

But you could both invalidate and remove (or filter the item based on "active" plugin status) so that it both gets removed immediately and that the data store is in sync with the server changes.

@joshuatf , I try to achieve the above in a3afcaf (#35542). It is a bit similar to the previous removeRecommendedPlugin, but now I name it as installAndActivateRecommendedPlugin based on the redux style guide. Because of the change, your review approval has been dismissed. Could you have another review on this please? I wonder if you have a suggestion on the action naming?

ecgan · 2022-11-22T17:28:38Z

On second thought, installAndActivateRecommendedPlugin may be confusing too like removeRecommendedPlugin, because someone calling installAndActivateRecommendedPlugin might expect it to persist but it may not be the case. 🤔

joshuatf

Testing well and no longer see the delay in the item being moved.

The action naming is better, but you're also right about it being possibly confusing. Left one more comment about this, but you can decide whether or not you want to implement. I've already held you up quite a bit on this PR 😅

joshuatf · 2022-11-23T17:28:42Z

plugins/woocommerce-admin/client/marketing/data/actions.js

 	}
 }
+
+export function* installAndActivateRecommendedPlugin(


I agree with your above comment that this name could be confusing, but I'll leave this up to you to decide whether or not you want to change it.

I still think the easiest fix for this might be to rely on the plugins data store to see if something is being installed and activated as opposed to removing it manually. Letting the data flow downstream (kicking off the install action and filtering in your view based on plugins that are installed/activated) is always going to be easier than handling state manually upstream (manually removing a plugin after kicking off an action in another data store).

The latter is prone to becoming out of sync and could errantly remove a recommended item when a plugin install or activation fails.

I am actually thinking about removing the marketing data store and have those marketing data (including the recommended plugins data) as local component state. That can be done in the future, possibly after Multichannel Marketing becomes the default experience and it replaces the current marketing page. When that happens, then it makes sense to have the data flow downstream and filter them in the view, and things are much simpler to see and understand without another overlapping data store. 🙂 👍

ecgan added 7 commits November 10, 2022 21:44

Add direct_install property to Plugin type.

c9b3fd6

One-click install and activate recommended plugins in DiscoverTools.

2adc4fb

The approach implemented here is similar to plugins/woocommerce-admin/client/tasks/fills/Marketing/index.tsx.

Reload installed extension list after installing and activating exten…

752a5eb

…sion.

Use removeRecommendedPlugin instead of invalidateResolution.

cf743e0

This is so that we don't need to make network request and display loading progress.

Code refactor on hooks and types.

d5dd68f

Rename usePlugins to useInstalledPlugins.

eb3bb27

Add changelog.

0192d92

ecgan added the focus: marketing label Nov 10, 2022

ecgan requested review from a team November 10, 2022 14:42

ecgan self-assigned this Nov 10, 2022

ecgan requested review from ilyasfoo and rjchow November 10, 2022 14:43

github-actions bot added focus: react admin plugin: woocommerce Issues related to the WooCommerce Core plugin. labels Nov 10, 2022

ecgan marked this pull request as ready for review November 10, 2022 14:54

Fix failed tests.

fac8079

ecgan mentioned this pull request Nov 11, 2022

CSS file is loaded in Marketing Overview page causing CSS issue woocommerce/google-listings-and-ads#1765

Closed

ecgan requested review from a team and removed request for ilyasfoo and rjchow November 14, 2022 10:42

joshuatf reviewed Nov 15, 2022

View reviewed changes

puntope reviewed Nov 16, 2022

View reviewed changes

plugins/woocommerce-admin/client/marketing/data/reducer.js Outdated Show resolved Hide resolved

puntope reviewed Nov 16, 2022

View reviewed changes

plugins/woocommerce-admin/client/marketing/data/actions.js Outdated Show resolved Hide resolved

puntope reviewed Nov 16, 2022

View reviewed changes

...ins/woocommerce-admin/client/marketing/overview-multichannel/DiscoverTools/DiscoverTools.tsx Outdated Show resolved Hide resolved

ecgan added 2 commits November 16, 2022 23:13

Rename el to plugin.

ccc4dd4

Remove unneeded selector variable and use inline selector string.

029d4b7

ecgan added 5 commits November 16, 2022 23:29

Change isResolving to hasFinishedResolution.

a3cde4b

isResolving is not accurate for the use of isLoading.

Remove unneeded useCallback.

ffbe20e

Add JSDOC.

aac6c8b

Rename UsePluginsType to UseInstalledPlugins.

39d823c

For consistency with the hook name.

Add more tests for DiscoverTools.

c649ad0

ecgan requested review from joshuatf and puntope November 16, 2022 18:02

Replace removeRecommendedPlugin with invalidateResolution.

38abe25

joshuatf previously approved these changes Nov 17, 2022

View reviewed changes

ecgan added 2 commits November 22, 2022 21:48

Use separate isInitializing and isLoading.

4db827e

This is to have different loading state when first loading and when refetching after installing an extension.

Code refactor with PluginsTabPanel.

768817d

ecgan dismissed joshuatf’s stale review via 768817d November 22, 2022 16:01

Use installAndActivate to replace refetch.

a3afcaf

This is so that the installed recommended plugin gets removed immediately and the data store is in sync with the server changes. This is similar to the previous removeRecommendedPlugin but with different naming following redux style guide.

ecgan requested a review from joshuatf November 22, 2022 16:53

Merge branch 'trunk' into feature/marketing-one-click-installation

23d46d5

Fix tests for DiscoverTools component.

e1ea4b0

joshuatf approved these changes Nov 23, 2022

View reviewed changes

Merge branch 'trunk' into feature/marketing-one-click-installation

90677fd

ecgan merged commit a811009 into trunk Nov 24, 2022

ecgan deleted the feature/marketing-one-click-installation branch November 24, 2022 13:46

github-actions bot added this to the 7.3.0 milestone Nov 24, 2022

github-actions bot mentioned this pull request Jan 13, 2023

Test WooCommerce 7.3 UVLabs/location-picker-at-checkout-for-woocommerce#59

Closed

ecgan mentioned this pull request Jan 27, 2023

Add Channels card into Multichannel Marketing page #36541

Merged

7 tasks

	const installAndActivate = ( slug: string ) => {
	setCurrentPlugin( slug );
	actionTask( 'marketing' );
	installAndActivatePlugins( [ slug ] )
	.then( ( response ) => {
	recordEvent( 'tasklist_marketing_install', {
	selected_extension: slug,
	installed_extensions: installedExtensions.map(
	( extension ) => extension.slug
	),
	section_order: pluginLists
	.map( ( list ) => list.key )
	.join( ', ' ),
	} );

	createNoticesFromResponse( response );
	setCurrentPlugin( null );
	onComplete( {
	redirectPath: getNewPath( { task: 'marketing' } ),
	} );
	} )
	.catch( ( response: { errors: Record< string, string > } ) => {
	createNoticesFromResponse( response );
	setCurrentPlugin( null );
	} );
	};

Add one-click installation for recommended extensions in multichannel Marketing page #35542

Add one-click installation for recommended extensions in multichannel Marketing page #35542

Uh oh!

Conversation

ecgan commented Nov 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

All Submissions:

Changes proposed in this Pull Request:

How to test the changes in this Pull Request:

Other information:

FOR PR REVIEWER ONLY:

Uh oh!

github-actions bot commented Nov 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results Summary

Uh oh!

ecgan commented Nov 11, 2022

Uh oh!

joshuatf left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ecgan commented Nov 16, 2022

Uh oh!

joshuatf commented Nov 16, 2022

Uh oh!

ecgan commented Nov 17, 2022

Uh oh!

joshuatf left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ecgan commented Nov 22, 2022

Uh oh!

ecgan commented Nov 22, 2022

Uh oh!

joshuatf left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ecgan commented Nov 10, 2022 •

edited

Loading

github-actions bot commented Nov 10, 2022 •

edited

Loading