gpls-skip-limit-if-blank.php: Added new snippet for skipping limit if any Field Value is blank.
#1179
Conversation
…if any Field Value is blank.
WalkthroughAdds a new PHP file registering a Changes
Sequence DiagramsequenceDiagram
participant Feed as Feed Processor
participant Hook as gpls_should_apply_rules
participant Cache as Feed Skip Cache
participant Rules as Rule Iterator
participant Logic as Value Evaluation
Feed->>Hook: apply_filters('gpls_should_apply_rules', should_apply, form_id, rule_test)
Hook->>Cache: check cached skip for feed_id (if present)
alt cached skip exists
Cache-->>Hook: return cached false
else no cache
Hook->>Rules: get rule_group / rules
loop each GPLS_Rule_Field
Rules->>Logic: get_limit_field_value(get_field())
alt value is array
Logic->>Logic: GFCommon::trim_deep -> check empty
alt array empty
Logic->>Cache: cache skip for feed_id
Logic-->>Hook: return false
end
else value is string
Logic->>Logic: trim whitespace -> check blank
alt blank
Logic->>Cache: cache skip for feed_id
Logic-->>Hook: return false
end
end
end
end
Hook-->>Feed: return should_apply (possibly false)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🔇 Additional comments (7)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
gp-limit-submissions/gpls-skip-limit-if-blank.php (1)
37-63: Logic correctly handles different value types.The handling of false values, arrays, and strings is appropriate. The use of
GFCommon::trim_deep()andrgblank()aligns with Gravity Forms conventions.Optional: The cache update logic at lines 45-47 and 59-61 is duplicated. If the static cache is retained (see separate comment), you could consolidate this:
// Helper to mark feed for skipping $mark_skip = function() use ( $feed_id, &$skip_feed_ids ) { if ( $feed_id ) { $skip_feed_ids[ $feed_id ] = true; } return false; }; // Then use: return $mark_skip();However, given the critical issue with the static cache itself, this refactoring may not be necessary.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
gp-limit-submissions/gpls-skip-limit-if-blank.php(1 hunks)
🔇 Additional comments (4)
gp-limit-submissions/gpls-skip-limit-if-blank.php (4)
1-8: Clear documentation.The header clearly explains the snippet's purpose and links to relevant documentation.
13-22: Good defensive programming.The early return guards and form ID targeting logic are appropriate. The strict type checking in
in_array()is a good practice.
24-24: Good use of defensive method checking.The
method_exists()check before callingget_feed_id()is appropriate defensive programming.
9-9: Filter hook registration looks appropriate.The filter parameters and priority (20) appear correct, and the final return properly preserves the original
$should_applyvalue when no blank values are detected.Also applies to: 66-67
| continue; | ||
| } | ||
|
|
||
| $value = $rule->get_limit_field_value( $rule->get_field() ); |
There was a problem hiding this comment.
Consider adding null check for get_field() result.
If get_field() can return null, passing it directly to get_limit_field_value() might cause issues. Consider adding a defensive check.
+ $field = $rule->get_field();
+ if ( ! $field ) {
+ continue;
+ }
- $value = $rule->get_limit_field_value( $rule->get_field() );
+ $value = $rule->get_limit_field_value( $field );Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In gpl-limit-submissions/gpls-skip-limit-if-blank.php around line 35, the call
passes $rule->get_field() directly into get_limit_field_value() without checking
for null; add a defensive null check: first get the field into a variable,
verify it is not null (and optionally that it is of the expected type/string),
and only then call get_limit_field_value($field); if null, handle appropriately
(return early, skip processing, or use a safe default) to avoid errors.
…if any Field Value is blank.
2 participants
Context
⛑️ Ticket(s): https://secure.helpscout.net/conversation/3089383028/89888
Summary
This snippet adds the option to skip a Limit Submissions feed whenever any field-based rule in the targeted form resolves to a blank value.
Example scenario
User has the following feed configured with the goal to prevent visitors from being able to submit the same email address and coupon code more than once.
While the limit submission feed does prevent a second submission using the same email and coupon code, it will also prevent any submission where the same email address is used but the coupon field is left blank. With this snippet installed, the limit submission feed is skipped unless both email and coupon fields contains values.