forms icon indicating copy to clipboard operation
forms copied to clipboard

ValidationScope: User unfriendly implementation

Open juniwalk opened this issue 9 years ago • 24 comments

Hello, I'd like to ask, could we come up with some way to rework disabling validation scope for selected fields?

Currently if you have 17 fields and one of them is AJAXified <select>, you have to do this:

$form->addSubmit('submit')->setValidationScope([
        $form['name'], $form['email'], $form['password'], $form['phone'], $form['ic'],
        $manage['isBanned'], $manage['isApproved'], $account, $address,
]);

Best way would be to disable this per-field.

juniwalk avatar Feb 17 '16 20:02 juniwalk

What about condition?

$form->addSelect('country', 'Country:', $countries);
$form->addSubmit('submit', 'Send');

$form['country']
    ->addConditionOn($form['submit'], $form::SUBMITTED)
        ->setRequired('Choose country');

dg avatar Feb 17 '16 20:02 dg

How does this help me when I have those countries loaded using AJAX and have $select->checkAllowedValues = FALSE;? I need to disable scope validation for that select for the submit to work.

Sorry if I haven't made myself clear about that.

juniwalk avatar Feb 17 '16 22:02 juniwalk

Why you need to disable scope validation for select?

dg avatar Feb 19 '16 21:02 dg

Because I am stupid, $select->setPrompt() is what I needed, not to disable validation scope. But I still think that disabling the scope per field would be better.

juniwalk avatar Feb 20 '16 12:02 juniwalk

@dg I am closing this as it is probably unnecessary to have it here.

juniwalk avatar Feb 25 '16 15:02 juniwalk

@dg I have to disable validation scope when I use $select->setRequired();, $select->setPrompt() is not applicable in this case.

The problem is when I submit form with AJAX data, the value of select is outside $data variable, I have to get it using $form->getHttpData() which is okay but the form fails to submit because value of select is required and there is none in the time of checking.

I am back to my initial issue, disabling validation scope in this way is not friendly.

juniwalk avatar Mar 02 '16 14:03 juniwalk

Can you try to implement any solution?

dg avatar Apr 01 '16 18:04 dg

@dg Well I have a few ideas

First idea

  • Add $validateScope property to BaseControl which will be bool.
  • Allow disabling scope per control using $control->disableValidationScope(); // just examples
  • Before the form is builded, iterate over each control and build validationScope and assign it to SubmitButton
  • But what if there is more than one SubmitButton control? Is that possible? That would cause problems.

Second idea

  • Add class Nette\Forms\ValidationScopeBuilder
  • Implement \Traversable or extend \ArrayObject
  • Pass form instance into it
  • Call $vsb->disableScopeFor('control');
  • Pass instance into $submit->setValidationScope($vsb);
  • This will get all current controls from the form, remove those disabled and foreach in setValidationScope will iterate over them.

Third idea

  • Each BaseControl can get it's parent Form instance
  • We could add disableValidationScopeFor('control') on the SubmitButton control
  • Add those control names into list and then just select all controls from form and leave out disabled ones

What do you think? Does any of those sound good?

juniwalk avatar Apr 01 '16 22:04 juniwalk

the form fails to submit because value of select is required and there is none in the time of checking.

Why not just set $select->checkAllowedValues = FALSE? Redesigning validation scope seems like overkill.


EDIT: Seems like we would need to modify ChoiceControl::getValue() to respect $checkAllowedValues as well. What do you think?


EDIT: No we can't do it, it would be a BC break.

JanTvrdik avatar Jun 19 '16 08:06 JanTvrdik

@JanTvrdik Yeah, it is particularly troubling issue this one. I don't mind working with validation scope, it's just not very intuitive to work with.

juniwalk avatar Jun 19 '16 15:06 juniwalk

I don't think that validation scopes should be used for this kind of issue at all.

Another random idea

$form->addSelect('country', 'Country', [$form->getHttpData('country') => '...'])
    ->setRequired();

JanTvrdik avatar Jun 19 '16 15:06 JanTvrdik

@JanTvrdik I don't think that validation scopes should be used for this kind of issue at all.

Really? Why?

With your another random idea, I see potential problem with setting default value for editing.

juniwalk avatar Jun 20 '16 10:06 juniwalk

@juniwalk Because if you don't want the value to be validated, you should not make the select box required. What's the point of setting validation rules if they should be ignored?

JanTvrdik avatar Jun 20 '16 11:06 JanTvrdik

@JanTvrdik I do want the value validated, and I need it to be required, but that does not play well with AJAX values feeded in to the select box ...

juniwalk avatar Jun 20 '16 11:06 juniwalk

I do want the value validated

Could you elaborate on what exactly and where do you validate?

JanTvrdik avatar Jun 20 '16 11:06 JanTvrdik

@JanTvrdik In this context I ment the requirement validation as i use this to suggest user names and I get back just int (user Id) which I then in the onSuccess (probably not properly done but that is how I use it).

juniwalk avatar Jun 20 '16 11:06 juniwalk

I'm sorry, I still don't understand it. How could you use the server-side validation when the select box is not validated at all because it is not included in the validation scope? What did I miss?

JanTvrdik avatar Jun 20 '16 12:06 JanTvrdik

@JanTvrdik Ignore it, we diverged from the problem here, validation is not the issue, getting the value is.

juniwalk avatar Jun 20 '16 12:06 juniwalk

@JanTvrdik Yes, and to disable the scope is absolute horrendous work to do (especially for large forms).

juniwalk avatar Jun 20 '16 12:06 juniwalk

->setValidationScope(array_diff_key($form->getControls(), ['country' => TRUE])) is horrendous?

edit: $form->getControls() returns ArrayIterator, so iterator_to_array($form->getControls()).

dg avatar Jun 20 '16 12:06 dg

@dg Hey, now that looks much better, would you mind if I send PR to docs with this example to help others like me?

juniwalk avatar Jun 20 '16 12:06 juniwalk

Of course I'll be happy. But try it please, I wrote from my head.

dg avatar Jun 20 '16 12:06 dg

@dg So I tried that array_diff_key and it throws an error PHP Warning: array_diff_key(): Argument #1 is not an array in ./app/forms/ProductForm.php:136

EDIT: I'd have to combine it with iterator_to_array(). Having $select->disableValidationScope('submit'); would be really lovely.

juniwalk avatar Jun 22 '16 07:06 juniwalk

Hi, I revisited this old issue. Now when I am more skilled in Nette I basically never modify validationScope, I do my own validation in onValidate event.

I was modifying it before to keep the clientSide validation so user gets alerted before submitting that he did not select something required, however I can live without it.

I think we can close this issue.

juniwalk avatar Mar 13 '24 09:03 juniwalk