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

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

Browse source
This commit is contained in:
Tom Hughes 2018-08-30 18:26:05 +01:00
commit a1b179fa38
11 changed files with 140 additions and 151 deletions
Download patch file Download diff file Expand all files Collapse all files

2
app/controllers/api_controller.rb
View file

@ -67,7 +67,7 @@ class ApiController < ApplicationController
if gpx_file.identifiable? if gpx_file.identifiable?
track << (XML::Node.new("name") << gpx_file.name) track << (XML::Node.new("name") << gpx_file.name)
track << (XML::Node.new("desc") << gpx_file.description) 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 end
else else
# use the anonymous track segment if the user hasn't allowed # use the anonymous track segment if the user hasn't allowed

43
app/controllers/traces_controller.rb
View file

@ -12,13 +12,13 @@ class TracesController < ApplicationController
before_action :check_api_writable, :only => [:api_create, :api_update, :api_delete] 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_read_gpx, :only => [:api_read, :api_data]
before_action :require_allow_write_gpx, :only => [:api_create, :api_update, :api_delete] 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] 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] 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.). # 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 # 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 # from display name, pick up user id if one user's traces only
display_name = params[:display_name] display_name = params[:display_name]
if display_name.present? if display_name.present?
@ -86,10 +86,10 @@ class TracesController < ApplicationController
end end
def mine def mine
redirect_to :action => :list, :display_name => current_user.display_name redirect_to :action => :index, :display_name => current_user.display_name
end end
def view def show
@trace = Trace.find(params[:id]) @trace = Trace.find(params[:id])
if @trace && @trace.visible? && if @trace && @trace.visible? &&
@ -97,11 +97,11 @@ class TracesController < ApplicationController
@title = t ".title", :name => @trace.name @title = t ".title", :name => @trace.name
else else
flash[:error] = t ".trace_not_found" flash[:error] = t ".trace_not_found"
redirect_to :action => "list" redirect_to :action => "index"
end end
rescue ActiveRecord::RecordNotFound rescue ActiveRecord::RecordNotFound
flash[:error] = t ".trace_not_found" flash[:error] = t ".trace_not_found"
redirect_to :action => "list" redirect_to :action => "index"
end end
def new def new
@ -126,7 +126,7 @@ class TracesController < ApplicationController
flash[:notice] = t ".trace_uploaded" 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 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 else
flash[:error] = t("traces.create.upload_failed") if @trace.valid? flash[:error] = t("traces.create.upload_failed") if @trace.valid?
@ -175,13 +175,24 @@ class TracesController < ApplicationController
head :forbidden head :forbidden
else else
@title = t ".title", :name => @trace.name @title = t ".title", :name => @trace.name
end
rescue ActiveRecord::RecordNotFound
head :not_found
end
if request.post? && params[:trace] def update
@trace.description = params[:trace][:description] @trace = Trace.find(params[:id])
@trace.tagstring = params[:trace][:tagstring]
@trace.visibility = params[:trace][:visibility] if !@trace.visible?
redirect_to :action => "view", :display_name => current_user.display_name if @trace.save head :not_found
end 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 end
rescue ActiveRecord::RecordNotFound rescue ActiveRecord::RecordNotFound
head :not_found head :not_found
@ -198,7 +209,7 @@ class TracesController < ApplicationController
trace.visible = false trace.visible = false
trace.save trace.save
flash[:notice] = t ".scheduled_for_deletion" 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 end
rescue ActiveRecord::RecordNotFound rescue ActiveRecord::RecordNotFound
head :not_found head :not_found
@ -413,4 +424,8 @@ class TracesController < ApplicationController
"public" "public"
end end
end end
def trace_params
params.require(:trace).permit(:description, :tagstring, :visibility)
end
end end

4
app/views/traces/_trace.html.erb
View file

@ -3,13 +3,13 @@
<td class="<%= cl %>"> <td class="<%= cl %>">
<% if STATUS != :gpx_offline %> <% if STATUS != :gpx_offline %>
<% if trace.inserted %> <% 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 %> <% else %>
<span class="trace_pending"><%= t '.pending' %></span> <span class="trace_pending"><%= t '.pending' %></span>
<% end %> <% end %>
<% end %> <% end %>
</td> </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 %>"> ... <span class="trace_summary" title="<%= trace.timestamp %>"> ...
<% if trace.inserted %> <% if trace.inserted %>
(<%= t '.count_points', :count => trace.size.to_s.gsub(/(\d)(?=(\d{3})+$)/,'\1,') %>) (<%= t '.count_points', :count => trace.size.to_s.gsub(/(\d)(?=(\d{3})+$)/,'\1,') %>)

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

@ -4,7 +4,7 @@
<img src="<%= url_for :controller => 'traces', :action => 'picture', :id => @trace.id, :display_name => @trace.user.display_name %>"> <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'> <div id='edit-trace-form' class='standard-form'>
<fieldset> <fieldset>

4
app/views/traces/georss.rss.builder
View file

@ -7,14 +7,14 @@ xml.rss("version" => "2.0",
xml.channel do xml.channel do
xml.title t(".title") xml.title t(".title")
xml.description 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.image do
xml.url image_url("mag_map-rss2.0.png") xml.url image_url("mag_map-rss2.0.png")
xml.title t(".title") xml.title t(".title")
xml.width 100 xml.width 100
xml.height 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 end
@traces.each do |trace| @traces.each do |trace|

4
app/views/traces/list.html.erb → app/views/traces/index.html.erb
View file

@ -5,11 +5,11 @@
<li><%= rss_link_to :action => 'georss', :display_name => @display_name, :tag => @tag %></li> <li><%= rss_link_to :action => 'georss', :display_name => @display_name, :tag => @tag %></li>
<li><%= link_to t('.upload_trace'), new_trace_path %></li> <li><%= link_to t('.upload_trace'), new_trace_path %></li>
<% if @tag %> <% 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> <li><%= link_to t('.see_my_traces'), :action => 'mine', :tag => nil, :page => nil %></li>
<% else %> <% else %>
<% if @display_name %> <% 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 %> <% end %>
<% if current_user && current_user != @target_user %> <% if current_user && current_user != @target_user %>
<li><%= link_to t('.see_my_traces'), :action => 'mine', :tag => nil, :page => nil %></li> <li><%= link_to t('.see_my_traces'), :action => 'mine', :tag => nil, :page => nil %></li>

8
app/views/traces/view.html.erb → app/views/traces/show.html.erb
View file

@ -40,7 +40,7 @@
<td><%= t '.tags' %></td> <td><%= t '.tags' %></td>
<td> <td>
<% unless @trace.tags.empty? %> <% 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 %> <% else %>
<i><%= t '.none' %></i> <i><%= t '.none' %></i>
<% end %> <% end %>
@ -57,10 +57,8 @@
<% if current_user && (current_user==@trace.user || current_user.administrator? || current_user.moderator?)%> <% if current_user && (current_user==@trace.user || current_user.administrator? || current_user.moderator?)%>
<div class="buttons"> <div class="buttons">
<% if current_user == @trace.user %> <% if current_user == @trace.user %>
<div> <%= link_to t('.edit_trace'), edit_trace_path(@trace), :class => "button" %>
<%= button_to t('.edit_track'), trace_edit_path(@trace) %>
</div>
<% end %> <% 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> </div>
<% end %> <% end %>

2
app/views/user/view.html.erb
View file

@ -56,7 +56,7 @@
<%= link_to t('.notes'), :controller => 'notes', :action=> 'mine' %> <%= link_to t('.notes'), :controller => 'notes', :action=> 'mine' %>
</li> </li>
<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> <span class='count-number'><%= number_with_delimiter(@user.traces.size) %></span>
</li> </li>

10
config/locales/en.yml
View file

@ -1723,9 +1723,11 @@ en:
visibility: "Visibility:" visibility: "Visibility:"
visibility_help: "what does this mean?" visibility_help: "what does this mean?"
visibility_help_url: "https://wiki.openstreetmap.org/wiki/Visibility_of_GPS_traces" visibility_help_url: "https://wiki.openstreetmap.org/wiki/Visibility_of_GPS_traces"
update:
updated: Trace updated
trace_optionals: trace_optionals:
tags: "Tags" tags: "Tags"
view: show:
title: "Viewing trace %{name}" title: "Viewing trace %{name}"
heading: "Viewing trace %{name}" heading: "Viewing trace %{name}"
pending: "PENDING" pending: "PENDING"
@ -1740,8 +1742,8 @@ en:
description: "Description:" description: "Description:"
tags: "Tags:" tags: "Tags:"
none: "None" none: "None"
edit_track: "Edit this trace" edit_trace: "Edit this trace"
delete_track: "Delete this trace" delete_trace: "Delete this trace"
trace_not_found: "Trace not found!" trace_not_found: "Trace not found!"
visibility: "Visibility:" visibility: "Visibility:"
confirm_delete: "Delete this trace?" confirm_delete: "Delete this trace?"
@ -1765,7 +1767,7 @@ en:
by: "by" by: "by"
in: "in" in: "in"
map: "map" map: "map"
list: index:
public_traces: "Public GPS traces" public_traces: "Public GPS traces"
my_traces: "My GPS traces" my_traces: "My GPS traces"
public_traces_from: "Public GPS traces from %{user}" public_traces_from: "Public GPS traces from %{user}"

22
config/routes.rb
View file

@ -189,30 +189,30 @@ OpenStreetMap::Application.routes.draw do
post "/preview/:type" => "site#preview", :as => :preview post "/preview/:type" => "site#preview", :as => :preview
# traces # traces
get "/user/:display_name/traces/tag/:tag/page/:page" => "traces#list", :page => /[1-9][0-9]*/ resources :traces, :except => [:show]
get "/user/:display_name/traces/tag/:tag" => "traces#list" get "/user/:display_name/traces/tag/:tag/page/:page" => "traces#index", :page => /[1-9][0-9]*/
get "/user/:display_name/traces/page/:page" => "traces#list", :page => /[1-9][0-9]*/ get "/user/:display_name/traces/tag/:tag" => "traces#index"
get "/user/:display_name/traces" => "traces#list" 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/tag/:tag/rss" => "traces#georss", :defaults => { :format => :rss }
get "/user/:display_name/traces/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/picture" => "traces#picture"
get "/user/:display_name/traces/:id/icon" => "traces#icon" 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/page/:page" => "traces#index", :page => /[1-9][0-9]*/
get "/traces/tag/:tag" => "traces#list" get "/traces/tag/:tag" => "traces#index"
get "/traces/page/:page" => "traces#list", :page => /[1-9][0-9]*/ get "/traces/page/:page" => "traces#index", :page => /[1-9][0-9]*/
get "/traces" => "traces#list"
get "/traces/tag/:tag/rss" => "traces#georss", :defaults => { :format => :rss } get "/traces/tag/:tag/rss" => "traces#georss", :defaults => { :format => :rss }
get "/traces/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/page/:page" => "traces#mine", :page => /[1-9][0-9]*/
get "/traces/mine/tag/:tag" => "traces#mine" get "/traces/mine/tag/:tag" => "traces#mine"
get "/traces/mine/page/:page" => "traces#mine" get "/traces/mine/page/:page" => "traces#mine"
get "/traces/mine" => "traces#mine" get "/traces/mine" => "traces#mine"
resources :traces, :only => [:new, :create]
post "/trace/create" => "traces#create" # remove after deployment post "/trace/create" => "traces#create" # remove after deployment
get "/trace/create", :to => redirect(:path => "/traces/new") get "/trace/create", :to => redirect(:path => "/traces/new")
get "/trace/:id/data" => "traces#data", :id => /\d+/, :as => "trace_data" 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+/ post "/trace/:id/delete" => "traces#delete", :id => /\d+/
# diary pages # diary pages

190
test/controllers/traces_controller_test.rb
View file

@ -56,35 +56,35 @@ class TracesControllerTest < ActionController::TestCase
assert_routing( assert_routing(
{ :path => "/traces", :method => :get }, { :path => "/traces", :method => :get },
{ :controller => "traces", :action => "list" } { :controller => "traces", :action => "index" }
) )
assert_routing( assert_routing(
{ :path => "/traces/page/1", :method => :get }, { :path => "/traces/page/1", :method => :get },
{ :controller => "traces", :action => "list", :page => "1" } { :controller => "traces", :action => "index", :page => "1" }
) )
assert_routing( assert_routing(
{ :path => "/traces/tag/tagname", :method => :get }, { :path => "/traces/tag/tagname", :method => :get },
{ :controller => "traces", :action => "list", :tag => "tagname" } { :controller => "traces", :action => "index", :tag => "tagname" }
) )
assert_routing( assert_routing(
{ :path => "/traces/tag/tagname/page/1", :method => :get }, { :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( assert_routing(
{ :path => "/user/username/traces", :method => :get }, { :path => "/user/username/traces", :method => :get },
{ :controller => "traces", :action => "list", :display_name => "username" } { :controller => "traces", :action => "index", :display_name => "username" }
) )
assert_routing( assert_routing(
{ :path => "/user/username/traces/page/1", :method => :get }, { :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( assert_routing(
{ :path => "/user/username/traces/tag/tagname", :method => :get }, { :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( assert_routing(
{ :path => "/user/username/traces/tag/tagname/page/1", :method => :get }, { :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( assert_routing(
@ -123,7 +123,7 @@ class TracesControllerTest < ActionController::TestCase
assert_routing( assert_routing(
{ :path => "/user/username/traces/1", :method => :get }, { :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( assert_routing(
{ :path => "/user/username/traces/1/picture", :method => :get }, { :path => "/user/username/traces/1/picture", :method => :get },
@ -151,12 +151,12 @@ class TracesControllerTest < ActionController::TestCase
{ :controller => "traces", :action => "data", :id => "1", :format => "xml" } { :controller => "traces", :action => "data", :id => "1", :format => "xml" }
) )
assert_routing( assert_routing(
{ :path => "/trace/1/edit", :method => :get }, { :path => "/traces/1/edit", :method => :get },
{ :controller => "traces", :action => "edit", :id => "1" } { :controller => "traces", :action => "edit", :id => "1" }
) )
assert_routing( assert_routing(
{ :path => "/trace/1/edit", :method => :post }, { :path => "/traces/1", :method => :put },
{ :controller => "traces", :action => "edit", :id => "1" } { :controller => "traces", :action => "update", :id => "1" }
) )
assert_routing( assert_routing(
{ :path => "/trace/1/delete", :method => :post }, { :path => "/trace/1/delete", :method => :post },
@ -164,8 +164,8 @@ class TracesControllerTest < ActionController::TestCase
) )
end end
# Check that the list of traces is displayed # Check that the index of traces is displayed
def test_list def test_index
user = create(:user) user = create(:user)
# The fourth test below is surpisingly sensitive to timestamp ordering when the timestamps are equal. # 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| 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") create(:tracetag, :trace => trace, :tag => "Birmingham")
end end
# First with the public list # First with the public index
get :list get :index
check_trace_list [trace_b, trace_a] check_trace_index [trace_b, trace_a]
# Restrict traces to those with a given tag # Restrict traces to those with a given tag
get :list, :params => { :tag => "London" } get :index, :params => { :tag => "London" }
check_trace_list [trace_a] check_trace_index [trace_a]
# Should see more when we are logged in # Should see more when we are logged in
get :list, :session => { :user => user } get :index, :session => { :user => user }
check_trace_list [trace_d, trace_c, trace_b, trace_a] check_trace_index [trace_d, trace_c, trace_b, trace_a]
# Again, we should see more when we are logged in # Again, we should see more when we are logged in
get :list, :params => { :tag => "London" }, :session => { :user => user } get :index, :params => { :tag => "London" }, :session => { :user => user }
check_trace_list [trace_c, trace_a] check_trace_index [trace_c, trace_a]
end end
# Check that I can get mine # Check that I can get mine
def test_list_mine def test_index_mine
user = create(:user) user = create(:user)
create(:trace, :visibility => "public") do |trace| create(:trace, :visibility => "public") do |trace|
create(:tracetag, :trace => trace, :tag => "Birmingham") create(:tracetag, :trace => trace, :tag => "Birmingham")
@ -214,15 +214,15 @@ class TracesControllerTest < ActionController::TestCase
# Now try when logged in # Now try when logged in
get :mine, :session => { :user => user } 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 # Fetch the actual index
get :list, :params => { :display_name => user.display_name }, :session => { :user => user } get :index, :params => { :display_name => user.display_name }, :session => { :user => user }
check_trace_list [trace_b] check_trace_index [trace_b]
end end
# Check the list of traces for a specific user # Check the index of traces for a specific user
def test_list_user def test_index_user
user = create(:user) user = create(:user)
second_user = create(:user) second_user = create(:user)
third_user = create(:user) third_user = create(:user)
@ -233,45 +233,45 @@ class TracesControllerTest < ActionController::TestCase
end end
# Test a user with no traces # Test a user with no traces
get :list, :params => { :display_name => second_user.display_name } get :index, :params => { :display_name => second_user.display_name }
check_trace_list [] check_trace_index []
# Test the user with the traces - should see only public ones # Test the user with the traces - should see only public ones
get :list, :params => { :display_name => user.display_name } get :index, :params => { :display_name => user.display_name }
check_trace_list [trace_b] check_trace_index [trace_b]
# Should still see only public ones when authenticated as another user # Should still see only public ones when authenticated as another user
get :list, :params => { :display_name => user.display_name }, :session => { :user => third_user } get :index, :params => { :display_name => user.display_name }, :session => { :user => third_user }
check_trace_list [trace_b] check_trace_index [trace_b]
# Should see all traces when authenticated as the target user # Should see all traces when authenticated as the target user
get :list, :params => { :display_name => user.display_name }, :session => { :user => user } get :index, :params => { :display_name => user.display_name }, :session => { :user => user }
check_trace_list [trace_c, trace_b] check_trace_index [trace_c, trace_b]
# Should only see traces with the correct tag when a tag is specified # 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 } get :index, :params => { :display_name => user.display_name, :tag => "London" }, :session => { :user => user }
check_trace_list [trace_c] check_trace_index [trace_c]
# Should get an error if the user does not exist # 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_response :not_found
assert_template "user/no_such_user" assert_template "user/no_such_user"
end end
# Check a multi-page list # Check a multi-page index
def test_list_paged def test_index_paged
# Create several pages worth of traces # Create several pages worth of traces
create_list(:trace, 50) create_list(:trace, 50)
# Try and get the list # Try and get the index
get :list get :index
assert_response :success assert_response :success
assert_select "table#trace_list tbody", :count => 1 do assert_select "table#trace_list tbody", :count => 1 do
assert_select "tr", :count => 20 assert_select "tr", :count => 20
end end
# Try and get the second page # Try and get the second page
get :list, :params => { :page => 2 } get :index, :params => { :page => 2 }
assert_response :success assert_response :success
assert_select "table#trace_list tbody", :count => 1 do assert_select "table#trace_list tbody", :count => 1 do
assert_select "tr", :count => 20 assert_select "tr", :count => 20
@ -299,55 +299,55 @@ class TracesControllerTest < ActionController::TestCase
check_trace_feed user.traces.tagged("Birmingham").visible_to_all check_trace_feed user.traces.tagged("Birmingham").visible_to_all
end end
# Test viewing a trace # Test showing a trace
def test_view def test_show
public_trace_file = create(:trace, :visibility => "public") public_trace_file = create(:trace, :visibility => "public")
# First with no auth, which should work since the trace is 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 } get :show, :params => { :display_name => public_trace_file.user.display_name, :id => public_trace_file.id }
check_trace_view public_trace_file check_trace_show public_trace_file
# Now with some other user, which should work since the trace is public # 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) } get :show, :params => { :display_name => public_trace_file.user.display_name, :id => public_trace_file.id }, :session => { :user => create(:user) }
check_trace_view public_trace_file check_trace_show public_trace_file
# And finally we should be able to do it with the owner of the trace # 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 } get :show, :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 check_trace_show public_trace_file
end end
# Check an anonymous trace can't be viewed by another user # 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") anon_trace_file = create(:trace, :visibility => "private")
# First with no auth # 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_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 # 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_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 # 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 } get :show, :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 check_trace_show anon_trace_file
end end
# Test viewing a trace that doesn't exist # Test showing a trace that doesn't exist
def test_view_not_found def test_show_not_found
deleted_trace_file = create(:trace, :deleted) deleted_trace_file = create(:trace, :deleted)
# First with a trace that has never existed # 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_response :redirect
assert_redirected_to :action => :list assert_redirected_to :action => :index
# Now with a trace that has been deleted # 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_response :redirect
assert_redirected_to :action => :list assert_redirected_to :action => :index
end end
# Test downloading a trace # 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 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 } post :create, :params => { :trace => { :gpx_file => file, :description => "New Trace", :tagstring => "new,trace", :visibility => "trackable" } }, :session => { :user => user }
assert_response :redirect 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] assert_match /file has been uploaded/, flash[:notice]
trace = Trace.order(:id => :desc).first trace = Trace.order(:id => :desc).first
assert_equal "a.gpx", trace.name assert_equal "a.gpx", trace.name
@ -594,7 +594,7 @@ class TracesControllerTest < ActionController::TestCase
# First with no auth # First with no auth
get :edit, :params => { :display_name => public_trace_file.user.display_name, :id => public_trace_file.id } get :edit, :params => { :display_name => public_trace_file.user.display_name, :id => public_trace_file.id }
assert_response :redirect 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 # 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) } 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 assert_response :success
end 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 # Test saving edits to a trace
def test_edit_post_with_details def test_update
public_trace_file = create(:trace, :visibility => "public") public_trace_file = create(:trace, :visibility => "public")
deleted_trace_file = create(:trace, :deleted) deleted_trace_file = create(:trace, :deleted)
@ -648,25 +622,25 @@ class TracesControllerTest < ActionController::TestCase
new_details = { :description => "Changed description", :tagstring => "new_tag", :visibility => "private" } new_details = { :description => "Changed description", :tagstring => "new_tag", :visibility => "private" }
# First with no auth # 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 assert_response :forbidden
# Now with some other user, which should fail # 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 assert_response :forbidden
# Now with a trace which doesn't exist # 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 assert_response :not_found
# Now with a trace which has been deleted # 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 assert_response :not_found
# Finally with a trace that we are allowed to edit # 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_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) trace = Trace.find(public_trace_file.id)
assert_equal new_details[:description], trace.description assert_equal new_details[:description], trace.description
assert_equal new_details[:tagstring], trace.tagstring 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 # 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 } 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_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) trace = Trace.find(public_trace_file.id)
assert_equal false, trace.visible 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 } post :delete, :params => { :display_name => public_trace_file.user.display_name, :id => public_trace_file.id }, :session => { :user => admin }
assert_response :redirect 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) trace = Trace.find(public_trace_file.id)
assert_equal false, trace.visible assert_equal false, trace.visible
end end
@ -1035,9 +1009,9 @@ class TracesControllerTest < ActionController::TestCase
end end
end end
def check_trace_list(traces) def check_trace_index(traces)
assert_response :success assert_response :success
assert_template "list" assert_template "index"
if !traces.empty? if !traces.empty?
assert_select "table#trace_list tbody", :count => 1 do assert_select "table#trace_list tbody", :count => 1 do
@ -1055,9 +1029,9 @@ class TracesControllerTest < ActionController::TestCase
end end
end end
def check_trace_view(trace) def check_trace_show(trace)
assert_response :success assert_response :success
assert_template "view" assert_template "show"
assert_select "table", :count => 1 do assert_select "table", :count => 1 do
assert_select "td", /^#{Regexp.quote(trace.name)} / assert_select "td", /^#{Regexp.quote(trace.name)} /