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

Add uniqueness validation providing query #701

Merged

Conversation

matthewmcgarvey
Copy link
Member

Fixes #368
Fixes luckyframework/lucky#1482

Allows for passing a query to the uniqueness validation without specifically passing in a criteria.

If all you wanted to do was limit the uniqueness validation by a different column, before you had to do:

validate_uniqueness_of email, query: UserQuery.new.tenant(1).email

Now you can do:

validate_uniqueness_of email, query: UserQuery.new.tenant(1)

It's a small change but it cuts down on confusion and allows users to be clear on what they are actually trying to do.

Additionally

Other changes were made (I can extract them if necessary)

  1. The docs were wrong and have been fixed (on the wrong overloads)
  2. The criteria overload was missing the limit to ignore the current record if it was an update
  3. Generics were added to the criteria and new overloads to add additional type safety

Comment on lines 26 to +27
attribute : Avram::Attribute,
query : Avram::Criteria,
query : Avram::Criteria(T::BaseQuery, _),
Copy link
Member Author

Choose a reason for hiding this comment

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

I had added generics to the Attribute and the Criteria to connect them. I think it might have worked but with my recent work on enums, I'm not sure if the types of the two would be the same so I took it off to be safe.

Copy link
Member

@jwoertink jwoertink left a comment

Choose a reason for hiding this comment

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

This looks good to me! I saw your note about the generic on the Criteria. I'm not sure either if the types of the two would be the same, but it's probably fine how you have it. We will know better for sure after running through some full testing building out a sample app and stuff.

@matthewmcgarvey matthewmcgarvey merged commit 9624465 into luckyframework:master Jul 10, 2021
@matthewmcgarvey matthewmcgarvey deleted the uniqueness-validations branch July 10, 2021 17:49
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.

Confusing validate_uniqueness_of experience Change how validate_uniqueness_of query works
2 participants