Skip to content

add ability to handle equality rule by validate.js#2

Open
drselump14 wants to merge 2 commits intojwald1:masterfrom
drselump14:master
Open

add ability to handle equality rule by validate.js#2
drselump14 wants to merge 2 commits intojwald1:masterfrom
drselump14:master

Conversation

@drselump14
Copy link

Equality rule on validate.js requires the value of another attribute.
This PR adding ability to handle the equality rule.

Copy link
Owner

@jwald1 jwald1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @drselump14 for the PR. Line 55 and 62 needs to be changed to let

// changed some var names
validatejsParams(attributeName) {
    const rule = this.rules[attributeName]

    if (!rule) {
      return
    }
   // const won't work
    let { value } = this.attributes.get(attributeName)
    const attributes = {
      [attributeName]: value,
    }

    if (rule.equality) {
      const otherAttributeName = rule.equality.attribute
      // const won't work
      let { value } = this.attributes.get(otherAttributeName)
      attributes[otherAttributeName] = value
    }

    return [attributes, { [attributeName]: this.rules[attributeName] }]
  }

Also, I think that it should be added to the README that equality validation needs to be set in both fields. For example, password and password_confirmation, if it's only set on the password_confirmation field than the user can change the password field without getting an error.

@drselump14
Copy link
Author

Hello, thanks for your comment.

I've updated the readme for the explanation of equality password validation.

Regarding your advice:

  1. I think it's better to use const instead of let in this context. I don't see any necessity to reassign the value, so const is fine. And if we compare it with previous implementation, most of them are defined in const. So, I don't think let is suitable in this case.

  2. The issue of putting an equality rule on password is, it will validate the password field before password_confirmation field has a value which in my opinion is not good approach from UX perspective.

@drselump14 drselump14 requested a review from jwald1 August 3, 2020 03:45
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

Successfully merging this pull request may close these issues.

2 participants