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