From: Simon Poole <simon@poole.ch>
Date: Sun, 21 Oct 2018 18:32:12 +0000 (+0100)
Subject: Calculate bounding box when deleting relations
X-Git-Tag: live~2849
X-Git-Url: https://git.openstreetmap.org/rails.git/commitdiff_plain/07ffb4c3f6e20d76a5a9dc476920e42f63b37d8e

Calculate bounding box when deleting relations

Closes #2030
Fixes #2020
---

diff --git a/app/models/relation.rb b/app/models/relation.rb
index 202db12da..f55711a69 100644
--- a/app/models/relation.rb
+++ b/app/models/relation.rb
@@ -384,7 +384,9 @@ class Relation < ActiveRecord::Base
         changed_members.collect { |type, _id, _role| type == "Relation" }
                        .inject(false) { |acc, elem| acc || elem }
 
-      update_members = if tags_changed || any_relations
+      # if the relation is being deleted tags_changed will be true and members empty
+      # so we need to use changed_members to create a correct bounding box
+      update_members = if visible && (tags_changed || any_relations)
                          # add all non-relation bounding boxes to the changeset
                          # FIXME: check for tag changes along with element deletions and
                          # make sure that the deleted element's bounding box is hit.
diff --git a/test/models/relation_test.rb b/test/models/relation_test.rb
index e6d0c0abd..6b76d82a4 100644
--- a/test/models/relation_test.rb
+++ b/test/models/relation_test.rb
@@ -207,4 +207,29 @@ class RelationTest < ActiveSupport::TestCase
     assert_equal 39, changeset.min_lat
     assert_equal 39, changeset.max_lat
   end
+
+  def test_changeset_bbox_delete_relation
+    orig_relation = create(:relation)
+    node1 = create(:node, :longitude => 116, :latitude => 39)
+    node2 = create(:node, :longitude => 39, :latitude => 116)
+    create(:relation_member, :relation => orig_relation, :member_type => "Node", :member_id => node1.id)
+    create(:relation_member, :relation => orig_relation, :member_type => "Node", :member_id => node2.id)
+    user = create(:user)
+    changeset = create(:changeset, :user => user)
+    assert_nil changeset.min_lon
+    assert_nil changeset.max_lon
+    assert_nil changeset.max_lat
+    assert_nil changeset.min_lat
+
+    new_relation = Relation.new
+    new_relation.id = orig_relation.id
+    new_relation.version = orig_relation.version
+    new_relation.changeset_id = changeset.id
+    orig_relation.delete_with_history!(new_relation, user)
+    changeset.reload
+    assert_equal 39, changeset.min_lon
+    assert_equal 116, changeset.max_lon
+    assert_equal 39, changeset.min_lat
+    assert_equal 116, changeset.max_lat
+  end
 end