Skip to content

Instantly share code, notes, and snippets.

Embed
What would you like to do?
# 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
@BGuimberteau

This comment has been minimized.

Copy link

commented Apr 7, 2014

It's possible to add ProjectScoped ?

I found this way clear.

@Tonkpils

This comment has been minimized.

Copy link

commented Apr 7, 2014

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

@dhh

This comment has been minimized.

Copy link
Owner Author

commented Apr 7, 2014

@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.

@dhh

This comment has been minimized.

Copy link
Owner Author

commented Apr 7, 2014

@Tonkpils, yes, updated.

@zamith

This comment has been minimized.

Copy link

commented Apr 7, 2014

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

@ntl

This comment has been minimized.

Copy link

commented Apr 7, 2014

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.

@dhh

This comment has been minimized.

Copy link
Owner Author

commented Apr 7, 2014

@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.

@anildigital

This comment has been minimized.

Copy link

commented Apr 7, 2014

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

@BGuimberteau

This comment has been minimized.

Copy link

commented Apr 7, 2014

@dhh thanks for explanation.

@atrauzzi

This comment has been minimized.

Copy link

commented Apr 7, 2014

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.

@dhh

This comment has been minimized.

Copy link
Owner Author

commented Apr 7, 2014

@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.

@atrauzzi

This comment has been minimized.

Copy link

commented Apr 7, 2014

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.

@mcmorgan

This comment has been minimized.

Copy link

commented Apr 7, 2014

@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?

@nambrot

This comment has been minimized.

Copy link

commented Apr 7, 2014

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

@dhh

This comment has been minimized.

Copy link
Owner Author

commented Apr 7, 2014

@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.

@pixeltrix

This comment has been minimized.

Copy link

commented Apr 7, 2014

@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
@kenn

This comment has been minimized.

Copy link

commented Apr 7, 2014

@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?

@dhh

This comment has been minimized.

Copy link
Owner Author

commented Apr 16, 2014

@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.

@seanpdoyle

This comment has been minimized.

Copy link

commented Apr 18, 2014

@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?

@jakubstraka

This comment has been minimized.

Copy link

commented May 27, 2017

@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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.