-
Notifications
You must be signed in to change notification settings - Fork 91
gw-conditional-logic-operator-does-not-contain.php
: Added snippet for does not contain conditional logic comparison.
#1073
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
…or does not contain conditional logic comparison.
WalkthroughA new PHP class, Changes
Sequence Diagram(s)sequenceDiagram
participant Init as "WordPress Init"
participant GF as "Gravity Forms System"
participant CLO as "GF_CLO_Does_Not_Contain"
participant Admin as "Admin Interface"
participant Form as "Form Frontend"
participant DB as "Database"
Init->>CLO: Instantiate on init action
CLO->>GF: Register filters (operator whitelist, script output, evaluation logic)
GF->>Admin: Request inline script output
CLO->>Admin: Output JS variable & function for "does not contain" operator
GF->>Form: Check if form supports conditional logic
Form->>CLO: Call load_form_script to attach necessary scripts
Form->>CLO: Evaluate condition with evaluate_operator
CLO->>CLO: Execute "does not contain" logic check
CLO->>DB: Convert operator to SQL ("NOT LIKE") format for field filters
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
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. 🪧 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: 2
🧹 Nitpick comments (5)
gravity-forms/gw-conditional-logic-operator-does-not-contain.php (5)
34-50
: Consider adding admin capability check for additional securityWhile the current page check provides some protection, consider also checking for appropriate admin capabilities before outputting the script.
public function output_admin_inline_script() { - if ( ! GFForms::get_page() && ! is_admin() && ! in_array( rgget( 'page' ), array( 'gp-email-users' ) ) ) { + if ( ( ! GFForms::get_page() && ! is_admin() && ! in_array( rgget( 'page' ), array( 'gp-email-users' ) ) ) || ! current_user_can( 'manage_options' ) ) { return; }
119-133
: Consider adding type checking for more robust evaluationThe operator evaluation could be more defensive by ensuring both values are strings before performing the string operation.
// If the field contains the target value, it's not a match. -$is_match = strpos( $field_value, $target_value ) === false; +$is_match = is_string( $field_value ) && is_string( $target_value ) ? + strpos( (string) $field_value, (string) $target_value ) === false : + true;
106-109
: Add option for case-insensitive matchingConsider adding an option for case-insensitive matching, which would be useful in many scenarios.
This would require modifications to both the JavaScript and PHP evaluation functions to support a case-insensitive option, potentially using
toLowerCase()
in JavaScript andstripos()
in PHP.
158-160
: Consider making the class instantiation filterableFor maximum flexibility, consider wrapping the class instantiation in a conditional that can be filtered.
# Configuration - -new GF_CLO_Does_Not_Contain(); +if ( apply_filters( 'gw_enable_clo_does_not_contain', true ) ) { + new GF_CLO_Does_Not_Contain(); +}
127-128
: Add documentation for the evaluation logicWhile the comment is helpful, consider adding more detailed documentation to explain the evaluation logic, especially for developers who might extend this functionality.
-// If the field contains the target value, it's not a match. +/** + * Evaluate if the field value does NOT contain the target value. + * + * strpos() returns false when the needle is not found in the haystack. + * Therefore, if strpos() returns false, it means the target value is not + * contained in the field value, which is what we want for "does not contain". + */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
gravity-forms/gw-conditional-logic-operator-does-not-contain.php
(1 hunks)
🔇 Additional comments (3)
gravity-forms/gw-conditional-logic-operator-does-not-contain.php (3)
1-15
: Well-structured plugin header with clear documentationThe plugin header follows WordPress standards and provides clear information about the functionality. Including the Loom instruction video link is a thoughtful addition for users who prefer visual instructions.
16-32
: Good initialization with proper hook registrationThe class is properly initialized with appropriate hooks into Gravity Forms' conditional logic system. The hook structure follows WordPress best practices.
135-151
: Good SQL conversion for the new operatorThe conversion to SQL-compatible field filters is well-implemented, properly adding wildcards for the LIKE operator and converting to NOT LIKE for the containment check.
gravity-forms/gw-conditional-logic-operator-does-not-contain.php
Outdated
Show resolved
Hide resolved
gravity-forms/gw-conditional-logic-operator-does-not-contain.php
Outdated
Show resolved
Hide resolved
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.
@saifsultanc I'm requesting changes, but feel free to push back on any suggestions you think are out of scope or don't make sense!
} | ||
|
||
gform.addFilter( 'gform_conditional_logic_operators', function( operators ) { | ||
operators.does_not_contain = 'does not contain'; |
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.
Might be worth setting $text = esc_html_e( 'does not contain' );
in a variable above in PHP, then using here and above on line 41. Or using the translation function here as well
gravity-forms/gw-conditional-logic-operator-does-not-contain.php
Outdated
Show resolved
Hide resolved
gravity-forms/gw-conditional-logic-operator-does-not-contain.php
Outdated
Show resolved
Hide resolved
$rule['gwclodncEvaluatingOperator'] = true; | ||
|
||
// If the field contains the target value, it's not a match. | ||
$is_match = strpos( $field_value, $target_value ) === false; |
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.
Would it make sense to make this check case insensitive, in the JS check and in convert_conditional_logic_to_field_filters()
?
…or does not contain conditional logic comparison.
Done some changes. Keeping overall consistent/similar to the IS IN snippet we already have here: https://github.com/gravitywiz/snippet-library/blob/master/gravity-forms/gw-conditional-logic-operator-is-in.php |
Context
⛑️ Ticket(s): https://secure.helpscout.net/conversation/2901482497/81913
Summary
Snippet that adds 'does not contain' operator for conditional logic comparison
Inspired from https://gravitywiz.com/snippet-library/gw-conditional-logic-operator-is-in/
Loom Demo:
https://www.loom.com/share/8e1b27ec47b341dbb4f0da2bec6a960b