This refactoring is based on a topic Marcel and I covered at RailsConf Europe.

Before

1
2
3
4
5
6
7
8
9
10
11
12
13

class Expense < ActiveRecord::Base
  belongs_to :payee
  protected
    
    # Nice and concise, but what happens as we add more rules
    # and how do we write test cases for the four different possible 
    # validation states?
    def validate
      errors.add("Not enough funds") if payee.balance - amount > 0
      errors.add("Charge is too great") if payee.account.maximum_allowable_charge > amount
    end
end

After

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37

class Expense < ActiveRecord::Base
  belongs_to  :payee
  
  # Instead of one large validation method, break each individual
  # rule into methods, and declare them here.
  validate      :ensure_balance_is_sufficient_to_cover_amount
  validate      :amount_does_not_exceed_maximum_allowable_charge

  protected

    # These validation callbacks simply add error messages if a particular 
    # condition is met.   Each of them can be tested and understood on their own
    # without having to understand the entire body of the validate method.
    def ensure_balance_is_sufficient_to_cover_amount
      errors.add("Not enough funds") if insufficient_funds?
    end
    
    def amount_does_not_exceed_maximum_allowable_charge
      errors.add("Charge is too great") if exceeds_maximum_allowable_charge?
    end
    
    # By defining separate predicate methods we can test each of them individually
    # and new programmers can see the intent of the code, not just the implementation.


    # Instead of subtracting the amount from the balance and checking if the
    # value is greater than 0,  change the implementation to mirror the intent.
    # There was a bug in the before code,  this is more obvious.
    def insufficient_funds?
      amount > payee.balance
    end
    
    def exceeds_maximum_allowable_charge?
      payee.account.maximum_allowable_charge > amount
    end
end

While the refactored version may have more lines of code, but don’t let that scare you. It’s far more important for code to be human readable than incredibly concise.

Today I’m reviewing Joe Van Dyk’s monkeycharger application, which is a web-service for storing and charging credit cards. I loved looking at this app, because its only interface is a RESTful web service: there is no HTML involved. (If you’ve never written an app that only exposes a web-service UI, you ought to. It’s a blast.)

In general, Joe has done a fantastic job with keeping the controllers slim and moving logic to models. The only significant gripe I had with the application is that it is not ActiveResource compatible.

For those of you that are late to the party, ActiveResource is the newest addition to the Rails family. It lets you declare and consume web-services using an ActiveRecord-like interface…BUT. It is opinionated software, just like the rest of Rails, and makes certain assumptions about the web-services being consumed.

1. The service must understand Rails-style REST URLs. (e.g. “POST /credit_cards.xml” to create a credit card, etc.)
2. The service must respond with a single XML-serialized object (Rails-style).
3. The service must make appropriate use of HTTP status codes (404 if the requested record cannot be found, 422 if any validations fail, etc.).

It’s really not much to ask, and working with ActiveResource (or “ares” as we affectively call it) is a real joy.

However, monkeycharger tends to do things like the following:

1
2
3
4
5
6
7
8
9
10

class AuthorizationsController < ApplicationController
  def create
    @credit_card   = Authorizer.prepare_credit_card_for_authorization(params)
    transaction_id = Authorizer::authorize!(:amount => params[:amount], :credit_card => @credit_card)
    response.headers['X-AuthorizationSuccess'] = true
    render :text => transaction_id
  rescue AuthorizationError => e
    render :text => e.message
  end
end

Three things: the request is not representing an “authorization” object, the response is not XML, and errors are not employing HTTP status codes to indicate failure.

Fortunately, this is all really, really easy to fix. First, you need (for this specific example) an Authorization model (to encapsulate both the the XML serialization and the actual authorization).

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20

class Authorization
  attr_reader :attributes

  def initialize(attributes)
    @attributes = attributes
  end

  def credit_card
    @credit_card ||= Authorizer.prepare_credit_card_for_authorization(attributes)
  end

  def authorize!
    @transaction_id = Authorizer.authorize!(:amount => attributes[:amount],
      :credit_card => credit_card)
  end

  def to_xml
    { :transaction_id => @transaction_id }.to_xml(:root => "authorization")
  end
end

Then, we rework the AuthorizationsController to use the model:

1
2
3
4
5
6
7
8

class AuthorizationsController < ApplicationController
  def create
    authorization = Authorization.new(params[:authorization])
    authorization.authorize!
    render :xml => authorization.to_xml, :status => :created
  rescue AuthorizationError => e
    render :xml => "<errors><error>#{e.message}</error></errors>", :status => :unprocessable_entity
  end

(Note the use of the “created” status, which is HTTP status code 201. Other verbs just use “ok”, status code 200, to indicate success. Also, with an error, we return an “unprocessable_entity” status, which is HTTP status code 422. ActiveResource will treat that as a failed validation.)

With that change, you could now use ActiveResource to authorize a credit card transaction:

1
2
3
4
5
6
7
8
9
10
11
12

class Authorization < ActiveResource::Base
  self.site = "http://my.monkeycharger.site"
end

auth = Authorization.new(:amount => 15, :credit_card_id => 1234,
  :remote_key => remote_key_for_card)

if auth.save
  puts "success: #{auth.transaction_id}"
else
  puts "error: #{auth.errors.full_messages.to_sentence}"
end

It should be mentioned, too, that making an app ActiveResource-compatible does nothing to harm compatibility with non-ActiveResource clients. Everything is XML, both ways, with HTTP status codes being used to report whether a request succeeded or not. Win-win!

Obviously, real, working code trumps theoretical whiteboard sketches every time, and Joe is to be congratulated on what’s done. Even though ActiveResource-compatibility can buy you a lot, you should always evaluate whether you really need it and implement accordingly.

I’m going to take a slightly different tack here, and review some of the unit tests in rails itself. They show up two common anti patterns, spurious assertions and coupling your tests to the implementation.

Perhaps the biggest benefit of a suite of unit tests is that they can provide a safety net, preventing you from accidentally adding new bugs or introducing regressions of old bugs. With a large codebase, the unit tests can also help new developers understand your intent, though they’re no substitute for comments. However if you’re not careful with what gets included in your test cases, you can end up with a liability.

Be careful what you assert

Whenever you add an assertion to your test suite you’re sending a signal to future developers that the behaviour you’re asserting is both intentional and desired. Future developers who try to refactor your code will see a failing test, and either give up, or waste time trying to figure out if the assertion is ‘real’ or whether it was merely added because that’s what the code happened to do at present.

For an example, take the test_hatbm_attribute_access_and_respond_to from associations_test.rb , especially the assertions that the project responds to access_level= and joined_on=. Because of the current implementation of respond_do?, those assertions pass. But should they?

In reality while those values will get stored in the object, they’ll never be written back to the database. This is a surprising result for some developers, and removing those accessor methods would go a long way to helping avoid some frustrating moments.

Mock and Stub with care

Mock object frameworks like flexmock and mocha make it really easy to test how your code interacts with another system or a third party library. However you should make sure that the thing that you’re mocking doesn’t merely reflect the current implementation of a method. To take a case from rails, take a look at setup_for_named_route in routing_test.rb.

It takes the seemingly sensible approach of building a stubbed-out implementation of url_for instead of trying to build a full implementation into the test cases. The stubbed version of url_for simply returns the arguments it was passed, this makes it extremely easy to work with and to test.

The problem is not with stubbing out the method, but in the way it is used in all the named route test cases. Take test_named_route_with_nested_controller.

1
2
3
4
5
6
7

def test_named_route_with_nested_controller
  rs.add_named_route :users, 'admin/user', :controller => '/admin/user', :action => 'index'
  x = setup_for_named_route.new
  assert_equal({:controller => '/admin/user', :action => 'index', :use_route => :users, :only_path => false},
  x.send(:users_url))
end

The strange hash value you see in the assertion is the result of the named route method calling url_for, and returning that. The current implementation of the named route helpers does this, but what if you wanted to implement a new version of named routes which completely avoids the costly call to url_for? Every single named route test fails, even though applications which use those methods will work fine.

In this situation you have two options, you could make your tests depend on the full implementation of url_for. This would probably slow down your test cases, and require a lot more setup code, but because the return values are correct you’re not likely to impede future refactoring.

The other option is to use different stubs for every test case. Leaving you with something like this:

1
2
3
4
5
6
7
8

def test_named_route_with_nested_controller
  rs.add_named_route :users, 'admin/user', :controller => '/admin/user', :action => 'index'
  generated_url = "http://test.named.routes/admin/user"
  x = setup_for_named_route.new
  x.stubs(:url_for).returns(generated_url)
  assert_equal(generated_url,  x.send(:users_url))
end

Doing this for each and every test case is going to be quite time consuming and make your test cases extremely verbose. As with all things in software you’ll have to make a judgement call on this trade off and make a choice between coupling or verbosity.

Whatever approach you choose, just remember that misleading test ‘failures’ can slow down refactoring, and end up reducing your ability to respond to change. As satisfying as ‘100% coverage’ or 2:1 ratios may be, don’t blindly add assertions or mock objects just to satisfy a tool. Every line in your test cases should be there for a reason, and should be placed there with just as much care as you’d use for a line of application code.

“Cargo culting”, when used in a computer-programming context, refers to the practice of using techniques (or even entire blocks of code) seen elsewhere without wholly understanding how they work. (The term “cargo cult”, if you are unfamiliar with it, has its own fascinating etymology, which is covered nicely at wikipedia.) Cargo culting is a dangerous phenomenon, watering down the state of the art and encouraging cookie-cutter code shoved blindly into black boxes.

Consider the following snippet of code, taken from a project that was submitted to us some time ago. (Alas, I cannot find the original submitter—I apologize for that!)

1
2
3

def account_code?
  !! @account_code.nil?
end

To me, this looks cargo-culted, since it is seems that the programmer did not understand what the ”!!” idiom was all about. They probably saw it used somewhere and “cargo culted” it, using it without knowledge, assuming that it was, for some reason, “necessary”.

Now, the way ”!!” works is this: take the value behind the ”!!”, negate it, and negate it again. It’s just double-negation: !(!(@account_code.nil?)). The ultimate effect is to take some value, and convert it into an honest-to-goodness “true” or “false”. (In my ever-so-humble opinion, the ”!!” idiom is an abomination: it’s far too clever for its own good. First of all, you rarely ever need a real boolean value, and for those times you do, it is better to be explicit in the conversion, by using a ternary operator or full-blown if statement, for instance.)

In other words, the double-negation of nil? results in absolutely no difference from the use of nil? by itself, since nil? will return a true/false value. This, in turn, means the effect in the original is actually not what was intended for the account_code? predicate. It should have returned “true” if the account code existed (was “non-nil”), not “false”. Thus, the method should have actually been written thus:

1
2
3

def account_code?
  ! @account_code.nil?
end

In this case, cargo-culting resulted in the code being buggy. This is not an uncommon outcome of using techniques or code without understanding their purpose. If you ever find yourself copying something into your own code, with a justfying “I-don’t-know-what-it-does, but-it-appears-to-work”, stop immediately. Do some research. Figure it out. Learn what it means.

Further, note that unless you actually need a true boolean value from that, you can shorten the implementation of the account_code? predicate even further:

1
2
3

def account_code?
  @account_code
end

This works because Ruby treats nil and false as false, and everything else as true.

If there is one thing that Koz and I want you, our readers, to come away from this site with, it is an understanding of why you should do things one way and not another. Ultimately, it makes the difference between being a mediocre programmer, and becoming a great programmer.

The first RailsWay free-for-all came off quite well. Many of you posted your favorite solutions to the problem of tab-based navigation, as posed by Nate Morse.

Jamis’ Take

Of all the solutions posted, my personal favorite was the pragmatic and simple CSS-based solution given by Mr. eel (Nate Morse came to the same solution independently):

I take a completely different approach. I ID the body of the page with the name of the current controller. Then I use a descendent CSS selector to highlight the current tab based on the body id and an id given to each link. I don’t bother with replacing the current tab link with a span. If the user wants to click that link again… then it’s the same as refreshing. Totally up to them.

With html like:

1 2 3 4 5 6

<body id="users"> <ul> <li><a href="/users" id="usersNav">Users</a></li> <li><a href="/comments" id="commentsNav">Comments</a></li> <li><a href="/posts" id="postsNav">Posts</a></li> </ul>

I would use CSS like this

1 2 3 4 5 6

#users #usersNav, #comments #commentsNav, #posts #postsNav { background:red; font-weight:bold; }

What a great approach. Although I would make the choice of the body ID explicit (rather than depending on the controller name), it is otherwise really nice. It shrugs off the whole issue of “should the current tab be a link” by saying it just doesn’t matter—every tab is always a link. Such pragmatism gets right to the heart of the Rails Way: implement just what matters, and nothing more.

Koz’s Take

A number of solutions relied on tightly coupling the controller and tabs. While this may seem like a time-saver at first, I believe that it’s unlikely to remain useful as your application grows. You’ll find yourself moving functionality into strange locations in order to make your tabs highlight correctly.

The problem is amplified with a restful application where your choice of controllers are dictated by the resources that you’re managing. You may have a list of comments in several different sections of your application, but not want to highlight the ‘comment’ tab whenever you display them.

Personally, I prefer the really simple approach of a before filter and a navigation partial.

1
2
3
4

def set_current_tab
  @current_tab = :people
end

Thanks, everyone for your submissions!

Another topic we touched briefly on at RailsConf was the idea of named callbacks.

Consider this snippet (also from Brian Cooke’s expense tracking application):

1
2
3
4
5
6
7
8

class Expense < ActiveRecord::Base
  protected
    def before_create
      if self.created_at == Time.now.to_date.to_time
        self.created_at = Time.now
      end
    end
end

One thing to keep in mind here is that when a new Expense record is created, the created_at column is used to track when the expense originally occurred, not when the record was created. As a special case, if the timestamp is 00:00 of the current day, then it is assumed to actually be the current time.

Now, looking at that code, it’s definitely not immediately obvious what it is trying to do. In fact, it took me a few minutes of steady concentration (and cross-referencing other parts of the project) to understand it. The fact that it uses a generic “before_create” callback makes it hard to know the purpose of the method, and the use of “Time.now.to_date.to_time” (though effective) is pretty intention-obscuring.

Here’s a clearer, more self-documenting approach, using a named callback:

1
2
3
4
5
6
7
8
9
10

class Expense < ActiveRecord::Base
  before_create :make_created_now_if_created_today

  protected
    def make_created_now_if_created_today
      if self.created_at == Time.now.beginning_of_day
        self.created_at = Time.now
      end
    end
end

The named callback helps make it clearer what the purpose of the method is (though in this case, an additional comment would not be amiss). Also, ActiveSupport comes to the rescue, allowing us to convert the convoluted “Time.now.to_date.to_time” into the more self-documenting “Time.now.beginning_of_day”. (Alternatively, you might prefer “Time.now.midnight”, though I find “beginning_of_day” to be clearer, since it reveals the intention better.)

Always look for ways to make your code document itself. Ruby is one of the most readable programming languages I’ve ever used, and it’s a pity to not take advantage of that readability as often as you can.

Koz and I have opinions, and we like sharing them. This site was designed as a platform for us to express those opinions.

You all have opinions, too, and some of you even have opinions that (gasp!) differ from ours. Don’t try to deny it—we read the comments on this site, too.

Well here’s your chance play the game. Nate Morse sent us an email shortly after RailsConf in which he described a problem that Koz and I would like to present to you, our readers. How would you respond?

I’ve been trying to figure out a clean way to implement a tabbed view on a website where the selected tab is written out in a <span /> element and anything unselected is a link to the particular action. So far, this is what I’ve come up with.

1 2 3 4 5 6 7 8 9

def tab_to(name, options = {}) url = options.is_a?(String) ? options : url_for(options.merge({:only_path => false})) current_url = url_for(:action => @current_action, :only_path => false) if (url == current_url) content_tag(:span, name) else link_to(name, options) end end

The @current_action variable is set in a before filter and used to determine if the tab I’m specifying is for the current action. Some of this code is lifted straight from the source of link_to (which is why it will accept either an options hash or a url) and is probably overkill. Also, it seems kind of hokey that I’m setting the :only_path key in a couple of places just to get the urls in a standard form. Is there a better way to do this?

Please post your responses in the comments. Because the comments aren’t editable, you should probably draft your response externally and then paste it in. You can use textile codes to format the text. For syntax-highlighted code snippets, just enclose the snippets in <macro:code> tags, with the lang attribute set to the language of the snippet (e.g., “ruby”, or “rhtml”). For example:

At the end of the week, Koz and I will write up a summary, highlighting a few of the responses. Read, set, go!

RailsConf 2007 was a blast. Koz and I were able to do a live “Rails Way” presentation in front of all 1,600 attendees, and it was a lot of fun. We’d like to thank everyone who helped (with their questions) to make it a success, and especially Chad Fowler (for inviting us to do a plenary session instead of a regular session) and O’Reilly and Ruby Central for all their work behind the scenes to bring the conference off so spectacularly.

Now that RailsConf is past, though, we’re looking forward to bringing The Rails Way out of hibernation. The next few articles will review the points we mentioned in our RailsConf presentation.

For this first article, I’ll be examining one of the controllers in an expense tracking application submitted by Brian Cooke. In general, the application was really well done, but I did find one example of a fat controller.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15

class ReportsController < ApplicationController
  def create
    report = params[:report]
    category_condition = report[:category] == "all" ? "" : "AND category_id = #{report[:category]}" 
    from = Date.parse("#{report["from(1i)"]}-#{report["from(2i)"]}-#{report["from(3i)"]}")
    to = Date.parse("#{report["to(1i)"]}-#{report["to(2i)"]}-#{report["to(3i)"]}").to_time+1.day-1
    @expenses = current_user.expenses.find(:all, 
                                           :conditions => ["created_at >= ? AND created_at <= ? #{category_condition}", from.to_s(:db), to.to_s(:db)], 
                                           :order => "created_at ASC")
    @graph = params[:report][:type] == "graph" 

    @total_price = 0.0
    @expenses.each {|exp| @total_price += exp.price}
  end
end

All that code obscures the purpose of the action. Looking at that, your brain has to do a lot of work to figure out what the intention is. To make the intention clearer, most of that code should be refactored into a model, but the question is: which model? You can get some idea of what model is needed by looking at the query parameter that is used: report.

Now, there is no “reports” table in the domain for this application. That does not mean there can’t be a Report model. Many people new to MVC who are learning Rails have the idea that all models correspond to database tables. The fact is that a model is just an object, persistent or otherwise.

Consider the ReportsController#create action, rewritten to take advantage of a Report model:

1
2
3
4
5

class ReportsController < ApplicationController
  def create
    @report = Report.new(current_user, params[:report])
  end
end

Now that’s skinny. Looking at that action, it is now immediately obvious (at a high level) what it is doing. Skinny controllers are intention-revealing. Fat controllers are intention-obscuring.

Let’s take a quick look at how the Report model breaks down:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53

class Report
  attr_reader :user, :options

  def initialize(user, options)
    @user = user
    @options = options || {}
  end

  def graph?
    @options[:type] == "graph" 
  end

  def total_price
    @total_price ||= expenses.sum(&:price)
  end

  def expenses
    @expenses ||= user.expenses.find(:all,
      :conditions => conditions,
      :order => "created_at ASC")
  end

  def from
    @from ||= date_from_options(:from)
  end

  def to
    @to ||= date_from_options(:to)
  end

  private

    def date_from_options(which)
      part = Proc.new { |n| options["#{which}(#{n}i)"] }
      if part[1]
        Date.new(part[1].to_i, part[2].to_i, part[3].to_i)
      else
        Date.today
      end
    end

    def conditions
      conditions = ["created_at between ? AND ?"]
      parameters = [from.to_time, to.to_time.end_of_day]

      if options[:category] != "all" 
        conditions << "category_id = ?" 
        parameters << options[:category]
      end

      [conditions.join(" AND "), *parameters]
    end
end

Now, this is a lot more lines of code than the original action. That’s not a bad thing! More lines of code are always better, if they make the code easier to read and maintain. In the above, I broke the original code into discrete parts, each of which went into it’s own method. This has several nice side-effects:

1. You can now test the Report model in isolation, instead of having to write a functional test and assert_select your way through the output.
2. The multiple instance variables in the original action are replaced now with getter attributes on the model, so in the view you have @report.total_price instead of the more ambiguous @total_price.
3. You can now use the form_for helper to simplify the view.

1
2
3
4
5

<%= form_for(:report, @report, ...) do |f| %>
  From: <%= f.date_select(:from) %>
  To: <%= f.date_select(:to) %>
  <!-- etc.... -->
<% end %>

Thanks to Brian Cooke for the submission!

Things have been a bit quiet around here while Jamis and I prepare for our talk at Rails Conf. In the meantime there are a few housekeeping announcements.

Sponsor

We’d like to thank Geoff Grosenbach’s Peep Code for sponsoring the site. Peep Code are high quality screen casts that take a particlar topic in Rails development, and cover it in depth. I’ve personally paid for a few of these, so when he offered to sponsor the site we were happy to oblige.

New Search

We’ve decided to try out Google co-op instead of mephisto’s built in search. In addition to searching this site, it’ll also search the rails wiki, the api docs and our personal blogs. If you have some sites you think we should add to the index, drop us a link in the comments. We’re looking for great reference and tutorial material, there’s gotta be some great links out there.

Submissions

Thanks to everyone who’s submitted code so far. Just because we haven’t used your example yet, doesn’t mean we won’t use it in the future. We’re especially interested in examples which are small enough to be covered in a single article, so if you have anything you’re sitting on, please send it in.

All the best, and we’re looking forward to seeing you all at RailsConf 07.

We’ve had several submissions in the past which failed to fully utilise rails’ collection proxies. For future reference, I figured I’d write up the most commonly missed opportunities. To demonstrate this we’ll assume a Todo list application where each user can manage multiple Todo lists, with a simple model like this:

1
2
3
4
5
6
7

class User < ActiveRecord::Base
  has_many :todo_lists
end

class TodoList < ActiveRecord::Base
  belongs_to :user
end

Case One – Restricting Access to User Data

If you’re following the new restful conventions the url to access a Todo List is going to look something like http://todo.example.com/todo_lists/1

To keep your application secure, you’ll want to ensure that users can’t just change the ID in the url and manage someone else’s tasks. This is one of the most commonly asked questions on mailing lists and IRC channels, and we’ve seen quite a few anti-patterns to solve it. Some of the most common anti-patterns we’ve seen are these:

Anti-Pattern #1: Manually specifying the IDs when you construct the queries;

1
2
3
4
5

def show
  unless @todo_list = TodoList.find_by_id_and_user_id(params[:id], current_user.id)
  redirect_to '/'
  end
end

Anti-Pattern #2: Querying globally, then checking ownership after the fact;

1
2
3
4

def show
  @todo_list = TodoList.find(params[:id])
  redirect_to '/' unless @todo_list.user_id = current_user.id
end

Anti-Pattern #3: Abusing with_scope for a this simple case either directly, or in an around_filter.

1
2
3
4
5

def show
  with_scope(:find=>{:user_id=>current_user.id}) do
    @todo_list = TodoList.find(params[:id])
  end
end

Best Practice: The most effective way to do this is to call find on the todo_lists association.

1
2
3

def show
  @todo_list = current_user.todo_lists.find(params[:id])
end

In the event that the user plays with the URL to point to something they don’t own, an ActiveRecord::RecordNotFound exception will be raised. In general I simply ignore these exceptions as it’s unlikely they’re caused by legitimate use, but if you want to handle them you can simply rescue the exception.

1
2
3
4
5
6

def show
  @todo_list = current_user.todo_lists.find(params[:id])
rescue ActiveRecord::RecordNotFound => e
  flash[:warning] = "Stop playing around with your urls"
  redirect_to '/'
end

Additionally, the find method on todo_lists is just like the regular find method; you’re not restricted to merely passing it an ID. So to build a secure search action you can do something like:

1
2
3
4

def search
  @todo_lists = current_user.todo_lists.find(:all, :conditions=>["name like ?", params[:q]],
       :include=>[:items])
end

That dispatching isn’t limited to find, you can call any method defined on the TodoList class. For example, say you wanted to encapsulate the search behaviour inside the TodoList class, you can define a class method:

1
2
3
4
5
6

class TodoList < ActiveRecord::Base
  # ...
  def self.search(query)
    find(:all, :conditions=>["name like ?", query], :include=>[:items])
  end
end

Then you can call that by using class methods on the has_many association:

1
2
3

def search
  @todo_lists = current_user.todo_lists.search(params[:q])
end

Case Two – Assigning Ownership

Just as you need to find TodoLists for a user, your Todo list application will also need to create new lists for a user. Frequently in submissions we’ll see something like this:

1
2
3
4
5
6

def create
  @todo_list = TodoList.new(params[:todo_list])
  @todo_list.user_id = current_user.id
  @todo_list.save!
  redirect_to todo_list_url(@todo_list)
end

Just like find, the has_many association creates a number of methods to help you create new members of the todo_lists collection. If there’s no validation on the todo list model, then the simplest option is just to use create!:

1
2
3
4

def create
  @todo_list = current_user.todo_lists.create! params[:todo_list]
  redirect_to todo_list_url(@todo_list)
end

This will raise an exception in the event the model fails validation, which prevents your application from silently failing. However, it doesn’t provide a particularly good user experience (in fact, it sucks). In situations where validation failures are likely, you can still use the association proxies:

1
2
3
4
5
6
7
8

def create
  @todo_list = current_user.todo_lists.build params[:todo_list]
  if @todo_list.save
    redirect_to todo_list_url(@todo_list)
  else
    render :action=>'new'
  end
end

Case Three – Special Queries

The final case I wanted to cover is where association declarations can be used to save you time is where you have a special query you’re frequently using. In our case, let’s say you occasionally want to get the user’s completed lists, and their active ones. The simplest way to get these is:

1
2
3
4

def show
  @completed_lists = current_user.todo_lists.find_all_by_complete(false)
  @active_lists    = current_user.todo_lists.find_all_by_complete(true)
end

However if you’ve been following our posts to date, you probably know that you should make your model match your domain, and that means your statements should read reasonably close to plain English. So if you want to find the user’s active_lists, you should probably do that with something like current_user.active_lists. A first pass at that may be.

1
2
3
4
5
6
7
8
9

class User < ActiveRecord::Base
  def completed_lists
    todo_lists.find_all_by_completed(true)
  end

  def active_lists
    todo_lists.find_all_by_completed(false)
  end
end

However a big downside to this approach is that repeated calls to active_lists will issue the query again, then construct the records from the result set, wasting time and memory. The obvious solution is to make sure the calls are cached, and the easiest way to do that is to use a has_many association.

1
2
3
4

class User < ActiveRecord::Base
  has_many :completed_lists, :class_name=>"TodoList", :conditions=>{:completed=>true }
  has_many :active_lists,    :class_name=>"TodoList", :conditions=>{:completed=>false}
end

In addition to the REST refactoring for SignOut, we thought we’d cover a few more changes we’d suggest.

The biggest thing which jumped out at me was that SignOut has no tests. If you’ve never written a test in rails all the different tools and frameworks available can make it seem quit daunting. It’s often hard to tell if you should you use rspec, heckle, autotest, rcov and where to start. So in this article we’ll take the set_away action from EmployeeController and write a few tests for it, using just the stuff you get for free with rails.

The action as it stands has some reasonably involved code for handling decoding time_back from the request parameters:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18

def set_away
  @employee = Employee.find(params[:id])
  if params[:date][:meridian] == "PM"
    if params[:date][:hour].to_i == 12
      hour = params[:date][:hour]
    else
      hour = (params[:date][:hour].to_i + 12).to_s
    end
  else
    if params[:date][:hour].to_i == 12
      hour = "0"
    else
      hour = params[:date][:hour]
    end
  end
  time_back = Time.mktime(params[:date][:year], params[:date][:month], params[:date][:day], hour, params[:date][:minute] )
  @employee.update_attributes(:reason => params[:employee][:reason], :time_out => Time.now, :time_in => time_back)
end

So there are four distinct cases that we need to ensure are tested:

1. A Meridian of “PM” and an hour of 12
2. A Meridian of “PM” and some other hour
3. A Meridian of “AM” and an hour of 12
4. A Meridian of “AM” and some other hour

First up we need some fixtures data for an employee, lets pretend I work there and create a test fixture for koz:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15

koz:
  id: 1
  firstname: Michael 
  lastname: Koziarski
  company_email: koz@example.com
  personal_email: michael@koziarski.com
  extension: 665
  cellphone: +64 21 555 1337
  homephone: +64 4 555 1337
  time_out:
  time_in:
  reason:
  created_on: <%= 5.days.ago.to_date.to_s(:db) %>
  modified_on: <%= Date.today.to_s(:db) %>
  initials: MAK

Now the tests in EmployeeControllerTest:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33

class EmployeeControllerTest < Test::Unit::TestCase
  def setup
    @controller = EmployeeController.new
    @request    = ActionController::TestRequest.new
    @response   = ActionController::TestResponse.new
  end
  # Ensure that the employees table gets reloaded as needed
  fixtures :employees

  # we can DRY up the test data by providing a default date 
  #and then just overriding what we need
  def default_date
    {:meridian=>"PM", :year=>now.year, :month=>now.month, 
     :day=>now.day, :minute=>"0", :hour=>"12"}
  end

  # Test the first case, PM and 12
  def test_set_away_for_pm_and_12_oclock
    # the default data matches this test case, 
    post :set_away :id=>employees(:koz).id, :date=>default_date
    # ensure no errors ocurred
    assert_response :success
    # reload our fixture to match the database changes
    koz = employees(:koz).reload
    # make sure the time_back attribute has been set 
    assert time_back = koz.time_back
    assert_equal 0, time_back.minute
    # 12PM is 12 midday, so the hour should be 12
    assert_equal 12, time_back.hour
    # We provided a reason here, make sure the controller has set it
    assert_equal "Because", koz.reason
  end
end

We could continue to write tests like that, but instead we can get a little clever and let ruby write the tests for us:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24

  def default_date
    {:meridian=>"PM", :year=>now.year, :month=>now.month, 
     :day=>now.day, :minute=>"0", :hour=>"12"}
  end

  TEST_SCENARIOS= {0=>{:hour=>"12", :meridian=>"AM"}, 5=>{:hour=>"5", :meridian=>"AM"}, 
                   18=>{:hour=>"6", :meridian=>"PM"}, 12=>{:hour=>"12", :meridian=>"PM"}}
  
  TEST_SCENARIOS.each do |hour, hash_overide|
    # Make sure we give our tests useful names, otherwise it's hard to tell what failed
    name = "test_with_hour_#{hash_overide[:hour]}_and_meridian_#{hash_overide[:meridian]}"
    define_method(name) do 
      post :set_away :id=>employees(:koz).id, :date=>default_date.merge(hash_override)
      # ensure no errors ocurred
      assert_response :success
      # reload our fixture to match the database changes
      koz = employees(:koz).reload
      assert time_back = koz.time_back
      assert_equal 0, time_back.minute
      # Make sure the hour matches our expected hour
      assert_equal hour, time_back.hour
    end
  end
end

Now you can refactor the set_away action without having to worry about introducing new date related bugs.

Here we go, continuing the RESTful refactoring of Randy Schmidt’s “SignOut” application. In Part 1 I talked at a high level about how the existing application could be viewed as an interface to several different resources. In this article, I’ll show you how to implement the “employees” resource. I’ll be doing this by just showing you heavily commented code snippets.

Note that, for this article, I am not using the SimplyHelpful plugin. This is mostly so that I can demonstrate how this all would work with current Rails, without all the convenience magic that SimplyHelpful adds in. Once you understand how this all fits together, though, SimplyHelpful is an enormous time saver.

Firstly, we define the routes we need:

1
2
3
4
5
6
7
8
9

ActionController::Routing::Routes.draw do |map|

  # So far, our RESTful implementation has only a single resource defined,
  # employees. We use map.resources to set up all the routes (named and
  # otherwise) that we will need. This assumes the existence of a controller
  # named EmployeesController.
  map.resources :employees

end

Then, we implement our controller:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102

# This replaces the AdminController in the original implementation. It
# provides an interface to the employees resource, allowing employees to be
# listed, edited, created, and destroyed.
#
# Note a few things about this implementation:
#
# * I'm using respond_to in a few of the actions, to demonstrate how trivial
#   it is to make your application into a web-service. There's really very
#   little reason _not_ to do it.
#
# * I'm not bothering with any kind of authorization mechanism, because the
#   original wasn't either. However, in practice, I'd be using HTTP
#   authentication in a before_filter to make sure only people authorized
#   as administrators were accessing this resource.
#
# * Note the extensive use of named routes; map.resources sets up a bunch for
#   you, for free, and they work WONDERS in cleaning up both your controllers
#   and your views.
class EmployeesController < ApplicationController
  
  # Rather than doing Employee.find(params[:id]) in all the actions that need
  # to, just use a before filter. This makes it obvious which actions need
  # an employee ID, and keeps things DRY.
  before_filter :find_employee, :only => %w(show update destroy)

  # Return a list of all employees currently in the system.
  def index
    @employees = Employee.find(:all)
    respond_to do |format|

      # If we don't give a block, the default behavior is used, which (for
      # HTML) is to render the "employees/index.rhtml" template.
      format.html

      # We specify the :root option here, because if the list of employees
      # is empty, you'd otherwise get tags like <NilClass>.
      format.xml { render :xml => @employees.to_xml(:root => "employees") }

    end
  end

  # We could technically leave this action out completely, since Rails will
  # find the new.rhtml template and render it just fine, but it's nice to be
  # able to tell by looking at the controller what actions are defined.
  def new
  end

  # Creates a new employee. It expects a hash to come in with a single
  # :employee key, which points to a subhash of the attributes to use to
  # create the employee.
  def create
    @employee = Employee.create(params[:employee])
    respond_to do |format|

      # If we're in HTML mode, redirect back to the master list.
      format.html { redirect_to(employees_path) }

      # If we're in XML mode, just return a 201 Created response.
      format.xml { head :created, :location => employee_path(@employee) }

    end
  end

  # Display the requested employee record. For this app, we just use this
  # to display the form for modifying the employee.
  def show
    respond_to do |format|
      format.html
      format.xml { render :xml => @employee.to_xml }
    end
  end

  # Update the specified employee record. Expects the same input format as 
  # the #create action.
  def update
    @employee.update_attributes(params[:employee])
    respond_to do |format|
      format.html { redirect_to(employee_path(@employee)) }

      # "head" is a wildly useful little method. It just returns a blank
      # HTTP response, which is often what you want when dealing with XML
      # requests. Here, we just say to return a "200 OK" response with no
      # body.
      format.xml  { head :ok }
    end
  end

  # Destroy the specified employee record.
  def destroy
    @employee.destroy
    respond_to do |format|
      format.html { redirect_to(employees_path) }
      format.xml  { head :ok }
    end
  end

  private

    def find_employee
      @employee = Employee.find(params[:id])
    end
end

And, lastly, we implement our RHTML views:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51

<!-- employees/index.rhtml
     Pretty much the same as the original, but this one makes use of
     the named routes provided by map.resources. Also note that for
     the "Delete" link, you have to set the :method to :delete, since
     RESTful actions use the same URL with different HTTP verbs to
     differentiate the actions. -->
<p><%= link_to "Add", new_employee_path, :class => "Add" %></p>
<ul>
<% @employees.each do |employee| %>
  <li>
    <%= employee.last_first %>
    <%= link_to "Edit", employee_path(employee), :class => "Edit" %>
    <%= link_to "Delete", employee_path(employee), :method => :delete,
          :confirm => "Are you sure you want to delete #{employee.name}?",
          :class => "Delete" %>
  </li>
<% end %>
</ul>

<!-- employees/new.rhtml
     We set up the form to POST to the /employees resource, which will
     create the new record. Note the use of form_for, which yields an instance
     of FormBuilder. This lets us do things like "f.text_field(:firstname)"
     and have the text field set up with the correct name ("employee[firstname]").
     This makes it dead simple to get the kind of nested hashes that RESTful
     actions expect (see #create and #update). -->
<% form_for :employee, Employee.new, :url => employees_path do |f| %>
  <%= render :partial => "employees/form", :locals => { :form => f } %>
<% end %>

<!-- employees/show.rhtml
     Pretty much the same as new.rhtml, but this time we post to the URI for
     a specific employee. We also set the HTTP verb to PUT, via the _method
     parameter. -->
<% form_for :employee, @employee, :url => employee_path(@employee) do |f| %>
  <%= hidden_field :_method, :put %>
  <%= render :partial => "employees/form", :locals => { :form => f } %>
<% end %>

<!-- employees/_form.rhtml
     This expects a local variable 'form' to exist, which points to a
     FormBuilder instance. -->
<dl>
  <dt><label for="employee_firstname">First Name:</label></dt>
  <dd><%= form.text_field :firstname, :size => '30', :maxlength => '100' %></dd> 

  <dt><label for="employee_initials">Initials:</label></dt>
  <dd><%= form.text_field :initials, :size => '5', :maxlength => '5' %></dd> 

  <!-- etc, etc, etc. -->
</dl>

And that’s the first resource! In the next article, I’ll illustrate the StatusesController.

Randy Schmidt sent us a project of his called SignOut, which he wrote for work and which “lets people sign in and out for meetings/lunch from their browser.” It’s a pretty simple application; an administrator first uses the admin interface to add the employees to the system, and employees can then go in and mark themselves as “away” or “not away”. (The repository is here, and we’ll be reviewing revision 47…which I can’t link to directly, unfortunately.)

As it stands, there are only a few things about the application that make me cringe; for the most part I have no complaint with how it was architected. So instead of pointing out the little nitpicks, I thought this might be a prime opportunity to demonstrate the thinking processes that go into taking an existing non-RESTful application, and redesigning it as a RESTful app.

What is REST?

REST is a term that is getting a lot of traffic these days. At the risk of grossly oversimplifying, it basically refers to a way of writing your app such that the underlying resources are exposed via a consistent, URL-based interface formed from the basic HTTP operations of “get”, “post”, “put”, and “delete”. (I have absolutely no doubt that some will take issue with this definition; please feel free to expound on your own view of the technique by commenting below.)

Sound like a lot of work? It’s not, really, especially if you do it RESTfully from the get-go, instead of reworking an existing application after the fact. But even if it is easy, why should you bother? That’s a valid question, and one worth addressing before going into this any further.

Why REST?

First of all, a RESTful application will generally have more controllers, but each controller will be smaller. Smaller objects are happy objects. Smaller objects are easier to maintain. Smaller objects are easier to refactor. Smaller objects are, quite frankly, better than bigger objects.

Secondly, a RESTful application in Rails is guided by convention, which means all RESTful Rails apps will share some basic things, like the format of their URLs. This means that we can now write a framework for consuming RESTful web services, and have that framework easily communicate with any application that conforms to those Rails conventions. And guess what? That framework already exists, and is called ActiveResource.

Thirdly (and I openly acknowledge that this is largely a matter of opinion), RESTful apps look a lot cleaner on the inside. Each line of code is nicely groomed and placed on it’s shelf. Order prevails. Chaos quails.

Convinced yet? Alrighty then. Let’s move on.

Identifying Resources

So, when writing a RESTful app, the first step is to identify the resources that you want to expose, and then create at least one controller for each of those resources. It is tempting to think of your models as this set of resources, but that’s not actually correct. In the SignOut application, there is only one model: Employee. But by looking at the actions of the two existing controllers, I was able to extract three different resources.

As the application is written, here’s the list of all of its actions:

• #index: show all employees
• #add: creates a new employee record
• #edit: displays the employee edit form (if called via GET) or updates a specific employee (if called via POST)
• #delete: deletes an existing employee

• #set_view: just sets a :view key in the session, which determines whether the status view should be brief or verbose.
• #status: shows a list of all employees in the system, along with their status. From this view, employees may mark themselves as “away”, or “available”.
• #periodic and EmployeeController#search: returns all employees matching the given criteria. These are pretty much the same operation, with the difference that #periodic is called every 10 seconds (so the view remains up-to-date).
• #set_away_info: returns the HTML form (via AJAX) that employees may use to make themselves as “away”. It allows things like the reason to be specified, along with the state.
• #set_away: updates the Employee record with the information specified by the #set_away_info form.
• #come_back: clears the away status information from the Employee’s record.

First off, note that the AdminController is the part that actually controls the employees, creating, reading, updating, and deleting them. Thus, in my refactoring, the AdminController gets renamed to EmployeesController (note the plural; controllers in a RESTful Rails app are named for the plural of the resource they control).

Second, note that the EmployeeController in the original version doesn’t really control the employees, it controls the employees’ statuses. So, I add a StatusesController. Lastly, note the EmployeeController#set_view action. This manipulates neither the employee, nor an employee’s status. It changes the view of an employee’s status, which suggests a third resource: views, which will be managed by a ViewsController.

So in my refactoring the three controllers and their actions are:

• #index: show all employees
• #new: display the “new employee” form
• #create: create a new employee
• #show: display the “edit” form for a specific employee
• #update: save an existing employee’s data
• #destroy: delete an existing employee

• #index: show the statuses of all employees. Specify a query parameter of :q for searches that filter this view.
• #show: show the away-info form for a specific employee
• #update: mark an employee as away
• #destroy: mark an employee as having come back from being away (“destroy” the away status)

• #show: sets the requested view in the session, and redirects back to StatusesController#index.

I’ll go through and implement these in subsequent articles, but don’t let that discourage you! Go ahead and think about how you would reimplement some of these yourself. Perhaps you’d even break the existing application into different resources than I did; feel free to try it out and see how it works.

Carl Fyffe submitted a small snippet of an existing application, asking how we’d polish it. We’ve decided to try out a new format for this kind of micro-refactoring; let us know what you think.

Before:

1
2
3
4
5
6
7
8
9
10
11
12

# returns Hash of Engagements keyed on date.
def self.find_by_year_month(year, month)
  date = Time.gm year, month
  engagements = {}
  all = find(:all, :conditions => ["start >= ? and start <= ?", date.last_month.end_of_month, date.next_month.beginning_of_month], :order => :start)
  all.each do |e|
    key = e.start.strftime('%Y%m%d')
    engagements[key] = [] unless engagements.has_key?(key)
    engagements[key].push e
  end
  engagements
end

After:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36

def self.find_by_year_month(year, month)
  # We're dealing with Dates, so let's use the Date class. Date can
  # be a little slower than Time, but as this method isn't going to
  # perform thousands of operations per request, we'll be fine.

  requested_date = Date.new(year, month, 1)

  # The range in the original code went from the last day of the 
  # previous month, to the first day of the following month.

  # Date#- subtracts days from a given date, which lets us find
<