Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Merge pull request #13776 from rails/dirty-enum

Implement the Dirty API with the Enum feature correctly.

Conflicts:
	activerecord/CHANGELOG.md
  • Loading branch information...
commit 9383de42a2cf23cc53052cec2f736864c1c562a1 2 parents c6ecfc1 + 55f6c8c
@rafaelfranca rafaelfranca authored
View
18 activerecord/CHANGELOG.md
@@ -1,3 +1,21 @@
+* 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
View
9 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(*)
View
31 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
View
72 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
View
1  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
View
1  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|
Please sign in to comment.
Something went wrong with that request. Please try again.