-
Notifications
You must be signed in to change notification settings - Fork 92
gpeb-process-feeds-on-edit.php: Fixed an issue with the snippet causing fatal error.
#1049
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…sing fatal error.
WalkthroughThe change introduces a conditional check within the Changes
Sequence Diagram(s)sequenceDiagram
participant Callback as gform_addon_pre_process_feeds
participant Feeds as $feeds Input
Callback ->> Feeds: Verify if feeds is an array
alt Feeds is not an array
Callback -->> Feeds: Return original feeds
else Feeds is an array
Callback ->> Callback: Filter feeds based on exclusion list
Callback -->> Feeds: Return filtered feeds
end
Suggested reviewers
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
✨ Finishing Touches
🪧 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
gp-entry-blocks/gpeb-process-feeds-on-edit.php (2)
31-43: Consider adding logging for unexpected input typesWhile the fix appropriately handles non-array input, you might want to consider adding some form of logging when this unexpected condition occurs. This could help identify why
$feedsoccasionally isn't an array, which might point to an underlying issue elsewhere.add_filter( 'gform_addon_pre_process_feeds', function( $feeds, $entry, $form ) use ( $excluded_feed_ids ) { // If no feeds are present, return. if ( ! is_array( $feeds ) ) { + // Log unexpected data type for debugging purposes + if ( function_exists( 'error_log' ) ) { + error_log( 'GPEB Process Feeds: Expected array for $feeds but received ' . gettype( $feeds ) ); + } return $feeds; } // Filter feeds excluding the ones in the excluded feed ids array.
13-13: Consider making excluded feed IDs customizableThe excluded feed IDs (103, 104) are hardcoded. For better flexibility, consider making this a configurable option through a filter or constant. This would allow site administrators to customize the exclusion list without modifying the code.
// Array of feed ids to exclude for processing, if you want all feeds to be process use an empty array. -$excluded_feed_ids = array( 103, 104 ); +$excluded_feed_ids = apply_filters( 'gpeb_process_feeds_excluded_ids', array( 103, 104 ) );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
gp-entry-blocks/gpeb-process-feeds-on-edit.php(1 hunks)
🔇 Additional comments (1)
gp-entry-blocks/gpeb-process-feeds-on-edit.php (1)
32-35: Good defensive programming addition!Adding this safety check to verify that
$feedsis an array before proceeding witharray_filter()is an excellent fix. This prevents the fatal error that would occur when attempting to use array functions on non-array data. The comment is also clear and explains the purpose of this check.
Context
⛑️ Ticket(s): https://secure.helpscout.net/conversation/2878525752/79569
Summary
The snippet is supposed to process feeds on a form when the entry is updated via GPEB, however, it's causing a critical error. Fixed the issue by adding a safety check.