views:

317

answers:

2

I have 4 models, Message, Group, User, Membership

class Group < ActiveRecord::Base
  has_many :memberships
  has_many :users, :through => :memberships
  has_many :groups_messages
  has_many :messages, :through => :groups_messages, :order => "created_at desc"

  named_scope :with_memberships, :include => :memberships

end

class Membership < ActiveRecord::Base
  belongs_to :user
  belongs_to :group
end

class User < ActiveRecord::Base
  has_many :memberships
  has_many :groups, :through => :memberships
end

class Message < ActiveRecord::Base
  has_and_belongs_to_many :recipients, :class_name => "User"
  has_many :groups_messages
  has_many :groups, :through => :groups_messages

  def accessible
   self.groups.with_memberships.map(&:user_ids).include?(User.current.id)
  end

end

Message can be posted to a Group, and if current User is a member of the group, he has a right to read it.

I am trying to check whether user is a member of the group with Message.accessible, but it produces one query to much:

 Group Load (0.1ms)   SELECT `groups`.* FROM `groups` INNER JOIN `groups_messages` ON `groups`.id = `groups_messages`.group_id WHERE ((`groups_messages`.message_id = 381)) AND ((`groups_messages`.message_id = 381)) 
 Membership Load (0.1ms)   SELECT `memberships`.* FROM `memberships` WHERE (`memberships`.group_id = 1) 
 User Load (0.1ms)   SELECT `users`.id FROM `users` INNER JOIN `memberships` ON `users`.id = `memberships`.user_id WHERE ((`memberships`.group_id = 1))

I don't need User Load query - user_id is contained in Membership, so last query is useless.

I tried changing accessible method to

 def accessible
   self.groups.with_memberships.exists?(:user_id=>User.current.id)
 end

But then it tries to use user_id in Group Load query and fails of course. How can i get rid of the last query? Rails 2.3.2

A: 

I think that query on users is caused by the call to User.current.id. Can you try passing in a parameter to the accessible method?

def accessible_to( user )
  self.groups.with_memberships.map(&:user_ids).include?(user.id)
end

I suppose this will be called from a controller, where current_user is (by convention) set, so in some controller action you'd do something like:

@message.accessible_to(current_user)
hgimenez
unfortunately, it's not the case. User.current.id is set way before(but I tried changing it just in case - didn't work).Please notice that the useless query loads users which have group memberships. In provided case, Message belongs to Group with id=1, so it loads users which are members of Group with id=1.
Pavel K.
Good point. What if you ask the user for their messages, instead of the messages that are accessible to a user: current_user.groups.messages
hgimenez
A: 

after understanding rails better, i realized that it was happening because of map(&:user_ids) in

def accessible
  self.groups.with_memberships.map(&:user_ids).include?(User.current.id)
end

this behaviour was absolutely normal, my mistake was that i expected it to behave like groups.membership_user_ids, and it was behaving as groups.user_ids

fixed by changing to

def accessible
 Membership.exists?(["user_id=? and group_id in (?)",User.current.id,self.group_ids])
end

actually the part include?(User.current.id) wasn't even working for some reason.

Pavel K.