-
-
Notifications
You must be signed in to change notification settings - Fork 52
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Default InputerFilter in Form creates Input, instead of ArrayInput for multi select elements #5
Comments
Still no feedback? At the moment I do not have time to find the origin of the bug and provide a patch. But it is actually a critical bug and should be fixed. I am wondering, why no one reported it until now. Originally posted by @eweso at zendframework/zend-form#204 (comment) |
Please review the code of conduct; the majority of developers on this project are volunteers, and there are close to 200 repositories we maintain. We simply cannot jump on every issue or pull request as it comes in. Additionally, pull requests, even those that simply provide a failing test, get priority over issues generally, as they provide us with more concrete information on how the reporter is attempting to execute the code, and what their expectations are.
Please demonstrate how you are doing this, so that we can verify that we are creating the scenario in the exact same manner you are; without that information, we may be unable to reproduce the issue. (E.g., the options you are passing to the multiselect may be invalid, or if you are using configuration, there might be a subtle typo leading to the scenario.) Originally posted by @weierophinney at zendframework/zend-form#204 (comment) |
I see. Thank you for your response! I will provide the relevant code within the next two weeks. I am a little busy right now. Originally posted by @eweso at zendframework/zend-form#204 (comment) |
Here we go. I hope, this helps. Version of Zend-Form is "2.5.3". Originally posted by @eweso at zendframework/zend-form#204 (comment) |
@eweso public function testValidationWithOptionMultiple()
{
// Element
$element = new SelectElement('foobar');
$element->setValueOptions(
[
'A' => 'A',
'B' => 'B',
'C' => 'C',
]
);
$element->setAttribute('multiple', true);
$element->setUseHiddenElement(true);
// Input filter
$inputFilter = new InputFilter();
$inputFilter->add($element->getInputSpecification());
// Tests
$inputFilter->setData([
'foobar' => [
'A',
'B',
],
]);
$this->assertTrue($inputFilter->isValid());
$inputFilter->setData([
'foobar' => 'A',
]);
$this->assertTrue($inputFilter->isValid());
$inputFilter->setData([
'foobar' => 'D',
]);
$this->assertFalse($inputFilter->isValid());
} The Originally posted by @froschdesign at zendframework/zend-form#204 (comment) |
If you do not add a filter or validator, you don't need an ArrayInput, this is true. But if you create a dynamic multi-select and you want to validate and filter all of it's entries, you need to have an ArrayInput, to iterate through all items of the select. If not, all other validators, like EmailValidator will return false, because EmailValidator expects a string, not an array. Originally posted by @eweso at zendframework/zend-form#204 (comment) |
Filters and validators are already there: https://github.com/zendframework/zend-form/blob/4419eef6dbe9d276e7e27c6a25f022be74534959/src/Element/Select.php#L288
If you set the the I'm sorry, but you're only describing your conclusion and not the actual problem. Originally posted by @froschdesign at zendframework/zend-form#204 (comment) |
This is not what I was talking about. I want another validator! I don't want to test, if the value is in the given set of value_options, I want to validate each value, because the client is allowed to manipulate the select values dynamically and the values which are set, are email addresses and those addresses need to be validated, before I work with them.
Here we go. If you add another validator, like EmailAddress, than the validation process fails. Because the Input is not an ArrayInput, the Validator validates the array as the EmailAddress-Validator value.
Originally posted by @eweso at zendframework/zend-form#204 (comment) |
And why is this info and code example missing in your first post? Your usage is wrong. You must set the See: https://docs.zendframework.com/zend-validator/validators/explode/#supported-options The last line of your code example should be included Originally posted by @froschdesign at zendframework/zend-form#204 (comment) |
@froschdesign allah knows why I’ve been following this one, but I have. None of the solution you have finally given is obvious from the documentation. GitHub is not the place for support. But you have, finally, given it. Why didn’t you suggest a PR on the docs? Originally posted by @kynx at zendframework/zend-form#204 (comment) |
@kynx But you are right, the conclusion of this issue report is to extend the documentation for the https://docs.zendframework.com/zend-form/element/select/ Originally posted by @froschdesign at zendframework/zend-form#204 (comment) |
@kynx Originally posted by @froschdesign at zendframework/zend-form#204 (comment) |
But why don't you use just the ArrayInput on multi select? I mean, what is the usecase for the ArrayInput, if we do not use it on array values? Why does the zf2 use the Explode-Validator as a ValidatorChain replacement, to validate array values? I do not get it, actually. I mean, first of all thank you for your hint to use the ExplodeValidator instead, but do you really think, that this is a good and clean solution? I always attach a configured InputFilter by the setInputFilter() method to Forms and because the Form creates a default InputFilter and merge the two InputFilters, you cannot change the type of the Input element. It will just fetch all Validators and Filters of the InputFilter I set and attach it to the given one. My solution to prevent this error was to use method setUseInputFilterDefaults(false) in the Form. This will prevent the form to create an InputFilter and therefore my InputFilter will not be merged, it will be used as it is. Originally posted by @eweso at zendframework/zend-form#204 (comment) |
@eweso Thank you in advance! Originally posted by @froschdesign at zendframework/zend-form#204 (comment) |
I don't think, that the behaviour will change. Therefore I will create a patch and pull request. But don't expect it in July. Beginning of August sounds realistic. Originally posted by @eweso at zendframework/zend-form#204 (comment) |
Update: I will start in September at the earliest. At the moment I have to much to do at work. I am sorry for that! Originally posted by @eweso at zendframework/zend-form#204 (comment) |
@eweso Originally posted by @froschdesign at zendframework/zend-form#204 (comment) |
ArrayInput does not support required so you'll run into that @eweso I agree the current solution is not optimal but it's a decent stop-gap solution. But better focus on improving inputfilters then we can fix this later. If you look at the suggested improvements in zendframework/zend-inputfilter#13 Input and InputFilter both implement the same interface. As a consequence you can then merge ArrayInput and CollectionInputFilter into a single class. Originally posted by @Erikvv at zendframework/zend-form#204 (comment) |
I already pulled a bugfix for the required problem on ArrayInput: There is no BC break on that. Originally posted by @eweso at zendframework/zend-form#204 (comment) |
The default InputFilter of Forms creates always an Input for Elements within the Form. Even, if the Select Element has the multiple attribute. The problem is, when you create an InputFilter and set it to the Form, then the default InputFilter won't be replaced, it will merge Validators and Filters with the new ones. So you're given ArrayInput will be an Input in the form and the validators will fail, because they receive an array as value, but expect a string|number.
Code to reproduce the issue
Just create a Form with a multi select.
Expected results
The default Input for Select with multiple attribute should be ArrayInput.
Actual results
The default Input for Select is always Input.
Originally posted by @eweso at zendframework/zend-form#204
The text was updated successfully, but these errors were encountered: