dm-validations creating the correct query?
Reported by whoahbot | June 8th, 2009 @ 03:03 PM | in 0.10.0
I'm not sure this is a bug, but an issue that came up when working with validations.
The query that uniqueness_validator creates when searching for another record with the same attribute does not include the field being validated in the :fields portion of the options hash.
In the dm-redis adapter, I don't load any fields not specified in the query to save trips back and forth to redis. The current behavior of dm-validations seems to work fine for all other adapters though, so my question is, is this improper behavior for an adapter, or should dm-validations include the field being validated?
I've attached a patch for dm-validations that fixes the issue for me and passes all tests.
Thanks!
Comments and changes to this ticket
-
Martin Gamsjaeger (snusnu) July 9th, 2009 @ 02:44 PM
- State changed from “new” to “unconfirmed”
I'm not sure if this had any impact on your problem, but you maybe want to look at
http://github.com/datamapper/dm-more/commit/fcb84b6280e6e30d1b640fc...
and see if the problem you described still exists after that.
-
Martin Gamsjaeger (snusnu) July 9th, 2009 @ 08:06 PM
- Assigned user set to “Martin Gamsjaeger (snusnu)”
- State changed from “unconfirmed” to “accepted”
- Milestone set to “0.10.0”
-
Martin Gamsjaeger (snusnu) July 11th, 2009 @ 02:40 PM
- State changed from “accepted” to “unconfirmed”
I had a look at this, applied your fork against my local copy and as you said all specs pass. However, I'm not sure why it would be necessary to fetch the field explicitly? If I interpret :fields right, then that's what gets pulled back from the storage engine? (i.e. the SELECT field_1, field_2 ... stuff in an rdbms?). The fieldname still gets merged into the query, so the right conditions should be applied. I don't see the point in fetching that field back, just for dumping it right away, after the checks if a resource was returned in the first place. Am I missing something?
-
Martin Gamsjaeger (snusnu) July 11th, 2009 @ 03:45 PM
- State changed from “unconfirmed” to “not-applicable”
To quote dkubb on IRC
I agree. dm-validations does what it should be doing for this. it does the most minimal thing that could work
I'm marking this as not-applicable
Please Sign in or create a free account to add a new ticket.
With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.
Create new ticket
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
Martin Gamsjaeger (snusnu)
whoahbot