From: Tom Hughes Date: Wed, 22 Mar 2017 19:19:59 +0000 (+0000) Subject: Merge remote-tracking branch 'openstreetmap/pull/1496' X-Git-Tag: live~4020 X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/b78bb9f6313f3139ac83784df52bf39a5619d3e7?hp=4ed160778492b8a2c5df66ea194233788a815e11 Merge remote-tracking branch 'openstreetmap/pull/1496' --- diff --git a/Gemfile b/Gemfile index 72fc32683..0f65c9c1d 100644 --- a/Gemfile +++ b/Gemfile @@ -95,6 +95,9 @@ gem "kgio" # Load secure_headers for Content-Security-Policy support gem "secure_headers" +# Load canonical-rails to generate canonical URLs +gem "canonical-rails" + # Used to generate logstash friendly log files gem "logstasher" diff --git a/Gemfile.lock b/Gemfile.lock index b3768cb74..03b033199 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -42,11 +42,13 @@ GEM public_suffix (~> 2.0, >= 2.0.2) arel (6.0.4) ast (2.3.0) - autoprefixer-rails (6.7.5) + autoprefixer-rails (6.7.7.1) execjs bigdecimal (1.1.0) builder (3.2.3) - capybara (2.12.1) + canonical-rails (0.1.2) + rails (>= 4.1, < 5.1) + capybara (2.13.0) addressable mime-types (>= 1.16) nokogiri (>= 1.3.3) @@ -67,7 +69,7 @@ GEM colorize (0.8.1) composite_primary_keys (8.1.4) activerecord (~> 4.2.0) - concurrent-ruby (1.0.4) + concurrent-ruby (1.0.5) coveralls (0.8.19) json (>= 1.8, < 3) simplecov (~> 0.12.0) @@ -89,9 +91,9 @@ GEM factory_girl_rails (4.8.0) factory_girl (~> 4.8.0) railties (>= 3.0.0) - faraday (0.10.1) + faraday (0.11.0) multipart-post (>= 1.2, < 3) - fspath (3.0.3) + fspath (3.1.0) geoip (1.6.3) globalid (0.3.7) activesupport (>= 4.1.0) @@ -100,7 +102,7 @@ GEM htmlentities (4.3.4) http_accept_language (2.0.5) i18n (0.8.1) - i18n-js (3.0.0.rc15) + i18n-js (3.0.0.rc16) i18n (~> 0.6, >= 0.6.6) image_optim (0.24.2) exifr (~> 1.2, >= 1.2.2) @@ -108,13 +110,13 @@ GEM image_size (~> 1.5) in_threads (~> 1.3) progress (~> 3.0, >= 3.0.1) - image_optim_rails (0.2.0) + image_optim_rails (0.3.0) image_optim (~> 0.24.0) rails sprockets image_size (1.5.0) - in_threads (1.3.1) - jquery-rails (4.2.2) + in_threads (1.4.0) + jquery-rails (4.3.1) rails-dom-testing (>= 1, < 3) railties (>= 4.2.0) thor (>= 0.14, < 2.0) @@ -138,11 +140,10 @@ GEM sprockets (>= 2, < 4) sprockets-rails (>= 2, < 4) tilt - libv8 (3.16.14.17) + libv8 (3.16.14.19) libxml-ruby (3.0.0) logstash-event (1.2.02) - logstasher (1.1.1) - activerecord (>= 4.0) + logstasher (1.2.1) activesupport (>= 4.0) logstash-event (~> 1.2.0) request_store @@ -159,7 +160,7 @@ GEM multi_json (1.12.1) multi_xml (0.6.0) multipart-post (2.0.0) - nokogiri (1.7.0.1) + nokogiri (1.7.1) mini_portile2 (~> 2.1.0) nokogumbo (1.4.9) nokogiri @@ -211,8 +212,8 @@ GEM mimemagic (= 0.3.0) parser (2.4.0.0) ast (~> 2.2) - pg (0.19.0) - poltergeist (1.13.0) + pg (0.20.0) + poltergeist (1.14.0) capybara (~> 2.1) cliver (~> 0.3.1) websocket-driver (>= 0.2.0) @@ -283,7 +284,7 @@ GEM sprockets (>= 2.8, < 4.0) sprockets-rails (>= 2.0, < 4.0) tilt (>= 1.1, < 3) - secure_headers (3.6.1) + secure_headers (3.6.2) useragent simplecov (0.12.0) docile (~> 1.1.0) @@ -304,12 +305,12 @@ GEM ref thor (0.19.4) thread_safe (0.3.6) - tilt (2.0.6) + tilt (2.0.7) timecop (0.8.1) tins (1.13.2) tzinfo (1.2.2) thread_safe (~> 0.1) - uglifier (3.0.4) + uglifier (3.1.9) execjs (>= 0.3.0, < 3) unicode-display_width (1.1.3) useragent (0.16.8) @@ -334,6 +335,7 @@ DEPENDENCIES actionpack-page_caching autoprefixer-rails bigdecimal (~> 1.1.0) + canonical-rails coffee-rails (~> 4.1.0) composite_primary_keys (~> 8.1.0) coveralls diff --git a/app/views/layouts/_head.html.erb b/app/views/layouts/_head.html.erb index ad91b01d1..377d55e9d 100644 --- a/app/views/layouts/_head.html.erb +++ b/app/views/layouts/_head.html.erb @@ -24,7 +24,10 @@ <%= tag("meta", { :name => "msapplication-TileColor", :content => "#00a300" }) %> <%= tag("meta", { :name => "msapplication-TileImage", :content => image_path("mstile-144x144.png") }) %> <%= tag("meta", { :name => "theme-color", :content => "#ffffff" }) %> - <%= tag("link", { :rel => "publisher", :href => "https://plus.google.com/111953119785824514010" }) %> + <%= canonical_tag %> + <% if defined?(PUBLISHER_URL) -%> + <%= tag("link", { :rel => "publisher", :href => PUBLISHER_URL }) %> + <% end -%> <%= tag("link", { :rel => "search", :type => "application/opensearchdescription+xml", :title => "OpenStreetMap Search", :href => asset_path("osm.xml") }) %> <%= tag("meta", { :name => "description", :content => "OpenStreetMap is the free wiki world map." }) %> <%= opengraph_tags(@title) %> diff --git a/config/example.application.yml b/config/example.application.yml index ec6048521..a10563f61 100644 --- a/config/example.application.yml +++ b/config/example.application.yml @@ -1,6 +1,9 @@ defaults: &defaults - # The server URL - server_url: "www.openstreetmap.org" + # The server protocol and host + server_protocol: "http" + server_url: "openstreetmap.example.com" + # Publisher + #publisher_url: "" # The generator generator: "OpenStreetMap server" copyright_owner: "OpenStreetMap and contributors" diff --git a/config/initializers/canonical_rails.rb b/config/initializers/canonical_rails.rb new file mode 100644 index 000000000..c443c8bf8 --- /dev/null +++ b/config/initializers/canonical_rails.rb @@ -0,0 +1,22 @@ +CanonicalRails.setup do |config| + # Force the protocol. If you do not specify, the protocol will be based on the incoming request's protocol. + + config.protocol = "#{SERVER_PROTOCOL}://" + + # This is the main host, not just the TLD, omit slashes and protocol. If you have more than one, pick the one you want to rank in search results. + + config.host = SERVER_URL + + # http://en.wikipedia.org/wiki/URL_normalization + # Trailing slash represents semantics of a directory, ie a collection view - implying an :index get route; + # otherwise we have to assume semantics of an instance of a resource type, a member view - implying a :show get route + # + # Acts as a whitelist for routes to have trailing slashes + + config.collection_actions = [:index] + + # Parameter spamming can cause index dilution by creating seemingly different URLs with identical or near-identical content. + # Unless whitelisted, these parameters will be omitted + + config.whitelisted_parameters = [] +end diff --git a/test/controllers/browse_controller_test.rb b/test/controllers/browse_controller_test.rb index 19f60035b..567fc0576 100644 --- a/test/controllers/browse_controller_test.rb +++ b/test/controllers/browse_controller_test.rb @@ -75,15 +75,16 @@ class BrowseControllerTest < ActionController::TestCase end def test_read_changeset_hidden_comments - create_list(:changeset_comment, 3) - create(:changeset_comment, :visible => false) + changeset = create(:changeset) + create_list(:changeset_comment, 3, :changeset => changeset) + create(:changeset_comment, :visible => false, :changeset => changeset) - browse_check "changeset", changesets(:normal_user_closed_change).id, "browse/changeset" + browse_check "changeset", changeset.id, "browse/changeset" assert_select "div.changeset-comments ul li", :count => 3 session[:user] = create(:moderator_user).id - browse_check "changeset", changesets(:normal_user_closed_change).id, "browse/changeset" + browse_check "changeset", changeset.id, "browse/changeset" assert_select "div.changeset-comments ul li", :count => 4 end diff --git a/test/factories/changeset_comments.rb b/test/factories/changeset_comments.rb index d12c1b653..04644580f 100644 --- a/test/factories/changeset_comments.rb +++ b/test/factories/changeset_comments.rb @@ -3,8 +3,7 @@ FactoryGirl.define do sequence(:body) { |n| "Changeset comment #{n}" } visible true - # FIXME: needs changeset factory - changeset_id 3 + changeset association :author, :factory => :user end diff --git a/test/factories/changesets.rb b/test/factories/changesets.rb new file mode 100644 index 000000000..f76b73126 --- /dev/null +++ b/test/factories/changesets.rb @@ -0,0 +1,8 @@ +FactoryGirl.define do + factory :changeset do + created_at Time.now.utc + closed_at Time.now.utc + 1.day + + user + end +end diff --git a/test/factories/old_node.rb b/test/factories/old_node.rb index ecf096eca..403ffc0ea 100644 --- a/test/factories/old_node.rb +++ b/test/factories/old_node.rb @@ -3,8 +3,7 @@ FactoryGirl.define do latitude 1 * GeoRecord::SCALE longitude 1 * GeoRecord::SCALE - # FIXME: needs changeset factory - changeset_id 1 + changeset # FIXME: needs node factory node_id 1000 diff --git a/test/helpers/changeset_helper_test.rb b/test/helpers/changeset_helper_test.rb index 8342f99d2..9c45b62dc 100644 --- a/test/helpers/changeset_helper_test.rb +++ b/test/helpers/changeset_helper_test.rb @@ -1,16 +1,27 @@ require "test_helper" class ChangesetHelperTest < ActionView::TestCase - fixtures :changesets, :users - def test_changeset_user_link - assert_equal "test2", changeset_user_link(changesets(:public_user_first_change)) - assert_equal "anonymous", changeset_user_link(changesets(:normal_user_first_change)) - assert_equal "deleted", changeset_user_link(changesets(:deleted_user_first_change)) + changeset = create(:changeset) + assert_equal %(#{changeset.user.display_name}), changeset_user_link(changeset) + + changeset = create(:changeset, :user => create(:user, :data_public => false)) + assert_equal "anonymous", changeset_user_link(changeset) + + changeset = create(:changeset, :user => create(:user, :deleted)) + assert_equal "deleted", changeset_user_link(changeset) end def test_changeset_details - assert_match %r{^Created .* by anonymous$}, changeset_details(changesets(:normal_user_first_change)) - assert_match %r{^Closed .* by test2$}, changeset_details(changesets(:public_user_closed_change)) + changeset = create(:changeset, :created_at => Time.utc(2007, 1, 1, 0, 0, 0), :user => create(:user, :data_public => false)) + # We need to explicitly reset the closed_at to some point in the future, and avoid the before_save callback + changeset.update_column(:closed_at, Time.now.utc + 1.day) # rubocop:disable Rails/SkipsModelValidations + + assert_match %r{^Created .* by anonymous$}, changeset_details(changeset) + + changeset = create(:changeset, :created_at => Time.utc(2007, 1, 1, 0, 0, 0), :closed_at => Time.utc(2007, 1, 2, 0, 0, 0)) + user_link = %(#{changeset.user.display_name}) + + assert_match %r{^Closed .* by #{user_link}$}, changeset_details(changeset) end end diff --git a/test/models/changeset_comment_test.rb b/test/models/changeset_comment_test.rb index 64fbb3663..5ef0c1d93 100644 --- a/test/models/changeset_comment_test.rb +++ b/test/models/changeset_comment_test.rb @@ -33,8 +33,9 @@ class ChangesetCommentTest < ActiveSupport::TestCase end def test_comments_of_changeset_count - create_list(:changeset_comment, 3, :changeset_id => changesets(:normal_user_closed_change).id) - assert_equal 3, Changeset.find(changesets(:normal_user_closed_change).id).comments.count + changeset = create(:changeset) + create_list(:changeset_comment, 3, :changeset_id => changeset.id) + assert_equal 3, Changeset.find(changeset.id).comments.count end def test_body_valid diff --git a/test/models/changeset_test.rb b/test/models/changeset_test.rb index 89200c9b5..a5176fa8b 100644 --- a/test/models/changeset_test.rb +++ b/test/models/changeset_test.rb @@ -1,12 +1,6 @@ require "test_helper" class ChangesetTest < ActiveSupport::TestCase - api_fixtures - - def test_changeset_count - assert_equal 9, Changeset.count - end - def test_from_xml_no_text no_text = "" message_create = assert_raise(OSM::APIBadXMLError) do diff --git a/test/models/node_test.rb b/test/models/node_test.rb index 2afff3358..6a19e242f 100644 --- a/test/models/node_test.rb +++ b/test/models/node_test.rb @@ -50,14 +50,15 @@ class NodeTest < ActiveSupport::TestCase # Check that you can create a node and store it def test_create + changeset = create(:changeset) node_template = Node.new( :latitude => 12.3456, :longitude => 65.4321, - :changeset_id => changesets(:normal_user_first_change).id, + :changeset_id => changeset.id, :visible => 1, :version => 1 ) - assert node_template.create_with_history(changesets(:normal_user_first_change).user) + assert node_template.create_with_history(changeset.user) node = Node.find(node_template.id) assert_not_nil node