[REF] Centralize logic for resolving settings pseudoconstants #14264
Conversation
|
(Standard links)
|
|
@colemanw jenkins is not sold |
| if (!empty($filters['name'])) { | ||
| $settingSpec = array_intersect_key($settingSpec, array_flip((array) $filters['name'])); | ||
| // FIXME: This is a workaround for settingsBag::setDb() called by unit tests with settings names that don't exist | ||
| $settingSpec += array_fill_keys((array) $filters['name'], []); |
There was a problem hiding this comment.
@eileenmcnaughton I had to add this line to mimic the old behavior.
Settings code is kind of a mess here. Settings::set() calls a validate function that doesn't actually work, and unit tests are counting on that so they can test make-believe settings :/
There was a problem hiding this comment.
FYI this is why the validate function doesn't work: If you pass in ['name' => 'foo'] as the $filters to this function then it will spit back out an array with the key foo even if there is no such setting!. IMO that's wrong, but out of scope for this PR so I left the errant behavior as-is.
Civi/Core/SettingsMetadata.php
Outdated
| * | ||
| * @param array $settingSpec | ||
| */ | ||
| protected static function _loadOptions(&$settingSpec) { |
There was a problem hiding this comment.
so on a style thing - my understanding is we've been moving away from the leading underscore on protected functions (and in fact it's inconsistent within the class)
There was a problem hiding this comment.
Agreed. I've updated the style thing.
|
Looks OK in quick testing - I'll kick the tyres a little more by doing a little further setting form cleanup |
Overview
Moves the pseudoconstant lookup for settings out of api3 and into the SettingsMetadata class.
Before
Code in an api3 silo.
After
Same code, better spot.
Notes
Also switches the settings form trait to use SettingsMetadata class instead of api3, and improves the filters on
getSettingsMetaData.