Skip to content

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

Merged
merged 1 commit into from
Jun 4, 2020

Conversation

rahulgdsouza
Copy link
Contributor

Why?

SDK currently does not support,

  • Dirty tracking nested objects.
  • Serializing nested objects for sending requests

@@ -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|
Copy link
Contributor Author

@rahulgdsouza rahulgdsouza Jun 2, 2020

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.

Copy link
Contributor Author

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|
Copy link
Contributor Author

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.

Copy link
Contributor Author

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

@rahulgdsouza rahulgdsouza marked this pull request as ready for review June 3, 2020 09:41
Copy link
Contributor

@murtron murtron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👌

@rahulgdsouza rahulgdsouza merged commit a4472ae into conorkeating/sdk-v4.1 Jun 4, 2020
@rahulgdsouza rahulgdsouza deleted the rgd/nested_objects_support branch July 6, 2020 06:18
rahulgdsouza added a commit that referenced this pull request Jul 14, 2020
* 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)
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.

2 participants