Skip to content

pass array of unique movies to serializer - #378

Merged
shishirmk merged 2 commits into
Netflix:devfrom
sdsd08013:pass_array_of_unique_movies
Apr 7, 2019
Merged

pass array of unique movies to serializer#378
shishirmk merged 2 commits into
Netflix:devfrom
sdsd08013:pass_array_of_unique_movies

Conversation

@sdsd08013

Copy link
Copy Markdown

According to JSON API document, id should be unique.
https://jsonapi.org/format/#document-resource-objects

id of movie generated by let(:movie) is always same.
https://github.com/Netflix/fast_jsonapi/blob/master/spec/shared/contexts/movie_context.rb#L438

So I replace array passed to serializer [movies, movies] to build_movies(2)

before:

MovieSerializer.new([movie, movie], options)

after:

movies = build_movies(2)
MovieSerializer.new(movies, options)

Comment thread README.md
}
options[:include] = [:actors, :'actors.agency', :'actors.agency.state']
MovieSerializer.new([movie, movie], options).serialized_json
MovieSerializer.new(movies, options).serialized_json

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The problem I see with this change in the documentation is that the variable movies is never declared (as far as I could tell) within the documentation. More of a nitpick than a usage issue.

@sdsd08013 sdsd08013 Jan 17, 2019

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

right.
I'm thinking about adding the below to Sample Object document

movies = 2.times.map do |i|
           m = Movie.new
           m.id = i + 1
           m.name = 'test movie'
           m.actor_ids = [1, 2, 3]
           m.owner_id = 3
           m.movie_type_id = 1
           m
         end

and replace [movie, movie] statement in the documentation to declared movies .

How do you think about the above change?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I wouldn't think you would want the title of both movies to be test movie. Other than that, seems appropriate.

Comment thread spec/lib/object_serializer_spec.rb Outdated
options[:links] = { self: 'self' }
options[:include] = [:actors]
serializable_hash = MovieSerializer.new([movie, movie], options).serializable_hash
movies = build_movies(2)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What do you think about making this a let so you only have to declare it once?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

thanks. I'll replace redundant code by using let statement.

@shishirmk
shishirmk merged commit 83e7fb6 into Netflix:dev Apr 7, 2019
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.

3 participants