CODEWITHSUNDEEP
ruby-on-railsmailboxer
Iriel
Iriel

Reputation: 33

Mailboxer N+1 Query detected

I managed to install Mailboxer with this tutorial but i have a recurring error :

N+1 Query detected
  Mailboxer::Message => [:sender]
  Add to your finder: :includes => [:sender]
N+1 Query method call stack
  app/views/conversations/_messages.html.erb:12:in `block in _app_views_conversations__messages_html_erb__3829465222244059655_69982701059040'
  app/views/conversations/_messages.html.erb:1:in `_app_views_conversations__messages_html_erb__3829465222244059655_69982701059040'
  app/views/conversations/show.html.erb:27:in `_app_views_conversations_show_html_erb__1439517360344897040_69982700516580'

  app/views/conversations/_messages.html.erb:12:in `block in _app_views_conversations__messages_html_erb__3829465222244059655_69982701059040'
  app/views/conversations/_messages.html.erb:1:in `_app_views_conversations__messages_html_erb__3829465222244059655_69982701059040'
  app/views/conversations/show.html.erb:27:in `_app_views_conversations_show_html_erb__1439517360344897040_69982700516580'

I had another one for :message but i fixed the problem with includes. If i try to do the same with :sender, i have this error:

Association named 'sender' was not found on Mailboxer::Receipt; perhaps you misspelled it?

Tutorial original code : conversations_controller.rb

class ConversationsController < ApplicationController
  before_action :authenticate_user! 

  def new
  end

  def create
    recipients = User.where(id: conversation_params[:recipients])
    conversation = current_user.send_message(recipients, conversation_params[:body], conversation_params[:subject]).conversation
    flash[:success] = "Your message was successfully sent!"
    redirect_to conversation_path(conversation)
  end

  def show
    @receipts = conversation.receipts_for(current_user)
    # mark conversation as read
    conversation.mark_as_read(current_user)
  end

  def reply
    current_user.reply_to_conversation(conversation, message_params[:body])
    flash[:notice] = "Your reply message was successfully sent!"
    redirect_to conversation_path(conversation)
  end

  def trash
    conversation.move_to_trash(current_user)
    redirect_to mailbox_inbox_path
  end

  def untrash
    conversation.untrash(current_user)
    redirect_to mailbox_inbox_path
  end

  private

  def conversation_params
    params.require(:conversation).permit(:subject, :body,recipients:[])
  end

  def message_params
    params.require(:message).permit(:body, :subject)
  end
end

Tutorial original code : show.html.erb

<div class="row">
  <div class="spacer"></div>
    <div class="col-md-6">
    <%= link_to "Compose", new_conversation_path, class: "btn btn-success" %>
    </div>
    <div class="col-md-6 text-right">
    <% if conversation.is_trashed?(current_user) %>
        <%= link_to 'Untrash', untrash_conversation_path(conversation), class: 'btn btn-info', method: :post %>
    <% else %>
        <%= link_to 'Move to trash', trash_conversation_path(conversation), class: 'btn btn-danger', method: :post,
                    data: {confirm: 'Are you sure?'} %>
    <% end %>
    </div>
  </div>

  <div class="col-md-4">
    <div class="panel panel-default">
      <div class="panel-body">
        <%= render 'mailbox/folders' %>
      </div>
    </div>
  </div>

  <div class="col-md-8">
    <div class="panel panel-default">
      <div class="panel-body">
        <%= render partial: 'messages' %>
      </div>
      <div class="panel-footer">
        <!-- Reply Form -->
        <%= form_for :message, url: reply_conversation_path(conversation) do |f| %>
            <div class="form-group">
              <%= f.text_area :body, placeholder: "Reply Message", rows: 4, class: "form-control" %>
            </div>
            <%= f.submit "Reply", class: 'btn btn-danger pull-right' %>
        <% end %>
        <div class="clearfix"></div>
      </div>
    </div>
  </div>

</div>

Tutorial original code : _messages.html.erb

<% @receipts.each do |receipt| %>
    <% message = receipt.message %>
    <div class="media">
      <div class="media-left">
        <!-- user avators can go here -->
        <a href="#">
          <img class="media-object" src="http://placehold.it/64x64" alt="...">
        </a>
      </div>
      <div class="media-body">
        <h4 class="media-heading">
          <%= message.sender.username %> <br>
          <small><b>Subject: </b><%= message.subject %></small><br>
          <small><b>Date: </b><%=  message.created_at.strftime("%A, %b %d, %Y at %I:%M%p") %></small>
        </h4>
        <%= message.body %>
      </div>
    </div>
<% end %>

If i delete <%= message.sender.username %> the problem is solved...

Any ideas ?

Upvotes: 1

Views: 127

Answers (1)

Simple Lime
Simple Lime

Reputation: 11035

Normally, to fix these N+1 query problems we use an includes and by passing a hash, we can includes nested associations. So looking at this code, calling receipt.message.sender is triggering the error, so we have a Receipt model, a message association on it and a sender associated to that. So if we can find where we load the Receipt we can add includes(message: :user) like we would for any other model.

Digging into the mailboxer gem, the receipts_for method in your show action is just a wrapper for a couple of scopes on Mailboxer::Receipt. Since this method just runs some scopes for you, we can chain onto the end of it as if it were a normal ActiveRecord where chain.

So with all that in mind, we should be able to add on our includes and avoid the N+1 query problem, ending up with something like

@receipts = conversation.receipts_for(current_user).includes(message: :sender)

Upvotes: 1

Related Questions