You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).
⌛ How to resolve this issue?
After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.
To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.
🚦 How do rate limits work?
CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.
For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.
Reviewing files that changed from the base of the PR and between daeef13 and 23097d5.
📒 Files selected for processing (5)
classes/controllers/FrmAddressesController.php
classes/controllers/FrmComboFieldsController.php
classes/models/fields/FrmFieldAddress.php
classes/models/fields/FrmFieldCombo.php
tests/phpunit/fields/test_FrmFieldAddress.php
📝 Walkthrough
Walkthrough
Adds a native address field type, shared combo-field helpers, address-specific rendering and formatting, form-builder and frontend templates, and updates PayPal/Square integrations to use the new Lite country-code lookup.
Defines shared combo-field helpers for defaults, placeholders, labels, attributes, and error classes, and makes inputs-container attributes publicly accessible from combo field models.
Address field model classes/models/fields/FrmFieldAddress.php
Defines the address field model with type-specific sub-fields, address formatting, import parsing, ZIP validation, sanitization, and export headings.
Address controller and field registration classes/controllers/FrmAddressesController.php, classes/factories/FrmFieldFactory.php, classes/models/FrmField.php, classes/controllers/FrmFieldsController.php
Adds the address controller, registers the address field type, adds it to field selection, removes it from Pro lists, and removes it from the unique-values upsell list.
View templates and styles classes/views/combo-fields/..., classes/views/frm-fields/front-end/address-field/..., classes/views/frm-fields/back-end/combo-field/sub-field-options.php, resources/scss/admin/components/form/_form-field.scss
Adds the address type selector, combo-field rendering templates, the address frontend template, default-value select rendering for sub-fields with options, and spacing styles for combo inputs.
Adds address-field tests and updates combo-field fixture names and expectations.
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Strategy11/formidable-forms#3093: Touches PayPal Lite address formatting and country-code handling, which this PR also updates to use FrmAddressesController.
Suggested reviewers
garretlaxton
Poem
🐇 A new address hops into place,
Line by line with a combo-field grace.
US, EU, and country codes too,
Lite takes the wheel and the templates do.
With tests in tow, the rabbit cheerily runs—
A tidy new field for everyone 🥕
We reviewed changes in b096bc4...23097d5 on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.
Some issues found as part of this review are outside of the diff in this pull request and aren't shown in the inline review comments due to GitHub's API limitations. You can see those issues on the DeepSource dashboard.
AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.
The reason will be displayed to describe this comment to others. Learn more.
Variable $field might not be defined
A variable has been used but not defined, which may result in warnings during program execution. This can also cause bugs since the intended usage scope of the variable is not known.
The reason will be displayed to describe this comment to others. Learn more.
Variable $field might not be defined
A variable has been used but not defined, which may result in warnings during program execution. This can also cause bugs since the intended usage scope of the variable is not known.
The reason will be displayed to describe this comment to others. Learn more.
Variable $field might not be defined
A variable has been used but not defined, which may result in warnings during program execution. This can also cause bugs since the intended usage scope of the variable is not known.
The reason will be displayed to describe this comment to others. Learn more.
Variable $field might not be defined
A variable has been used but not defined, which may result in warnings during program execution. This can also cause bugs since the intended usage scope of the variable is not known.
The reason will be displayed to describe this comment to others. Learn more.
Variable $field might not be defined
A variable has been used but not defined, which may result in warnings during program execution. This can also cause bugs since the intended usage scope of the variable is not known.
The reason will be displayed to describe this comment to others. Learn more.
Variable $field might not be defined
A variable has been used but not defined, which may result in warnings during program execution. This can also cause bugs since the intended usage scope of the variable is not known.
The reason will be displayed to describe this comment to others. Learn more.
Variable $field might not be defined
A variable has been used but not defined, which may result in warnings during program execution. This can also cause bugs since the intended usage scope of the variable is not known.
The reason will be displayed to describe this comment to others. Learn more.
Variable $field might not be defined
A variable has been used but not defined, which may result in warnings during program execution. This can also cause bugs since the intended usage scope of the variable is not known.
The reason will be displayed to describe this comment to others. Learn more.
Variable $field might not be defined
A variable has been used but not defined, which may result in warnings during program execution. This can also cause bugs since the intended usage scope of the variable is not known.
The reason will be displayed to describe this comment to others. Learn more.
Variable $field might not be defined
A variable has been used but not defined, which may result in warnings during program execution. This can also cause bugs since the intended usage scope of the variable is not known.
The reason will be displayed to describe this comment to others. Learn more.
Variable $field might not be defined
A variable has been used but not defined, which may result in warnings during program execution. This can also cause bugs since the intended usage scope of the variable is not known.
The reason will be displayed to describe this comment to others. Learn more.
Variable $field might not be defined
A variable has been used but not defined, which may result in warnings during program execution. This can also cause bugs since the intended usage scope of the variable is not known.
Normalize optional address keys before using them.
After Line 182 relaxes validation to line1, Line 195 and Lines 198-206 still read optional keys directly. Missing keys now trigger undefined-index notices on payment AJAX paths.
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@square/controllers/FrmSquareLiteAppController.php` around lines 182 - 206,
The code validates that the address array contains the required line1 key but
then directly accesses optional address keys (line2, city, state, zip) without
verifying they exist first, causing undefined-index notices. After the initial
address validation and before accessing individual address fields in the section
where you populate the details array (around lines 198-206), ensure each
optional address key is safely accessed using either isset() checks or the null
coalescing operator (??). This should be done for address['line2'],
address['city'], address['state'], address['zip'], and address['country'] to
provide default values when these keys are missing from the address array.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@classes/controllers/FrmAddressesController.php`:
- Around line 140-152: The code accesses $field['address_type'] multiple times
in conditional checks without ensuring it exists, which will cause
undefined-index notices for legacy or incomplete field configurations. Add a
defensive default assignment for $field['address_type'] at the beginning of the
get_sub_fields() method or at the start of this code block, before the first
conditional that checks for 'europe', 'us', or 'generic' address types. Use a
sensible default like 'generic' to keep rendering deterministic when the
address_type key is absent from the field configuration.
- Line 276: The `get_country_code()` method accesses `self::$country_codes`
directly as an array without validating that the filtered result from the
`frm_country_codes` filter is actually an array. If a callback returns a
non-array value, this causes type errors when the method is invoked in payment
processing paths. Add validation where the filter result is assigned to
`self::$country_codes` to ensure it returns an array, and provide a default
empty array as a fallback if the filter returns a non-array value or null. This
will prevent type errors when `get_country_code()` is called during PayPal and
Square payment processing.
In `@classes/controllers/FrmComboFieldsController.php`:
- Around line 112-116: The FrmComboFieldsController is using $atts['key'] to
look up sub-field identifiers in methods like include_placeholder and when
accessing the default_value array, but downstream callers are passing the
sub-field identifier under the 'name' key instead, causing silent lookup
failures and undefined index notices. Replace all references to $atts['key']
with $atts['name'] in the combo field helper methods to align the sub-field key
contract, specifically in the include_placeholder function call and the
default_value array access around lines 112-116, and apply the same fix to the
corresponding code at lines 179-180.
- Around line 105-109: The $field_obj variable returned from
FrmFieldFactory::get_field_type() has a broader type, but the
address_string_to_array() method is specific to address field types. After
checking that $atts['field']['type'] === 'address', narrow the type of
$field_obj to the specific address field type (using an instanceof check, type
assertion, or type cast) before calling address_string_to_array() on it to
satisfy static analysis checks and prevent a fatal runtime error.
- Around line 164-165: The code directly accesses `$field[ $option_name ]`
without first verifying that the key exists in the array, which causes PHP
notices when the key is absent. Guard the array access by using isset() to check
if the key exists before attempting to access it. Modify the condition to first
verify that the key is present in the $field array, and only then check if its
value is not empty. This will prevent undefined array key notices from being
rendered.
In `@classes/models/fields/FrmFieldAddress.php`:
- Around line 333-339: The current CSV import parsing for addresses uses
is_numeric() to distinguish zip codes from country names, which fails for
alphanumeric postal codes like Canadian (M5V 3A8) or UK (SW1A 1AA) formats.
Replace the is_numeric() check with a more robust validation method that can
handle alphanumeric postal code formats. This may involve using regex patterns
to match common postal code formats or implementing a smarter detection
mechanism that doesn't rely purely on numeric validation. Make sure to apply
this fix consistently across all affected locations where address field parsing
occurs, including the code sections around lines 333-339 and the additional
locations mentioned in the review (354-370).
- Around line 146-147: In the FrmFieldAddress class, the translatable_strings
method currently assigns the result of parent::translatable_strings() to the
$strings variable and then returns it on the next line. This violates the
PHP-CS-Fixer return_assignment rule. Remove the intermediate variable assignment
and directly return the result of the parent::translatable_strings() call in a
single statement.
In `@classes/views/frm-fields/front-end/address-field/address-field.php`:
- Around line 47-58: The helper functions
FrmComboFieldsController::maybe_add_error_class,
FrmComboFieldsController::add_atts_to_input, and
FrmComboFieldsController::get_dropdown_label expect a key parameter in the $atts
array, but the code is passing name instead. Update the compact() calls on lines
47, 56, and 58 (and line 82 mentioned in "Also applies to") to pass key instead
of name to these helper methods, ensuring the parameter name matches what the
combo-field helper contracts expect and preventing undefined index notices.
---
Outside diff comments:
In `@square/controllers/FrmSquareLiteAppController.php`:
- Around line 182-206: The code validates that the address array contains the
required line1 key but then directly accesses optional address keys (line2,
city, state, zip) without verifying they exist first, causing undefined-index
notices. After the initial address validation and before accessing individual
address fields in the section where you populate the details array (around lines
198-206), ensure each optional address key is safely accessed using either
isset() checks or the null coalescing operator (??). This should be done for
address['line2'], address['city'], address['state'], address['zip'], and
address['country'] to provide default values when these keys are missing from
the address array.
🪄 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: cff847f2-64dc-4ad9-9060-9e7518bbded9
📥 Commits
Reviewing files that changed from the base of the PR and between b096bc4 and 3937b09.
The reason will be displayed to describe this comment to others. Learn more.
🩺 Stability & Availability | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash# Audit local callbacks for frm_country_codes and inspect return values manually.
rg -n --type=php "add_filter\s*\(\s*'frm_country_codes'".
Repository: Strategy11/formidable-forms
Length of output: 165
🏁 Script executed:
#!/bin/bash# Find and inspect the FrmAddressesController.php file
find . -name "FrmAddressesController.php" -type f
Repository: Strategy11/formidable-forms
Length of output: 214
🏁 Script executed:
#!/bin/bash# Look for the apply_filters call with frm_country_codes
rg -n "frm_country_codes" --type=php .| head -30
Repository: Strategy11/formidable-forms
Length of output: 303
🏁 Script executed:
#!/bin/bash# Search for maybe_define_country_codes method
rg -n "maybe_define_country_codes" --type=php .
Repository: Strategy11/formidable-forms
Length of output: 549
🏁 Script executed:
#!/bin/bash# Read the FrmAddressesController.php file around lines 270-280 to see get_country_code
sed -n '270,290p' ./classes/controllers/FrmAddressesController.php | cat -n
Repository: Strategy11/formidable-forms
Length of output: 752
🏁 Script executed:
#!/bin/bash# Read around line 551 to see the filter application
sed -n '545,560p' ./classes/controllers/FrmAddressesController.php | cat -n
Repository: Strategy11/formidable-forms
Length of output: 486
🏁 Script executed:
#!/bin/bash# Get the full file outline to understand structure
ast-grep outline ./classes/controllers/FrmAddressesController.php
Repository: Strategy11/formidable-forms
Length of output: 228
🏁 Script executed:
#!/bin/bash# Read the complete maybe_define_country_codes method
sed -n '286,560p' ./classes/controllers/FrmAddressesController.php | cat -n
Repository: Strategy11/formidable-forms
Length of output: 17206
🏁 Script executed:
#!/bin/bash# Search for PayPal and Square payment integration usage with country codes
rg -n -A 5 "get_country_code" ./classes/controllers/FrmAddressesController.php
Repository: Strategy11/formidable-forms
Length of output: 340
🏁 Script executed:
#!/bin/bash# Look for PayPal and Square payment path code
find . -name "*[Pp]ay[Pp]al*" -o -name "*[Ss]quare*"| grep -E "\.(php|js)$"| head -10
Repository: Strategy11/formidable-forms
Length of output: 672
🏁 Script executed:
#!/bin/bash# Search for get_country_code usage in PayPal files
rg -n "get_country_code" ./paypal/ --type=php
Repository: Strategy11/formidable-forms
Length of output: 578
🏁 Script executed:
#!/bin/bash# Look for Square integration
find . -path "./paypal" -prune -o -name "*[Ss]quare*" -type f -print | head -20
Repository: Strategy11/formidable-forms
Length of output: 1245
🏁 Script executed:
#!/bin/bash# Search for square payment processing
rg -n "square" --type=php . -i | grep -i "payment\|country\|code"| head -20
Repository: Strategy11/formidable-forms
Length of output: 3174
🏁 Script executed:
#!/bin/bash# Search for get_country_code usage in Square files
rg -n "get_country_code" ./square/ --type=php
Repository: Strategy11/formidable-forms
Length of output: 305
🏁 Script executed:
#!/bin/bash# Look at PayPal usage context around line 415 and 478
sed -n '410,420p' ./paypal/controllers/FrmPayPalLiteAppController.php | cat -n
Repository: Strategy11/formidable-forms
Length of output: 575
🏁 Script executed:
#!/bin/bash# Look at context around line 478 in PayPal
sed -n '473,483p' ./paypal/controllers/FrmPayPalLiteAppController.php | cat -n
Repository: Strategy11/formidable-forms
Length of output: 569
Validate filtered country-code map before indexing in payment paths.
The frm_country_codes filter output is used directly as an array map without validation. If a callback returns a non-array value, get_country_code() will throw a type error when accessed in PayPal and Square payment processing (lines 415, 478, and 195 respectively).
[failure] 276-276:
Method FrmAddressesController::get_country_code() should return string but returns mixed.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@classes/controllers/FrmAddressesController.php` at line 276, The
`get_country_code()` method accesses `self::$country_codes` directly as an array
without validating that the filtered result from the `frm_country_codes` filter
is actually an array. If a callback returns a non-array value, this causes type
errors when the method is invoked in payment processing paths. Add validation
where the filter result is assigned to `self::$country_codes` to ensure it
returns an array, and provide a default empty array as a fallback if the filter
returns a non-array value or null. This will prevent type errors when
`get_country_code()` is called during PayPal and Square payment processing.
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Align sub-field key contract (name vs key) across combo helpers.
These helpers read $atts['key'], but downstream callers pass name, so placeholder/default/error lookup can silently fail and produce undefined-index notices.
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@classes/controllers/FrmComboFieldsController.php` around lines 112 - 116, The
FrmComboFieldsController is using $atts['key'] to look up sub-field identifiers
in methods like include_placeholder and when accessing the default_value array,
but downstream callers are passing the sub-field identifier under the 'name' key
instead, causing silent lookup failures and undefined index notices. Replace all
references to $atts['key'] with $atts['name'] in the combo field helper methods
to align the sub-field key contract, specifically in the include_placeholder
function call and the default_value array access around lines 112-116, and apply
the same fix to the corresponding code at lines 179-180.
‼️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.
[warning] 164-164: Avoid side effects in a file that defines symbols
Context: echo '
' . wp_kses_post( $field[ $option_name ] ) . '
';
Note: [CWE-710] Improper Adherence to Coding Standards.
(no-side-effect)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@classes/controllers/FrmComboFieldsController.php` around lines 164 - 165, The
code directly accesses `$field[ $option_name ]` without first verifying that the
key exists in the array, which causes PHP notices when the key is absent. Guard
the array access by using isset() to check if the key exists before attempting
to access it. Modify the condition to first verify that the key is present in
the $field array, and only then check if its value is not empty. This will
prevent undefined array key notices from being rendered.
The reason will be displayed to describe this comment to others. Learn more.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Harden CSV import parsing for 5-part addresses.
Using is_numeric( $last_item ) to distinguish zip vs country misparses valid alphanumeric postal codes (e.g., CA/UK), shifting line2/city/state/zip into wrong keys.
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@classes/models/fields/FrmFieldAddress.php` around lines 333 - 339, The
current CSV import parsing for addresses uses is_numeric() to distinguish zip
codes from country names, which fails for alphanumeric postal codes like
Canadian (M5V 3A8) or UK (SW1A 1AA) formats. Replace the is_numeric() check with
a more robust validation method that can handle alphanumeric postal code
formats. This may involve using regex patterns to match common postal code
formats or implementing a smarter detection mechanism that doesn't rely purely
on numeric validation. Make sure to apply this fix consistently across all
affected locations where address field parsing occurs, including the code
sections around lines 333-339 and the additional locations mentioned in the
review (354-370).
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Pass key to combo-field helper contracts, not name.
Line 47, Line 56, Line 58, and Line 82 pass name, but these helpers consume $atts['key']. That breaks per-subfield placeholder/default/error-class behavior and can emit undefined-index notices.
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@classes/views/frm-fields/front-end/address-field/address-field.php` around
lines 47 - 58, The helper functions
FrmComboFieldsController::maybe_add_error_class,
FrmComboFieldsController::add_atts_to_input, and
FrmComboFieldsController::get_dropdown_label expect a key parameter in the $atts
array, but the code is passing name instead. Update the compact() calls on lines
47, 56, and 58 (and line 82 mentioned in "Also applies to") to pass key instead
of name to these helper methods, ensuring the parameter name matches what the
combo-field helper contracts expect and preventing undefined index notices.
Consider using FrmAppHelper::selected() for consistent selected-state handling.
The current strict comparison (===) may fail when $default_value and $opt_value have mismatched types (e.g., integer vs. string). The codebase provides FrmAppHelper::selected() (see FrmAppHelper.php:2185-2191) which handles selected state consistently across Lite and Pro by delegating to FrmProAppHelper::selected() when available, or falling back to WordPress's selected() otherwise.
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@classes/views/frm-fields/back-end/combo-field/sub-field-options.php` at line
58, The current implementation uses a strict comparison operator (===) to
determine the selected state in the ternary operator, which can fail due to type
mismatches between $default_value and $opt_value. Replace the ternary operator
logic that checks for selected state with a call to FrmAppHelper::selected(),
passing the $default_value[$sub_field['name']] and $opt_value as arguments. This
helper method handles type-safe comparison internally and provides consistent
behavior across Lite and Pro versions by delegating to
FrmProAppHelper::selected() when available or falling back to WordPress's
selected() function otherwise.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/phpunit/fields/test_FrmFieldAddress.php`:
- Line 286: The PHPDoc return type annotation on line 286 has invalid syntax
with `void` (which cannot be used for data providers) and an extra closing
bracket. Remove the `void` keyword and correct the bracket matching so the
return type reads as a proper generic array type like `@return
array<(array<string> | string)>`, or alternatively remove the return type tag
entirely if the return type is clear from context.
- Around line 220-225: The assertArrayHasKey assertions in the test are checking
for CSV column keys with an incorrect format. The controller implementation
generates keys using the pattern field_id concatenated with underscore and the
column name (e.g., '10_line1'), but the test assertions are expecting keys with
a 'field_' prefix and hyphen separators (e.g., 'field_10-line1'). Update all six
assertArrayHasKey calls to use the correct key format by replacing 'field_10-'
with '10_' so the keys match the actual output from the controller, ensuring the
test properly validates the CSV column headers.
---
Nitpick comments:
In `@classes/views/frm-fields/back-end/combo-field/sub-field-options.php`:
- Line 58: The current implementation uses a strict comparison operator (===) to
determine the selected state in the ternary operator, which can fail due to type
mismatches between $default_value and $opt_value. Replace the ternary operator
logic that checks for selected state with a call to FrmAppHelper::selected(),
passing the $default_value[$sub_field['name']] and $opt_value as arguments. This
helper method handles type-safe comparison internally and provides consistent
behavior across Lite and Pro versions by delegating to
FrmProAppHelper::selected() when available or falling back to WordPress's
selected() function otherwise.
🪄 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: 69d0c870-ffe5-437f-9ce4-b6ccf1b9d987
📥 Commits
Reviewing files that changed from the base of the PR and between 3937b09 and 67a23bd.
The return type @return void array<(array<string> | string)>> is invalid—data providers cannot return void, and the syntax has an extra closing bracket.
Or simply remove the tag if the return type is clear from context.
📝 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.
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/phpunit/fields/test_FrmFieldAddress.php` at line 286, The PHPDoc return
type annotation on line 286 has invalid syntax with `void` (which cannot be used
for data providers) and an extra closing bracket. Remove the `void` keyword and
correct the bracket matching so the return type reads as a proper generic array
type like `@return array<(array<string> | string)>`, or alternatively remove the
return type tag entirely if the return type is clear from context.
Keep the US-address expectations aligned with the model contract.
Line 72 and the us provider case expect country to be absent, but classes/models/fields/FrmFieldAddress.php:get_processed_sub_fields() only removes country for generic; for us it keeps country and only turns state into a select. These assertions will drift the controller/model contract and fail if both layers stay aligned.
Suggested fix
- $this->assertArrayNotHasKey( 'country', $result, 'Result should not have country for US address.' );+ $this->assertArrayHasKey( 'country', $result, 'Result should have country for US address.' );
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/phpunit/fields/test_FrmFieldAddress.php` around lines 57 - 72, Update
the US-address test expectations to match the
`FrmFieldAddress::get_processed_sub_fields()` contract: for the `us` provider,
`country` should still be present and only `state` should become a select.
Adjust the assertions in `test_get_sub_fields_us_returns_correct_fields()` so
they align with `FrmAddressesController::get_sub_fields()` and the model
behavior, and only assert `country` is removed for the `generic` case.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/phpunit/fields/test_FrmFieldAddress.php`:
- Around line 214-217: The test for FrmFieldAddress::get_export_headings is
asserting an aggregate field_10 export heading that the inherited
FrmFieldCombo::get_export_headings implementation does not produce. Remove the
main field column assertion from test_FrmFieldAddress and keep the checks
focused on the per-subfield keys returned by FrmFieldAddress, using the existing
$address_field and get_export_headings() call as the locator.
---
Outside diff comments:
In `@tests/phpunit/fields/test_FrmFieldAddress.php`:
- Around line 57-72: Update the US-address test expectations to match the
`FrmFieldAddress::get_processed_sub_fields()` contract: for the `us` provider,
`country` should still be present and only `state` should become a select.
Adjust the assertions in `test_get_sub_fields_us_returns_correct_fields()` so
they align with `FrmAddressesController::get_sub_fields()` and the model
behavior, and only assert `country` is removed for the `generic` case.
🪄 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: 1efa16e5-ce04-465d-aadb-001777c7d802
📥 Commits
Reviewing files that changed from the base of the PR and between 67a23bd and d69ed83.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit