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

Mongoid document proxy binding doesn't work with aliased updated_at field #86

Open
onomated opened this issue Jan 3, 2015 · 2 comments

Comments

@onomated
Copy link

onomated commented Jan 3, 2015

The current implementation of Garner::Mixins::Mongoid::Document::_latest_by_updated_at currently checks for the existence of the updated_at field. This doesn't cover documents that alias the updated_at field (typically by including include Mongoid::Timestamps::Short). This should consider aliased fields. The latest document query can also be optimized to return a single document. Something along these lines:

def self._latest_by_updated_at
  # Only find the latest if we can order by :updated_at
  return nil unless fields['updated_at'] || aliased_fields['updated_at'] 
  only(:_id, :_type, :updated_at).order_by(updated_at: :desc).limit(1).first
end

May serve to document the proxied binding behavior on model classes in the readme, so devs know to create indexes on the updated_at field if they so choose.

Cheers!

@onomated onomated changed the title Mongoid document proxy_binding doesn't work with aliased updated_at field Mongoid document proxy binding doesn't work with aliased updated_at field Jan 3, 2015
@dblock
Copy link
Contributor

dblock commented Jan 3, 2015

Absolutely, would love a PR!

@onomated
Copy link
Author

onomated commented Jan 3, 2015

Sounds good. I've tested locally so I'll make a push request for your review!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants