From: Andy Allan <andy@gravitystorm.co.uk>
Date: Wed, 11 Jul 2012 13:07:50 +0000 (+0100)
Subject: Don't show coordinates for deleted nodes
X-Git-Tag: live~5475
X-Git-Url: https://git.openstreetmap.org/rails.git/commitdiff_plain/2c67c079ac39cefd3b096524fc0b7364b0eb21d7?hp=bda393e7e2604ae17b884cfd577438e34197ba5c

Don't show coordinates for deleted nodes
---

diff --git a/app/models/node.rb b/app/models/node.rb
index 0953241d9..2f528076e 100644
--- a/app/models/node.rb
+++ b/app/models/node.rb
@@ -201,11 +201,14 @@ class Node < ActiveRecord::Base
   def to_xml_node(changeset_cache = {}, user_display_name_cache = {})
     el1 = XML::Node.new 'node'
     el1['id'] = self.id.to_s
-    el1['lat'] = self.lat.to_s
-    el1['lon'] = self.lon.to_s
     el1['version'] = self.version.to_s
     el1['changeset'] = self.changeset_id.to_s
 
+    if self.visible?
+      el1['lat'] = self.lat.to_s
+      el1['lon'] = self.lon.to_s
+    end
+
     if changeset_cache.key?(self.changeset_id)
       # use the cache if available
     else
diff --git a/app/models/old_node.rb b/app/models/old_node.rb
index 9426b2d23..c2ec43c9c 100644
--- a/app/models/old_node.rb
+++ b/app/models/old_node.rb
@@ -52,8 +52,11 @@ class OldNode < ActiveRecord::Base
       el2['v'] = v.to_s
       el1 << el2
     end
-    el1['lat'] = self.lat.to_s
-    el1['lon'] = self.lon.to_s
+
+    if self.visible?
+      el1['lat'] = self.lat.to_s
+      el1['lon'] = self.lon.to_s
+    end
     
     el1['changeset'] = self.changeset.id.to_s
     if self.changeset.user.data_public?
diff --git a/app/views/browse/_node_details.html.erb b/app/views/browse/_node_details.html.erb
index e237f6234..23176eab2 100644
--- a/app/views/browse/_node_details.html.erb
+++ b/app/views/browse/_node_details.html.erb
@@ -5,10 +5,12 @@
 
   <%= render :partial => "common_details", :object => node_details %>
 
+  <% if node_details.visible -%>
   <tr>
     <th><%= t 'browse.node_details.coordinates' %></th>
     <td><div class="geo"><%= link_to(content_tag(:span, number_with_delimiter(node_details.lat), :class => "latitude") + ", " + content_tag(:span, number_with_delimiter(node_details.lon), :class => "longitude"), {:controller => 'site', :action => 'index', :lat => h(node_details.lat), :lon => h(node_details.lon), :zoom => "18"}) %></div></td>
   </tr>
+  <% end -%>
 
   <% unless node_details.ways.empty? and node_details.containing_relation_members.empty? %>
     <tr valign="top">
diff --git a/app/views/browse/node.html.erb b/app/views/browse/node.html.erb
index 7b0ad746c..df777c960 100644
--- a/app/views/browse/node.html.erb
+++ b/app/views/browse/node.html.erb
@@ -7,10 +7,13 @@
 <% end %>
 <%= render :partial => "navigation" %>
 <h2><%= t'browse.node.node_title', :node_name => @name %></h2>
+<% if @node.visible -%>
 <%= render :partial => "map", :object => @node %>
+<% end -%>
 <%= render :partial => "node_details", :object => @node %>
 <hr />
-<%= raw t'browse.node.download', :download_xml_link => link_to(t('browse.node.download_xml'), :controller => "old_node", :action => "version", :version => @node.version),
-                                 :view_history_link => link_to(t('browse.node.view_history'), :action => "node_history"),
-                                 :edit_link => link_to(t('browse.node.edit'), :controller => "site", :action => "edit", :lat => @node.lat, :lon => @node.lon, :zoom => 18, :node => @node.id)
-%>
+<%= link_to(t('browse.node.download_xml'), :controller => "old_node", :action => "version", :version => @node.version) %>
+| <%= link_to(t('browse.node.view_history'), :action => "node_history") %>
+<% if @node.visible -%>
+| <%= link_to(t('browse.node.edit'), :controller => "site", :action => "edit", :lat => @node.lat, :lon => @node.lon, :zoom => 18, :node => @node.id) %>
+<% end -%>
diff --git a/app/views/browse/node_history.html.erb b/app/views/browse/node_history.html.erb
index 9966d558d..5f70492b4 100644
--- a/app/views/browse/node_history.html.erb
+++ b/app/views/browse/node_history.html.erb
@@ -3,7 +3,9 @@
 @title = t('browse.node_history.node_history') + ' | ' + @name
 %>
 <h2><%= raw t'browse.node_history.node_history_title', :node_name => link_to(h(@name), :action => "node", :id => @node.id) %></h2>
+<% if @node.visible -%>
 <%= render :partial => "map", :object => @node %>
+<% end -%>
 <% @node.old_nodes.reverse.each do |node| %>
   <%= render :partial => "node_details", :object => node %>
   <hr />
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 4dc7888d2..6108b7207 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -174,7 +174,6 @@ en:
     node:
       node: "Node"
       node_title: "Node: %{node_name}"
-      download: "%{download_xml_link}, %{view_history_link} or %{edit_link}"
       download_xml: "Download XML"
       view_history: "view history"
       edit: "edit"
diff --git a/test/functional/node_controller_test.rb b/test/functional/node_controller_test.rb
index 5544728f1..6903dd60b 100644
--- a/test/functional/node_controller_test.rb
+++ b/test/functional/node_controller_test.rb
@@ -231,9 +231,9 @@ class NodeControllerTest < ActionController::TestCase
     assert @response.body.to_i > current_nodes(:public_visible_node).version,
        "delete request should return a new version number for node"
 
-    # this won't work since the node is already deleted
-    content(nodes(:invisible_node).to_xml)
-    delete :delete, :id => current_nodes(:invisible_node).id
+    # deleting the same node twice doesn't work
+    content(nodes(:public_visible_node).to_xml)
+    delete :delete, :id => current_nodes(:public_visible_node).id
     assert_response :gone
 
     # this won't work since the node never existed