Fix `read_timeliness_attribute_before_type_cast`

[TastyPi]

@attributes[attr_name] returns an ActiveModel::Attribute not a value, this fixes that.

I'm not familiar enough with how the library works to write a test that hits this code path, all I know is in our codebase validation with format doesn't work without this change. All the current tests pass before and after this change, so I don't think any of the existing tests are hitting this code path.

[TastyPi]

@adzap could this fix be merged please?

[tagliala]

Is it possible to have a reproducible test case, so we can write a failing spec?

[TastyPi]

@tagliala I've added a spec that works in my branch but fails on master. I couldn't use the standard TestModel defined in this repo because its custom implementation of attributes= and method_missing made it impossible to hit that code path, so I created a model specifically for the spec that mirrors the model I've been using.

While creating this I did discover that to get my model working, I needed to add include ValidatesTimeliness::AttributeMethods and define_attribute_methods :my_date_attribute, but frankly if it can be made to work with just adding the ValidatesTimeliness::ORM::ActiveModel that seems preferable.

Also, it would be really nice to have documentation on how to use this library with an ActiveModel that is not an ActiveRecord, I could have used it "correctly" in the first place (though I still think only having to include one concern is way nicer).

[adzap]

Thanks for getting a spec together @TastyPi . You are right this was untested and an undocumented and it needs both.

There is an existing ActiveModel spec file completely empty here https://github.com/adzap/validates_timeliness/blob/master/spec/validates_timeliness/orm/active_model_spec.rb

Would you mind adapting your spec to work in there. Your spec seems to be focussed on the format option for some reason. Perhaps use the ActiveRecord ORM spec for guide on a minimal ORM spec https://github.com/adzap/validates_timeliness/blob/master/spec/validates_timeliness/orm/active_record_spec.rb. Though not all the specs for ActiveRecord are appropriate for ActiveModel.

[TastyPi]

@adzap it's focused on the format option because that's the only option (afaict) that needs to get the "before type cast" value, and that's the only time this code path is hit.

I don't have the time right now to write tests covering the whole ORM, work is pretty busy and I've literally used this library for less than a week so I'm not familiar with all the ins and outs (I just needed a way to validate the format of some dates, and this looked like a good choice). I can maybe look at it in a month or two, but it would be nice to get the bug fix merged sooner. I can move the tests I've written into the ActiveModel spec file or anywhere else more appropriate, but I can't really commit to more than that I'm afraid.

[adzap]

@TastyPi ok cool. I will merge this as the format spec is a missing spec for validator generally. Thanks.

[adzap]

I've released a new beta with this change and some others to improve the ActiveModel testing and code. It was quite outdated and probably needs more work. Please give it a test @TastyPi