views:

57

answers:

2

I'm implementing a Blog with Post and votable Comments.

When loading a Post, I want to eagerly load all votes by the current user for the Post's Comments.

Something like this (which doesn't work):

@post.comments.all(:joins => :votes, :conditions => ['votes.user_id = ?', current_user.id])

Each Comment has a method called rated_by?

def rated_by?(actor)
  votes.find_by_user_id(actor.id)
end

The problem is that ActiveRecord will run a query for each rated_by? call, even though my @post.comments finder joined all the relevant votes.

I had a look at the act_as_rateable plugin but it has the same problem, running a query for each record, not using joins.

+1  A: 

you need to use

:include => :votes

joins doesn't load your data, it just join the query in the db.

amikazmi
+1 - Wish I had been on fast enough to answer this :-)
Topher Fangio
The problem with this solution is that it loads ALL VOTES, not just the one by the user. If a post has 100 votes, that could be a problem. It should load just the user's one vote.
Alexandre
+1  A: 

Double Secret Edit: I was answering another question and came across something that should work for you. It's a bit of a crazy hack involving the Thread.current global hash. And probably not advised at all, but it works.

It involves creating a second has_many votes association on Comments

class Comment < ActiveRecord::Base
  has_many :votes
  belongs_to :post
  has_many :current_user_votes, :class_name => "Vote",
    :conditions => '`#{Vote.table_name}`.user_id = \
    #{Thread.current[:current_user].id}'
end

It also requires you to set Thread.current[:current_user] = current_user in the controller where you're going to be calling these methods.

Then you should be able to do

@post.comments.find(:all, :include => :current_user_votes)

To get a list of comments, that have eager loaded only the :current_user_votes. All in one query. If you're getting multiple posts at once, you can do this.

Post.find(:all, :include => { :comments => :current_user_votes},
  :conditions => ...)

Which will populate a list of posts, and eager load their comments which in turn will each have their current_user_votes eager loaded.

Original Answer (preserved for posterity)

I don't think it's possible to select all of one model eager load only the relevant associations in one query.

The best you're going to get is pretty much what you've done. Select all of one model and then for each them load only the relevant association with a named scope or finder.

This statement that doesn't work is only selecting comments the user has voted on.

@post.comments.all(:joins => :votes, 
  :conditions => ['votes.user_id = ?', current_user.id])

This statement selects the same set of comments, but also eager loads all votes for the comments it selects.

@post.comments.all(:include => :votes, 
  :conditions => ['votes.user_id = ?', current_user.id])

Really what you're going to have to do is call rated_by? on each comment. You might be able to minimize database impact by using a named scope. But I honestly don't think it's going to make an improvement.

If you're so worried about hitting the database so hard you could do something like this:

class Post < ActiveRecord::Base
  has_many :comments
  has_many :votes, :through => :comments
  ...
end

class Vote < ActiveRecord::Base
  belongs_to :comments
  ...

  named_scope :made_by_user, lambda {|user| 
    {:conditions => {:user_id => user}}
  }
end

@users_votes = @post.votes.made_by_use(current_user)

@comments = @post.comments.find(:all, :include => :votes)

@comments.each{|comment|
  user_voted_this_on_this_comment = comment.votes & @user_votes
  ...
}

Honestly I don't think it's worth the effort.

P.S. There's a Ruby convention regarding methods names that end in a question mark should always return a boolean value.

EmFi
Thanks for the reply EmFi. The thing is I want all Comments, but I will only display the Vote link if the user hasn't yet voted.
Alexandre
Basically I need to set a condition on the :include, as in only include Votes by the user, but still return all Posts.
Alexandre
I don't think that's possible, without using straight SQL and encapsulating the results in the ActiveRecord objects yourself. I was editing my answer to reflect that when you commented. However, the new version has a bit of a workaround.
EmFi
Would 1 call per Post to fetch the vote for the current user be a scalable approach if on any one page there are 30 posts and so 30 queries of the type "SELECT * FROM votes WHERE user_id = ?' ?
Alexandre
Honestly, I'm not sure. You could work things so there's one query per page by crafting a custom SQL query. Personally I think you're being paranoid about optimization. It's one thing to design with optimization in mind. It's another to halt your progress until you're sure you've optimized everything.
EmFi
I freaked out when I saw all these SQL calls. :P Thanks for the guidance.
Alexandre
I was answering another question, when I figured out the way to do exactly what you're looking for. Answer updated.
EmFi
Why is it so not recommended? Is it worst than running tens of sql queries?
Alexandre
Only that it's relying on an internal aspect of the rails implementation. There's no guarantee that it won't change between Rails releases.
EmFi
Is there no other way to pass the request state?
Alexandre
None that I know of. Good luck finding one. Keep in mind just because it relies on internal implementation doesn't meant that it will change. Just that it could during a core upgrade.
EmFi