Add more tests for the dirty feature for enums

5 files changed, 53 insertions, 8 deletions

diff --git a/activerecord/lib/active_record/attribute_methods/dirty.rb b/activerecord/lib/active_record/attribute_methods/dirty.rb
index f7065d183f..68168bb729 100644
--- a/activerecord/lib/active_record/attribute_methods/dirty.rb
+++ b/activerecord/lib/active_record/attribute_methods/dirty.rb
@@ -45,7 +45,6 @@ module ActiveRecord
 
         save_changed_attribute(attr, value)
 
-        # Carry on.
         super(attr, value)
       end
 
diff --git a/activerecord/lib/active_record/enum.rb b/activerecord/lib/active_record/enum.rb
index 06e87cf854..0990a4a871 100644
--- a/activerecord/lib/active_record/enum.rb
+++ b/activerecord/lib/active_record/enum.rb
@@ -123,14 +123,26 @@ module ActiveRecord
       def _enum_methods_module
         @_enum_methods_module ||= begin
           mod = Module.new do
-            def save_changed_attribute(attr_name, value)
-              if self.class.enum_attribute?(attr_name)
-                old = clone_attribute_value(:read_attribute, attr_name)
-                changed_attributes[attr_name] = self.class.public_send(attr_name.pluralize).key old
-              else
-                super
+            private
+              def save_changed_attribute(attr_name, value)
+                if self.class.enum_attribute?(attr_name)
+                  if attribute_changed?(attr_name)
+                    old = changed_attributes[attr_name]
+
+                    if self.class.public_send(attr_name.pluralize)[old] == value
+                      changed_attributes.delete(attr_name)
+                    end
+                  else
+                    old = clone_attribute_value(:read_attribute, attr_name)
+
+                    if old != value
+                      changed_attributes[attr_name] = self.class.public_send(attr_name.pluralize).key old
+                    end
+                  end
+                else
+                  super
+                end
               end
-            end
           end
           include mod
           mod
diff --git a/activerecord/test/cases/enum_test.rb b/activerecord/test/cases/enum_test.rb
index 0fe7dfe4ea..9fbfebcca2 100644
--- a/activerecord/test/cases/enum_test.rb
+++ b/activerecord/test/cases/enum_test.rb
@@ -91,6 +91,38 @@ class EnumTest < ActiveRecord::TestCase
     assert @book.attribute_changed?(:status, from: old_status, to: 'published')
   end
 
+  test "enum didn't change" do
+    old_status = @book.status
+    @book.status = old_status
+    assert_not @book.attribute_changed?(:status)
+  end
+
+  test "persist changes that are dirty" do
+    old_status = @book.status
+    @book.status = :published
+    assert @book.attribute_changed?(:status)
+    @book.status = :written
+    assert @book.attribute_changed?(:status)
+  end
+
+  test "reverted changes that are not dirty" do
+    old_status = @book.status
+    @book.status = :published
+    assert @book.attribute_changed?(:status)
+    @book.status = old_status
+    assert_not @book.attribute_changed?(:status)
+  end
+
+  test "reverted changes are not dirty going from nil to value and back" do
+    book = Book.create!(nullable_status: nil)
+
+    book.nullable_status = :married
+    assert book.attribute_changed?(:nullable_status)
+
+    book.nullable_status = nil
+    assert_not book.attribute_changed?(:nullable_status)
+  end
+
   test "assign non existing value raises an error" do
     e = assert_raises(ArgumentError) do
       @book.status = :unknown
diff --git a/activerecord/test/models/book.rb b/activerecord/test/models/book.rb
index 4cb2c7606b..2170018068 100644
--- a/activerecord/test/models/book.rb
+++ b/activerecord/test/models/book.rb
@@ -9,6 +9,7 @@ class Book < ActiveRecord::Base
 
   enum status: [:proposed, :written, :published]
   enum read_status: {unread: 0, reading: 2, read: 3}
+  enum nullable_status: [:single, :married]
 
   def published!
     super
diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb
index 9a7d918a25..99a53434f6 100644
--- a/activerecord/test/schema/schema.rb
+++ b/activerecord/test/schema/schema.rb
@@ -97,6 +97,7 @@ ActiveRecord::Schema.define do
     t.column :name, :string
     t.column :status, :integer, default: 0
     t.column :read_status, :integer, default: 0
+    t.column :nullable_status, :integer
   end
 
   create_table :booleans, force: true do |t|