From: Anton Khorev <tony29@yandex.ru>
Date: Thu, 21 Mar 2024 17:02:18 +0000 (+0300)
Subject: Simplify and test downloaded changeset element sorting
X-Git-Tag: live~830^2
X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/9c0d74fe690b76b1d307c3764532f4f89c7192dd?hp=--cc

Simplify and test downloaded changeset element sorting
---

9c0d74fe690b76b1d307c3764532f4f89c7192dd
diff --git a/app/controllers/api/changesets_controller.rb b/app/controllers/api/changesets_controller.rb
index 8676d16d3..60e6f25d8 100644
--- a/app/controllers/api/changesets_controller.rb
+++ b/app/controllers/api/changesets_controller.rb
@@ -130,13 +130,7 @@ module Api
       # almost sensible ordering available. this would be much nicer if
       # global (SVN-style) versioning were used - then that would be
       # unambiguous.
-      elements.sort! do |a, b|
-        if a.timestamp == b.timestamp
-          a.version <=> b.version
-        else
-          a.timestamp <=> b.timestamp
-        end
-      end
+      elements.sort_by! { |e| [e.timestamp, e.version] }
 
       # generate an output element for each operation. note: we avoid looking
       # at the history because it is simpler - but it would be more correct to
diff --git a/test/controllers/api/changesets_controller_test.rb b/test/controllers/api/changesets_controller_test.rb
index 50ced0ff1..e5c6c0481 100644
--- a/test/controllers/api/changesets_controller_test.rb
+++ b/test/controllers/api/changesets_controller_test.rb
@@ -1995,6 +1995,40 @@ module Api
       end
     end
 
+    test "sorts downloaded elements by timestamp" do
+      changeset = create(:changeset)
+      node1 = create(:old_node, :version => 2, :timestamp => "2020-02-01", :changeset => changeset)
+      node0 = create(:old_node, :version => 2, :timestamp => "2020-01-01", :changeset => changeset)
+
+      get changeset_download_path(changeset)
+      assert_response :success
+      assert_dom "modify", :count => 2 do |modify|
+        assert_dom modify[0], ">node", :count => 1 do |node|
+          assert_dom node, ">@id", node0.node_id.to_s
+        end
+        assert_dom modify[1], ">node", :count => 1 do |node|
+          assert_dom node, ">@id", node1.node_id.to_s
+        end
+      end
+    end
+
+    test "sorts downloaded elements by version" do
+      changeset = create(:changeset)
+      node1 = create(:old_node, :version => 3, :timestamp => "2020-01-01", :changeset => changeset)
+      node0 = create(:old_node, :version => 2, :timestamp => "2020-01-01", :changeset => changeset)
+
+      get changeset_download_path(changeset)
+      assert_response :success
+      assert_dom "modify", :count => 2 do |modify|
+        assert_dom modify[0], ">node", :count => 1 do |node|
+          assert_dom node, ">@id", node0.node_id.to_s
+        end
+        assert_dom modify[1], ">node", :count => 1 do |node|
+          assert_dom node, ">@id", node1.node_id.to_s
+        end
+      end
+    end
+
     ##
     # check that the bounding box of a changeset gets updated correctly
     # FIXME: This should really be moded to a integration test due to the with_controller