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

CustomFields - Improve metadata about which custom groups belong to which entities #23336

Merged
merged 3 commits into from
May 5, 2022

Conversation

colemanw
Copy link
Member

@colemanw colemanw commented May 2, 2022

Overview

This makes the relationship between Custom Field Groups, entity types and subtypes discoverable via APIv4 metadata. This paves the way for Afform and SearchKit to handle custom fields intelligently, and for extensions to easily provide new types of entities with complex custom data mapping.

Before

Metadata did not exist. Custom field groups to load for each entity type was hard-coded on forms and templates throughout CiviCRM. Generic handling was not possible.

After

Metadata-driven tools like Afform and SearchKit will be able to gather custom fields for entity types and sub-types via the API.
Virtual entities with sub-types (like ECK) can easily extend the mapping.
Unit tests added.

Technical Details

In addition to adding metadata, this PR resolves a few oddities:

  • Custom field support for Grant entities was still hard-coded in core even though CiviGrant has been moved to an extension. I switched it to use the cg_extend_objects optionGroup and removed the Grant hard-coding from core.
  • There was general confusion about whether sub-type pseudo-values (e.g. ParticipantEventName, ParticipantStatus) were allowed in the CustomGroup.extends column or if they were only for presentation on the custom group quickform. I investigated and it turns out to be the latter. APIv3 CustomGroup.create has some special logic to transform e.g. extends: 'ParticipantEventName' into extends: 'Participant' with the appropriate value for extends_entity_column_id. I kept that around but removed unreachable code for handling those pseudo-values being stored in the extends column.
  • There was a very odd use of the OptionValue.description column by CiviCase, where it was actually storing the name of a callable function to return the list of case types. That's problematic for several reasons, not the least of which is that description is a localized field so the whole thing would break in multilingual mode! I switched over to a "grouping" value of case_type_id and taught the code to just call getFields on that field name.
  • APIv3 was having trouble handling custom data for entities with odd names (like PCP) or recently-renamed BAOs (like DedupeRule). I improved the APIv3 entity-to-name mapping to solve this.

@civibot
Copy link

civibot bot commented May 2, 2022

(Standard links)

@civibot civibot bot added the master label May 2, 2022
@colemanw colemanw force-pushed the customGroupExtends branch from 2c6b254 to b40d860 Compare May 2, 2022 19:06
@colemanw colemanw marked this pull request as draft May 2, 2022 19:18
@colemanw colemanw force-pushed the customGroupExtends branch from b40d860 to 54dbcc1 Compare May 3, 2022 17:20
@colemanw colemanw changed the title Custom group extends CustomFields - Improve metadata about which custom groups belong to which entities May 3, 2022
@colemanw colemanw force-pushed the customGroupExtends branch from 54dbcc1 to 4fe046b Compare May 4, 2022 03:14
@colemanw colemanw marked this pull request as ready for review May 4, 2022 14:54
@colemanw colemanw force-pushed the customGroupExtends branch 3 times, most recently from 5263a86 to 2acbf1b Compare May 4, 2022 15:27
colemanw added a commit to colemanw/de.systopia.eck that referenced this pull request May 4, 2022
See civicrm/civicrm-core#23336
which makes the callable function in 'description' obsolete.
@colemanw colemanw force-pushed the customGroupExtends branch 3 times, most recently from 781dd13 to d6f6dec Compare May 4, 2022 19:12
@colemanw
Copy link
Member Author

colemanw commented May 4, 2022

PR review note: in addition to new tests added in this PR, this functionality is massively covered by existing tests, notably the APIv3 syntaxConformanceTest which checks every entity for custom fields. Lots of other APIv4 and APIv3 tests cover custom data as well.

@colemanw colemanw force-pushed the customGroupExtends branch 2 times, most recently from 04bd6ad to 20c8b8d Compare May 4, 2022 21:57
colemanw added 3 commits May 4, 2022 18:13
… types

Fixes dev/core#2905
Before: Hard coded, plus a very strange use of the 'description' field to store a callback function.

After: The 'grouping' field in the OptionValue for custom extends and custom type is used.
APIv4 getfields can then retrieve the necessary options.
This allows the API to filter custom groups by entity type and other values.
@colemanw colemanw force-pushed the customGroupExtends branch from 20c8b8d to dab64f2 Compare May 4, 2022 22:13
@eileenmcnaughton
Copy link
Contributor

I gave this an r-run by creating custom fields for some of the more obscure types - grant, survey, participant-event and was able to create the fields & edit and save the values.

I'm OK with merging this but I just want to be sure that some thought has gone into the cleanup on the Grant mgd option value - since we seem to have had no-end of issues around grant mgd option values - I guess unlike the others this one is all new?

@eileenmcnaughton
Copy link
Contributor

@colemanw I'm going to merge this since I don't think my question above is blocking & I know you have targetted the upgrade scripts at 5.49 - if necessary we can fix as a follow up. @demeritcowboy might have thoughts on it too

@eileenmcnaughton eileenmcnaughton merged commit 79667be into civicrm:master May 5, 2022
@eileenmcnaughton eileenmcnaughton deleted the customGroupExtends branch May 5, 2022 02:31
@colemanw
Copy link
Member Author

colemanw commented May 5, 2022

Thanks @eileenmcnaughton. I believe we've got all the kinks worked out for those managed entities at this point. Should be smooth on upgrades and new installs alike, as in both cases this will be inserting a new option value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants