Implement migration logic and UI from old PL modules to new standalone plugins in PL settings screen

[felixarntz]

Feature Description

Follow-up to #651: For any site for which the old module slugs are still set in the option for active modules, an additional UI component should be added to the updated PL settings screen to make it easy, yet explicitly intentional, for the admin user to install and activate the respective standalone plugins with a single click.

This will require defining a mapping between the old module slugs and their standalone plugins, so that the plugin can determine where to point the user.

Requirements

• If any of the module slugs that are already available as standalone plugins are still remaining in the option for active modules, some sort of prompt (e.g. an admin notice) should be displayed on the Settings > Performance screen.
  • The check for whether this prompt needs to be shown should be a reusable function (important for Implement admin pointer to indicate to the user they need to activate the new standalone plugins #653).
  • Which exact modules need to be replaced with their plugins is less important to be reusable, though it could certainly be part of a reusable function too if it makes sense.
• The prompt shown on the screen should include:
  • A brief explanation (1-2 sentences) to the user that they have modules activated which will soon be removed from the Performance Lab plugin, and that they need to instead install/activate the relevant standalone plugins
  • A list of which modules are affected (if it's a single module, we could alternatively consider putting it into the floating text, since a list of 1 item is a bit odd)
  • One of the following:
    • If the current user has the install_plugins and activate_plugins capabilities: A primary CTA button (see below) to migrate the active legacy modules to their respective standalone plugins
    • If the current user lacks any of the two capabilities: A CTA message prompting the user to contact an administrator with access to installing/activating plugins
• When the primary CTA button is clicked, logic (could be implemented with a full page reload/form submission, or AJAX, whatever works best) should be run that:
  • Installs and activates the respective standalone plugins as needed.
  • Deactivates the respective legacy modules.

[felixarntz]

@10upsimon The requirements here are extremely detailed, and I would argue go too far into detail, to the degree it is very challenging to review them as it's almost like a code review without code :)

I took a look at your draft PR and left some feedback (#806 (review)), which is similar to what I would have as feedback on the requirements: Rather than the detailed list of what to implement, I think the description here would benefit from a much higher-level outline of requirements, while instead providing context for why the requirements are like that. For example, I am questioning why we need to introduce REST endpoints when WP core already provides AJAX functions and REST endpoints to manage plugin installation, activation, etc.

Since the PL plugin is a WP core feature plugin, we should stick as much as possible to what core provides us, rather than working with more "elaborate" custom code and architecture patterns, also to keep our own maintenance burden as little as possible. While the code in WP core itself is probably not the most elegant and maintainable, us implementing custom "duplicate" versions of it overall makes our maintenance burden worse, unless absolutely necessary for a technical limitation with core's code that we can't work around.

Were there specific hurdles you ran into while trying to replicate the behavior from the Plugins > Add New screen? If so, what were they specifically, what were the core limitations, and why did they lead you to propose what is outlined in the description? This kind of context would be helpful to have. Maybe there are valid reasons to reimplement parts of the functionality core already provides, but without such context to me some of the requirements seem like duplicate work.

[10upsimon]

@felixarntz (cc @mukeshpanchal27) I've taken a stab at simplifying (in terms of technicalities) the requirements, although I've found it rather challenging to not go into detail for context sake. Therefore, I've split the requirements section into two sub sections, a "summary" and "detailed" section. The summary is a very top down view of the requirements, while the detailed section goes far more into describing the intended outcome while making it quite clear where and how core currently implements and handles certain plugin actions.

In terms of the challenges (including suspected hurdles) I faced when exploring the usage of the existing updates.js logic (replying to your last comment @felixarntz ), I've surfaced some of this in a lighter manner in the updated requirements, but specifically:

• The file applies logic for not just plugins, but also themes, and thus could be seen as largely unnecessary in terms of size to usable logic ratio.
• The file employs logic for plugin imports as well, similar to the point above this may be seen as largely unnecessary in terms of size to usable logic ratio, but if simply copying the logic and lightening it up, this should not be an issue.
• Initially I encountered some challenges after tweaking some logic within updates.js (during discovery) and mistakenly identified this as a failed check agains pagenow, but I now see that this only applies to authentication modal callbacks, and plugin imports, so this point is now moot.
• The updates script enqueues thickbox as well, and triggers the lightbox when the plugin card is triggered. This is undesirable (I believe). This could be solved by trimming of the logic that triggers such light box behaviour, removal of the dependency itself (in the new script no doubt) and removal of any generated URLs that were used/forwarded to the lightbox.
• There are ajax referrer checks in existing plugin ajax action hooks that expect the value to be updates. I am not sure if this is actually a problem as I was unsure of this was set by the logic as a result of the script name (updates) itself, and this warrants looking into. It may not be a challenge at all.

Looking more into the updates.js file while updating the requirements (and this comment), there are a ton of useful things it does do, handling session timeouts via the authentication modal being a very big one, so i do believe we should be using this logic as much as we possibly can.

[felixarntz]

Thank you @10upsimon, the updated version is much more clear to me and outlines your thoughts and considerations in more detail.

Leaving a few follow up notes:

• I think in this instance we should prioritize the ease of maintenance over performance. As in, the fact that updates.js loads lots of things we don't care about on our page shouldn't be a concern, as re-implementing (even if copied) the relevant logic from this file would be much more challenging, especially as the file already takes care of lots of advanced quirks, such as accessible update feedback etc.
• About the markup, as far as I can tell there shouldn't be any conflicts if we reuse a very similar DOM structure and markup to the plugin cards in Plugins > Add New. You mention this already in the requirements, and I think by doing that we should be well set up for the selectors etc. used in updates.js.
• Regarding thickbox, I may be missing something but as far as I can see that shouldn't be a concern either since it only triggers on DOM elements that match a .thickbox selector. We can leave that off on our rendered plugin cards.
• The pagenow global is probably the most complicated piece. I don't think we can realistically override it (e.g. to enforce plugin-install), as that may cause unintended side effects. But at first glance (we need to verify) it also looks like the pagenow specific logic is only used to modify some DOM elements in ways we can potentially adjust separately, i.e. in our own event listener executed alongside the corresponding event listener in updates.js. And most importantly, there are only 4 instances where pagenow is checked to be plugin-install, which is the one value that matters for anything that we want to do on our page. So those hacks would only be needed in few instances. Potentially we can even wrap certain wp.updates.* functions by accessing the original functions, but wrapping them in our own functions that we then replace the respective wp.updates.* function with. Since that would only apply to our own admin page, I think that would be fine.
• You mention the AJAX action to update a plugin, but that is not something we would want to expose in our screen. I think we're aligned on that since you rightfully don't mention it in the high-level requirements, but just wanted to clarify. In other words: While the Plugins > Add New screen supports a state where a card's primary button says "Update Now", I think in our screen it's sufficient to go with the following 3 possible states for each card:
  • "Install Now" primary button (if it's easy, we can include the "More Details" secondary popup link here, but not crucial really)
  • "Activate Now" primary button, "Delete" secondary red link/button
  • "Active" disabled primary button, "Deactivate" secondary red link/button

I think at this point there's little value in updating the requirements here further since they overall look very solid. Let's go ahead and try what happens in reality when using the updates.js script. I would suggest you start a draft PR to:

• Render the relevant plugin cards on the server-side, employing the markup duplicated from the plugin cards in core as much as possible/needed.
  • You can use the plugins_api() function to retrieve the information on the plugins needed from wordpress.org. Either make individual plugin-information requests for each plugin we expose, or you can make a single query-plugins request, passing 'author' => 'wordpressdotorg' (since all our standalone plugins are and will be by that author), and then extract our specific plugins out of the response data. The latter is a bit wasteful but if we have too many plugins, it may still be faster than individual requests per plugin, so that's a tradeoff to consider.
• Enqueue the updates.js file.
• See what works and what doesn't. We can then start making tweaks in our own JS code or potentially PHP code related to the AJAX actions as needed.

Let me know what you think!

[felixarntz]

@10upsimon I am just realizing that this issue was originally intended for something different than what we're discussing now:

• The original intent was to use this issue for implementing the migration logic and UI to get from modules to plugins.
• What we're discussing here so far is technically part of Update PL settings screen with new UI to manage standalone plugins and built-in modules #651, as we're merely speaking about the general UI to manage standalone plugins, not related to migrating the old modules.

Not a big deal, but right now the two issues are therefore somewhat duplicates. Maybe we can bring the conclusions from our conversation here back to #651, and then either continue to use this issue for its original purpose or we close it and open a new one for that purpose?

[felixarntz]

@10upsimon I've outlined a rough set of requirements for this in the issue description.

[mukeshpanchal27]

Fixed by #899 (in feature/creating-standalone-plugins branch).