Make node update work with the new require data public to edit policy. Added convenience testing method for the require data public. Add 2 new fixtures that are owned by the public user.

test/fixtures/current_nodes.yml

@@ -150,3 +150,14 @@ node_with_versions:
   version: 4
   tile: <%= QuadTile.tile_for_point(1,1) %>
   timestamp: 2008-01-01 00:04:00
+  
+public_visible_node:
+  id: 16
+  latitude: <%= 1*SCALE %>
+  longitude: <%= 1*SCALE %>
+  changeset_id: 2
+  visible: true
+  version: 1
+  tile: <%= QuadTile.tile_for_point(1,1) %>
+  timestamp: 2007-01-01 00:00:00
+

test/fixtures/nodes.yml

@@ -180,3 +180,14 @@ node_with_versions_v4:
   version: 4
   tile: <%= QuadTile.tile_for_point(1,1) %>
   timestamp: 2008-01-01 00:04:00
+  
+public_visible_node:
+  id: 16
+  latitude: <%= 1*SCALE %>
+  longitude: <%= 1*SCALE %>
+  changeset_id: 2
+  visible: true
+  version: 1
+  tile: <%= QuadTile.tile_for_point(1,1) %>
+  timestamp: 2007-01-01 00:00:00
+

test/functional/node_controller_test.rb

@@ -127,12 +127,17 @@ class NodeControllerTest < ActionController::TestCase
   # tests whether the API works and prevents incorrect use while trying
   # to update nodes.
   def test_update
+    ## First test with no user credentials
     # try and update a node without authorisation
     # first try to delete node without auth
     content current_nodes(:visible_node).to_xml
     put :update, :id => current_nodes(:visible_node).id
     assert_response :unauthorized
     
+    
+    
+    ## Second test with the private user
+    
     # setup auth
     basic_authorization(users(:normal_user).email, "test")
 
@@ -140,7 +145,62 @@ class NodeControllerTest < ActionController::TestCase
 
     # try and update in someone else's changeset
     content update_changeset(current_nodes(:visible_node).to_xml,
-                             changesets(:second_user_first_change).id)
+                             changesets(:public_user_first_change).id)
+    put :update, :id => current_nodes(:visible_node).id
+    assert_require_public_data "update with other user's changeset should be forbidden when date isn't public"
+
+    # try and update in a closed changeset
+    content update_changeset(current_nodes(:visible_node).to_xml,
+                             changesets(:normal_user_closed_change).id)
+    put :update, :id => current_nodes(:visible_node).id
+    assert_require_public_data "update with closed changeset should be forbidden, when data isn't public"
+
+    # try and update in a non-existant changeset
+    content update_changeset(current_nodes(:visible_node).to_xml, 0)
+    put :update, :id => current_nodes(:visible_node).id
+    assert_require_public_data("update with changeset=0 should be forbidden, when data isn't public")
+
+    ## try and submit invalid updates
+    content xml_attr_rewrite(current_nodes(:visible_node).to_xml, 'lat', 91.0);
+    put :update, :id => current_nodes(:visible_node).id
+    assert_require_public_data "node at lat=91 should be forbidden, when data isn't public"
+
+    content xml_attr_rewrite(current_nodes(:visible_node).to_xml, 'lat', -91.0);
+    put :update, :id => current_nodes(:visible_node).id
+    assert_require_public_data "node at lat=-91 should be forbidden, when data isn't public"
+    
+    content xml_attr_rewrite(current_nodes(:visible_node).to_xml, 'lon', 181.0);
+    put :update, :id => current_nodes(:visible_node).id
+    assert_require_public_data "node at lon=181 should be forbidden, when data isn't public"
+
+    content xml_attr_rewrite(current_nodes(:visible_node).to_xml, 'lon', -181.0);
+    put :update, :id => current_nodes(:visible_node).id
+    assert_require_public_data "node at lon=-181 should be forbidden, when data isn't public"
+    
+    ## finally, produce a good request which should work
+    content current_nodes(:visible_node).to_xml
+    put :update, :id => current_nodes(:visible_node).id
+    assert_require_public_data "should have failed with a forbidden when data isn't public"
+
+    
+    
+    
+    ## Finally test with the public user
+    
+    # try and update a node without authorisation
+    # first try to delete node without auth
+    content current_nodes(:visible_node).to_xml
+    put :update, :id => current_nodes(:visible_node).id
+    assert_response :forbidden
+    
+    # setup auth
+    basic_authorization(users(:public_user).email, "test")
+
+    ## trying to break changesets
+
+    # try and update in someone else's changeset
+    content update_changeset(current_nodes(:visible_node).to_xml,
+                              changesets(:normal_user_first_change).id)
     put :update, :id => current_nodes(:visible_node).id
     assert_response :conflict, "update with other user's changeset should be rejected"
 
@@ -195,8 +255,8 @@ class NodeControllerTest < ActionController::TestCase
        "should not be able to put 'p1r4at3s!' in the version field"
     
     ## finally, produce a good request which should work
-    content current_nodes(:visible_node).to_xml
+    content current_nodes(:public_visible_node).to_xml
-    put :update, :id => current_nodes(:visible_node).id
+    put :update, :id => current_nodes(:public_visible_node).id
     assert_response :success, "a valid update request failed"
   end
 

test/functional/relation_controller_test.rb

@@ -269,6 +269,7 @@ class RelationControllerTest < ActionController::TestCase
   # happen to the correct tables and the API gives sensible results. 
   # this is to test a case that gregory marler noticed and posted to
   # josm-dev.
+  ## FIXME Move this to an integration test
   def test_update_relation_tags
     basic_authorization "test@example.com", "test"
     rel_id = current_relations(:multi_tag_relation).id

test/test_helper.rb

@@ -122,6 +122,13 @@ class Test::Unit::TestCase
   def content(c)
     @request.env["RAW_POST_DATA"] = c.to_s
   end
-
+  
+  # Used to check that the error header and the forbidden responses are given
+  # when the owner of the changset has their data not marked as public
+  def assert_require_public_data(msg = "Shouldn't be able to use API when the user's data is not public")
+    assert_response :forbidden, msg
+    assert_equal @response.headers['Error'], "You must make your edits public to upload new data", "Wrong error message"
+  end
+  
   # Add more helper methods to be used by all tests here...
 end

test/unit/node_test.rb

@@ -2,6 +2,10 @@ require File.dirname(__FILE__) + '/../test_helper'
 
 class NodeTest < Test::Unit::TestCase
   api_fixtures
+  
+  def test_node_count
+    assert_equal 16, Node.count
+  end
 
   def test_node_too_far_north
 	  invalid_node_test(:node_too_far_north)

test/unit/old_node_test.rb

@@ -2,6 +2,10 @@ require File.dirname(__FILE__) + '/../test_helper'
 
 class OldNodeTest < Test::Unit::TestCase
   api_fixtures
+  
+  def test_old_node_count
+    assert_equal 19, OldNode.count
+  end
 
   def test_node_too_far_north
 	  invalid_node_test(:node_too_far_north)