Ensure advanced search displaying related contacts works consitently

[jmcclelland]

Overview

An advanced search, in which contacts are display as related contacts, fails to find any results under certain conditions. See: https://lab.civicrm.org/dev/core/-/issues/2707

Before

1 Click Search -> Advanced Search
2. Under "Display results as" choose "Related Contacts" and choose "Employee of" as the relationship type.
3. Then select "Organization" as the contact type.

I expect this to display all individuaul contacts that have a Employee of Relationship to an organization. Instead, I get no results.

It seems that the search is looking for all related contacts that have their contact type set to Organization, instead of all related contacts with an employer that is an organization.

After

When conducting a search to find individual contacts that have an Employee of Relationship to an organization using the "Display Results as" functionality, the expected contacts are displayed.

Technical Details

A recent commit changed how the WHERE clause is generated when searching for related contacts.

By fixing one part of that commit, we seem to have the intended functionality.

Comments

I have not fully tested to ensure that the functionality of the original commit is preserved with this change.

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

[demeritcowboy]

I can take a look.
This would go against 5.40, since that's the convention, and also it's pretty close to 5.40 release so there might not be another 5.39.

[demeritcowboy]

The second fail is expected. The first one seems at first glance to mean this and the original PR aren't compatible as-is.

CRM_Case_BAO_QueryTest::testAdvancedSearchWithDisplayRelationshipsAndCaseType
Failed asserting that actual size 8 matches expected size 1.
/home/jenkins/bknix-dfl/build/core-20997-69j42/web/sites/all/modules/civicrm/tests/phpunit/CRM/Case/BAO/QueryTest.php:172

CRM_Contact_SelectorTest::testSelectorQuery with data set #11 (array('Test display relationships', 'CRM_Contact_Selector', array(), array('1_b_a'), null, array(), 'advanced', 0, null, false, array('SELECT contact_a.id as contac...egion`', 'WHERE 1 AND displayRelType.re...d = 0)')))
different sql
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'WHERE 1 AND displayRelType.relationship_type_id = 1 AND displayRelType.is_active = 1 AND ( 1 ) AND (contact_a.is_deleted = 0)'
+'AND displayRelType.relationship_type_id = 1 AND displayRelType.is_active = 1 AND ( 1 ) AND (contact_a.is_deleted = 0)'

[jmcclelland]

Thanks @demeritcowboy - I'm still puzzling over how to fix it, but I figure adding a new test to lock in the behavior I'm seeking couldn't hurt.

[jmcclelland]

I think we should wait for @jaapjansma to return to review.

I think that commit 4f4fb80 changes the logic behind the "display results as related contacts" searches - and either it's wrong or my understanding of how it works is wrong. I'm a bit overwhelmed by the logic - so could really go either way :).

I came to this opinion by more carefully examining the logic behind the failing test: testAdvancedSearchWithDisplayRelationshipsAndCaseType. The test expects to retreive one result, but I would expect the test to retrieve two results (and it retrieves two results if I revert commit 4f4fb80).

The test creates a single client (John) with a housing case in which Alexa is the benefits specialist and a day care case in which Sandra is the specialist.

The search is: Find all contacts with a housing case and then display all the contacts with a benefits specialist relationship to those contacts.

Should it return just Alexa or both Alexa and Sandra?

I expect it to return both - since Sandra has a benefits specialist relationship to a contact that has a housing case - even though it's not the case she has a relationship to.

In this particular situation I can understand wanting it the other way and getting both contacts would be suprising. On the other hand, I think the general logic of a "display related contacts" search is: first, find everyone who matches the criteria. Second, find everyone with the given relationship to them. Selectively applying the initial criteria to the second search seems complicated.

I haven't really investigated how searching for related contacts using the relationships accordian works, but. I think that method would make more sense if you want to search for multiple criteria including a relationship type (although I think this search has been somewhat broken for a while).

[eileenmcnaughton]

I took a look at this to see if I could find my way thought this - it seems the 2 use cases disagree about whether the original WHERE should be overwritten.

My instinct is that we should consider reverting #20002 in favour of the long-standing functionality to give us time to fix.

[demeritcowboy]

I don't have any objections to reverting 20002. It may also mean relooking at #18779 later but I think that only affects searches involving the cases accordion.

[eileenmcnaughton]

thanks @demeritcowboy - my instinct is this is a long-standing feature so we need to prioritise it over something that was just added - but hopefully we can get re-fixed fairly quickly

[eileenmcnaughton]

Closing this as we reverted #20002 for now to deal with the regression in time for the releases to go out