Merge remote-tracking branch 'openstreetmap/pull/1496'
authorTom Hughes <tom@compton.nu>
Wed, 22 Mar 2017 19:19:59 +0000 (19:19 +0000)
committerTom Hughes <tom@compton.nu>
Wed, 22 Mar 2017 19:19:59 +0000 (19:19 +0000)
13 files changed:
Gemfile
Gemfile.lock
app/views/layouts/_head.html.erb
config/example.application.yml
config/initializers/canonical_rails.rb [new file with mode: 0644]
test/controllers/browse_controller_test.rb
test/factories/changeset_comments.rb
test/factories/changesets.rb [new file with mode: 0644]
test/factories/old_node.rb
test/helpers/changeset_helper_test.rb
test/models/changeset_comment_test.rb
test/models/changeset_test.rb
test/models/node_test.rb

diff --git a/Gemfile b/Gemfile
index 72fc32683f2e02ee50f7a73b404735a40f8c4ab1..0f65c9c1dbfa5695b85fca93c1131dd98c170431 100644 (file)
--- 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"
 
index b3768cb7499cd6079a6f85d0dcc1e798d49e2a9b..03b033199303d99ded745205859e6cf9eb9ff224 100644 (file)
@@ -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
index ad91b01d1fff83f61eefd5f92437a16c5ab1a93e..377d55e9dedf821501c396855e70e0162d55261c 100644 (file)
   <%= 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) %>
index ec6048521e00f119b49e0f3d6c2a3bc71b381104..a10563f61fa904cc7fb2fd103c0b6840d119d2ad 100644 (file)
@@ -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 (file)
index 0000000..c443c8b
--- /dev/null
@@ -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
index 19f60035b89dc9fcc91b29315a794dbdf6be2328..567fc0576cabe29b7ba05173b9433748832feceb 100644 (file)
@@ -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
 
index d12c1b653c391f2c8d94ccd597ad38d27ae7980e..04644580fd30eda3d43446e4f48707e885fe28a0 100644 (file)
@@ -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 (file)
index 0000000..f76b731
--- /dev/null
@@ -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
index ecf096ecad11035fc848c5ea33975552968e9f17..403ffc0ea04b539c0ad2b4a5fea8463104c5b77d 100644 (file)
@@ -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
index 8342f99d251693eefbc7ba6e58fda0d9c7b8224f..9c45b62dc8228b79017dce349fe645f16186e67a 100644 (file)
@@ -1,16 +1,27 @@
 require "test_helper"
 
 class ChangesetHelperTest < ActionView::TestCase
-  fixtures :changesets, :users
-
   def test_changeset_user_link
-    assert_equal "<a href=\"/user/test2\">test2</a>", 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 %(<a href="/user/#{URI.encode(changeset.user.display_name)}">#{changeset.user.display_name}</a>), 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 <abbr title='Mon, 01 Jan 2007 00:00:00 \+0000'>.*</abbr> by anonymous$}, changeset_details(changesets(:normal_user_first_change))
-    assert_match %r{^Closed <abbr title='Created: Mon, 01 Jan 2007 00:00:00 \+0000&#10;Closed: Tue, 02 Jan 2007 00:00:00 \+0000'>.*</abbr> by <a href="/user/test2">test2</a>$}, 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 <abbr title='Mon, 01 Jan 2007 00:00:00 \+0000'>.*</abbr> 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 = %(<a href="/user/#{URI.encode(changeset.user.display_name)}">#{changeset.user.display_name}</a>)
+
+    assert_match %r{^Closed <abbr title='Created: Mon, 01 Jan 2007 00:00:00 \+0000&#10;Closed: Tue, 02 Jan 2007 00:00:00 \+0000'>.*</abbr> by #{user_link}$}, changeset_details(changeset)
   end
 end
index 64fbb36637e253fec6ea954ef009c5a114f0d86b..5ef0c1d93ee9de6fca964fdc5e383fff0dd13a00 100644 (file)
@@ -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
index 89200c9b5bfc1dd8a8073d91a49b56c41c9af752..a5176fa8bf7a351d5cd2ce70cb6b9f1582637a4e 100644 (file)
@@ -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
index 2afff3358da85e9197c835d49ada75c56e7ded09..6a19e242f2fe1a224f17b00046dfcd71fa2796f1 100644 (file)
@@ -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