-
Notifications
You must be signed in to change notification settings - Fork 57
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
Persist default values of aliased attributes #96
base: master
Are you sure you want to change the base?
Persist default values of aliased attributes #96
Conversation
@@ -958,19 +958,26 @@ | |||
describe DirtyTrackingModel do | |||
it 'stores the default on creation' do | |||
model = DirtyTrackingModel.create! | |||
model = DirtyTrackingModel.find(model.id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I considered this but wasn't expecting anyone to use aliased attributes in this case. Is this something you are doing? |
super | self.class.typed_stores.keys.map(&:to_s).select do |store| | ||
@attributes.key?(store) && @attributes[store].value_before_type_cast.blank? | ||
super | self.class.typed_store_attribute_names.select do |store| | ||
read_attribute_before_type_cast(store).blank? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I was checking @attribute.key?
in case it's a partially loaded model (I should have added a test for it TBH)
No, I just wanted to make sure all the cases were covered. 😄 I wasn't sure how much aliased attributes were used at Shopify. But if they are explicitly not supported, I think it's fine. |
Yeah, I don't really see a use case for it to be honest. That said I initially took that decision in #94 because it was in much more of a hot path, it probably wouldn't matter here. We could also cache (or precompute) that array on the class. |
1f150f6 made it so that default values are always persisted. This commit adds support for aliased attributes to that change.
fa10113
to
3a38ad9
Compare
1f150f6 made it so that default values are always persisted. This commit adds support for aliased attributes to that change.