Fix range_hash sym to string conversion. Add tests. #128
Conversation
|
The new tests added in this commit fail without this fix. It is related to the change from PR #124 |
| @@ -50,7 +50,8 @@ | |||
| @chain.query = {:name => 'Josh', :email => '[email protected]'} | |||
| @chain.send(:index_query).should == {:hash_value => '[email protected]'} | |||
|
|
|||
| @chain.query = {:name => 'Josh', 'created_at.gt' => @time} | |||
| # All keys are converted to_sym in the where method, so we should pass them in query as Symbols. | |||
| @chain.query = {:name => 'Josh', 'created_at.gt'.to_sym => @time} | |||
There was a problem hiding this comment.
While passing 'name' as symbol instead of a string seems natural, "created_at.gt" seems like an extra burden on the caller. I would expect most people would just call it with "created_at.gt" in string form, which is probably what the test should express.
There was a problem hiding this comment.
Only that for the tests it has to be called this way because it writes it directly on the query object. When you use Model.where("created_at.gt" => 'whatever') it will make "created_at.gt" symbol in the where method. No one said that it has to be sym when people use them.
|
this looks good to me |
|
+1 range queries are really challenging with the symbol/string confusion in this ORM |
range_hash method was failing when the key was a Symbol.