Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughUpdated test fixture for a date field ( Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
|
|
Overall Grade |
Security Reliability Complexity Hygiene |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| PHP | Apr 1, 2026 5:44p.m. | Review ↗ | |
| JavaScript | Apr 1, 2026 5:44p.m. | Review ↗ |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/phpunit/base/frm_factory.php`:
- Around line 145-153: The options-handling branch can fail when $field->options
is empty or when an option is a string rather than an array; update the branch
that handles types 'checkbox','select','radio' to first guard that
$field->options is non-empty and return a safe default (e.g., null or empty
string) if empty, then normalize the chosen $random_option so you safely extract
a value: if FrmField::is_option_true($field, 'separate_value') return the
option's 'value' when present otherwise return the option itself (or the string)
after converting array/string shapes into a consistent scalar; adjust the code
around $random_option and FrmField::is_option_true to avoid PHP notices and
invalid return types.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7dd35f42-24f6-4841-a0ca-c3f0f1657b20
📒 Files selected for processing (1)
tests/phpunit/base/frm_factory.php
| if ( in_array( $field->type, array( 'checkbox', 'select', 'radio' ), true ) ) { | ||
| // If checkbox, dropdown or radio field, return a random choice from the options. | ||
| $random_option = $field->options[ array_rand( $field->options ) ]; | ||
| if ( FrmField::is_option_true( $field, 'separate_value' ) ) { | ||
| return $random_option['value']; | ||
| } | ||
|
|
||
| return $random_option; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) Option-based field definitions in tests (check for missing options nearby):"
rg -nP --type=php -C3 "['\"]type['\"]\s*=>\s*['\"](checkbox|select|radio)['\"]" tests
echo
echo "2) separate_value usage (inspect whether selected options are arrays/strings in those setups):"
rg -nP --type=php -C3 "separate_value" tests classes
echo
echo "3) Option-array shapes (value/label/fallback patterns):"
rg -nP --type=php -C2 "['\"](value|label)['\"]\s*=>" tests classesRepository: Strategy11/formidable-forms
Length of output: 50384
Guard empty options and normalize option shape before returning values.
Line 147 can fail when $field->options is empty, and line 149 assumes every selected option is an array with 'value'. In this codebase, options can be strings or arrays with variable keys, so this code path can produce runtime errors and invalid test values. Empty options arrays are created in tests, confirming the empty-check is necessary.
Proposed fix
public function set_field_value( $field ) {
- if ( in_array( $field->type, array( 'checkbox', 'select', 'radio' ), true ) ) {
- // If checkbox, dropdown or radio field, return a random choice from the options.
- $random_option = $field->options[ array_rand( $field->options ) ];
- if ( FrmField::is_option_true( $field, 'separate_value' ) ) {
- return $random_option['value'];
- }
-
- return $random_option;
- }
+ if ( in_array( $field->type, array( 'checkbox', 'select', 'radio' ), true ) ) {
+ // If checkbox, dropdown or radio field, return a random choice from the options.
+ $options = is_array( $field->options ) ? array_values( $field->options ) : array();
+ if ( ! empty( $options ) ) {
+ $random_option = $options[ array_rand( $options ) ];
+
+ if ( ! is_array( $random_option ) ) {
+ return $random_option;
+ }
+
+ if ( FrmField::is_option_true( $field, 'separate_value' ) ) {
+ return $random_option['value'] ?? $random_option['label'] ?? reset( $random_option );
+ }
+
+ return $random_option['label'] ?? $random_option['value'] ?? reset( $random_option );
+ }
+ }
$value = rand_str();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/phpunit/base/frm_factory.php` around lines 145 - 153, The
options-handling branch can fail when $field->options is empty or when an option
is a string rather than an array; update the branch that handles types
'checkbox','select','radio' to first guard that $field->options is non-empty and
return a safe default (e.g., null or empty string) if empty, then normalize the
chosen $random_option so you safely extract a value: if
FrmField::is_option_true($field, 'separate_value') return the option's 'value'
when present otherwise return the option itself (or the string) after converting
array/string shapes into a consistent scalar; adjust the code around
$random_option and FrmField::is_option_true to avoid PHP notices and invalid
return types.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
tests/phpunit/base/frm_factory.php (1)
151-154:⚠️ Potential issue | 🔴 CriticalNormalize option shape before reading
['value']to avoid runtime failure.Line 154 assumes the random option is always an array. If options include scalar strings, this can fatally fail when
separate_valueis enabled.Proposed fix
if ( in_array( $field->type, array( 'checkbox', 'select', 'radio' ), true ) ) { - if ( empty( $field->options ) ) { + $options = is_array( $field->options ) ? array_values( $field->options ) : array(); + if ( empty( $options ) ) { return ''; } // If checkbox, dropdown or radio field, return a random choice from the options. - $random_option = $field->options[ array_rand( $field->options ) ]; + $random_option = $options[ array_rand( $options ) ]; if ( FrmField::is_option_true( $field, 'separate_value' ) ) { - return $random_option['value']; + if ( is_array( $random_option ) ) { + return $random_option['value'] ?? $random_option['label'] ?? (string) reset( $random_option ); + } + return $random_option; } + if ( is_array( $random_option ) ) { + return $random_option['label'] ?? $random_option['value'] ?? (string) reset( $random_option ); + } return $random_option; }#!/bin/bash set -euo pipefail echo "1) Confirm current set_field_value implementation context:" rg -nP --type=php -C4 "function set_field_value|array_rand|separate_value" tests/phpunit/base/frm_factory.php echo echo "2) Find option-based field definitions and option value shapes in tests/classes:" rg -nP --type=php -C3 "['\"]type['\"]\s*=>\s*['\"](checkbox|select|radio)['\"]|['\"]options['\"]\s*=>|['\"]label['\"]\s*=>|['\"]value['\"]\s*=>" tests classes echo echo "3) Find fixtures/usages where separate_value is enabled:" rg -nP -C2 "separate_value"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/phpunit/base/frm_factory.php` around lines 151 - 154, In the block that picks $random_option (used by set_field_value in tests/phpunit/base/frm_factory.php) normalize the option shape before accessing ['value']: detect if $random_option is a scalar (string/int) or an object and convert it to an associative array with a 'value' key (e.g. treat scalar as ['value' => $random_option]), or fall back to using a 'label' when 'value' is missing; perform this normalization right after $random_option = $field->options[ array_rand( $field->options ) ] and before the FrmField::is_option_true(...) branch so that references to $random_option['value'] are safe.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@tests/phpunit/base/frm_factory.php`:
- Around line 151-154: In the block that picks $random_option (used by
set_field_value in tests/phpunit/base/frm_factory.php) normalize the option
shape before accessing ['value']: detect if $random_option is a scalar
(string/int) or an object and convert it to an associative array with a 'value'
key (e.g. treat scalar as ['value' => $random_option]), or fall back to using a
'label' when 'value' is missing; perform this normalization right after
$random_option = $field->options[ array_rand( $field->options ) ] and before the
FrmField::is_option_true(...) branch so that references to
$random_option['value'] are safe.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 45feb7d2-91f0-4412-b033-82c587f2939d
📒 Files selected for processing (2)
.htaccesstests/phpunit/base/frm_factory.php
💤 Files with no reviewable changes (1)
- .htaccess
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3051 +/- ##
============================================
- Coverage 26.60% 26.59% -0.02%
- Complexity 8917 9078 +161
============================================
Files 145 148 +3
Lines 30018 30476 +458
============================================
+ Hits 7987 8105 +118
- Misses 22031 22371 +340 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Fix https://github.com/Strategy11/formidable-api/issues/213
Updates:
end_yearof field with keydate-fieldto2050to fix errors like below in the API plugin:Summary by CodeRabbit
Tests
Chores