Remove business plugins (cleanup of #3079) #3170

Tug · 2016-02-08T23:10:25Z

In the context of removing the eCommerce feature from the Business plan upgrade, the plugins page becomes available to Jetpack sites only. The "Plugins" menu entry is being removed on #3079

This PR removes references to plugins related to the Business plan (eCommerce plugins) and display the "Plugins not supported" page at /plugins/:site for all wp.com sites.

Testing instructions

git fetch -a && git checkout remove/business-plugins && make run

Test 1 (Wp.com)

Go to Calypso and select a wordpress.com site. Check that the Plugins menu is not displayed (because the site does not have any plugin not because Sidebar: Remove Plugins menu item for all sites except Jetpack ones #3079 changes are included in this PR)
Go to /plugins/:site and check that the "Plugins not supported" page is displayed
Go to /plugins/browse/:site and check that the "Plugins not supported" page is displayed
Go to /plugins/akismet/:site and check that the "Plugins not supported" page is displayed
Go to /plugins and check that only your jetpack sites are counted.

Test 2 (Jetpack, test that nothing broke)

Go to Calypso and select a jetpack site. Check that the Plugins menu is displayed
Go to the Plugins page and check that your plugins are all there
Try to install, activate, deactivate, remove a plugin, (and update if you have one which is not updated) it should work without any error
Back to the main Plugins page, click on the Edit All and try to bulk remove, bulk update, bulk activate bulk deactivate, bulk autoupdate and bulk disable autoupdate your plugins, it should work without any error

Code Review Instructions

Review first commit first and check that all mentions and related features of wpcom plugins were removed
Review second commit

Reviewing

Product Review
Code Review

cc: @fabianapsimoes @drewblaisdell @scruffian

scruffian · 2016-02-12T15:39:53Z

@Tug - one small piece of feedback - the first commit here could have been smaller, which would have made it easier to grok. Not a big deal, but just something to think about.

Tug · 2016-02-12T15:45:43Z

I tried to but all those changes are related and once you decide to remove a feature it isn't easy to group the changes into several commit or you end up with a broken version between each commits.
Anyway I understand how it might be a pain to review at the moment, I'll try harder next time ;)

scruffian · 2016-02-12T16:38:41Z

These changes look good and work as expected. Its probably a good idea to have @drewblaisdell take a look too, since he knows this code better than me.

drewblaisdell · 2016-02-12T20:25:37Z

👍 The changes look good and work as expected. I added two commits to remove some remaining .com plugins code.

These changes aren't complex, but touch many lines of code - @enejb or @lezama would probably be interested in looking at this.

enejb · 2016-02-12T20:55:50Z

client/my-sites/plugins/plugin-autoupdate-toggle/index.jsx

@@ -52,10 +51,6 @@ module.exports = React.createClass( {
 			return null;
 		}

-		if ( ! this.props.wporg ) {


I haven't tested the code but I don't think this looks right.
A plugin can be a non .org plugin and still be visible in calypso.

The this.props.wporg indicated if a plugin is found in the .org repo or not.

yep, this is used for custom plugins, not for business ones

enejb · 2016-02-12T21:04:52Z

I haven't tested the code yet but it looks like some assumptions here are wrong.

There is a different between a not wporg plugin and a plugin that is found on .com.

A non wporg plugins is custom plugin that was build for the jetpack site that is not available for updates from .org. These plugins are often paid plugins for example. Sensei is one of them.

We currently don't support plugin updates and autoupdates for such plugins.

johnHackworth · 2016-02-12T21:47:54Z

client/my-sites/plugins/plugin-list-header/index.jsx

-		const { isWpCom } = this.props;
-		let options = [];
-		let actions = [];
+		const options = [];


wow, I didn't know you can push new elements in an array declared as constant! nice to know!

:) Yeah, the difference between const and let is related to assignment, not mutation.

Tug · 2016-02-13T19:32:30Z

client/my-sites/plugins/plugin-site/plugin-site.jsx

@@ -19,10 +18,6 @@ module.exports = React.createClass( {
 			return;
 		}

-		if ( ! this.props.wporg ) {


This one is a bit confusing though.
Since PluginSite is used at /plugins/:pluginName to list the sites a plugin can be install on, I think it makes sense to disable the "Available sites" list component for non wporg plugins (in plugins.jsx)
Otherwise we'll get a nice error if we click on Install:

lezama · 2016-02-14T22:07:12Z

client/my-sites/plugins/main.jsx

-					this.props.wporgFetchPluginData( plugin.slug );
-				}
-				return assign( {}, plugin, pluginData );
+			let pluginData = WporgPluginsSelectors.getPlugin( this.props.wporgPlugins, plugin.slug );


this could be a const :)

Tug · 2016-02-14T23:32:09Z

@lezama Thank you for reviewing,
I followed your comments and did some other code cleanup ;)

fabianapsimoes · 2016-02-15T15:14:59Z

Product 👍

Very nice, Tug. Thanks for the detailed instructions!

lezama · 2016-02-15T15:33:51Z

Thanks Tug! I think @enejb or @johnHackworth should be the ones giving the final 👍

johnHackworth · 2016-02-15T16:22:49Z

I've been testing it and both code and product-wise looks good

good work!

enejb · 2016-02-15T17:25:32Z

Great work Tug!

Tug · 2016-02-15T18:03:50Z

Thanks, it's always fun removing code ;)

stephanethomas · 2016-02-16T11:16:46Z

client/lib/plugins/utils.js

@@ -207,42 +204,6 @@ PluginUtils = {
 		} );
 	},

-	addWpcomIcon: function( plugin ) {
-		if ( plugin.slug && plugin.wpcom ) {
-			return _assign( {}, plugin, { icon: '/calypso/images/upgrades/plugins/' + plugin.slug + '.png' } );


We should also remove the corresponding images.

Well spotted!

stephanethomas · 2016-02-16T13:13:29Z

Great job, that wasn't an easy one!

… given by `this.translate`

- Fix deactivating bulk plugins - Prefer const instead of let - Use Object.assign which is available in Babel now - Simplify a render method: print the elements of a dropdown selector directly instead of appendinf them to an array - Remove unused code

Remove business plugins (cleanup of #3079)

Tug added [Status] In Progress Jetpack Plugins labels Feb 8, 2016

Tug self-assigned this Feb 8, 2016

Tug added this to the Amber: Backlog milestone Feb 8, 2016

Tug force-pushed the remove/business-plugins branch from c321890 to 4dfa2aa Compare February 8, 2016 23:16

Tug mentioned this pull request Feb 8, 2016

Remove Plugins page for non-Jetpack sites (cleanup of #3079) #3166

Closed

Tug force-pushed the remove/business-plugins branch 2 times, most recently from 359e8a7 to 5dfe4c6 Compare February 9, 2016 14:23

fabianapsimoes removed this from the Amber: Backlog milestone Feb 11, 2016

Tug force-pushed the remove/business-plugins branch 4 times, most recently from 2e5defe to 6153696 Compare February 12, 2016 11:28

Tug added this to the Amber: Backlog milestone Feb 12, 2016

Tug added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed [Status] In Progress labels Feb 12, 2016

enejb reviewed Feb 12, 2016
View reviewed changes

johnHackworth reviewed Feb 12, 2016
View reviewed changes

drewblaisdell added [Status] Needs Author Reply and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Feb 12, 2016

Tug reviewed Feb 13, 2016
View reviewed changes

Tug force-pushed the remove/business-plugins branch 2 times, most recently from a427990 to 5d6186a Compare February 13, 2016 19:48

lezama reviewed Feb 14, 2016
View reviewed changes

Tug force-pushed the remove/business-plugins branch from 3d07c22 to f918f1c Compare February 15, 2016 00:13

johnHackworth added [Status] Ready to Merge and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Feb 15, 2016

stephanethomas reviewed Feb 16, 2016
View reviewed changes

fabianapsimoes removed this from the Amber: Backlog milestone Feb 24, 2016

fabianapsimoes removed the Business Plugins label Feb 25, 2016

Tug force-pushed the remove/business-plugins branch 3 times, most recently from 8040c4a to 575f899 Compare February 25, 2016 19:31

Tug added 4 commits February 25, 2016 21:07

Plugins: remove business plugins

4631897

Framework: Fix warning in AcceptDialog due to a complex message being…

eff37a8

… given by `this.translate`

Plugins: Remove business plugins images

cdf53d3

Tug force-pushed the remove/business-plugins branch from 575f899 to cdf53d3 Compare February 25, 2016 20:08

Tug added a commit that referenced this pull request Feb 25, 2016

Merge pull request #3170 from Automattic/remove/business-plugins

48392a2

Remove business plugins (cleanup of #3079)

Tug merged commit 48392a2 into master Feb 25, 2016

Tug deleted the remove/business-plugins branch February 25, 2016 20:24

Tug removed the [Status] Ready to Merge label Feb 25, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove business plugins (cleanup of #3079) #3170

Remove business plugins (cleanup of #3079) #3170

Tug commented Feb 8, 2016

scruffian commented Feb 12, 2016

Tug commented Feb 12, 2016

scruffian commented Feb 12, 2016

drewblaisdell commented Feb 12, 2016

enejb Feb 12, 2016

johnHackworth Feb 12, 2016

enejb commented Feb 12, 2016

johnHackworth Feb 12, 2016

drewblaisdell Feb 12, 2016

Tug Feb 13, 2016

lezama Feb 14, 2016

Tug commented Feb 14, 2016

fabianapsimoes commented Feb 15, 2016

lezama commented Feb 15, 2016

johnHackworth commented Feb 15, 2016

enejb commented Feb 15, 2016

Tug commented Feb 15, 2016

stephanethomas Feb 16, 2016

Tug Feb 16, 2016

stephanethomas commented Feb 16, 2016

Remove business plugins (cleanup of #3079) #3170

Remove business plugins (cleanup of #3079) #3170

Conversation

Tug commented Feb 8, 2016

Testing instructions

Test 1 (Wp.com)

Test 2 (Jetpack, test that nothing broke)

scruffian commented Feb 12, 2016

Tug commented Feb 12, 2016

scruffian commented Feb 12, 2016

drewblaisdell commented Feb 12, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

enejb commented Feb 12, 2016

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

Tug commented Feb 14, 2016

fabianapsimoes commented Feb 15, 2016

lezama commented Feb 15, 2016

johnHackworth commented Feb 15, 2016

enejb commented Feb 15, 2016

Tug commented Feb 15, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stephanethomas commented Feb 16, 2016