Harden radio save_other_choice lookup for JSON fields#382
Conversation
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
cbravobernal
left a comment
There was a problem hiding this comment.
Verified locally:
- The two new tests reproduce the reported warning against current trunk (
Undefined array key "ID"inclass-acf-field-radio.php:347) and pass with this fix. - Full PHPUnit suite (2230 tests), PHPStan, and phpcs all green on this branch.
The defensive selector build (ID → key → bail) and the acf_get_field() return check both look right, and behavior for valid persisted fields is unchanged.
Verification run via Claude Code, reviewed by @cbravobernal.
60fac64 to
7e8ff26
Compare
tapheret2
left a comment
There was a problem hiding this comment.
Review: Harden radio save_other_choice lookup for JSON fields
Files: includes/fields/class-acf-field-radio.php, tests/php/includes/fields/test-class-acf-field-radio.php
Size: +50 / -9
Notes
- Focused diff from a static pass; no obvious red flags, but I only reviewed the patch text.
Nits
- Any user-facing change that should be mentioned in the PR description?
- Need a changelog/release note for the next tag?
Thanks @meravi — independent review pass on the diff.
Summary
This hardens
acf_field_radio::update_value()whensave_other_choiceis enabled and the runtime field is JSON-only or missing identifiers.Changes
ID->key-> bail early).acf_get_field()fails or returns a field without DBID.choicesto an array before appending custom values.Tests
test_update_value_save_other_choice_handles_json_field_without_idtest_update_value_save_other_choice_handles_missing_field_identifierFixes #381