CODEWITHSUNDEEP
ruby-on-railsactiverecordeager-loading
Ernesto G
Ernesto G

Reputation: 545

How to avoid N+1 in this situation

I'm trying to implement a "liking" system in my app. I render a table with orders, then the current user is able to "like" an order so she will get notifications when the status of the order changes. The problem is that I'm in an N+1 issue, since each time the table gets rendered, the program makes as many queries as orders are displayed to detect if an order has already been "liked" by the user.

I have read that this can be avoided by using "includes" to eager load the associated records, but I can't wrap my head around how to do it, particularly in my case.

I have these models and associations:

user.rb Did I include the likes? the method which is triggering the N+1 alert:

class User < ApplicationRecord
  devise :database_authenticatable, :recoverable, :rememberable, :trackable, 
  :validatable
  has_many :likes

  def likes?(order)
    order.likes.where(user_id: id).any?
  end
end

like.rb

class Like < ApplicationRecord
  belongs_to :user
  belongs_to :order
end

order.rb

class Order < ApplicationRecord

 has_many :likes
 .
 .
 .

For each row of the table I render this partial to show if the order is liked or not:

<% if current_user.likes?(order) %>
  <%= link_to "<i class='fa fa-fire fa-2x fa-like'></i>".html_safe, 
  order_like_path(order), method: :delete, remote: true %>
<%else%>
  <%= link_to "<i class='fa fa-fire fa-2x fa-unlike'></i>".html_safe, 
  order_like_path(order), method: :post, remote: true %>
<%end%>

This is the query:

Rendered orders/_likes.html.erb (135.5ms)
Like Exists (0.5ms)  SELECT  1 AS one FROM "likes" WHERE "likes"."order_id" 
=$1 AND "likes"."user_id" = $2 LIMIT $3  [["order_id", 7875], ["user_id", 
1], ["LIMIT", 1]]

EDIT. I add the index action in case it is useful:

  def index
    orders = request.query_string.present? ? Order.search(params, 
    current_user) : Order.pendientes
    if params[:button] == 'report'
      build_report(orders)
    else
    @orders = orders.order("#{sort_column} # 
    {sort_direction}").page(params[:page]).per(params[:paginas])
    end
  end

Upvotes: 1

Views: 113

Answers (3)

Jignesh Gohel
Jignesh Gohel

Reputation: 6552

class User < ApplicationRecord
  has_many :likes

  has_many :liked_orders, through: :likes, class_name: 'Order'

  def liked_orders_id
    @liked_orders_id ||= liked_orders.pluck(:id)
  end

  def liked_order?(order_id)
    liked_orders_id.include?(order_id)
  end
end

The root cause behind your problem to me seems to be in the way you have implemented the likes?(order) method in User model

  def likes?(order)
    order.likes.where(user_id: id).any?
  end

Every-time you invoke this method on a loaded User, it first loads Order instance, then on that loaded Order, loads its associated Like instances and on those loaded Like instances applies the user_id filter.

Update

The liked_orders association should be defined as

  has_many :liked_orders, through: :likes, source: :order

Upvotes: 2

Vasilisa
Vasilisa

Reputation: 4640

It is in the OrdersController show or index action, right? You need to redefine instance variable like this:

@orders = current_user.orders.includes(:likes)
or 
@order = current_user.orders.find(params[:id]).includes(:likes)

And move likes? method to the Order model (change it liked_by for example).

def liked_by?(user)
  likes.where(user_id: user.id).exists?
end

In the view you'll have

<% if order.liked_by?(current_user) %>

In this case likes will be preloaded and you avoid N+1 issue.

It is a good idea to add bullet gem to the app, it will warn you about N+1 queries and give suggestions about includes

UPDATE:

Just add includes to the existing @orders

@orders = orders.includes(:likes).order("#{sort_column} # 
    {sort_direction}").page(params[:page]).per(params[:paginas])

Upvotes: 0

Deepesh
Deepesh

Reputation: 6418

What generally I do in this situation is that as you have the orders already on the view and you have the user so I fetch:

likes = current_user.likes.where(order: orders)
liked_order_ids = likes.pluck(:order_id)

And I will pass the liked_order_ids each time to the _likes partial and check liked_order_ids.include?(order.id)

I have not fetched the user.likes directly because there may be many orders he has liked and all are not present on the current page. If they are you can fetch them directly like this:

liked_order_ids = current_user.likes.pluck(:order_id)

So this way it will not execute any new queries or cached queries too.

The way you are trying to do is search in the order likes, so going through order object. Instead you have the user through which you can find the likes as it belongs to him too. As order will always be multiple and user will be single it will execute a single query to find it instead of searching multiple times in database using order.

Obviously there are many more ways around it. The choice will depend on you and your situation.

Upvotes: 2

Related Questions