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.

Sign up for GitHub

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

Jetpack Sites: Use active modules list added to /me/sites endpoint #1700

Merged
merged 3 commits into from
Dec 17, 2015
Merged

Jetpack Sites: Use active modules list added to /me/sites endpoint #1700

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1b2ff16
Jetpack Sites: Use new list of active modules on /me/sites endpoint
lezama Dec 16, 2015
68f9027
Jetpack Site: check if `this.modules` is defined, just in case ¯\_(ツ)_/¯
lezama Dec 16, 2015
17c4e89
Jetpack Site: `includes` is only supported on Chrome 47
lezama Dec 17, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
97 changes: 26 additions & 71 deletions client/lib/site/jetpack.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
/**
* External dependencies
*/
var debug = require( 'debug' )( 'calypso:site:jetpack' ),
find = require( 'lodash/collection/find' );
var debug = require( 'debug' )( 'calypso:site:jetpack' );

/**
* Internal dependencies
Expand All @@ -27,6 +26,10 @@ function JetpackSite( attributes ) {

JetpackSite.prototype.updateComputedAttributes = function() {
JetpackSite.super_.prototype.updateComputedAttributes.apply( this );
if ( this.jetpack_modules ) {
this.modules = this.jetpack_modules;
delete this.jetpack_modules;
}
this.hasMinimumJetpackVersion = versionCompare( this.options.jetpack_version, config( 'jetpack_min_version' ) ) >= 0;
// Only sites that have the minimum Jetpack Version and siteurl matches the main_network_site can update plugins
// unmapped_url is more likely to equal main_network_site because they should both be set to siteurl option
Expand All @@ -46,7 +49,7 @@ JetpackSite.prototype.canManage = function() {
// for versions 3.4 and higher, canManage can be determined by the state of the Manage Module
if ( this.versionCompare( '3.4', '>=' ) ) {
// if we haven't fetched the modules yet, we default to true
if ( this.modulesFetched ) {
if ( this.modules ) {
return this.isModuleActive( 'manage' );
}
return true;
Expand All @@ -72,32 +75,6 @@ JetpackSite.prototype.fetchAvailableUpdates = function() {
}.bind( this ) );
};

JetpackSite.prototype.fetchModules = function() {
if ( ! this.hasMinimumJetpackVersion || this.fetchingModules ) {
return;
}

this.fetchingModules = true;
wpcom.undocumented().jetpackModules( this.ID, function( error, data ) {
if ( error || ! data.modules ) {
debug( 'error fetching Modules data from api', error );
return;
}

this.fetchingModules = false;
this.modulesFetched = true;
this.set( { modules: data.modules } );
this.emit( 'change' );
}.bind( this ) );
};

JetpackSite.prototype.getModule = function( moduleId ) {
if ( this.modulesFetched ) {
return find( this.modules, { id: moduleId } );
}
this.fetchModules();
};

JetpackSite.prototype.isSecondaryNetworkSite = function() {
return this.options.is_multi_site &&
this.options.unmapped_url !== this.options.main_network_site;
Expand All @@ -109,13 +86,7 @@ JetpackSite.prototype.isMainNetworkSite = function() {
};

JetpackSite.prototype.isModuleActive = function( moduleId ) {
var module = this.getModule( moduleId );

if ( module ) {
return module.active;
}

return false;
return this.modules && this.modules.indexOf( moduleId ) > -1;
};

JetpackSite.prototype.verifyModulesActive = function( moduleIds, callback ) {
Expand All @@ -125,12 +96,6 @@ JetpackSite.prototype.verifyModulesActive = function( moduleIds, callback ) {
moduleIds = [ moduleIds ];
}

if ( ! this.modulesFetched ) {
this.fetchModules();
this.once( 'change', this.verifyModulesActive.bind( this, moduleIds, callback ) );
return;
}

modulesActive = moduleIds.every( function( moduleId ) {
return this.isModuleActive( moduleId );
}, this );
Expand All @@ -152,8 +117,8 @@ JetpackSite.prototype.handleError = function( error, action, plugin, module ) {
return;
}

if ( module && module.name ) {
moduleTranslationArgs = { args: { module: module.name, site: this.domain } };
if ( module ) {
moduleTranslationArgs = { args: { module: module, site: this.domain } };
}

remoteManagementUrl = this.getRemoteManagementURL();
Expand Down Expand Up @@ -249,63 +214,53 @@ JetpackSite.prototype.callHome = function() {
};

JetpackSite.prototype.activateModule = function( moduleId, callback ) {
var module = this.getModule( moduleId );
debug( 'activate module', moduleId );

if ( ! module.id ) {
if ( ! moduleId ) {
callback && callback( new Error( 'No id' ) );
return;
}

if ( module.active ) {
if ( this.isModuleActive( moduleId ) ) {
// Nothing to do
callback();
return;
}

this.toggleModule( module, callback );
this.toggleModule( moduleId, callback );
};

JetpackSite.prototype.deactivateModule = function( moduleId, callback ) {
var module = this.getModule( moduleId );
debug( 'deactivate module', moduleId );

if ( ! module.id ) {
callback && callback( new Error( 'No id' ) );
return;
}

if ( ! module.active ) {
if ( ! this.isModuleActive( moduleId ) ) {
// Nothing to do
callback();
return;
}

this.toggleModule( module, callback );
this.toggleModule( moduleId, callback );
};

JetpackSite.prototype.toggleModule = function( module, callback ) {
var method;
JetpackSite.prototype.toggleModule = function( moduleId, callback ) {
const isActive = this.isModuleActive( moduleId ),
method = isActive ? 'jetpackModulesDeactivate' : 'jetpackModulesActivate',
prevActiveModules = Object.assign( {}, this.modules );

if ( typeof module === 'string' ) {
module = this.getModule( module );
if ( isActive ) {
this.modules = this.modules.filter( module => module !== moduleId );
} else {
this.modules = [ ...this.modules, moduleId ];
}

method = module.active ? 'jetpackModulesDeactivate' : 'jetpackModulesActivate';

wpcom.undocumented()[ method ]( this.ID, module.id, function( error, data ) {
debug( 'module toggled', this.URL, module.id, data );
wpcom.undocumented()[ method ]( this.ID, moduleId, function( error, data ) {
debug( 'module toggled', this.URL, moduleId, data );

if ( error ) {
debug( 'error toggling module from api', error );
this.modules = prevActiveModules;
this.emit( 'change' );
callback && callback( error );
return;
}

module.active = ! module.active;
this.emit( 'change' );
callback && callback( null, data );
callback && callback( error, data );
}.bind( this ) );

this.emit( 'change' );
Expand Down
2 changes: 1 addition & 1 deletion client/lib/sites-list/list.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -459,7 +459,7 @@ SitesList.prototype.getSelectedOrAllJetpackCanManage = function() {
return site.jetpack &&
site.capabilities &&
site.capabilities.manage_options &&
( ! site.modules || site.canManage() );
site.canManage();
} );
};

Expand Down
2 changes: 1 addition & 1 deletion client/my-sites/menus/main.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ var Menus = React.createClass( {
site = this.props.site,
menu;

if ( site && site.jetpack && site.modulesFetched && ! site.canManage() ) {
if ( site && site.jetpack && ! site.canManage() ) {
return this.renderJetpackManageDisabledMessage( site );
}

Expand Down
9 changes: 0 additions & 9 deletions client/my-sites/plugins/controller.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,15 +182,6 @@ controller = {
renderPluginsBrowser( context, siteUrl );
},

jetpackManageActive: function( context, next ) {
sites.getSelectedOrAll().forEach( function( site ) {
if ( site.jetpack ) {
site.fetchModules();
}
} );
next();
},

jetpackCanUpdate: function( filter, context, next ) {
let redirectToPlugins = false,
selectedSites = sites.getSelectedOrAllWithPlugins();
Expand Down
6 changes: 3 additions & 3 deletions client/my-sites/plugins/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ module.exports = function() {
}

if ( config.isEnabled( 'manage/plugins' ) ) {
page( '/plugins', controller.siteSelection, controller.navigation, pluginsController.jetpackManageActive, pluginsController.plugins.bind( null, 'all' ) );
page( '/plugins', controller.siteSelection, controller.navigation, pluginsController.plugins.bind( null, 'all' ) );
[ 'active', 'inactive', 'updates' ].forEach( function( filter ) {
page( '/plugins/' + filter + '/:site_id?', controller.siteSelection, controller.navigation, pluginsController.jetpackManageActive, pluginsController.jetpackCanUpdate.bind( null, filter ), pluginsController.plugins.bind( null, filter ) );
page( '/plugins/' + filter + '/:site_id?', controller.siteSelection, controller.navigation, pluginsController.jetpackCanUpdate.bind( null, filter ), pluginsController.plugins.bind( null, filter ) );
} );
page( '/plugins/:plugin/:business_plugin?/:site_id?', controller.siteSelection, controller.navigation, pluginsController.jetpackManageActive, pluginsController.plugin );
page( '/plugins/:plugin/:business_plugin?/:site_id?', controller.siteSelection, controller.navigation, pluginsController.plugin );
}
};
4 changes: 1 addition & 3 deletions client/my-sites/plugins/main.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -775,9 +775,7 @@ export default React.createClass( {
);
}

if ( selectedSite &&
selectedSite.modulesFetched &&
! selectedSite.canManage() ) {
if ( selectedSite && ! selectedSite.canManage() ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to note somewhere: canManage only exists on Jetpack sites, so we need to either check selectedSite.jetpack before calling it, or add the same method to a .com site in a way that makes sense. I'm not sure what the best solution is, but we pushed an update to check if the site is a Jetpack site before calling this to fix it in the meantime.

return (
<Main>
<JetpackManageErrorPage
Expand Down
4 changes: 1 addition & 3 deletions client/my-sites/plugins/plugin.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -303,9 +303,7 @@ export default React.createClass( {
return this.getPluginDoesNotExistView( selectedSite );
}

if ( selectedSite &&
selectedSite.modulesFetched &&
! selectedSite.canManage() ) {
if ( selectedSite && ! selectedSite.canManage() ) {
return (
<MainComponent>
<JetpackManageErrorPage
Expand Down
2 changes: 1 addition & 1 deletion client/my-sites/plugins/plugins-browser/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ module.exports = React.createClass( {
render() {
const selectedSite = this.props.sites.getSelectedSite();
if ( this.state.accessError ||
( selectedSite && selectedSite.modulesFetched && ! selectedSite.canManage() )
( selectedSite && ! selectedSite.canManage() )
) {
return this.renderAccessError( selectedSite );
}
Expand Down
8 changes: 2 additions & 6 deletions client/my-sites/plugins/plugins-manage-mixin.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,10 @@ module.exports = {
if ( ! selectedSite ) {
return true;
}
if ( ! selectedSite || ! selectedSite.modulesFetched ) {
return true;
}
if ( ! selectedSite.getModule( 'manage' ) ||
! selectedSite.getModule( 'manage' ).active ) {
if ( ! selectedSite.isModuleActive( 'manage' ) ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should also add there another if:

if ( ! selectedSite.modules ) {
    return true;
}

for the case where we don't know yet if it can manage or not

Copy link
Contributor

Choose a reason for hiding this comment

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

Since the modules are now returned part of the Site object / me/sites request, I don't think the scenario you describe is possible.

Copy link
Contributor

Choose a reason for hiding this comment

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

yup, I wasn't getting that there are no delay now. \o/

return false;
}
return true;
},

};
};
2 changes: 1 addition & 1 deletion client/my-sites/site-settings/form-base.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ module.exports = {
error,
this.props.site.isModuleActive( module ) ? 'deactivateModule' : 'activateModule',
{},
this.props.site.getModule( module )
module
);
} else {
if( 'protect' === module ) {
Expand Down
2 changes: 1 addition & 1 deletion client/my-sites/site-settings/section-security.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ module.exports = React.createClass( {
);
}

if ( site.modulesFetched && ! site.canManage() ) {
if ( ! site.canManage() ) {
return (
<JetpackManageErrorPage
template="optInManage"
Expand Down
2 changes: 0 additions & 2 deletions client/post-editor/editor-sharing/publicize-options.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,6 @@ module.exports = React.createClass( {
return;
}

this.props.site.fetchModules();

// Refresh the list of connections so that the user is given the latest
// possible state. Also prevents a possible infinite loading state due
// to connections previously returning a 400 error
Expand Down
12 changes: 1 addition & 11 deletions client/post-editor/media-modal/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,6 @@ module.exports = React.createClass( {
MediaActions.setLibrarySelectedItems( nextProps.site.ID, [] );
}

if ( nextProps.site && nextProps.site.jetpack && ! nextProps.site.modulesFetched &&
( ! this.props.site || this.props.site.ID !== nextProps.site.ID ) ) {
nextProps.site.fetchModules();
}

if ( ! nextProps.visible || this.props.visible === nextProps.visible ) {
return;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This makes me happy to see removed 😃

Expand All @@ -76,11 +71,6 @@ module.exports = React.createClass( {
},

componentDidMount: function() {
var site = this.props.site;
if ( site && site.jetpack && ! site.modulesFetched ) {
site.fetchModules();
}

debug( '%s component mounted.', this.constructor.name );

this.statsTracking = {};
Expand Down Expand Up @@ -304,7 +294,7 @@ module.exports = React.createClass( {
action: 'confirm',
label: this.translate( 'Continue' ),
isPrimary: true,
disabled: isDisabled || ! this.props.site || ( this.props.site.jetpack && ! this.props.site.modulesFetched ),
disabled: isDisabled || ! this.props.site,
onClick: this.setView.bind( this, ModalViews.GALLERY )
} );
} else {
Expand Down
2 changes: 1 addition & 1 deletion shared/my-sites/themes/main.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ var Themes = React.createClass( {
return <JetpackUpgradeMessage site={ site } />;
}

if ( isJetpack && jetpackEnabled && site.modulesFetched && ! site.canManage() ) {
if ( isJetpack && jetpackEnabled && ! site.canManage() ) {
return <JetpackManageDisabledMessage site={ site } />;
}

Expand Down