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

added original value to uniqueness validations #48

Merged
merged 3 commits into from
Jan 12, 2015

Conversation

dannysperry
Copy link
Collaborator

Added original_value to the data-validate uniqueness hash on the element
Added the original value to the proper areas in the Validator/Validation Ruby classes

Validation#validate! only runs rails validate_each method if the validation isn't a uniqueness validator that has a value equal to the original value and if the original value in those cases isn't an empty string(because we don't care about uniqueness on empty inputs)

@dannysperry
Copy link
Collaborator Author

@joecorcoran my OSS experience is limited, but its my understanding that we don't merge our own PR's. Let me know you're thoughts on how you want to handle this.

@@ -147,6 +152,9 @@
'value' : el.value,
'kind' : kind
};
if (kind == 'uniqueness') {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Only a tiny thing but I'd like to maintain the style of using === in JS when testing for equality, and using == only when you need/want the ambiguity. It doesn't affect things at all in this case but it's a good habit to develop.

@joecorcoran
Copy link
Collaborator

Aside from the little note above this looks great, thanks @dannysperry, please merge! Agree that PRs need a second look wherever possible.

dannysperry added a commit that referenced this pull request Jan 12, 2015
added original value to uniqueness validations
@dannysperry dannysperry merged commit 7324727 into judgegem:master Jan 12, 2015
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