Skip to content

ActiveModel::Errors#added? doesn't work when certain options were present during validation #54483

Open
@NobodysNightmare

Description

@NobodysNightmare

Steps to reproduce

# frozen_string_literal: true

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  gem "rails"

  gem "sqlite3"
end

require "active_record/railtie"
require "minitest/autorun"

# This connection will do for database-independent bug reports.
ENV["DATABASE_URL"] = "sqlite3::memory:"

class TestApp < Rails::Application
  config.load_defaults Rails::VERSION::STRING.to_f
  config.eager_load = false
  config.logger = Logger.new($stdout)
  config.secret_key_base = "secret_key_base"

  config.active_record.encryption.primary_key = "primary_key"
  config.active_record.encryption.deterministic_key = "deterministic_key"
  config.active_record.encryption.key_derivation_salt = "key_derivation_salt"
end
Rails.application.initialize!

ActiveRecord::Schema.define do
  create_table :posts, force: true do |t|
  end
end

class Post < ActiveRecord::Base
end

class BugTest < ActiveSupport::TestCase
  # This fails and describes my high-level problem
  def test_finds_error_with_callback_options
    post = Post.new

    # this is very similar to what a validate call might add
    error = post.errors.add(:base, :too_short, allow_nil: false, count: 42)

    assert_equal true, post.errors.added?(error.attribute, error.type, error.options)
  end

  # Succeeds because no allow_nil was passed
  def test_finds_error_without_callback_options
    post = Post.new

    error = post.errors.add(:base, :too_short, count: 42)

    assert_equal true, post.errors.added?(error.attribute, error.type, error.options)
  end

  # This is the culprit for my higher level problem
  def test_error_with_callback_options_strictly_matches_itself
    post = Post.new

    error = post.errors.add(:base, :too_short, allow_nil: false, count: 42)
    assert_equal true, error.strict_match?(error.attribute, error.type, **error.options)
  end

  # Success case for culprit, because of missing allow_nil
  def test_error_without_callback_options_strictly_matches_itself
    post = Post.new

    error = post.errors.add(:base, :too_short, count: 42)
    assert_equal true, error.strict_match?(error.attribute, error.type, **error.options)
  end
end

Expected behavior

Checking for presence of an ActiveModel::Error using added?(e.attribute, e.type, e.options) should always work, because I literally pass in the interface that was used to create the error.

Actual behavior

It doesn't find the error in some cases, because some options are treated special by ActiveModel::Error#strict_match?, which is implemented like

options == @options.except(*CALLBACKS_OPTIONS + MESSAGE_OPTIONS)

This means I can create an Error with options that I will later not be able to validate during strict_match?, even when comparing the error to itself.

One possible fix that comes to my mind would be:

options.except(*CALLBACKS_OPTIONS + MESSAGE_OPTIONS) == @options.except(*CALLBACKS_OPTIONS + MESSAGE_OPTIONS)

Though written that way it doesn't look too nice.

Additional notes

I used allow_nil for my examples, but other validation options such as if or unless are affected the same way.

System configuration

Rails version: 8.0 (but also at least 7.1)

Ruby version: 3.3.6 (unrelated)

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions