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

Form Validation: required_with and required_without implementations #1837

Closed
atishhamte opened this issue Mar 14, 2019 · 4 comments
Closed

Comments

@atishhamte
Copy link
Contributor

In the Form Validation Class, required_with and required_without are validation methods where the descriptions and implementation are for fields exist in $data or not.
But according to logic, it should be like,
[confirm_password => required_with[password]]
The confirm_password is required when the password is there in data and its value is non-empty

[email => required_without[mobile]]
The email is required when the mobile is there in data and its value is empty or blank

@lonnieezell
Copy link
Member

Right, but if we check that confirm_password exists first, we reduce checks we have to do. Even if the other field exists (i.e. password in your required_with example), we still want the test to pass if confirm_password is there. So that is handled first, and then if that's not there, we check the remaining fields in $data to see if the test should pass or not.

What am I missing?

@atishhamte
Copy link
Contributor Author

atishhamte commented Mar 20, 2019

For the above case, I can explain with the help of the campaign for which I kept the registration process same.
For normal signup, password and confirm_password is mandatory. But in case of the campaign, password and confirm_password is not compulsory to fill. So this check will work as the confirm_password is required_with password

@atishhamte
Copy link
Contributor Author

Should I generate the PR for the same?

@lonnieezell
Copy link
Member

I'm still confused. In the case of campaign, if they enter password, then confirm_password is required when using the required_with method. However, if they don't provide password, then confirm_password isn't required.

Looking at the code, it first checks if confirm_password is present. If it is, it allows the test to pass because if password is present, this field is here (hence required_with), and if password is not present we don't want to break the test. The test doesn't say it only exists when the other field is present. Only that the field must be there. In this case, if confirm_password is present it would make no difference on what would happen when password was missing.

If confirm_password is not present, then it checks to see if any of the fields we are required to be with are present. If they are present, then we have to fail the test because we've already determined that confirm_password isn't present, and now one of the fields this was required is present and has data.

That matches the intended usage of the method, doesn't it? The tests in RulesTest.php support that unless there's a flaw in that test.

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

2 participants