Autocomplete-select custom field is not searchable

changed milestone to %5.26.0

added needs:concept approval sig:bug labels

changed the description

@colemanw @eileen @JoeMurray I need your help in deciding the correct approach to fix this issue.

Currently 'Autocomplete Select' (with String/Int data type) custom field is a single select field to record value. But, when it is enabled for searching, in other words, used as a filter in search form(s), it is rendered as multi-select autocomplete filter which can take multiple options and eventually on searchform submit, pass comma separated values which result into this issue. In order to fix this issue I have few solutions in mind, but not sure which will be the correct approach. These are:

To make the search filter consistent like in case of other type of custom fields, make it a single select autocomplete filter in the search form. e.g. a 'Contact Reference' custom field is a 'Autocomplete' single valued field and when used as a filter retain this same behaivour.
With #1 (closed) we will lose the feature of selecting multiple choices for 'Autocomplete Select' HTML type custom field as a filter in search form. In order to support the current behavior:

Add formrule validation to avoid using a comma in option value, because comma is used as value separator in select2 widget. So it leads to various issues if an option value contains comma.
Add PHP code in core, to handle the comma-separated filter value for this HTML kind of custom field(s).
(optional) add a special setting under 'Search Preferences' where user can decide the value seprator for mult-select fields (set default to ',') and tweak the select2.entityRef widget to accept the valueseparator from setting. Because, some may want to use ',' in option value.

Please let me know your thoughts.

Thanks! Monish

Functionally, we need to support multi-select on search.

From a product management perspective I am opposed to the idea of a setting for something so technical.

I think it is not a good idea to try at this point to validate all option values for these sorts of fields to exclude a comma; there are too many instances that might have them in use and it is merely for convenience that we want to remove them. It should be possible to produce SQL along the lines of civicrm_value_donor_information_3.test_12 IN (1,'2,2a',3)

I believe that this is a bug that we should fix in core. I'm not sure there is a need for a big discussion about this.

I agree with Joe on this.

I think the thing is for BAO_Query::convertFormValues to convert them to

'custom_id' => ['IN' => ['abc', 'def']];

& for them to be like that before they hit the Query.

We should be able to support commas in labels but not in the value fields

@eileen yes agree. And on # 2.1 by 'option value', I meant OptionValue's value where comma shouldn't be supported but do I need to add this validation rule for all kinds of selectable fields (i.e. radio/checkbox/select/Autocomplete-select) or just for 'Autocomplete select'? because its the only widget as a filter, which passes a comma separated values of form submit.

@monish.deb do you mean the validation rule would be on the form where you configure the available options for custom fields? I'd be inclined to go with it being generic. I bet commas in the others would bite us sooner or later in other ways

Yes validation on custom field form, to be more specific, need to extend the check here for 'Autocomplete Select' and also add a validation rule to prevent comma on value.

For other types of mult-select widget like checkbox it isn't an issue because the submitted formvalue is in array (not imploded with comma) and we already have the code to detect and construct where subclause using 'IN' or 'NOT IN'.

I have submitted a PR https://github.com/civicrm/civicrm-core/pull/17569 for the fix

I'm a little uncomfortable 'fixing' the lack of a validation to prevent commas in civicrm_option_value.value.

I agree that there is likely not a strong argument in favour of supporting this.

But adding a validation means we would need to do an upgrade script to a) remove / munge to a new value excluding commas any that exist or they would no longer be editable and b) update all of the custom field values that have the existing value. There may be knock-on effects on custom code depending on constants for example, that refer to specific option values.

I agree with @JoeMurray that adding validation would also need to remove comma and munge optionValue's value to new value in the upgrade script. However, having ',' in value ONLY affects 'Autocomplete Select' widget as a filter so I was thinking how about triggering a popup notification on 'New Custom Field' form whenever someone chooses String/Number + 'Autocomplete Select' and notifies with, say 'It's not recommended to use ',' in the option value.'

Earlier I did an investigation on how we can fix the entityRef widget, and only way I can think of is to indicate a different value separator in select2 setting for imploding values for 'Autocomplete Select' filter but there's always a chance that a separator is present in OptionValue's value.

Let's leave out of scope removing commas from option_value.value, and aim to support

'custom_id' => ['IN' => ['abc', 'def', 'f,g']];

Sure. I have updated the PR https://github.com/civicrm/civicrm-core/pull/17569 (removed the added validation rule).

mentioned in commit caec1f13

mentioned in commit 71dfa06c

closed

changed the description

Autocomplete-select custom field is not searchable

Reproduction steps

Current behavior

Expected behavior

Technical comment

Comments

Child items ...

Activity