Fixing the loris form '0' emptying issue #2781
Conversation
sruthymathew123
added
the
Category: Bug
|
We could start by not returning empty arrays when we mean "there is no value here". Gee, I wonder, what value has the meaning of not having a value? |
php/libraries/LorisForm.class.inc
Outdated
| @@ -1180,7 +1180,7 @@ class LorisForm | |||
|
|
|||
| if (isset($el['required']) | |||
| && $el['required'] === true | |||
| && empty($value) | |||
| && (is_null($value) || (empty($value) && $value !=0)) | |||
There was a problem hiding this comment.
Could this just read
&& $value)? This will evaluate to false in each of the cases you want here.
http://stackoverflow.com/questions/2382490/how-does-true-false-work-in-php#2382510
There was a problem hiding this comment.
It can be this because empty(0) evaluates to true even though it should be false
There was a problem hiding this comment.
Yeah I made a mistake, good catch
php/libraries/LorisForm.class.inc
Outdated
| @@ -1180,7 +1180,7 @@ class LorisForm | |||
|
|
|||
| if (isset($el['required']) | |||
| && $el['required'] === true | |||
| && empty($value) | |||
| && (is_null($value) || (empty($value) && $value !=0)) | |||
There was a problem hiding this comment.
@driusan suggests that we should just use $value === "" instead of this, @sruthymathew123 can you see if that works?
|
@jstirling91 It is working for that case and anyway wide testing is needed. |
|
@jstirling91 I think this is in your court. Are your changes requested done or can the review be dismissed? |
|
@driusan the comments have been addressed. Though this pull request is to 17.0-dev, do we want to add this to a 17.0.4 release or should we switch it to 17.1-dev? |
|
@jstirling91 This plus your API fix (once it passes Travis) in a 17.0.4 sounds good to me. |
$this->form->addRule("value", "Required", 'required'); sets the 'required fail' when the user submits a value of "0", because empty($value) is true for 0.
This updates the code to explicitly check for the empty string to detect no input, and avoids PHP's empty() function for validation.
$this->form->addRule("value", "Required", 'required'); sets the 'required fail' when the user submits a value of "0", because empty($value) is true for 0.
This updates the code to explicitly check for the empty string to detect no input, and avoids PHP's empty() function for validation.
Loris form $this->form->addRule("value", "Required", 'required'); sets the 'required fail' for the value '0' which is sometimes a valid value/option for some instruments.
empty($value) is true for 0 which cause the issue.
So going for a better option to overcome that issue.