From 59950bff8739d34d1b005822924bd91fb29a8b3f Mon Sep 17 00:00:00 2001 From: Eric Chan Date: Wed, 13 Sep 2017 01:37:40 -0400 Subject: [PATCH 1/3] Fix sort_natural on sorting with non-string values --- lib/liquid/standardfilters.rb | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/lib/liquid/standardfilters.rb b/lib/liquid/standardfilters.rb index 7c18c0d..9ebb763 100644 --- a/lib/liquid/standardfilters.rb +++ b/lib/liquid/standardfilters.rb @@ -143,11 +143,19 @@ module Liquid ary = InputIterator.new(input) if property.nil? - ary.sort { |a, b| a.casecmp(b) } + ary.sort { |a, b| a.to_s.casecmp(b.to_s) } elsif ary.empty? # The next two cases assume a non-empty array. [] elsif ary.first.respond_to?(:[]) && !ary.first[property].nil? - ary.sort { |a, b| a[property].casecmp(b[property]) } + ary.sort do |a, b| + a = a[property] + b = b[property] + if a && b + a[property].to_s.casecmp(b[property].to_s) + else + a ? -1 : 1 + end + end end end From cfe1844de920970ceb91db600e987b18bb2c8462 Mon Sep 17 00:00:00 2001 From: Eric Chan Date: Wed, 13 Sep 2017 22:17:59 -0400 Subject: [PATCH 2/3] Added test coverage for sort_natural --- lib/liquid/standardfilters.rb | 2 +- test/integration/standard_filter_test.rb | 41 ++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/lib/liquid/standardfilters.rb b/lib/liquid/standardfilters.rb index 9ebb763..f14ba1d 100644 --- a/lib/liquid/standardfilters.rb +++ b/lib/liquid/standardfilters.rb @@ -151,7 +151,7 @@ module Liquid a = a[property] b = b[property] if a && b - a[property].to_s.casecmp(b[property].to_s) + a.to_s.casecmp(b.to_s) else a ? -1 : 1 end diff --git a/test/integration/standard_filter_test.rb b/test/integration/standard_filter_test.rb index 9de2106..c53aae1 100644 --- a/test/integration/standard_filter_test.rb +++ b/test/integration/standard_filter_test.rb @@ -208,6 +208,47 @@ class StandardFiltersTest < Minitest::Test assert_equal expectation, @filters.sort(input, "price") end + def test_sort_natural_when_property_is_sometimes_missing_puts_nils_last + input = [ + { "price" => "4", "handle" => "alpha" }, + { "handle" => "beta" }, + { "price" => "1", "handle" => "gamma" }, + { "handle" => "delta" }, + { "price" => 2, "handle" => "epsilon" } + ] + expectation = [ + { "price" => "1", "handle" => "gamma" }, + { "price" => 2, "handle" => "epsilon" }, + { "price" => "4", "handle" => "alpha" }, + { "handle" => "delta" }, + { "handle" => "beta" } + ] + assert_equal expectation, @filters.sort_natural(input, "price") + end + + def test_sort_natural_case_check + input = [ + { "key" => "X" }, + { "key" => "Y" }, + { "key" => "Z" }, + { "fake" => "t" }, + { "key" => "a" }, + { "key" => "b" }, + { "key" => "c" } + ] + expectation = [ + { "key" => "a" }, + { "key" => "b" }, + { "key" => "c" }, + { "key" => "X" }, + { "key" => "Y" }, + { "key" => "Z" }, + { "fake" => "t" } + ] + assert_equal expectation, @filters.sort_natural(input, "key") + assert_equal ["a", "b", "c", "X", "Y", "Z"], @filters.sort_natural(["X", "Y", "Z", "a", "b", "c"]) + end + def test_sort_empty_array assert_equal [], @filters.sort([], "a") end From deb10ebc7a8b1979b7c5daec83f2e69d9b68591d Mon Sep 17 00:00:00 2001 From: Eric Chan Date: Thu, 14 Sep 2017 02:00:43 -0400 Subject: [PATCH 3/3] Sorting support for data with undefined values --- lib/liquid/standardfilters.rb | 28 +++++++++++++++++------- test/integration/standard_filter_test.rb | 15 +++++++++++++ 2 files changed, 35 insertions(+), 8 deletions(-) diff --git a/lib/liquid/standardfilters.rb b/lib/liquid/standardfilters.rb index f14ba1d..34b4a49 100644 --- a/lib/liquid/standardfilters.rb +++ b/lib/liquid/standardfilters.rb @@ -121,17 +121,23 @@ module Liquid def sort(input, property = nil) ary = InputIterator.new(input) if property.nil? - ary.sort + ary.sort do |a, b| + if !a.nil? && !b.nil? + a <=> b + else + a.nil? ? 1 : -1 + end + end elsif ary.empty? # The next two cases assume a non-empty array. [] - elsif ary.first.respond_to?(:[]) && !ary.first[property].nil? + elsif ary.all? { |el| el.respond_to?(:[]) } ary.sort do |a, b| a = a[property] b = b[property] - if a && b + if !a.nil? && !b.nil? a <=> b else - a ? -1 : 1 + a.nil? ? 1 : -1 end end end @@ -143,17 +149,23 @@ module Liquid ary = InputIterator.new(input) if property.nil? - ary.sort { |a, b| a.to_s.casecmp(b.to_s) } + ary.sort do |a, b| + if !a.nil? && !b.nil? + a.to_s.casecmp(b.to_s) + else + a.nil? ? 1 : -1 + end + end elsif ary.empty? # The next two cases assume a non-empty array. [] - elsif ary.first.respond_to?(:[]) && !ary.first[property].nil? + elsif ary.all? { |el| el.respond_to?(:[]) } ary.sort do |a, b| a = a[property] b = b[property] - if a && b + if !a.nil? && !b.nil? a.to_s.casecmp(b.to_s) else - a ? -1 : 1 + a.nil? ? 1 : -1 end end end diff --git a/test/integration/standard_filter_test.rb b/test/integration/standard_filter_test.rb index c53aae1..6c91f16 100644 --- a/test/integration/standard_filter_test.rb +++ b/test/integration/standard_filter_test.rb @@ -190,6 +190,11 @@ class StandardFiltersTest < Minitest::Test assert_equal [{ "a" => 1 }, { "a" => 2 }, { "a" => 3 }, { "a" => 4 }], @filters.sort([{ "a" => 4 }, { "a" => 3 }, { "a" => 1 }, { "a" => 2 }], "a") end + def test_sort_with_nils + assert_equal [1, 2, 3, 4, nil], @filters.sort([nil, 4, 3, 2, 1]) + assert_equal [{ "a" => 1 }, { "a" => 2 }, { "a" => 3 }, { "a" => 4 }, {}], @filters.sort([{ "a" => 4 }, { "a" => 3 }, {}, { "a" => 1 }, { "a" => 2 }], "a") + end + def test_sort_when_property_is_sometimes_missing_puts_nils_last input = [ { "price" => 4, "handle" => "alpha" }, @@ -208,6 +213,16 @@ class StandardFiltersTest < Minitest::Test assert_equal expectation, @filters.sort(input, "price") end + def test_sort_natural + assert_equal ["a", "B", "c", "D"], @filters.sort_natural(["c", "D", "a", "B"]) + assert_equal [{ "a" => "a" }, { "a" => "B" }, { "a" => "c" }, { "a" => "D" }], @filters.sort_natural([{ "a" => "D" }, { "a" => "c" }, { "a" => "a" }, { "a" => "B" }], "a") + end + + def test_sort_natural_with_nils + assert_equal ["a", "B", "c", "D", nil], @filters.sort_natural([nil, "c", "D", "a", "B"]) + assert_equal [{ "a" => "a" }, { "a" => "B" }, { "a" => "c" }, { "a" => "D" }, {}], @filters.sort_natural([{ "a" => "D" }, { "a" => "c" }, {}, { "a" => "a" }, { "a" => "B" }], "a") + end + def test_sort_natural_when_property_is_sometimes_missing_puts_nils_last input = [ { "price" => "4", "handle" => "alpha" },