#991 subtypes not filled in when editing a smart group

[bhahumanists]

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

Overview

Fixes contact subtypes not being carried over when editing a smart group created in Advanced Search.

Before

If you edit a smart group that contains a contact subtype (and it was created in Advanced Search), the subtype is not included in the form. Subsequent searches then ignore it. Similarly it's not included in the qill.

After

The contact subtype appears correctly.

Technical Details

N/A

Comments

This PR isn't a great fix as there's likely a better place for it, but I'm not sure where that would be. I can't put it in normalizeFormValues() as that doesn't run in this context, and there doesn't seem to be anywhere suitable in the preProcess.

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

@bhahumanists does this affect other search forms (contribution search etc). If not I'm OK with this as a location for the fix.

I feel like there was an issue in a previous attempt to fix this - maybe @jitendrapurohit or @pradpnayak remembers?

Note there is a style warning blocking jenkins - do you know about those?

[MegaphoneJon]

The fix Eileen is referring to is #14373.

I did an r-run and confirm that this fixes core#991. There's a slight side effect that it will change the search very slightly by adding the contact type - e.g. searching for "contact subtype = Student" will become "contact type = Individual and contact subtype = Student". I was concerned about mixing contact subtypes across types - e.g. "Student" and "Team" only finding contacts who are both "Individual" and "Organization" - but this works as expected (Individual or Organization).

[MegaphoneJon]

I just did a code review as well.

The code itself looks reasonable if we agree that this is the correct approach. @bhahumanists I read on the ticket that @scardinius doesn't experience this bug on "Find Contacts" search. If this problem doesn't exist on Search Builder either, I'd feel comfortable with the code going here and would recommend a merge-on-pass. I don't think can affect Contribution Search per Eileen's comment above because contact subtype isn't present on Contribution Search.

[eileenmcnaughton]

test this pleasee

[eileenmcnaughton]

I'm OK with a fix at this level & with merging this based on @MegaphoneJon 's review

[bhahumanists]

I'll fix the whitespace issue (once I can figure out what it is, anyway)

[seamuslee001]

Jenkins re test this please

[colemanw]

retest this please

[eileenmcnaughton]

I think the fail relates

[yashodha]

test this please

[bhahumanists]

Updated. I don't think the fails relate now...?

[yashodha]

@eileenmcnaughton is this ready to merge?

[seamuslee001]

Jenkins re test this please