From d0089f0ce81ef10ca7ed43969d83d8b77014fa11 Mon Sep 17 00:00:00 2001 From: Andy Allan Date: Wed, 29 Aug 2018 17:58:37 +0800 Subject: [PATCH] Rename traces#list to traces#index --- app/controllers/traces_controller.rb | 12 +-- app/views/traces/georss.rss.builder | 4 +- .../traces/{list.html.erb => index.html.erb} | 4 +- app/views/traces/show.html.erb | 2 +- app/views/user/view.html.erb | 2 +- config/locales/en.yml | 2 +- config/routes.rb | 17 ++- test/controllers/traces_controller_test.rb | 102 +++++++++--------- 8 files changed, 72 insertions(+), 73 deletions(-) rename app/views/traces/{list.html.erb => index.html.erb} (89%) diff --git a/app/controllers/traces_controller.rb b/app/controllers/traces_controller.rb index b6982894d..c77d884b3 100644 --- a/app/controllers/traces_controller.rb +++ b/app/controllers/traces_controller.rb @@ -18,7 +18,7 @@ class TracesController < ApplicationController # 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,7 +86,7 @@ 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 show @@ -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? @@ -209,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 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 @@
  • <%= rss_link_to :action => 'georss', :display_name => @display_name, :tag => @tag %>
  • <%= link_to t('.upload_trace'), new_trace_path %>
    • <% if @tag %> -
  • <%= link_to t('.see_all_traces'), :controller => 'traces', :action => 'list', :display_name => nil, :tag => nil, :page => nil %>
    • +
  • <%= link_to t('.see_all_traces'), :controller => 'traces', :action => 'index', :display_name => nil, :tag => nil, :page => nil %>
  • <%= link_to t('.see_my_traces'), :action => 'mine', :tag => nil, :page => nil %>
    • <% else %> <% if @display_name %> -
  • <%= link_to t('.see_all_traces'), :controller => 'traces', :action => 'list', :display_name => nil, :tag => nil, :page => nil %>
    • +
  • <%= link_to t('.see_all_traces'), :controller => 'traces', :action => 'index', :display_name => nil, :tag => nil, :page => nil %>
    • <% end %> <% if current_user && current_user != @target_user %>
  • <%= link_to t('.see_my_traces'), :action => 'mine', :tag => nil, :page => nil %>
    • diff --git a/app/views/traces/show.html.erb b/app/views/traces/show.html.erb index 59c8a0a1f..b052d0337 100644 --- a/app/views/traces/show.html.erb +++ b/app/views/traces/show.html.erb @@ -40,7 +40,7 @@ <%= t '.tags' %> <% 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 %> <%= t '.none' %> <% 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' %>
  • - <%= 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 %> <%= number_with_delimiter(@user.traces.size) %>
    • diff --git a/config/locales/en.yml b/config/locales/en.yml index af54e0890..232d60ca7 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1766,7 +1766,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 410db3c25..e912871e4 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -189,26 +189,25 @@ 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" + 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#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, :edit, :update] + resources :traces, :only => [:index, :new, :create, :edit, :update] 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" diff --git a/test/controllers/traces_controller_test.rb b/test/controllers/traces_controller_test.rb index 29e359956..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( @@ -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 @@ -323,12 +323,12 @@ class TracesControllerTest < ActionController::TestCase # First with no auth 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 :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 :show, :params => { :display_name => anon_trace_file.user.display_name, :id => anon_trace_file.id }, :session => { :user => anon_trace_file.user } @@ -342,12 +342,12 @@ class TracesControllerTest < ActionController::TestCase # First with a trace that has never existed 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 :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 @@ -671,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 @@ -681,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 @@ -1009,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 -- 2.43.2