documents.rb

# config/routes.rb
resources :documents do
  scope module: 'documents' do
    resources :versions do
      post :restore, on: :member
    end

    resource :lock
  end
end


# app/controllers/documents_controller.rb
class DocumentsController < ApplicationController
  include ProjectScoped

  def index
    @documents = @project.documents
  end

  def show
    @document = @project.documents.find(params[:id])
  end

  def new
    @document = Document.new
  end

  def create
    @document = @project.documents.create! document_params.merge(creator: current_person)
  end
end


# app/controllers/documents/locks_controller.rb
module Documents
  class LocksController < ApplicationController
    include DocumentScoped, ProjectScoped

    def update
      @document.lock!(current_person)
    end

    def destroy
      @document.unlock!(current_person)
    end
  end
end


# app/controllers/documents/versions_controller.rb
module Documents
  class VersionsController < ApplicationController
    include DocumentScoped, ProjectScoped

    before_action :set_version

    def show
    end

    def restore
      @document.restore!(@version)
    end


    private
      def set_version
        @version = @document.versions.find(params[:id])
      end
  end
end


# app/controllers/concerns/document_scoped.rb
module DocumentScoped
  extend ActiveSupport::Concern
  
  included do
    before_action :set_document
  end
  
  private
    def set_document
      @document = @project.documents.find(params[:document_id])
    end
end

It's possible to add ProjectScoped ?

I found this way clear.

Wouldn't including DocumentScoped set that before action already? https://gist.github.com/dhh/10022098#file-documents-rb-L36-L38

@BGuimberteau, I simplified a bit for this example. In Basecamp, we actually have BucketScoped, since many things can be a child of either a Calendar or a Project. But the base concept is the same.

@Tonkpils, yes, updated.

@dhh What about restore? What's the reasoning for creating a new controller or adding a non CRUD action?

Sometimes CRUD isn't the best vocabulary to describe what your app is doing. What else do you do in this case? "Create a restoration?" Rails apps that overly commit to CRUD 100% of the time usually have really disgusting routes, imo.

@zamith, whether there's enough there, and whether it's likely to lead to proliferation. #restore in this case could have been turned into a Documents::VersionRestorationController, but it wouldn't have lead to further clarification over what we have now.

Isn't this just an interface segregation principle applied to controllers?

@dhh thanks for explanation.

This is a nice and concise example. I do wonder if there would be situations where you'd want logic encapsulated outside request scope (ie: via service objects). This is in contrast to having functionality bound to a specific request+response context by way of being inside controllers.

Best use for this so far I've encountered is when I have a web app that also hosts it's own API, the only time I'd want my business objects being touched by controllers at that point is for output.

@atrauzzi, I've yet to see a compelling "make action a service object" example in the wild. Maybe they exist somewhere, though. Then again, maybe unicorns are real too.

If your controller action is shared between browser and API, just use respond_to for the API. Don't make it a separate controller.

That makes sense and can definitely stay clean provided the surface area of both the web site and the API are identical.

Although an API that mimics the routes of a web site may not always be the nicest to use and the needs sometimes will not be 1:1. When that divergence happens, there is some risk of duplication.

@dhh thanks for this. Since we are comparing one form to another, it would be nice to see what the controller looks like with all the actions stuffed in there. Is there a separate gist with that?

I assume ProjectScoped works similarly to DocumentScoped. In that case, does it matter in which order you include DocumentScoped/ProjectScoped?

@nambrot, yeah, same concept. It matters because ProjectScoped must be loaded first, since DocumentScoped depends on it. It's a Ruby quirk that #include loads things in reverse order, so "include DocumentScoped, ProjectScoped" will load ProjectScoped first, then DocumentScoped.

@mcmorgan, our code in Basecamp started this way, so don't have the before. Wouldn't be hard to recreate though.

@atrauzzi, when they diverge, you can't reuse the logic anyway. Per definition.

@dhh you can simplify the routes by wrapping the nested resources with a scope, e.g:

resources :documents do
  scope module: 'documents' do
    resources :versions do
      post :restore, on: :member
    end

    resource :lock
  end
end

@dhh Re: DocumentScoped / ProjectScoped order, is it a Ruby quirk or ActiveSupport::Concern's? I thought ActiveSupport::Concern saves dependencies in an Array and we could reverse it if that's what we want. Was there any reason that we don't do it that way?

@pixeltrix, yes of course. I've updated the gist to reflect that. Definitely nicer!

@kenn, that's a Ruby quirk. Concern is a module itself, it doesn't change how #include operates.

@dhh Wouldn't including ProjectScoped in DocumentScoped make the module dependency explicit, and thus allow you to remove ProjectScoped's inclusion from Documents::VersionsController and Documents::LocksController, avoiding the module ordering quirk entirely?

Also, any particular reason why you call Document.new here, instead of @project.documents.build?

@dhh would you then apply this namespacing also for models?

So document's versions would go to document folder as version.rb with class Document::Version? Same thing with locks to lock.rb to document folder with class Document::Lock.

Final tree structure would be then

models/document/version.rb
models/document/lock

class Document::Version
   # code here
end

or wrap those classes in plural module?

models/documents/version.rb
models/documents/lock

module Documents
  class Version
    # code here
  end
end