gpcc-copy-to-conditionally-hidden-fields.php: Fixed an issue with copying value with multi input field triggers.
#1069
Conversation
…opying value with multi input field triggers.
WalkthroughThe update enhances the Gravity Forms Copy Cat functionality by adding logic to handle conditional fields. For each target field, the code retrieves an associated condition field from the submission entry. If the condition field represents a multi-input field (detected via a dot in its identifier), the identifier is split to access the specific index. The code then verifies whether the retrieved condition value matches the expected value from the field’s choices. If the condition is not met, the process skips the value copying and continues to the next target. No public APIs were altered in this change. Changes
Sequence Diagram(s)sequenceDiagram
participant GPCC as "Copy Cat Module"
participant Entry as "Submission Entry"
GPCC ->> Entry: Retrieve associated condition field value
Note right of GPCC: Check if field is multi-input (identifier contains ".")
GPCC ->> Entry: Parse identifier to access specific input (if applicable)
GPCC ->> Entry: Compare retrieved value with expected condition
alt Condition met
GPCC ->> GPCC: Copy source field value to target field
else Condition not met
GPCC ->> GPCC: Skip copying and continue to the next target
end
Suggested reviewers
✨ 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 (
|
![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 (3)
gp-copy-cat/gpcc-copy-to-conditionally-hidden-fields.php (3)
32-35: Missing error handling for non-existent fields.The code retrieves source and condition fields without verifying their existence first. If either
$target['source']or$target['condition']refers to a non-existent field ID, this could lead to PHP errors.Consider adding validation before attempting to access these fields:
+ if ( ! isset( $target['source'] ) || ! isset( $target['condition'] ) ) { + continue; + } + $source_field = GFAPI::get_field( $form, $target['source'] ); $source_values = $source_field->get_value_submission( array() ); $condition_field = GFAPI::get_field( $form, $target['condition'] ); + + if ( empty( $source_field ) || empty( $condition_field ) ) { + continue; + } + $condition_value = $condition_field->get_value_submission( $entry );
37-48: Improve multi-input field condition handling with additional validations.The code correctly identifies multi-input fields by checking for dots in the identifier, but could benefit from additional validations to prevent potential issues.
Consider these improvements:
// for multi-input fields, we need to check the index // to see if the condition is met. if ( strpos( $target['condition'], '.' ) !== false ) { list( $base, $index ) = explode( '.', $target['condition'] ); + + // Ensure index is numeric and that choices exists + if ( ! is_numeric( $index ) || ! isset( $condition_field['choices'] ) || ! is_array( $condition_field['choices'] ) ) { + continue; + } + if ( isset( $condition_field['choices'][ (int) $index ] ) ) { if ( $condition_field['choices'][ (int) $index ]['value'] !== $condition_value ) { continue; } } else { continue; } }
32-48: Add comprehensive documentation for the conditional copy logic.While the code includes basic comments, adding more detailed documentation would improve maintainability, especially since this is handling complex form field interactions.
Consider adding more detailed documentation before the conditional logic:
+ /** + * Handle conditional copying based on field conditions + * + * For multi-input fields (like checkboxes, radio buttons), + * we need to check if the specific choice is selected + * before copying the value to the target field. + * + * @param array $target The target field configuration. + * @param array $entry The form entry data. + * @param array $source_field The source field object. + */ $source_field = GFAPI::get_field( $form, $target['source'] ); $source_values = $source_field->get_value_submission( array() ); $condition_field = GFAPI::get_field( $form, $target['condition'] ); $condition_value = $condition_field->get_value_submission( $entry );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
gp-copy-cat/gpcc-copy-to-conditionally-hidden-fields.php(1 hunks)
🔇 Additional comments (2)
gp-copy-cat/gpcc-copy-to-conditionally-hidden-fields.php (2)
50-57: LGTM! The value copying logic is robust.The implementation properly handles both array values (multi-input fields) and scalar values, maintaining the input structure when copying from source to target fields.
39-44:❓ Verification inconclusive
Logic for condition comparison might be incorrect.
The current implementation compares
$condition_field['choices'][(int)$index]['value']with$condition_value, but based on how Gravity Forms stores multi-input field values in entries, this comparison may not work correctly in all scenarios.Consider verifying if the comparison logic works for all expected scenarios. For radiobutton and checkbox inputs, the condition value might need to be retrieved differently.
🏁 Script executed:
#!/bin/bash # Search for any similar condition handling in the codebase to understand best practices rg "condition.*field.*choices.*value" --type php -g "!vendor/*" -A 5 -B 5Length of output: 1132
Attention: Verify Multi-Input Condition Comparison Logic
The code currently compares
$condition_field['choices'][(int) $index]['value']with
$condition_valuein
gp-copy-cat/gpcc-copy-to-conditionally-hidden-fields.php(lines 39–44). Given that Gravity Forms may store multi-input field values differently—especially for radiobutton and checkbox inputs—it’s important to double-check that this comparison correctly handles all expected scenarios.
- Action Required:
- Please verify that retrieving the condition value in this way aligns with Gravity Forms’ storage for both radiobutton and checkbox inputs.
- Consider reviewing or testing against sample entries to ensure that the type cast and direct value comparison don’t overlook any edge cases.
| if ( strpos( $target['condition'], '.' ) !== false ) { | ||
| list( $base, $index ) = explode( '.', $target['condition'] ); | ||
| if ( isset( $condition_field['choices'][ (int) $index ] ) ) { | ||
| if ( $condition_field['choices'][ (int) $index ]['value'] !== $condition_value ) { | ||
| continue; | ||
| } | ||
| } else { | ||
| continue; | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add support for non-choice conditions.
The current implementation only handles conditions for fields with choices. Consider expanding support to other field types that don't use the 'choices' property.
Add support for other field types by checking the field type:
if ( strpos( $target['condition'], '.' ) !== false ) {
list( $base, $index ) = explode( '.', $target['condition'] );
- if ( isset( $condition_field['choices'][ (int) $index ] ) ) {
- if ( $condition_field['choices'][ (int) $index ]['value'] !== $condition_value ) {
- continue;
- }
- } else {
- continue;
- }
+
+ // Handle different field types differently
+ if ( isset( $condition_field['type'] ) ) {
+ switch ( $condition_field['type'] ) {
+ case 'checkbox':
+ case 'radio':
+ case 'select':
+ if ( isset( $condition_field['choices'][ (int) $index ] ) ) {
+ if ( $condition_field['choices'][ (int) $index ]['value'] !== $condition_value ) {
+ continue;
+ }
+ } else {
+ continue;
+ }
+ break;
+
+ default:
+ // For other field types, use a different comparison method
+ if ( $condition_value != $target['expected_value'] ) {
+ continue;
+ }
+ break;
+ }
+ } else {
+ continue;
+ }
}Note: This implementation assumes there's an 'expected_value' property in the target configuration for non-choice field types. You'll need to adjust this based on your specific needs.
📝 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.
| if ( strpos( $target['condition'], '.' ) !== false ) { | |
| list( $base, $index ) = explode( '.', $target['condition'] ); | |
| if ( isset( $condition_field['choices'][ (int) $index ] ) ) { | |
| if ( $condition_field['choices'][ (int) $index ]['value'] !== $condition_value ) { | |
| continue; | |
| } | |
| } else { | |
| continue; | |
| } | |
| } | |
| if ( strpos( $target['condition'], '.' ) !== false ) { | |
| list( $base, $index ) = explode( '.', $target['condition'] ); | |
| // Handle different field types differently | |
| if ( isset( $condition_field['type'] ) ) { | |
| switch ( $condition_field['type'] ) { | |
| case 'checkbox': | |
| case 'radio': | |
| case 'select': | |
| if ( isset( $condition_field['choices'][ (int) $index ] ) ) { | |
| if ( $condition_field['choices'][ (int) $index ]['value'] !== $condition_value ) { | |
| continue; | |
| } | |
| } else { | |
| continue; | |
| } | |
| break; | |
| default: | |
| // For other field types, use a different comparison method | |
| if ( $condition_value != $target['expected_value'] ) { | |
| continue; | |
| } | |
| break; | |
| } | |
| } else { | |
| continue; | |
| } | |
| } |
Context
⛑️ Ticket(s): https://secure.helpscout.net/conversation/2893866689/81562
Summary
The snippet above to Copy Values for Conditionally Hidden Fields doesn't support conditional copy cat.
Demo of the issue (Samuel):
https://www.loom.com/share/a49b8943c29e4955a807135bd7df24f9
Demo of the fix:
https://www.loom.com/share/43be9c8dbb8740448fd3102b8fb995a5