-
-
Notifications
You must be signed in to change notification settings - Fork 825
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
APIv4 - Allow field options to be returned in multiple formats #17167
Conversation
(Standard links)
|
This sounds sweet. :) |
I haven't looks at the code but I like the pr description! |
OK - so code is constrained to apiv4 - it feels like a contract change - what will the impact be? |
@eileenmcnaughton it's backward-compatible. Passing in the same api params will still get you the same result, so no impact on current code. But it adds support for a new way of passing in the param that will get you the multi-dimensional result. |
nice |
10eb5da
to
99997b6
Compare
Ok I think this is good now; it does everything I envisioned and has exposure in the Explorer + sufficient test coverage. |
@colemanw sorry we broke it. Happy to MOP it & you can merge once tests pass on the rebased version |
unrelated fails |
This addresses the e-notice we have been seeing when running civibuild.create [PHP Notice] Undefined index: id at /src/wikimedia/fundraising/crm/drupal/sites/all/modules/civicrm/Civi/Api4/Action/CustomField/CustomFieldSaveTrait.php:53 and instead correctly creates the labels for our custom groups. There are 2 ways to declare the option values for a custom field - as a value=>label array, as a value => array of values array We mostly do the first but donor segment & status are declared as arrays (& probably we will move that way with the others). However the code upstream now (since 2020) expects 'id' to be populated with 'value' - so this does that. It also removes handling for the first sort of array as core does that fine now. The code has evolved a bit over time in core - in particular with civicrm/civicrm-core#17167 Note once a civibuild has been done both the new style & the old style have values - ie donor segment & gift source https://wmff.localhost:32353/civicrm/admin/custom/group/field/option?reset=1&action=browse&gid=8&fid=22 https://wmff.localhost:32353/civicrm/admin/custom/group/field/option?reset=1&action=browse&gid=11&fid=164 Bug: T342943 Change-Id: Id637fd75a42faff4b6ee525422ef2965984b37cb
Overview
Gives more flexibility about how options are returned from APIv4.
Added visibility in the API explorer.
Before
In
GetFields
,loadOptions
is a boolean param, and the option list returned is single-dimensional.After
In
GetFields
,loadOptions
can be either a boolean or an array. If an array, the option list returned will contain the keys requested.Technical Stuff 👀
Field pseudoconstants are stored in many different places (
option_values
table, other tables, callbacks, etc.) but that all gets normalized when they pass throughDAO::buildOptions()
which returns a one-dimensional list. Well and good, but some option lists contain other info (icon, color, description...), and that flat list has been a source of growing pains.The end goal behind this PR is to standardize what a multi-dimensional pseudoconstant list looks like. The idea is that regardless of how the options are stored and what the columns in that table look like, when reading/writing an option list (via the api anyway) the array keys will look like this:
civicrm_option_values
table this maps tovalue
column, in other tables it usually maps toid
)#aabbcc
crm-i fa-rocket
🚀