Effective Rails - Part 2 - Hiding ActiveRecord

By JC Grubbs
about 1 year ago
views

Effective Rails - Part 2 - Hiding ActiveRecord

This is the second post in a series exploring some of the ways we can use the Rails framework more effectively when building large applications. In this edition, we'll be sticking with the ActiveRecord theme from the previous post and look at the effects of the ActiveRecord API on our controllers and views.

The Problems

If you subscribe at all to the Single Responsibility Principle (SRP) then it's likely that the saying "Skinny Controller, Fat Model" offends you a bit. I'm of the opinion that anything inheriting from ActiveRecord::Base is doomed to one purpose: database access. As soon as you fatten up your model with some tasty domain logic, it's in serious danger of choking.

Single responsibility works in two directions though. Not only do we need to ensure that a structure has one responsibility but also that its concerns do not leak out. When the rest of our application becomes entangled with an ActiveRecord class through it's spidery API we are in just as much danger.

Let's look at some code.

# app/models/order.rb
class Order < ActiveRecord::Base; end

# app/controllers/controller.rb
class OrdersController < ApplicationController

  def index
    @orders = Order.where(customer: current_customer, status: "open")
        .order("created_at DESC")
        .limit(params[:page_size]).offset(params[:page])
  end

end

How harmless could that be? We've pulled some records from the database and we're passing them to the view. DANGER, WILL ROBINSON! In this seemingly innocuous line of code we've caused some serious coupling in our system. Firstly, the controller knows what the structure of an order looks like. It knows that an order is attached to a customer, and that the field is called "customer" rather than "client", or "user". It knows that one of the statuses for an order is "open", and that it's identified by a string and not an ID. It knows that there's a created at field on the record, and that they can be paged.

The plot thickens when we realize what we're sending off to our view: an ActiveRecord::Relation.

# app/views/orders/index.html.erb
<div id="todays-orders">
  <h3>Today's Orders</h3
  <% @orders.where("created_at = ?", Date.today).each do |order| %>
    ...
  <% end %>
</div>
<div id="past-orders">
  <h3>Past Orders</h3>
  <% @orders.where("created_at < ?", Date.today).each do |order| %>
    ...
  <% end %>
</div>

The ActiveRecord API has now leaked into the controller, and yet again into our view where it can be insidiously passed to a partial. When doing this, it's tempting to be able to just call where, order, or includes right on this relation object inside the view thus committing the egregious sin of placing logic in the view.

Just to beat the horse further into the ground, let's look at what happens next in the story.

# app/controllers/admin/fulfillment_controller.rb
class FulfillmentController < Admin::ApplicationController

  def open_orders
    @orders = Order.where(customer: current_customer, status: "open")
        .order("created_at DESC")
        .limit(params[:page_size]).offset(params[:page])
  end

end

Gasp…not only are we violating SRP in the OrdersController but now we're violating Don't Repeat Yourself (DRY) in the FulfillmentController. If the structure of Order changes we now have two controllers to fix let alone whatever views we may have infected.

The Fixes

We essentially have two things to fix at this point. We need to shove the query API calls back into the model and we need to pass something to the view that's less ActiveRecordish.

The first is easy, ActiveRecord already provides a reasonable abstraction for this in the form of scopes. For example, we could do something like this:

# app/models/order.rb
class Order < ActiveRecord::Base

  def self.recent_open_orders_for_customer(customer, page, size=20)
    open.for_customer(customer).by_recency.paged(page, size).to_a
  end

  scope :open, ->{ where(status: "open") }
  scope :for_customer, ->(customer) { where(customer: customer) }
  scope :by_recency, ->{ order("created_at DESC") }
  scope :paged, ->(page, size=20) { offset(size).limit(size) }

end

# app/controllers/orders_controller.rb
class OrdersController < ApplicationController

  def index
    @orders = Order.recent_open_orders_for_customer(
      current_customer, params[:page], params[:page_size])
  end

end

Now we have a set of scopes with descriptive names and an intention revealing convenience method. The goal of our controller action becomes much clearer merely because of the method name. Further, any changes to the attributes or structure of Order are isolated within the ActiveRecord class and not sprinkled throughout the application.

One more thing to note, scopes naturally return an ActiveRecord::Relation,which is basically a lazy query that hasn't yet been executed. This is one of the issues we have with the view, in that relation objects can be chained with more query syntax. To prevent this from happening, we've added a .to_a to the end of our scope chain in the recent_open_orders_for_customer method. This doesn't prevent the view from interacting with the ActiveRecord API for each model in the array, but it definitely stops additional query behavior from being tacked on.

So how do we get those handy groups of "Today's Orders" and "Past Orders" to work with just a plain array. Well, we could just replace our where call to a find_all and pass it a block. But that's just as ugly. A better approach is to introduce a presenter.

Now, people say the word "presenter" and mean a variety of things..it's actually become something of a religious war. I'm not interested in that. I don't care whether you use a tool like Draper or Decent Exposure or simply hand roll something like I'm about to do.

# app/presenters/orders_presenter.rb
OrdersPresenter = Struct.new(:orders) do
  def todays_orders
    orders.find_all do |o|
      (Date.today..Date.tomorrow).covers?(o.created_at)
    end
  end

  def older_orders
    orders.find_all do |o| { o.created_at < Date.today }
  end
end

# app/controllers/orders_controller.rb
class OrdersController < ApplicationController

  def index
    orders = Order.recent_open_orders_for_customer(
      current_customer, params[:page], params[:page_size])
    @orders_presenter = OrdersPresenter.new(orders)
  end

end

# app/views/orders/index.html.erb
<div id="todays-orders">
  <h3>Today's Orders</h3
  <% @orders_presenter.todays_orders.each do |order| %>
    ...
  <% end %>
</div>
<div id="past-orders">
  <h3>Past Orders</h3>
  <% @orders_presenter.older_orders.each do |order| %>
    ...
  <% end %>
</div>

It's a subtle difference, but we now have much less leakage of the ActiveRecord API into our view. In fact, we could probably replace our order objects with value objects without changing our views at all.

--

The value gained by separating concerns like this is enormous. Even small changes like these can have huge wins in a large system and are well worth the effort. Next time you're interacting with an ActiveRecord class, ask yourself "Am I tying myself to ActiveRecord in a way that might hurt me down the line?".