When, a Rails plugin

Posted by Dan Croak

Feb 15

Ever write this? 
1
2
3
4
5
6
7
8
9
10

before_filter :authorize

protected

def authorize
  unless logged_in?
    session[:return_to] = request.request_uri
    redirect_to login_url and return false
  end
end
You want your before_filter to authorize the current_user unless they are logged in. So why not say it like you mean it? 
1
2
3
4
5
6
7
8

before_filter :authorize, :unless => logged_in?

protected

def authorize
  session[:return_to] = request.request_uri
  redirect_to login_url and return false
end

Paris Hilton is a fan of moving conditional logic into filters, callbacks, and validations.This is more expressive. The conditional logic is no longer hidden in the method.

The authorize method now has a single responsibility: to authorize. The before_filter, responsible for performing some action before your controller action is called, is now smarter about when it is supposed to run.


Introducing When

The When Rails plugin adds :if and :unless modifiers to before_filters, most ActiveRecord callbacks, and validations.

Get it:

piston import https://svn.thoughtbot.com/plugins/when/trunk vendor/plugins/when
Then start improving your controllers: 
1
2
3
4
5
6
7
8
9
10
11
12

before_filter :deny_access, :unless => :admin?

protected

def deny_access
  flash[:failure] = "You do not have access to that page."
  redirect_to home_url
end

def admin?
  logged_in? and current_user.admin?
end
Improve your models, too: 
1
2
3
4
5
6
7

before_create :encrypt_password,
  :unless => lambda {|user| user.password_confirmation.blank?}

after_save :send_alerts, :if => :alerts?

validate_on_create :add_unsupported_type_error, 
  :unless => lambda {|document| SUPPORTED_FILE_TYPES.include? document.file_type}

Coding without ifs?

Jared makes a strong argument that this is actually removing conditional logic from your program. I tend to agree.

It reminds me of defining constants in your environments so you don’t write conditional logic that checks the RAILS_ENV. That kind of control flow belongs in the framework.

Whether this is removing conditional logic is moot, though. It just feels right, and isn’t that why we write Ruby?

Update ... After releasing this, we found out this is already in Rails trunk. Awesome! It’s a great feature and should be in the framework. So, if you’re not running on Edge, use the when plugin to get this feature now. Then, remove it when you upgrade to the next version of Rails, when you’ll get :ifs and :unlesses “for free.” Happy coding!

Comments on this post

Josh Peek

Feb 15

Josh Peek said,

The refactoring of ActiveSupport::Callbacks adds conditionals everywhere (that uses the module). However, controller filters have not been moved to it, yet. Its kind of difficult because of the complexity of the filter logic.

Pete Forde

Feb 15

Pete Forde said,

Oh man, that’s some sweet sticky right there.

Dan Croak

Feb 15

Dan Croak said,

Josh, you’re breaking my heart! I just checked out http://svn.rubyonrails.org/rails/trunk and see what you’re talking about ...

Well, for those folks (like us) who don’t run on edge, at least we’ll have this benefit a little earlier. Also, its comforting knowing that the test coverage for this little plugin is nuts:

792 tests, 1536 assertions, 0 failures, 0 errors

Mark Wilden

Feb 15

Mark Wilden said,

Actually, the improved #authorize does more than just authorize – it saves the user’s place, purely for his convenience. That has nothing to do with authorization, but it’s the obvious place to put this code.

One could make the point that you’re now splitting the logic between #authorize and the before_filter call. What the usual “authorize” does (including this one) is handle everything to do with ensuring the user is authorized, and what to do if he isn’t, and where to go afterwards. So a better name might be #ensure_authorized.

///ark

Matt Jankowski

Feb 15

Matt Jankowski said,

Could something like this be added to #verify as well?

What if you could pass arbitrary symbols to #verify, instead of only using it’s built in conditionals? verify :logged_in?, :redirect => :login_url, :add_flash => { :failure => 'Log in first' }

For any filters which were only doing redirection and flash setting, that’s pretty straightforward.

justin

Feb 15

justin said,

That would have looked more impressive up on Github… too bad…

anonymous

Feb 15

anonymous said,

“After releasing this, we found out this is already in Rails trunk. Awesome! It’s a great feature and should be in the framework. So, if you’re not running on Edge (like us), use the when plugin to get this feature now.”

If your running edge, then why didn’t you see this in trunk?

Chad Pytel

Feb 15

Chad Pytel said,

@anonymous: I guess the wording we used in the update is ambigious, we meant it to mean that we are not running on Edge.

Keith

Feb 16

Keith said,

Sorry to be negative, but this is actually in my opinion a poor refactoring. Instead of making it clearer as you claim, you now have a misnamed method. “Authorize” now simply redirects to the login page – it does not “authorize” anything. You can rename the method to something more expressive, like “bounce_to_login” or something (not a great name either…), however in either case you are now requiring the developer reading this to look at the entire file to understand the logic. In a larger controller, where the method being called from the filter might be at the bottom, it could be easy to miss. The “unimproved” code is completely understandable at first glance. Again, at the very least, the method should have a name that says what it does.

I’m not against using conditionals in filters at all, but this particular example as written is not really an improvement.

Oleg

Feb 17

Oleg said,

I honestly don’t understand how putting complex conditionals right there on the filter improves readability of the code. :if => logged_in? might be fine but the whole lambda thing seriously should be crammed in the method.

Dan Croak

Feb 17

Dan Croak said,

@anonymous: what Chad said. I edited that line to be less ambiguous.

@Keith: Fair point. I wouldn’t be against renaming :authorize to :login if it fit the tastes of the development team.

@Oleg: In general, the lambda option is important to have in the arsenal. If what you write in the lambda is short and expressive enough, there is no need to create a method with an intention-revealing name later in the model or controller. Particularly in a fat model, you don’t want to have to bounce down 100 lines to find out what :password_confirmed? actually does. Similar to Keith’s comment, I believe taste may dictate which option (method vs. lambda) you prefer in a particular scenario.

Floyd

Feb 17

Floyd said,

Whoa, hold on, are you saying there is no objectively correct solution to the problem of method naming? Jesus, no, just always use ‘each’ or something—anything than leave it at a matter of taste.

Steve Tooke

Feb 20

Steve Tooke said,

Merb’s controller filters now have conditionals too. Inspired by this post.

See Here

Sorry, comments are closed for this article.

© 2000 - 2009 by thoughtbot, inc.
written by a bushel of tiny robots