dev/core#155 Allow saving of new option value with value = 0.

[mattwire]

Overview

Allow saving of new option value with value = 0.

Before

Could not save new option value with value=0.
After

Can save option value with value=0, no warning that datatype does not match.

Technical Details

Code was using empty() and !$variable which both treat 0 as FALSE. These are changed to !isset() and $variable === FALSE respectively.

Comments

Long-standing bug that's been around since 4.6!

[eileenmcnaughton]

[GitScan\Exception\ProcessErrorException]
Process failed:
[[ COMMAND: git apply --check --ignore-whitespace ]]
[[ CWD: /home/jenkins/buildkit/build/core-12229-23ix1/sites/all/modules/civicrm ]]
[[ EXIT CODE: 1 ]]
[[ STDOUT ]]
[[ STDERR ]]
error: patch failed: CRM/Admin/Form/Options.php:450
error: CRM/Admin/Form/Options.php: patch does not apply

[eileenmcnaughton]

@mattwire I have been discussing with @colemanw switching over the OptionValue form to use the api so this might be a short-lived change. There are some metadata things to work through to make that step but basically that is where we want to go (& to eventually allow custom data on that form although we might need to think about whether that custom data can be extended by option group)

[eileenmcnaughton]

Merging this as it does address the issue as described and the clean ups all appear correct. Per comments above - we do hope to deprecate the function

[eileenmcnaughton]

@mattwire is there a gitlab issue to close?

[JoeMurray]

I realize this has been merged, and I like the idea in general. However, I'm wondering if any thought or testing went into whether having a zero value option will cause issues in code that uses option values.

[mattwire]

@JoeMurray This only affects creating new ones, it's always been possible to update to value=0 via UI after creation. The value 0 is being used quite extensively on a few sites which have wellbeing style questionnaires , eg How are you feeling 0,1,2,3,4

[JoeMurray]

I understood this was only for new. I'm pleased to hear that you have experience that using 0 as a value has not led to issues being surfaced elsewhere in code. I'm satisfied.