Skip to content
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

Conditional validations and fields #66

Closed
ivanvanderbyl opened this issue Jul 14, 2016 · 11 comments
Closed

Conditional validations and fields #66

ivanvanderbyl opened this issue Jul 14, 2016 · 11 comments

Comments

@ivanvanderbyl
Copy link
Contributor

I have a use case where some form fields are only required if a certain value is selected somewhere else in the form (in this case a checkbox). Upon investigating how validations and changesets work, I can see it's not so trivial to add support for this because not only do you need to not disable the validations, but you also need to ensure that field doesn't get propagated to the model on execute.

Any thoughts on this? Am I missing some not so obvious way to achieve this? Otherwise, I'll have a crack at implementing it soon.

@poteto
Copy link
Collaborator

poteto commented Jul 14, 2016

Hmm unless I'm misreading it, I think you could do this using ember-changeset-validations with a custom validator. Are you using that add-on for validations? This add-on is only the logic for the changeset itself.

@ivanvanderbyl
Copy link
Contributor Author

Hi @poteto ! I am indeed using changeset-validations, but as far as I can tell the individual validations don't receive a reference to the changeset, so there's no way to query the value of the checkbox during validation.

Upon thinking over this further, I don't think it's just disabling the validation that is needed, because in that case you might just return true and assume the attr in valid. What is really needed is excluding that value from the changeset and not validating it.

@poteto
Copy link
Collaborator

poteto commented Jul 18, 2016

@ivanvanderbyl The validator function receives the changes object, which is a map of all the changes that have been made. Would that work? Also, this issue is probably more relevant on the ember-changeset-validations addon!

https://github.com/DockYard/ember-changeset-validations#writing-your-own-validators

@ivanvanderbyl
Copy link
Contributor Author

ivanvanderbyl commented Jul 18, 2016

Yes, I noticed that, however changes alone aren't enough to solve this problem.

Imagine the scenario: You have a form with a conditional section, in this case it collects a bunch of EC2 configuration details like VPC ID, Security Group, AZ, and Subnets. These are all required fields only if the "Use EC2" checkbox is ticked, otherwise they must send the default value of that attr on the model, which is null.

Now I can easily discard the validations of these properties if useEc2 is false. However, if these fields change I can't set default value back to null from within the validate function as it's not very "pure", and I'm trying to only use the changeset helper to construct the Changeset.

A solution I'm attempting is adding an additional form helper to flood-simple-form which exposes a conditional section, ala fieldset, with its own changeset and validations. If the condition is true and it's valid it gets merged into the main changeset, otherwise it's removed. At least, that's the thinking, we'll see how well it works in practice.

@poteto
Copy link
Collaborator

poteto commented Jul 18, 2016

I see, my original intention for not passing in the changeset itself as a param to the validator function was to enforce a more functional approach with no side effects. Even the changes object that is sent to the validator function is a copy of the real object.

I suspect your use-case would be better off handled by a custom action rather than in a validator. Let me know if you manage to solve this with the action / merging, or if not we can revisit / discuss again!

@ivanvanderbyl
Copy link
Contributor Author

I like that approach, oh and by validator I was actually referring to a custom validate action, not the validator function from changeset-validations.

This approach so far has one snag: you can't merge invalid changesets, which is probably something you want, but now I'm trying to think off a good way to keep track of multiple changesets, and aggregate their errors so that a single list of errors is available to the form. Shouldn't be impossible :)

@poteto
Copy link
Collaborator

poteto commented Jul 18, 2016

I think I wouldn't mind merging invalid changesets, I was probably a little heavy handed there. Want to open a PR for it?

@ivanvanderbyl
Copy link
Contributor Author

You know, that would actually make this a lot easier. I'll create a PR in the morning :)

@allthesignals
Copy link

I know I know this is very old, but I'm facing the same issue, and I don't quite understand how merge applies here?

I ask because I'm also implementing a conditional validation. I'm able to get this to work mostly, but the changeset doesn't seem to re-validate correctly because the conditional validation depends on attrs other than the one being updated.

This leads me to believe I might be missing a possible use for merge? Do you recall (almost 4 years later) how this was useful for you?

@snewcomer
Copy link
Collaborator

@allthesignals Do you have an example (or dummy app)? I'd be happy to look into this and possibly get working.

@allthesignals
Copy link

@snewcomer I think I got it working, I have a PR up, here's a component class in particular: https://github.com/NYCPlanning/labs-applicant-portal/pull/207/files#diff-fa73b7a98e0df38cb1686492b3077971

Here's my conditional validator: https://github.com/NYCPlanning/labs-applicant-portal/pull/207/files#diff-2a9c92ebab030a35b86d85c9f750a967 in the same PR. There is an issue with conditional validators that I was able to get around by explicitly re-validating a changeset when a field input changed.

I was mainly asking because I've gone through multiple iterations of this, trying to land on the "right" approach. The approach I linked above seems simplest (before I was wrapping stuff in proxies).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants