active_data icon indicating copy to clipboard operation
active_data copied to clipboard

Fix checking if a error was already added for nested validator

Open shurikp opened this issue 2 years ago • 0 comments

Description

When

  • running validations several times (not sure why exactly, but that sometimes happens in Granite)
  • having :message set explicitly in the options

This check:

to.import(error, attribute: key) unless to.added?(key, error.type, error.options)

Doesn't do it's job well, since it gets to the strict matching in ActiveModel::Error:

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

and hence if we give :message in the options - it doesn't match it right.

This either seems a bug in ActiveModel or interface intended to be used without ever providing those keys in the options:

ActiveModel 6.1 and 7:

> errors = ActiveModel::Errors.new(nil)
> errors.add(:full_name, :weired, message: 'is weired')
> errors.added?(:full_name, :weired, message: 'is weired')
=> false
> errors.added?(:full_name, :weired)
=> true

ActiveModel 6.0 however behaves as one would expect:

> errors = ActiveModel::Errors.new(nil)
> errors.add(:full_name, :weired, message: 'is weired')
> errors.added?(:full_name, :weired, message: 'is weired')
=> true

How to test

I added a spec (and also explicitly set :message in the tested model), that fails if we don't omit those keys

Fixing ActiveModel?

Maybe I'll also make a PR in rails later, to my taste seems like a bug, if they merge it, this one won't be needed

shurikp avatar Sep 02 '22 07:09 shurikp