Search ext: support complex joins & HAVING clause in api4 smart groups

[colemanw]

Overview

Improves the new search extension and APIv4 smart groups in core to support any entity that can join with Contact, including full support for calculated fields and the HAVING clause.

Before

Limited smart group support.

After

Improved smart group support; with test.

[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).

[colemanw]

Failure looks unrelated. retest this please

[eileenmcnaughton]

@colemanw I tried to figure out how to test this - I thought I might have been able to create a smart group like this but I can't

civicrm/search#/create/LineItem/?join=%5B%5B"Contribution%20AS%20contribution",false%5D,%5B"PriceField%20AS%20price_field",false%5D%5D&select=%5B"financial_type_id:label","contribution.contact_id","qty","line_total"%5D&where=%5B%5B"line_total",">","2"%5D%5D

I also can't spot any new 'having' option civicrm/search#/create/LineItem/?join=%5B%5B"Contribution%20AS%20contribution",false%5D,%5B"PriceField%20AS%20price_field",false%5D%5D&select=%5B"GROUP_CONCAT(DISTINCT%20financial_type_id:label)","GROUP_CONCAT(contribution.contact_id)","SUM(qty)","SUM(line_total)"%5D&where=%5B%5B"line_total",">","2"%5D%5D&groupBy=%5B"price_field_value_id","financial_type_id"%5D

[colemanw]

@eileenmcnaughton your first screenshot seems to not have this PR applied. The "Missing contact_id" error message is different in this version, so likely you need to ensure the patch is applied and flush caches.

In the Search UI, the "HAVING" clause is named "Filter" and appears in the right column underneath the "Where" criteria. It is only visible when one or more grouping columns is selected... although once more sql functions are allowed it might be useful even when grouping isn't in use, so I'll probably lift that restriction.

[eileenmcnaughton]

Hmm

http://core-18644-91yzb.test-3.civicrm.org:8001/civicrm/search#/create/LineItem/?join=%5B%5B%22Contribution%20AS%20contribution%22,false%5D,%5B%22PriceField%20AS%20price_field%22,false%5D%5D&select=%5B%22financial_type_id:label%22,%22contribution.contact_id%22,%22qty%22,%22line_total%22%5D&where=%5B%5B%22line_total%22,%22%3E%22,%222%22%5D%5D

seems to work but the actual save

[eileenmcnaughton]

On the having - this should return rows but doesn't I believe

http://core-18644-91yzb.test-3.civicrm.org:8001/civicrm/search#/create/LineItem/?join=%5B%5B%22Contribution%20AS%20contribution%22,false%5D,%5B%22PriceField%20AS%20price_field%22,false%5D%5D&select=%5B%22GROUP_CONCAT(DISTINCT%20financial_type_id:label)%22,%22GROUP_CONCAT(contribution.contact_id)%22,%22SUM(qty)%22,%22SUM(line_total)%22%5D&where=%5B%5B%22line_total%22,%22%3E%22,%222%22%5D%5D&groupBy=%5B%22financial_type_id%22%5D&having=%5B%5B%22SUM(line_total)%22,%22%3E%3D%22,%223%22%5D%5D

[colemanw]

On the having - this should return rows but doesn't I believe

For me it does return results (for nicety I changed LIST(contact_id) to COUNT(contact_id) but it works either way):

[eileenmcnaughton]

This is what I'm trying - but no response

http://core-18644-91yzb.test-3.civicrm.org:8001/civicrm/search#/create/LineItem/?join=%5B%5B%22Contribution%20AS%20contribution%22,false%5D,%5B%22PriceField%20AS%20price_field%22,false%5D%5D&select=%5B%22GROUP_CONCAT(DISTINCT%20financial_type_id:label)%22,%22COUNT(contribution.contact_id)%22,%22SUM(qty)%22,%22SUM(line_total)%22%5D&where=%5B%5B%22line_total%22,%22%3E%22,%222%22%5D%5D&groupBy=%5B%22financial_type_id%22%5D&having=%5B%5B%22SUM(line_total)%22,%22%3E%3D%22,%223%22%5D%5D

[colemanw]

@eileenmcnaughton I clicked on that link and without tweaking any of the criteria I clicked "Search" and it gave me 3 results:

[colemanw]

@eileenmcnaughton I've updated this PR. I think everything is now working as it should.
HOWEVER the exact scenario you were testing above does not work, and that's because contact_id is an aggregated column. We still don't have a way to handle that, but I don't think that should block this PR which is generally an improvement.

[eileenmcnaughton]

@colemanw - locally still hitting IDS issues - I'll try on the test site once it's stable - note 'action' is not resolved

[colemanw]

@eileenmcnaughton you can always give yourself the IDS bypass permission locally

[eileenmcnaughton]

@colemanw if you want to punt IDS to a separate PR / follow up I'll test as admin - I think it's important to fix but happy to not make it blocking on this

[eileenmcnaughton]

Actually @colemanw I get that unresolved action tag for demo user even when the params are not in the url

[eileenmcnaughton]

@lcdservices is this on your radar - this will make it possible to create smart groups based on line items. I currently see some limitations (which I'll worry about once these changes are finalised) - but I suspect you will have good input on it.

[eileenmcnaughton]

Just loading the page.... as demo

[colemanw]

@eileenmcnaughton I think again the problem was you're using an outdated test site. That error was fixed by 7a56c06 and looks good to me on the latest demo:
http://core-18644-95ouo.test-1.civicrm.org:8001/civicrm/search#/create/Contact/

[eileenmcnaughton]

OK - I agree that link works & I was able to

1. create a smart group based on line item criteria
2. use HAVING criteria

Let's merge this & then try to put some use cases through it

[colemanw]

Thanks @eileenmcnaughton I pushed up one more small tweak for code sanitation so we'll wait for this latest test run then merge.