Jamie Gaskins

Ruby/Rails developer, coffee addict

Rails idioms considered harmful

Published May 17, 2014

I'm not sure if it's Rails itself or the community by which it is surrounded, but something encourages us to treat our Rails apps as if they are simply a web interface to the database. Consider the idiomatic Rails create controller action:

class ThingsController < ApplicationController
  def create
    @thing = Thing.new(params[:thing])

    if @thing.save
      redirect_to @thing, notice: 'Thing was saved.'
    else
      render :new
    end
  end
end

The good news is that, in a lot of cases, this is about all you need. If you're adding a product to your e-store's catalog, publishing an article, or any other time you're just giving the app some data to be presented later, it's perfectly reasonable to treat it as nothing more than an SQL INSERT statement wrapped in a web request.

The problem I see is when people essentially copy and paste this code (whether they actually copy/paste or rewrite it with little to no modification, the end result is the same) for absolutely every create action in their app. If, instead of a ThingsController, this were a UsersController, would you still treat it as a simple CRUD controller?

The reason I'm writing this is that, at work about a week ago, someone suggested I do just that, and it's been bothering me. At the domain level, "creating a user" tells me absolutely nothing. From the user's perspective, they're not creating a user; they are the user. They're creating an account or, put another way, registering.

License, registration, and proof of email address

I've tried renaming the User class to UserAccount or simply Account in the past and it feels awkward. What if other users can be added to the same Account for payment-consolidation purposes? How do you reference the user themselves if calling the registration process "creating a user account", which simply INSERTs a record into the user_accounts table?

First, I've stopped thinking of a lot of scenarios like this as simple CRUD. When a user registers with your app, you're not simply inserting a record into the database. After the user record is saved, you'll usually send a welcome email or an email asking the user to click a link to confirm their email address. Maybe you'll add them to a mailing list or create some default data for them.

Where would you put all of that logic? If you put it into the controller, then you have to duplicate it all any time you add a user outside the normal registration process. This could happen through the admin interface, in a Rails console, a user-invitation process, etc.

A lot of apps I've worked on put that logic in after_create callbacks on the model instead. When setting up associated models, this is definitely an advantage over putting it in the controller because whether you add the user through the normal registration process or through the admin interface, the default data will be setup with no duplication. Sounds great, right? It does until you realize that every time you test a persisted user model you're inserting, at a minimum, twice as many objects into the database as necessary. We maintain an app at work that performs over 10 inserts per user because it follows this pattern (it's okay, we didn't write it like that; it came to us that way).

I consider both of these approaches to be harmful and yet both are used in countless Rails apps that people pay actual money for.

Okay, rocket surgeon, where do I put this crap?

The way I've been dealing with user registrations — and several other processes that aren't basic CRUD — by instantiating what basically amounts to a fake ActiveRecord model. You talk to it somewhat like an ActiveRecord model, but it stores the data elsewhere.

class UserRegistration
  include ActiveModel::Validations

  attr_reader :user

  validates_presence_of :email
  validates_presence_of :password
  validate :passwords_match

  delegate :email, :password, :password_confirmation, to: :user

  def initialize(attributes={})
    @user = User.new(attributes)
  end

  def self.create(attributes={})
    new(attributes).save
  end

  def save
    ActiveRecord::Base.transaction do
      if valid? && user.save
        create_defaults
        UserRegistrationMailer.welcome(user).deliver
        return user # We don't want to return the result of the transaction call
      end
    end
  end

  def passwords_match
    unless user.password == user.password_confirmation
      errors[:base] << 'Passwords do not match'
    end
  end

  def create_defaults
    user.foos.create
    user.bars.create
  end
end

This way, if you use a UserRegistrationsController that uses the idiomatic Rails create action as shown at the top of the article, it will save the user just as you would normally, insert associated models, and send out the welcome email. This makes sure that inserting a user model is as lightweight as possible (the validations are in the registration, so are not run during testing) while still allowing you to insert user records (including fully registered users) easily from elsewhere in the app, such as a Rails console.

As a bonus, if you were including other information than just the user data, such as payment information, you could pass it to the UserRegistration and sort it out there.

class UserRegistration
  # ...

  attr_accessor :user, :credit_card

  def initialize(attributes={})
    @credit_card = CreditCard.new(attributes.slice(:card_number, :expiration_date, :cvv))
    @user = User.new(attributes.slice(:email, :password, :password_confirmation))
  end

  def save
    if valid? && user.save
      payment_gateway.add user
      payment_gateway.charge user, AMOUNT
    end
  end

  def payment_gateway
    @payment_gateway ||= PaymentGateway.new
  end
end

See how in the initialize method, we split out the attributes for the separate models. Without this object to wrap the two models, you'd have to use accepts_nested_attributes_for, which is poor form. Sure, there's a little extra code to maintain, but this isn't a bad thing. Since you're doing validations here, you don't need to do them on the User model. This lets you forget entirely about bypassing validations when adding the user from the admin interface and will increase the speed of any tests that hit the database.

TwitterGithubRss