her
her copied to clipboard
Scopes on associations
Punch a hole in the association_proxy for scopes. Also inject the parents id if needed.
Addresses https://github.com/remiprev/her/issues/215
LGTM :+1:
good job :+1:
I was hoping this would work for custom_get (which are kinda like server-side scopes) but it doesn't because of this bit of evilness in the scope method.
# Add the scope method to the Relation class
Relation.instance_eval do
define_method(name) { |*args| instance_exec(*args, &code) }
end
@remiprev, is this really necessary? Can't the relation fall back to the model's class methods in method_missing? I know ActiveRecord does this, though admittedly it is a bit messy when deciding whether methods like find should go to the collection or the model class.
:+1:
@hubert Anything I can do to help get this merged? Also, thank you for stepping up to help maintain Her.
@dturnerTS i've blocked out some time on saturday (tomorrow) to work through pending pull requests.
thanks for the kind words. happy to contribute back to the gem.
@edtjones, if you're looking at this one, I'd really appreciate having a chat with you about it.
@chewi definitely, and looks really useful. Need to absorb fully. Are there any downsides / expected failure modes which mean we should be careful about merging this in?
I'd like it to work more like it does in Active Record, where you can call any class method through an association. The code I quoted above highlights the difference between Her and Active Record. I'm a little hazy on the details after all this time but Active Record is very mature now and I think we should strive to use the same approaches wherever possible as they have clearly served it well.
Any plans on merging this PR?
hi @shashiprab @chewi this has been hanging around for ages - sorry! Don't have the mental bandwidth to deal with the implications of merging this at the moment but I would be delighted if someone else could review.
Remember happily applying this patch a few years back and eventually moving a few things around. Also have a rough patch that would let us remove collection_path
as @kinopyo pointed out.
class User
include Her::Model
has_many :comments
end
class Comment
include Her::Model
belongs_to :user
scope :approved, -> { where(approved: true) }
end
> user = User.find(1)
# GET "/users/1"
> user.comments.approved
# GET "/users/1/comments?approved=true"
I'll put a PR together soon.