Feat: add Useful Plugins in the admin setting and Cimo download notice in the editor #3635
Conversation
|
Warning Rate limit exceeded@kaeizen has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 13 minutes and 48 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughAdds an admin "Useful Plugins" feature with server-side helpers and AJAX endpoints, a React UI to install/activate plugins with optimistic updates, a Cimo media-pane download/activation notice with polling and dismiss persistence, permission-gated lazy loading of the notice, and related styles and admin menu wiring. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as UsefulPlugins UI
participant JS as Frontend (apiFetch)
participant PHP as Stackable_Useful_Plugins
participant WP as WordPress Core
User->>UI: Click Install/Activate
UI->>UI: set status → installing/activating (optimistic)
UI->>JS: apiFetch POST (install/activate) with nonce & slug
JS->>PHP: AJAX request
PHP->>WP: verify nonce & capabilities, perform install/activate
WP-->>PHP: result
PHP-->>JS: JSON success/error
alt success or folder_exists
JS->>UI: update to installed/activated (after small delay)
else error
JS->>UI: revert status and show error
end
sequenceDiagram
participant Media as WP Media Frame
participant Cimo as CimoDownloadNotice
participant AJAX as Stackable_Useful_Plugins AJAX
Media->>Cimo: details pane open
Cimo->>AJAX: poll stackable_check_cimo_status
AJAX-->>Cimo: status (+ optional action URL)
alt installing/activating
Cimo->>Media: show action, schedule further polls
else activated or hidden
Cimo->>Media: show activated state or skip insertion
end
Cimo->>AJAX: onDismiss -> persist hide setting via settings endpoint
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🤖 Pull request artifacts
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
src/lazy-components/cimo/index.php (2)
72-117: Misleading method name.The method is named
enqueue_script()but it doesn't enqueue any script—it only prepares localization data. Consider renaming toprepare_localization_data()orprepare_cimo_data()to better reflect its actual behavior.
73-73: Consider caching the option value.The option
stackable_hide_cimo_noticeis retrieved multiple times (lines 73 and 126). Consider caching it as a class property to avoid redundant database calls.Also applies to: 126-126
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
plugin.php(1 hunks)src/components/image-control2/index.js(5 hunks)src/init.php(1 hunks)src/lazy-components/cimo/index.js(1 hunks)src/lazy-components/cimo/index.php(1 hunks)src/lazy-components/cimo/style.scss(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
src/init.php (1)
freemius/includes/class-freemius.php (1)
admin_url(16224-16228)
src/lazy-components/cimo/index.js (1)
src/components/image-control2/index.js (1)
CimoDownloadNotice(87-87)
src/components/image-control2/index.js (1)
src/lazy-components/cimo/index.js (1)
CimoDownloadNotice(14-128)
src/lazy-components/cimo/index.php (2)
src/init.php (1)
__construct(35-74)src/lazy-components/cimo/index.js (1)
data(15-15)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: build
- GitHub Check: PHP 8.2 and WP latest
- GitHub Check: PHP 8.2 and WP 6.6.2
- GitHub Check: PHP 7.3 and WP latest
- GitHub Check: PHP 8.2 and WP 6.5.5
- GitHub Check: PHP 8.2 and WP 6.7.2
- GitHub Check: PHP 7.3 and WP 6.5.5
🔇 Additional comments (5)
src/lazy-components/cimo/index.php (5)
1-16: LGTM!The file header, security checks, and class definition follow WordPress best practices.
44-55: LGTM!The plugin installation check is implemented correctly with proper inclusion of WordPress plugin functions.
57-68: LGTM!The plugin activation check correctly handles both class existence and active plugin detection.
119-122: LGTM!Simple helper method that correctly merges the cimo data into the localization arguments.
170-170: LGTM!Standard WordPress instantiation pattern.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/lazy-components/cimo/index.js (1)
178-191: Ensure the React root is unmounted when removing the notice.We drop the container node but never call
root.unmount(), so React keeps the fiber tree and delegated listeners alive until GC, which is wasteful if the notice is toggled often. Capture the root instance and unmount it before detaching the element.- if ( details && ! this.el.querySelector( '.stk-cimo-notice' ) ) { - const noticeDiv = document.createElement( 'div' ) - noticeDiv.className = 'stk-cimo-notice' - - const onDismiss = () => { - if ( noticeDiv && noticeDiv.parentNode ) { - noticeDiv.parentNode.removeChild( noticeDiv ) - } - } - - createRoot( noticeDiv ).render( <CimoDownloadNotice onDismiss={ onDismiss } /> ) + if ( details && ! this.el.querySelector( '.stk-cimo-notice' ) ) { + const noticeDiv = document.createElement( 'div' ) + noticeDiv.className = 'stk-cimo-notice' + const reactRoot = createRoot( noticeDiv ) + + const onDismiss = () => { + if ( noticeDiv && noticeDiv.parentNode ) { + reactRoot.unmount() + noticeDiv.parentNode.removeChild( noticeDiv ) + } + } + + reactRoot.render( <CimoDownloadNotice onDismiss={ onDismiss } /> )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/lazy-components/cimo/index.js(1 hunks)src/lazy-components/cimo/index.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/lazy-components/cimo/index.js (1)
src/components/image-control2/index.js (1)
CimoDownloadNotice(87-87)
src/lazy-components/cimo/index.php (2)
src/init.php (1)
__construct(35-74)src/lazy-components/cimo/index.js (1)
data(15-15)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: build
- GitHub Check: PHP 8.2 and WP latest
- GitHub Check: PHP 7.3 and WP 6.5.5
- GitHub Check: PHP 7.3 and WP latest
- GitHub Check: PHP 8.2 and WP 6.6.2
- GitHub Check: PHP 8.2 and WP 6.5.5
- GitHub Check: PHP 8.2 and WP 6.7.2
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/lazy-components/cimo/index.php (3)
71-119: Consider checking activation status earlier for efficiency.The current flow checks installation status and capabilities before checking if the plugin is activated (line 102, 117). If the plugin is already activated, no user needs to see the notice, regardless of their capabilities.
Consider this optimization to short-circuit earlier:
public function enqueue_script() { $hide_cimo = get_option( 'stackable_hide_cimo_notice', false ); if ( $hide_cimo ) { return; } + + // No need to expose the notice if plugin is already activated + if ( self::is_plugin_activated() ) { + return; + } $is_installed = self::is_plugin_installed();This avoids the installation check and capability checks entirely when the plugin is already activated.
139-148: Optional: Avoid exposingcimodata when the plugin is activated or hidden.The
localize_hide_cimo_noticemethod always runs (hooked at priority 999 on line 25), even when the plugin is activated or the notice is hidden. This results in exposingcimo: { hideNotice: true/false }in these cases.Since the JavaScript code checks for
cimo?.statusto determine whether to render the notice, exposinghideNoticealone is unnecessary whenstatusisn't present.Consider adding an early return:
public function localize_hide_cimo_notice( $args ) { + // Only expose hideNotice if we're already exposing cimo data + if ( ! isset( $args['cimo'] ) ) { + return $args; + } + $hide_cimo = get_option( 'stackable_hide_cimo_notice', false ); - if ( isset( $args['cimo'] ) ) { - $args['cimo']['hideNotice'] = $hide_cimo; - return $args; - } - - $args[ 'cimo' ] = array( 'hideNotice' => $hide_cimo ); + $args['cimo']['hideNotice'] = $hide_cimo; return $args; }This is a minor optimization that reduces unnecessary data in the localized script.
61-61: Minor maintenance consideration:class_existscheck relies on Cimo's internal structure.The check for
class_exists( 'Cimo_Script_Loader' )is a good performance optimization to avoid file I/O, but it assumes Cimo will always expose this specific class. If Cimo's internal structure changes (class renamed, removed, or namespaced), this check could break.Since you already have a fallback to
is_plugin_active(), this is low risk. Just be aware that if Cimo updates significantly, this optimization might need adjustment.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/lazy-components/cimo/index.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/lazy-components/cimo/index.php (1)
src/lazy-components/cimo/index.js (1)
data(15-15)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: PHP 8.2 and WP latest
- GitHub Check: PHP 8.2 and WP 6.5.5
- GitHub Check: PHP 7.3 and WP 6.5.5
- GitHub Check: PHP 7.3 and WP latest
- GitHub Check: PHP 8.2 and WP 6.6.2
- GitHub Check: PHP 8.2 and WP 6.7.2
- GitHub Check: build
🔇 Additional comments (1)
src/lazy-components/cimo/index.php (1)
1-202: Excellent work addressing all security concerns from the previous review.All critical security issues have been resolved:
- Nonce verification is properly implemented in the AJAX handler
- Capability checks gate both the UI (lines 81-85) and the AJAX endpoint (lines 175-179)
- Input sanitization and validation are in place
- Settings registration uses the correct
rest_sanitize_booleancallbackThe implementation follows WordPress best practices and the code is well-structured.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
plugin.php (1)
292-293: Consider conditional loading for the cross-marketing component.The Cimo cross-marketing component is loaded unconditionally on every request, but based on the AI summary, it appears in the media details pane (editor UI). Loading editor-specific functionality on frontend requests wastes resources.
Consider wrapping this in an
is_admin()check for better performance:// For cross-marketing -require_once( plugin_dir_path( __FILE__ ) . 'src/lazy-components/cimo/index.php' ); +if ( is_admin() ) { + require_once( plugin_dir_path( __FILE__ ) . 'src/lazy-components/cimo/index.php' ); +}Alternatively, move this include into the
if ( is_admin() )block starting at line 320 where other admin-specific components are loaded.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
plugin.php(1 hunks)src/init.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/init.php (1)
freemius/includes/class-freemius.php (1)
admin_url(16224-16228)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: PHP 8.2 and WP latest
- GitHub Check: PHP 8.2 and WP 6.6.2
- GitHub Check: PHP 8.2 and WP 6.7.2
- GitHub Check: PHP 7.3 and WP latest
- GitHub Check: PHP 8.2 and WP 6.5.5
- GitHub Check: PHP 7.3 and WP 6.5.5
- GitHub Check: build
🔇 Additional comments (1)
src/init.php (1)
352-352: LGTM!The addition of
ajaxUrlto the localized script data follows the standard WordPress pattern for providing AJAX endpoints to frontend scripts. This is properly scoped to the block editor context and pairs well with the existing nonce on line 344 for secure AJAX requests.
kaeizen
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
plugin.php(2 hunks)src/welcome/admin.js(2 hunks)src/welcome/admin.scss(5 hunks)src/welcome/getting-started.scss(0 hunks)src/welcome/index.php(4 hunks)src/welcome/useful-plugins.js(1 hunks)src/welcome/useful-plugins.php(1 hunks)src/welcome/useful-plugins.scss(1 hunks)
💤 Files with no reviewable changes (1)
- src/welcome/getting-started.scss
✅ Files skipped from review due to trivial changes (1)
- src/welcome/useful-plugins.scss
🧰 Additional context used
🧬 Code graph analysis (3)
src/welcome/index.php (1)
src/welcome/notification.php (1)
stackable_welcome_notification(91-145)
src/welcome/admin.js (2)
src/util/element.js (2)
createRoot(16-31)createRoot(16-31)src/welcome/useful-plugins.js (2)
UsefulPlugins(112-118)UsefulPlugins(112-118)
src/welcome/useful-plugins.php (1)
src/welcome/useful-plugins.js (2)
PLUGINS(11-19)status(38-38)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: build
- GitHub Check: PHP 8.2 and WP 6.7.2
- GitHub Check: PHP 8.2 and WP 6.6.2
- GitHub Check: PHP 8.2 and WP 6.5.5
- GitHub Check: PHP 7.3 and WP latest
- GitHub Check: PHP 8.2 and WP latest
- GitHub Check: PHP 7.3 and WP 6.5.5
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
src/welcome/useful-plugins.php (4)
97-103: Remove duplicate include statement.Lines 97-99 and 101-103 both include
plugin.php. The second include is redundant.Apply this diff:
if ( ! function_exists( 'get_plugins' ) ) { include_once( ABSPATH . 'wp-admin/includes/plugin.php' ); } - - if ( ! function_exists( 'is_plugin_active' ) ) { - include_once( ABSPATH . 'wp-admin/includes/plugin.php' ); - }
201-233: Remove unnecessary return statements afterwp_send_json_error.The
wp_send_json_error()function callswp_die()internally, which exits execution. Thereturnstatements on lines 210, 215, and 229 are therefore unreachable and unnecessary.Apply this diff:
if ( ! check_ajax_referer( 'stk_activate_useful_plugin', 'nonce', false ) ) { wp_send_json_error( array( 'status' => 'error', 'message' => 'Security check failed.' ), 403 ); - return; } if ( ! current_user_can( 'activate_plugins' ) ) { wp_send_json_error( array( 'status' => 'error', 'message' => 'Insufficient permissions.' ), 403 ); - return; } // Clear the plugins cache to ensure newly installed plugins are recognized (avoids activation errors due to outdated plugin cache) wp_clean_plugins_cache(); if ( ! function_exists( 'activate_plugin' ) ) { include_once( ABSPATH . 'wp-admin/includes/plugin.php' ); } $result = activate_plugin( $full_slug, '', false, true ); if ( is_wp_error( $result ) ) { wp_send_json_error( array( 'status' => 'error', 'message' => 'Failed to activate plugin.' ), 500 ); - return; }
268-268: Add validation before accessing array key.Line 268 directly accesses
self::$PLUGINS[$slug]without verifying the key exists. Although$slugis hardcoded on line 244, defensive coding suggests validating the key exists before access to prevent potential PHP notices if the code is modified later.Apply this diff:
+ if ( ! isset( self::$PLUGINS[ $slug ] ) ) { + wp_send_json_error( array( 'status' => 'error', 'message' => 'Invalid plugin slug.' ), 400 ); + } + $full_slug = self::$PLUGINS[ $slug ][ 'full_slug' ];
270-284: Simplify nested ternary for better readability.The nested ternary operators on lines 273-274 and 274-283 make the code harder to understand and maintain. Consider refactoring to use explicit conditional statements.
Apply this diff:
if ( $action === 'install' && ! self::is_plugin_installed( $full_slug ) ) { $response[ 'status' ] = 'installing'; - } else if ( ! self::is_plugin_activated( $full_slug ) ) { - $response[ 'status' ] = $action === 'install' ? 'installed' : 'activating'; - $response[ 'action' ] = $action === 'install' ? html_entity_decode( wp_nonce_url( - add_query_arg( - [ - 'action' => 'activate', - 'plugin' => $full_slug, - ], - admin_url( 'plugins.php' ) - ), - 'activate-plugin_' . $full_slug - ) ) : ''; + } elseif ( ! self::is_plugin_activated( $full_slug ) ) { + if ( $action === 'install' ) { + $response[ 'status' ] = 'installed'; + $response[ 'action' ] = html_entity_decode( wp_nonce_url( + add_query_arg( + [ + 'action' => 'activate', + 'plugin' => $full_slug, + ], + admin_url( 'plugins.php' ) + ), + 'activate-plugin_' . $full_slug + ) ); + } else { + $response[ 'status' ] = 'activating'; + $response[ 'action' ] = ''; + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/welcome/useful-plugins.js(1 hunks)src/welcome/useful-plugins.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/welcome/useful-plugins.js
🧰 Additional context used
🧬 Code graph analysis (1)
src/welcome/useful-plugins.php (2)
src/welcome/useful-plugins.js (2)
PLUGINS(11-19)status(39-39)src/welcome/index.php (1)
__construct(13-25)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: build
- GitHub Check: PHP 7.3 and WP 6.5.5
- GitHub Check: PHP 7.3 and WP latest
- GitHub Check: PHP 8.2 and WP latest
- GitHub Check: PHP 8.2 and WP 6.7.2
- GitHub Check: PHP 8.2 and WP 6.6.2
- GitHub Check: PHP 8.2 and WP 6.5.5
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/welcome/useful-plugins.php (1)
119-126: Add error handling forplugins_apicall.The
plugins_api()call can returnfalseor aWP_Errorobject. The current code checks$plugin_info &&but does not explicitly verifyis_wp_error(), which can lead to warnings when accessing properties on an error object.Apply this diff to add proper error handling:
$plugin_info = plugins_api( 'plugin_information', [ 'slug' => $plugin['slug'], - 'fields' =>[ 'icons' => true, 'sections' => false ], - ] ); + 'fields' => [ 'icons' => true, 'sections' => false ], + ] ); - $icon_url = $plugin_info && isset( $plugin_info->icons ) - && is_array( $plugin_info->icons ) && ! empty( $plugin_info->icons ) - ? array_values( $plugin_info->icons )[0] : ''; + $icon_url = ''; + if ( ! is_wp_error( $plugin_info ) && $plugin_info && isset( $plugin_info->icons ) + && is_array( $plugin_info->icons ) && ! empty( $plugin_info->icons ) ) { + $icon_url = array_values( $plugin_info->icons )[0]; + }Based on learnings
🧹 Nitpick comments (1)
src/welcome/useful-plugins.php (1)
97-103: Consolidate duplicate includes.Both
get_plugins()andis_plugin_active()are defined in the same file (plugin.php), so the second conditional include on lines 101-103 is redundant.Apply this diff to consolidate:
if ( ! function_exists( 'plugins_api' ) ) { include_once( ABSPATH . 'wp-admin/includes/plugin-install.php' ); } - if ( ! function_exists( 'get_plugins' ) ) { + if ( ! function_exists( 'get_plugins' ) || ! function_exists( 'is_plugin_active' ) ) { include_once( ABSPATH . 'wp-admin/includes/plugin.php' ); } - - if ( ! function_exists( 'is_plugin_active' ) ) { - include_once( ABSPATH . 'wp-admin/includes/plugin.php' ); - }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/welcome/useful-plugins.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/welcome/useful-plugins.php (1)
src/welcome/useful-plugins.js (2)
PLUGINS(11-19)status(39-39)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: PHP 8.2 and WP latest
- GitHub Check: PHP 8.2 and WP 6.7.2
- GitHub Check: PHP 7.3 and WP 6.5.5
- GitHub Check: PHP 7.3 and WP latest
- GitHub Check: PHP 8.2 and WP 6.6.2
- GitHub Check: build
- GitHub Check: PHP 8.2 and WP 6.5.5
🔇 Additional comments (3)
src/welcome/useful-plugins.php (3)
141-176: LGTM! Closure correctly captures Cimo data.The closure properly captures only
$cimo_data(which includes the action link set at line 170), avoiding any variable scoping issues. The implementation correctly provides editor-specific localization data for the Cimo plugin.
218-250: LGTM! Robust AJAX activation handler.The implementation includes all necessary security checks (input validation, nonce verification, capability check), proper cache clearing to prevent stale plugin data issues, and appropriate error handling. The use of silent mode for activation is correct in an AJAX context.
252-304: LGTM! Well-designed polling endpoint.The status check handler implements comprehensive security (nonce, input validation, action-specific capability checks) and clear state transitions for polling (installing → installed → activating → activated). The logic correctly provides activation URLs when transitioning from installed to activating state.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/welcome/useful-plugins.php (3)
59-82: Consider reducing code duplication in helper methods.Both
is_plugin_installed()andis_plugin_activated()include the same file check forwp-admin/includes/plugin.php. While this works correctly, you could reduce duplication by extracting the include logic into a shared private helper method.Example:
+ private static function ensure_plugin_functions_loaded() { + if ( ! function_exists( 'get_plugins' ) || ! function_exists( 'is_plugin_active' ) ) { + include_once( ABSPATH . 'wp-admin/includes/plugin.php' ); + } + } + public static function is_plugin_installed( $plugin_slug ) { - if ( ! function_exists( 'get_plugins' ) ) { - include_once( ABSPATH . 'wp-admin/includes/plugin.php' ); - } + self::ensure_plugin_functions_loaded(); $all_plugins = get_plugins(); if ( isset( $all_plugins[ $plugin_slug ] ) ) { return true; } return false; } public static function is_plugin_activated( $plugin_slug ) { - if ( ! function_exists( 'is_plugin_active' ) ) { - include_once( ABSPATH . 'wp-admin/includes/plugin.php' ); - } + self::ensure_plugin_functions_loaded(); if ( is_plugin_active( $plugin_slug ) ) { return true; } return false; }
86-88: Consider clarifying capability check logic.The nested ternary works correctly but could be more readable with explicit conditionals or named constants.
Example refactor:
- $current_user_cap = current_user_can( 'install_plugins' ) ? 2 : ( - current_user_can( 'activate_plugins') ? 1 : 0 - ); + // Determine user capability level: 2 = can install, 1 = can activate, 0 = no permissions + if ( current_user_can( 'install_plugins' ) ) { + $current_user_cap = 2; + } elseif ( current_user_can( 'activate_plugins' ) ) { + $current_user_cap = 1; + } else { + $current_user_cap = 0; + }
220-252: LGTM - Secure AJAX handler with good practices.The method properly validates input, verifies nonce, checks capabilities, and clears the plugin cache before activation (line 238). Error handling is comprehensive.
Optional improvement: Consider internationalizing error messages for consistency with the rest of the codebase:
if ( ! $slug || ! $full_slug ) { - wp_send_json_error( array( 'status' => 'error', 'message' => 'Invalid slug.' ), 400 ); + wp_send_json_error( array( 'status' => 'error', 'message' => __( 'Invalid slug.', STACKABLE_I18N ) ), 400 ); } if ( ! check_ajax_referer( 'stk_activate_useful_plugin', 'nonce', false ) ) { - wp_send_json_error( array( 'status' => 'error', 'message' => 'Security check failed.' ), 403 ); + wp_send_json_error( array( 'status' => 'error', 'message' => __( 'Security check failed.', STACKABLE_I18N ) ), 403 ); return; } if ( ! current_user_can( 'activate_plugins' ) ) { - wp_send_json_error( array( 'status' => 'error', 'message' => 'Insufficient permissions.' ), 403 ); + wp_send_json_error( array( 'status' => 'error', 'message' => __( 'Insufficient permissions.', STACKABLE_I18N ) ), 403 ); return; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/welcome/useful-plugins.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/welcome/useful-plugins.php (2)
src/welcome/useful-plugins.js (2)
PLUGINS(11-19)status(39-39)src/welcome/index.php (1)
__construct(13-25)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: PHP 8.2 and WP 6.7.2
- GitHub Check: PHP 8.2 and WP latest
- GitHub Check: PHP 7.3 and WP latest
- GitHub Check: PHP 7.3 and WP 6.5.5
- GitHub Check: PHP 8.2 and WP 6.5.5
- GitHub Check: PHP 8.2 and WP 6.6.2
- GitHub Check: build
🔇 Additional comments (6)
src/welcome/useful-plugins.php (6)
11-43: LGTM - Well-structured class initialization.The constructor properly wires up hooks and filters, and the conditional filter registration on line 40-42 correctly limits the localization to admin contexts only.
45-57: LGTM - Settings registration follows WordPress best practices.The setting is properly configured with type, sanitization, REST visibility, and default value.
180-191: LGTM - Admin localization data properly configured.The method correctly generates nonces, includes the AJAX URL, and merges plugin data for the admin settings interface.
193-206: LGTM - Clean utility method for data merging.The conditional logic correctly handles both nested and direct data merging patterns.
208-218: LGTM - Properly handles optional Cimo data.The method safely checks for the existence of the
cimokey before attempting to modify it, preventing potential undefined index issues.
309-309: LGTM - Standard WordPress plugin instantiation pattern.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/welcome/useful-plugins.js (1)
84-93: Fix inconsistent status update reference.Line 91 uses
usefulPlugins[ plugin.id ].statusinstead ofpluginData.status, which is inconsistent with lines 88 and 98. SincepluginDatais already extracted and null-guarded at the component's initialization, all status mutations should usepluginDatato maintain consistency and avoid potential runtime errors ifusefulPluginsis undefined.Apply this diff to fix the inconsistency:
if ( response.success || response.data?.errorCode === 'folder_exists' ) { pluginData.status = successStatus setStatus( successStatus ) } else { - usefulPlugins[ plugin.id ].status = prevStatus + pluginData.status = prevStatus setStatus( prevStatus ) }
🧹 Nitpick comments (2)
src/welcome/useful-plugins.php (2)
217-249: Plugin activation handler is secure and well-structured.The method properly validates input, checks nonces and capabilities, and clears the plugin cache before activation to avoid stale data issues.
Optional improvement: Consider internationalizing error messages for consistency:
if ( ! $slug || ! $full_slug ) { - wp_send_json_error( array( 'status' => 'error', 'message' => 'Invalid slug.' ), 400 ); + wp_send_json_error( array( 'status' => 'error', 'message' => __( 'Invalid slug.', STACKABLE_I18N ) ), 400 ); } if ( ! check_ajax_referer( 'stk_activate_useful_plugin', 'nonce', false ) ) { - wp_send_json_error( array( 'status' => 'error', 'message' => 'Security check failed.' ), 403 ); + wp_send_json_error( array( 'status' => 'error', 'message' => __( 'Security check failed.', STACKABLE_I18N ) ), 403 ); return; } if ( ! current_user_can( 'activate_plugins' ) ) { - wp_send_json_error( array( 'status' => 'error', 'message' => 'Insufficient permissions.' ), 403 ); + wp_send_json_error( array( 'status' => 'error', 'message' => __( 'Insufficient permissions.', STACKABLE_I18N ) ), 403 ); return; }
259-306: Status polling endpoint is secure and addresses previous review.The method properly implements security checks and correctly calls
wp_clean_plugins_cache()at line 287 to ensure fresh plugin status, addressing the previous review concern about stale cache data.Optional improvement: Similar to
do_plugin_activate(), consider internationalizing the error messages for consistency (lines 263, 274, 280).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/welcome/useful-plugins.js(1 hunks)src/welcome/useful-plugins.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/welcome/useful-plugins.php (2)
src/welcome/useful-plugins.js (2)
PLUGINS(11-19)status(39-39)src/welcome/index.php (1)
__construct(13-25)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: PHP 8.2 and WP 6.5.5
- GitHub Check: PHP 8.2 and WP 6.7.2
- GitHub Check: PHP 7.3 and WP 6.5.5
- GitHub Check: PHP 8.2 and WP latest
- GitHub Check: PHP 7.3 and WP latest
- GitHub Check: PHP 8.2 and WP 6.6.2
- GitHub Check: build
🔇 Additional comments (6)
src/welcome/useful-plugins.js (2)
1-36: LGTM: Imports and constants are well-structured.The imports, plugin definitions, and status constants are properly organized and the button labels are correctly internationalized.
103-128: LGTM: Render implementation is clean and accessible.The render methods properly utilize WordPress components and include accessibility features like alt text for icons. The conditional rendering of the spinner based on status is well-implemented.
src/welcome/useful-plugins.php (4)
1-57: LGTM: Class structure and settings registration are well-implemented.The class follows WordPress conventions with proper security checks, hook registrations, and settings API usage. The constructor properly wires up admin and AJAX handlers.
59-82: LGTM: Helper methods are clean and properly guard includes.The static helper methods correctly lazy-load WordPress admin functions and follow best practices for checking plugin status.
85-138: LGTM: Plugin information gathering is robust.The method properly checks capabilities, consolidates includes, and handles
plugins_api()errors withis_wp_error()checks. Previous review concerns have been addressed.
140-203: LGTM: Localization methods are consistent and well-structured.The methods properly build action URLs with appropriate nonces and consistently apply
html_entity_decode(). The generic helper pattern is clean.
Summary by CodeRabbit
New Features
Style