Rails: Finding Associated Objects Safely

This one’s simple but I’ve run across it a dozen times today so I’ve decided to vent my frustration via education (I think this is healthy…right?).

You have the following simple models:

class Customer < ActiveRecord::Base
  has_many :orders
end

class Order < ActiveRecord::Base
  belongs_to :customer
end

Now, almost invariably, in your OrdersController you have a destroy method that needs to ensure that Customers can only delete their own Orders.

Assuming @customer holds the current user’s Customer record, don’t waste your time doing this:

def destroy
  order = Orders.find(params[:id])
  @customer.orders.delete(order) if @customer.orders.include?(order)
end

Instead, simply search for the record inside of the Customer’s Orders:

def destroy
  order = @customer.orders.find(params[:id])
  @customer.orders.delete(order)
end

This eliminates an extra database query and keeps us safe if we decide to add anything else to that method and fail to notice the inline check that determines if the user is actually allowed to touch that Order (in the second version we’ll immediately bomb out with an ActiveRecord::RecordNotFound exception if the record doesn’t belong to the expected Customer.)

fin.