Skip to content
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

Add pseudoconstant to UFField dao #14041

Merged
merged 1 commit into from
Apr 13, 2019

Conversation

colemanw
Copy link
Member

@colemanw colemanw commented Apr 12, 2019

Overview

When adding a UFField to a profile, the fieldname passes through a validation function which checks all available fields. I'm using that same function as a pseudoconstant provider so the api can expose the option list and provide the same validation.

Before

Field appears as a textfield in api explorer.

After

image

Notes

Also fixes a fixme by using Civi::$statics instead of a local static var.

@civibot
Copy link

civibot bot commented Apr 12, 2019

(Standard links)

@civibot civibot bot added the master label Apr 12, 2019
@eileenmcnaughton
Copy link
Contributor

@totten @colemanw @seamuslee001 I just want to take a minute to think about this.

We have a lot of precedent for linking id fields via pseudoconstants - generally to name fields on other tables.

This is a new pattern in that it links a text field to a title field coming from a function. I suspect there could be field titles that are the same for different fields - although there is a reasonable case to say that should just be fixed (perhaps easier in theory than practice I'm not sure)

I can't think of a problem per se but I know @colemanw elsewhere is trying to look at pseudoconstant nuance & I just wanted to check if we are starting a new protocol here & if so if there is any reason to hesitate

To articulate...
CiviCRM option lists are traditionally flat arrays of [key => value, key => value].
However, some option lists are more complicated than that.
Some of our lists might contain id, name, label, icon, color, and description.
So I want to evolve our pseudoconstant getter to return a richer format. And standardize it so it's predictable.
But one of the irksome things about our pseudoconstants is that some of them have not 1 but 2 unique keys (id and name).
Arguably, this is just plain wrong and should be done away with.
So while civicrm_participant.campaign_id is an FK to civicrm_campaign.id it should arguably be changed to FK to civicrm_campaign.name.
Which would make database migrations easier for one thing.
And also clear up the mess of some pseudoconstants having 2 unique identifiers.

@colemanw
Copy link
Member Author

colemanw commented Apr 12, 2019

Hi @eileenmcnaughton. No I don't think this is starting a new standard or deviating from what we already do. There are many places where we use a callback to list options that are not an FK to another field:

grep -R "<callback>" xml/schema/
xml/schema/Contribute/Product.xml:      <callback>CRM_Core_SelectValues::periodType</callback>
xml/schema/Contribute/Product.xml:      <callback>CRM_Core_SelectValues::getPremiumUnits</callback>
xml/schema/Contribute/Product.xml:      <callback>CRM_Core_SelectValues::getPremiumUnits</callback>
xml/schema/Dedupe/RuleGroup.xml:      <callback>CRM_Core_SelectValues::getDedupeRuleTypes</callback>
xml/schema/Contact/ACLContactCache.xml:      <callback>CRM_ACL_BAO_ACL::operation</callback>
xml/schema/Contact/Relationship.xml:      <callback>CRM_Core_SelectValues::getPermissionedRelationshipOptions</callback>
xml/schema/Contact/Relationship.xml:      <callback>CRM_Core_SelectValues::getPermissionedRelationshipOptions</callback>
xml/schema/Contact/Group.xml:      <callback>CRM_Core_SelectValues::groupVisibility</callback>
xml/schema/Contact/Group.xml:      <callback>CRM_Core_PseudoConstant::allGroup</callback>
xml/schema/Contact/SubscriptionHistory.xml:      <callback>CRM_Core_SelectValues::getSubscriptionHistoryMethods</callback>
xml/schema/Contact/SubscriptionHistory.xml:      <callback>CRM_Core_SelectValues::groupContactStatus</callback>
xml/schema/Contact/Contact.xml:      <callback>CRM_Core_SelectValues::pmf</callback>
xml/schema/Contact/GroupContact.xml:      <callback>CRM_Core_SelectValues::groupContactStatus</callback>
xml/schema/Price/PriceField.xml:      <callback>CRM_Price_BAO_PriceField::htmlTypes</callback>
xml/schema/Campaign/CampaignGroup.xml:      <callback>CRM_Core_SelectValues::getCampaignGroupTypes</callback>
xml/schema/Financial/PaymentProcessorType.xml:      <callback>CRM_Core_SelectValues::billingMode</callback>
xml/schema/Event/ParticipantStatusType.xml:      <callback>CRM_Event_PseudoConstant::participantStatusClassOptions</callback>
xml/schema/ACL/ACL.xml:      <callback>CRM_ACL_BAO_ACL::operation</callback>
xml/schema/Member/MembershipType.xml:      <callback>CRM_Core_SelectValues::membershipTypeUnitList</callback>
xml/schema/Member/MembershipType.xml:      <callback>CRM_Core_SelectValues::periodType</callback>
xml/schema/Member/MembershipType.xml:      <callback>CRM_Core_SelectValues::memberVisibility</callback>
xml/schema/Member/MembershipType.xml:      <callback>CRM_Core_SelectValues::memberAutoRenew</callback>
xml/schema/Member/MembershipStatus.xml:      <callback>CRM_Core_SelectValues::eventDate</callback>
xml/schema/Member/MembershipStatus.xml:      <callback>CRM_Core_SelectValues::unitList</callback>
xml/schema/Member/MembershipStatus.xml:      <callback>CRM_Core_SelectValues::eventDate</callback>
xml/schema/Member/MembershipStatus.xml:      <callback>CRM_Core_SelectValues::unitList</callback>
xml/schema/Core/WordReplacement.xml:       <callback>CRM_Core_SelectValues::getWordReplacementMatchType</callback>
xml/schema/Core/ActionSchedule.xml:      <callback>CRM_Core_SelectValues::getRecurringFrequencyUnits</callback>
xml/schema/Core/ActionSchedule.xml:      <callback>CRM_Core_SelectValues::getRecurringFrequencyUnits</callback>
xml/schema/Core/ActionSchedule.xml:      <callback>CRM_Core_SelectValues::getRecurringFrequencyUnits</callback>
xml/schema/Core/OptionGroup.xml:      <callback>CRM_Utils_Type::dataTypes</callback>
xml/schema/Core/Managed.xml:      <callback>CRM_Core_ManagedEntities::getCleanupOptions</callback>
xml/schema/Core/Note.xml:      <callback>CRM_Core_BAO_Note::entityTables</callback>
xml/schema/Core/StatusPreference.xml:      <callback>CRM_Utils_Check::getSeverityList</callback>
xml/schema/Core/CustomGroup.xml:      <callback>CRM_Core_SelectValues::customGroupStyle</callback>
xml/schema/Core/CustomField.xml:      <callback>CRM_Core_BAO_CustomField::dataType</callback>
xml/schema/Core/CustomField.xml:      <callback>CRM_Core_SelectValues::customHtmlType</callback>
xml/schema/Core/UFField.xml:      <callback>CRM_Core_BAO_UFField::getAvailableFieldTitles</callback>
xml/schema/Core/UFField.xml:      <callback>CRM_Core_SelectValues::ufVisibility</callback>
xml/schema/Core/Extension.xml:      <callback>CRM_Core_SelectValues::getExtensionTypes</callback>
xml/schema/Core/UFJoin.xml:      <callback>CRM_Core_BAO_UFJoin::entityTables</callback>
xml/schema/Core/Job.xml:      <callback>CRM_Core_SelectValues::getJobFrequency</callback>
xml/schema/Core/Email.xml:      <callback>CRM_Core_PseudoConstant::emailOnHoldOptions</callback>
xml/schema/Core/MappingField.xml:      <callback>CRM_Core_SelectValues::getSearchBuilderOperators</callback>
xml/schema/Mailing/MailingAB.xml:      <callback>CRM_Mailing_PseudoConstant::abStatus</callback>
xml/schema/Mailing/MailingAB.xml:      <callback>CRM_Mailing_PseudoConstant::abTestCriteria</callback>
xml/schema/Mailing/MailingAB.xml:      <callback>CRM_Mailing_PseudoConstant::abWinnerCriteria</callback>
xml/schema/Mailing/Mailing.xml:      <callback>CRM_Mailing_PseudoConstant::mailingTypes</callback>
xml/schema/Mailing/Mailing.xml:      <callback>CRM_Mailing_BAO_Mailing::getTemplateTypeNames</callback>
xml/schema/Mailing/Mailing.xml:      <callback>CRM_Core_SelectValues::groupVisibility</callback>
xml/schema/Mailing/Mailing.xml:      <callback>CRM_Core_SelectValues::emailSelectMethods</callback>
xml/schema/Mailing/MailingGroup.xml:      <callback>CRM_Core_SelectValues::getMailingGroupTypes</callback>
xml/schema/Mailing/MailingGroup.xml:      <callback>CRM_Mailing_BAO_Mailing::mailingGroupEntityTables</callback>
xml/schema/Mailing/Component.xml:      <callback>CRM_Core_SelectValues::mailingComponents</callback>
xml/schema/Mailing/MailingJob.xml:      <callback>CRM_Core_SelectValues::getMailingJobStatus</callback>

Also to clarify, this function returns machine name keys and that's what's used to validate the value, not the title which may be translated.

And just to reiterate, this is the exact function used to validate the passed-in value. E.g. it already won't accept anything other than what's in this list.

@eileenmcnaughton
Copy link
Contributor

@colemanw ok

@eileenmcnaughton eileenmcnaughton merged commit 23f407d into civicrm:master Apr 13, 2019
@colemanw colemanw deleted the fieldTitles branch April 13, 2019 02:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants