diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 636c72a8e4582..f73d2ab366257 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,9 @@ +* Only warn about negative enums if a positive form that would cause conflicts exists. + + Fixes #39065. + + *Alex Ghiculescu* + * Allow the inverse of a `has_one` association that was previously autosaved to be loaded. Fixes #34255. diff --git a/activerecord/lib/active_record/enum.rb b/activerecord/lib/active_record/enum.rb index fc49f752aa4c2..5ef8bad342103 100644 --- a/activerecord/lib/active_record/enum.rb +++ b/activerecord/lib/active_record/enum.rb @@ -173,6 +173,7 @@ def enum(definitions) _enum_methods_module.module_eval do pairs = values.respond_to?(:each_pair) ? values.each_pair : values.each_with_index + value_method_names = [] pairs.each do |label, value| if enum_prefix == true prefix = "#{name}_" @@ -186,6 +187,7 @@ def enum(definitions) end value_method_name = "#{prefix}#{label}#{suffix}" + value_method_names << value_method_name enum_values[label] = value label = label.to_s @@ -200,8 +202,6 @@ def enum(definitions) # scope :active, -> { where(status: 0) } # scope :not_active, -> { where.not(status: 0) } if enum_scopes != false - klass.send(:detect_negative_condition!, value_method_name) - klass.send(:detect_enum_conflict!, name, value_method_name, true) klass.scope value_method_name, -> { where(attr => value) } @@ -209,6 +209,7 @@ def enum(definitions) klass.scope "not_#{value_method_name}", -> { where.not(attr => value) } end end + klass.send(:detect_negative_enum_conditions!, value_method_names) if enum_scopes != false end enum_values.freeze end @@ -264,10 +265,16 @@ def raise_conflict_error(enum_name, method_name, type: "instance", source: "Acti } end - def detect_negative_condition!(method_name) - if method_name.start_with?("not_") && logger - logger.warn "An enum element in #{self.name} uses the prefix 'not_'." \ - " This will cause a conflict with auto generated negative scopes." + def detect_negative_enum_conditions!(method_names) + return unless logger + + method_names.select { |m| m.start_with?("not_") }.each do |potential_not| + inverted_form = potential_not.sub("not_", "") + if method_names.include?(inverted_form) + logger.warn "Enum element '#{potential_not}' in #{self.name} uses the prefix 'not_'." \ + " This has caused a conflict with auto generated negative scopes." \ + " Avoid using enum elements starting with 'not' where the positive form is also an element." + end end end end diff --git a/activerecord/test/cases/enum_test.rb b/activerecord/test/cases/enum_test.rb index 0ae156320a5df..1b59f5d8bc65f 100644 --- a/activerecord/test/cases/enum_test.rb +++ b/activerecord/test/cases/enum_test.rb @@ -567,14 +567,18 @@ def self.name; "Book"; end assert_raises(NoMethodError) { klass.proposed } end - test "enums with a negative condition log a warning" do + test "enum logs a warning if auto-generated negative scopes would clash with other enum names" do old_logger = ActiveRecord::Base.logger logger = ActiveSupport::LogSubscriber::TestHelper::MockLogger.new ActiveRecord::Base.logger = logger - expected_message = "An enum element in Book uses the prefix 'not_'."\ - " This will cause a conflict with auto generated negative scopes." + expected_message_1 = "Enum element 'not_sent' in Book uses the prefix 'not_'."\ + " This has caused a conflict with auto generated negative scopes."\ + " Avoid using enum elements starting with 'not' where the positive form is also an element." + + # this message comes from ActiveRecord::Scoping::Named, but it's worth noting that both occur in this case + expected_message_2 = "Creating scope :not_sent. Overwriting existing method Book.not_sent." Class.new(ActiveRecord::Base) do def self.name @@ -585,7 +589,76 @@ def self.name end end - assert_match(expected_message, logger.logged(:warn).first) + assert_includes(logger.logged(:warn), expected_message_1) + assert_includes(logger.logged(:warn), expected_message_2) + ensure + ActiveRecord::Base.logger = old_logger + end + + test "enum logs a warning if auto-generated negative scopes would clash with other enum names regardless of order" do + old_logger = ActiveRecord::Base.logger + logger = ActiveSupport::LogSubscriber::TestHelper::MockLogger.new + + ActiveRecord::Base.logger = logger + + expected_message_1 = "Enum element 'not_sent' in Book uses the prefix 'not_'."\ + " This has caused a conflict with auto generated negative scopes."\ + " Avoid using enum elements starting with 'not' where the positive form is also an element." + + # this message comes from ActiveRecord::Scoping::Named, but it's worth noting that both occur in this case + expected_message_2 = "Creating scope :not_sent. Overwriting existing method Book.not_sent." + + Class.new(ActiveRecord::Base) do + def self.name + "Book" + end + silence_warnings do + enum status: [:not_sent, :sent] + end + end + + assert_includes(logger.logged(:warn), expected_message_1) + assert_includes(logger.logged(:warn), expected_message_2) + ensure + ActiveRecord::Base.logger = old_logger + end + + test "enum doesn't log a warning if no clashes detected" do + old_logger = ActiveRecord::Base.logger + logger = ActiveSupport::LogSubscriber::TestHelper::MockLogger.new + + ActiveRecord::Base.logger = logger + + Class.new(ActiveRecord::Base) do + def self.name + "Book" + end + silence_warnings do + enum status: [:not_sent] + end + end + + assert_empty(logger.logged(:warn)) + ensure + ActiveRecord::Base.logger = old_logger + end + + test "enum doesn't log a warning if opting out of scopes" do + old_logger = ActiveRecord::Base.logger + logger = ActiveSupport::LogSubscriber::TestHelper::MockLogger.new + + ActiveRecord::Base.logger = logger + + Class.new(ActiveRecord::Base) do + def self.name + "Book" + end + silence_warnings do + enum status: [:not_sent, :sent], _scopes: false + end + end + + assert_empty(logger.logged(:warn)) ensure ActiveRecord::Base.logger = old_logger end