Support for Nested Relationships for the Json Adapter #1114

NullVoxPopuli · 2015-09-01T17:46:45Z

This PR is based off of #1111, cause I needed the refactoring.

Discussion of idea here: #1113

Basically:

BlogSerializer < #...
  has_many :posts, include: { author: [:bio], comments: [:author]] }
end

resulting json may look like:

{
  blog: {
    posts: [
      {
        author: {
          bio: {...}
        },
        comments: [
          {  author: {...} }
        ]
      }
    ]
  }
}

See the included tests for more examples.

all tests tpass remove binding.pry require

joaomdmoura · 2015-09-04T05:05:17Z

lib/active_model/serializer/adapter/json.rb

@@ -47,6 +26,151 @@ def fragment_cache(cached_hash, non_cached_hash)
          Json::FragmentCache.new().fragment_cache(cached_hash, non_cached_hash)
        end

+        private


joaomdmoura · 2015-09-04T05:27:54Z

lib/active_model/serializer/adapter/json.rb

+          end
+        end
+
+        def resource_object_for(serializer)


Nice abstraction! 👍

…nships to nested_associations_to_include

bf4 · 2015-09-04T05:51:46Z

test/adapter/json/nested_relationships_test.rb

+            @first_comment.author = @author
+            @second_comment.author = @author
+
+            serializer = ComplexNestedAuthorSerializer.new(@author)


loving the name

joaomdmoura · 2015-09-04T06:30:32Z

lib/active_model/serializer/adapter/json.rb

+          # have the include array normalized
+          nested_associations = include_array_to_hash(nested_associations)
+
+          included_associations = if nested_associations.present?


so nested_associations should be an normalized array with the nested objects you need.

Eg.
BlogSerializer with has_many :posts include: [:author]

so nested_associations will tell you that you need author but inside this if you check if it is included iinside serializer.associations with .select. But serializer.associations will have only posts.

Initially I thought this was to check if it's an already rendered relationship and avoid render it again. but it seems it is used here to loop the associations.

What I presume is the the code inside this if might never be executed unless you have something like (probably):

Eg.
BlogSerializer with has_many :posts include: [:author.posts]

then you will find the posts inside nested_associations as a member of serializer.associations.

The if body actually decides what nested association to render.

Cause, lets say a post has a ton of associations, but you only want the author.
by the time you get to rendering the posts, [:author] will be passed in as nested_associations and the select filters for :author among all of a post's associations.

(The tests also cover this scenario)

joaomdmoura · 2015-09-04T06:47:41Z

test/adapter/json/nested_relationships_test.rb

+  # it would normally be silly to have this in production code, cause a post's
+  # author in this case is always going to be your root object
+  has_many :posts, include: [:author, comments: [:author]]
+end


Would be nice to cover scenarios using multiple association declarations as well 👍

I'll add a test for that real quick, just to be sure

joaomdmoura · 2015-09-04T07:00:32Z

We made a nice and big review here, this is complex but solid, something we aim to have on 0.10.x.
I think it's almost there, let's check the existing comments and then get back to it.

bf4 · 2015-09-04T07:00:37Z

lib/active_model/serializer/adapter/json.rb

+          hashes = include_array.select{|a| a.is_a?(Hash)}
+          non_hashes = include_array - hashes
+
+          hashes += non_hashes.map{ |association_name| { association_name => [] } }


better to mutate hashes than generate a new object, I think

this seems more like a function than a (private) property of the JSON Adapter.. What would you think about adding a

module ActiveModel::Serializer::Utils module_function def include_array_to_hash(include_array) # still don't trust input # but this also allows # include: :author syntax include_array = Array[*include_array].compact result = {} hashes = include_array.select{|a| a.is_a?(Hash)} non_hashes = include_array - hashes hashes += non_hashes.map{ |association_name| { association_name => [] } } # now merge all the hashes hashes.each{|hash| result.merge!(hash) } result end end

and then Utils.include_array_to_hash or mix it in

I like that. Calling it as a class metho would be my preference.

…fied the test for it. yay

joaomdmoura · 2015-09-05T19:27:29Z

Just waiting to merge #1017 before going back to this

NullVoxPopuli · 2015-09-05T19:43:43Z

woo!

joaomdmoura · 2015-09-06T21:45:28Z

related links:
#921
#1073

…izers into nested-relationships Conflicts: lib/active_model/serializer/adapter/json.rb

…sociation declaration in the JSON adapter initial refactor to make way for easy feature adding refactored a bit more, and matched some method names to that of json_api rename add_relationship methods to be less awkward tests added all tests tpass remove binding.pry require fixed app warnings removed unused method changed restrict_to to nested_associations and changed nested_relationships to nested_associations_to_include removed unneeded parameter from add_resource_relationships simplify key check added another test, showing multiple relationship serializers refactored include_array_to_hash to a utils module - this also simplified the test for it. yay fixed merge conflicts

Conflicts: test/action_controller/serialization_test.rb

NullVoxPopuli · 2015-09-07T15:50:07Z

Well, I accidentally committed a merge instead of a rebase. tried to checkout my last commit, and rebase that then force push to this branch... didn't work out so well. haha

NullVoxPopuli · 2015-09-07T16:13:31Z

Closed in favor of #1124 (Cleaner git history - but same changes)

joaomdmoura · 2015-09-07T16:22:11Z

👍

NullVoxPopuli changed the title ~~Support for Nested Relationships~~ Support for Nested Relationships for the Json Adapter Sep 1, 2015

NullVoxPopuli force-pushed the nested-relationships branch from bef5e07 to db5d174 Compare September 1, 2015 18:02

NullVoxPopuli added 5 commits September 1, 2015 15:29

initial refactor to make way for easy feature adding

edfec6c

refactored a bit more, and matched some method names to that of json_api

8df52d5

rename add_relationship methods to be less awkward

4f2ad3e

tests added

a8e8515

all tests tpass remove binding.pry require

fixed app warnings

05ec32a

NullVoxPopuli force-pushed the nested-relationships branch from c75fbe3 to 05ec32a Compare September 1, 2015 19:31

NullVoxPopuli mentioned this pull request Sep 2, 2015

[RFC]Thoughts on an explicit way to define nested relationships? #1113

Closed

joaomdmoura reviewed Sep 4, 2015
View reviewed changes

removed unused method

8a6ee84

joaomdmoura reviewed Sep 4, 2015
View reviewed changes

changed restrict_to to nested_associations and changed nested_relatio…

ce75e78

…nships to nested_associations_to_include

bf4 reviewed Sep 4, 2015
View reviewed changes

removed unneeded parameter from add_resource_relationships

de2d9b4

joaomdmoura reviewed Sep 4, 2015
View reviewed changes

simplify key check

2fd748e

joaomdmoura reviewed Sep 4, 2015
View reviewed changes

This was referenced Sep 4, 2015

added option to include nested associations #952

Closed

refactor to make way for easy feature adding #1111

Closed

bf4 reviewed Sep 4, 2015
View reviewed changes

joaomdmoura added this to the 0.10 milestone Sep 4, 2015

joaomdmoura added C: Feature V: 0.10.x labels Sep 4, 2015

NullVoxPopuli added 2 commits September 4, 2015 03:00

added another test, showing multiple relationship serializers

85406f9

refactored include_array_to_hash to a utils module - this also simpli…

75d7a95

…fied the test for it. yay

This was referenced Sep 6, 2015

Side loading data with embed_ids like 0.9 #1084

Closed

Nested Associations #1077

Closed

NullVoxPopuli and others added 5 commits September 7, 2015 11:10

Merge branch 'master' of github.com:NullVoxPopuli/active_model_serial…

670c724

…izers into nested-relationships Conflicts: lib/active_model/serializer/adapter/json.rb

Factor with_adapter + force cache clear before each test.

9f8f7fc

Minor style improvements.

f11fa95

Merge branch 'rebase-nested-associations' into nested-relationships

608741f

Conflicts: test/action_controller/serialization_test.rb

NullVoxPopuli mentioned this pull request Sep 7, 2015

Nested associations for json adapter #1124

Closed

NullVoxPopuli closed this Sep 7, 2015

NullVoxPopuli mentioned this pull request Sep 7, 2015

Support nested associations for Json and Attributes adapters + Refactor Attributes adapter #1127

Merged

NullVoxPopuli deleted the nested-relationships branch September 8, 2015 12:19

remear mentioned this pull request Mar 15, 2016

Side loading doesn't work in 0.10.0.rc1 (non-JSONAPI) #899

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for Nested Relationships for the Json Adapter #1114

Support for Nested Relationships for the Json Adapter #1114

NullVoxPopuli commented Sep 1, 2015

joaomdmoura Sep 4, 2015

joaomdmoura Sep 4, 2015

NullVoxPopuli Sep 4, 2015

bf4 Sep 4, 2015

joaomdmoura Sep 4, 2015

NullVoxPopuli Sep 4, 2015

joaomdmoura Sep 4, 2015

NullVoxPopuli Sep 4, 2015

joaomdmoura commented Sep 4, 2015

bf4 Sep 4, 2015

NullVoxPopuli Sep 4, 2015

bf4 Sep 4, 2015

NullVoxPopuli Sep 4, 2015

joaomdmoura commented Sep 5, 2015

NullVoxPopuli commented Sep 5, 2015

joaomdmoura commented Sep 6, 2015

NullVoxPopuli commented Sep 7, 2015

NullVoxPopuli commented Sep 7, 2015

joaomdmoura commented Sep 7, 2015

Support for Nested Relationships for the Json Adapter #1114

Support for Nested Relationships for the Json Adapter #1114

Conversation

NullVoxPopuli commented Sep 1, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joaomdmoura commented Sep 4, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joaomdmoura commented Sep 5, 2015

NullVoxPopuli commented Sep 5, 2015

joaomdmoura commented Sep 6, 2015

NullVoxPopuli commented Sep 7, 2015

NullVoxPopuli commented Sep 7, 2015

joaomdmoura commented Sep 7, 2015