Merge remote-tracking branch 'upstream/pull/4612'

app/controllers/api/changesets_controller.rb

@@ -130,13 +130,7 @@ def download
       # 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

test/controllers/api/changesets_controller_test.rb

@@ -1995,6 +1995,40 @@ def test_changeset_download
       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