-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Fix fields
option to restrict relationships as well.
#1297
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -44,13 +44,7 @@ def object | |
def initialize(serializer, options = {}) | ||
super | ||
@include_tree = IncludeTree.from_include_args(options[:include]) | ||
|
||
fields = options.delete(:fields) | ||
if fields | ||
@fieldset = ActiveModel::Serializer::Fieldset.new(fields) | ||
else | ||
@fieldset = options[:fieldset] | ||
end | ||
@fieldset = options[:fieldset] || ActiveModel::Serializer::Fieldset.new(options.delete(:fields)) | ||
end | ||
|
||
def serializable_hash(options = nil) | ||
|
@@ -174,7 +168,10 @@ def relationship_value_for(serializer, options = {}) | |
end | ||
|
||
def relationships_for(serializer) | ||
serializer.associations.each_with_object({}) do |association, hash| | ||
resource_type = resource_identifier_type_for(serializer) | ||
requested_associations = fieldset.fields_for(resource_type) || '*' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like that the try is gone :-) |
||
include_tree = IncludeTree.from_include_args(requested_associations) | ||
serializer.associations(include_tree).each_with_object({}) do |association, hash| | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In words, what does this diff do? - serializer.associations.each_with_object({}) do |association, hash|
+ resource_type = resource_identifier_type_for(serializer)
+ requested_associations = fieldset.fields_for(resource_type) || '*'
+ include_tree = IncludeTree.from_include_args(requested_associations)
+ serializer.associations(include_tree).each_with_object({}) do |association, hash| It's supposed to somehow remove associations somehow? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It filters out associations not present in the fieldset of the resource (unless no fieldset has been provided for that resource). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe it's just the method and variable names. I can be pretty literal-minded and it looks like whereas before we would iterate over the associations, now we ask the serializer what it's There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Exactly. The There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @beauby I guess this means that an explanation needs to be in code comments. :-) |
||
hash[association.key] = { data: relationship_value_for(association.serializer, association.options) } | ||
end | ||
end | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,7 +2,7 @@ module ActiveModel | |
class Serializer | ||
class Fieldset | ||
def initialize(fields) | ||
@raw_fields = fields | ||
@raw_fields = fields || {} | ||
end | ||
|
||
def fields | ||
|
@@ -21,7 +21,7 @@ def fields_for(type) | |
|
||
def parsed_fields | ||
if raw_fields.is_a?(Hash) | ||
raw_fields.inject({}) { |h, (k, v)| h[k.to_sym] = v.map(&:to_sym); h } | ||
raw_fields.each_with_object({}) { |(k, v), h| h[k.to_sym] = v.map(&:to_sym) } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This (isomorphic) change is not necessary for this PR, but it makes the code slightly cleaner. |
||
else | ||
{} | ||
end | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,89 @@ | ||
require 'test_helper' | ||
|
||
module ActiveModel | ||
class Serializer | ||
module Adapter | ||
class JsonApi | ||
class FieldsTest < Minitest::Test | ||
Post = Class.new(::Model) | ||
class PostSerializer < ActiveModel::Serializer | ||
type 'posts' | ||
attributes :title, :body | ||
belongs_to :author | ||
has_many :comments | ||
end | ||
|
||
Author = Class.new(::Model) | ||
class AuthorSerializer < ActiveModel::Serializer | ||
type 'authors' | ||
attributes :name, :birthday | ||
end | ||
|
||
Comment = Class.new(::Model) | ||
class CommentSerializer < ActiveModel::Serializer | ||
type 'comments' | ||
attributes :body | ||
belongs_to :author | ||
end | ||
|
||
def setup | ||
@author = Author.new(id: 1, name: 'Lucas', birthday: '10.01.1990') | ||
@comment1 = Comment.new(id: 7, body: 'cool', author: @author) | ||
@comment2 = Comment.new(id: 12, body: 'awesome', author: @author) | ||
@post = Post.new(id: 1337, title: 'Title 1', body: 'Body 1', | ||
author: @author, comments: [@comment1, @comment2]) | ||
@comment1.post = @post | ||
@comment2.post = @post | ||
end | ||
|
||
def test_fields_attributes | ||
fields = { posts: [:title] } | ||
hash = serializable(@post, adapter: :json_api, fields: fields).serializable_hash | ||
expected = { | ||
title: 'Title 1' | ||
} | ||
|
||
assert_equal(expected, hash[:data][:attributes]) | ||
end | ||
|
||
def test_fields_relationships | ||
fields = { posts: [:author] } | ||
hash = serializable(@post, adapter: :json_api, fields: fields).serializable_hash | ||
expected = { | ||
author: { | ||
data: { | ||
type: 'authors', | ||
id: '1' | ||
} | ||
} | ||
} | ||
|
||
assert_equal(expected, hash[:data][:relationships]) | ||
end | ||
|
||
def test_fields_included | ||
fields = { posts: [:author], comments: [:body] } | ||
hash = serializable(@post, adapter: :json_api, fields: fields, include: 'comments').serializable_hash | ||
expected = [ | ||
{ | ||
type: 'comments', | ||
id: '7', | ||
attributes: { | ||
body: 'cool' | ||
} | ||
}, { | ||
type: 'comments', | ||
id: '12', | ||
attributes: { | ||
body: 'awesome' | ||
} | ||
} | ||
] | ||
|
||
assert_equal(expected, hash[:included]) | ||
end | ||
end | ||
end | ||
end | ||
end | ||
end |
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.
nice!