[VIP-1939] Display integrations in plugins list in WP-Admin #6545
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #6545 +/- ##
=============================================
+ Coverage 34.60% 34.64% +0.04%
- Complexity 5028 5067 +39
=============================================
Files 294 295 +1
Lines 20827 20929 +102
=============================================
+ Hits 7207 7251 +44
- Misses 13620 13678 +58 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
shekharnwagh
changed the title
shekharnwagh
force-pushed
the
add/integration-plugins-entry
branch
from
9937aee to
f3cbfba
Compare
Previously, Gutenberg and RTC plugins were registered for display immediately after require_once in RealTimeCollaborationIntegration, which meant they could appear in the WordPress plugins list even if they failed to load due to compatibility checks or other issues.
The trait uses static properties shared across all integrations. Previous approach used a manual reset method between tests which added test-specific code to trait.
The previous pattern used /s modifier and .*? which could match across multiple error divs, causing it to remove unrelated error notices.
shekharnwagh
force-pushed
the
add/integration-plugins-entry
branch
from
6fcc440 to
52bba6a
Compare
shekharnwagh
requested review from
chriszarate,
Copilot,
maxschmeling and
pkevan
There was a problem hiding this comment.
Pull Request Overview
This PR adds functionality to display plugins loaded by VIP Integrations in the WordPress admin plugins list. Since integration plugins are loaded from mu-plugins/vip-integrations/ via require_once, they don't appear in the standard plugins list which only scans wp-content/plugins/.
- Created a reusable
IntegrationPluginDisplayTraitfor making integration plugins visible in WordPress admin - Integrated the trait into
RealTimeCollaborationIntegrationto display Gutenberg and RTC plugins - Added comprehensive test coverage for the trait's functionality
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| vip-integrations.php | Includes the new trait file in the autoloader |
| integrations/integration-plugin-display-trait.php | New trait providing plugin display functionality with hooks and filters |
| integrations/real-time-collaboration.php | Integrates the trait and registers Gutenberg/RTC plugins for display |
| tests/integrations/test-integration-plugin-display-trait.php | Comprehensive test suite for the trait functionality |
| foreach ( $integration_plugins as $plugin ) { | ||
| // Remove error notices for "Plugin file does not exist" | ||
| // Pattern matches the exact WordPress error format: | ||
| // <div ... class="notice error"><p>The plugin <code>PATH</code> has been deactivated due to an error: Plugin file does not exist.</p></div> | ||
| $escaped_plugin = preg_quote( $plugin, '/' ); | ||
| $pattern = '/<div[^>]*\bnotice\b[^>]*\berror\b[^>]*><p>The plugin <code>' . $escaped_plugin . '<\/code> has been deactivated due to an error: Plugin file does not exist\.<\/p><\/div>\s*/'; | ||
| $buffer = preg_replace( $pattern, '', $buffer ); | ||
| } |
There was a problem hiding this comment.
The nested loop with regex operations in the output buffer callback could be inefficient for large numbers of integration plugins. Consider pre-compiling all patterns into a single regex with alternation (|) or caching the compiled patterns.
| foreach ( $integration_plugins as $plugin ) { | |
| // Remove error notices for "Plugin file does not exist" | |
| // Pattern matches the exact WordPress error format: | |
| // <div ... class="notice error"><p>The plugin <code>PATH</code> has been deactivated due to an error: Plugin file does not exist.</p></div> | |
| $escaped_plugin = preg_quote( $plugin, '/' ); | |
| $pattern = '/<div[^>]*\bnotice\b[^>]*\berror\b[^>]*><p>The plugin <code>' . $escaped_plugin . '<\/code> has been deactivated due to an error: Plugin file does not exist\.<\/p><\/div>\s*/'; | |
| $buffer = preg_replace( $pattern, '', $buffer ); | |
| } | |
| // Pre-compile a single regex pattern that matches any plugin error notice | |
| if ( empty( $integration_plugins ) ) { | |
| return $buffer; | |
| } | |
| $escaped_plugins = array_map( function( $plugin ) { | |
| return preg_quote( $plugin, '/' ); | |
| }, $integration_plugins ); | |
| $alternation = implode( '|', $escaped_plugins ); | |
| $pattern = '/<div[^>]*\bnotice\b[^>]*\berror\b[^>]*><p>The plugin <code>(' . $alternation . ')<\/code> has been deactivated due to an error: Plugin file does not exist\.<\/p><\/div>\s*/'; | |
| $buffer = preg_replace( $pattern, '', $buffer ); |
| function () { | ||
| if ( ob_get_level() > 0 ) { | ||
| ob_end_flush(); | ||
| } | ||
| }, |
There was a problem hiding this comment.
The output buffer cleanup only checks if any buffer exists, but doesn't verify it's the specific buffer started by this trait. If other code also uses output buffering, this could flush the wrong buffer. Consider storing the buffer level when starting and only flush if it matches.
| if ( empty( $integration_plugins ) || static::$buffer_started ) { | ||
| return; | ||
| } | ||
|
|
||
| static::$buffer_started = true; |
There was a problem hiding this comment.
The static flag $buffer_started could cause race conditions in concurrent requests or if multiple instances try to start buffers simultaneously. The flag is set after the check, creating a potential window where multiple buffers could be started.
| * | ||
| * @var string | ||
| */ | ||
| private static $plugin_display_message = 'Enabled via WPVIP Integrations'; |
There was a problem hiding this comment.
is there something more concise we can use here - feels a little long, particularly when it spans two lines.
There was a problem hiding this comment.
Open to ideas. Worded it so it's similar to badge - Enabled via code shown when we activate plugin from code using wpcom_vip_load_plugin.
What do we think using just WPVIP Integration ? But that might be slightly confusing when that shows up for Gutenberg plugin.
There was a problem hiding this comment.
Potentially something as simple as a link to the integrations center via a link Settings?
There was a problem hiding this comment.
Just took a quick look at the code in this repo and I couldn't find any references to dashboard.wpvip.com. If we have add it, it'll be some work constructing the url and maintaining it in unlikely scenario the routing in dashboard changes.
There was a problem hiding this comment.
perhaps we get away with a link to the docs? like: https://docs.wpvip.com/integrations/
There was a problem hiding this comment.
How about Enabled via integrations and integrations links to https://docs.wpvip.com/integrations/?
There was a problem hiding this comment.
I wouldn't be against adding a link or something better down the road, but the simple but clear text that you already have in there is fine with me even though it splits to two lines.
There was a problem hiding this comment.
So using regex and ob doesn't seem entirely safe. There are better ways, for example:
https://developer.wordpress.org/reference/hooks/plugin_action_links/
|
|
||
| // Cleanup and error suppression | ||
| add_action( 'admin_init', [ $this, 'cleanup_recently_activated' ], 1 ); | ||
| add_action( 'admin_notices', [ $this, 'suppress_deactivation_notices' ], 1 ); |
There was a problem hiding this comment.
curious as to when these get triggered, as it looks like they would only appear when the plugin activation/deactivation links are present and a user clicks on them?
There was a problem hiding this comment.
Thinking through this again, yeah we won't need this since we've already removed the links to activate/deactivate plugin and the activation happens via code. Removed in a38323e
| * | ||
| * @var string | ||
| */ | ||
| private static $plugin_display_message = 'Enabled via WPVIP Integrations'; |
There was a problem hiding this comment.
I wouldn't be against adding a link or something better down the road, but the simple but clear text that you already have in there is fine with me even though it splits to two lines.
Integration plugins never enter the recently_activated list since they're filtered out of active_plugins and users can't deactivate them via the UI. This method is therefore unnecessary.
The I'm open to better solutions. Is there a specific WordPress hook or filter that prevents the validation check for plugins in non-standard directories? @rinatkhaziev |
Co-authored-by: Max Schmeling <[email protected]>
|
OIC, I misunderstood the diff. Okay, in this particular case simply removing diff --git a/integrations/integration-plugin-display-trait.php b/integrations/integration-plugin-display-trait.php
index 9376cfb55..ce872a255 100644
--- a/integrations/integration-plugin-display-trait.php
+++ b/integrations/integration-plugin-display-trait.php
@@ -122,7 +122,7 @@ trait IntegrationPluginDisplayTrait {
add_filter( 'network_admin_plugin_action_links', [ $this, 'filter_plugin_action_links' ], 10, 2 );
// Active plugin management (display only)
- add_filter( 'option_active_plugins', [ $this, 'filter_active_plugins_for_display' ] );
+ // add_filter( 'option_active_plugins', [ $this, 'filter_active_plugins_for_display' ] );
add_filter( 'site_option_active_sitewide_plugins', [ $this, 'filter_network_active_plugins_for_display' ] );
// Prevent accidental database writes
@@ -130,8 +130,8 @@ trait IntegrationPluginDisplayTrait {
add_filter( 'pre_update_site_option_active_sitewide_plugins', [ $this, 'filter_update_network_active_plugins' ] );
// Cleanup and error suppression
- add_action( 'admin_notices', [ $this, 'suppress_deactivation_notices' ], 1 );
- add_action( 'network_admin_notices', [ $this, 'suppress_deactivation_notices' ], 1 );
+ // add_action( 'admin_notices', [ $this, 'suppress_deactivation_notices' ], 1 );
+ // add_action( 'network_admin_notices', [ $this, 'suppress_deactivation_notices' ], 1 );
}
/**
|
Removing 
|
Description
This PR adds functionality to display plugins loaded by VIP Integrations in the WordPress admin plugins list, providing better visibility.
Plugins loaded by VIP Integrations are loaded via
require_oncefrom themu-plugins/vip-integrations/directory. Since WordPress'sget_plugins()only scans the standardwp-content/plugins/directory, these integration plugins are invisible in the wp-admin plugins list.To solve this, created a reusable
IntegrationPluginDisplayTraitthat:all_pluginsfilterThis approach is similar to how
wpcom_vip_load_plugin()displays code-activated plugins, but tweaked for the plugin code being inmu-plugins/vip-integrationsdir.Changelog Description
Added
IntegrationPluginDisplayTraitthat displays integration plugins in the WordPress admin plugins listPre-review checklist
Please make sure the items below have been covered before requesting a review:
Pre-deploy checklist
Steps to Test
Check out PR.
Copy the
/vip-integrationsdirectory from the vip-go-mu-plugins-built repo to/vip-integrationsdirectory of this repo.Run
composer installandnpm installto install dependencies.Activate the 'Real-Time Collaboration' integration on one of your test sites and note down its application ID.
Spin up a local dev for that site using command -
vip dev-env create @<application_id>.production -s <slug> -a <path_to_app_code> -u <path_to_local_vip-go-mu-plugins_dir>Start it using the command -
vip dev-env start --slug <slug> --editor vscodeThat should open a VS Code window. Modify the
<app_name>/integrations-config/integrations.jsonto add following entry -Navigate to the
wp-admin>Pluginspage and verify that the custom build of Gutenberg and the latest version of the Real-Time Collaboration plugin activated via VIP Integrations are displayed in the WordPress admin plugins list.