Merge remote-tracking branch 'openstreetmap/pull/1508'

diff --git a/app/models/node.rb b/app/models/node.rb
index f8559dca75d74431153342f4c4c987281a5354e8..6346907f1ef88354ee098f1ee8c72c774898b1ad 100644 (file)
--- a/app/models/node.rb
+++ b/app/models/node.rb
@@ -185,8 +185,8 @@ class Node < ActiveRecord::Base
     add_metadata_to_xml_node(el, self, changeset_cache, user_display_name_cache)
 
     if visible?
-      el["lat"] = lat.to_s
-      el["lon"] = lon.to_s
+      el["lat"] = format("%.7f", lat)
+      el["lon"] = format("%.7f", lon)
     end
 
     add_tags_to_xml_node(el, node_tags)

diff --git a/test/controllers/api_controller_test.rb b/test/controllers/api_controller_test.rb
index 5aac44a4fe7242b0184116e950a713092932cfef..7d249acf333a24dc70404de41719cb81d541a9d9 100644 (file)
--- a/test/controllers/api_controller_test.rb
+++ b/test/controllers/api_controller_test.rb
@@ -69,7 +69,7 @@ class ApiControllerTest < ActionController::TestCase
     assert_response :success, "Expected scucess with the map call"
     assert_select "osm[version='#{API_VERSION}'][generator='#{GENERATOR}']", :count => 1 do
       assert_select "bounds[minlon='#{minlon}'][minlat='#{minlat}'][maxlon='#{maxlon}'][maxlat='#{maxlat}']", :count => 1
-      assert_select "node[id='#{node.id}'][lat='#{node.lat}'][lon='#{node.lon}'][version='#{node.version}'][changeset='#{node.changeset_id}'][visible='#{node.visible}'][timestamp='#{node.timestamp.xmlschema}']", :count => 1 do
+      assert_select "node[id='#{node.id}'][lat='#{format('%.7f', node.lat)}'][lon='#{format('%.7f', node.lon)}'][version='#{node.version}'][changeset='#{node.changeset_id}'][visible='#{node.visible}'][timestamp='#{node.timestamp.xmlschema}']", :count => 1 do
         # This should really be more generic
         assert_select "tag[k='#{tag.k}'][v='#{tag.v}']"
       end
@@ -91,7 +91,7 @@ class ApiControllerTest < ActionController::TestCase
     assert_response :success, "The map call should have succeeded"
     assert_select "osm[version='#{API_VERSION}'][generator='#{GENERATOR}']", :count => 1 do
       assert_select "bounds[minlon='#{node.lon}'][minlat='#{node.lat}'][maxlon='#{node.lon}'][maxlat='#{node.lat}']", :count => 1
-      assert_select "node[id='#{node.id}'][lat='#{node.lat}'][lon='#{node.lon}'][version='#{node.version}'][changeset='#{node.changeset_id}'][visible='#{node.visible}'][timestamp='#{node.timestamp.xmlschema}']", :count => 1 do
+      assert_select "node[id='#{node.id}'][lat='#{format('%.7f', node.lat)}'][lon='#{format('%.7f', node.lon)}'][version='#{node.version}'][changeset='#{node.changeset_id}'][visible='#{node.visible}'][timestamp='#{node.timestamp.xmlschema}']", :count => 1 do
         # This should really be more generic
         assert_select "tag[k='#{tag.k}'][v='#{tag.v}']"
       end

diff --git a/test/controllers/browse_controller_test.rb b/test/controllers/browse_controller_test.rb
index 567fc0576cabe29b7ba05173b9433748832feceb..12262b22403bd73dba6882ec169637e0f08ab40f 100644 (file)
--- a/test/controllers/browse_controller_test.rb
+++ b/test/controllers/browse_controller_test.rb
@@ -62,11 +62,11 @@ class BrowseControllerTest < ActionController::TestCase
   end
 
   def test_read_node
-    browse_check "node", nodes(:visible_node).node_id, "browse/feature"
+    browse_check "node", create(:node).id, "browse/feature"
   end
 
   def test_read_node_history
-    browse_check "node_history", nodes(:visible_node).node_id, "browse/history"
+    browse_check "node_history", create(:node, :with_history).id, "browse/history"
   end
 
   def test_read_changeset
@@ -135,7 +135,11 @@ class BrowseControllerTest < ActionController::TestCase
   # then please make it more easily (and robustly) testable!
   ##
   def test_redacted_node
-    get :node, :id => current_nodes(:redacted_node).id
+    node = create(:node, :with_history, :deleted, :version => 2)
+    node_v1 = node.old_nodes.find_by(:version => 1)
+    node_v1.redact!(create(:redaction))
+
+    get :node, :id => node.id
     assert_response :success
     assert_template "feature"
 
@@ -147,7 +151,11 @@ class BrowseControllerTest < ActionController::TestCase
   end
 
   def test_redacted_node_history
-    get :node_history, :id => nodes(:redacted_node_redacted_version).node_id
+    node = create(:node, :with_history, :deleted, :version => 2)
+    node_v1 = node.old_nodes.find_by(:version => 1)
+    node_v1.redact!(create(:redaction))
+
+    get :node_history, :id => node.id
     assert_response :success
     assert_template "browse/history"
 

diff --git a/test/controllers/way_controller_test.rb b/test/controllers/way_controller_test.rb
index 650977078a1c31c1899e453ebd63e79bb1fcdaf1..ae52c036f43d02f4692e02607cfb30ff1614bdcc 100644 (file)
--- a/test/controllers/way_controller_test.rb
+++ b/test/controllers/way_controller_test.rb
@@ -74,7 +74,7 @@ class WayControllerTest < ActionController::TestCase
       way.nodes.each do |n|
         count = (way.nodes - (way.nodes - [n])).length
         assert_select "osm way nd[ref='#{n.id}']", count
-        assert_select "osm node[id='#{n.id}'][version='#{n.version}'][lat='#{n.lat}'][lon='#{n.lon}']", 1
+        assert_select "osm node[id='#{n.id}'][version='#{n.version}'][lat='#{format('%.7f', n.lat)}'][lon='#{format('%.7f', n.lon)}']", 1
       end
     end
   end

diff --git a/test/factories/node.rb b/test/factories/node.rb
index 85eded4054ba42bebe350bae2186b413ed7224d0..cbebbb00986096245b972bafa98223472199198c 100644 (file)
--- a/test/factories/node.rb
+++ b/test/factories/node.rb
@@ -8,5 +8,24 @@ FactoryGirl.define do
     visible true
     timestamp Time.now
     version 1
+
+    trait :deleted do
+      visible false
+    end
+
+    trait :with_history do
+      after(:create) do |node, _evaluator|
+        (1..node.version).each do |n|
+          create(:old_node, :node_id => node.id, :version => n)
+        end
+
+        # For deleted nodes, make sure the most recent old_node is also deleted.
+        if node.visible == false
+          latest = node.old_nodes.find_by(:version => node.version)
+          latest.visible = false
+          latest.save
+        end
+      end
+    end
   end
 end

diff --git a/test/helpers/browse_helper_test.rb b/test/helpers/browse_helper_test.rb
index 32cf96c6b2441b3b792abaf55e9003b6acfebff9..62aa73f94833489d458547c54e874c545fd4eb87 100644 (file)
--- a/test/helpers/browse_helper_test.rb
+++ b/test/helpers/browse_helper_test.rb
@@ -17,77 +17,93 @@ class BrowseHelperTest < ActionView::TestCase
   end
 
   def test_printable_name
-    add_tags_selection(current_nodes(:node_with_name))
-    create(:node_tag, :node => current_nodes(:node_with_ref_without_name), :k => "ref", :v => "3.1415926")
-    add_old_tags_selection(nodes(:node_with_name_current_version))
-    add_old_tags_selection(nodes(:node_with_name_redacted_version))
-
-    # current_nodes(:redacted_node) is deleted, so has no tags.
-    assert_dom_equal "17", printable_name(current_nodes(:redacted_node))
-    assert_dom_equal "<bdi>Test Node</bdi> (<bdi>18</bdi>)", printable_name(current_nodes(:node_with_name))
-    assert_dom_equal "<bdi>Test Node</bdi> (<bdi>18</bdi>)", printable_name(nodes(:node_with_name_current_version))
-    assert_dom_equal "18", printable_name(nodes(:node_with_name_redacted_version))
-    assert_dom_equal "<bdi>Test Node</bdi> (<bdi>18, v2</bdi>)", printable_name(nodes(:node_with_name_current_version), true)
-    assert_dom_equal "18, v1", printable_name(nodes(:node_with_name_redacted_version), true)
-    assert_dom_equal "<bdi>3.1415926</bdi> (<bdi>19</bdi>)", printable_name(current_nodes(:node_with_ref_without_name))
+    node = create(:node, :with_history, :version => 2)
+    node_v1 = node.old_nodes.find_by(:version => 1)
+    node_v2 = node.old_nodes.find_by(:version => 2)
+    node_v1.redact!(create(:redaction))
+
+    add_tags_selection(node)
+    add_old_tags_selection(node_v2)
+    add_old_tags_selection(node_v1)
+
+    node_with_ref_without_name = create(:node)
+    create(:node_tag, :node => node_with_ref_without_name, :k => "ref", :v => "3.1415926")
+
+    deleted_node = create(:node, :deleted)
+
+    assert_dom_equal deleted_node.id.to_s, printable_name(deleted_node)
+    assert_dom_equal "<bdi>Test Node</bdi> (<bdi>#{node.id}</bdi>)", printable_name(node)
+    assert_dom_equal "<bdi>Test Node</bdi> (<bdi>#{node.id}</bdi>)", printable_name(node_v2)
+    assert_dom_equal node.id.to_s, printable_name(node_v1)
+    assert_dom_equal "<bdi>Test Node</bdi> (<bdi>#{node.id}, v2</bdi>)", printable_name(node_v2, true)
+    assert_dom_equal "#{node.id}, v1", printable_name(node_v1, true)
+    assert_dom_equal "<bdi>3.1415926</bdi> (<bdi>#{node_with_ref_without_name.id}</bdi>)", printable_name(node_with_ref_without_name)
 
     I18n.locale = "pt"
 
-    assert_dom_equal "17", printable_name(current_nodes(:redacted_node))
-    assert_dom_equal "<bdi>Nó teste</bdi> (<bdi>18</bdi>)", printable_name(current_nodes(:node_with_name))
-    assert_dom_equal "<bdi>Nó teste</bdi> (<bdi>18</bdi>)", printable_name(nodes(:node_with_name_current_version))
-    assert_dom_equal "18", printable_name(nodes(:node_with_name_redacted_version))
-    assert_dom_equal "<bdi>Nó teste</bdi> (<bdi>18, v2</bdi>)", printable_name(nodes(:node_with_name_current_version), true)
-    assert_dom_equal "18, v1", printable_name(nodes(:node_with_name_redacted_version), true)
-    assert_dom_equal "<bdi>3.1415926</bdi> (<bdi>19</bdi>)", printable_name(current_nodes(:node_with_ref_without_name))
+    assert_dom_equal deleted_node.id.to_s, printable_name(deleted_node)
+    assert_dom_equal "<bdi>Nó teste</bdi> (<bdi>#{node.id}</bdi>)", printable_name(node)
+    assert_dom_equal "<bdi>Nó teste</bdi> (<bdi>#{node.id}</bdi>)", printable_name(node_v2)
+    assert_dom_equal node.id.to_s, printable_name(node_v1)
+    assert_dom_equal "<bdi>Nó teste</bdi> (<bdi>#{node.id}, v2</bdi>)", printable_name(node_v2, true)
+    assert_dom_equal "#{node.id}, v1", printable_name(node_v1, true)
+    assert_dom_equal "<bdi>3.1415926</bdi> (<bdi>#{node_with_ref_without_name.id}</bdi>)", printable_name(node_with_ref_without_name)
 
     I18n.locale = "pt-BR"
 
-    assert_dom_equal "17", printable_name(current_nodes(:redacted_node))
-    assert_dom_equal "<bdi>Nó teste</bdi> (<bdi>18</bdi>)", printable_name(current_nodes(:node_with_name))
-    assert_dom_equal "<bdi>Nó teste</bdi> (<bdi>18</bdi>)", printable_name(nodes(:node_with_name_current_version))
-    assert_dom_equal "18", printable_name(nodes(:node_with_name_redacted_version))
-    assert_dom_equal "<bdi>Nó teste</bdi> (<bdi>18, v2</bdi>)", printable_name(nodes(:node_with_name_current_version), true)
-    assert_dom_equal "18, v1", printable_name(nodes(:node_with_name_redacted_version), true)
-    assert_dom_equal "<bdi>3.1415926</bdi> (<bdi>19</bdi>)", printable_name(current_nodes(:node_with_ref_without_name))
+    assert_dom_equal deleted_node.id.to_s, printable_name(deleted_node)
+    assert_dom_equal "<bdi>Nó teste</bdi> (<bdi>#{node.id}</bdi>)", printable_name(node)
+    assert_dom_equal "<bdi>Nó teste</bdi> (<bdi>#{node.id}</bdi>)", printable_name(node_v2)
+    assert_dom_equal node.id.to_s, printable_name(node_v1)
+    assert_dom_equal "<bdi>Nó teste</bdi> (<bdi>#{node.id}, v2</bdi>)", printable_name(node_v2, true)
+    assert_dom_equal "#{node.id}, v1", printable_name(node_v1, true)
+    assert_dom_equal "<bdi>3.1415926</bdi> (<bdi>#{node_with_ref_without_name.id}</bdi>)", printable_name(node_with_ref_without_name)
 
     I18n.locale = "de"
 
-    assert_dom_equal "17", printable_name(current_nodes(:redacted_node))
-    assert_dom_equal "<bdi>Test Node</bdi> (<bdi>18</bdi>)", printable_name(current_nodes(:node_with_name))
-    assert_dom_equal "<bdi>Test Node</bdi> (<bdi>18</bdi>)", printable_name(nodes(:node_with_name_current_version))
-    assert_dom_equal "18", printable_name(nodes(:node_with_name_redacted_version))
-    assert_dom_equal "<bdi>Test Node</bdi> (<bdi>18, v2</bdi>)", printable_name(nodes(:node_with_name_current_version), true)
-    assert_dom_equal "18, v1", printable_name(nodes(:node_with_name_redacted_version), true)
-    assert_dom_equal "<bdi>3.1415926</bdi> (<bdi>19</bdi>)", printable_name(current_nodes(:node_with_ref_without_name))
+    assert_dom_equal deleted_node.id.to_s, printable_name(deleted_node)
+    assert_dom_equal "<bdi>Test Node</bdi> (<bdi>#{node.id}</bdi>)", printable_name(node)
+    assert_dom_equal "<bdi>Test Node</bdi> (<bdi>#{node.id}</bdi>)", printable_name(node_v2)
+    assert_dom_equal node.id.to_s, printable_name(node_v1)
+    assert_dom_equal "<bdi>Test Node</bdi> (<bdi>#{node.id}, v2</bdi>)", printable_name(node_v2, true)
+    assert_dom_equal "#{node.id}, v1", printable_name(node_v1, true)
+    assert_dom_equal "<bdi>3.1415926</bdi> (<bdi>#{node_with_ref_without_name.id}</bdi>)", printable_name(node_with_ref_without_name)
   end
 
   def test_link_class
-    add_tags_selection(current_nodes(:node_with_name))
+    node = create(:node, :with_history, :version => 2)
+    node_v1 = node.old_nodes.find_by(:version => 1)
+    node_v2 = node.old_nodes.find_by(:version => 2)
+    node_v1.redact!(create(:redaction))
 
-    assert_equal "node", link_class("node", current_nodes(:visible_node))
-    assert_equal "node deleted", link_class("node", current_nodes(:invisible_node))
-    assert_equal "node deleted", link_class("node", current_nodes(:redacted_node))
-    assert_equal "node building yes shop gift tourism museum", link_class("node", current_nodes(:node_with_name))
+    add_tags_selection(node)
+    add_old_tags_selection(node_v2)
+    add_old_tags_selection(node_v1)
 
-    add_old_tags_selection(nodes(:node_with_name_current_version))
-    add_old_tags_selection(nodes(:node_with_name_redacted_version))
-    assert_equal "node building yes shop gift tourism museum", link_class("node", nodes(:node_with_name_current_version))
-    assert_equal "node deleted", link_class("node", nodes(:node_with_name_redacted_version))
+    assert_equal "node", link_class("node", create(:node))
+    assert_equal "node deleted", link_class("node", create(:node, :deleted))
+
+    assert_equal "node building yes shop gift tourism museum", link_class("node", node)
+    assert_equal "node building yes shop gift tourism museum", link_class("node", node_v2)
+    assert_equal "node deleted", link_class("node", node_v1)
   end
 
   def test_link_title
-    add_tags_selection(current_nodes(:node_with_name))
+    node = create(:node, :with_history, :version => 2)
+    node_v1 = node.old_nodes.find_by(:version => 1)
+    node_v2 = node.old_nodes.find_by(:version => 2)
+    node_v1.redact!(create(:redaction))
+
+    add_tags_selection(node)
+    add_old_tags_selection(node_v2)
+    add_old_tags_selection(node_v1)
 
-    assert_equal "", link_title(current_nodes(:visible_node))
-    assert_equal "", link_title(current_nodes(:invisible_node))
-    assert_equal "", link_title(current_nodes(:redacted_node))
-    assert_equal "building=yes, shop=gift, and tourism=museum", link_title(current_nodes(:node_with_name))
+    assert_equal "", link_title(create(:node))
+    assert_equal "", link_title(create(:node, :deleted))
 
-    add_old_tags_selection(nodes(:node_with_name_current_version))
-    add_old_tags_selection(nodes(:node_with_name_redacted_version))
-    assert_equal "building=yes, shop=gift, and tourism=museum", link_title(nodes(:node_with_name_current_version))
-    assert_equal "", link_title(nodes(:node_with_name_redacted_version))
+    assert_equal "building=yes, shop=gift, and tourism=museum", link_title(node)
+    assert_equal "building=yes, shop=gift, and tourism=museum", link_title(node_v2)
+    assert_equal "", link_title(node_v1)
   end
 
   def test_format_key
@@ -122,24 +138,29 @@ class BrowseHelperTest < ActionView::TestCase
   end
 
   def test_icon_tags
-    add_tags_selection(current_nodes(:node_with_name))
+    node = create(:node, :with_history, :version => 2)
+    node_v1 = node.old_nodes.find_by(:version => 1)
+    node_v2 = node.old_nodes.find_by(:version => 2)
+    node_v1.redact!(create(:redaction))
+
+    add_tags_selection(node)
 
-    tags = icon_tags(current_nodes(:node_with_name))
+    tags = icon_tags(node)
     assert_equal 3, tags.count
     assert tags.include?(%w(building yes))
     assert tags.include?(%w(tourism museum))
     assert tags.include?(%w(shop gift))
 
-    add_old_tags_selection(nodes(:node_with_name_current_version))
-    add_old_tags_selection(nodes(:node_with_name_redacted_version))
+    add_old_tags_selection(node_v2)
+    add_old_tags_selection(node_v1)
 
-    tags = icon_tags(nodes(:node_with_name_current_version))
+    tags = icon_tags(node_v2)
     assert_equal 3, tags.count
     assert tags.include?(%w(building yes))
     assert tags.include?(%w(tourism museum))
     assert tags.include?(%w(shop gift))
 
-    tags = icon_tags(nodes(:node_with_name_redacted_version))
+    tags = icon_tags(node_v1)
     assert_equal 3, tags.count
     assert tags.include?(%w(building yes))
     assert tags.include?(%w(tourism museum))

diff --git a/test/models/node_test.rb b/test/models/node_test.rb
index 4384c6287eb8aa0eb4730d933fe3ac9ad509f135..bd664cfca407f4e7e38692854b69f641b423d574 100644 (file)
--- a/test/models/node_test.rb
+++ b/test/models/node_test.rb
@@ -61,6 +61,14 @@ class NodeTest < ActiveSupport::TestCase
     assert_in_delta 76.543 * OldNode::SCALE, node.longitude, 0.000001
   end
 
+  # Ensure the lat/lon is formatted as a decimal e.g. not 4.0e-05
+  def test_lat_lon_xml_format
+    node = build(:node, :latitude => 0.00004 * OldNode::SCALE, :longitude => 0.00008 * OldNode::SCALE)
+
+    assert_match /lat="0.0000400"/, node.to_xml.to_s
+    assert_match /lon="0.0000800"/, node.to_xml.to_s
+  end
+
   # Check that you can create a node and store it
   def test_create
     changeset = create(:changeset)

diff --git a/test/models/redaction_test.rb b/test/models/redaction_test.rb
index 7ea3e458d1138dc2d52dc66a591f383b0255dbf7..2e629867b6996e8ce960fdbd905fd3df68a06d9c 100644 (file)
--- a/test/models/redaction_test.rb
+++ b/test/models/redaction_test.rb
@@ -12,8 +12,8 @@ class RedactionTest < ActiveSupport::TestCase
   end
 
   def test_cannot_redact_current_via_old
-    node = create(:node)
-    node_v1 = create(:old_node, :node_id => node.id)
+    node = create(:node, :with_history)
+    node_v1 = node.old_nodes.find_by(:version => 1)
     r = create(:redaction)
     assert_equal(false, node_v1.redacted?, "Expected node to not be redacted already.")
     assert_raise(OSM::APICannotRedactError) do
@@ -22,9 +22,9 @@ class RedactionTest < ActiveSupport::TestCase
   end
 
   def test_can_redact_old
-    node = create(:node, :version => 2)
-    node_v1 = create(:old_node, :node_id => node.id)
-    node_v2 = create(:old_node, :node_id => node.id, :version => 2)
+    node = create(:node, :with_history, :version => 2)
+    node_v1 = node.old_nodes.find_by(:version => 1)
+    node_v2 = node.old_nodes.find_by(:version => 2)
     r = create(:redaction)
 
     assert_equal(false, node_v1.redacted?, "Expected node to not be redacted already.")