From f4fbdb1b4ed47a45f8c1c63756af0b6f6873196b Mon Sep 17 00:00:00 2001
From: Yoshiyuki Hirano <yhirano@me.com>
Date: Mon, 23 Dec 2019 21:11:07 -1000
Subject: [PATCH] Allow AR::Enum definitions with boolean values

If `AR::Enum` is used for boolean field, it would be not expected
behavior for us.

fixes #38075

Problem:

In case of using boolean for enum, we can set with string (hash key)
to instance, but we cannot set with actual value (hash value).

```ruby
class Post < ActiveRecord::Base
  enum status: { enabled: true, disabled: false }
end

post.status = 'enabled'
post.status # 'enabled'

post.status = true
post.status # 'enabled'

post.status = 'disabled'
post.status # 'disabled'

post.status = false
post.status # nil (This is not expected behavior)
```

After looking into `AR::Enum::EnumType#cast`, I found that `blank?`
method converts from false value to nil (it seems it may not intentional behavior).

In this patch, I improved that if it defines enum with boolean,
it returns reasonable behavior.
---
 activerecord/lib/active_record/enum.rb |  4 ++--
 activerecord/test/cases/enum_test.rb   | 10 ++++++++++
 activerecord/test/fixtures/books.yml   |  1 +
 activerecord/test/models/book.rb       |  1 +
 activerecord/test/schema/schema.rb     |  1 +
 5 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/activerecord/lib/active_record/enum.rb b/activerecord/lib/active_record/enum.rb
index 7fa9f43156..9e404dfedd 100644
--- a/activerecord/lib/active_record/enum.rb
+++ b/activerecord/lib/active_record/enum.rb
@@ -123,12 +123,12 @@ def initialize(name, mapping, subtype)
       end
 
       def cast(value)
-        return if value.blank?
-
         if mapping.has_key?(value)
           value.to_s
         elsif mapping.has_value?(value)
           mapping.key(value)
+        elsif value.blank?
+          nil
         else
           assert_valid_value(value)
         end
diff --git a/activerecord/test/cases/enum_test.rb b/activerecord/test/cases/enum_test.rb
index 0ae156320a..490038428c 100644
--- a/activerecord/test/cases/enum_test.rb
+++ b/activerecord/test/cases/enum_test.rb
@@ -236,6 +236,16 @@ class EnumTest < ActiveRecord::TestCase
     assert_nil @book.status
   end
 
+  test "assign false value to a field defined as not boolean" do
+    @book.status = false
+    assert_nil @book.status
+  end
+
+  test "assign false value to a field defined as boolean" do
+    @book.boolean_status = false
+    assert_equal "disabled", @book.boolean_status
+  end
+
   test "assign long empty string value" do
     @book.status = "   "
     assert_nil @book.status
diff --git a/activerecord/test/fixtures/books.yml b/activerecord/test/fixtures/books.yml
index 699623a6f9..ed4d673fe6 100644
--- a/activerecord/test/fixtures/books.yml
+++ b/activerecord/test/fixtures/books.yml
@@ -10,6 +10,7 @@ awdr:
   illustrator_visibility: :visible
   font_size: :medium
   difficulty: :medium
+  boolean_status: :enabled
 
 rfr:
   author_id: 1
diff --git a/activerecord/test/models/book.rb b/activerecord/test/models/book.rb
index 43b82e6047..de51267093 100644
--- a/activerecord/test/models/book.rb
+++ b/activerecord/test/models/book.rb
@@ -18,6 +18,7 @@ class Book < ActiveRecord::Base
   enum font_size: [:small, :medium, :large], _prefix: :with, _suffix: true
   enum difficulty: [:easy, :medium, :hard], _suffix: :to_read
   enum cover: { hard: "hard", soft: "soft" }
+  enum boolean_status: { enabled: true, disabled: false }
 
   def published!
     super
diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb
index 8a9ccc6b71..f2f95f3b2c 100644
--- a/activerecord/test/schema/schema.rb
+++ b/activerecord/test/schema/schema.rb
@@ -117,6 +117,7 @@
     t.column :cover, :string, default: "hard"
     t.string :isbn, **case_sensitive_options
     t.datetime :published_on
+    t.boolean :boolean_status
     t.index [:author_id, :name], unique: true
     t.index :isbn, where: "published_on IS NOT NULL", unique: true
   end