Improve inbox count function logic#2225
Conversation
WalkthroughThe pull request modifies the Changes
Sequence DiagramsequenceDiagram
participant EH as EntriesHelper
participant Inbox as FrmInbox
participant ProSettings as FrmProSettingsController
EH->>Inbox: Create Inbox instance
EH->>Inbox: Count unread messages
alt No unread messages
EH-->>EH: Return 0
else Unread messages exist
EH->>ProSettings: Check inbox_badge method
alt inbox_badge method exists
ProSettings-->>EH: Modify inbox count
end
EH-->>EH: Return unread message count
end
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
classes/helpers/FrmEntriesHelper.php (1)
881-895: LGTM! Consider adding type hints.The changes improve the inbox count logic by making it more maintainable with early returns and cleaner logic flow. The code is now easier to follow and understand.
Consider adding type hints to improve code clarity:
- public static function get_visible_unread_inbox_count() { + public static function get_visible_unread_inbox_count(): int {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
classes/helpers/FrmEntriesHelper.php(1 hunks)stubs.php(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cypress
| class FrmProSettingsController { | ||
| /** | ||
| * @param string $count | ||
| * @return string | ||
| */ | ||
| public static function inbox_badge( $count ) { | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Codebase verification
Type declarations in stubs.php should be updated to use integer types
The method signature in stubs.php should be changed to use int instead of string for both parameter and return types, as:
- The method is called with an integer count from
count($inbox->unread()) - The return value is used in an integer context and returned by a method that declares
@return int
🔗 Analysis chain
Verify the method signature.
The inbox_badge method signature indicates it takes a string parameter and returns a string, but it's being called with an integer in get_visible_unread_inbox_count. Please ensure the parameter and return type declarations match the actual usage.
Run the following script to verify the method usage:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the usage of inbox_badge method.
# Test: Search for all calls to inbox_badge. Expect: Only occurrences of the new signature.
rg -A 5 $'inbox_badge'
Length of output: 1356
Script:
#!/bin/bash
# Find the context around $inbox_count in FrmEntriesHelper.php
rg -B 5 -A 5 "\$inbox_count" "classes/helpers/FrmEntriesHelper.php"
Length of output: 66
Script:
#!/bin/bash
# Find the complete method context in FrmEntriesHelper.php
rg -B 15 -A 15 "inbox_badge" "classes/helpers/FrmEntriesHelper.php"
Length of output: 623
1 participant
The other check was missing a license check. Rather than adding more logic, I'm just calling a function that handles everything that already existed.