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

Same should match, regardless of if its set or not. Leave the required, to required. #763

Closed
MGParisi opened this issue Oct 20, 2021 · 9 comments

Comments

@MGParisi
Copy link

MGParisi commented Oct 20, 2021

Subject of the issue

Trying to validate passwords for Updates. If the password is blank, it shouldn't update the field. The validation thus should be nullable, but the nullable isn't working. At all.

` public function rules($update = FALSE) {
$rules = [
'name' => 'required',
'roles' => 'required',
'zip-codes' => 'required|array|min:1',
'zip-codes.*' => 'required|postal_code_with:US',

    ];

    if($update) {
        $rules['password'] = 'nullable|same:confirm-password;
        $rules['confirm-password'] = 'nullable|same:password';
        //$rules['password'] ='same:confirm-password|max:255';
        $rules['email'] = 'required|email';
    } else {
        $rules['password'] = 'required|string|same:confirm-password|max:255|min:3';
        $rules['email'] = 'required|email|unique:users,email';
    }
    return $rules;
}`

Your environment

  • version of this package: Latest
  • version of Laravel: 8

Steps to reproduce

add the above rule, and find that same is required. When same is not required (check docs). Same is just same and can be same if its blank.

Expected behaviour

Should check if they're null, and if not, check if their string and the same. Tried a huge number of these, but nothing works.

Actual behaviour

It keeps reporting password is enabled. Even when Update is true.

@MGParisi
Copy link
Author

Possible Solution:
In jsvalidation.js Line 5574
`
/**
* Validate that two attributes match.
*
* @return {boolean}
*/
Same: function(value, element, params) {

        var target=laravelValidation.helpers.dependentElement(
            this, element, params[0]
        );

        if (target!==undefined) {
            return String(value) === String(this.elementValue(target));
        }
        return false;
    },

`

if (target!==undefined) { shouldn't be here. Looking into how we can test both, the target and the source for undefined.

@MGParisi
Copy link
Author

MGParisi commented Oct 20, 2021

The Code feature in this editor is not working right... not sure what is wrong.

Tried this: /**
* Validate that two attributes match.
*
* @return {boolean}
*/
Same: function(value, element, params) {

        var target=laravelValidation.helpers.dependentElement(
            this, element, params[0]
        );

        if (target!==undefined) {
            return String(value) === String(this.elementValue(target));
        } elseif (target ==undefined && value == undefined ) {
            return true;
        }

        return false;
    },

Seems to be working

@MGParisi
Copy link
Author

MGParisi commented Oct 20, 2021

https://laravel.com/docs/8.x/validation#rule-same
The rule same works without confirmation of required. As that is a separate confirmation. A bit troubled, as this should had been caught.

https://laravel.com/docs/8.x/validation#rule-required-if

The required_unless also seems to be incorrect. As I leave both null, and its still asking for them to be confirmed.

@MGParisi MGParisi changed the title Nullable isn't working. Same should match, regardless of if its set or not. Leave the required, to required. Oct 20, 2021
@bytestream
Copy link
Collaborator

To be clear, you're saying same, nullable, and required_unless don't work?

You might wish to use #505 instead. See the usage section at the top

@MGParisi
Copy link
Author

To be clear, you're saying same, nullable, and required_unless don't work?

You might wish to use #505 instead. See the usage section at the top

I'm not sure about required_unless or nullable. Nullable will not work with same, because same applies the undefined rule above and causes the failure of the verification.

Same, should only check if the two values are the same. If their both blank, they should pass. But the original code will pass false when the target is undefined. Which seems to be the issue. So Same fails, regardless of nullable or required.

@MGParisi
Copy link
Author

Am I wrong to assume the solution #505 is to essentially do away with this solution? This was working without JS Validation. It only fails with your plugin enabled. Would hate to have to ditch this plugin.

@bytestream
Copy link
Collaborator

The PR stops using client-side validation and uses jquery validation to do server side validation before the form is submit. The main disadvantage is that you lose the instant validation on key press / changing between fields. The biggest advantage is that it uses Laravel validation server-side so you don't have to worry about broken / in-complete / inconsistent client side rules.

@MGParisi
Copy link
Author

Ok, warning though. The solution above is not correct. And doesn't fix it.

@bytestream
Copy link
Collaborator

Closing with reference to #505 (comment)

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