-
Notifications
You must be signed in to change notification settings - Fork 57
feat(integrations): add inactive plugin state #4721
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.
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
base: trunk
Are you sure you want to change the base?
Changes from all commits
1ee5f52
d957f2e
2298649
91d88e2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -64,6 +64,23 @@ public function get_setup_url() { | |
| return $newsletters_configuration_manager->get_settings_url(); | ||
| } | ||
|
|
||
| /** | ||
| * Get the plugins this integration depends on, with their install/active status. | ||
| * | ||
| * @return array List of associative arrays with keys `slug`, `name`, `is_active`, `is_installed`. | ||
| */ | ||
| public function get_required_plugins() { | ||
| $status = \Newspack\Plugin_Manager::get_managed_plugin_status( 'newspack-newsletters' ); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Non-blocking:
|
||
| return [ | ||
| [ | ||
| 'slug' => 'newspack-newsletters', | ||
| 'name' => __( 'Newspack Newsletters', 'newspack-plugin' ), | ||
| 'is_active' => 'active' === $status, | ||
| 'is_installed' => 'uninstalled' !== $status, | ||
|
chickenn00dle marked this conversation as resolved.
|
||
| ], | ||
| ]; | ||
| } | ||
|
|
||
| /** | ||
| * Register the settings fields declared by this integration. | ||
| * | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -88,6 +88,15 @@ const AudienceIntegrations = ( props, ref ) => { | |
| } ); | ||
| }, [] ); | ||
|
|
||
| const handleActivatePlugin = useCallback( | ||
| pluginSlug => | ||
| apiFetch( { | ||
| path: `/newspack/v1/plugins/${ pluginSlug }/activate`, | ||
| method: 'POST', | ||
| } ).then( () => fetchSettings() ), | ||
|
chickenn00dle marked this conversation as resolved.
|
||
| [ fetchSettings ] | ||
| ); | ||
|
Comment on lines
+91
to
+98
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should have an in-flight guard, loading state, and |
||
|
|
||
| const sharedProps = { | ||
| integrations, | ||
| pendingChanges, | ||
|
|
@@ -97,6 +106,7 @@ const AudienceIntegrations = ( props, ref ) => { | |
| onFieldChange: handleFieldChange, | ||
| onSave: handleSave, | ||
| onToggleEnabled: handleToggleEnabled, | ||
| onActivatePlugin: handleActivatePlugin, | ||
| }; | ||
|
|
||
| return ( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,7 @@ | ||
| /** | ||
| * WordPress dependencies | ||
| */ | ||
| import { __ } from '@wordpress/i18n'; | ||
| import { __, sprintf } from '@wordpress/i18n'; | ||
| import { Icon, envelope } from '@wordpress/icons'; | ||
|
|
||
| /** | ||
|
|
@@ -32,7 +32,9 @@ const DEFAULT_ICON = { | |
| backgroundColor: colors[ 'neutral-100' ], | ||
| }; | ||
|
|
||
| export const SettingsSection = ( { integrations, loading, onToggleEnabled, history } ) => { | ||
| const getMissingPlugins = integration => ( integration.required_plugins || [] ).filter( plugin => ! plugin.is_active ); | ||
|
|
||
| export const SettingsSection = ( { integrations, loading, onToggleEnabled, onActivatePlugin, history } ) => { | ||
| const integrationIds = Object.keys( integrations ); | ||
|
|
||
| return ( | ||
|
|
@@ -54,22 +56,41 @@ export const SettingsSection = ( { integrations, loading, onToggleEnabled, histo | |
| { ! loading && integrationIds.length > 0 && ( | ||
| <Grid columns={ 2 } gutter={ 16 }> | ||
| { integrationIds.map( id => { | ||
| const { enabled, is_set_up: isSetUp, setup_url, name, description } = integrations[ id ]; | ||
| const integration = integrations[ id ]; | ||
| const { enabled, is_set_up: isSetUp, setup_url, name, description } = integration; | ||
| const missingPlugins = getMissingPlugins( integration ); | ||
| const uninstalledPlugin = missingPlugins.find( plugin => ! plugin.is_installed ); | ||
| const activatablePlugin = missingPlugins.length && ! uninstalledPlugin ? missingPlugins[ 0 ] : null; | ||
|
Comment on lines
+62
to
+63
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Non-blocking: Today's ESP has a single required plugin, so this is latent, but the framework is clearly designed for multi-plugin. Either chain activations or restructure to |
||
| const requirements = missingPlugins.length | ||
| ? sprintf( | ||
| /* translators: %s: comma-separated list of required plugin names. */ | ||
| __( 'Requires %s', 'newspack-plugin' ), | ||
| missingPlugins.map( plugin => plugin.name ).join( ', ' ) | ||
| ) | ||
| : undefined; | ||
| const isEnabled = enabled; | ||
| const needsSetup = ! isSetUp && !! setup_url; | ||
| const goToSetup = () => { | ||
| window.location.href = setup_url; | ||
| }; | ||
| let enableLabel = isSetUp ? __( 'Enable', 'newspack-plugin' ) : __( 'Connect', 'newspack-plugin' ); | ||
| let onEnable = needsSetup ? goToSetup : () => onToggleEnabled( id, true ); | ||
| if ( activatablePlugin ) { | ||
| enableLabel = __( 'Activate', 'newspack-plugin' ); | ||
| onEnable = () => onActivatePlugin( activatablePlugin.slug ); | ||
| } | ||
| return ( | ||
| <CardFeature | ||
| key={ id } | ||
| title={ name } | ||
| description={ description } | ||
| icon={ INTEGRATION_ICONS[ id ] || DEFAULT_ICON } | ||
| enabled={ isEnabled } | ||
| enableLabel={ isSetUp ? __( 'Enable', 'newspack-plugin' ) : __( 'Connect', 'newspack-plugin' ) } | ||
| requirements={ requirements } | ||
| requirementsActionable={ !! activatablePlugin } | ||
| enableLabel={ enableLabel } | ||
|
chickenn00dle marked this conversation as resolved.
|
||
| configureLabel={ needsSetup ? __( 'Configure', 'newspack-plugin' ) : undefined } | ||
| onEnable={ needsSetup ? goToSetup : () => onToggleEnabled( id, true ) } | ||
| onEnable={ onEnable } | ||
| onConfigure={ needsSetup ? goToSetup : () => history?.push( `/settings/${ id }` ) } | ||
| moreControls={ | ||
| isEnabled | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.