Use the conventional 'destroy' method name for destroying traces

diff --git a/app/abilities/ability.rb b/app/abilities/ability.rb
index c34f357a97a41b2febfbdb7fa1b0a505ded8e3aa..f0cebb380a46ba2ee2572fc54884e0eb8bd93353 100644 (file)
--- a/app/abilities/ability.rb
+++ b/app/abilities/ability.rb
@@ -40,7 +40,7 @@ class Ability
         can [:new, :create, :reply, :show, :inbox, :outbox, :mark, :destroy], Message
         can [:close, :reopen], Note
         can [:new, :create], Report
-        can [:mine, :new, :create, :edit, :update, :delete], Trace
+        can [:mine, :new, :create, :edit, :update, :destroy], Trace
         can [:account, :go_public, :make_friend, :remove_friend], User
 
         if user.moderator?

diff --git a/app/controllers/traces_controller.rb b/app/controllers/traces_controller.rb
index a0852d2ce2665959714c02c35c5cdc4c03504425..b800d305e03fc495f9bc9a7bdf5752a2ebd83bcf 100644 (file)
--- a/app/controllers/traces_controller.rb
+++ b/app/controllers/traces_controller.rb
@@ -7,9 +7,9 @@ class TracesController < ApplicationController
 
   authorize_resource
 
-  before_action :check_database_writable, :only => [:new, :create, :edit, :delete]
+  before_action :check_database_writable, :only => [:new, :create, :edit, :destroy]
   before_action :offline_warning, :only => [:mine, :show]
-  before_action :offline_redirect, :only => [:new, :create, :edit, :delete, :data]
+  before_action :offline_redirect, :only => [:new, :create, :edit, :destroy, :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
@@ -184,7 +184,7 @@ class TracesController < ApplicationController
     head :not_found
   end
 
-  def delete
+  def destroy
     trace = Trace.find(params[:id])
 
     if !trace.visible?

diff --git a/app/views/traces/show.html.erb b/app/views/traces/show.html.erb
index 0ebbd827fc53c75166f64a107b612fdc3eaf8856..b25ecad5376345b0475d2e4fee1efc80c2b730a3 100644 (file)
--- a/app/views/traces/show.html.erb
+++ b/app/views/traces/show.html.erb
@@ -59,6 +59,6 @@
     <% if current_user == @trace.user %>
       <%= link_to t(".edit_trace"), edit_trace_path(@trace), :class => "button" %>
     <% end %>
-    <%= button_to t(".delete_trace"), { :controller => "traces", :action => "delete", :id => @trace.id }, { :data => { :confirm => t(".confirm_delete") } } %>
+    <%= button_to t(".delete_trace"), { :controller => "traces", :action => "destroy", :method => :delete, :id => @trace.id }, { :data => { :confirm => t(".confirm_delete") } } %>
   </div>
 <% end %>

diff --git a/config/routes.rb b/config/routes.rb
index d936072d78e7cee4b6d2e12841671c8c14bff41a..d61da1ea93872cd20429b60728fa00a91bb5e86c 100644 (file)
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -208,7 +208,6 @@ OpenStreetMap::Application.routes.draw do
   get "/trace/create", :to => redirect(:path => "/traces/new")
   get "/trace/:id/data" => "traces#data", :id => /\d+/, :as => "trace_data"
   get "/trace/:id/edit", :to => redirect(:path => "/traces/%{id}/edit")
-  post "/trace/:id/delete" => "traces#delete", :id => /\d+/
 
   # diary pages
   resources :diary_entries, :path => "diary", :only => [:new, :create, :index] do

diff --git a/test/controllers/traces_controller_test.rb b/test/controllers/traces_controller_test.rb
index 45b0358f5f957538ff2bd0b77ade2cf0a088810a..059242af9957357f460078dd9981a9d6765d4c4d 100644 (file)
--- a/test/controllers/traces_controller_test.rb
+++ b/test/controllers/traces_controller_test.rb
@@ -115,8 +115,8 @@ class TracesControllerTest < ActionController::TestCase
       { :controller => "traces", :action => "update", :id => "1" }
     )
     assert_routing(
-      { :path => "/trace/1/delete", :method => :post },
-      { :controller => "traces", :action => "delete", :id => "1" }
+      { :path => "/traces/1", :method => :delete },
+      { :controller => "traces", :action => "destroy", :id => "1" }
     )
   end
 
@@ -637,39 +637,39 @@ class TracesControllerTest < ActionController::TestCase
     assert_equal new_details[:visibility], trace.visibility
   end
 
-  # Test deleting a trace
-  def test_delete
+  # Test destroying a trace
+  def test_destroy
     public_trace_file = create(:trace, :visibility => "public")
     deleted_trace_file = create(:trace, :deleted)
 
     # First with no auth
-    post :delete, :params => { :display_name => public_trace_file.user.display_name, :id => public_trace_file.id }
+    delete :destroy, :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 :delete, :params => { :display_name => public_trace_file.user.display_name, :id => public_trace_file.id }, :session => { :user => create(:user) }
+    delete :destroy, :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 :delete, :params => { :display_name => create(:user).display_name, :id => 0 }, :session => { :user => create(:user) }
+    delete :destroy, :params => { :display_name => create(:user).display_name, :id => 0 }, :session => { :user => create(:user) }
     assert_response :not_found
 
     # Now with a trace has already been deleted
-    post :delete, :params => { :display_name => deleted_trace_file.user.display_name, :id => deleted_trace_file.id }, :session => { :user => deleted_trace_file.user }
+    delete :destroy, :params => { :display_name => deleted_trace_file.user.display_name, :id => deleted_trace_file.id }, :session => { :user => deleted_trace_file.user }
     assert_response :not_found
 
     # 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 }
+    delete :destroy, :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 => :index, :display_name => public_trace_file.user.display_name
     trace = Trace.find(public_trace_file.id)
     assert_equal false, trace.visible
 
-    # Finally with a trace that is deleted by an admin
+    # Finally with a trace that is destroyed by an admin
     public_trace_file = create(:trace, :visibility => "public")
     admin = create(:administrator_user)
 
-    post :delete, :params => { :display_name => public_trace_file.user.display_name, :id => public_trace_file.id }, :session => { :user => admin }
+    delete :destroy, :params => { :display_name => public_trace_file.user.display_name, :id => public_trace_file.id }, :session => { :user => admin }
     assert_response :redirect
     assert_redirected_to :action => :index, :display_name => public_trace_file.user.display_name
     trace = Trace.find(public_trace_file.id)