-
Notifications
You must be signed in to change notification settings - Fork 465
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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix dirty attribute changes on update #1149
Conversation
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.
It's easy
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.
Makes sense to me!
T.unsafe(@has_one[attr_sym]).create_instance(data: data_hash, session: session)) | ||
child = T.unsafe(@has_one[attr_sym]).create_instance(data: data_hash, session: session) | ||
instance.public_send("#{attribute}=", child) | ||
instance.original_state[attr_sym] = child.to_hash(true) |
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.
This was a breaking change for our application that went unreported in the changelog. We were creating an instance and immediately calling save on it, which with this change resulted in an empty hash being sent to Shopify (since the original state matched the current state after instantiation)
I.E. ShopifyAPI::ObjectName.build(attrs).save!
<- this fails now AFAICT
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.
Not sure if we were using the library improperly since I didn't see mention of this is the docs, but if not I can go update the changelog for others who might run into this. cc @nelsonwittwer
Description
Fixes #1133 and #1037 which both report a substantial issue with tracking what attributes were actually modified by the user since the rest resources were initialized.
The root cause of this issue, as pointed out by @kaarelss, was symbols vs hashes when comparing drift on attributes 馃う . I also needed to 1) set associations in
original_state
since our API allows us to modify associated resources and 2) needed to ignore read only attributes inoriginal_state
.How has this been tested?
Checklist: