Merge pull request #13776 from rails/dirty-enum

Implement the Dirty API with the Enum feature correctly.

Conflicts:
	activerecord/CHANGELOG.md

activerecord/CHANGELOG.md

+*   Make enum fields work as expected with the `ActiveModel::Dirty` API.
+
+    Before this change, using the dirty API would have surprising results:
+
+        conversation = Conversation.new
+        conversation.status = :active
+        conversation.status = :archived
+        conversation.status_was # => 0
+
+    After this change, the same code would result in:
+
+        conversation = Conversation.new
+        conversation.status = :active
+        conversation.status = :archived
+        conversation.status_was # => "active"
+
+    *Rafael Mendonça França*
+
 *   `has_one` and `belongs_to` accessors don't add ORDER BY to the queries anymore.
 
     Since Rails 4.0, we add an ORDER BY in the `first` method to ensure consistent results

activerecord/lib/active_record/attribute_methods/dirty.rb

@@ -43,6 +43,12 @@ def reload(*)
       def write_attribute(attr, value)
         attr = attr.to_s
 
+        save_changed_attribute(attr, value)
+
+        super(attr, value)
+      end
+
+      def save_changed_attribute(attr, value)
         # The attribute already has an unsaved change.
         if attribute_changed?(attr)
           old = changed_attributes[attr]
@@ -51,9 +57,6 @@ def write_attribute(attr, value)
           old = clone_attribute_value(:read_attribute, attr)
           changed_attributes[attr] = old if _field_changed?(attr, old, value)
         end
-
-        # Carry on.
-        super(attr, value)
       end
 
       def update_record(*)

activerecord/lib/active_record/enum.rb

@@ -63,6 +63,12 @@ module ActiveRecord
   #
   #   Conversation.where("status <> ?", Conversation.statuses[:archived])
   module Enum
+    DEFINED_ENUMS = {} # :nodoc:
+
+    def enum_mapping_for(attr_name) # :nodoc:
+      DEFINED_ENUMS[attr_name.to_s]
+    end
+
     def enum(definitions)
       klass = self
       definitions.each do |name, values|
@@ -107,6 +113,8 @@ def enum(definitions)
             # def active!() update! status: :active end
             define_method("#{value}!") { update! name => value }
           end
+
+          DEFINED_ENUMS[name.to_s] = enum_values
         end
       end
     end
@@ -114,7 +122,28 @@ def enum(definitions)
     private
       def _enum_methods_module
         @_enum_methods_module ||= begin
-          mod = Module.new
+          mod = Module.new do
+            private
+              def save_changed_attribute(attr_name, value)
+                if (mapping = self.class.enum_mapping_for(attr_name))
+                  if attribute_changed?(attr_name)
+                    old = changed_attributes[attr_name]
+
+                    if mapping[old] == value
+                      changed_attributes.delete(attr_name)
+                    end
+                  else
+                    old = clone_attribute_value(:read_attribute, attr_name)
+
+                    if old != value
+                      changed_attributes[attr_name] = mapping.key old
+                    end
+                  end
+                else
+                  super
+                end
+              end
+          end
           include mod
           mod
         end

activerecord/test/cases/enum_test.rb

@@ -51,6 +51,78 @@ class EnumTest < ActiveRecord::TestCase
     assert @book.written?
   end
 
+  test "enum changed attributes" do
+    old_status = @book.status
+    @book.status = :published
+    assert_equal old_status, @book.changed_attributes[:status]
+  end
+
+  test "enum changes" do
+    old_status = @book.status
+    @book.status = :published
+    assert_equal [old_status, 'published'], @book.changes[:status]
+  end
+
+  test "enum attribute was" do
+    old_status = @book.status
+    @book.status = :published
+    assert_equal old_status, @book.attribute_was(:status)
+  end
+
+  test "enum attribute changed" do
+    @book.status = :published
+    assert @book.attribute_changed?(:status)
+  end
+
+  test "enum attribute changed to" do
+    @book.status = :published
+    assert @book.attribute_changed?(:status, to: 'published')
+  end
+
+  test "enum attribute changed from" do
+    old_status = @book.status
+    @book.status = :published
+    assert @book.attribute_changed?(:status, from: old_status)
+  end
+
+  test "enum attribute changed from old status to new status" do
+    old_status = @book.status
+    @book.status = :published
+    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

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

activerecord/test/schema/schema.rb

@@ -97,6 +97,7 @@ def create_table(*args, &block)
     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|