From c51f9b61ce1e167f5f58f07441adcfa117694301 Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Wed, 11 Feb 2015 14:56:26 -0700 Subject: Refactor enum to be defined in terms of the attributes API In addition to cleaning up the implementation, this allows type casting behavior to be applied consistently everywhere. (#where for example). A good example of this was the previous need for handling value to key conversion in the setter, because the number had to be passed to `where` directly. This is no longer required, since we can just pass the string along to where. (It's left around for backwards compat) Fixes #18387 --- activerecord/CHANGELOG.md | 5 +++ activerecord/lib/active_record/enum.rb | 81 ++++++++++++++++------------------ activerecord/test/cases/enum_test.rb | 17 ++++++- 3 files changed, 59 insertions(+), 44 deletions(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 0265ab0c11..2ee3c90f89 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,8 @@ +* Have `enum` perform type casting consistently with the rest of Active + Record, such as `where`. + + *Sean Griffin* + * `scoping` no longer pollutes the current scope of sibling classes when using STI. e.x. diff --git a/activerecord/lib/active_record/enum.rb b/activerecord/lib/active_record/enum.rb index f053372cfb..b70d52aa87 100644 --- a/activerecord/lib/active_record/enum.rb +++ b/activerecord/lib/active_record/enum.rb @@ -79,6 +79,37 @@ module ActiveRecord super end + class EnumType < Type::Value + def initialize(name, mapping) + @name = name + @mapping = mapping + end + + def type_cast_from_user(value) + return if value.blank? + + if mapping.has_key?(value) + value.to_s + elsif mapping.has_value?(value) + mapping.key(value) + else + raise ArgumentError, "'#{value}' is not a valid #{name}" + end + end + + def type_cast_from_database(value) + mapping.key(value) + end + + def type_cast_for_database(value) + mapping.fetch(value, value) + end + + protected + + attr_reader :name, :mapping + end + def enum(definitions) klass = self definitions.each do |name, values| @@ -90,37 +121,19 @@ module ActiveRecord detect_enum_conflict!(name, name.to_s.pluralize, true) klass.singleton_class.send(:define_method, name.to_s.pluralize) { enum_values } - _enum_methods_module.module_eval do - # def status=(value) self[:status] = statuses[value] end - klass.send(:detect_enum_conflict!, name, "#{name}=") - define_method("#{name}=") { |value| - if enum_values.has_key?(value) || value.blank? - self[name] = enum_values[value] - elsif enum_values.has_value?(value) - # Assigning a value directly is not a end-user feature, hence it's not documented. - # This is used internally to make building objects from the generated scopes work - # as expected, i.e. +Conversation.archived.build.archived?+ should be true. - self[name] = value - else - raise ArgumentError, "'#{value}' is not a valid #{name}" - end - } - - # def status() statuses.key self[:status] end - klass.send(:detect_enum_conflict!, name, name) - define_method(name) { enum_values.key self[name] } + detect_enum_conflict!(name, name) + detect_enum_conflict!(name, "#{name}=") - # def status_before_type_cast() statuses.key self[:status] end - klass.send(:detect_enum_conflict!, name, "#{name}_before_type_cast") - define_method("#{name}_before_type_cast") { enum_values.key self[name] } + attribute name, EnumType.new(name, enum_values) + _enum_methods_module.module_eval do pairs = values.respond_to?(:each_pair) ? values.each_pair : values.each_with_index pairs.each do |value, i| enum_values[value] = i # def active?() status == 0 end klass.send(:detect_enum_conflict!, name, "#{value}?") - define_method("#{value}?") { self[name] == i } + define_method("#{value}?") { self[name] == value.to_s } # def active!() update! status: :active end klass.send(:detect_enum_conflict!, name, "#{value}!") @@ -128,7 +141,7 @@ module ActiveRecord # scope :active, -> { where status: 0 } klass.send(:detect_enum_conflict!, name, value, true) - klass.scope value, -> { klass.where name => i } + klass.scope value, -> { klass.where name => value } end end defined_enums[name.to_s] = enum_values @@ -138,25 +151,7 @@ module ActiveRecord private def _enum_methods_module @_enum_methods_module ||= begin - mod = Module.new do - private - def save_changed_attribute(attr_name, old) - if (mapping = self.class.defined_enums[attr_name.to_s]) - value = _read_attribute(attr_name) - if attribute_changed?(attr_name) - if mapping[old] == value - clear_attribute_changes([attr_name]) - end - else - if old != value - set_attribute_was(attr_name, mapping.key(old)) - end - end - else - super - end - end - end + mod = Module.new include mod mod end diff --git a/activerecord/test/cases/enum_test.rb b/activerecord/test/cases/enum_test.rb index 346fcab6ea..ed568413a2 100644 --- a/activerecord/test/cases/enum_test.rb +++ b/activerecord/test/cases/enum_test.rb @@ -26,6 +26,17 @@ class EnumTest < ActiveRecord::TestCase assert_equal @book, Book.unread.first end + test "build from scope" do + assert Book.proposed.build.proposed? + refute Book.proposed.build.written? + assert Book.where(status: Book.statuses[:proposed]).build.proposed? + end + + test "find via where" do + assert_equal @book, Book.where(status: "proposed").first + refute_equal @book, Book.where(status: "written").first + end + test "update by declaration" do @book.written! assert @book.written? @@ -161,7 +172,11 @@ class EnumTest < ActiveRecord::TestCase end test "_before_type_cast returns the enum label (required for form fields)" do - assert_equal "proposed", @book.status_before_type_cast + if @book.status_came_from_user? + assert_equal "proposed", @book.status_before_type_cast + else + assert_equal "proposed", @book.status + end end test "reserved enum names" do -- cgit v1.2.3