Allow other base tables for api4-based smart groups #17003
Conversation
|
(Standard links)
|
| if (CRM_Core_DAO::singleValueQuery("SELECT COUNT(*) {$contactQuery['from']}") > 0) { | ||
| CRM_Core_DAO::executeQuery("INSERT IGNORE INTO $tempTable (group_id, contact_id) {$contactQuery['select']} {$contactQuery['from']}"); | ||
| } | ||
| CRM_Core_DAO::executeQuery("INSERT IGNORE INTO $tempTable (group_id, contact_id) {$contactQuery['select']} {$contactQuery['from']}"); |
There was a problem hiding this comment.
I removed the SELECT COUNT for 2 reasons:
- It was causing the API query to crash because swapping the
SELECTclause forSELECT COUNT(*)loses our field alias which theHAVINGclause relies on. - I'm unclear about the effectiveness of this optimization. It looks to me like it's causing the query to run twice, so I'm not sure how that's more efficient. Looks like fairly recent work of @seamuslee001 though so I'll let him chime in here and tell me why I'm wrong :)
There was a problem hiding this comment.
@colemanw Can you pull this bit out into a separate PR? It was part of the refactoring to improve performance / reduce database deadlocks work so changing it carries quite a high risk (ie. needs testing thoroughly). That said, the refactoring was an improvement rather than an end-point so I'm happy to see further improvements like this :-)
For example, in theory we should not have needed the INSERT IGNORE - INSERT should have been ok, but in practise it wasn't.. (probably because further improvement is required to reduce concurrency).
colemanw
force-pushed
the
smartererGroups
branch
from
2b83dfa to
d07cceb
Compare
|
retest this please |
colemanw
force-pushed
the
smartererGroups
branch
2 times, most recently
from
9f74998 to
6e4e89d
Compare
|
The "additional cleanup" commit was causing tests to crash so I'm splitting that off to a separate PR to investigate. This PR should be merge-ready now. |
|
retest this please |
colemanw
force-pushed
the
smartererGroups
branch
from
6e4e89d to
4810225
Compare
Allow other base tables for api4-based smart groups
Overview
Expands support for API based smart groups, allowing any api entity as the starting point, not just
Contact.Before
Could only create a smart group from
ContactAPI entity.After
Any API entity allowed. The API explorer supports this, and also now provides some rudimentary validation to ensure contact_id is selected by the API query.
Technical Details
The
GroupContactCachewas tacking on an additionalWHEREclause to exclude removed group contacts. I converted it to aHAVINGclause for API queries because that's more flexible.