gptos-terms-only-modifier.php: Added snippet to only display terms with the TOS checkbox.
#1051
Conversation
…with the TOS checkbox.
|
Caution Review failedThe pull request is closed. WalkthroughA new PHP file has been added to introduce a Changes
Sequence Diagram(s)sequenceDiagram
participant GF as Gravity Forms
participant Modifier as TermsOnlyModifier
participant Terms as get_terms
GF->>Modifier: Trigger gform_merge_tag_filter($value, merge_tag, options, field)
alt Field type != "tos"
Modifier-->>GF: Return original value
else Field type == "tos"
Modifier->>Modifier: Check for "terms_only" in options
alt "terms_only" not present
Modifier-->>GF: Return original value
else "terms_only" present
alt merge_tag != "all_fields"
Modifier->>Modifier: Wrap value in unordered list
end
Modifier->>Terms: Retrieve terms (get_terms)
Terms-->>Modifier: Return terms data
Modifier->>Modifier: Format terms (wpautop)
Modifier-->>GF: Return formatted terms
end
end
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🪧 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)
gp-terms-of-service/gptos-terms-only-modifier.php (1)
10-27: Consider adding defensive coding around form retrievalThe code doesn't handle potential failures when retrieving the form with
GFAPI::get_form(). If the form doesn't exist, this could cause errors.- $value = wpautop( $field->get_terms( GFAPI::get_form( $field->formId ) ) ); + $form = GFAPI::get_form( $field->formId ); + if ( ! $form || is_wp_error( $form ) ) { + return $value; + } + $value = wpautop( $field->get_terms( $form ) );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
gp-terms-of-service/gptos-terms-only-modifier.php(1 hunks)
🔇 Additional comments (4)
gp-terms-of-service/gptos-terms-only-modifier.php (4)
1-9: Documentation looks goodThe file header clearly explains the purpose of this snippet - adding a
:terms_onlymodifier to display only the terms content from a Terms of Service field. The inclusion of documentation links and an instruction video is helpful for users.
10-13: Early validation check looks appropriateGood use of early return pattern to ensure only Terms of Service fields are processed by this modifier.
15-18: Clean validation for the modifierProperly checks for the presence of the 'terms_only' modifier in the options before proceeding.
20-22: Clarify the purpose of the conditional list wrappingThe logic for non-'all_fields' merge tags is unclear. If this modifier is specifically for the 'all_fields' merge tag (as indicated in the comments), why are we handling other merge tags differently?
Can you explain the reasoning behind wrapping the value in a list item only when the merge tag is not 'all_fields'? This seems counterintuitive to the stated purpose.
| $value = wpautop( $field->get_terms( GFAPI::get_form( $field->formId ) ) ); | ||
|
|
||
| return $value; |
There was a problem hiding this comment.
Potential type error in field access
Line 24 assumes $field is an object by calling $field->get_terms(), but line 11 treats it as an array with $field['type']. This inconsistency could cause a PHP fatal error.
- $value = wpautop( $field->get_terms( GFAPI::get_form( $field->formId ) ) );
+ $value = wpautop( rgar( $field, 'type' ) === 'tos' ? $field->get_terms( GFAPI::get_form( $field->formId ) ) : '' );Additionally, there's no error handling for the GFAPI::get_form() call - what if the form isn't found?
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| $value = wpautop( $field->get_terms( GFAPI::get_form( $field->formId ) ) ); | |
| return $value; | |
| $value = wpautop( rgar( $field, 'type' ) === 'tos' ? $field->get_terms( GFAPI::get_form( $field->formId ) ) : '' ); | |
| return $value; |
…with the TOS checkbox.
Context
⛑️ Ticket(s): https://secure.helpscout.net/conversation/2876696517/79496
https://gravitywiz.slack.com/archives/GMP0ZMNSE/p1741974702635409
Summary
Added a
:terms_onlymerge tag modifier that would allow you to display the terms in notifications or confirmations without also having to display the value /text of the checkbox.https://www.loom.com/share/d69c48bea2d1429ab019310d2bc6c1e6