From e69e8c9018a36dc6d5ceca3752887b43c25e0428 Mon Sep 17 00:00:00 2001 From: Andy Allan Date: Wed, 17 May 2017 09:56:44 +0100 Subject: [PATCH 1/6] Refactor remaining way model tests to use factories. Note that test_max_nodes_per_way_limit still has issues - see #1516 --- test/models/way_test.rb | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/test/models/way_test.rb b/test/models/way_test.rb index 807b71d5bf..fe8dacf331 100644 --- a/test/models/way_test.rb +++ b/test/models/way_test.rb @@ -10,11 +10,16 @@ def test_db_count end def test_bbox - node = current_nodes(:used_node_1) - [:visible_way, - :invisible_way, - :used_way].each do |way_symbol| - way = current_ways(way_symbol) + node = create(:node) + visible_way = create(:way) + create(:way_node, :way => visible_way, :node => node) + invisible_way = create(:way) + create(:way_node, :way => invisible_way, :node => node) + used_way = create(:way) + create(:way_node, :way => used_way, :node => node) + create(:relation_member, :member => used_way) + + [visible_way, invisible_way, used_way].each do |way| assert_equal node.bbox.min_lon, way.bbox.min_lon, "min_lon" assert_equal node.bbox.min_lat, way.bbox.min_lat, "min_lat" assert_equal node.bbox.max_lon, way.bbox.max_lon, "max_lon" @@ -25,18 +30,22 @@ def test_bbox # Check that the preconditions fail when you are over the defined limit of # the maximum number of nodes in each way. def test_max_nodes_per_way_limit + node_a = create(:node) + node_b = create(:node) + node_c = create(:node) + way = create(:way_with_nodes, :nodes_count => 1) # Take one of the current ways and add nodes to it until we are near the limit way = Way.find(current_ways(:visible_way).id) assert way.valid? # it already has 1 node 1.upto(MAX_NUMBER_OF_WAY_NODES / 2) do - way.add_nd_num(current_nodes(:used_node_1).id) - way.add_nd_num(current_nodes(:used_node_2).id) + way.add_nd_num(node_a.id) + way.add_nd_num(node_b.id) end way.save # print way.nds.size assert way.valid? - way.add_nd_num(current_nodes(:visible_node).id) + way.add_nd_num(node_c.id) assert way.valid? end From 084ae0f19109766a7eea248f1d308e24eda1578c Mon Sep 17 00:00:00 2001 From: Andy Allan Date: Wed, 17 May 2017 10:30:28 +0100 Subject: [PATCH 2/6] Refactor get_nodes_undelete test to use factories instead of fixtures. --- test/models/old_way_test.rb | 41 ++++++++++++++++++++++++++----------- 1 file changed, 29 insertions(+), 12 deletions(-) diff --git a/test/models/old_way_test.rb b/test/models/old_way_test.rb index a1f47b13a0..16b5d33633 100644 --- a/test/models/old_way_test.rb +++ b/test/models/old_way_test.rb @@ -105,19 +105,36 @@ def test_tags end def test_get_nodes_undelete - way = ways(:way_with_versions_v3) - node_tag = create(:node_tag, :node => current_nodes(:node_with_versions)) - node_tag2 = create(:node_tag, :node => current_nodes(:used_node_1)) - nodes = OldWay.find(way.id).get_nodes_undelete + way = create(:way, :with_history, :version => 4) + way_v3 = way.old_ways.find_by(:version => 3) + way_v4 = way.old_ways.find_by(:version => 4) + node_a = create(:node, :with_history, :version => 4) + node_b = create(:node, :with_history, :version => 3) + node_c = create(:node, :with_history, :version => 2) + node_d = create(:node, :with_history, :deleted, :version => 1) + create(:old_way_node, :old_way => way_v3, :node => node_a, :sequence_id => 1) + create(:old_way_node, :old_way => way_v3, :node => node_b, :sequence_id => 2) + create(:old_way_node, :old_way => way_v4, :node => node_c, :sequence_id => 1) + node_tag = create(:node_tag, :node => node_a) + node_tag2 = create(:node_tag, :node => node_b) + node_tag3 = create(:node_tag, :node => node_d) + + nodes = OldWay.find(way_v3.id).get_nodes_undelete assert_equal 2, nodes.size - assert_equal [1.0, 1.0, 15, 4, { node_tag.k => node_tag.v }, true], nodes[0] - assert_equal [3.0, 3.0, 3, 1, { node_tag2.k => node_tag2.v }, true], nodes[1] - - way = ways(:way_with_redacted_versions_v2) - node_tag3 = create(:node_tag, :node => current_nodes(:invisible_node)) - nodes = OldWay.find(way.id).get_nodes_undelete + assert_equal [node_a.lon, node_a.lat, node_a.id, node_a.version, { node_tag.k => node_tag.v }, true], nodes[0] + assert_equal [node_b.lon, node_b.lat, node_b.id, node_b.version, { node_tag2.k => node_tag2.v }, true], nodes[1] + + redacted_way = create(:way, :with_history, :version => 3) + redacted_way_v2 = redacted_way.old_ways.find_by(:version => 2) + redacted_way_v3 = redacted_way.old_ways.find_by(:version => 3) + create(:old_way_node, :old_way => redacted_way_v2, :node => node_b, :sequence_id => 1) + create(:old_way_node, :old_way => redacted_way_v2, :node => node_d, :sequence_id => 2) + create(:old_way_node, :old_way => redacted_way_v3, :node => node_c, :sequence_id => 1) + redacted_way_v2.redact!(create(:redaction)) + + nodes = OldWay.find(redacted_way_v2.id).get_nodes_undelete assert_equal 2, nodes.size - assert_equal [3.0, 3.0, 3, 1, { node_tag2.k => node_tag2.v }, true], nodes[0] - assert_equal [2.0, 2.0, 2, 1, { node_tag3.k => node_tag3.v }, false], nodes[1] + assert_equal [node_b.lon, node_b.lat, node_b.id, node_b.version, { node_tag2.k => node_tag2.v }, true], nodes[0] + assert_equal [node_d.lon, node_d.lat, node_d.id, node_d.version, { node_tag3.k => node_tag3.v }, false], nodes[1] end end From c7d612df2d3a6d60582216a28a1426f351a41970 Mon Sep 17 00:00:00 2001 From: Andy Allan Date: Wed, 17 May 2017 11:11:04 +0100 Subject: [PATCH 3/6] Refactor the relation_for_nwr tests to use factories This is a bit verbose, since the original tests assumed various other entities were silently not selected. This also improves the tests since it considers the case of how to handle a relation with the target member appearing more than once. --- test/controllers/relation_controller_test.rb | 65 +++++++++++++++++--- 1 file changed, 55 insertions(+), 10 deletions(-) diff --git a/test/controllers/relation_controller_test.rb b/test/controllers/relation_controller_test.rb index 0366ceef03..bb8f2d437c 100644 --- a/test/controllers/relation_controller_test.rb +++ b/test/controllers/relation_controller_test.rb @@ -68,21 +68,65 @@ def test_read # check that all relations containing a particular node, and no extra # relations, are returned from the relations_for_node call. def test_relations_for_node + node = create(:node) + # should include relations with that node as a member + relation_with_node = create(:relation_member, :member => node).relation + # should ignore relations without that node as a member + _relation_without_node = create(:relation_member).relation + # should ignore relations with the node involved indirectly, via a way + way = create(:way_node, :node => node).way + _relation_with_way = create(:relation_member, :member => way).relation + # should ignore relations with the node involved indirectly, via a relation + second_relation = create(:relation_member, :member => node).relation + _super_relation = create(:relation_member, :member => second_relation).relation + # should combine multiple relation_member references into just one relation entry + create(:relation_member, :member => node, :relation => relation_with_node, :sequence_id => 2) + # should not include deleted relations + deleted_relation = create(:relation, :deleted) + create(:relation_member, :member => node, :relation => deleted_relation) + check_relations_for_element(:relations_for_node, "node", - current_nodes(:node_used_by_relationship).id, - [:visible_relation, :used_relation]) + node.id, + [relation_with_node, second_relation]) end def test_relations_for_way + way = create(:way) + # should include relations with that way as a member + relation_with_way = create(:relation_member, :member => way).relation + # should ignore relations without that way as a member + _relation_without_way = create(:relation_member).relation + # should ignore relations with the way involved indirectly, via a relation + second_relation = create(:relation_member, :member => way).relation + _super_relation = create(:relation_member, :member => second_relation).relation + # should combine multiple relation_member references into just one relation entry + create(:relation_member, :member => way, :relation => relation_with_way, :sequence_id => 2) + # should not include deleted relations + deleted_relation = create(:relation, :deleted) + create(:relation_member, :member => way, :relation => deleted_relation) + check_relations_for_element(:relations_for_way, "way", - current_ways(:used_way).id, - [:visible_relation]) + way.id, + [relation_with_way, second_relation]) end def test_relations_for_relation + relation = create(:relation) + # should include relations with that relation as a member + relation_with_relation = create(:relation_member, :member => relation).relation + # should ignore any relation without that relation as a member + _relation_without_relation = create(:relation_member).relation + # should ignore relations with the relation involved indirectly, via a relation + second_relation = create(:relation_member, :member => relation).relation + _super_relation = create(:relation_member, :member => second_relation).relation + # should combine multiple relation_member references into just one relation entry + create(:relation_member, :member => relation, :relation => relation_with_relation, :sequence_id => 2) + # should not include deleted relations + deleted_relation = create(:relation, :deleted) + create(:relation_member, :member => relation, :relation => deleted_relation) check_relations_for_element(:relations_for_relation, "relation", - current_relations(:used_relation).id, - [:visible_relation]) + relation.id, + [relation_with_relation, second_relation]) end def check_relations_for_element(method, type, id, expected_relations) @@ -96,10 +140,11 @@ def check_relations_for_element(method, type, id, expected_relations) # we should have only the expected number of relations assert_select "osm>relation", expected_relations.size - # and each of them should contain the node we originally searched for - expected_relations.each do |r| - relation_id = current_relations(r).id - assert_select "osm>relation[id='#{relation_id}']>member[type='#{type}'][ref='#{id}']", 1 + # and each of them should contain the element we originally searched for + expected_relations.each do |relation| + # The relation should appear once, but the element could appear multiple times + assert_select "osm>relation[id='#{relation.id}']", 1 + assert_select "osm>relation[id='#{relation.id}']>member[type='#{type}'][ref='#{id}']" end end From 63a2f1f1b4ce571d2a3048628fa756fcc7aa1cbb Mon Sep 17 00:00:00 2001 From: Andy Allan Date: Wed, 17 May 2017 11:29:11 +0100 Subject: [PATCH 4/6] Refactor some relation_controller tests to use factories. --- test/controllers/relation_controller_test.rb | 82 +++++++++----------- 1 file changed, 38 insertions(+), 44 deletions(-) diff --git a/test/controllers/relation_controller_test.rb b/test/controllers/relation_controller_test.rb index bb8f2d437c..19acbc1d51 100644 --- a/test/controllers/relation_controller_test.rb +++ b/test/controllers/relation_controller_test.rb @@ -193,13 +193,17 @@ def test_relations # ------------------------------------- def test_create - basic_authorization users(:normal_user).email, "test" + private_user = create(:user, :data_public => false) + private_changeset = create(:changeset, :user => private_user) + user = create(:user) + changeset = create(:changeset, :user => user) + node = create(:node) + way = create(:way_with_nodes, :nodes_count => 2) - # put the relation in a dummy fixture changset - changeset_id = changesets(:normal_user_first_change).id + basic_authorization private_user.email, "test" # create an relation without members - content "" + content "" put :create # hope for forbidden, due to user assert_response :forbidden, @@ -208,9 +212,8 @@ def test_create ### # create an relation with a node as member # This time try with a role attribute in the relation - nid = current_nodes(:used_node_1).id - content "" + - "" + + content "" + + "" + "" put :create # hope for forbidden due to user @@ -220,9 +223,8 @@ def test_create ### # create an relation with a node as member, this time test that we don't # need a role attribute to be included - nid = current_nodes(:used_node_1).id - content "" + - "" + "" + content "" + + "" + "" put :create # hope for forbidden due to user assert_response :forbidden, @@ -230,11 +232,9 @@ def test_create ### # create an relation with a way and a node as members - nid = current_nodes(:used_node_1).id - wid = current_ways(:used_way).id - content "" + - "" + - "" + + content "" + + "" + + "" + "" put :create # hope for forbidden, due to user @@ -242,13 +242,10 @@ def test_create "relation upload did not return success status" ## Now try with the public user - basic_authorization users(:public_user).email, "test" - - # put the relation in a dummy fixture changset - changeset_id = changesets(:public_user_first_change).id + basic_authorization user.email, "test" # create an relation without members - content "" + content "" put :create # hope for success assert_response :success, @@ -263,9 +260,9 @@ def test_create "saved relation contains members but should not" assert_equal checkrelation.tags.length, 1, "saved relation does not contain exactly one tag" - assert_equal changeset_id, checkrelation.changeset.id, + assert_equal changeset.id, checkrelation.changeset.id, "saved relation does not belong in the changeset it was assigned to" - assert_equal users(:public_user).id, checkrelation.changeset.user_id, + assert_equal user.id, checkrelation.changeset.user_id, "saved relation does not belong to user that created it" assert_equal true, checkrelation.visible, "saved relation is not visible" @@ -276,9 +273,8 @@ def test_create ### # create an relation with a node as member # This time try with a role attribute in the relation - nid = current_nodes(:used_node_1).id - content "" + - "" + + content "" + + "" + "" put :create # hope for success @@ -294,9 +290,9 @@ def test_create "saved relation does not contain exactly one member" assert_equal checkrelation.tags.length, 1, "saved relation does not contain exactly one tag" - assert_equal changeset_id, checkrelation.changeset.id, + assert_equal changeset.id, checkrelation.changeset.id, "saved relation does not belong in the changeset it was assigned to" - assert_equal users(:public_user).id, checkrelation.changeset.user_id, + assert_equal user.id, checkrelation.changeset.user_id, "saved relation does not belong to user that created it" assert_equal true, checkrelation.visible, "saved relation is not visible" @@ -308,9 +304,8 @@ def test_create ### # create an relation with a node as member, this time test that we don't # need a role attribute to be included - nid = current_nodes(:used_node_1).id - content "" + - "" + "" + content "" + + "" + "" put :create # hope for success assert_response :success, @@ -325,9 +320,9 @@ def test_create "saved relation does not contain exactly one member" assert_equal checkrelation.tags.length, 1, "saved relation does not contain exactly one tag" - assert_equal changeset_id, checkrelation.changeset.id, + assert_equal changeset.id, checkrelation.changeset.id, "saved relation does not belong in the changeset it was assigned to" - assert_equal users(:public_user).id, checkrelation.changeset.user_id, + assert_equal user.id, checkrelation.changeset.user_id, "saved relation does not belong to user that created it" assert_equal true, checkrelation.visible, "saved relation is not visible" @@ -338,11 +333,9 @@ def test_create ### # create an relation with a way and a node as members - nid = current_nodes(:used_node_1).id - wid = current_ways(:used_way).id - content "" + - "" + - "" + + content "" + + "" + + "" + "" put :create # hope for success @@ -358,9 +351,9 @@ def test_create "saved relation does not have exactly two members" assert_equal checkrelation.tags.length, 1, "saved relation does not contain exactly one tag" - assert_equal changeset_id, checkrelation.changeset.id, + assert_equal changeset.id, checkrelation.changeset.id, "saved relation does not belong in the changeset it was assigned to" - assert_equal users(:public_user).id, checkrelation.changeset.user_id, + assert_equal user.id, checkrelation.changeset.user_id, "saved relation does not belong to user that created it" assert_equal true, checkrelation.visible, "saved relation is not visible" @@ -472,14 +465,15 @@ def test_create_invalid # Test creating a relation, with some invalid XML # ------------------------------------- def test_create_invalid_xml - basic_authorization users(:public_user).email, "test" + user = create(:user) + changeset = create(:changeset, :user => user) + node = create(:node) - # put the relation in a dummy fixture changeset that works - changeset_id = changesets(:public_user_first_change).id + basic_authorization user.email, "test" # create some xml that should return an error - content "" + - "" + + content "" + + "" + "" put :create # expect failure From f0bacf837d2deeabfc9db11de89d7aa238f0b63a Mon Sep 17 00:00:00 2001 From: Andy Allan Date: Wed, 17 May 2017 11:59:22 +0100 Subject: [PATCH 5/6] Update more relation_controller tests to use factories. --- test/controllers/relation_controller_test.rb | 141 ++++++++++--------- 1 file changed, 75 insertions(+), 66 deletions(-) diff --git a/test/controllers/relation_controller_test.rb b/test/controllers/relation_controller_test.rb index 19acbc1d51..fc8b8bbe55 100644 --- a/test/controllers/relation_controller_test.rb +++ b/test/controllers/relation_controller_test.rb @@ -403,16 +403,18 @@ def test_update_relation_tags # and the API gives sensible results. this is to test a case that # gregory marler noticed and posted to josm-dev. def test_update_relation_tags_via_upload - basic_authorization users(:public_user).email, "test" - rel_id = current_relations(:multi_tag_relation).id - create_list(:relation_tag, 4, :relation => current_relations(:multi_tag_relation)) - cs_id = changesets(:public_user_first_change).id + user = create(:user) + changeset = create(:changeset, :user => user) + relation = create(:relation) + create_list(:relation_tag, 4, :relation => relation) - with_relation(rel_id) do |rel| + basic_authorization user.email, "test" + + with_relation(relation.id) do |rel| # alter one of the tags tag = rel.find("//osm/relation/tag").first tag["v"] = "some changed value" - update_changeset(rel, cs_id) + update_changeset(rel, changeset.id) # check that the downloaded tags are the same as the uploaded tags... new_version = with_update_diff(rel) do |new_rel| @@ -420,22 +422,24 @@ def test_update_relation_tags_via_upload end # check the original one in the current_* table again - with_relation(rel_id) { |r| assert_tags_equal rel, r } + with_relation(relation.id) { |r| assert_tags_equal rel, r } # now check the version in the history - with_relation(rel_id, new_version) { |r| assert_tags_equal rel, r } + with_relation(relation.id, new_version) { |r| assert_tags_equal rel, r } end end def test_update_wrong_id - basic_authorization users(:public_user).email, "test" - rel_id = current_relations(:multi_tag_relation).id - cs_id = changesets(:public_user_first_change).id + user = create(:user) + changeset = create(:changeset, :user => user) + relation = create(:relation) + other_relation = create(:relation) - with_relation(rel_id) do |rel| - update_changeset(rel, cs_id) + basic_authorization user.email, "test" + with_relation(relation.id) do |rel| + update_changeset(rel, changeset.id) content rel - put :update, :id => current_relations(:visible_relation).id + put :update, :id => other_relation.id assert_response :bad_request end end @@ -445,13 +449,13 @@ def test_update_wrong_id # ------------------------------------- def test_create_invalid - basic_authorization users(:public_user).email, "test" + user = create(:user) + changeset = create(:changeset, :user => user) - # put the relation in a dummy fixture changset - changeset_id = changesets(:public_user_first_change).id + basic_authorization user.email, "test" # create a relation with non-existing node as member - content "" + + content "" + "" + "" put :create @@ -487,52 +491,58 @@ def test_create_invalid_xml # ------------------------------------- def test_delete + private_user = create(:user, :data_public => false) + private_user_closed_changeset = create(:changeset, :closed, :user => private_user) + user = create(:user) + closed_changeset = create(:changeset, :closed, :user => user) + changeset = create(:changeset, :user => user) + relation = create(:relation) + used_relation = create(:relation) + super_relation = create(:relation_member, :member => used_relation).relation + deleted_relation = create(:relation, :deleted) + multi_tag_relation = create(:relation) + create_list(:relation_tag, 4, :relation => multi_tag_relation) + ## First try to delete relation without auth - delete :delete, :id => current_relations(:visible_relation).id + delete :delete, :id => relation.id assert_response :unauthorized ## Then try with the private user, to make sure that you get a forbidden - basic_authorization(users(:normal_user).email, "test") + basic_authorization(private_user.email, "test") # this shouldn't work, as we should need the payload... - delete :delete, :id => current_relations(:visible_relation).id + delete :delete, :id => relation.id assert_response :forbidden # try to delete without specifying a changeset - content "" - delete :delete, :id => current_relations(:visible_relation).id + content "" + delete :delete, :id => relation.id assert_response :forbidden # try to delete with an invalid (closed) changeset - content update_changeset(current_relations(:visible_relation).to_xml, - changesets(:normal_user_closed_change).id) - delete :delete, :id => current_relations(:visible_relation).id + content update_changeset(relation.to_xml, + private_user_closed_changeset.id) + delete :delete, :id => relation.id assert_response :forbidden # try to delete with an invalid (non-existent) changeset - content update_changeset(current_relations(:visible_relation).to_xml, 0) - delete :delete, :id => current_relations(:visible_relation).id + content update_changeset(relation.to_xml, 0) + delete :delete, :id => relation.id assert_response :forbidden # this won't work because the relation is in-use by another relation - content(relations(:used_relation).to_xml) - delete :delete, :id => current_relations(:used_relation).id + content(used_relation.to_xml) + delete :delete, :id => used_relation.id assert_response :forbidden # this should work when we provide the appropriate payload... - content(relations(:visible_relation).to_xml) - delete :delete, :id => current_relations(:visible_relation).id + content(relation.to_xml) + delete :delete, :id => relation.id assert_response :forbidden # this won't work since the relation is already deleted - content(relations(:invisible_relation).to_xml) - delete :delete, :id => current_relations(:invisible_relation).id - assert_response :forbidden - - # this works now because the relation which was using this one - # has been deleted. - content(relations(:used_relation).to_xml) - delete :delete, :id => current_relations(:used_relation).id + content(deleted_relation.to_xml) + delete :delete, :id => deleted_relation.id assert_response :forbidden # this won't work since the relation never existed @@ -540,72 +550,71 @@ def test_delete assert_response :forbidden ## now set auth for the public user - basic_authorization(users(:public_user).email, "test") + basic_authorization(user.email, "test") # this shouldn't work, as we should need the payload... - delete :delete, :id => current_relations(:visible_relation).id + delete :delete, :id => relation.id assert_response :bad_request # try to delete without specifying a changeset - content "" - delete :delete, :id => current_relations(:visible_relation).id + content "" + delete :delete, :id => relation.id assert_response :bad_request assert_match(/Changeset id is missing/, @response.body) # try to delete with an invalid (closed) changeset - content update_changeset(current_relations(:visible_relation).to_xml, - changesets(:normal_user_closed_change).id) - delete :delete, :id => current_relations(:visible_relation).id + content update_changeset(relation.to_xml, + closed_changeset.id) + delete :delete, :id => relation.id assert_response :conflict # try to delete with an invalid (non-existent) changeset - content update_changeset(current_relations(:visible_relation).to_xml, 0) - delete :delete, :id => current_relations(:visible_relation).id + content update_changeset(relation.to_xml, 0) + delete :delete, :id => relation.id assert_response :conflict # this won't work because the relation is in a changeset owned by someone else - content(relations(:used_relation).to_xml) - delete :delete, :id => current_relations(:used_relation).id + content update_changeset(relation.to_xml, create(:changeset).id) + delete :delete, :id => relation.id assert_response :conflict, "shouldn't be able to delete a relation in a changeset owned by someone else (#{@response.body})" # this won't work because the relation in the payload is different to that passed - content(relations(:public_used_relation).to_xml) - delete :delete, :id => current_relations(:used_relation).id - assert_not_equal relations(:public_used_relation).id, current_relations(:used_relation).id + content update_changeset(relation.to_xml, changeset.id) + delete :delete, :id => create(:relation).id assert_response :bad_request, "shouldn't be able to delete a relation when payload is different to the url" # this won't work because the relation is in-use by another relation - content(relations(:public_used_relation).to_xml) - delete :delete, :id => current_relations(:public_used_relation).id + content update_changeset(used_relation.to_xml, changeset.id) + delete :delete, :id => used_relation.id assert_response :precondition_failed, "shouldn't be able to delete a relation used in a relation (#{@response.body})" - assert_equal "Precondition failed: The relation 5 is used in relation 6.", @response.body + assert_equal "Precondition failed: The relation #{used_relation.id} is used in relation #{super_relation.id}.", @response.body # this should work when we provide the appropriate payload... - content(relations(:multi_tag_relation).to_xml) - delete :delete, :id => current_relations(:multi_tag_relation).id + content update_changeset(multi_tag_relation.to_xml, changeset.id) + delete :delete, :id => multi_tag_relation.id assert_response :success # valid delete should return the new version number, which should # be greater than the old version number - assert @response.body.to_i > current_relations(:visible_relation).version, + assert @response.body.to_i > multi_tag_relation.version, "delete request should return a new version number for relation" # this won't work since the relation is already deleted - content(relations(:invisible_relation).to_xml) - delete :delete, :id => current_relations(:invisible_relation).id + content update_changeset(deleted_relation.to_xml, changeset.id) + delete :delete, :id => deleted_relation.id assert_response :gone # Public visible relation needs to be deleted - content(relations(:public_visible_relation).to_xml) - delete :delete, :id => current_relations(:public_visible_relation).id + content update_changeset(super_relation.to_xml, changeset.id) + delete :delete, :id => super_relation.id assert_response :success # this works now because the relation which was using this one # has been deleted. - content(relations(:public_used_relation).to_xml) - delete :delete, :id => current_relations(:public_used_relation).id + content update_changeset(used_relation.to_xml, changeset.id) + delete :delete, :id => used_relation.id assert_response :success, "should be able to delete a relation used in an old relation (#{@response.body})" From 2e8c0d471fbefa97d1dd4fb5c5749280a4187f88 Mon Sep 17 00:00:00 2001 From: Andy Allan Date: Wed, 17 May 2017 16:05:23 +0100 Subject: [PATCH 6/6] Ensure invisible_way is actually deleted --- test/models/way_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/models/way_test.rb b/test/models/way_test.rb index fe8dacf331..3413530466 100644 --- a/test/models/way_test.rb +++ b/test/models/way_test.rb @@ -13,7 +13,7 @@ def test_bbox node = create(:node) visible_way = create(:way) create(:way_node, :way => visible_way, :node => node) - invisible_way = create(:way) + invisible_way = create(:way, :deleted) create(:way_node, :way => invisible_way, :node => node) used_way = create(:way) create(:way_node, :way => used_way, :node => node)