Default values should not be marked dirty #18
Conversation
| @@ -167,7 +167,9 @@ def _define_accessors_methods(store_name, *keys, prefix: nil, suffix: nil) # :no | |||
| end | |||
|
|
|||
| define_method(accessor_key) do | |||
| read_store_attribute(store_name, key) | |||
| next_value = read_store_attribute(store_name, key) | |||
| clear_attribute_change(store_name) unless changed? | |||
There was a problem hiding this comment.
It seems that this change is redundant (or we don't have a test which proves it's required 🙂).
Also, if we still need it, shouldn't we check for this particular store attribute changed only? Currently, we consider all the attributes changes.
There was a problem hiding this comment.
Hmm let me take another look. I think the changed? is what calls changed_in_place? which allows us to do the comparison against the raw DB value.
You might be right about clearing the entire store though, I'll add a test where we change a couple of attributes.
|
Ok came at this with a different angle because The tests should now cover that too. |
|
Awesome. We have one failing test for Rails 5, could you please take a look? (A quick fix could be to not include the patch for Rails <6—no need to spend a lot of time on supporting old versions 🙂) |
|
Thanks a lot! Released in 0.9.0 |
Fixes #12
What is the purpose of this pull request?
Don't mark attributes that have a default value as dirty when they haven't actually changed
What changes did you make? (overview)
From https://api.rubyonrails.org/classes/ActiveModel/Type/Value.html#method-i-changed_in_place-3FOnly mark the value as changed if it differs from the read value from the database, before being cast.Overwrite
changesand compare the previous and current values and remove them from the hash if they're the same. It's sloppy but effective.Is there anything you'd like reviewers to focus on?
This is unfamiliar Rails core territory to me so my confidence is not super high. I tried a few approaches and this seemed to get the closest to where I was expecting to need to make the call but I could be missing something.Checklist