From 2fece9036d8ea778fca63a0d5df9a9dfc330dfa5 Mon Sep 17 00:00:00 2001
From: Ryuta Kamizono <kamipo@gmail.com>
Date: Fri, 10 Aug 2018 19:42:00 +0900
Subject: Fix numericality validator not to be affected by custom getter

Since fe9547b6, numericality validator would parse raw value only when a
value came from user to work type casting to a value from database.

But that was caused a regression that the validator would work against
getter value instead of parsed raw value, a getter is sometimes
customized by people. #33550

There we never guarantees that the value before type cast was going to
the used in this validation (actually here is only place that getter
value might not be used), but we should not change the behavior unless
there is some particular reason.

The purpose of fe9547b6 is to work type casting to a value from
database. We could achieve the purpose by using `read_attribute`,
without using getter value.

Fixes #33550.
---
 .../has_and_belongs_to_many_associations_test.rb       |  2 +-
 activerecord/test/cases/validations_test.rb            | 18 +++++++++++++++++-
 activerecord/test/models/price_estimate.rb             |  8 ++++++++
 3 files changed, 26 insertions(+), 2 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb
index f414fbf64b..482302055d 100644
--- a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb
@@ -781,7 +781,7 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase
   end
 
   def test_has_many_through_polymorphic_has_manys_works
-    assert_equal [10, 20].to_set, pirates(:redbeard).treasure_estimates.map(&:price).to_set
+    assert_equal ["$10.00", "$20.00"].to_set, pirates(:redbeard).treasure_estimates.map(&:price).to_set
   end
 
   def test_symbols_as_keys
diff --git a/activerecord/test/cases/validations_test.rb b/activerecord/test/cases/validations_test.rb
index a33877f43a..66763c727f 100644
--- a/activerecord/test/cases/validations_test.rb
+++ b/activerecord/test/cases/validations_test.rb
@@ -3,11 +3,11 @@
 require "cases/helper"
 require "models/topic"
 require "models/reply"
-require "models/person"
 require "models/developer"
 require "models/computer"
 require "models/parrot"
 require "models/company"
+require "models/price_estimate"
 
 class ValidationsTest < ActiveRecord::TestCase
   fixtures :topics, :developers
@@ -183,6 +183,22 @@ class ValidationsTest < ActiveRecord::TestCase
     assert_not_predicate klass.new(wibble: BigDecimal("97.179")), :valid?
   end
 
+  def test_numericality_validator_wont_be_affected_by_custom_getter
+    price_estimate = PriceEstimate.new(price: 50)
+
+    assert_equal "$50.00", price_estimate.price
+    assert_equal 50, price_estimate.price_before_type_cast
+    assert_equal 50, price_estimate.read_attribute(:price)
+
+    assert_predicate price_estimate, :price_came_from_user?
+    assert_predicate price_estimate, :valid?
+
+    price_estimate.save!
+
+    assert_not_predicate price_estimate, :price_came_from_user?
+    assert_predicate price_estimate, :valid?
+  end
+
   def test_acceptance_validator_doesnt_require_db_connection
     klass = Class.new(ActiveRecord::Base) do
       self.table_name = "posts"
diff --git a/activerecord/test/models/price_estimate.rb b/activerecord/test/models/price_estimate.rb
index f1f88d8d8d..669d0991f7 100644
--- a/activerecord/test/models/price_estimate.rb
+++ b/activerecord/test/models/price_estimate.rb
@@ -1,6 +1,14 @@
 # frozen_string_literal: true
 
 class PriceEstimate < ActiveRecord::Base
+  include ActiveSupport::NumberHelper
+
   belongs_to :estimate_of, polymorphic: true
   belongs_to :thing, polymorphic: true
+
+  validates_numericality_of :price
+
+  def price
+    number_to_currency super
+  end
 end
-- 
cgit v1.2.3