The Refactoring
Today’s refactoring took me quite a bit of setup work. I wanted to convert the InvoiceController
to a REST controller in order to take advantage of all the built in helpers in Rails. I’m only going to show the routing changes I made, if want to dig deeper into the change there are view and test changes in the commit.
Before
1
2
|
# config/routes.rb
# ... Empty ... |
# config/routes.rb
# ... Empty ...
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
|
# test/integration/routing_test.rb
require "#{File.dirname(__FILE__)}/../test_helper"
class RoutingTest 'invoice', :action => 'index', :id => 'name'
should_route :get, "/invoice/new/name", :controller => 'invoice', :action => 'new', :id => 'name'
should_route :get, "/invoice/autocreate/name", :controller => 'invoice', :action => 'autocreate', :id => 'name'
should_route :get, "/invoice/show/name", :controller => 'invoice', :action => 'show', :id => 'name'
should_route :get, "/invoice/edit/name", :controller => 'invoice', :action => 'edit', :id => 'name'
should_route :get, "/invoice/autofill/name", :controller => 'invoice', :action => 'autofill', :id => 'name'
should_route :get, "/invoice/outstanding/name", :controller => 'invoice', :action => 'outstanding', :id => 'name'
should_route :post, "/invoice/create/name", :controller => 'invoice', :action => 'create', :id => 'name'
should_route :post, "/invoice/update/name", :controller => 'invoice', :action => 'update', :id => 'name'
should_route :post, "/invoice/destroy/name", :controller => 'invoice', :action => 'destroy', :id => 'name'
end
end |
# test/integration/routing_test.rb
require "#{File.dirname(__FILE__)}/../test_helper"
class RoutingTest 'invoice', :action => 'index', :id => 'name'
should_route :get, "/invoice/new/name", :controller => 'invoice', :action => 'new', :id => 'name'
should_route :get, "/invoice/autocreate/name", :controller => 'invoice', :action => 'autocreate', :id => 'name'
should_route :get, "/invoice/show/name", :controller => 'invoice', :action => 'show', :id => 'name'
should_route :get, "/invoice/edit/name", :controller => 'invoice', :action => 'edit', :id => 'name'
should_route :get, "/invoice/autofill/name", :controller => 'invoice', :action => 'autofill', :id => 'name'
should_route :get, "/invoice/outstanding/name", :controller => 'invoice', :action => 'outstanding', :id => 'name'
should_route :post, "/invoice/create/name", :controller => 'invoice', :action => 'create', :id => 'name'
should_route :post, "/invoice/update/name", :controller => 'invoice', :action => 'update', :id => 'name'
should_route :post, "/invoice/destroy/name", :controller => 'invoice', :action => 'destroy', :id => 'name'
end
end
After
1
2
3
4
5
6
7
8
9
10
11
|
# config/routes.rb
ActionController::Routing::Routes.draw do |map|
map.resources(:invoice,
:collection => {
:autocreate => :get,
:autofill => :get
},
:member => {
:outstanding => :get
})
end |
# config/routes.rb
ActionController::Routing::Routes.draw do |map|
map.resources(:invoice,
:collection => {
:autocreate => :get,
:autofill => :get
},
:member => {
:outstanding => :get
})
end
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
|
# test/integration/routing_test.rb
require "#{File.dirname(__FILE__)}/../test_helper"
class RoutingTest 'invoice', :action => 'index'
should_route :get, "/invoice/new", :controller => 'invoice', :action => 'new'
should_route :get, "/invoice/autocreate", :controller => 'invoice', :action => 'autocreate'
should_route :get, "/invoice/100", :controller => 'invoice', :action => 'show', :id => '100'
should_route :get, "/invoice/100/edit", :controller => 'invoice', :action => 'edit', :id => '100'
should_route :get, "/invoice/autofill", :controller => 'invoice', :action => 'autofill'
should_route :get, "/invoice/100/outstanding", :controller => 'invoice', :action => 'outstanding', :id => '100'
should_route :post, "/invoice", :controller => 'invoice', :action => 'create'
should_route :put, "/invoice/100", :controller => 'invoice', :action => 'update', :id => '100'
should_route :delete, "/invoice/100", :controller => 'invoice', :action => 'destroy', :id => '100'
end
end |
# test/integration/routing_test.rb
require "#{File.dirname(__FILE__)}/../test_helper"
class RoutingTest 'invoice', :action => 'index'
should_route :get, "/invoice/new", :controller => 'invoice', :action => 'new'
should_route :get, "/invoice/autocreate", :controller => 'invoice', :action => 'autocreate'
should_route :get, "/invoice/100", :controller => 'invoice', :action => 'show', :id => '100'
should_route :get, "/invoice/100/edit", :controller => 'invoice', :action => 'edit', :id => '100'
should_route :get, "/invoice/autofill", :controller => 'invoice', :action => 'autofill'
should_route :get, "/invoice/100/outstanding", :controller => 'invoice', :action => 'outstanding', :id => '100'
should_route :post, "/invoice", :controller => 'invoice', :action => 'create'
should_route :put, "/invoice/100", :controller => 'invoice', :action => 'update', :id => '100'
should_route :delete, "/invoice/100", :controller => 'invoice', :action => 'destroy', :id => '100'
end
end
Review
This refactoring, combined with removing the project requirement, has really helped to update the design of the Invoice Plugin. After a few more bug fixes, it should be ready for it’s first official release (over two years in the making).
Reference commit