APIv4-based smart groups

[colemanw]

Overview

Allows smart groups to be created with APIv4 params instead of search form values. Adds a button to save them from the api explorer.

Before

Smart groups only work with search form values.

After

• SavedSearch entity expanded to hold api params.
• Smart groups updated to work with api savedSearch.
• Button in API explorer:

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[eileenmcnaughton]

@colemanw style warning

[eileenmcnaughton]

@colemanw one issue we have with existing smart groups is that they can become a security hole in some cases ie

1. create a smart group based on custom field -is_superhero=1

2. assign an acl to that group

3. delete the custom field

• we wind up with everyone being able o access the ACL.

I feel this has mostly been seen with custom groups but probably could be seen with an entity tag or similar & I think we should have some (tested) way to identify what entities are required for a saved search to be valid

@seamuslee001

[colemanw]

In that scenario the APIv4 query would bomb with a missing sql column. But if we were to fix that issue then yes we'd wind up with the potential security implication.

[eileenmcnaughton]

@colemanw which is good & we should lock that in with a test. It doesn't have to be in scope for this PR but we should be thinking about how to identify when a search is invalidated by a DB change

[eileenmcnaughton]

@colemanw I have a feeling determining required entities will be easier when creating a saved search than when loading it - what about a new key for required entities - so it might look like

$requiredEntities = [
  ['FinancialType' => ['name' => 'Event fee']]
];

@totten

[colemanw]

I'm not sure that's necessary and would worry about that information getting out of sync with the API params. If we can parse it from the params once then we can do it again.

[seamuslee001]

@eileenmcnaughton @colemanw My thinking is that if all the where entities are no longer present in the database for whatever reason, rather than trying to run the SQL we should throw and Exception or something so that it can bubble up somewhere sensible. The other part of things would be handy to have is that we have no way when say a user goes to delete a custom field or a group or a tag for instance, saying Hey this thing is required as part of smart groups x y z a b c (or maybe saved search rather than smart group) Where as if you look at price sets if a price set is linked to a related entity (contribution, contribution page, event) then it will stop you from deleting it which feels like a better user flow. I tend to feel an Exception is much nicer error than say a DB error

[colemanw]

retest this please

[eileenmcnaughton]

@colemanw my position on the depends-on-entities is that it's not blocking but we should continue to think about it & keep it at the top of mind. But, can we add a test to ensure that the use of a deleted-custom-group in the where clause throws an exception to protect against someone fixing it

[eileenmcnaughton]

I think this is OK to merge as is - there are some outstanding things

1. the dependency discussion per above
2. docs

However, as a first step in this area I feel this is solid and we can still evolve it more over the next few months.

This will have no impact on anyone who doesn't actively use this new function

[colemanw]

@eileenmcnaughton take note of the commit message in 47cef48 - that's not the first time I've had to fix a crash caused by the use of api.get in an upgrade fn.