Fix issue with entries page when inbox badge is disabled#2202
Conversation
WalkthroughThe pull request modifies the Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
classes/controllers/FrmEntriesController.php (1)
336-338: Verify Pro version check and settings access.The code checks for Pro version installation and settings access, but the condition could be more concise and safer.
Consider this more defensive approach:
-if ( FrmAppHelper::pro_is_installed() && is_callable( 'FrmProAppHelper::get_settings' ) ) { +if ( FrmAppHelper::pro_is_installed() && method_exists( 'FrmProAppHelper', 'get_settings' ) ) {This change uses
method_exists()which is more reliable for checking method availability thanis_callable().
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
classes/controllers/FrmEntriesController.php(1 hunks)
🔇 Additional comments (2)
classes/controllers/FrmEntriesController.php (2)
340-343: 🛠️ Refactor suggestionImprove inbox badge configuration check.
The condition
! empty( $settings->inbox ) && ! isset( $settings->inbox['badge'] )might not cover all edge cases.Consider this more robust approach:
-$inbox_badge_off = ! empty( $settings->inbox ) && ! isset( $settings->inbox['badge'] ); +$inbox_badge_off = ! empty( $settings->inbox ) && ( ! isset( $settings->inbox['badge'] ) || empty( $settings->inbox['badge'] ) );This change ensures the badge is considered off when either the setting is missing or explicitly disabled.
Likely invalid or redundant comment.
346-349: Verify unread count initialization.The fallback behavior for unread count looks good, but let's verify its usage across the codebase.
✅ Verification successful
Let me gather more information about the inbox badge implementation.
Unread count initialization is properly handled with badge settings
The verification shows that the unread count initialization is correctly implemented with proper consideration for badge settings:
- The code checks for badge settings before initializing the unread count
- When badges are disabled (
$inbox_badge_off), the unread count is set to 0- The fallback initialization using
FrmInbox->unread()only happens when unread count is not already set- The unread count is used consistently across the codebase for menu name generation and badge display
The implementation follows a clear pattern:
- Check badge settings
- Set count to 0 if badges are disabled
- Initialize count via inbox if not set
- Apply count to menu name
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any other places where unread count is used to ensure consistency # Search for unread count usage rg -A 3 "unread\(\)" . # Search for inbox badge related settings ast-grep --pattern 'inbox["\']?\s*=>\s*\[.*badge.*\]'Length of output: 972
Script:
#!/bin/bash # Search for badge-related code and menu name usage rg -A 5 "menu_name.*unread" . # Look for other inbox badge implementations rg -A 5 "badge" . # Search for FrmInbox class usage ast-grep --pattern 'new FrmInbox'Length of output: 2524
No description provided.