dev/core/issues/726, Fixed fatal error when searched using group type

[pradpnayak]

Overview

https://lab.civicrm.org/dev/core/issues/726

Before

After

Comments

Error occurs when there is no group of Mailing list or Access list.

[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]

@pradpnayak is this something that we can replicate in a unit test?

[eileenmcnaughton]

test this please

[pradpnayak]

@eileenmcnaughton sure, will do!

[eileenmcnaughton]

I tested & confirmed that I can replicate this & the above fixes - once you get a chance to add a test it's all good IMHO

[eileenmcnaughton]

@jitendrapurohit how does this tie into the one you just submitted? (@pradpnayak was going to do a test but hasn't gotten to that)

[jitendrapurohit]

If I apply this fix with or without my PR applied, I get a DB syntax error on my usecase described at #13727 (comment) as the where clause that will be formed on searching contacts => groups => is null would be -

WHERE  (  (  ( ( `civicrm_group_contact-5c77bcc5c2eb5`.group_id IS NULL 0  ) )  )  )

Notice the 0 added at the end of the condition in all the cases. Maybe, we should limit it more to not execute this line in case of empty or null operators.

[eileenmcnaughton]

Yes, early return makes sense - we might also try using

CRM_Core_DAO::createSQLFilter

to generate the string.

We obviously don't have much test cover on this fn

[eileenmcnaughton]

This really does want a test but I'm gonna let it slip through without as

1. it's clearly a bug fix & is replicable
2. I'm confident it is a very safe fix
3. it's stalled for a bit & since it's also kinda trivial & @pradpnayak has a strong track record of adding tests I'm gonna push it in rather than out

[jitendrapurohit]

The error mentioned above still replicates for us. This time it displayed a fatal error on one of the mailing page -

Steps in https://lab.civicrm.org/dev/core/issues/768 can be followed to replicate it easily on search builder.

We fixed it by -

if (empty($regularGroupIDs) && strpos($op, 'NULL') === FALSE) {
   $regularGroupIDs = [0];
}

@eileenmcnaughton @pradpnayak does it makes sense to avoid this assignment for NULL operators?

[eileenmcnaughton]

@jitendrapurohit are we ignoring the op later on?