An alternate approach to Avdi’s method of slimming down Rails controllers

I recently watched/read Avdi’s fantastic article Slimming down hefty Rails controllers AND models using domain model events on Ruby Tapas. PS – you should probably subscribe to Ruby Tapas.

While being amazed by Avdi’s refactoring skills and watching him plug in proven software development patterns, I couldn’t help but think about how I would have approached this problem.

Recently, I’ve been trying to be extremely adherent to “The Rails Way.” I’ve been using as few gems as possible, and just working with what Rails gives you out of the box. It’s been an eye-opening experience to say the least.

If you haven’t yet watched the episode, or read the accompanying article, essentially the problem is a long update method in the controller for a project management app. It has a bunch of nested conditionals with methods to send out updates via email or Websockets depending on what has been updated. Because so much of this depends on the logged-in user and other session-specific variables, breaking the functionality out into a “domain object” didn’t make a ton of sense.

What Avdi observed (pun intended) is that these are actually life-cycle events for a task and associated project.

Looking over this action again, we realize something: hidden in all these conditionals is a series of domain-model lifecycle events, each with different concrete actions which trigger on those events.

  1. There are some actions to perform anytime the task is successfully updated.
  2. There are actions to take when the task has moved from one project to another.
  3. There are actions to perform when the task is newly created.
  4. There are actions that happen when the task’s status has changed.
  5. There are actions for when the task has been reassigned.

He then proceeded to add observers to the Task model and subsequently send out notifications to listeners whenever a life-cycle event occurred.

Immediately, I see a lot more objects here. I see a Move, a StatusChange, and a Reassignment; all resources related to a Task. Only a traditional updating of attributes belongs in this method and controller.

As an aside, I’m not exactly sure why handling the case of a newly created task is in the update action of the controller instead of create, but I’ll assume there is something here that we don’t know. For simplicity, I’m just going to skip it.

Let’s start with some routes:


resources :tasks do
  resource :move, only: [:create]
  resource :status_change, only: [:create]
  resource :reassignment, only: [:create]
end

Now, instead of just hitting the update action, anytime something happens with a Task, we’ve got a very distinct route for each special case.

Because the controller in the example is just returning JSON, I’m going to assume that this is a Javascript front-end communicating with the Rails app. This would require a change in the front-end logic to POST to a subresource url instead of a PUT to the task url.

Here’s our first controller to handle the case of when a Task moves from one project to another.


# app/controllers/tasks/moves_controller.rb

class Tasks::MovesController < ApplicationController
  def create
    old_project_id = @task.project_id

    if @task.update_attributes params.require(:task).permit(:project_id)
      push_project_update old_project_id
      push_project_update @task.project_id

      respond_to do |format|
        format.json { render "show", status: :accepted }
      end
    else
      respond_with @task do |format|
        format.json { render @task.errors.messages, status: :unprocessable_entity }
      end
    end
  end
end

Now, let’s examine what needs to happen when a status change occurs.


# app/controllers/tasks/status_changes_controller.rb

class Tasks::StatusChangesController < ApplicationController
  def create
    old_status = @task.status

    if @task.update_attributes params.require(:task).permit(:status)
      notifee = @task.readers(false) - current_user
      if notifee
        mail_completion_notice(notifee) if @task.status == Status::COMPLETED
        mail_uncomplete_notice(notifee) if old_status == Status::COMPLETED
      end

      respond_to do |format|
        format.json { render "show", status: :accepted }
      end
    else
      respond_with @task do |format|
        format.json { render @task.errors.messages, status: :unprocessable_entity }
      end
    end
  end
end

I’ll leave it to the reader to come up with their own implementation of a Tasks::ReassignmentsController but I think you can see how this method works.

I’d also challenge you to start thinking very strictly in terms of REST and “The Rails Way.” This codebase is the perfect example that rarely does an app just have basic CRUD needs. More often, there are special cases, and paths that require a different response despite technically falling under one of the basic CRUD actions.

Think about updating a User in your own codebase. Does the same thing happen when they update their password as when they update their name? Or, are you sending a email in once case and not in the other?

Instead of nesting conditionals inside of your update action in your UsersController could their be a Users::PasswordsController? Start flushing out more and more sub-resources with small controllers for these special cases.

It may not follow the classic patterns, but I’ve enjoyed this method of thinking recently.

I’d love to hear what you think as well!