-
Notifications
You must be signed in to change notification settings - Fork 137
Added dirty_tracking support for nested objects #525
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
Conversation
@@ -97,7 +112,7 @@ | |||
it 'an initialized ApiResource is equal to one generated from a response' do | |||
class ConcreteApiResource; include Intercom::Traits::ApiResource; end | |||
initialized_api_resource = ConcreteApiResource.new(object_json) | |||
except(object_json, 'type').keys.each do |attribute| | |||
except(object_json, 'type', 'nested_fields').keys.each do |attribute| |
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.
Temporarily added.
- The test checks for equality, The typed objects currently have no definition for equality.
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.
Equality for lightweight classes will be addressed in a separate PR.
@@ -107,12 +122,28 @@ class ConcreteApiResource; include Intercom::Traits::ApiResource; end | |||
|
|||
api_resource.from_hash(object_hash) | |||
initialized_api_resource = ConcreteApiResource.new(object_hash) | |||
|
|||
except(object_json, 'type').keys.each do |attribute| | |||
except(object_json, 'type', 'nested_fields').keys.each do |attribute| |
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.
Temporarily added.
- Typed objects are created only when the hash has a key
type
as string. Symbol is not taken into account.
Even though this test passes. It should've ideally failed like the previous one.
Added nested_fields
here just to maintain consistency.
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.
Will be addressed in a separate PR, Possible solutions
- Manual check of type as string/symbol
- Hash.symbolize_keys
- Hash.with_indifferent_access
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.
LGTM 👌
* Articles endpoint SDK support * Collections Endpoint Support (#523) * Sections Endpoint Support (#524) * Added Collections endpoint SDK support * Added test coverage * Added dirty_tracking support for nested objects (#525) * Added support to check for equality of two resources (#527) * Updated README and logged changes for v4.1.0 (#526)
Why?
SDK currently does not support,