ensure checkbox imports work on contacts

[jmcclelland]

Overview

Ensure checkboxes with string values are imported properly.

See https://lab.civicrm.org/dev/core/-/issues/3850 and the fix in #24848

The initial fix was tested, but I think I may have tested when importing the integer values? I'm not sure, but now when I test with string values for checkboxes it fails.

Before

When importing a check box custom field using the multiple choice values in the import, the user gets a validation error: "1" is not a valid option for field custom_1.

After

The import is properly recorded in the database.

Technical Details

I followed the lead of the early PR and removed code that seems unnecessary now.

[civibot]

(Standard links)

• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[eileenmcnaughton]

Thanks @jmcclelland - we should target the 5.56 rc branch since this is a regression

[jmcclelland]

@eileenmcnaughton should I edit this PR and change the target from master to 5.56 or open a new PR?

[eileenmcnaughton]

@jmcclelland I realised I wasn't quite sure how to replicate the described error - this PR e99a2a1#diff-775f76b58122b4c51d682ada372364fefe37d6f13f4eba37fb361b88d6de40f5R863 does seem to test text labels

Based on my general feeling that almost all that function is better out that in & handled elsewhere I will merge this if you tell me you have UI-tested it & it improves the situation - but ideally we would pin down the issue & add a test & fix in the rc. It depends where you are at with it.

[jmcclelland]

I have this code running in production on our 5.51 sites. But, I still don't have a solid grasp on why this caused breakage and why my fix actually worked, so it might be better to pause on this. I may need a week or two before I can circle back and try to get a test in place.

[eileenmcnaughton]

@jmcclelland ok sure - I'll close the gitlab for now as we don't have a known bug at the moment

I'm also Ok to merge this to master on the basis of it being 'code that should go' - if you have it running with this patch on a bunch of prod sites. In general I think that function is mostly code-to-be-removed.

[civicrm-builder]

Can one of the admins verify this patch?

[stesi561]

@jmcclelland did you get further with this or should it be closed?

[eileenmcnaughton]

ohh - thanks for highlighting this one @stesi561 - we have a report of this bug again & I have managed to replicate but hadn't started trying to fix yet...

[eileenmcnaughton]

Closing in favour of #25639 which is against the rc