Mongoid criteria refactor #20
Conversation
Mongoid::Criterion seems to have been dropped in Mongoid 4
Here's an example of a CHANGELOG.md entry: * [#20](https://github.com/mongoid/mongoid-scroll/pull/20): Mongoid criteria refactor - [@asgerb](https://github.com/asgerb).Generated by 🚫 danger |
|
|
||
| private | ||
|
|
||
| def raise_multiple_sort_fields_error |
There was a problem hiding this comment.
Should be a bang method I think, so raise_multiple_sort_fields_error!, for clarity that it fails loudly.
| options.sort && options.sort.keys.size != 1 | ||
| end | ||
|
|
||
| def no_sort_option? |
There was a problem hiding this comment.
Maybe flip it to sort_option??
| field.foreign_key? && field.object_id_field? ? bson_type : field.type | ||
| end | ||
|
|
||
| def bson_type |
There was a problem hiding this comment.
You should add this to mongoid-compatibility, maybe Mongoid::Compatibility::ObjectId#bson_type. Obviously fine for now here.
|
@asgerb Are you interested in joining the maintainers group? Maybe make the next release? (what's your rubygems email/name if so). |
|
Hey @dblock, thanks for the quick merge, happy that this was useful 🙂 |
|
I think you'll do fine @asgerb so this could be your first! You seem to know what you're doing otherwise :) Get a Rubygems login and let me know. I can help you avoid doing anything stupid and we have instructions for things like releasing. Otherwise it's just reviewing other people's PRs and helping in any way you can. |
|
Sounds good, I'm up for it 🙂 I'll sign up on rubygems then! |
|
OK I'm on Rubygems with a@asgerbehnckejacobsen.dk 🙂 |
|
All added @asgerb. Welcome! |
A proposal for a refactor of the mongoid criteria.
I’ve moved it from
Mongoid::Criterion::ScrollabletoMongoid::Criteria::ScrollablesinceCriterionwas removed in version 4 of mongoid.I’ve changed the
criteria = selftocriteria = dup. This allowed me to use bang methods, which provided a benefit when setting the default sort option.Otherwise I haven’t changed anything substantial about the code, but have tried to make it more readable. This means I’ve moved the named variables to method calls. I guess one downside of this is performance?
Perhaps some of the decisions are going a bit too far, looking forward to your thoughts!
I’ve also added a spec which checks that the base criteria does not change, but have kept the
cursor_criteria = criteria.dupfrom the original code. While I don’t think thedupwas needed before, it is needed now, since the named variables have been removed.