Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dev/core#2162: allow reports to filter multi-select fields and find entities with multiple selections #18978

Conversation

AsylumSeekersCentre
Copy link
Contributor

Currently, Reports which filter on multi-select fields will only find entities with single selections. The generated SQL syntax is not compatible with the storage format of multi-select custom fields.

Overview

Here is the issue which this fix addresses:
https://lab.civicrm.org/dev/core/-/issues/2162

The detailed description and reproduction steps are documented there.

Before

Contacts with multiple selections in a multi-select custom field would never appear in a Report which filters on that custom field. I believe this is a regression, though I haven't worked out exactly when it changed.

After

With this change, reports can filter correctly on these fields.

Technical Details

The addCustomDataToColumns function constructs a $customDAO object, which is then used to determine the operator (among other things). It wasn't fetching the civicrm_custom_field.serialize field when it constructed this object, so it concluded that custom fields are never serialized.

By fetching this field, it appears to function as intended.

Comments

When I wrote the initial issue description, I was under the impression that the change may have been deliberate. I now think that it was probably accidental, and reports filtering for "is one of" a multi-select field should find entities with more than one selection.

@civibot
Copy link

civibot bot commented Nov 16, 2020

(Standard links)

@civibot civibot bot added the master label Nov 16, 2020
@seamuslee001
Copy link
Contributor

@colemanw this looks right to me does it look right to you? @eileenmcnaughton

@eileenmcnaughton
Copy link
Contributor

Yes that makes sense.

@eileenmcnaughton eileenmcnaughton merged commit 4e279c7 into civicrm:master Nov 23, 2020
@eileenmcnaughton
Copy link
Contributor

(I think I made a similar change in extended reports)

@AsylumSeekersCentre AsylumSeekersCentre deleted the fixReportFilterMultiSelect branch November 23, 2020 04:11
@mlutfy mlutfy changed the title Fix issue #2162: allow reports to filter multi-select fields and find entities with multiple selections dev/core#2162: allow reports to filter multi-select fields and find entities with multiple selections Dec 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants