Convert api element history actions to versions resources

diff --git a/app/abilities/api_ability.rb b/app/abilities/api_ability.rb
index 55476ab53f72440f5871b8df3a6419c3fd83c82b..e774f6820df3f364c3d766f1c2bd25129f12a0dc 100644 (file)
--- a/app/abilities/api_ability.rb
+++ b/app/abilities/api_ability.rb
@@ -15,8 +15,7 @@ class ApiAbility
       can [:read, :download], Changeset
       can :read, Tracepoint
       can :read, User
       can [:read, :download], Changeset
       can :read, Tracepoint
       can :read, User

-      can :read, [Node, Way, Relation]
-      can [:history, :read], [OldNode, OldWay, OldRelation]
+      can :read, [Node, Way, Relation, OldNode, OldWay, OldRelation]

       can :read, UserBlock
 
       if user&.active?
       can :read, UserBlock
 
       if user&.active?

diff --git a/app/controllers/api/old_elements_controller.rb b/app/controllers/api/old_elements_controller.rb
index 73e57c1f82355415baeef6a4ee044e3a741b942c..bdbc5e392e21dd00409b5f93dc3494e00f7de334 100644 (file)
--- a/app/controllers/api/old_elements_controller.rb
+++ b/app/controllers/api/old_elements_controller.rb
@@ -4,17 +4,17 @@
 module Api
   class OldElementsController < ApiController
     before_action :check_api_writable, :only => [:redact]
 module Api
   class OldElementsController < ApiController
     before_action :check_api_writable, :only => [:redact]

-    before_action :setup_user_auth, :only => [:history, :show]
+    before_action :setup_user_auth, :only => [:index, :show]

     before_action :authorize, :only => [:redact]
 
     authorize_resource
 
     before_action :authorize, :only => [:redact]
 
     authorize_resource
 

-    before_action :lookup_old_element, :except => [:history]
-    before_action :lookup_old_element_versions, :only => [:history]
+    before_action :lookup_old_element, :except => [:index]
+    before_action :lookup_old_element_versions, :only => [:index]

 
     before_action :set_request_formats, :except => [:redact]
 
 
     before_action :set_request_formats, :except => [:redact]
 

-    def history
+    def index

       # the .where() method used in the lookup_old_element_versions
       # call won't throw an error if no records are found, so we have
       # to do that ourselves.
       # the .where() method used in the lookup_old_element_versions
       # call won't throw an error if no records are found, so we have
       # to do that ourselves.

diff --git a/app/controllers/api/old_nodes_controller.rb b/app/controllers/api/old_nodes_controller.rb
index c2831c48ba558e34b0b937f0a423d1818d571591..eb6c1acf5fff1ac32f6cf13701b777cc6e68ee4f 100644 (file)
--- a/app/controllers/api/old_nodes_controller.rb
+++ b/app/controllers/api/old_nodes_controller.rb
@@ -7,7 +7,7 @@ module Api
     end
 
     def lookup_old_element_versions
     end
 
     def lookup_old_element_versions

-      @elements = OldNode.where(:node_id => params[:id]).order(:version)
+      @elements = OldNode.where(:node_id => params[:node_id]).order(:version)

     end
   end
 end
     end
   end
 end

diff --git a/app/controllers/api/old_relations_controller.rb b/app/controllers/api/old_relations_controller.rb
index 4679384dd3877084ad9378a8e17b6aa75e9c606d..6321b9adfb051f3748013170886fc460cc7f0b39 100644 (file)
--- a/app/controllers/api/old_relations_controller.rb
+++ b/app/controllers/api/old_relations_controller.rb
@@ -7,7 +7,7 @@ module Api
     end
 
     def lookup_old_element_versions
     end
 
     def lookup_old_element_versions

-      @elements = OldRelation.where(:relation_id => params[:id]).order(:version)
+      @elements = OldRelation.where(:relation_id => params[:relation_id]).order(:version)

     end
   end
 end
     end
   end
 end

diff --git a/app/controllers/api/old_ways_controller.rb b/app/controllers/api/old_ways_controller.rb
index 0afa3059ccfce9a9ff1b350e7fbe1f5c3f41b82d..73226dee6e322cc8117f50d7bd9069d11e6afb0e 100644 (file)
--- a/app/controllers/api/old_ways_controller.rb
+++ b/app/controllers/api/old_ways_controller.rb
@@ -7,7 +7,7 @@ module Api
     end
 
     def lookup_old_element_versions
     end
 
     def lookup_old_element_versions

-      @elements = OldWay.where(:way_id => params[:id]).order(:version)
+      @elements = OldWay.where(:way_id => params[:way_id]).order(:version)

     end
   end
 end
     end
   end
 end

diff --git a/app/views/api/old_nodes/history.json.jbuilder b/app/views/api/old_nodes/index.json.jbuilder
similarity index 100%
rename from app/views/api/old_nodes/history.json.jbuilder
rename to app/views/api/old_nodes/index.json.jbuilder

diff --git a/app/views/api/old_nodes/history.xml.builder b/app/views/api/old_nodes/index.xml.builder
similarity index 100%
rename from app/views/api/old_nodes/history.xml.builder
rename to app/views/api/old_nodes/index.xml.builder

diff --git a/app/views/api/old_relations/history.json.jbuilder b/app/views/api/old_relations/index.json.jbuilder
similarity index 100%
rename from app/views/api/old_relations/history.json.jbuilder
rename to app/views/api/old_relations/index.json.jbuilder

diff --git a/app/views/api/old_relations/history.xml.builder b/app/views/api/old_relations/index.xml.builder
similarity index 100%
rename from app/views/api/old_relations/history.xml.builder
rename to app/views/api/old_relations/index.xml.builder

diff --git a/app/views/api/old_ways/history.json.jbuilder b/app/views/api/old_ways/index.json.jbuilder
similarity index 100%
rename from app/views/api/old_ways/history.json.jbuilder
rename to app/views/api/old_ways/index.json.jbuilder

diff --git a/app/views/api/old_ways/history.xml.builder b/app/views/api/old_ways/index.xml.builder
similarity index 100%
rename from app/views/api/old_ways/history.xml.builder
rename to app/views/api/old_ways/index.xml.builder

diff --git a/app/views/old_elements/index.html.erb b/app/views/old_elements/index.html.erb
index d4ecbfa60480c618041bb7902bd601abd91ec644..eaf7787fa8d2c6f92e7399ee70e9ecb2d25d0a63 100644 (file)
--- a/app/views/old_elements/index.html.erb
+++ b/app/views/old_elements/index.html.erb
@@ -5,7 +5,7 @@
 <%= render :partial => "browse/#{@type}", :collection => @feature.send(:"old_#{@type}s").reverse %>
 
 <div class='secondary-actions'>
 <%= render :partial => "browse/#{@type}", :collection => @feature.send(:"old_#{@type}s").reverse %>
 
 <div class='secondary-actions'>

-  <%= link_to t("browse.download_xml"), :controller => "api/old_#{@type.pluralize}", :action => "history" %>
+  <%= link_to t("browse.download_xml"), send(:"api_#{@type}_versions_path", @feature.id) %>

   &middot;
   <%= link_to t("browse.view_details"), :controller => @type.pluralize, :action => :show %>
   <% if params[:show_redactions] %>
   &middot;
   <%= link_to t("browse.view_details"), :controller => @type.pluralize, :action => :show %>
   <% if params[:show_redactions] %>

diff --git a/config/routes.rb b/config/routes.rb
index 2b86326986dae9e1c07c2df8ac060aa82769e24d..0d9950d048628f9d161e8feb45eb13ef1ba4787f 100644 (file)
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -30,15 +30,12 @@ OpenStreetMap::Application.routes.draw do
     post "changeset/comment/:id/hide" => "changeset_comments#destroy", :as => :changeset_comment_hide, :id => /\d+/
     post "changeset/comment/:id/unhide" => "changeset_comments#restore", :as => :changeset_comment_unhide, :id => /\d+/
 
     post "changeset/comment/:id/hide" => "changeset_comments#destroy", :as => :changeset_comment_hide, :id => /\d+/
     post "changeset/comment/:id/unhide" => "changeset_comments#restore", :as => :changeset_comment_unhide, :id => /\d+/
 

-    get "node/:id/history" => "old_nodes#history", :as => :api_node_history, :id => /\d+/

     post "node/:id/:version/redact" => "old_nodes#redact", :as => :node_version_redact, :version => /\d+/, :id => /\d+/
     get "node/:id/:version" => "old_nodes#show", :as => :api_old_node, :id => /\d+/, :version => /\d+/
 
     post "node/:id/:version/redact" => "old_nodes#redact", :as => :node_version_redact, :version => /\d+/, :id => /\d+/
     get "node/:id/:version" => "old_nodes#show", :as => :api_old_node, :id => /\d+/, :version => /\d+/
 

-    get "way/:id/history" => "old_ways#history", :as => :api_way_history, :id => /\d+/

     post "way/:id/:version/redact" => "old_ways#redact", :as => :way_version_redact, :version => /\d+/, :id => /\d+/
     get "way/:id/:version" => "old_ways#show", :as => :api_old_way, :id => /\d+/, :version => /\d+/
 
     post "way/:id/:version/redact" => "old_ways#redact", :as => :way_version_redact, :version => /\d+/, :id => /\d+/
     get "way/:id/:version" => "old_ways#show", :as => :api_old_way, :id => /\d+/, :version => /\d+/
 

-    get "relation/:id/history" => "old_relations#history", :as => :api_relation_history, :id => /\d+/

     post "relation/:id/:version/redact" => "old_relations#redact", :as => :relation_version_redact, :version => /\d+/, :id => /\d+/
     get "relation/:id/:version" => "old_relations#show", :as => :api_old_relation, :id => /\d+/, :version => /\d+/
   end
     post "relation/:id/:version/redact" => "old_relations#redact", :as => :relation_version_redact, :version => /\d+/, :id => /\d+/
     get "relation/:id/:version" => "old_relations#show", :as => :api_old_relation, :id => /\d+/, :version => /\d+/
   end


@@ -50,6 +47,7 @@ OpenStreetMap::Application.routes.draw do
         resources :ways, :only => :index
         resources :relations, :only => :index
       end
         resources :ways, :only => :index
         resources :relations, :only => :index
       end

+      resources :versions, :path => "history", :controller => :old_nodes, :only => :index

     end
     put "node/create" => "nodes#create", :as => nil
 
     end
     put "node/create" => "nodes#create", :as => nil
 


@@ -61,6 +59,7 @@ OpenStreetMap::Application.routes.draw do
       scope :module => :ways do
         resources :relations, :only => :index
       end
       scope :module => :ways do
         resources :relations, :only => :index
       end

+      resources :versions, :path => "history", :controller => :old_ways, :only => :index

     end
     put "way/create" => "ways#create", :as => nil
 
     end
     put "way/create" => "ways#create", :as => nil
 


@@ -72,6 +71,7 @@ OpenStreetMap::Application.routes.draw do
       scope :module => :relations do
         resources :relations, :only => :index
       end
       scope :module => :relations do
         resources :relations, :only => :index
       end

+      resources :versions, :path => "history", :controller => :old_relations, :only => :index

     end
     put "relation/create" => "relations#create", :as => nil
 
     end
     put "relation/create" => "relations#create", :as => nil
 

diff --git a/test/controllers/api/old_nodes_controller_test.rb b/test/controllers/api/old_nodes_controller_test.rb
index 8ce19f3ea2f5f80669801e34fc3d9115d5f0181c..2b1585be206d5101e5f1902ed2d3c66ff03798a1 100644 (file)
--- a/test/controllers/api/old_nodes_controller_test.rb
+++ b/test/controllers/api/old_nodes_controller_test.rb
@@ -2,24 +2,20 @@ require "test_helper"
 
 module Api
   class OldNodesControllerTest < ActionDispatch::IntegrationTest
 
 module Api
   class OldNodesControllerTest < ActionDispatch::IntegrationTest

-    #
-    # TODO: test history
-    #
-

     ##
     # test all routes which lead to this controller
     def test_routes
       assert_routing(
         { :path => "/api/0.6/node/1/history", :method => :get },
     ##
     # test all routes which lead to this controller
     def test_routes
       assert_routing(
         { :path => "/api/0.6/node/1/history", :method => :get },

-        { :controller => "api/old_nodes", :action => "history", :id => "1" }
+        { :controller => "api/old_nodes", :action => "index", :node_id => "1" }

       )
       assert_routing(
       )
       assert_routing(

-        { :path => "/api/0.6/node/1/2", :method => :get },
-        { :controller => "api/old_nodes", :action => "show", :id => "1", :version => "2" }
+        { :path => "/api/0.6/node/1/history.json", :method => :get },
+        { :controller => "api/old_nodes", :action => "index", :node_id => "1", :format => "json" }

       )
       assert_routing(
       )
       assert_routing(

-        { :path => "/api/0.6/node/1/history.json", :method => :get },
-        { :controller => "api/old_nodes", :action => "history", :id => "1", :format => "json" }
+        { :path => "/api/0.6/node/1/2", :method => :get },
+        { :controller => "api/old_nodes", :action => "show", :id => "1", :version => "2" }

       )
       assert_routing(
         { :path => "/api/0.6/node/1/2.json", :method => :get },
       )
       assert_routing(
         { :path => "/api/0.6/node/1/2.json", :method => :get },


@@ -31,6 +27,49 @@ module Api
       )
     end
 
       )
     end
 

+    def test_index
+      node = create(:node, :version => 2)
+      create(:old_node, :node_id => node.id, :version => 1, :latitude => 60 * OldNode::SCALE, :longitude => 30 * OldNode::SCALE)
+      create(:old_node, :node_id => node.id, :version => 2, :latitude => 61 * OldNode::SCALE, :longitude => 31 * OldNode::SCALE)
+
+      get api_node_versions_path(node)
+
+      assert_response :success
+      assert_dom "osm:root", 1 do
+        assert_dom "> node", 2 do |dom_nodes|
+          assert_dom dom_nodes[0], "> @id", node.id.to_s
+          assert_dom dom_nodes[0], "> @version", "1"
+          assert_dom dom_nodes[0], "> @lat", "60.0000000"
+          assert_dom dom_nodes[0], "> @lon", "30.0000000"
+
+          assert_dom dom_nodes[1], "> @id", node.id.to_s
+          assert_dom dom_nodes[1], "> @version", "2"
+          assert_dom dom_nodes[1], "> @lat", "61.0000000"
+          assert_dom dom_nodes[1], "> @lon", "31.0000000"
+        end
+      end
+    end
+
+    ##
+    # test that redacted nodes aren't visible in the history
+    def test_index_redacted
+      node = create(:node, :with_history, :version => 2)
+      node_v1 = node.old_nodes.find_by(:version => 1)
+      node_v1.redact!(create(:redaction))
+
+      get api_node_versions_path(node)
+      assert_response :success, "Redaction shouldn't have stopped history working."
+      assert_select "osm node[id='#{node_v1.node_id}'][version='#{node_v1.version}']", 0,
+                    "redacted node #{node_v1.node_id} version #{node_v1.version} shouldn't be present in the history."
+
+      # not even to a logged-in user
+      auth_header = bearer_authorization_header
+      get api_node_versions_path(node), :headers => auth_header
+      assert_response :success, "Redaction shouldn't have stopped history working."
+      assert_select "osm node[id='#{node_v1.node_id}'][version='#{node_v1.version}']", 0,
+                    "redacted node #{node_v1.node_id} version #{node_v1.version} shouldn't be present in the history, even when logged in."
+    end
+

     ##
     # test the version call by submitting several revisions of a new node
     # to the API and ensuring that later calls to version return the
     ##
     # test the version call by submitting several revisions of a new node
     # to the API and ensuring that later calls to version return the


@@ -191,7 +230,7 @@ module Api
     def test_lat_lon_xml_format
       old_node = create(:old_node, :latitude => (0.00004 * OldNode::SCALE).to_i, :longitude => (0.00008 * OldNode::SCALE).to_i)
 
     def test_lat_lon_xml_format
       old_node = create(:old_node, :latitude => (0.00004 * OldNode::SCALE).to_i, :longitude => (0.00008 * OldNode::SCALE).to_i)
 

-      get api_node_history_path(old_node.node_id)
+      get api_node_versions_path(old_node.node_id)

       assert_match(/lat="0.0000400"/, response.body)
       assert_match(/lon="0.0000800"/, response.body)
     end
       assert_match(/lat="0.0000400"/, response.body)
       assert_match(/lon="0.0000800"/, response.body)
     end


@@ -279,26 +318,6 @@ module Api
       assert_response :forbidden, "Redacted node shouldn't be visible via the version API, even when logged in."
     end
 
       assert_response :forbidden, "Redacted node shouldn't be visible via the version API, even when logged in."
     end
 

-    ##
-    # test that redacted nodes aren't visible in the history
-    def test_history_redacted
-      node = create(:node, :with_history, :version => 2)
-      node_v1 = node.old_nodes.find_by(:version => 1)
-      node_v1.redact!(create(:redaction))
-
-      get api_node_history_path(node)
-      assert_response :success, "Redaction shouldn't have stopped history working."
-      assert_select "osm node[id='#{node_v1.node_id}'][version='#{node_v1.version}']", 0,
-                    "redacted node #{node_v1.node_id} version #{node_v1.version} shouldn't be present in the history."
-
-      # not even to a logged-in user
-      auth_header = bearer_authorization_header
-      get api_node_history_path(node), :headers => auth_header
-      assert_response :success, "Redaction shouldn't have stopped history working."
-      assert_select "osm node[id='#{node_v1.node_id}'][version='#{node_v1.version}']", 0,
-                    "redacted node #{node_v1.node_id} version #{node_v1.version} shouldn't be present in the history, even when logged in."
-    end
-

     ##
     # test the redaction of an old version of a node, while being
     # authorised as a moderator.
     ##
     # test the redaction of an old version of a node, while being
     # authorised as a moderator.


@@ -318,11 +337,11 @@ module Api
       assert_response :success, "After redaction, node should not be gone for moderator, when flag passed."
 
       # and when accessed via history
       assert_response :success, "After redaction, node should not be gone for moderator, when flag passed."
 
       # and when accessed via history

-      get api_node_history_path(node)
+      get api_node_versions_path(node)

       assert_response :success, "Redaction shouldn't have stopped history working."
       assert_select "osm node[id='#{node_v3.node_id}'][version='#{node_v3.version}']", 0,
                     "node #{node_v3.node_id} version #{node_v3.version} should not be present in the history for moderators when not passing flag."
       assert_response :success, "Redaction shouldn't have stopped history working."
       assert_select "osm node[id='#{node_v3.node_id}'][version='#{node_v3.version}']", 0,
                     "node #{node_v3.node_id} version #{node_v3.version} should not be present in the history for moderators when not passing flag."

-      get api_node_history_path(node, :show_redactions => "true"), :headers => auth_header
+      get api_node_versions_path(node, :show_redactions => "true"), :headers => auth_header

       assert_response :success, "Redaction shouldn't have stopped history working."
       assert_select "osm node[id='#{node_v3.node_id}'][version='#{node_v3.version}']", 1,
                     "node #{node_v3.node_id} version #{node_v3.version} should still be present in the history for moderators when passing flag."
       assert_response :success, "Redaction shouldn't have stopped history working."
       assert_select "osm node[id='#{node_v3.node_id}'][version='#{node_v3.version}']", 1,
                     "node #{node_v3.node_id} version #{node_v3.version} should still be present in the history for moderators when passing flag."


@@ -346,7 +365,7 @@ module Api
       assert_response :forbidden, "Redacted node shouldn't be visible via the version API."
 
       # and when accessed via history
       assert_response :forbidden, "Redacted node shouldn't be visible via the version API."
 
       # and when accessed via history

-      get api_node_history_path(node), :headers => auth_header
+      get api_node_versions_path(node), :headers => auth_header

       assert_response :success, "Redaction shouldn't have stopped history working."
       assert_select "osm node[id='#{node_v3.node_id}'][version='#{node_v3.version}']", 0,
                     "redacted node #{node_v3.node_id} version #{node_v3.version} shouldn't be present in the history."
       assert_response :success, "Redaction shouldn't have stopped history working."
       assert_select "osm node[id='#{node_v3.node_id}'][version='#{node_v3.version}']", 0,
                     "redacted node #{node_v3.node_id} version #{node_v3.version} shouldn't be present in the history."


@@ -399,7 +418,7 @@ module Api
       assert_response :success, "After unredaction, node should not be gone for moderator."
 
       # and when accessed via history
       assert_response :success, "After unredaction, node should not be gone for moderator."
 
       # and when accessed via history

-      get api_node_history_path(node)
+      get api_node_versions_path(node)

       assert_response :success, "Unredaction shouldn't have stopped history working."
       assert_select "osm node[id='#{node_v1.node_id}'][version='#{node_v1.version}']", 1,
                     "node #{node_v1.node_id} version #{node_v1.version} should now be present in the history for moderators without passing flag."
       assert_response :success, "Unredaction shouldn't have stopped history working."
       assert_select "osm node[id='#{node_v1.node_id}'][version='#{node_v1.version}']", 1,
                     "node #{node_v1.node_id} version #{node_v1.version} should now be present in the history for moderators without passing flag."


@@ -411,7 +430,7 @@ module Api
       assert_response :success, "After unredaction, node should be visible to normal users."
 
       # and when accessed via history
       assert_response :success, "After unredaction, node should be visible to normal users."
 
       # and when accessed via history

-      get api_node_history_path(node)
+      get api_node_versions_path(node)

       assert_response :success, "Unredaction shouldn't have stopped history working."
       assert_select "osm node[id='#{node_v1.node_id}'][version='#{node_v1.version}']", 1,
                     "node #{node_v1.node_id} version #{node_v1.version} should now be present in the history for normal users without passing flag."
       assert_response :success, "Unredaction shouldn't have stopped history working."
       assert_select "osm node[id='#{node_v1.node_id}'][version='#{node_v1.version}']", 1,
                     "node #{node_v1.node_id} version #{node_v1.version} should now be present in the history for normal users without passing flag."

diff --git a/test/controllers/api/old_relations_controller_test.rb b/test/controllers/api/old_relations_controller_test.rb
index 880c340115d13e98c04e2c57aca1ebb919757b1b..506c6a1ed5314cf50cb397edfca95d052978c8e6 100644 (file)
--- a/test/controllers/api/old_relations_controller_test.rb
+++ b/test/controllers/api/old_relations_controller_test.rb
@@ -7,15 +7,15 @@ module Api
     def test_routes
       assert_routing(
         { :path => "/api/0.6/relation/1/history", :method => :get },
     def test_routes
       assert_routing(
         { :path => "/api/0.6/relation/1/history", :method => :get },

-        { :controller => "api/old_relations", :action => "history", :id => "1" }
+        { :controller => "api/old_relations", :action => "index", :relation_id => "1" }

       )
       assert_routing(
       )
       assert_routing(

-        { :path => "/api/0.6/relation/1/2", :method => :get },
-        { :controller => "api/old_relations", :action => "show", :id => "1", :version => "2" }
+        { :path => "/api/0.6/relation/1/history.json", :method => :get },
+        { :controller => "api/old_relations", :action => "index", :relation_id => "1", :format => "json" }

       )
       assert_routing(
       )
       assert_routing(

-        { :path => "/api/0.6/relation/1/history.json", :method => :get },
-        { :controller => "api/old_relations", :action => "history", :id => "1", :format => "json" }
+        { :path => "/api/0.6/relation/1/2", :method => :get },
+        { :controller => "api/old_relations", :action => "show", :id => "1", :version => "2" }

       )
       assert_routing(
         { :path => "/api/0.6/relation/1/2.json", :method => :get },
       )
       assert_routing(
         { :path => "/api/0.6/relation/1/2.json", :method => :get },


@@ -27,19 +27,52 @@ module Api
       )
     end
 
       )
     end
 

-    # -------------------------------------
-    # Test reading old relations.
-    # -------------------------------------
-    def test_history
-      # check that a visible relations is returned properly
-      get api_relation_history_path(create(:relation, :with_history))
+    ##
+    # check that a visible relations is returned properly
+    def test_index
+      relation = create(:relation, :with_history, :version => 2)
+
+      get api_relation_versions_path(relation)
+

       assert_response :success
       assert_response :success

+      assert_dom "osm:root", 1 do
+        assert_dom "> relation", 2 do |dom_relations|
+          assert_dom dom_relations[0], "> @id", relation.id.to_s
+          assert_dom dom_relations[0], "> @version", "1"
+
+          assert_dom dom_relations[1], "> @id", relation.id.to_s
+          assert_dom dom_relations[1], "> @version", "2"
+        end
+      end
+    end

 
 

-      # check chat a non-existent relations is not returned
-      get api_relation_history_path(0)
+    ##
+    # check that a non-existent relations is not returned
+    def test_index_invalid
+      get api_relation_versions_path(0)

       assert_response :not_found
     end
 
       assert_response :not_found
     end
 

+    ##
+    # test that redacted relations aren't visible in the history
+    def test_index_redacted
+      relation = create(:relation, :with_history, :version => 2)
+      relation_v1 = relation.old_relations.find_by(:version => 1)
+      relation_v1.redact!(create(:redaction))
+
+      get api_relation_versions_path(relation)
+      assert_response :success, "Redaction shouldn't have stopped history working."
+      assert_select "osm relation[id='#{relation_v1.relation_id}'][version='#{relation_v1.version}']", 0,
+                    "redacted relation #{relation_v1.relation_id} version #{relation_v1.version} shouldn't be present in the history."
+
+      # not even to a logged-in user
+      auth_header = bearer_authorization_header
+      get api_relation_versions_path(relation), :headers => auth_header
+      assert_response :success, "Redaction shouldn't have stopped history working."
+      assert_select "osm relation[id='#{relation_v1.relation_id}'][version='#{relation_v1.version}']", 0,
+                    "redacted relation #{relation_v1.relation_id} version #{relation_v1.version} shouldn't be present in the history, even when logged in."
+    end
+

     ##
     # test the redaction of an old version of a relation, while not being
     # authorised.
     ##
     # test the redaction of an old version of a relation, while not being
     # authorised.


@@ -118,27 +151,6 @@ module Api
       assert_response :forbidden, "Redacted relation shouldn't be visible via the version API, even when logged in."
     end
 
       assert_response :forbidden, "Redacted relation shouldn't be visible via the version API, even when logged in."
     end
 

-    ##
-    # test that redacted relations aren't visible in the history
-    def test_history_redacted
-      relation = create(:relation, :with_history, :version => 2)
-      relation_v1 = relation.old_relations.find_by(:version => 1)
-      relation_v1.redact!(create(:redaction))
-
-      get api_relation_history_path(relation)
-      assert_response :success, "Redaction shouldn't have stopped history working."
-      assert_select "osm relation[id='#{relation_v1.relation_id}'][version='#{relation_v1.version}']", 0,
-                    "redacted relation #{relation_v1.relation_id} version #{relation_v1.version} shouldn't be present in the history."
-
-      # not even to a logged-in user
-      auth_header = bearer_authorization_header
-      get api_old_relation_path(relation_v1.relation_id, relation_v1.version), :headers => auth_header
-      get api_relation_history_path(relation), :headers => auth_header
-      assert_response :success, "Redaction shouldn't have stopped history working."
-      assert_select "osm relation[id='#{relation_v1.relation_id}'][version='#{relation_v1.version}']", 0,
-                    "redacted relation #{relation_v1.relation_id} version #{relation_v1.version} shouldn't be present in the history, even when logged in."
-    end
-

     ##
     # test the redaction of an old version of a relation, while being
     # authorised as a moderator.
     ##
     # test the redaction of an old version of a relation, while being
     # authorised as a moderator.


@@ -159,11 +171,11 @@ module Api
       assert_response :success, "After redaction, relation should not be gone for moderator, when flag passed."
 
       # and when accessed via history
       assert_response :success, "After redaction, relation should not be gone for moderator, when flag passed."
 
       # and when accessed via history

-      get api_relation_history_path(relation), :headers => auth_header
+      get api_relation_versions_path(relation), :headers => auth_header

       assert_response :success, "Redaction shouldn't have stopped history working."
       assert_select "osm relation[id='#{relation_v3.relation_id}'][version='#{relation_v3.version}']", 0,
                     "relation #{relation_v3.relation_id} version #{relation_v3.version} should not be present in the history for moderators when not passing flag."
       assert_response :success, "Redaction shouldn't have stopped history working."
       assert_select "osm relation[id='#{relation_v3.relation_id}'][version='#{relation_v3.version}']", 0,
                     "relation #{relation_v3.relation_id} version #{relation_v3.version} should not be present in the history for moderators when not passing flag."

-      get api_relation_history_path(relation, :show_redactions => "true"), :headers => auth_header
+      get api_relation_versions_path(relation, :show_redactions => "true"), :headers => auth_header

       assert_response :success, "Redaction shouldn't have stopped history working."
       assert_select "osm relation[id='#{relation_v3.relation_id}'][version='#{relation_v3.version}']", 1,
                     "relation #{relation_v3.relation_id} version #{relation_v3.version} should still be present in the history for moderators when passing flag."
       assert_response :success, "Redaction shouldn't have stopped history working."
       assert_select "osm relation[id='#{relation_v3.relation_id}'][version='#{relation_v3.version}']", 1,
                     "relation #{relation_v3.relation_id} version #{relation_v3.version} should still be present in the history for moderators when passing flag."


@@ -188,7 +200,7 @@ module Api
       assert_response :forbidden, "Redacted relation shouldn't be visible via the version API."
 
       # and when accessed via history
       assert_response :forbidden, "Redacted relation shouldn't be visible via the version API."
 
       # and when accessed via history

-      get api_relation_history_path(relation), :headers => auth_header
+      get api_relation_versions_path(relation), :headers => auth_header

       assert_response :success, "Redaction shouldn't have stopped history working."
       assert_select "osm relation[id='#{relation_v3.relation_id}'][version='#{relation_v3.version}']", 0,
                     "redacted relation #{relation_v3.relation_id} version #{relation_v3.version} shouldn't be present in the history."
       assert_response :success, "Redaction shouldn't have stopped history working."
       assert_select "osm relation[id='#{relation_v3.relation_id}'][version='#{relation_v3.version}']", 0,
                     "redacted relation #{relation_v3.relation_id} version #{relation_v3.version} shouldn't be present in the history."


@@ -239,7 +251,7 @@ module Api
       assert_response :success, "After unredaction, relation should not be gone for moderator."
 
       # and when accessed via history
       assert_response :success, "After unredaction, relation should not be gone for moderator."
 
       # and when accessed via history

-      get api_relation_history_path(relation), :headers => auth_header
+      get api_relation_versions_path(relation), :headers => auth_header

       assert_response :success, "Redaction shouldn't have stopped history working."
       assert_select "osm relation[id='#{relation_v1.relation_id}'][version='#{relation_v1.version}']", 1,
                     "relation #{relation_v1.relation_id} version #{relation_v1.version} should still be present in the history for moderators."
       assert_response :success, "Redaction shouldn't have stopped history working."
       assert_select "osm relation[id='#{relation_v1.relation_id}'][version='#{relation_v1.version}']", 1,
                     "relation #{relation_v1.relation_id} version #{relation_v1.version} should still be present in the history for moderators."


@@ -251,7 +263,7 @@ module Api
       assert_response :success, "After redaction, node should not be gone for normal user."
 
       # and when accessed via history
       assert_response :success, "After redaction, node should not be gone for normal user."
 
       # and when accessed via history

-      get api_relation_history_path(relation), :headers => auth_header
+      get api_relation_versions_path(relation), :headers => auth_header

       assert_response :success, "Redaction shouldn't have stopped history working."
       assert_select "osm relation[id='#{relation_v1.relation_id}'][version='#{relation_v1.version}']", 1,
                     "relation #{relation_v1.relation_id} version #{relation_v1.version} should still be present in the history for normal users."
       assert_response :success, "Redaction shouldn't have stopped history working."
       assert_select "osm relation[id='#{relation_v1.relation_id}'][version='#{relation_v1.version}']", 1,
                     "relation #{relation_v1.relation_id} version #{relation_v1.version} should still be present in the history for normal users."

diff --git a/test/controllers/api/old_ways_controller_test.rb b/test/controllers/api/old_ways_controller_test.rb
index cc9eaf312375b322ba94f27ff38099125450fddf..5b8b6be0783ae8bb07b52f2eae8627a6b0857f9f 100644 (file)
--- a/test/controllers/api/old_ways_controller_test.rb
+++ b/test/controllers/api/old_ways_controller_test.rb
@@ -7,15 +7,15 @@ module Api
     def test_routes
       assert_routing(
         { :path => "/api/0.6/way/1/history", :method => :get },
     def test_routes
       assert_routing(
         { :path => "/api/0.6/way/1/history", :method => :get },

-        { :controller => "api/old_ways", :action => "history", :id => "1" }
+        { :controller => "api/old_ways", :action => "index", :way_id => "1" }

       )
       assert_routing(
       )
       assert_routing(

-        { :path => "/api/0.6/way/1/2", :method => :get },
-        { :controller => "api/old_ways", :action => "show", :id => "1", :version => "2" }
+        { :path => "/api/0.6/way/1/history.json", :method => :get },
+        { :controller => "api/old_ways", :action => "index", :way_id => "1", :format => "json" }

       )
       assert_routing(
       )
       assert_routing(

-        { :path => "/api/0.6/way/1/history.json", :method => :get },
-        { :controller => "api/old_ways", :action => "history", :id => "1", :format => "json" }
+        { :path => "/api/0.6/way/1/2", :method => :get },
+        { :controller => "api/old_ways", :action => "show", :id => "1", :version => "2" }

       )
       assert_routing(
         { :path => "/api/0.6/way/1/2.json", :method => :get },
       )
       assert_routing(
         { :path => "/api/0.6/way/1/2.json", :method => :get },


@@ -27,28 +27,59 @@ module Api
       )
     end
 
       )
     end
 

-    # -------------------------------------
-    # Test reading old ways.
-    # -------------------------------------
+    ##
+    # check that a visible way is returned properly
+    def test_index
+      way = create(:way, :with_history, :version => 2)
+
+      get api_way_versions_path(way)

 
 

-    def test_history_visible
-      # check that a visible way is returned properly
-      get api_way_history_path(create(:way, :with_history))

       assert_response :success
       assert_response :success

+      assert_dom "osm:root", 1 do
+        assert_dom "> way", 2 do |dom_ways|
+          assert_dom dom_ways[0], "> @id", way.id.to_s
+          assert_dom dom_ways[0], "> @version", "1"
+
+          assert_dom dom_ways[1], "> @id", way.id.to_s
+          assert_dom dom_ways[1], "> @version", "2"
+        end
+      end

     end
 
     end
 

-    def test_history_invisible
-      # check that an invisible way's history is returned properly
-      get api_way_history_path(create(:way, :with_history, :deleted))
+    ##
+    # check that an invisible way's history is returned properly
+    def test_index_invisible
+      get api_way_versions_path(create(:way, :with_history, :deleted))

       assert_response :success
     end
 
       assert_response :success
     end
 

-    def test_history_invalid
-      # check chat a non-existent way is not returned
-      get api_way_history_path(0)
+    ##
+    # check chat a non-existent way is not returned
+    def test_index_invalid
+      get api_way_versions_path(0)

       assert_response :not_found
     end
 
       assert_response :not_found
     end
 

+    ##
+    # test that redacted ways aren't visible in the history
+    def test_index_redacted
+      way = create(:way, :with_history, :version => 2)
+      way_v1 = way.old_ways.find_by(:version => 1)
+      way_v1.redact!(create(:redaction))
+
+      get api_way_versions_path(way)
+      assert_response :success, "Redaction shouldn't have stopped history working."
+      assert_select "osm way[id='#{way_v1.way_id}'][version='#{way_v1.version}']", 0,
+                    "redacted way #{way_v1.way_id} version #{way_v1.version} shouldn't be present in the history."
+
+      # not even to a logged-in user
+      auth_header = bearer_authorization_header
+      get api_way_versions_path(way), :headers => auth_header
+      assert_response :success, "Redaction shouldn't have stopped history working."
+      assert_select "osm way[id='#{way_v1.way_id}'][version='#{way_v1.version}']", 0,
+                    "redacted node #{way_v1.way_id} version #{way_v1.version} shouldn't be present in the history, even when logged in."
+    end
+

     ##
     # check that we can retrieve versions of a way
     def test_version
     ##
     # check that we can retrieve versions of a way
     def test_version


@@ -159,26 +190,6 @@ module Api
       assert_response :forbidden, "Redacted way shouldn't be visible via the version API, even when logged in."
     end
 
       assert_response :forbidden, "Redacted way shouldn't be visible via the version API, even when logged in."
     end
 

-    ##
-    # test that redacted ways aren't visible in the history
-    def test_history_redacted
-      way = create(:way, :with_history, :version => 2)
-      way_v1 = way.old_ways.find_by(:version => 1)
-      way_v1.redact!(create(:redaction))
-
-      get api_way_history_path(way)
-      assert_response :success, "Redaction shouldn't have stopped history working."
-      assert_select "osm way[id='#{way_v1.way_id}'][version='#{way_v1.version}']", 0,
-                    "redacted way #{way_v1.way_id} version #{way_v1.version} shouldn't be present in the history."
-
-      # not even to a logged-in user
-      auth_header = bearer_authorization_header
-      get api_way_history_path(way), :headers => auth_header
-      assert_response :success, "Redaction shouldn't have stopped history working."
-      assert_select "osm way[id='#{way_v1.way_id}'][version='#{way_v1.version}']", 0,
-                    "redacted node #{way_v1.way_id} version #{way_v1.version} shouldn't be present in the history, even when logged in."
-    end
-

     ##
     # test the redaction of an old version of a way, while being
     # authorised as a moderator.
     ##
     # test the redaction of an old version of a way, while being
     # authorised as a moderator.


@@ -198,11 +209,11 @@ module Api
       assert_response :success, "After redaction, node should not be gone for moderator, when flag passed."
 
       # and when accessed via history
       assert_response :success, "After redaction, node should not be gone for moderator, when flag passed."
 
       # and when accessed via history

-      get api_way_history_path(way), :headers => auth_header
+      get api_way_versions_path(way), :headers => auth_header

       assert_response :success, "Redaction shouldn't have stopped history working."
       assert_select "osm way[id='#{way_v3.way_id}'][version='#{way_v3.version}']", 0,
                     "way #{way_v3.way_id} version #{way_v3.version} should not be present in the history for moderators when not passing flag."
       assert_response :success, "Redaction shouldn't have stopped history working."
       assert_select "osm way[id='#{way_v3.way_id}'][version='#{way_v3.version}']", 0,
                     "way #{way_v3.way_id} version #{way_v3.version} should not be present in the history for moderators when not passing flag."

-      get api_way_history_path(way, :show_redactions => "true"), :headers => auth_header
+      get api_way_versions_path(way, :show_redactions => "true"), :headers => auth_header

       assert_response :success, "Redaction shouldn't have stopped history working."
       assert_select "osm way[id='#{way_v3.way_id}'][version='#{way_v3.version}']", 1,
                     "way #{way_v3.way_id} version #{way_v3.version} should still be present in the history for moderators when passing flag."
       assert_response :success, "Redaction shouldn't have stopped history working."
       assert_select "osm way[id='#{way_v3.way_id}'][version='#{way_v3.version}']", 1,
                     "way #{way_v3.way_id} version #{way_v3.version} should still be present in the history for moderators when passing flag."


@@ -226,7 +237,7 @@ module Api
       assert_response :forbidden, "Redacted node shouldn't be visible via the version API."
 
       # and when accessed via history
       assert_response :forbidden, "Redacted node shouldn't be visible via the version API."
 
       # and when accessed via history

-      get api_way_history_path(way), :headers => auth_header
+      get api_way_versions_path(way), :headers => auth_header

       assert_response :success, "Redaction shouldn't have stopped history working."
       assert_select "osm way[id='#{way_v3.way_id}'][version='#{way_v3.version}']", 0,
                     "redacted way #{way_v3.way_id} version #{way_v3.version} shouldn't be present in the history."
       assert_response :success, "Redaction shouldn't have stopped history working."
       assert_select "osm way[id='#{way_v3.way_id}'][version='#{way_v3.version}']", 0,
                     "redacted way #{way_v3.way_id} version #{way_v3.version} shouldn't be present in the history."


@@ -278,7 +289,7 @@ module Api
       assert_response :success, "After unredaction, node should not be gone for moderator."
 
       # and when accessed via history
       assert_response :success, "After unredaction, node should not be gone for moderator."
 
       # and when accessed via history

-      get api_way_history_path(way), :headers => auth_header
+      get api_way_versions_path(way), :headers => auth_header

       assert_response :success, "Unredaction shouldn't have stopped history working."
       assert_select "osm way[id='#{way_v1.way_id}'][version='#{way_v1.version}']", 1,
                     "way #{way_v1.way_id} version #{way_v1.version} should still be present in the history for moderators."
       assert_response :success, "Unredaction shouldn't have stopped history working."
       assert_select "osm way[id='#{way_v1.way_id}'][version='#{way_v1.version}']", 1,
                     "way #{way_v1.way_id} version #{way_v1.version} should still be present in the history for moderators."


@@ -290,7 +301,7 @@ module Api
       assert_response :success, "After redaction, node should not be gone for moderator, when flag passed."
 
       # and when accessed via history
       assert_response :success, "After redaction, node should not be gone for moderator, when flag passed."
 
       # and when accessed via history

-      get api_way_history_path(way), :headers => auth_header
+      get api_way_versions_path(way), :headers => auth_header

       assert_response :success, "Redaction shouldn't have stopped history working."
       assert_select "osm way[id='#{way_v1.way_id}'][version='#{way_v1.version}']", 1,
                     "way #{way_v1.way_id} version #{way_v1.version} should still be present in the history for normal users."
       assert_response :success, "Redaction shouldn't have stopped history working."
       assert_select "osm way[id='#{way_v1.way_id}'][version='#{way_v1.version}']", 1,
                     "way #{way_v1.way_id} version #{way_v1.version} should still be present in the history for normal users."


@@ -323,7 +334,7 @@ module Api
     # look at all the versions of the way in the history and get each version from
     # the versions call. check that they're the same.
     def check_history_equals_versions(way_id)
     # look at all the versions of the way in the history and get each version from
     # the versions call. check that they're the same.
     def check_history_equals_versions(way_id)

-      get api_way_history_path(way_id)
+      get api_way_versions_path(way_id)

       assert_response :success, "can't get way #{way_id} from API"
       history_doc = XML::Parser.string(@response.body).parse
       assert_not_nil history_doc, "parsing way #{way_id} history failed"
       assert_response :success, "can't get way #{way_id} from API"
       history_doc = XML::Parser.string(@response.body).parse
       assert_not_nil history_doc, "parsing way #{way_id} history failed"