dev/core#244 Allow use of custom fields of type select without specifying an optiongroup #12440
Conversation
This file contains 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
|
(Standard links)
|
| @@ -419,9 +419,6 @@ public function getOptions($context = NULL) { | |||
| elseif ($this->data_type === 'Boolean') { | |||
| $options = $context == 'validate' ? array(0, 1) : CRM_Core_SelectValues::boolean(); | |||
| } | |||
| else { | |||
| return FALSE; | |||
| } | |||
| CRM_Utils_Hook::customFieldOptions($this->id, $options, FALSE); | |||
| CRM_Utils_Hook::fieldOptions($this->getEntity(), "custom_{$this->id}", $options, array('context' => $context)); | |||
| return $options; | |||
There was a problem hiding this comment.
don't you need to define $options = FALSE at the start to avoid e-notices?
There was a problem hiding this comment.
@eileenmcnaughton Yes I do, and now I have :-)
…n option group (so they can be populated dynamically using hook_civicrm_fieldOptions
mattwire
force-pushed
the
customfield_selectnooptiongroup
branch
from
ec15a25 to
6c5d3fc
Compare
|
Hmm, I think there's more than just 1 place where this assumption is made. I'm going to work up an api4 patch to better guess whether a field may have associated options, bearing this in mind. |
colemanw
added
the
merge ready
|
I think this is right |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Overview
For an extension I had a requirement to add a "Select Month" custom field. The options are populated dynamically using hook_civicrm_fieldOptions and there is no need for an option group to be associated with them (manually specifying the months of the year in an optiongroup means they won't be updated to match the current locale).
Additionally, if populated dynamically using the hook_civicrm_fieldOptions and there is an optiongroup associated with the field, the "Edit options" element is automatically added to the UI which is not desirable and does not work properly as the optiongroup is not populated.
There is a single line in CRM_Core_BAO_CustomField::getOptions which causes the function to return before calling hooks if the element is a Select and doesn't have an option group defined which makes it impossible to populate the field using hooks. Removing this allows the hook to populate the custom field values and everything works.
For an example see https://github.com/mattwire/uk.co.mjwconsult.variablerecurpayments (Edit a Membership Type and select the Pro-Rata Start Month).
#12439 is also required.
Before
Could not dynamically populate a custom field of type select using hooks without specifying an optiongroup as well.
After
Can dynamically populate a custom field of type select using hooks without specifying an optiongroup (create the custom field via XML or API).
Technical Details
The code makes an assumption in one place that a select element will have an optiongroup. But this unnecessarily restricts the ability to dynamically populate the fields.
https://lab.civicrm.org/dev/core/issues/244