Marketing Plugins: Use plugin package

[psealock]

wc-admin now has Plugin manipulation as part of its packages:

https://github.com/woocommerce/woocommerce-admin/tree/master/packages/data/src/plugins

Marketing should consider using the package or contributing to it instead of its own plugin operations, https://github.com/woocommerce/woocommerce-admin/tree/master/client/marketing/data.

cc @jconroy

[jconroy]

Nice thanks @psealock

[danielbitzer]

Hi @psealock, the new plugins data store returns only an array of plugin slugs for getInstalledPlugins and getActivePlugins selectors.

For the marketing tab we need to get a lot more data about the installed plugins. See the current marketing endpoint: https://github.com/woocommerce/woocommerce-admin/blob/26f18357ac94f91d459548a830f396c041477118/src/API/MarketingOverview.php#L184-L186

Do you have any recommendations as to the best way forward in this case?

[jconroy]

@danielbitzer ahh right

(just musing, and even if we had a better API for the plugins themselves to provide the data we use we'd still need to actually pull it I guess)

[psealock]

@danielbitzer Just noting that there is a flag to add plugins to a whitelist, I know this isn't what you're asking about.

https://github.com/woocommerce/woocommerce-admin/blob/bd6ad79ebccdf1ee3d0036681aa3de548fe456c6/src/API/Plugins.php#L302-L304

For the marketing tab we need to get a lot more data about the installed plugins.

The class InstalledExtensions just gets data about plugins, right? How about adding a method to the plugins API in src/API/Plugins.php? Maybe /get-data?

@joshuatf What do you think?

[danielbitzer]

The class InstalledExtensions just gets data about plugins, right?

Yes and a lot of the data is specific to marketing.

How about adding a method to the plugins API in src/API/Plugins.php? Maybe /get-data?

I think that would work well as long as we can inject marketing specific props to the data via a filter.

[danielbitzer]

How about adding a method to the plugins API in src/API/Plugins.php? Maybe /get-data?

@psealock I'm not quite sure what the best way to proceed with this is. In the marketing tab, when a plugin is activated we need to access certain props like whether the plugin is configured or not. This is something that we need to fetch dynamically after activation.

I think there are few ways forward:

1. The getActivePlugins and getInstalledPlugins actions return objects with data rather than just the plugin slugs.
2. We add a getPluginsData action that retrieves all installed plugins with props like name, slug, isActive and with the option to add more props via a filter. The problem with this solution is it makes getActivePlugins and getInstalledPlugins redundant.
3. Add getPluginData( pluginSlug ) action that retrieves the data for a single plugin. I'm not sure how this would work from a redux perspective. Mutating the state would be more complex and we would probably need to perform more API requests than option 2.

[psealock]

The problem with this solution is it makes getActivePlugins and getInstalledPlugins redundant.

Maybe thats ok. Combining getActivePlugins and getInstalledPlugins into getPluginData could remove an unnecessary request and simplify the API.

Add getPluginData( pluginSlug ) action that retrieves the data for a single plugin

Or you can make it optionally accept an array of slugs to limit the response.

@joshuatf What do you think?

[jconroy]

@joshuatf apologies for the double ping, wanted check in to see if you'd had a chance to look over this?

[joshuatf]

No problem, @jconroy 😄

We add a getPluginsData action that retrieves all installed plugins with props like name, slug, isActive and with the option to add more props via a filter. The problem with this solution is it makes getActivePlugins and getInstalledPlugins redundant.

I'm not opposed to a solution like this as long as the schema makes sense and is extensible. Something like:

{
  [pluginName]: {
    isActive: false,
    isInstalled: true, // This is probably redundant and not necessary since the plugin key exists.
    data: {}, // Additional data that can be filtered via a hook.
}

One caveat to this is that we'll need to re-parse the entire plugin data array on each install/activation and return that in a response to update the local redux store.

Add getPluginData( pluginSlug ) action that retrieves the data for a single plugin. I'm not sure how this would work from a redux perspective. Mutating the state would be more complex and we would probably need to perform more API requests than option 2.

This is more or less the way we currently handle things. However, since we preload this data, multiple API requests have not been necessary. The same could be done for this issue, but I can see how a plugin data array make this a more intuitive API.

I'm also not sure of long-term consequences to preloading as much as we do.

[danielbitzer]

we'll need to re-parse the entire plugin data array on each install/activation and return that in a response to update the local redux store

Wouldn't it be possible to only return plugin data for plugins that have been modified in the response? E.g. activate plugin X and Y, return new data for plugins X and Y and then replace just those 2 entries in the plugins store.

[becdetat]

@psealock @jconroy @danielbitzer @joshuatf There hasn't been any action on this ticket in a few months, does it need to stick around or can it be closed?

[jconroy]

Hey @becdetat It can stick around, I think we are somewhat blocked on approach (Dan had a question) and it hasn't been a priority but we'll get back here eventually

[becdetat]

NP

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had any recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[moon0326]

I'm closing this as this has been stale for some time. Please feel free to open it again or submit a new issue to discuss.