Restructure notes URLs according to standard rails conventions

app/controllers/notes_controller.rb

@@ -1,4 +1,4 @@
-class NoteController < ApplicationController
+class NotesController < ApplicationController
 
   layout 'site', :only => [:mine]
 
@@ -11,7 +11,7 @@ class NoteController < ApplicationController
 
   ##
   # Return a list of notes in a given area
-  def list
+  def index
     # Figure out the bbox - we prefer a bbox argument but also
     # support the old, deprecated, method with four arguments
     if params[:bbox]
@@ -93,7 +93,7 @@ class NoteController < ApplicationController
 
   ##
   # Add a comment to an existing note
-  def update
+  def comment
     # Check the arguments are sane
     raise OSM::APIBadUserInput.new("No id was given") unless params[:id]
     raise OSM::APIBadUserInput.new("No text was given") unless params[:text]
@@ -145,7 +145,7 @@ class NoteController < ApplicationController
 
   ##
   # Get a feed of recent notes and comments
-  def rss
+  def feed
     # Get any conditions that need to be applied
     notes = closed_condition(Note.scoped)
 
@@ -170,7 +170,7 @@ class NoteController < ApplicationController
 
   ##
   # Read a note
-  def read
+  def show
     # Check the arguments are sane
     raise OSM::APIBadUserInput.new("No id was given") unless params[:id]
 
@@ -190,7 +190,7 @@ class NoteController < ApplicationController
 
   ##
   # Delete (hide) a note
-  def delete
+  def destroy
     # Check the arguments are sane
     raise OSM::APIBadUserInput.new("No id was given") unless params[:id]
 
@@ -230,10 +230,10 @@ class NoteController < ApplicationController
 
     # Render the result
     respond_to do |format|
-      format.rss { render :action => :list }
-      format.xml { render :action => :list }
-      format.json { render :action => :list }
-      format.gpx { render :action => :list }
+      format.rss { render :action => :index }
+      format.xml { render :action => :index }
+      format.json { render :action => :index }
+      format.gpx { render :action => :index }
     end
   end
 

config/routes.rb

@@ -74,19 +74,25 @@ OpenStreetMap::Application.routes.draw do
   match 'api/0.6/swf/trackpoints' => 'swf#trackpoints', :via => :get
 
   # Map notes API
-  match 'api/0.6/notes' => 'note#list', :format => :xml
-  match 'api/0.6/notes/search' => 'note#search', :format => :xml
-  match 'api/0.6/notes/rss'  => 'note#rss', :format => :rss
-  match 'api/0.6/note/create' => 'note#create'
-  match 'api/0.6/note/:id/comment' => 'note#update', :id => /\d+/
-  match 'api/0.6/note/:id/close' => 'note#close', :id => /\d+/
-  match 'api/0.6/note/:id' => 'note#read', :via => :get, :id => /\d+/, :format => :xml
-  match 'api/0.6/note/:id' => 'note#delete', :via => :delete, :id => /\d+/
-  match 'api/0.6/notes/addPOIexec' => 'note#create'
-  match 'api/0.6/notes/closePOIexec' => 'note#close'
-  match 'api/0.6/notes/editPOIexec' => 'note#update'
-  match 'api/0.6/notes/getGPX' => 'note#list', :format => :gpx
-  match 'api/0.6/notes/getRSSfeed' => 'note#rss', :format => :rss
+  scope "api/0.6" do
+    resources :notes, :except => [ :new, :edit, :update ], :constraints => { :id => /\d+/ }, :defaults => { :format => "xml" } do
+      collection do
+        get 'search'
+        get 'feed', :defaults => { :format => "rss" }
+      end
+
+      member do
+        post 'comment'
+        post 'close'
+      end
+    end
+
+    match 'notes/addPOIexec' => 'notes#create', :via => :post
+    match 'notes/closePOIexec' => 'notes#close', :via => :post
+    match 'notes/editPOIexec' => 'notes#comment', :via => :post
+    match 'notes/getGPX' => 'notes#index', :via => :get, :format => "gpx"
+    match 'notes/getRSSfeed' => 'notes#feed', :via => :get, :format => "rss"
+  end
 
   # Data browsing
   match '/browse/start' => 'browse#start', :via => :get
@@ -100,7 +106,7 @@ OpenStreetMap::Application.routes.draw do
   match '/browse/note/:id' => 'browse#note', :via => :get, :id => /\d+/
   match '/user/:display_name/edits' => 'changeset#list', :via => :get
   match '/user/:display_name/edits/feed' => 'changeset#feed', :via => :get, :format => :atom
-  match '/user/:display_name/notes' => 'note#mine', :via => :get
+  match '/user/:display_name/notes' => 'notes#mine', :via => :get
   match '/browse/friends' => 'changeset#list', :via => :get, :friends => true, :as => "friend_changesets"
   match '/browse/nearby' => 'changeset#list', :via => :get, :nearby => true, :as => "nearby_changesets"
   match '/browse/changesets' => 'changeset#list', :via => :get

test/functional/browse_controller_test.rb

@@ -39,6 +39,10 @@ class BrowseControllerTest < ActionController::TestCase
       { :path => "/browse/changeset/1", :method => :get },
       { :controller => "browse", :action => "changeset", :id => "1" }
     )
+    assert_routing(
+      { :path => "/browse/note/1", :method => :get },
+      { :controller => "browse", :action => "note", :id => "1" }
+    )
   end
 
   def test_start

test/functional/notes_controller_test.rb

@@ -1,8 +1,122 @@
 require File.dirname(__FILE__) + '/../test_helper'
 
-class NoteControllerTest < ActionController::TestCase
+class NotesControllerTest < ActionController::TestCase
   fixtures :users, :notes, :note_comments
 
+  ##
+  # test all routes which lead to this controller
+  def test_routes
+    assert_routing(
+      { :path => "/api/0.6/notes", :method => :post },
+      { :controller => "notes", :action => "create", :format => "xml" }
+    )
+    assert_routing(
+      { :path => "/api/0.6/notes/1", :method => :get },
+      { :controller => "notes", :action => "show", :id => "1", :format => "xml" }
+    )
+    assert_recognizes(
+      { :controller => "notes", :action => "show", :id => "1", :format => "xml" },
+      { :path => "/api/0.6/notes/1.xml", :method => :get }
+    )
+    assert_routing(
+      { :path => "/api/0.6/notes/1.rss", :method => :get },
+      { :controller => "notes", :action => "show", :id => "1", :format => "rss" }
+    )
+    assert_routing(
+      { :path => "/api/0.6/notes/1.json", :method => :get },
+      { :controller => "notes", :action => "show", :id => "1", :format => "json" }
+    )
+    assert_routing(
+      { :path => "/api/0.6/notes/1.gpx", :method => :get },
+      { :controller => "notes", :action => "show", :id => "1", :format => "gpx" }
+    )
+    assert_routing(
+      { :path => "/api/0.6/notes/1/comment", :method => :post },
+      { :controller => "notes", :action => "comment", :id => "1", :format => "xml" }
+    )
+    assert_routing(
+      { :path => "/api/0.6/notes/1/close", :method => :post },
+      { :controller => "notes", :action => "close", :id => "1", :format => "xml" }
+    )
+    assert_routing(
+      { :path => "/api/0.6/notes/1", :method => :delete },
+      { :controller => "notes", :action => "destroy", :id => "1", :format => "xml" }
+    )
+
+    assert_routing(
+      { :path => "/api/0.6/notes", :method => :get },
+      { :controller => "notes", :action => "index", :format => "xml" }
+    )
+    assert_recognizes(
+      { :controller => "notes", :action => "index", :format => "xml" },
+      { :path => "/api/0.6/notes.xml", :method => :get }
+    )
+    assert_routing(
+      { :path => "/api/0.6/notes.rss", :method => :get },
+      { :controller => "notes", :action => "index", :format => "rss" }
+    )
+    assert_routing(
+      { :path => "/api/0.6/notes.json", :method => :get },
+      { :controller => "notes", :action => "index", :format => "json" }
+    )
+    assert_routing(
+      { :path => "/api/0.6/notes.gpx", :method => :get },
+      { :controller => "notes", :action => "index", :format => "gpx" }
+    )
+
+    assert_routing(
+      { :path => "/api/0.6/notes/search", :method => :get },
+      { :controller => "notes", :action => "search", :format => "xml" }
+    )
+    assert_recognizes(
+      { :controller => "notes", :action => "search", :format => "xml" },
+      { :path => "/api/0.6/notes/search.xml", :method => :get }
+    )
+    assert_routing(
+      { :path => "/api/0.6/notes/search.rss", :method => :get },
+      { :controller => "notes", :action => "search", :format => "rss" }
+    )
+    assert_routing(
+      { :path => "/api/0.6/notes/search.json", :method => :get },
+      { :controller => "notes", :action => "search", :format => "json" }
+    )
+    assert_routing(
+      { :path => "/api/0.6/notes/search.gpx", :method => :get },
+      { :controller => "notes", :action => "search", :format => "gpx" }
+    )
+
+    assert_routing(
+      { :path => "/api/0.6/notes/feed", :method => :get },
+      { :controller => "notes", :action => "feed", :format => "rss" }
+    )
+
+    assert_recognizes(
+      { :controller => "notes", :action => "create" },
+      { :path => "/api/0.6/notes/addPOIexec", :method => :post }
+    )
+    assert_recognizes(
+      { :controller => "notes", :action => "close" },
+      { :path => "/api/0.6/notes/closePOIexec", :method => :post }
+    )
+    assert_recognizes(
+      { :controller => "notes", :action => "comment" },
+      { :path => "/api/0.6/notes/editPOIexec", :method => :post }
+    )
+    assert_recognizes(
+      { :controller => "notes", :action => "index", :format => "gpx" },
+      { :path => "/api/0.6/notes/getGPX", :method => :get }
+    )
+    assert_recognizes(
+      { :controller => "notes", :action => "feed", :format => "rss" },
+      { :path => "/api/0.6/notes/getRSSfeed", :method => :get }
+    )
+
+    assert_routing(
+      { :path => "/user/username/notes", :method => :get },
+      { :controller => "notes", :action => "mine", :display_name => "username" }
+    )
+  end
+
   def test_note_create_success
     assert_difference('Note.count') do
       assert_difference('NoteComment.count') do
@@ -12,7 +126,7 @@ class NoteControllerTest < ActionController::TestCase
     assert_response :success
     id = @response.body.sub(/ok/,"").to_i
 
-    get :read, {:id => id, :format => 'json'}
+    get :show, {:id => id, :format => 'json'}
     assert_response :success
     js = ActiveSupport::JSON.decode(@response.body)
     assert_not_nil js
@@ -66,11 +180,11 @@ class NoteControllerTest < ActionController::TestCase
 
   def test_note_comment_create_success
     assert_difference('NoteComment.count') do
-      post :update, {:id => notes(:open_note_with_comment).id, :name => "new_tester2", :text => "This is an additional comment"}
+      post :comment, {:id => notes(:open_note_with_comment).id, :name => "new_tester2", :text => "This is an additional comment"}
     end
     assert_response :success
 
-    get :read, {:id => notes(:open_note_with_comment).id, :format => 'json'}
+    get :show, {:id => notes(:open_note_with_comment).id, :format => 'json'}
     assert_response :success
     js = ActiveSupport::JSON.decode(@response.body)
     assert_not_nil js
@@ -85,22 +199,22 @@ class NoteControllerTest < ActionController::TestCase
 
   def test_note_comment_create_fail
     assert_no_difference('NoteComment.count') do
-      post :update, {:name => "new_tester2", :text => "This is an additional comment"}
+      post :comment, {:name => "new_tester2", :text => "This is an additional comment"}
     end
     assert_response :bad_request
 
     assert_no_difference('NoteComment.count') do
-      post :update, {:id => notes(:open_note_with_comment).id, :name => "new_tester2"}
+      post :comment, {:id => notes(:open_note_with_comment).id, :name => "new_tester2"}
     end
     assert_response :bad_request
 
     assert_no_difference('NoteComment.count') do
-      post :update, {:id => 12345, :name => "new_tester2", :text => "This is an additional comment"}
+      post :comment, {:id => 12345, :name => "new_tester2", :text => "This is an additional comment"}
     end
     assert_response :not_found
 
     assert_no_difference('NoteComment.count') do
-      post :update, {:id => notes(:hidden_note_with_comment).id, :name => "new_tester2", :text => "This is an additional comment"}
+      post :comment, {:id => notes(:hidden_note_with_comment).id, :name => "new_tester2", :text => "This is an additional comment"}
     end
     assert_response :gone
   end
@@ -109,7 +223,7 @@ class NoteControllerTest < ActionController::TestCase
     post :close, {:id => notes(:open_note_with_comment).id}
     assert_response :success
 
-    get :read, {:id => notes(:open_note_with_comment).id, :format => 'json'}
+    get :show, {:id => notes(:open_note_with_comment).id, :format => 'json'}
     assert_response :success
     js = ActiveSupport::JSON.decode(@response.body)
     assert_not_nil js
@@ -134,29 +248,25 @@ class NoteControllerTest < ActionController::TestCase
   end
 
   def test_note_read_success
-#    get :read, {:id => notes(:open_note).id}
-#    assert_response :success      
-#    assert_equal "application/xml", @response.content_type
-
-    get :read, {:id => notes(:open_note).id, :format => "xml"}
+    get :show, {:id => notes(:open_note).id, :format => "xml"}
     assert_response :success
     assert_equal "application/xml", @response.content_type
 
-    get :read, {:id => notes(:open_note).id, :format => "rss"}
+    get :show, {:id => notes(:open_note).id, :format => "rss"}
     assert_response :success
     assert_equal "application/rss+xml", @response.content_type
 
-    get :read, {:id => notes(:open_note).id, :format => "json"}
+    get :show, {:id => notes(:open_note).id, :format => "json"}
     assert_response :success
     assert_equal "application/json", @response.content_type
 
-    get :read, {:id => notes(:open_note).id, :format => "gpx"}
+    get :show, {:id => notes(:open_note).id, :format => "gpx"}
     assert_response :success
     assert_equal "application/gpx+xml", @response.content_type
   end
 
   def test_note_read_hidden_comment
-    get :read, {:id => notes(:note_with_hidden_comment).id, :format => 'json'}
+    get :show, {:id => notes(:note_with_hidden_comment).id, :format => 'json'}
     assert_response :success
     js = ActiveSupport::JSON.decode(@response.body)
     assert_not_nil js
@@ -167,73 +277,73 @@ class NoteControllerTest < ActionController::TestCase
   end
 
   def test_note_read_fail
-    post :read
+    post :show
     assert_response :bad_request
 
-    get :read, {:id => 12345}
+    get :show, {:id => 12345}
     assert_response :not_found
 
-    get :read, {:id => notes(:hidden_note_with_comment).id}
+    get :show, {:id => notes(:hidden_note_with_comment).id}
     assert_response :gone
   end
 
   def test_note_delete_success
-    delete :delete, {:id => notes(:open_note_with_comment).id}
+    delete :destroy, {:id => notes(:open_note_with_comment).id}
     assert_response :success
 
-    get :read, {:id => notes(:open_note_with_comment).id, :format => 'json'}
+    get :show, {:id => notes(:open_note_with_comment).id, :format => 'json'}
     assert_response :gone
   end
 
   def test_note_delete_fail
-    delete :delete
+    delete :destroy
     assert_response :bad_request
 
-    delete :delete, {:id => 12345}
+    delete :destroy, {:id => 12345}
     assert_response :not_found
 
-    delete :delete, {:id => notes(:hidden_note_with_comment).id}
+    delete :destroy, {:id => notes(:hidden_note_with_comment).id}
     assert_response :gone
   end
 
   def test_get_notes_success
-#    get :list, {:bbox => '1,1,1.2,1.2'}
+#    get :index, {:bbox => '1,1,1.2,1.2'}
 #    assert_response :success
 #    assert_equal "text/javascript", @response.content_type
 
-    get :list, {:bbox => '1,1,1.2,1.2', :format => 'rss'}
+    get :index, {:bbox => '1,1,1.2,1.2', :format => 'rss'}
     assert_response :success
     assert_equal "application/rss+xml", @response.content_type
 
-    get :list, {:bbox => '1,1,1.2,1.2', :format => 'json'}
+    get :index, {:bbox => '1,1,1.2,1.2', :format => 'json'}
     assert_response :success
     assert_equal "application/json", @response.content_type
 
-    get :list, {:bbox => '1,1,1.2,1.2', :format => 'xml'}
+    get :index, {:bbox => '1,1,1.2,1.2', :format => 'xml'}
     assert_response :success
     assert_equal "application/xml", @response.content_type
 
-    get :list, {:bbox => '1,1,1.2,1.2', :format => 'gpx'}
+    get :index, {:bbox => '1,1,1.2,1.2', :format => 'gpx'}
     assert_response :success
     assert_equal "application/gpx+xml", @response.content_type
   end
 
   def test_get_notes_large_area
-#    get :list, {:bbox => '-2.5,-2.5,2.5,2.5'}
+#    get :index, {:bbox => '-2.5,-2.5,2.5,2.5'}
 #    assert_response :success
 
-#    get :list, {:l => '-2.5', :b => '-2.5', :r => '2.5', :t => '2.5'}
+#    get :index, {:l => '-2.5', :b => '-2.5', :r => '2.5', :t => '2.5'}
 #    assert_response :success
 
-    get :list, {:bbox => '-10,-10,12,12'}
+    get :index, {:bbox => '-10,-10,12,12'}
     assert_response :bad_request
 
-    get :list, {:l => '-10', :b => '-10', :r => '12', :t => '12'}
+    get :index, {:l => '-10', :b => '-10', :r => '12', :t => '12'}
     assert_response :bad_request
   end
 
   def test_get_notes_closed
-    get :list, {:bbox => '1,1,1.7,1.7', :closed => '7', :format => 'json'}
+    get :index, {:bbox => '1,1,1.7,1.7', :closed => '7', :format => 'json'}
     assert_response :success
     assert_equal "application/json", @response.content_type
     js = ActiveSupport::JSON.decode(@response.body)
@@ -241,7 +351,7 @@ class NoteControllerTest < ActionController::TestCase
     assert_equal "FeatureCollection", js["type"]
     assert_equal 4, js["features"].count
 
-    get :list, {:bbox => '1,1,1.7,1.7', :closed => '0', :format => 'json'}
+    get :index, {:bbox => '1,1,1.7,1.7', :closed => '0', :format => 'json'}
     assert_response :success
     assert_equal "application/json", @response.content_type
     js = ActiveSupport::JSON.decode(@response.body)
@@ -249,7 +359,7 @@ class NoteControllerTest < ActionController::TestCase
     assert_equal "FeatureCollection", js["type"]
     assert_equal 4, js["features"].count
 
-    get :list, {:bbox => '1,1,1.7,1.7', :closed => '-1', :format => 'json'}
+    get :index, {:bbox => '1,1,1.7,1.7', :closed => '-1', :format => 'json'}
     assert_response :success
     assert_equal "application/json", @response.content_type
     js = ActiveSupport::JSON.decode(@response.body)
@@ -259,30 +369,26 @@ class NoteControllerTest < ActionController::TestCase
   end
 
   def test_get_notes_bad_params
-    get :list, {:bbox => '-2.5,-2.5,2.5'}
+    get :index, {:bbox => '-2.5,-2.5,2.5'}
     assert_response :bad_request
 
-    get :list, {:bbox => '-2.5,-2.5,2.5,2.5,2.5'}
+    get :index, {:bbox => '-2.5,-2.5,2.5,2.5,2.5'}
     assert_response :bad_request
 
-    get :list, {:b => '-2.5', :r => '2.5', :t => '2.5'}
+    get :index, {:b => '-2.5', :r => '2.5', :t => '2.5'}
     assert_response :bad_request
 
-    get :list, {:l => '-2.5', :r => '2.5', :t => '2.5'}
+    get :index, {:l => '-2.5', :r => '2.5', :t => '2.5'}
     assert_response :bad_request
 
-    get :list, {:l => '-2.5', :b => '-2.5', :t => '2.5'}
+    get :index, {:l => '-2.5', :b => '-2.5', :t => '2.5'}
     assert_response :bad_request
 
-    get :list, {:l => '-2.5', :b => '-2.5', :r => '2.5'}
+    get :index, {:l => '-2.5', :b => '-2.5', :r => '2.5'}
     assert_response :bad_request
   end
 
   def test_search_success
-#    get :search, {:q => 'note 1'}
-#    assert_response :success
-#    assert_equal "text/javascript", @response.content_type
-
     get :search, {:q => 'note 1', :format => 'xml'}
     assert_response :success
     assert_equal "application/xml", @response.content_type
@@ -306,20 +412,20 @@ class NoteControllerTest < ActionController::TestCase
   end
 
   def test_rss_success
-#    get :rss
-#    assert_response :success
-#    assert_equal "application/rss+xml", @response.content_type
+    get :feed, {:format => 'rss'}
+    assert_response :success
+    assert_equal "application/rss+xml", @response.content_type
 
-#    get :rss, {:bbox=>'1,1,1.2,1.2'}
-#    assert_response :success	
-#    assert_equal "application/rss+xml", @response.content_type
+    get :feed, {:bbox=>'1,1,1.2,1.2', :format => 'rss'}
+    assert_response :success	
+    assert_equal "application/rss+xml", @response.content_type
   end
 
   def test_rss_fail
-    get :rss, {:bbox=>'1,1,1.2'}
+    get :feed, {:bbox=>'1,1,1.2'}
     assert_response :bad_request
 
-    get :rss, {:bbox=>'1,1,1.2,1.2,1.2'}
+    get :feed, {:bbox=>'1,1,1.2,1.2,1.2'}
     assert_response :bad_request
   end