Rails idioms considered harmful
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 INSERT
s 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.