Conversation
…tification is undefined can trigger
WalkthroughAdded is_admin() guards so admin_menu hooks are registered only in admin requests. Affected constructors in src/pro.php and src/welcome/notification-rate.php; no public signatures changed. Runtime behavior now skips admin hook registration on front-end. Changes
Sequence Diagram(s)sequenceDiagram
participant R as Request
participant WP as WordPress
participant P as Plugin (Pro)
participant N as NotificationRate
R->>WP: Bootstrap plugin
WP->>P: Construct
alt is_admin() true
P->>WP: add_action('admin_menu', check_pro_notice_date)
P->>WP: add_action('admin_menu', go_premium_notice_old_raters)
else front-end
P-->>WP: Skip admin_menu registrations
end
WP->>N: Construct
alt is_admin() true
N->>WP: add_action('admin_menu', check_activation_date)
else front-end
N-->>WP: Skip admin_menu registration
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a PHP error where stackable_add_welcome_notification is undefined during REST API calls by adding is_admin() checks to prevent admin-specific actions from being registered in non-admin contexts.
- Added
is_admin()guards around admin action registrations in notification classes - Prevents admin menu hooks from being registered during REST API requests
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/welcome/notification-rate.php | Added is_admin() check around admin menu action registration |
| src/pro.php | Added is_admin() check around admin menu action registrations |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
🤖 Pull request artifacts
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/welcome/notification-rate.php (1)
24-26: Optional hardening: guard the notification call and/or instantiate only in adminTo further bulletproof against edge loads where the function may not be present, consider guarding the call site too. Also optionally instantiate the notifier only in admin to skip doing any work outside wp-admin.
You can add a simple existence check in
check_activation_date(outside this hunk):if ( $elapsed_time > self::RATING_NOTICE_TIME && function_exists( 'stackable_add_welcome_notification' ) ) { stackable_add_welcome_notification( 'rate', sprintf( __( 'If Stackable has been valuable to you, consider %sgiving us a 5-star rating on WordPress.org%s. It would not only make our day but also help others discover us too.', STACKABLE_I18N ), '<a href="https://wordpress.org/support/plugin/stackable-ultimate-gutenberg-blocks/reviews/?rate=5#new-post" target="_rate">', '</a>' ) ); }Or instantiate only in admin:
if ( is_admin() ) { new Stackable_Welcome_Notification_Rate(); }src/pro.php (1)
90-93: Optional hardening: guard theshow_notificationcall siteFor extra safety if load order varies, you can guard the call to
stackable_add_welcome_notification(outside this hunk):public function show_notification() { if ( function_exists( 'stackable_add_welcome_notification' ) ) { stackable_add_welcome_notification( 'premium', sprintf( __( 'We hope you\'re enjoying Stackable. If you want more, you may want to check out %sStackable Premium%s. Ready to upgrade and do more? %sGo premium now%s', STACKABLE_I18N ), '<a href="https://wpstackable.com/premium/?utm_source=wp-settings-notification&utm_campaign=gopremium&utm_medium=wp-dashboard" target="_blank">', '</a>', '<a href="https://wpstackable.com/premium/?utm_source=wp-settings-notification&utm_campaign=gopremium&utm_medium=wp-dashboard">', '</a>' ) ); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/pro.php(1 hunks)src/welcome/notification-rate.php(1 hunks)
🔇 Additional comments (3)
src/welcome/notification-rate.php (2)
24-26: Good fix: admin-only hook registration prevents REST/CLI undefined-function errorsWrapping the
admin_menuhook registration withis_admin()is the right scope boundary and should stopstackable_add_welcome_notificationfrom being referenced in REST requests where it isn’t loaded. Low-risk, preserves admin behavior.
24-26: All notifier calls are admin-onlyI’ve verified every invocation of
stackable_add_welcome_notificationand confirmed:• It’s defined in
src/welcome/notification.phpand only called in:
–src/pro.phpwithinshow_notification()(triggered viacheck_pro_notice_date/go_premium_notice_old_raterson theadmin_menuhook)
–src/welcome/notification-rate.phpinsidecheck_activation_dateon theadmin_menuhook
• No other hooks or direct calls (including REST or CLI contexts) reach the notifier
• Noadmin_noticesor other non-admin hooks register these methodsAll notifier paths are safely gated by
is_admin(). No further changes needed.src/pro.php (1)
90-93: Correct scoping: admin-only registration of admin_menu hooksGating the
admin_menuregistrations withis_admin()addresses the REST-triggered undefined-function error without changing admin behavior. Looks good.
Summary by CodeRabbit