dev/core#589 - Fix saving autocomplete search preferences

[colemanw]

Overview

Fixes recent regression of saving autocomplete-related preferences on the Search Settings form.

Before

Autocomplete Contact Search and Contact Reference Options preferences do not save when submitting the Search Settings form.
See https://lab.civicrm.org/dev/core/issues/589 for details.

After

Settings are able to be saved.

Technical Details

I'm not sure exactly what caused this to regress but there has been a lot of refactoring of the settings forms of late, with the goal of making the forms more metadata-driven. When investigating the bug I noticed those two fields were getting special treatment in a number of places, for what appeared to be legacy reasons. Instead of trying to put another band-aid on those 2 fields, I removed all of their special treatment so they are completely metadata-driven. And now they work :)

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

[MegaphoneJon]

I'm only partway through testing this, but I thought I'd post my preliminary results from r-run.

• The patch fixes saving autocomplete search preferences, so that's a good improvement.
• The pre-selected "Contact Name" has an "X" in most places where this appears, but not here (see screenshot item 1). This is cosmetic and shouldn't block merging a patch that fixes a regression.
• The labels for the checkboxes (item 2) didn't appear until I flushed caches. That's fine as long as upgrade clears caches.
• Setting "postal code" (item 3) for contact reference fields doesn't seem to work. It does work on auto-complete.
• This doesn't resolve the issue of QuickSearch by "name/email" not searching by email, but perhaps it's not intended to.
  

I'll do a code-level review now, but I'll admit that I haven't kept up with recent changes in this area so it will be slow.

[eileenmcnaughton]

@colemanw is this a released fix or just in the rc? I actually thought we tested those fields :-(

[MegaphoneJon]

OK, I've just reviewed the code as well.
Insofar as these changes are toward a metadata-driven approach, there's actually very little code to review. In CRM/Admin/Form/Setting.php no code was added, only removed, and it's all related to getting/saving these two options. Since r-run confirmed this worked, that's fine.

I see that getContactAutocompleteOptions() and getContactReferenceOptions() were refactored to remove an array flip. This appears to be necessary to support "checkboxes" as the html_type and quick_form_type. Since it only affects this form and it seems to work, this is fine too.

The rest is just metadata changes. This could be merged on pass with a follow-up fix to the postal code issue and the QuickSearch name/email issue or this could be updated to fix those issues.

[eileenmcnaughton]

thanks heap @MegaphoneJon - do you know if the 5.8 release is affected?

[MegaphoneJon]

@eileenmcnaughton Just tested and the answer is yes, 5.8.0 is affected.

[eileenmcnaughton]

@colemanw I'll merge a 5.8 pr if you create one