has-friendship / has_friendship

Add friendship to ActiveRecord models
MIT License
198 stars 98 forks source link

How to check if a user is friends with a specific user? #5

Closed jordansinger closed 9 years ago

jordansinger commented 9 years ago

I'd like to return a boolean in a search query for each user if the current user is friends with that user. Possible with some .includes and .where?

sungwoncho commented 9 years ago

I added an instance method friends_with? for Friendables (https://github.com/sungwoncho/has_friendship/pull/6). Upgrade the gem to use it.

jordansinger commented 9 years ago

Thanks for creating the method, I have a few questions and potentially an issue as well.

First of all, if I check current_user.friends_with?(user) when they are not friends, it returns false (which is correct), but when I flip it so that user.friends_with?(current_user) when they are not friends, it returns true (which is incorrect), but if they are in fact friends it returns the correct value, so I think there may be some issue in the logic, because that should return the same value in both scenarios.

Secondly, I'd appreciate if I could get some help on improving a query that's making multiple SELECT calls.

I have the following code:

<% User.all.each do |user| %>
    <%= current_user.friends_with?(user) %>
<% end %>

This returns the correct friends_with value when it's checking the current_user against an arbitrary user, but like I said above, when flipped (<%= user.friends_with?(current_user) %>), it returns true always. So my question is how can I, if at all possible, avoid querying the friendships table every single iteration and do some sort of .includes or .preload in order to not have to hit the database every iteration.

So instead of:

  HasFriendship::Friendship Load (1.5ms)  SELECT `friendships`.* FROM `friendships`  WHERE `friendships`.`friend_id` = 1 AND `friendships`.`status` = 'accepted'
  HasFriendship::Friendship Load (0.8ms)  SELECT `friendships`.* FROM `friendships`  WHERE `friendships`.`friend_id` = 2 AND `friendships`.`status` = 'accepted'
  HasFriendship::Friendship Load (0.3ms)  SELECT `friendships`.* FROM `friendships`  WHERE `friendships`.`friend_id` = 3 AND `friendships`.`status` = 'accepted'
  HasFriendship::Friendship Load (0.3ms)  SELECT `friendships`.* FROM `friendships`  WHERE `friendships`.`friend_id` = 4 AND `friendships`.`status` = 'accepted'
  HasFriendship::Friendship Load (0.2ms)  SELECT `friendships`.* FROM `friendships`  WHERE `friendships`.`friend_id` = 5 AND `friendships`.`status` = 'accepted'
  HasFriendship::Friendship Load (0.2ms)  SELECT `friendships`.* FROM `friendships`  WHERE `friendships`.`friend_id` = 6 AND `friendships`.`status` = 'accepted'

it would be more like

HasFriendship::Friendship Load (0.4ms)  SELECT `friendships`.* FROM `friendships`  WHERE `friendships`.`friendable_type` = 'User' AND `friendships`.`friendable_id` IN (1, 2, 3, 4, 5, 6)

Thanks Sung!

sungwoncho commented 9 years ago

I fixed the #friends_with? logic and pushed the patch to devel branch.

By the design of your code, I don't think it's possible to dial it down to one query, because it is iterating through every user and calling #friends_with on each of them. Maybe you can save a reference to current_user.friends collection and check by calling #include?(user).

@friends = current_user.friends
...
<% User.all.each do |user| %>
    <%= @friends.include?(user) %>
<% end %>

Let me know if you have a better way.