GIANT ROBOTS SMASHING INTO OTHER GIANT ROBOTS

Written by thoughtbot

Regulators!!! Mount up

In my constant quest to REST up web apps and create a better world for our children and our children's children, I've started to use nested controllers to create a local namespace/hierarchy in the controllers of our Rails apps. The pattern here is simple. Let's say you have a User model and an Article model. User has_many articles. You need to have both a global all articles page and a user-specific articles by this user page.

Clear black night

Your routes will look like something like this...

map.resources :users, :has_many => :articles
map.resources :articles

Then in articles_controller you might have an #index action like...

class ArticlesController < ActionController::Base
  def index
    if params[:user_id].blank?
      @articles = Article.paginate :all, :page => params[:page]
    else
      @articles = current_user.articles.paginate :all, :page => params[:page]
    end
  end
end

Clear white moon

Now, that's not that bad, there's only one conditional, but this is a pretty simple example. Using the nested approach to create a namespace, we would do this in the routes...

map.resources :users { |users| users.resources :articles, :controller => 'users/articles' }
map.resources :articles

The :controller option is going to tell all of users/:user_id/articles/.... routes which previously went to the top-level ArticlesController (in app/controllers/articles_controller.rb) to go to the Users::ArticlesController (which is found in app/controllers/users/articles_controller.rb) instead. So now to handle the global and user article listings we'll have two controllers, each with an index action.

class ArticlesController < ActionController::Base
  def index
    @articles = Article.paginate :all, :page => params[:page]
  end
end

class Users::ArticlesController < ActionController::Base
  def index
    @articles = current_user.articles.paginate :all, :page => params[:page]
  end
end

You could argue that this ends up being more lines of code (you would be right, barely!) and that it ends up being more files (you are completely right!), and I won't disagree. I'm comfortable with both of those things, and I think the gain you get from moving the conditional logic to the routes and out of the controller (not to mention the corresponding simplification of the tests) makes it worth it. Also, this is a very simple example which only has one action, and one level of nesting, and one modeled association. I've found that the benefit gained from this pattern only goes up as the scenario gets more and more complex.

Handy with the steel

Now, to the actual point of this blog post! If you are embracing the above pattern, and you have singular resources, make sure that you do not use a singular resource name that is the same as any model you have. I was recently applying this pattern to an application which has User, Blog and Post models. User has_one blog. Blog has_many posts. Post belongs_to blog. I attempted to use routes like:

map.resources :users, :as => :people do |users|
  users.resource :blog, :controller => 'users/blog' do |blog|
    blog.resources :posts, :controller => 'users/blog/posts', :collection => { :list => :get } do |posts|
      posts.resources :comments, :controller => 'users/blog/posts/comments'
      posts.resource :tags, :controller => 'users/blog/posts/tags'
    end
  end
end

# Creates routes like:
# PUT /people/:user_id/blog/posts/:post_id/tags => Users::Blog::Posts::TagsController#update
# GET /people/:user_id/blog => Users::BlogController#show

After this was in place, and controllers like Users::Blog::Posts (in app/controller/users/blog/posts_controller.rb) existed, things seemed to be going according to plan, but then the development team started to get different errors, on different machines, some of the time. Actually, that's not giving it enough credit - many of the errors were consistently repeatable on one person's machine but not at all on someone else's (we attempted to resolve by comparing ruby versions and so on, but never arrived at a conclusion). We encountered four separate problems:

  • While running functional tests, get a ActionController::NonInferrableControllerError, which basically means that the functional test cannot figure out which controller you are actually trying to test. Even using the #tests method to specify the controller did not resolve this. We were able to fix this by explicitly requiring the controller in the functional test file.
  • RoutingError in functional tests, because even though the test class and controller class were matching up and loading correctly, the controller wound up being incorrect when it was actually tested.
  • There were places where we could not reference the Blog constant correctly so a simple Blog.find couldn't be used, and we had to do ::Blog.find instead to use the right Blog.
  • Warning from ruby about top level constants conflicting.

213 will regulate

So, in this particular example the issue was that since we already had a Blog model class, the implicit Users::Blog::Posts module that was created by that controller was colliding with that top level Blog constant from the model. The solution was pretty simple - we just had to rename the Blog in there to Blogs, and adjust the corresponding controllers. The general rule to take from this is dont use modules to create namespaces in controllers that are also names of pre-existing constants from other class definitions. This is probably the largest hiccup I've found using this approach, and once we actually understand what was happening, it was a pretty straightforward fix.

We're using this pattern in an application with 107 controller classes that have ~350 actions between them - and having the controllers organized into separate directories on the filesystem and separate namespace/hierarchy in the codebase definitely makes the application more manageable.