Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate all associated records regardless of their dirty flag #5135

Closed

Conversation

mitio
Copy link
Contributor

@mitio mitio commented Feb 22, 2012

Proposes a solution for issue #5134.

ActiveRecord tests after this change (I had problems running tests for the mysql, mysql2 and postgresql adapters):

(...)
3253 tests, 9918 assertions, 0 failures, 0 errors, 3 skips
(...)
Errors running test_mysql, test_mysql2, test_postgresql

@josevalim
Copy link
Contributor

IMHO the bug is on dirty/changed. I'm not pleased with the idea of validating all records. If you have many records associated, it is an unnecessary overhead. Maybe dirty could mean: I tried to set something in this field different from the actual value. While changed would mean: this value really changed. Then we could rely on using dirty for the auto save detection.

@josevalim
Copy link
Contributor

Another possible place for this solution is where we are doing the typecast of the string "foo" to an integer. Closing this because I don't think it is the most appropriate solution to the described problem. Thanks!

@josevalim josevalim closed this Feb 23, 2012
@mitio
Copy link
Contributor Author

mitio commented Feb 23, 2012

The "dirty" solution sounds the most feasible. I'll try to do something on the subject when I get some more time.

And as for the typecast to an integer solition — I'm not sure what exactly do you mean by that? I doubt that there we could do much good without changing standard Fixnum#to_s semantics and possibly breaking tons of code. Moreover, my original problem was not with integer columns, but with decimal ones.

@mitio
Copy link
Contributor Author

mitio commented Feb 23, 2012

After I looked around, I think I see what you may have had in mind with that "the place where we're doing the typecast to int". The place where the typecast is performed is where the knowledge of "is this value the same as the original one after the typecast or not?" is contained. Also, I think it is not that trivial to implement, so I would appreciate any suggestions on how to approach this.

We would have to be able to somehow compare the following values, either one by one at write_attribute time, or at some other stage:

price = Price.find ...

price.price = 'foo'
price.attributes
price.attributes_before_type_cast

# => {"id"=>1, "price"=>0, "product_id"=>1, "created_at"=>Wed, 22 Feb 2012 17:22:26 UTC +00:00, "updated_at"=>Wed, 22 Feb 2012 17:22:26 UTC +00:00}
# => {"id"=>1, "price"=>"foo", "product_id"=>1, "created_at"=>"2012-02-22 17:22:26.991756", "updated_at"=>"2012-02-22 17:22:26.991756"}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants