Merge pull request #26628 from mjhoy/fix-number-to-human-25742

round before calculating exponent in number_to_human_converter

5 files changed, 79 insertions, 30 deletions

diff --git a/activesupport/lib/active_support/number_helper.rb b/activesupport/lib/active_support/number_helper.rb
index 880340ca86..9cb2821cb6 100644
--- a/activesupport/lib/active_support/number_helper.rb
+++ b/activesupport/lib/active_support/number_helper.rb
@@ -4,6 +4,7 @@ module ActiveSupport
 
     eager_autoload do
       autoload :NumberConverter
+      autoload :RoundingHelper
       autoload :NumberToRoundedConverter
       autoload :NumberToDelimitedConverter
       autoload :NumberToHumanConverter
diff --git a/activesupport/lib/active_support/number_helper/number_to_human_converter.rb b/activesupport/lib/active_support/number_helper/number_to_human_converter.rb
index 56185ddf4b..040343b5dd 100644
--- a/activesupport/lib/active_support/number_helper/number_to_human_converter.rb
+++ b/activesupport/lib/active_support/number_helper/number_to_human_converter.rb
@@ -9,6 +9,7 @@ module ActiveSupport
       self.validate_float = true
 
       def convert # :nodoc:
+        @number = RoundingHelper.new(options).round(number)
         @number = Float(number)
 
         # for backwards compatibility with those that didn't add strip_insignificant_zeros to their locale files
@@ -20,10 +21,7 @@ module ActiveSupport
         exponent = calculate_exponent(units)
         @number = number / (10**exponent)
 
-        until (rounded_number = NumberToRoundedConverter.convert(number, options)) != NumberToRoundedConverter.convert(1000, options)
-          @number = number / 1000.0
-          exponent += 3
-        end
+        rounded_number = NumberToRoundedConverter.convert(number, options)
         unit = determine_unit(units, exponent)
         format.gsub("%n".freeze, rounded_number).gsub("%u".freeze, unit).strip
       end
diff --git a/activesupport/lib/active_support/number_helper/number_to_rounded_converter.rb b/activesupport/lib/active_support/number_helper/number_to_rounded_converter.rb
index 1f013990ea..c32d85a45f 100644
--- a/activesupport/lib/active_support/number_helper/number_to_rounded_converter.rb
+++ b/activesupport/lib/active_support/number_helper/number_to_rounded_converter.rb
@@ -5,26 +5,14 @@ module ActiveSupport
       self.validate_float = true
 
       def convert
-        precision = options.delete :precision
+        helper = RoundingHelper.new(options)
+        rounded_number = helper.round(number)
 
-        if precision
-          case number
-          when Float, String
-            @number = BigDecimal(number.to_s)
-          when Rational
-            @number = BigDecimal(number, digit_count(number.to_i) + precision)
-          else
-            @number = number.to_d
-          end
-
-          if options.delete(:significant) && precision > 0
-            digits, rounded_number = digits_and_rounded_number(precision)
+        if precision = options[:precision]
+          if options[:significant] && precision > 0
+            digits = helper.digit_count(rounded_number)
             precision -= digits
             precision = 0 if precision < 0 # don't let it be negative
-          else
-            rounded_number = number.round(precision)
-            rounded_number = rounded_number.to_i if precision == 0 && rounded_number.finite?
-            rounded_number = rounded_number.abs if rounded_number.zero? # prevent showing negative zeros
           end
 
           formatted_string =
@@ -38,7 +26,7 @@ module ActiveSupport
               "%00.#{precision}f" % rounded_number
             end
         else
-          formatted_string = number
+          formatted_string = rounded_number
         end
 
         delimited_number = NumberToDelimitedConverter.convert(formatted_string, options)
@@ -79,14 +67,6 @@ module ActiveSupport
             number
           end
         end
-
-        def absolute_number(number)
-          number.respond_to?(:abs) ? number.abs : number.to_d.abs
-        end
-
-        def zero?
-          number.respond_to?(:zero?) ? number.zero? : number.to_d.zero?
-        end
     end
   end
 end
diff --git a/activesupport/lib/active_support/number_helper/rounding_helper.rb b/activesupport/lib/active_support/number_helper/rounding_helper.rb
new file mode 100644
index 0000000000..d9644df17d
--- /dev/null
+++ b/activesupport/lib/active_support/number_helper/rounding_helper.rb
@@ -0,0 +1,64 @@
+module ActiveSupport
+  module NumberHelper
+    class RoundingHelper # :nodoc:
+      attr_reader :options
+
+      def initialize(options)
+        @options = options
+      end
+
+      def round(number)
+        return number unless precision
+        number = convert_to_decimal(number)
+        if significant && precision > 0
+          round_significant(number)
+        else
+          round_without_significant(number)
+        end
+      end
+
+      def digit_count(number)
+        return 1 if number.zero?
+        (Math.log10(absolute_number(number)) + 1).floor
+      end
+
+      private
+        def round_without_significant(number)
+          number = number.round(precision)
+          number = number.to_i if precision == 0 && number.finite?
+          number = number.abs if number.zero? # prevent showing negative zeros
+          number
+        end
+
+        def round_significant(number)
+          return 0 if number.zero?
+          digits = digit_count(number)
+          multiplier = 10**(digits - precision)
+          (number / BigDecimal.new(multiplier.to_f.to_s)).round * multiplier
+        end
+
+        def convert_to_decimal(number)
+          case number
+          when Float, String
+            number = BigDecimal(number.to_s)
+          when Rational
+            number = BigDecimal(number, digit_count(number.to_i) + precision)
+          else
+            number = number.to_d
+          end
+        end
+
+        def precision
+          options[:precision]
+        end
+
+        def significant
+          options[:significant]
+        end
+
+        def absolute_number(number)
+          number.respond_to?(:abs) ? number.abs : number.to_d.abs
+        end
+    end
+  end
+end
diff --git a/activesupport/test/number_helper_test.rb b/activesupport/test/number_helper_test.rb
index dc0c34d4e2..4caf1428ea 100644
--- a/activesupport/test/number_helper_test.rb
+++ b/activesupport/test/number_helper_test.rb
@@ -321,12 +321,18 @@ module ActiveSupport
           gangster = { hundred: "hundred bucks", million: "thousand quids" }
           assert_equal "1 hundred bucks", number_helper.number_to_human(100, units: gangster)
           assert_equal "25 hundred bucks", number_helper.number_to_human(2500, units: gangster)
+          assert_equal "1000 hundred bucks", number_helper.number_to_human(100_000, units: gangster)
+          assert_equal "1 thousand quids", number_helper.number_to_human(999_999, units: gangster)
+          assert_equal "1 thousand quids", number_helper.number_to_human(1_000_000, units: gangster)
           assert_equal "25 thousand quids", number_helper.number_to_human(25000000, units: gangster)
           assert_equal "12300 thousand quids", number_helper.number_to_human(12345000000, units: gangster)
 
           #Spaces are stripped from the resulting string
           assert_equal "4", number_helper.number_to_human(4, units: { unit: "", ten: "tens " })
           assert_equal "4.5  tens", number_helper.number_to_human(45, units: { unit: "", ten: " tens   " })
+
+          #Uses only the provided units and does not try to use larger ones
+          assert_equal "1000 kilometers", number_helper.number_to_human(1_000_000, units: { unit: "meter", thousand: "kilometers" })
         end
       end