On Sat, Mar 15, 2008 at 5:43 PM, Ben Mabey <[EMAIL PROTECTED]> wrote:
> Zach Dennis wrote:
>  > Can someone change this implementation and still have your tests pass,
>  > but have the implementation be broken? If they can then yes it is
>  > worth the 40 lines.
>  >
>  > Pat mentioned that he uses very skinny controllers and does
>  > integration tests which go through the whole stack to make sure things
>  > work. So if he doesn't test the controller#action's because they are
>  > too simple he at least will have a failing integration test if someone
>  > breaks the implementation.
>  >
>  > The problem I have with what Pat is doing is that it takes a lot of
>  > discipline from the developer to not let the controller grow into a
>  > cesspool of logic and interaction that shouldn't be there, but you put
>  > it there because you aren't "testing" it directly. From what I've seen
>  > most Rails apps have important interaction in the controller actions
>  > and logic in the before filters. I would not personally take this
>  > approach on customer paid for software. Pat may be more disciplined
>  > then me though. =)
>  >
>  >
>
>  I totally agree with this point.  Using interaction-based testing really
>  helps in forcing the logic down into the model.
>
>  I'm curious about your comment: "From what I've seen most Rails apps
>
> have important interaction in the controller actions
>  and logic in the before filters".  What are you exactly referring to?  I
>  generally use before filters for authentication... How have you seen
>  them abused?

I've seen before filters be abused. Authentication is typical an
application level responsibility and it makes sense for the controller
to protect its resources. No qualms there. I have inherited code bases
which abused before filters for things like checking permissions for
business requirements. For example:

  class ProjectsController < ApplicationController
     before_filter :login_required # i'm ok with this
     before_filter :ensure_project_manager, :only => [:new, :create]
     before_filter :ensure_project_write_access, :only => [:edit, :update]
     # ... I've seen this filter list go on and on and on, based on
any possible persmission

I would rather introduce a Manager/Service object which is responsible
for accessing a project, and allow that do handle the responsibility
for knowing what permissions allow someone to create or edit a
project.

I've been experimenting with using an approach similar in spirit to
what Pat is doing, but slightly different:

  class ApplicationController < ActionController::Base
    rescue_from AccessDenied, ResourceNotFoundError do |exception|
      redirect_to "/access_denied.html"
    end
    # ...
  end

  class ProjectsController < ApplicationController
    before_filter :login_required

    def new
      @project = ProjectManager.new_project
    end

    def create
      ProjectManager.create_project params[:project] do |project_creation|
        project_creation.success do |project|
          flash[:notice] = "Project created successfully"
          @project = project
        end

        project_creation.failure do |project|
          flash[:notice] = "Project creation failed"
          @project = project
          render :action => "new"
        end
    end
    # ...
  end

I tend to like this for projects I've worked on where it doesn't make
sense to shove all of the responsibility onto a model because it
abstracts out the "Manager" who is in charge of ensuring permissions
and doing odds and ends things involved in the creation of a project
that the model should not be doing (like upload an associated picture
of the project manager or ensure memberships status).

In the above approach the application controller is setup to rescue
from certain application exceptions, and the controllers just do their
thing, and the managers which enforce the business logic will throw
the exceptions it a business rule has been violated.

-- 
Zach Dennis
http://www.continuousthinking.com
_______________________________________________
rspec-users mailing list
rspec-users@rubyforge.org
http://rubyforge.org/mailman/listinfo/rspec-users

Reply via email to