makandra / active_type

Make any Ruby object quack like ActiveRecord
MIT License
1.09k stars 74 forks source link

NestedAttributes::Association#add_errors_to_parent version check #159

Closed atcruice closed 2 years ago

atcruice commented 2 years ago

I believe the intention of https://github.com/makandra/active_type/blob/v2.1.2/lib/active_type/nested_attributes/association.rb#L133 was to address an issue identified in Rails 6.1, https://github.com/makandra/active_type/pull/135 (i.e.: apply this pathway for any version of ActiveRecord >= 6.1).

However, the nature of the &&ed expressions means Rails v7.0.3 fails this condition and executes the else branch which is causing some unusually attributed nested errors.

ActiveRecord::VERSION::STRING # => "7.0.3"
ActiveRecord::VERSION::MAJOR >= 6 && ActiveRecord::VERSION::MINOR >= 1
# 7 >= 6 && 0 >= 1
# true && false
# false

I understand Gem::Version would provide the correct comparison here.

ActiveRecord::VERSION::STRING # => "7.0.3"
Gem::Version.new(ActiveRecord::VERSION::STRING) >= Gem::Version.new("6.1") # => true

Happy to open a pull request if my understanding regarding the version check is correct.

kratob commented 2 years ago

You are correct, that check is not sane and it should be fixed exactly as you describe it. Would be happy to get a pull request.