cst1/openstreetmap-website
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Use the conventional 'destroy' method name for destroying traces

Browse source
This commit is contained in:
Andy Allan 2019-12-04 11:59:18 +01:00
parent dba3940571
commit 44c6fdc273
5 changed files with 16 additions and 17 deletions
Download patch file Download diff file Expand all files Collapse all files

2
app/abilities/ability.rb
View file

@ -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?

6
app/controllers/traces_controller.rb
View file

@ -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?

2
app/views/traces/show.html.erb
View file

@ -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 %>

1
config/routes.rb
View file

@ -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

22
test/controllers/traces_controller_test.rb
View file

@ -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)