AdminUI - dynamically generate afform tabs for CustomGroups of style Tab with table

[ufundo]

Overview

Following #31484 - this adds a SearchKit and tab for each CustomGroup where style = Tab with table

Before

Hard-coded Tab with table:

Error when adding a record (from @colemanw )

After

Generated afform tab:

Additional functionality:

• create form without weird error
• bulk actions
• tweakability through SearchKit / FormBuilder

e.g.

add a filter field:

Technical Details

For each multi-value Tab with table custom group, we generate:

• a SavedSearch managed record with active fields where Display in table is set
• a SearchDisplay managed record based on this SavedSearch, with the same columns
• a search-type Afform with the necessary meta to displace the existing custom group tab (*)

There's a bit of back-n-forth in terms of the interplay between afform_core and civicrm_admin_ui:

The SearchKit bits are added in civicrm_admin_ui only.

The code for Afform generation is added to CustomGroup.GetAfforms action, which is implemented in afform_core because it includes the Custom Group blocks, which are already provided by afform_core. But generating the additional forms is gated based on having civicrm_admin_ui enabled. (These forms depend on the search kit bits from civicrm_admin_ui.)

(*) - this required small patch to the tabset hook in afform, because it uses a convention for displacing tabs based on matching the afform name to the existing tab ID. But I really wanted the new afforms to use the CustomGroup name, whereas the existing tabs are keyed by the custom group id.

The whole thing lends itself to more generalisability. I've tried to structure in a way that supports some of the next steps below.

Questions

Permissions:

• for the create/update forms, I'm hopeful RBAC on the entities is providing the guard we need
• here I've used 'access all custom data' but I'm not too sure if that is right?

Deprecation:

• I've not replicated the Copy button on each row. Is it useful? (If so, I think it might be something to implement more generally in afform: some variant path to Prefill where you are prefilling with the copyable fields from another entity)

Next steps

• EntityRef fields will display as IDs for now, would be good to pick a better column
• I think it should be fairly trivial to add a "grid" style tab where style = Tab.
• it shouldn't be much of a leap to e.g. add tabs for Event CustomGroups to the event tabset
• I'd like to add a general listing/directory page for any multi-record CustomGroup, no matter what entity it extends

[civibot]

🤖 Thank you for contributing to CiviCRM! ❤️ We will need to test and review this PR. 👷

Introduction for new contributors...

• If this is your first PR, an admin will greenlight automated testing with the command ok to test or add to whitelist.
• A series of tests will automatically run. You can see the results at the bottom of this page (if there are any problems, it will include a link to see what went wrong).
• A demo site will be built where anyone can try out a version of CiviCRM that includes your changes.
• If this process needs to be repeated, an admin will issue the command test this please to rerun tests and build a new demo site.
• Before this PR can be merged, it needs to be reviewed. Please keep in mind that reviewers are volunteers, and their response time can vary from a few hours to a few weeks depending on their availability and their knowledge of this particular part of CiviCRM.
• A great way to speed up this process is to "trade reviews" with someone - find an open PR that you feel able to review, and leave a comment like "I'm reviewing this now, could you please review mine?" (include a link to yours). You don't have to wait for a response to get started (and you don't have to stop at one!) the more you review, the faster this process goes for everyone 😄
• To ensure that you are credited properly in the final release notes, please add yourself to contributor-key.yml
• For more information about contributing, see CONTRIBUTING.md.

Quick links for reviewers...

• ➡️ PR Demo site (can be rebuilt with the command test this please).
• 📖 Review standards
• 🗒️ Review template (brief or verbose)

➡️ Online demo of this PR 🔗

[ufundo]

Further question... group-level config settings - it's very evident during this that we are trying to interpret settings that were based on a different context, so I'm wondering of the scope to add/alter the config options available on CustomGroup / CustomField to be more suited to this new paradigm?

Immediate examples of things I'd consider adding:

• CustomGroup level option to enable/disable a directory listing page
• CustomGroup level option to specify permission requirement for the directory listing page
• CustomField level option to say this field is a "default filter" on listings/tabs etc

You may be thinking: "well you can just customise any of those things in afform" -- but I think there is a case for being able to specify the defaults as part of the CustomGroup / CustomField meta. Would make packaging that sort of stuff up into extensions much easier for one. (You provide managed records for CustomGroup / CustomField and all of the afforms will be derived from this, you don't have to package the CustomGroups and the Afforms, and keep those things in sync, and update your packaged afforms in order to pull through improvements in the default CustomGroup => Afform derivation.)

[colemanw]

@ufundo Q: What happens if you set a "Maximum number of multiple records" limit on the custom group? Does the afform respect that?

[ufundo]

What happens if you set a "Maximum number of multiple records" limit on the custom group? Does the afform respect that?

Good question. I haven't done anything for that, so I'm guessing:

• if it's restricted at BAO / Api4 layer I suspect it would just crash on submit
• otherwise it will be ignored

It's a bit tricky because the Add form won't know about the restriction until you hit submit with a given entity_id; but from a user perspective you don't want to go through the trouble of filling in the form, only to find you can't submit it.

Easy option: hide the button to the Add form in the tab?

Hard option: some kind of custom angular directive that immediately checks any value added to entity_id field on the form, and warns that you can't make more records for that entity?

[ufundo]

if it's restricted at BAO / Api4 layer I suspect it would just crash on submit

Looks like you can create records exceeding the configured limit using Api4. So it's prob just implemented in the QuickForm atm.

Easy option: hide the button to the Add form in the tab?

Good enough for now?

[ufundo]

FYI @lcdservices - work so far on making an Afform/Search Kit tab for "Tab with table" CustomGroups

You can see it working on the test site here:
http://core-31503-3isqy.test-1.civicrm.org:8003/civicrm/contact/view?reset=1&cid=127

[colemanw]

@ufundo Easy option: hide the button to the Add form in the tab?

I think the correct option is to implement the Api4 checkAccess action for CustomValue entities. That could be a very simple implementation that checks the current count of records with given entity_id and compares it with the max allowable in the custom group.
SearchKit already calls that api to check permissions before displaying buttons, so I think with that api implemented the searchKit display will "just work".

[ufundo]

I think the correct option is to implement the Api4 checkAccess action for CustomValue entities. That could be a very simple implementation that checks the current count of records with given entity_id and compares it with the max allowable in the custom group.

That sounds good!

SearchKit already calls that api to check permissions before displaying buttons, so I think with that api implemented the searchKit display will "just work".

I think the other piece would be making the create form url the canonical url for "Create" - at the moment it's just using a (relatively) hard coded URL. Some hook on getLinks?

[ufundo]

Hmm - even if we make our form the canonical "create" form - SearchDisplay run is going to have to know what entity_id it needs to create for 🤔

Well lets see

[colemanw]

@ufundo I would say just make the url to this form the canonical link in the api. Don't worry about a hook.
You can use [entity_id] as a token in the path.

[ufundo]

I've made a checkAccess check which seems to work for stopping Custom_Playing_Card.create when you hit max_multiple.

Adding the link is less successful: for some reason trying to add that add link to the SearchKit crashes it, with a strange complaint about getDaoEntity in Civi/API/Request.

(Other minor point: you get the option for Update form button in SearchKit, but you still have the Update searchaction taks button, so you get two indistguishable options that do different things)

[ufundo]

Adding the link is less successful: for some reason trying to add that add link to the SearchKit crashes it, with a strange complaint about getDaoEntity in Civi/API/Request.

Realised my paths were generating wrong. I don't know why it cause the error it did, but seems fixed now.

And the Add button does indeed show/hide interactively correctly 😁

[ufundo]

You can use [entity_id] as a token in the path.

So clever how it derives the toolbar token values from the where in the search! And then checks access based on those parameters too... 🤯

[colemanw]

So clever how it derives the toolbar token values from the where in the search! And then checks access based on those parameters too... 🤯

Thanks, I really wanted to put all the cleverness into SearchKit itself to enable no-code CRUD forms for most entities.

[colemanw]

@ufundo I'm not entirely sure I understand the benefit to wrapping those getter functions into API actions. I'm not opposed, just not quite sure why anybody would ever need to call them as APIs.

[ufundo]

I'm not entirely sure I understand the benefit to wrapping those getter functions into API actions. I'm not opposed, just not quite sure why anybody would ever need to call them as APIs.

I thought it was an efficient and consistent way to fetch all the required info for each CustomGroup in one go, and then process them sequentially. I didn't realise there was already a cache, so no need to fetch.

I still think it's structurally tidy to do it an "entity"-based way. And generally it's nice to use consistent APIs across all the code (I'm going to have to go look up how the filters param to CRM_Core_BAO_CustomGroup::getAll works, but I know how Api4 ->where behaves. I can tweak the where very easily to pull in other CustomGroups. I can very easily test specific groups using different filters / using the Api explorer.)

[ufundo]

Although in most use-cases swapping the CustomGroup::get api for a BasicGetAction that used the cache backend would be not noticeable, all of the SQL functionality would disappear, including joins, group-by, and field transformations, which would break more advanced uses of the api.

I was thinking it would involve some kind of check on whether the get params were suitably simple that the api could answer without a DB query. Certainly no explicit or implicit joins or group by. Field formatting and things like that are probably harder to triage.

But it feels like all the calls I've written in this PR are fairly clearly answerable based on the cache.

[ufundo]

I gave it a go #31508 and it does seem to work... though it opens another can of worms around case sensitivity. Happy to switch back to the regular cache for now if you'd prefer to avoid that can @colemanw ?

The can was useful to open for a minute at least :

• the difference in case sensitivity broke this call https://github.com/civicrm/civicrm-core/pull/31503/files#r1852493514
• this made me realise it was buggy anyway, it was a fluke that I was getting the right group name back after the snake case transformation
• Fixed that with ca5a32a

[ufundo]

Ok I've switched to using CRM_Core_BAO_CustomGroup for the in action calls for now.

[ufundo]

retest this please

[ufundo]

@colemanw I think this is ready to go? (using the pre-existing caching for now)

[colemanw]

Looks great! Thanks @ufundo