search

makandra / active_type

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

Attribute is not a number #193

Open denzelem opened 1 month ago

denzelem commented 1 month ago

I noticed that the validate_numericality_of matcher from shoulda_matcher does not work with an instance of an ActiveType record.

The reason for this is a add_disallow_non_numeric_value_matcher check in the shoulda_matcher gem.

Do you think there are chances to align the behavior in ActiveType to an ActiveRecord record?

Example:

Screencast from 21.10.2024 14:51:16.webm

Source: https://github.com/denzelem/active_type_issue_20241021

kratob commented 2 weeks ago

Hi @denzelem, I was not immediately able to reproduce the issue. I've added a test, see https://github.com/makandra/active_type/pull/194/files#diff-5184c6c38fa94ccc8652f3f4d561a673c85bb1b5fabe4ca89eb7e3ec83d489b7R1-R38 . However, this does not fail.

Could you possibly add a failing test case, then I could have another look?

denzelem commented 2 weeks ago

@kratob Thanks for the setup. I've added a failing test case in 16fd9e845138ca18bbd1ae929976281ef54dfe1e to your feature branch. Hope I didn't miss the point, but it seems to be a problem when using virtual Integer Attributes. It also looks like an unexpected behavior in ActiveModel, I added this test case, too.

kratob commented 1 week ago

I've dug a bit for the underlying issue, and I'm not sure we're going to fix this. I'll align this with the team, but I think this will open a whole can of worms.

So this has nothing to do with shoulda-matchers; it's simply that the validates_numericality matcher does actually not work as you might expect. The validator will never see the non-numerical value that was assigned to the attribute, because we cast to an integer and you will never get a not_a_number error.

This does work for "real" attributes because of this: https://github.com/rails/rails/blob/9a9222874ade2d6f300647897a8ad99e1f05aa32/activemodel/lib/active_model/validations/numericality.rb#L125C1-L139C1

We could try to implement ..._came_from_user? for ActiveType, but it feels increasingly difficult to mimick the ever-changing subtleties across ActiveRecord versions. I think the whole mechanism we use for our virtual attributes is not fit for this.