[REF] Switch OptionGroup BAO to use new centralized logic to make name from title #22654
Conversation
|
(Standard links)
|
colemanw
force-pushed
the
optionGroupName
branch
from
ae45a43 to
e3b5fc0
Compare
… title Switches APIv4 to use writeRecords for the OptionGroup entity; keeps v3 the same for backward-compatability.
colemanw
force-pushed
the
optionGroupName
branch
from
2a393cb to
ddba05d
Compare
| @@ -423,6 +423,7 @@ public static function toBeSkipped_custom_data_creatable($sequential = FALSE) { | |||
| 'Profile', | |||
| 'Payment', | |||
| 'Order', | |||
| 'OptionGroup', | |||
There was a problem hiding this comment.
This PR triggered a bizarre test failure, why it ever worked before I don't know, but no, attaching custom data to the OptionGroup entity makes no sense.
| $optionGroup->save(); | ||
| $customGroupTitle = CRM_Core_DAO::getFieldValue('CRM_Core_DAO_CustomGroup', $params['custom_group_id'], 'title'); | ||
| $optionGroup = CRM_Core_DAO_OptionGroup::writeRecord([ | ||
| 'title' => "$customGroupTitle :: {$params['label']}", |
There was a problem hiding this comment.
So this is a material change it seems @colemanw what prompted this change?
There was a problem hiding this comment.
So in order to take advantage of the auto-name-generation, I needed to switch from direct DAO insert to using the WriteRecord function. The name gets derived from the title, which previously was just the label of the custom field. I wanted it to be more descriptive. It doesn't matter too much because neither name nor title are ever shown in the UI anywhere for auto-generated custom groups, but for techie looking at the sql table and trying to make sense of what's in it, I thought this more precise label would be nice.
There was a problem hiding this comment.
@colemanw are they shown at the list of option groups i.e. at /admin/options?reset=1 also as part of a drop down when creating a new custom field right?
There was a problem hiding this comment.
Yes you kind of have to go looking for it, but it will show in the UI in those places. Again, because you have to go looking, I think the name of the custom group + the name of the custom field is an improvement over just the field name.
Overview
Refactors OptionGroup code to use centralized logic from #22570
Before
Some artistically creative code in OptionGroup BAO to make a name from the title.
After
OptionGroup BAO
addfunction deprecated, which triggers APIv4 to usewriteRecords()with its centralized logic for creating name from title.APIv3 kept the same to minimize impact on existing code.
Notes
I stumbled into this because I was writing a unit test which created two custom field groups with the same named fields in both groups. The current "artistic" approach to creating a unique name was to append a timestamp to it, but in a unit test the fields are created at the same time, so this didn't work.
I decided to do this refactor to move things in a more standardized direction; it required extending the
CRM_Core_DAO::makeNameFromLabelfunction to handle titles as well as labels, and to cope better with the maxlength of thenamefield.