Uniqueness validations #46
Conversation
README.md
Outdated
| Also, note that as of now the `ActiveStash::Validations` module only supports a very simple uniqueness validation | ||
| and doesn't yet support conditions or any option other than `:message`. |
There was a problem hiding this comment.
Ah, that's going to be insufficient for the initial use-case I'm after this feature for.
coderdan
force-pushed
the
uniqueness-validations
branch
from
88c945f
to
e0f00dd
Compare
|
|
||
| Instead, you can include `ActiveStash::Validations` and your uniqueness validation will now work via a query | ||
| to the CipherStash index on the validated field. | ||
| If there is no index on the field then validations will fail. |
There was a problem hiding this comment.
Does this mean that including ActiveStash::Validations overrides all ActiveRecord uniqueness validations? Say, if I have two fields, one encrypted and the other one unencrypted, with (separate) uniqueness validations on each, will that still work?
There was a problem hiding this comment.
Not sure. Will look into that.
There was a problem hiding this comment.
I'll add a test and check this out.
There was a problem hiding this comment.
@mpalmer @coderdan I tested this out, and ActiveStash::Validations overrides all ActiveRecord uniqueness validations.
In this example, first_name doesn't have a stash_index applied, and has a uniqueness validation.
The error returned is expected no Exception, got #<RuntimeError: No indexes available for 'first_name'> with backtrace:
There was a problem hiding this comment.
I'll try something like the below pseudo code and see if I can get it working.
module ClassMethods
def validates_uniqueness_of(*attr_names)
# if stash index available for attr_name
validates_with ActiveStash::Validations::UniquenessValidator, _merge_attributes(attr_names)
# else
validates_with UniquenessValidator, _merge_attributes(attr_names)
# end
end
endThere was a problem hiding this comment.
This issue has been fixed with this change
edbcba8
| @@ -50,6 +50,11 @@ def order(*args) | |||
| self | |||
| end | |||
|
|
|||
| def exists? | |||
| self.load | |||
There was a problem hiding this comment.
Isn't this redundant? I was under the impression that #first called #load itself.
There was a problem hiding this comment.
Supposedly, yes but it wasn't working without this line. I'll double check but something weird with lazy loading.
mpalmer
dismissed
their stale review
So that this PR drops back to "Review Required"
spec/support/user.rb
Outdated
| # Used for testing | ||
| attr_accessor :skip_validations | ||
|
|
||
| validates :email, uniqueness: true, if: Proc.new { |user| user.perform_validations? } |
There was a problem hiding this comment.
| validates :email, uniqueness: true, if: Proc.new { |user| user.perform_validations? } | |
| validates_uniqueness_of :email, if: Proc.new { |user| user.perform_validations? } |
Finally figured out WTF is going on in Devise. Turns out that validates_uniqueness_of is what Devise is calling, and that doesn't end up getting turned into a validates uniqueness: true call, but instead does something totally different. NFI what, yet, but this at least gives us a failing test case to work from.
There was a problem hiding this comment.
@mpalmer Dan and I paired on this.
We fixed this by adding in this code to override the validates_uniqueness_of method to point to ActiveStash validations.
I've pushed this change up, and the existing test is passing, as the ActiveStash validation return string of failed: Email already exists" is being returned now.
fimac
force-pushed
the
uniqueness-validations
branch
5 times, most recently
from
a8a7d09
to
2dd1c74
Compare
… using ActiveStash validation
fimac
force-pushed
the
uniqueness-validations
branch
from
2dd1c74
to
4b464fc
Compare
No description provided.