diff --git a/app/abilities/ability.rb b/app/abilities/ability.rb index e31e3e930..3ba2ab707 100644 --- a/app/abilities/ability.rb +++ b/app/abilities/ability.rb @@ -22,7 +22,7 @@ class Ability can [:create, :update], :password can :read, Redaction can [:create, :destroy], :session - can [:read, :data, :georss], Trace + can [:read, :data], Trace can [:read, :create, :suspended, :auth_success, :auth_failure], User can :read, UserBlock end diff --git a/app/controllers/traces/feeds_controller.rb b/app/controllers/traces/feeds_controller.rb new file mode 100644 index 000000000..293fbab3b --- /dev/null +++ b/app/controllers/traces/feeds_controller.rb @@ -0,0 +1,20 @@ +module Traces + class FeedsController < ApplicationController + before_action :authorize_web + before_action :set_locale + before_action :check_database_readable + + authorize_resource :class => Trace + + def show + @traces = Trace.visible_to_all.visible + + @traces = @traces.joins(:user).where(:users => { :display_name => params[:display_name] }) if params[:display_name] + + @traces = @traces.tagged(params[:tag]) if params[:tag] + @traces = @traces.order("timestamp DESC") + @traces = @traces.limit(20) + @traces = @traces.includes(:user) + end + end +end diff --git a/app/controllers/traces_controller.rb b/app/controllers/traces_controller.rb index d723bac5b..14648dc9c 100644 --- a/app/controllers/traces_controller.rb +++ b/app/controllers/traces_controller.rb @@ -2,7 +2,7 @@ class TracesController < ApplicationController include UserMethods include PaginationMethods - layout "site", :except => :georss + layout "site" before_action :authorize_web before_action :set_locale @@ -192,17 +192,6 @@ class TracesController < ApplicationController head :not_found end - def georss - @traces = Trace.visible_to_all.visible - - @traces = @traces.joins(:user).where(:users => { :display_name => params[:display_name] }) if params[:display_name] - - @traces = @traces.tagged(params[:tag]) if params[:tag] - @traces = @traces.order("timestamp DESC") - @traces = @traces.limit(20) - @traces = @traces.includes(:user) - end - private def do_create(file, tags, description, visibility) diff --git a/app/views/traces/_description.html.erb b/app/views/traces/feeds/_description.html.erb similarity index 100% rename from app/views/traces/_description.html.erb rename to app/views/traces/feeds/_description.html.erb diff --git a/app/views/traces/georss.rss.builder b/app/views/traces/feeds/show.rss.builder similarity index 88% rename from app/views/traces/georss.rss.builder rename to app/views/traces/feeds/show.rss.builder index ad5bd45b2..dc10a12d6 100644 --- a/app/views/traces/georss.rss.builder +++ b/app/views/traces/feeds/show.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 => :index, :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 => :index, :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/index.html.erb b/app/views/traces/index.html.erb index 4311ba082..dfa5f03de 100644 --- a/app/views/traces/index.html.erb +++ b/app/views/traces/index.html.erb @@ -16,8 +16,8 @@
- <%= link_to({ :action => :georss, :display_name => @target_user&.display_name, :tag => params[:tag] }, - { :class => "btn btn-secondary btn-sm" }) do %> + <%= link_to traces_feed_path(:display_name => @target_user&.display_name, :tag => params[:tag]), + :class => "btn btn-secondary btn-sm" do %> @@ -66,7 +66,7 @@ <% end %> <% content_for :auto_discovery_link_tag do %> - <%= auto_discovery_link_tag :rss, :action => "georss", :display_name => @target_user&.display_name, :tag => params[:tag] %> + <%= auto_discovery_link_tag :rss, traces_feed_path(:display_name => @target_user&.display_name, :tag => params[:tag]) %> <% end %> <% if @traces.size > 0 %> diff --git a/config/locales/en.yml b/config/locales/en.yml index a112f3b0d..ba70cbfb6 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -2616,13 +2616,14 @@ en: offline: heading: "GPX Storage Offline" message: "The GPX file storage and upload system is currently unavailable." - georss: - title: "OpenStreetMap GPS Traces" - description: - description_with_count: - one: "GPX file with %{count} point from %{user}" - other: "GPX file with %{count} points from %{user}" - description_without_count: "GPX file from %{user}" + feeds: + show: + title: "OpenStreetMap GPS Traces" + description: + description_with_count: + one: "GPX file with %{count} point from %{user}" + other: "GPX file with %{count} points from %{user}" + description_without_count: "GPX file from %{user}" application: permission_denied: You do not have permission to access that action require_cookies: diff --git a/config/routes.rb b/config/routes.rb index d858d80b1..6efd17759 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -211,30 +211,30 @@ OpenStreetMap::Application.routes.draw do post "/preview/:type" => "site#preview", :as => :preview # traces - resources :traces, :except => [:show] + resources :traces, :id => /\d+/, :except => [:show] get "/user/:display_name/traces/tag/:tag/page/:page", :page => /[1-9][0-9]*/, :to => redirect(:path => "/user/%{display_name}/traces/tag/%{tag}") get "/user/:display_name/traces/tag/:tag" => "traces#index" get "/user/:display_name/traces/page/:page", :page => /[1-9][0-9]*/, :to => redirect(:path => "/user/%{display_name}/traces") 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", :as => "show_trace" - scope "/user/:display_name/traces/:trace_id", :module => :traces do + get "/user/:display_name/traces/:id" => "traces#show", :id => /\d+/, :as => "show_trace" + scope "/user/:display_name/traces/:trace_id", :module => :traces, :trace_id => /\d+/ do get "picture" => "pictures#show", :as => "trace_picture" get "icon" => "icons#show", :as => "trace_icon" end get "/traces/tag/:tag/page/:page", :page => /[1-9][0-9]*/, :to => redirect(:path => "/traces/tag/%{tag}") get "/traces/tag/:tag" => "traces#index" get "/traces/page/:page", :page => /[1-9][0-9]*/, :to => redirect(:path => "/traces") - 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", :page => /[1-9][0-9]*/, :to => redirect(:path => "/traces/mine/tag/%{tag}") get "/traces/mine/tag/:tag" => "traces#mine" get "/traces/mine/page/:page", :page => /[1-9][0-9]*/, :to => redirect(:path => "/traces/mine") get "/traces/mine" => "traces#mine" 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") + get "/trace/:id/edit", :id => /\d+/, :to => redirect(:path => "/traces/%{id}/edit") + + namespace :traces, :path => "" do + resource :feed, :path => "(/user/:display_name)/traces(/tag/:tag)/rss", :only => :show, :defaults => { :format => :rss } + end # diary pages resources :diary_entries, :path => "diary", :only => [:new, :create, :index] do diff --git a/test/controllers/traces/feeds_controller_test.rb b/test/controllers/traces/feeds_controller_test.rb new file mode 100644 index 000000000..b0642bf5e --- /dev/null +++ b/test/controllers/traces/feeds_controller_test.rb @@ -0,0 +1,109 @@ +require "test_helper" + +module Traces + class FeedsControllerTest < ActionDispatch::IntegrationTest + ## + # test all routes which lead to this controller + def test_routes + assert_routing( + { :path => "/traces/rss", :method => :get }, + { :controller => "traces/feeds", :action => "show", :format => :rss } + ) + assert_routing( + { :path => "/traces/tag/tagname/rss", :method => :get }, + { :controller => "traces/feeds", :action => "show", :tag => "tagname", :format => :rss } + ) + assert_routing( + { :path => "/user/username/traces/rss", :method => :get }, + { :controller => "traces/feeds", :action => "show", :display_name => "username", :format => :rss } + ) + assert_routing( + { :path => "/user/username/traces/tag/tagname/rss", :method => :get }, + { :controller => "traces/feeds", :action => "show", :display_name => "username", :tag => "tagname", :format => :rss } + ) + end + + def test_show + user = create(:user) + # The fourth test below is surprisingly sensitive to timestamp ordering when the timestamps are equal. + trace_a = create(:trace, :visibility => "public", :timestamp => 4.seconds.ago) do |trace| + create(:tracetag, :trace => trace, :tag => "London") + end + trace_b = create(:trace, :visibility => "public", :timestamp => 3.seconds.ago) do |trace| + create(:tracetag, :trace => trace, :tag => "Birmingham") + end + create(:trace, :visibility => "private", :user => user, :timestamp => 2.seconds.ago) do |trace| + create(:tracetag, :trace => trace, :tag => "London") + end + create(:trace, :visibility => "private", :user => user, :timestamp => 1.second.ago) do |trace| + create(:tracetag, :trace => trace, :tag => "Birmingham") + end + + # First with the public feed + get traces_feed_path + check_trace_feed [trace_b, trace_a] + + # Restrict traces to those with a given tag + get traces_feed_path(:tag => "London") + check_trace_feed [trace_a] + end + + def test_show_user + user = create(:user) + second_user = create(:user) + create(:user) + create(:trace) + trace_b = create(:trace, :visibility => "public", :timestamp => 4.seconds.ago, :user => user) + trace_c = create(:trace, :visibility => "public", :timestamp => 3.seconds.ago, :user => user) do |trace| + create(:tracetag, :trace => trace, :tag => "London") + end + create(:trace, :visibility => "private") + + # Test a user with no traces + get traces_feed_path(:display_name => second_user) + check_trace_feed [] + + # Test the user with the traces - should see only public ones + get traces_feed_path(:display_name => user) + check_trace_feed [trace_c, trace_b] + + # Should only see traces with the correct tag when a tag is specified + get traces_feed_path(:display_name => user, :tag => "London") + check_trace_feed [trace_c] + + # Should no traces if the user does not exist + get traces_feed_path(:display_name => "UnknownUser") + check_trace_feed [] + end + + private + + def check_trace_feed(traces) + assert_response :success + assert_template "traces/feeds/show" + assert_equal "application/rss+xml", @response.media_type + assert_select "rss", :count => 1 do + assert_select "channel", :count => 1 do + assert_select "title" + assert_select "description" + assert_select "link" + assert_select "image" + assert_select "item", :count => traces.length do |items| + traces.zip(items).each do |trace, item| + assert_select item, "title", trace.name + assert_select item, "link", "http://www.example.com/user/#{ERB::Util.u(trace.user.display_name)}/traces/#{trace.id}" + assert_select item, "guid", "http://www.example.com/user/#{ERB::Util.u(trace.user.display_name)}/traces/#{trace.id}" + assert_select item, "description" do + assert_dom_encoded do + assert_select "img[src='#{trace_icon_url trace.user, trace}']" + end + end + # assert_select item, "dc:creator", trace.user.display_name + assert_select item, "pubDate", trace.timestamp.rfc822 + end + end + end + end + end + end +end diff --git a/test/controllers/traces_controller_test.rb b/test/controllers/traces_controller_test.rb index c5f503d62..78d714289 100644 --- a/test/controllers/traces_controller_test.rb +++ b/test/controllers/traces_controller_test.rb @@ -30,23 +30,6 @@ class TracesControllerTest < ActionDispatch::IntegrationTest { :controller => "traces", :action => "mine", :tag => "tagname" } ) - assert_routing( - { :path => "/traces/rss", :method => :get }, - { :controller => "traces", :action => "georss", :format => :rss } - ) - assert_routing( - { :path => "/traces/tag/tagname/rss", :method => :get }, - { :controller => "traces", :action => "georss", :tag => "tagname", :format => :rss } - ) - assert_routing( - { :path => "/user/username/traces/rss", :method => :get }, - { :controller => "traces", :action => "georss", :display_name => "username", :format => :rss } - ) - assert_routing( - { :path => "/user/username/traces/tag/tagname/rss", :method => :get }, - { :controller => "traces", :action => "georss", :display_name => "username", :tag => "tagname", :format => :rss } - ) - assert_routing( { :path => "/user/username/traces/1", :method => :get }, { :controller => "traces", :action => "show", :display_name => "username", :id => "1" } @@ -333,61 +316,6 @@ class TracesControllerTest < ActionDispatch::IntegrationTest end end - # Check the RSS feed - def test_rss - user = create(:user) - # The fourth test below is surprisingly sensitive to timestamp ordering when the timestamps are equal. - trace_a = create(:trace, :visibility => "public", :timestamp => 4.seconds.ago) do |trace| - create(:tracetag, :trace => trace, :tag => "London") - end - trace_b = create(:trace, :visibility => "public", :timestamp => 3.seconds.ago) do |trace| - create(:tracetag, :trace => trace, :tag => "Birmingham") - end - create(:trace, :visibility => "private", :user => user, :timestamp => 2.seconds.ago) do |trace| - create(:tracetag, :trace => trace, :tag => "London") - end - create(:trace, :visibility => "private", :user => user, :timestamp => 1.second.ago) do |trace| - create(:tracetag, :trace => trace, :tag => "Birmingham") - end - - # First with the public feed - get traces_rss_path - check_trace_feed [trace_b, trace_a] - - # Restrict traces to those with a given tag - get traces_rss_path(:tag => "London") - check_trace_feed [trace_a] - end - - # Check the RSS feed for a specific user - def test_rss_user - user = create(:user) - second_user = create(:user) - create(:user) - create(:trace) - trace_b = create(:trace, :visibility => "public", :timestamp => 4.seconds.ago, :user => user) - trace_c = create(:trace, :visibility => "public", :timestamp => 3.seconds.ago, :user => user) do |trace| - create(:tracetag, :trace => trace, :tag => "London") - end - create(:trace, :visibility => "private") - - # Test a user with no traces - get traces_rss_path(:display_name => second_user.display_name) - check_trace_feed [] - - # Test the user with the traces - should see only public ones - get traces_rss_path(:display_name => user.display_name) - check_trace_feed [trace_c, trace_b] - - # Should only see traces with the correct tag when a tag is specified - get traces_rss_path(:display_name => user.display_name, :tag => "London") - check_trace_feed [trace_c] - - # Should no traces if the user does not exist - get traces_rss_path(:display_name => "UnknownUser") - check_trace_feed [] - end - # Test showing a trace def test_show public_trace_file = create(:trace, :visibility => "public") @@ -721,34 +649,6 @@ class TracesControllerTest < ActionDispatch::IntegrationTest private - def check_trace_feed(traces) - assert_response :success - assert_template "georss" - assert_equal "application/rss+xml", @response.media_type - assert_select "rss", :count => 1 do - assert_select "channel", :count => 1 do - assert_select "title" - assert_select "description" - assert_select "link" - assert_select "image" - assert_select "item", :count => traces.length do |items| - traces.zip(items).each do |trace, item| - assert_select item, "title", trace.name - assert_select item, "link", "http://www.example.com/user/#{ERB::Util.u(trace.user.display_name)}/traces/#{trace.id}" - assert_select item, "guid", "http://www.example.com/user/#{ERB::Util.u(trace.user.display_name)}/traces/#{trace.id}" - assert_select item, "description" do - assert_dom_encoded do - assert_select "img[src='#{trace_icon_url trace.user, trace}']" - end - end - # assert_select item, "dc:creator", trace.user.display_name - assert_select item, "pubDate", trace.timestamp.rfc822 - end - end - end - end - end - def check_trace_index(traces) assert_response :success assert_template "index"