From: Tom Hughes <tom@compton.nu>
Date: Fri, 9 Mar 2012 23:01:21 +0000 (+0000)
Subject: Fix various routing and route generation errors
X-Git-Tag: live~5750
X-Git-Url: https://git.openstreetmap.org/rails.git/commitdiff_plain/58d889436fc05c1a3c32cf6aa28cc751badf99ad?hp=6391ea5a7ae59c13ca3b5317ba9a4b0fa27aa788

Fix various routing and route generation errors
---

diff --git a/app/controllers/trace_controller.rb b/app/controllers/trace_controller.rb
index d0e70e933..7b2314be3 100644
--- a/app/controllers/trace_controller.rb
+++ b/app/controllers/trace_controller.rb
@@ -187,7 +187,7 @@ class TraceController < ApplicationController
         @trace.tagstring = params[:trace][:tagstring]
         @trace.visibility = params[:trace][:visibility]
         if @trace.save
-          redirect_to :action => 'view'
+          redirect_to :action => 'view', :display_name => @user.display_name
         end
       end
     else
diff --git a/app/views/message/_message_summary.html.erb b/app/views/message/_message_summary.html.erb
index 70fd44035..82d528a04 100644
--- a/app/views/message/_message_summary.html.erb
+++ b/app/views/message/_message_summary.html.erb
@@ -1,7 +1,7 @@
 <% this_colour = cycle('lightgrey', 'white') %>
 
 <tr id="inbox-<%= message_summary.id %>" class="inbox-row<%= "-unread" if not message_summary.message_read? %>">
-  <td class="inbox-sender" bgcolor="<%= this_colour %>"><%= link_to h(message_summary.sender.display_name), :controller => 'user', :action => message_summary.sender.display_name %></td>
+  <td class="inbox-sender" bgcolor="<%= this_colour %>"><%= link_to h(message_summary.sender.display_name), :controller => 'user', :action => 'view', :display_name => message_summary.sender.display_name %></td>
   <td class="inbox-subject" bgcolor="<%= this_colour %>"><%= link_to h(message_summary.title), :controller => 'message', :action => 'read', :message_id => message_summary.id  %></td>
   <td class="inbox-sent nowrap" bgcolor="<%= this_colour %>"><%= l message_summary.sent_on, :format => :friendly %></td>
   <% if message_summary.message_read? %>
diff --git a/app/views/message/inbox.html.erb b/app/views/message/inbox.html.erb
index bcd2da33a..0cbc7c77c 100644
--- a/app/views/message/inbox.html.erb
+++ b/app/views/message/inbox.html.erb
@@ -1,4 +1,4 @@
-<h2><%= t'message.inbox.my_inbox'%>/<%= link_to t('message.inbox.outbox'), url_for(:controller => "user", :action => "outbox", :id => @user.display_name) %></h2>
+<h2><%= t'message.inbox.my_inbox'%>/<%= link_to t('message.inbox.outbox'), outbox_path(@user.display_name) %></h2>
 
 <%= render :partial => "message_count" %>
 
diff --git a/app/views/message/outbox.html.erb b/app/views/message/outbox.html.erb
index fd8958126..ece514289 100644
--- a/app/views/message/outbox.html.erb
+++ b/app/views/message/outbox.html.erb
@@ -1,4 +1,4 @@
-<h2><%= raw(t'message.outbox.my_inbox', :inbox_link => link_to(t('message.outbox.inbox'), url_for(:controller => "user", :action => "inbox", :id => @user.display_name))) %>/<%= t'message.outbox.outbox' %></h2>
+<h2><%= raw(t'message.outbox.my_inbox', :inbox_link => link_to(t('message.outbox.inbox'), inbox_path(@user.display_name))) %>/<%= t'message.outbox.outbox' %></h2>
 
 <p><%= t'message.outbox.messages', :count => @user.sent_messages.size %>
 
diff --git a/app/views/user/view.html.erb b/app/views/user/view.html.erb
index 1729f6818..b323ff158 100644
--- a/app/views/user/view.html.erb
+++ b/app/views/user/view.html.erb
@@ -19,7 +19,7 @@
     <!-- Displaying user's own profile page -->
     <%= link_to t('user.view.my diary'), :controller => 'diary_entry', :action => 'list', :display_name => @user.display_name %>
     |
-    <%= link_to t('user.view.new diary entry'), :controller => 'diary_entry', :action => 'new', :display_name => @user.display_name %>
+    <%= link_to t('user.view.new diary entry'), :controller => 'diary_entry', :action => 'new' %>
     |
     <%= link_to t('user.view.my comments' ), :controller => 'diary_entry', :action => 'comments', :display_name => @this_user.display_name %>
     |
@@ -122,8 +122,8 @@
   <% if friends.empty? %>
     <%= t 'user.view.no friends' %>
   <% else %>
-    <%= link_to t('user.view.friends_changesets'), {:controller => 'browse', :action => 'friends'} %><br/>
-    <%= link_to t('user.view.friends_diaries'), {:controller => 'diary', :action => 'friends'} %><br/><br/>
+    <%= link_to t('user.view.friends_changesets'), friend_changesets_path %><br/>
+    <%= link_to t('user.view.friends_diaries'), friend_diaries_path %><br/><br/>
     <table id="friends">
       <%= render :partial => "contact", :collection => friends %>
     </table>
@@ -134,8 +134,8 @@
   <% if nearby.empty? %>
     <%= t 'user.view.no nearby users' %>
   <% else %>
-    <%= link_to t('user.view.nearby_changesets'), {:controller => 'browse', :action => 'nearby'} %><br/>
-    <%= link_to t('user.view.nearby_diaries'), {:controller => 'diary', :action => 'nearby'} %><br/><br/>
+    <%= link_to t('user.view.nearby_changesets'), nearby_changesets_path %><br/>
+    <%= link_to t('user.view.nearby_diaries'), nearby_diaries_path %><br/><br/>
     <table id="nearbyusers">
       <%= render :partial => "contact", :collection => nearby %>
     </table>
diff --git a/app/views/user_blocks/_block.html.erb b/app/views/user_blocks/_block.html.erb
index 0e2b3a287..362cfd107 100644
--- a/app/views/user_blocks/_block.html.erb
+++ b/app/views/user_blocks/_block.html.erb
@@ -19,6 +19,6 @@
   <td class="<%= c1 %>"><%= link_to t('user_block.partial.show'), block %></td>
   <td class="<%= c1 %>"><% if @user and @user.id == block.creator_id and block.active? %><%= link_to t('user_block.partial.edit'), edit_user_block_path(block) %><% end %></td>
   <% if show_revoke_link %>
-  <td class="<%= c1 %>"><% if block.active? %><%= link_to t('user_block.partial.revoke'), block, :confirm => t('user_block.partial.confirm'), :action => :revoke %><% end %></td>
+  <td class="<%= c1 %>"><% if block.active? %><%= link_to t('user_block.partial.revoke'), :controller => 'user_blocks', :action => 'revoke', :id => block.id %><% end %></td>
   <% end %>
 </tr>
diff --git a/config/routes.rb b/config/routes.rb
index cc86a0814..5952bae52 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -85,8 +85,8 @@ OpenStreetMap::Application.routes.draw do
   match '/browse/changeset/:id' => 'browse#changeset', :via => :get, :as => :changeset, :id => /\d+/
   match '/user/:display_name/edits' => 'changeset#list', :via => :get
   match '/user/:display_name/edits/feed' => 'changeset#feed', :via => :get, :format => :atom
-  match '/browse/friends' => 'changeset#list', :via => :get, :friends => true
-  match '/browse/nearby' => 'changeset#list', :via => :get, :nearby => true
+  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
   match '/browse/changesets/feed' => 'changeset#feed', :via => :get, :format => :atom
   match '/browse' => 'changeset#list', :via => :get
@@ -102,7 +102,7 @@ OpenStreetMap::Application.routes.draw do
   match '/login' => 'user#login', :via => [:get, :post]
   match '/logout' => 'user#logout', :via => [:get, :post]
   match '/offline' => 'site#offline', :via => :get
-  match '/key' => 'site#key', :via => :get
+  match '/key' => 'site#key', :via => :post
   match '/user/new' => 'user#new', :via => :get
   match '/user/terms' => 'user#terms', :via => [:get, :post]
   match '/user/save' => 'user#save', :via => :post
@@ -144,13 +144,13 @@ OpenStreetMap::Application.routes.draw do
   match '/traces/mine' => 'trace#mine', :via => :get
   match '/trace/create' => 'trace#create', :via => [:get, :post]
   match '/trace/:id/data' => 'trace#data', :via => :get
-  match '/trace/:id/edit' => 'trace#edit', :via => [:get, :post]
+  match '/trace/:id/edit' => 'trace#edit', :via => [:get, :post, :put]
   match '/trace/:id/delete' => 'trace#delete', :via => :post
 
   # diary pages
   match '/diary/new' => 'diary_entry#new', :via => [:get, :post]
-  match '/diary/friends' => 'diary_entry#list', :friends => true, :via => :get
-  match '/diary/nearby' => 'diary_entry#list', :nearby => true  , :via => :get
+  match '/diary/friends' => 'diary_entry#list', :friends => true, :via => :get, :as => "friend_diaries"
+  match '/diary/nearby' => 'diary_entry#list', :nearby => true, :via => :get, :as => "nearby_diaries"
   match '/user/:display_name/diary/rss' => 'diary_entry#rss', :via => :get, :format => :rss
   match '/diary/:language/rss' => 'diary_entry#rss', :via => :get, :format => :rss
   match '/diary/rss' => 'diary_entry#rss', :via => :get, :format => :rss
@@ -178,7 +178,7 @@ OpenStreetMap::Application.routes.draw do
   match '/users/:status' => 'user#list', :via => [:get, :post]
 
   # geocoder
-  match '/geocoder/search' => 'geocoder#search', :via => :get
+  match '/geocoder/search' => 'geocoder#search', :via => :post
   match '/geocoder/search_latlon' => 'geocoder#search_latlon', :via => :get
   match '/geocoder/search_us_postcode' => 'geocoder#search_us_postcode', :via => :get
   match '/geocoder/search_uk_postcode' => 'geocoder#search_uk_postcode', :via => :get
@@ -186,7 +186,7 @@ OpenStreetMap::Application.routes.draw do
   match '/geocoder/search_osm_namefinder' => 'geocoder#search_osm_namefinder', :via => :get
   match '/geocoder/search_osm_nominatim' => 'geocoder#search_osm_nominatim', :via => :get
   match '/geocoder/search_geonames' => 'geocoder#search_geonames', :via => :get
-  match '/geocoder/description' => 'geocoder#description', :via => :get
+  match '/geocoder/description' => 'geocoder#description', :via => :post
   match '/geocoder/description_osm_namefinder' => 'geocoder#description_osm_namefinder', :via => :get
   match '/geocoder/description_osm_nominatim' => 'geocoder#description_osm_nominatim', :via => :get
   match '/geocoder/description_geonames' => 'geocoder#description_geonames', :via => :get
@@ -196,12 +196,12 @@ OpenStreetMap::Application.routes.draw do
   match '/export/finish' => 'export#finish', :via => :post
 
   # messages
-  match '/user/:display_name/inbox' => 'message#inbox', :via => :get
-  match '/user/:display_name/outbox' => 'message#outbox', :via => :get
+  match '/user/:display_name/inbox' => 'message#inbox', :via => :get, :as => "inbox"
+  match '/user/:display_name/outbox' => 'message#outbox', :via => :get, :as => "outbox"
   match '/message/new/:display_name' => 'message#new', :via => [:get, :post]
   match '/message/read/:message_id' => 'message#read', :via => :get
   match '/message/mark/:message_id' => 'message#mark', :via => :post
-  match '/message/reply/:message_id' => 'message#reply', :via => :get
+  match '/message/reply/:message_id' => 'message#reply', :via => [:get, :post]
   match '/message/delete/:message_id' => 'message#delete', :via => :post
 
   # oauth admin pages (i.e: for setting up new clients, etc...)
diff --git a/test/functional/geocoder_controller_test.rb b/test/functional/geocoder_controller_test.rb
index e9d07b24e..b678125d2 100644
--- a/test/functional/geocoder_controller_test.rb
+++ b/test/functional/geocoder_controller_test.rb
@@ -6,7 +6,7 @@ class GeocoderControllerTest < ActionController::TestCase
   # test all routes which lead to this controller
   def test_routes
     assert_routing(
-      { :path => "/geocoder/search", :method => :get },
+      { :path => "/geocoder/search", :method => :post },
       { :controller => "geocoder", :action => "search" }
     )
     assert_routing(
@@ -39,7 +39,7 @@ class GeocoderControllerTest < ActionController::TestCase
     )
 
     assert_routing(
-      { :path => "/geocoder/description", :method => :get },
+      { :path => "/geocoder/description", :method => :post },
       { :controller => "geocoder", :action => "description" }
     )
     assert_routing(
diff --git a/test/functional/message_controller_test.rb b/test/functional/message_controller_test.rb
index 1a47f02d6..a2bb1f805 100644
--- a/test/functional/message_controller_test.rb
+++ b/test/functional/message_controller_test.rb
@@ -33,6 +33,10 @@ class MessageControllerTest < ActionController::TestCase
       { :path => "/message/reply/1", :method => :get },
       { :controller => "message", :action => "reply", :message_id => "1" }
     )
+    assert_routing(
+      { :path => "/message/reply/1", :method => :post },
+      { :controller => "message", :action => "reply", :message_id => "1" }
+    )
     assert_routing(
       { :path => "/message/delete/1", :method => :post },
       { :controller => "message", :action => "delete", :message_id => "1" }
diff --git a/test/functional/site_controller_test.rb b/test/functional/site_controller_test.rb
index 753e1c859..caef3481b 100644
--- a/test/functional/site_controller_test.rb
+++ b/test/functional/site_controller_test.rb
@@ -43,7 +43,7 @@ class SiteControllerTest < ActionController::TestCase
       { :controller => "site", :action => "offline" }
     )
     assert_routing(
-      { :path => "/key", :method => :get },
+      { :path => "/key", :method => :post },
       { :controller => "site", :action => "key" }
     )
     assert_routing(
diff --git a/test/functional/trace_controller_test.rb b/test/functional/trace_controller_test.rb
index aca2a59d8..8ff29c437 100644
--- a/test/functional/trace_controller_test.rb
+++ b/test/functional/trace_controller_test.rb
@@ -140,6 +140,10 @@ class TraceControllerTest < ActionController::TestCase
       { :path => "/trace/1/edit", :method => :post },
       { :controller => "trace", :action => "edit", :id => "1" }
     )
+    assert_routing(
+      { :path => "/trace/1/edit", :method => :put },
+      { :controller => "trace", :action => "edit", :id => "1" }
+    )
     assert_routing(
       { :path => "/trace/1/delete", :method => :post },
       { :controller => "trace", :action => "delete", :id => "1" }