From: Tom Hughes <tom@compton.nu>
Date: Thu, 30 Aug 2018 17:26:05 +0000 (+0100)
Subject: Merge remote-tracking branch 'upstream/pull/1964'
X-Git-Tag: live~2912
X-Git-Url: https://git.openstreetmap.org/rails.git/commitdiff_plain/a1b179fa384d689e6cb78c2b8d863621e7bff152?hp=720da8b78ce22cb4049aecd2caafba38b7e778a9

Merge remote-tracking branch 'upstream/pull/1964'
---

diff --git a/app/controllers/api_controller.rb b/app/controllers/api_controller.rb
index 021ada0cd..81b8bca53 100644
--- a/app/controllers/api_controller.rb
+++ b/app/controllers/api_controller.rb
@@ -67,7 +67,7 @@ class ApiController < ApplicationController
           if gpx_file.identifiable?
             track << (XML::Node.new("name") << gpx_file.name)
             track << (XML::Node.new("desc") << gpx_file.description)
-            track << (XML::Node.new("url") << url_for(:controller => "traces", :action => "view", :display_name => gpx_file.user.display_name, :id => gpx_file.id))
+            track << (XML::Node.new("url") << url_for(:controller => "traces", :action => "show", :display_name => gpx_file.user.display_name, :id => gpx_file.id))
           end
         else
           # use the anonymous track segment if the user hasn't allowed
diff --git a/app/controllers/traces_controller.rb b/app/controllers/traces_controller.rb
index ab4c8f3b8..c77d884b3 100644
--- a/app/controllers/traces_controller.rb
+++ b/app/controllers/traces_controller.rb
@@ -12,13 +12,13 @@ class TracesController < ApplicationController
   before_action :check_api_writable, :only => [:api_create, :api_update, :api_delete]
   before_action :require_allow_read_gpx, :only => [:api_read, :api_data]
   before_action :require_allow_write_gpx, :only => [:api_create, :api_update, :api_delete]
-  before_action :offline_warning, :only => [:mine, :view]
+  before_action :offline_warning, :only => [:mine, :show]
   before_action :offline_redirect, :only => [:new, :create, :edit, :delete, :data, :api_create, :api_delete, :api_data]
   around_action :api_call_handle_error, :only => [:api_create, :api_read, :api_update, :api_delete, :api_data]
 
   # Counts and selects pages of GPX traces for various criteria (by user, tags, public etc.).
   #  target_user - if set, specifies the user to fetch traces for.  if not set will fetch all traces
-  def list
+  def index
     # from display name, pick up user id if one user's traces only
     display_name = params[:display_name]
     if display_name.present?
@@ -86,10 +86,10 @@ class TracesController < ApplicationController
   end
 
   def mine
-    redirect_to :action => :list, :display_name => current_user.display_name
+    redirect_to :action => :index, :display_name => current_user.display_name
   end
 
-  def view
+  def show
     @trace = Trace.find(params[:id])
 
     if @trace && @trace.visible? &&
@@ -97,11 +97,11 @@ class TracesController < ApplicationController
       @title = t ".title", :name => @trace.name
     else
       flash[:error] = t ".trace_not_found"
-      redirect_to :action => "list"
+      redirect_to :action => "index"
     end
   rescue ActiveRecord::RecordNotFound
     flash[:error] = t ".trace_not_found"
-    redirect_to :action => "list"
+    redirect_to :action => "index"
   end
 
   def new
@@ -126,7 +126,7 @@ class TracesController < ApplicationController
         flash[:notice] = t ".trace_uploaded"
         flash[:warning] = t ".traces_waiting", :count => current_user.traces.where(:inserted => false).count if current_user.traces.where(:inserted => false).count > 4
 
-        redirect_to :action => :list, :display_name => current_user.display_name
+        redirect_to :action => :index, :display_name => current_user.display_name
       else
         flash[:error] = t("traces.create.upload_failed") if @trace.valid?
 
@@ -175,13 +175,24 @@ class TracesController < ApplicationController
       head :forbidden
     else
       @title = t ".title", :name => @trace.name
+    end
+  rescue ActiveRecord::RecordNotFound
+    head :not_found
+  end
 
-      if request.post? && params[:trace]
-        @trace.description = params[:trace][:description]
-        @trace.tagstring = params[:trace][:tagstring]
-        @trace.visibility = params[:trace][:visibility]
-        redirect_to :action => "view", :display_name => current_user.display_name if @trace.save
-      end
+  def update
+    @trace = Trace.find(params[:id])
+
+    if !@trace.visible?
+      head :not_found
+    elsif current_user.nil? || @trace.user != current_user
+      head :forbidden
+    elsif @trace.update(trace_params)
+      flash[:notice] = t ".updated"
+      redirect_to :action => "show", :display_name => current_user.display_name
+    else
+      @title = t ".title", :name => @trace.name
+      render :action => "edit"
     end
   rescue ActiveRecord::RecordNotFound
     head :not_found
@@ -198,7 +209,7 @@ class TracesController < ApplicationController
       trace.visible = false
       trace.save
       flash[:notice] = t ".scheduled_for_deletion"
-      redirect_to :action => :list, :display_name => trace.user.display_name
+      redirect_to :action => :index, :display_name => trace.user.display_name
     end
   rescue ActiveRecord::RecordNotFound
     head :not_found
@@ -413,4 +424,8 @@ class TracesController < ApplicationController
       "public"
     end
   end
+
+  def trace_params
+    params.require(:trace).permit(:description, :tagstring, :visibility)
+  end
 end
diff --git a/app/views/traces/_trace.html.erb b/app/views/traces/_trace.html.erb
index 9b8b16a63..b81e19025 100644
--- a/app/views/traces/_trace.html.erb
+++ b/app/views/traces/_trace.html.erb
@@ -3,13 +3,13 @@
   <td class="<%= cl %>">
     <% if STATUS != :gpx_offline %>
       <% if trace.inserted %>
-        <a href="<%= url_for :controller => 'traces', :action => 'view', :id => trace.id, :display_name => trace.user.display_name %>"><img src="<%= url_for :controller => 'traces', :action => 'icon', :id => trace.id, :display_name => trace.user.display_name %>" border="0" alt="" /></a>
+        <a href="<%= url_for :controller => 'traces', :action => 'show', :id => trace.id, :display_name => trace.user.display_name %>"><img src="<%= url_for :controller => 'traces', :action => 'icon', :id => trace.id, :display_name => trace.user.display_name %>" border="0" alt="" /></a>
       <% else %>
         <span class="trace_pending"><%= t '.pending' %></span>
       <% end %>
     <% end %>
   </td>
-  <td class="<%= cl %>"><%= link_to trace.name, { :controller => 'traces', :action => 'view', :display_name => trace.user.display_name, :id => trace.id } %>
+  <td class="<%= cl %>"><%= link_to trace.name, { :controller => 'traces', :action => 'show', :display_name => trace.user.display_name, :id => trace.id } %>
     <span class="trace_summary" title="<%= trace.timestamp %>"> ...
       <% if trace.inserted %>
         (<%= t '.count_points', :count => trace.size.to_s.gsub(/(\d)(?=(\d{3})+$)/,'\1,') %>)
diff --git a/app/views/traces/edit.html.erb b/app/views/traces/edit.html.erb
index 6657a33ad..b5b4a84d6 100644
--- a/app/views/traces/edit.html.erb
+++ b/app/views/traces/edit.html.erb
@@ -4,7 +4,7 @@
 
 <img src="<%= url_for :controller => 'traces', :action => 'picture', :id => @trace.id, :display_name => @trace.user.display_name %>">
 
-<%= form_for @trace, :method => :post, :url => { :action => "edit" } do |f| %>
+<%= form_for @trace do |f| %>
 
 <div id='edit-trace-form' class='standard-form'>
   <fieldset>
diff --git a/app/views/traces/georss.rss.builder b/app/views/traces/georss.rss.builder
index 66dad596d..c2f569ae9 100644
--- a/app/views/traces/georss.rss.builder
+++ b/app/views/traces/georss.rss.builder
@@ -7,14 +7,14 @@ xml.rss("version" => "2.0",
   xml.channel do
     xml.title t(".title")
     xml.description t(".title")
-    xml.link url_for(:controller => :traces, :action => :list, :only_path => false)
+    xml.link url_for(:controller => :traces, :action => :index, :only_path => false)
 
     xml.image do
       xml.url image_url("mag_map-rss2.0.png")
       xml.title t(".title")
       xml.width 100
       xml.height 100
-      xml.link url_for(:controller => :traces, :action => :list, :only_path => false)
+      xml.link url_for(:controller => :traces, :action => :index, :only_path => false)
     end
 
     @traces.each do |trace|
diff --git a/app/views/traces/list.html.erb b/app/views/traces/index.html.erb
similarity index 89%
rename from app/views/traces/list.html.erb
rename to app/views/traces/index.html.erb
index 3cd90f3e7..0fcaeb6f7 100644
--- a/app/views/traces/list.html.erb
+++ b/app/views/traces/index.html.erb
@@ -5,11 +5,11 @@
     <li><%= rss_link_to :action => 'georss', :display_name => @display_name, :tag => @tag %></li>
     <li><%= link_to t('.upload_trace'), new_trace_path %></li>
     <% if @tag %>
-      <li><%= link_to t('.see_all_traces'), :controller => 'traces', :action => 'list', :display_name => nil, :tag => nil, :page => nil %></li>
+      <li><%= link_to t('.see_all_traces'), :controller => 'traces', :action => 'index', :display_name => nil, :tag => nil, :page => nil %></li>
       <li><%= link_to t('.see_my_traces'), :action => 'mine', :tag => nil, :page => nil %></li>
     <% else %>
       <% if @display_name %>
-        <li><%= link_to t('.see_all_traces'), :controller => 'traces', :action => 'list', :display_name => nil, :tag => nil, :page => nil %></li>
+        <li><%= link_to t('.see_all_traces'), :controller => 'traces', :action => 'index', :display_name => nil, :tag => nil, :page => nil %></li>
       <% end %>
       <% if current_user && current_user != @target_user %>
         <li><%= link_to t('.see_my_traces'), :action => 'mine', :tag => nil, :page => nil %></li>
diff --git a/app/views/traces/view.html.erb b/app/views/traces/show.html.erb
similarity index 90%
rename from app/views/traces/view.html.erb
rename to app/views/traces/show.html.erb
index 648160c2f..178cfbe73 100644
--- a/app/views/traces/view.html.erb
+++ b/app/views/traces/show.html.erb
@@ -40,7 +40,7 @@
     <td><%= t '.tags' %></td>
     <td>
     <% unless @trace.tags.empty? %>
-      <%= raw(@trace.tags.collect { |tag| link_to tag.tag, { :controller => 'traces', :action => 'list', :tag => tag.tag, :id => nil } }.join(", ")) %>
+      <%= raw(@trace.tags.collect { |tag| link_to tag.tag, { :controller => 'traces', :action => 'index', :tag => tag.tag, :id => nil } }.join(", ")) %>
     <% else %>
       <i><%= t '.none' %></i>
     <% end %>
@@ -57,10 +57,8 @@
 <% if current_user && (current_user==@trace.user || current_user.administrator? || current_user.moderator?)%>
   <div class="buttons">
     <% if current_user == @trace.user %>
-      <div>
-        <%= button_to t('.edit_track'), trace_edit_path(@trace) %>
-      </div>
+      <%= link_to t('.edit_trace'), edit_trace_path(@trace), :class => "button" %>
     <% end %>
-    <%= button_to t('.delete_track'), { :controller => 'traces', :action => 'delete', :id => @trace.id }, :data => { :confirm => t('.confirm_delete') } %>
+    <%= button_to t('.delete_trace'), { :controller => 'traces', :action => 'delete', :id => @trace.id }, :data => { :confirm => t('.confirm_delete') } %>
   </div>
 <% end %>
diff --git a/app/views/user/view.html.erb b/app/views/user/view.html.erb
index 0acb3f014..0e99a2861 100644
--- a/app/views/user/view.html.erb
+++ b/app/views/user/view.html.erb
@@ -56,7 +56,7 @@
             <%= link_to t('.notes'), :controller => 'notes', :action=> 'mine' %>
           </li>
           <li>
-            <%= link_to t('.traces'), :controller => 'traces', :action => 'list', :display_name => @user.display_name %>
+            <%= link_to t('.traces'), :controller => 'traces', :action => 'index', :display_name => @user.display_name %>
             <span class='count-number'><%= number_with_delimiter(@user.traces.size) %></span>
           </li>
 
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 87ffcaac8..3c1fca361 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -1723,9 +1723,11 @@ en:
       visibility: "Visibility:"
       visibility_help: "what does this mean?"
       visibility_help_url: "https://wiki.openstreetmap.org/wiki/Visibility_of_GPS_traces"
+    update:
+      updated: Trace updated
     trace_optionals:
       tags: "Tags"
-    view:
+    show:
       title: "Viewing trace %{name}"
       heading: "Viewing trace %{name}"
       pending: "PENDING"
@@ -1740,8 +1742,8 @@ en:
       description: "Description:"
       tags: "Tags:"
       none: "None"
-      edit_track: "Edit this trace"
-      delete_track: "Delete this trace"
+      edit_trace: "Edit this trace"
+      delete_trace: "Delete this trace"
       trace_not_found: "Trace not found!"
       visibility: "Visibility:"
       confirm_delete: "Delete this trace?"
@@ -1765,7 +1767,7 @@ en:
       by: "by"
       in: "in"
       map: "map"
-    list:
+    index:
       public_traces: "Public GPS traces"
       my_traces: "My GPS traces"
       public_traces_from: "Public GPS traces from %{user}"
diff --git a/config/routes.rb b/config/routes.rb
index a8c216bf5..286a53274 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -189,30 +189,30 @@ OpenStreetMap::Application.routes.draw do
   post "/preview/:type" => "site#preview", :as => :preview
 
   # traces
-  get "/user/:display_name/traces/tag/:tag/page/:page" => "traces#list", :page => /[1-9][0-9]*/
-  get "/user/:display_name/traces/tag/:tag" => "traces#list"
-  get "/user/:display_name/traces/page/:page" => "traces#list", :page => /[1-9][0-9]*/
-  get "/user/:display_name/traces" => "traces#list"
+  resources :traces, :except => [:show]
+  get "/user/:display_name/traces/tag/:tag/page/:page" => "traces#index", :page => /[1-9][0-9]*/
+  get "/user/:display_name/traces/tag/:tag" => "traces#index"
+  get "/user/:display_name/traces/page/:page" => "traces#index", :page => /[1-9][0-9]*/
+  get "/user/:display_name/traces" => "traces#index"
   get "/user/:display_name/traces/tag/:tag/rss" => "traces#georss", :defaults => { :format => :rss }
   get "/user/:display_name/traces/rss" => "traces#georss", :defaults => { :format => :rss }
-  get "/user/:display_name/traces/:id" => "traces#view"
+  get "/user/:display_name/traces/:id" => "traces#show"
   get "/user/:display_name/traces/:id/picture" => "traces#picture"
   get "/user/:display_name/traces/:id/icon" => "traces#icon"
-  get "/traces/tag/:tag/page/:page" => "traces#list", :page => /[1-9][0-9]*/
-  get "/traces/tag/:tag" => "traces#list"
-  get "/traces/page/:page" => "traces#list", :page => /[1-9][0-9]*/
-  get "/traces" => "traces#list"
+  get "/traces/tag/:tag/page/:page" => "traces#index", :page => /[1-9][0-9]*/
+  get "/traces/tag/:tag" => "traces#index"
+  get "/traces/page/:page" => "traces#index", :page => /[1-9][0-9]*/
   get "/traces/tag/:tag/rss" => "traces#georss", :defaults => { :format => :rss }
   get "/traces/rss" => "traces#georss", :defaults => { :format => :rss }
   get "/traces/mine/tag/:tag/page/:page" => "traces#mine", :page => /[1-9][0-9]*/
   get "/traces/mine/tag/:tag" => "traces#mine"
   get "/traces/mine/page/:page" => "traces#mine"
   get "/traces/mine" => "traces#mine"
-  resources :traces, :only => [:new, :create]
   post "/trace/create" => "traces#create" # remove after deployment
   get "/trace/create", :to => redirect(:path => "/traces/new")
   get "/trace/:id/data" => "traces#data", :id => /\d+/, :as => "trace_data"
-  match "/trace/:id/edit" => "traces#edit", :via => [:get, :post], :id => /\d+/, :as => "trace_edit"
+  post "trace/:id/edit" => "traces#update" # remove after deployment
+  get "/trace/:id/edit", :to => redirect(:path => "/traces/%{id}/edit")
   post "/trace/:id/delete" => "traces#delete", :id => /\d+/
 
   # diary pages
diff --git a/test/controllers/traces_controller_test.rb b/test/controllers/traces_controller_test.rb
index fbbbe4adc..e11fe1465 100644
--- a/test/controllers/traces_controller_test.rb
+++ b/test/controllers/traces_controller_test.rb
@@ -56,35 +56,35 @@ class TracesControllerTest < ActionController::TestCase
 
     assert_routing(
       { :path => "/traces", :method => :get },
-      { :controller => "traces", :action => "list" }
+      { :controller => "traces", :action => "index" }
     )
     assert_routing(
       { :path => "/traces/page/1", :method => :get },
-      { :controller => "traces", :action => "list", :page => "1" }
+      { :controller => "traces", :action => "index", :page => "1" }
     )
     assert_routing(
       { :path => "/traces/tag/tagname", :method => :get },
-      { :controller => "traces", :action => "list", :tag => "tagname" }
+      { :controller => "traces", :action => "index", :tag => "tagname" }
     )
     assert_routing(
       { :path => "/traces/tag/tagname/page/1", :method => :get },
-      { :controller => "traces", :action => "list", :tag => "tagname", :page => "1" }
+      { :controller => "traces", :action => "index", :tag => "tagname", :page => "1" }
     )
     assert_routing(
       { :path => "/user/username/traces", :method => :get },
-      { :controller => "traces", :action => "list", :display_name => "username" }
+      { :controller => "traces", :action => "index", :display_name => "username" }
     )
     assert_routing(
       { :path => "/user/username/traces/page/1", :method => :get },
-      { :controller => "traces", :action => "list", :display_name => "username", :page => "1" }
+      { :controller => "traces", :action => "index", :display_name => "username", :page => "1" }
     )
     assert_routing(
       { :path => "/user/username/traces/tag/tagname", :method => :get },
-      { :controller => "traces", :action => "list", :display_name => "username", :tag => "tagname" }
+      { :controller => "traces", :action => "index", :display_name => "username", :tag => "tagname" }
     )
     assert_routing(
       { :path => "/user/username/traces/tag/tagname/page/1", :method => :get },
-      { :controller => "traces", :action => "list", :display_name => "username", :tag => "tagname", :page => "1" }
+      { :controller => "traces", :action => "index", :display_name => "username", :tag => "tagname", :page => "1" }
     )
 
     assert_routing(
@@ -123,7 +123,7 @@ class TracesControllerTest < ActionController::TestCase
 
     assert_routing(
       { :path => "/user/username/traces/1", :method => :get },
-      { :controller => "traces", :action => "view", :display_name => "username", :id => "1" }
+      { :controller => "traces", :action => "show", :display_name => "username", :id => "1" }
     )
     assert_routing(
       { :path => "/user/username/traces/1/picture", :method => :get },
@@ -151,12 +151,12 @@ class TracesControllerTest < ActionController::TestCase
       { :controller => "traces", :action => "data", :id => "1", :format => "xml" }
     )
     assert_routing(
-      { :path => "/trace/1/edit", :method => :get },
+      { :path => "/traces/1/edit", :method => :get },
       { :controller => "traces", :action => "edit", :id => "1" }
     )
     assert_routing(
-      { :path => "/trace/1/edit", :method => :post },
-      { :controller => "traces", :action => "edit", :id => "1" }
+      { :path => "/traces/1", :method => :put },
+      { :controller => "traces", :action => "update", :id => "1" }
     )
     assert_routing(
       { :path => "/trace/1/delete", :method => :post },
@@ -164,8 +164,8 @@ class TracesControllerTest < ActionController::TestCase
     )
   end
 
-  # Check that the list of traces is displayed
-  def test_list
+  # Check that the index of traces is displayed
+  def test_index
     user = create(:user)
     # The fourth test below is surpisingly sensitive to timestamp ordering when the timestamps are equal.
     trace_a = create(:trace, :visibility => "public", :timestamp => 4.seconds.ago) do |trace|
@@ -181,25 +181,25 @@ class TracesControllerTest < ActionController::TestCase
       create(:tracetag, :trace => trace, :tag => "Birmingham")
     end
 
-    # First with the public list
-    get :list
-    check_trace_list [trace_b, trace_a]
+    # First with the public index
+    get :index
+    check_trace_index [trace_b, trace_a]
 
     # Restrict traces to those with a given tag
-    get :list, :params => { :tag => "London" }
-    check_trace_list [trace_a]
+    get :index, :params => { :tag => "London" }
+    check_trace_index [trace_a]
 
     # Should see more when we are logged in
-    get :list, :session => { :user => user }
-    check_trace_list [trace_d, trace_c, trace_b, trace_a]
+    get :index, :session => { :user => user }
+    check_trace_index [trace_d, trace_c, trace_b, trace_a]
 
     # Again, we should see more when we are logged in
-    get :list, :params => { :tag => "London" }, :session => { :user => user }
-    check_trace_list [trace_c, trace_a]
+    get :index, :params => { :tag => "London" }, :session => { :user => user }
+    check_trace_index [trace_c, trace_a]
   end
 
   # Check that I can get mine
-  def test_list_mine
+  def test_index_mine
     user = create(:user)
     create(:trace, :visibility => "public") do |trace|
       create(:tracetag, :trace => trace, :tag => "Birmingham")
@@ -214,15 +214,15 @@ class TracesControllerTest < ActionController::TestCase
 
     # Now try when logged in
     get :mine, :session => { :user => user }
-    assert_redirected_to :action => "list", :display_name => user.display_name
+    assert_redirected_to :action => "index", :display_name => user.display_name
 
-    # Fetch the actual list
-    get :list, :params => { :display_name => user.display_name }, :session => { :user => user }
-    check_trace_list [trace_b]
+    # Fetch the actual index
+    get :index, :params => { :display_name => user.display_name }, :session => { :user => user }
+    check_trace_index [trace_b]
   end
 
-  # Check the list of traces for a specific user
-  def test_list_user
+  # Check the index of traces for a specific user
+  def test_index_user
     user = create(:user)
     second_user = create(:user)
     third_user = create(:user)
@@ -233,45 +233,45 @@ class TracesControllerTest < ActionController::TestCase
     end
 
     # Test a user with no traces
-    get :list, :params => { :display_name => second_user.display_name }
-    check_trace_list []
+    get :index, :params => { :display_name => second_user.display_name }
+    check_trace_index []
 
     # Test the user with the traces - should see only public ones
-    get :list, :params => { :display_name => user.display_name }
-    check_trace_list [trace_b]
+    get :index, :params => { :display_name => user.display_name }
+    check_trace_index [trace_b]
 
     # Should still see only public ones when authenticated as another user
-    get :list, :params => { :display_name => user.display_name }, :session => { :user => third_user }
-    check_trace_list [trace_b]
+    get :index, :params => { :display_name => user.display_name }, :session => { :user => third_user }
+    check_trace_index [trace_b]
 
     # Should see all traces when authenticated as the target user
-    get :list, :params => { :display_name => user.display_name }, :session => { :user => user }
-    check_trace_list [trace_c, trace_b]
+    get :index, :params => { :display_name => user.display_name }, :session => { :user => user }
+    check_trace_index [trace_c, trace_b]
 
     # Should only see traces with the correct tag when a tag is specified
-    get :list, :params => { :display_name => user.display_name, :tag => "London" }, :session => { :user => user }
-    check_trace_list [trace_c]
+    get :index, :params => { :display_name => user.display_name, :tag => "London" }, :session => { :user => user }
+    check_trace_index [trace_c]
 
     # Should get an error if the user does not exist
-    get :list, :params => { :display_name => "UnknownUser" }
+    get :index, :params => { :display_name => "UnknownUser" }
     assert_response :not_found
     assert_template "user/no_such_user"
   end
 
-  # Check a multi-page list
-  def test_list_paged
+  # Check a multi-page index
+  def test_index_paged
     # Create several pages worth of traces
     create_list(:trace, 50)
 
-    # Try and get the list
-    get :list
+    # Try and get the index
+    get :index
     assert_response :success
     assert_select "table#trace_list tbody", :count => 1 do
       assert_select "tr", :count => 20
     end
 
     # Try and get the second page
-    get :list, :params => { :page => 2 }
+    get :index, :params => { :page => 2 }
     assert_response :success
     assert_select "table#trace_list tbody", :count => 1 do
       assert_select "tr", :count => 20
@@ -299,55 +299,55 @@ class TracesControllerTest < ActionController::TestCase
     check_trace_feed user.traces.tagged("Birmingham").visible_to_all
   end
 
-  # Test viewing a trace
-  def test_view
+  # Test showing a trace
+  def test_show
     public_trace_file = create(:trace, :visibility => "public")
 
     # First with no auth, which should work since the trace is public
-    get :view, :params => { :display_name => public_trace_file.user.display_name, :id => public_trace_file.id }
-    check_trace_view public_trace_file
+    get :show, :params => { :display_name => public_trace_file.user.display_name, :id => public_trace_file.id }
+    check_trace_show public_trace_file
 
     # Now with some other user, which should work since the trace is public
-    get :view, :params => { :display_name => public_trace_file.user.display_name, :id => public_trace_file.id }, :session => { :user => create(:user) }
-    check_trace_view public_trace_file
+    get :show, :params => { :display_name => public_trace_file.user.display_name, :id => public_trace_file.id }, :session => { :user => create(:user) }
+    check_trace_show public_trace_file
 
     # And finally we should be able to do it with the owner of the trace
-    get :view, :params => { :display_name => public_trace_file.user.display_name, :id => public_trace_file.id }, :session => { :user => public_trace_file.user }
-    check_trace_view public_trace_file
+    get :show, :params => { :display_name => public_trace_file.user.display_name, :id => public_trace_file.id }, :session => { :user => public_trace_file.user }
+    check_trace_show public_trace_file
   end
 
   # Check an anonymous trace can't be viewed by another user
-  def test_view_anon
+  def test_show_anon
     anon_trace_file = create(:trace, :visibility => "private")
 
     # First with no auth
-    get :view, :params => { :display_name => anon_trace_file.user.display_name, :id => anon_trace_file.id }
+    get :show, :params => { :display_name => anon_trace_file.user.display_name, :id => anon_trace_file.id }
     assert_response :redirect
-    assert_redirected_to :action => :list
+    assert_redirected_to :action => :index
 
     # Now with some other user, which should not work since the trace is anon
-    get :view, :params => { :display_name => anon_trace_file.user.display_name, :id => anon_trace_file.id }, :session => { :user => create(:user) }
+    get :show, :params => { :display_name => anon_trace_file.user.display_name, :id => anon_trace_file.id }, :session => { :user => create(:user) }
     assert_response :redirect
-    assert_redirected_to :action => :list
+    assert_redirected_to :action => :index
 
     # And finally we should be able to do it with the owner of the trace
-    get :view, :params => { :display_name => anon_trace_file.user.display_name, :id => anon_trace_file.id }, :session => { :user => anon_trace_file.user }
-    check_trace_view anon_trace_file
+    get :show, :params => { :display_name => anon_trace_file.user.display_name, :id => anon_trace_file.id }, :session => { :user => anon_trace_file.user }
+    check_trace_show anon_trace_file
   end
 
-  # Test viewing a trace that doesn't exist
-  def test_view_not_found
+  # Test showing a trace that doesn't exist
+  def test_show_not_found
     deleted_trace_file = create(:trace, :deleted)
 
     # First with a trace that has never existed
-    get :view, :params => { :display_name => create(:user).display_name, :id => 0 }
+    get :show, :params => { :display_name => create(:user).display_name, :id => 0 }
     assert_response :redirect
-    assert_redirected_to :action => :list
+    assert_redirected_to :action => :index
 
     # Now with a trace that has been deleted
-    get :view, :params => { :display_name => deleted_trace_file.user.display_name, :id => deleted_trace_file.id }, :session => { :user => deleted_trace_file.user }
+    get :show, :params => { :display_name => deleted_trace_file.user.display_name, :id => deleted_trace_file.id }, :session => { :user => deleted_trace_file.user }
     assert_response :redirect
-    assert_redirected_to :action => :list
+    assert_redirected_to :action => :index
   end
 
   # Test downloading a trace
@@ -558,7 +558,7 @@ class TracesControllerTest < ActionController::TestCase
     assert_not_equal "trackable", user.preferences.where(:k => "gps.trace.visibility").first.v
     post :create, :params => { :trace => { :gpx_file => file, :description => "New Trace", :tagstring => "new,trace", :visibility => "trackable" } }, :session => { :user => user }
     assert_response :redirect
-    assert_redirected_to :action => :list, :display_name => user.display_name
+    assert_redirected_to :action => :index, :display_name => user.display_name
     assert_match /file has been uploaded/, flash[:notice]
     trace = Trace.order(:id => :desc).first
     assert_equal "a.gpx", trace.name
@@ -594,7 +594,7 @@ class TracesControllerTest < ActionController::TestCase
     # First with no auth
     get :edit, :params => { :display_name => public_trace_file.user.display_name, :id => public_trace_file.id }
     assert_response :redirect
-    assert_redirected_to :controller => :user, :action => :login, :referer => trace_edit_path(:display_name => public_trace_file.user.display_name, :id => public_trace_file.id)
+    assert_redirected_to :controller => :user, :action => :login, :referer => edit_trace_path(:display_name => public_trace_file.user.display_name, :id => public_trace_file.id)
 
     # Now with some other user, which should fail
     get :edit, :params => { :display_name => public_trace_file.user.display_name, :id => public_trace_file.id }, :session => { :user => create(:user) }
@@ -613,34 +613,8 @@ class TracesControllerTest < ActionController::TestCase
     assert_response :success
   end
 
-  # Test fetching the edit page for a trace using POST
-  def test_edit_post_no_details
-    public_trace_file = create(:trace, :visibility => "public")
-    deleted_trace_file = create(:trace, :deleted)
-
-    # First with no auth
-    post :edit, :params => { :display_name => public_trace_file.user.display_name, :id => public_trace_file.id }
-    assert_response :forbidden
-
-    # Now with some other user, which should fail
-    post :edit, :params => { :display_name => public_trace_file.user.display_name, :id => public_trace_file.id }, :session => { :user => create(:user) }
-    assert_response :forbidden
-
-    # Now with a trace which doesn't exist
-    post :edit, :params => { :display_name => create(:user).display_name, :id => 0 }, :session => { :user => create(:user) }
-    assert_response :not_found
-
-    # Now with a trace which has been deleted
-    post :edit, :params => { :display_name => deleted_trace_file.user.display_name, :id => deleted_trace_file.id }, :session => { :user => deleted_trace_file.user }
-    assert_response :not_found
-
-    # Finally with a trace that we are allowed to edit
-    post :edit, :params => { :display_name => public_trace_file.user.display_name, :id => public_trace_file.id }, :session => { :user => public_trace_file.user }
-    assert_response :success
-  end
-
   # Test saving edits to a trace
-  def test_edit_post_with_details
+  def test_update
     public_trace_file = create(:trace, :visibility => "public")
     deleted_trace_file = create(:trace, :deleted)
 
@@ -648,25 +622,25 @@ class TracesControllerTest < ActionController::TestCase
     new_details = { :description => "Changed description", :tagstring => "new_tag", :visibility => "private" }
 
     # First with no auth
-    post :edit, :params => { :display_name => public_trace_file.user.display_name, :id => public_trace_file.id, :trace => new_details }
+    put :update, :params => { :display_name => public_trace_file.user.display_name, :id => public_trace_file.id, :trace => new_details }
     assert_response :forbidden
 
     # Now with some other user, which should fail
-    post :edit, :params => { :display_name => public_trace_file.user.display_name, :id => public_trace_file.id, :trace => new_details }, :session => { :user => create(:user) }
+    put :update, :params => { :display_name => public_trace_file.user.display_name, :id => public_trace_file.id, :trace => new_details }, :session => { :user => create(:user) }
     assert_response :forbidden
 
     # Now with a trace which doesn't exist
-    post :edit, :params => { :display_name => create(:user).display_name, :id => 0 }, :session => { :user => create(:user), :trace => new_details }
+    put :update, :params => { :display_name => create(:user).display_name, :id => 0 }, :session => { :user => create(:user), :trace => new_details }
     assert_response :not_found
 
     # Now with a trace which has been deleted
-    post :edit, :params => { :display_name => deleted_trace_file.user.display_name, :id => deleted_trace_file.id, :trace => new_details }, :session => { :user => deleted_trace_file.user }
+    put :update, :params => { :display_name => deleted_trace_file.user.display_name, :id => deleted_trace_file.id, :trace => new_details }, :session => { :user => deleted_trace_file.user }
     assert_response :not_found
 
     # Finally with a trace that we are allowed to edit
-    post :edit, :params => { :display_name => public_trace_file.user.display_name, :id => public_trace_file.id, :trace => new_details }, :session => { :user => public_trace_file.user }
+    put :update, :params => { :display_name => public_trace_file.user.display_name, :id => public_trace_file.id, :trace => new_details }, :session => { :user => public_trace_file.user }
     assert_response :redirect
-    assert_redirected_to :action => :view, :display_name => public_trace_file.user.display_name
+    assert_redirected_to :action => :show, :display_name => public_trace_file.user.display_name
     trace = Trace.find(public_trace_file.id)
     assert_equal new_details[:description], trace.description
     assert_equal new_details[:tagstring], trace.tagstring
@@ -697,7 +671,7 @@ class TracesControllerTest < ActionController::TestCase
     # Now with a trace that we are allowed to delete
     post :delete, :params => { :display_name => public_trace_file.user.display_name, :id => public_trace_file.id }, :session => { :user => public_trace_file.user }
     assert_response :redirect
-    assert_redirected_to :action => :list, :display_name => public_trace_file.user.display_name
+    assert_redirected_to :action => :index, :display_name => public_trace_file.user.display_name
     trace = Trace.find(public_trace_file.id)
     assert_equal false, trace.visible
 
@@ -707,7 +681,7 @@ class TracesControllerTest < ActionController::TestCase
 
     post :delete, :params => { :display_name => public_trace_file.user.display_name, :id => public_trace_file.id }, :session => { :user => admin }
     assert_response :redirect
-    assert_redirected_to :action => :list, :display_name => public_trace_file.user.display_name
+    assert_redirected_to :action => :index, :display_name => public_trace_file.user.display_name
     trace = Trace.find(public_trace_file.id)
     assert_equal false, trace.visible
   end
@@ -1035,9 +1009,9 @@ class TracesControllerTest < ActionController::TestCase
     end
   end
 
-  def check_trace_list(traces)
+  def check_trace_index(traces)
     assert_response :success
-    assert_template "list"
+    assert_template "index"
 
     if !traces.empty?
       assert_select "table#trace_list tbody", :count => 1 do
@@ -1055,9 +1029,9 @@ class TracesControllerTest < ActionController::TestCase
     end
   end
 
-  def check_trace_view(trace)
+  def check_trace_show(trace)
     assert_response :success
-    assert_template "view"
+    assert_template "show"
 
     assert_select "table", :count => 1 do
       assert_select "td", /^#{Regexp.quote(trace.name)} /