FIX: required extrafields: bad check to see if empty + prevent letting it empty (Issue #11169) #11202
Conversation
| @@ -1860,8 +1860,7 @@ function setOptionalsFromPost($extralabels, &$object, $onlykey='') | |||
| if ($this->attributes[$object->table_element]['required'][$key]) // Value is required | |||
| { | |||
| // Check if empty without using GETPOST, value can be alpha, int, array, etc... | |||
| if ((! is_array($_POST["options_".$key]) && empty($_POST["options_".$key]) && $_POST["options_".$key] != '0') | |||
| || (is_array($_POST["options_".$key]) && empty($_POST["options_".$key]))) | |||
| if (empty($_POST["options_".$key])) | |||
There was a problem hiding this comment.
If value is '0', empty will be true, so it will return an error.
For a combo list with multiple choice with first choice that has key '0' and label "My value 0" will not be possible to be selected. Old code seems good.
There was a problem hiding this comment.
I reverted to the old check but I don't really understand your use case. The tooltips for checkbox- and select-typed extrafields say that the keys should not be '0', so that Dolibarr itself could propose the empty choice with '0' key and empty label. Is the tooltip incorrect ?
Still, making the '0' choice available if the field is required seems an issue to me.
There was a problem hiding this comment.
The "tooltips" has right for combo.
But the test should works also for string or value. And for string '0' is not empty.
I pushed a better fix...
| @@ -1057,7 +1057,7 @@ function showInputField($key, $value, $moreparam='', $keysuffix='', $keyprefix=' | |||
| } | |||
|
|
|||
| $out.='<select class="flat '.$morecss.' maxwidthonsmartphone" name="'.$keyprefix.$key.$keysuffix.'" id="'.$keyprefix.$key.$keysuffix.'" '.($moreparam?$moreparam:'').'>'; | |||
| $out.='<option value="0"> </option>'; | |||
| if(empty($required)) $out.='<option value="0"> </option>'; | |||
There was a problem hiding this comment.
If you remove this line when field is required, it means the combo list will be automatically set to a default value (the first one). The record will be saved with a value that was not decided/defined explicitely by user. Why setting the default value on first value and not last one.
When a field is mandatory, we want the user to enter it and if we automatically set it to a random value (first one is same than random), we make the oposite of what we want. The check is done later (except if bug) after submitting. We can also avoid empty value by adding a required HTML 5 flag may be.
No description provided.