Simplify and test downloaded changeset element sorting

diff --git a/app/controllers/api/changesets_controller.rb b/app/controllers/api/changesets_controller.rb
index 8676d16d3699192698aa2e7fe37e566463a37811..60e6f25d8ae370c1f5df98e2bb91bfbefc9cfc11 100644 (file)
--- 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 50ced0ff1906aef38377086c46e193a674805d63..e5c6c0481d96701cd6a6a0a074b5cdadf43bd653 100644 (file)
--- 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