-
Notifications
You must be signed in to change notification settings - Fork 1.4k
type
directive for serializer to control type field with json-api adapter
#1213
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
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 |
---|---|---|
|
@@ -46,6 +46,10 @@ def self.inherited(base) | |
super | ||
end | ||
|
||
def self.type(type) | ||
self._type = type | ||
end | ||
|
||
def self.attributes(*attrs) | ||
attrs = attrs.first if attrs.first.class == Array | ||
|
||
|
@@ -122,6 +126,7 @@ def self.get_serializer_for(klass) | |
end | ||
|
||
attr_accessor :object, :root, :meta, :meta_key, :scope | ||
class_attribute :_type, instance_writer: false | ||
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.
|
||
|
||
def initialize(object, options = {}) | ||
self.object = object | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -71,6 +71,7 @@ def serializable_hash_for_single_resource(options) | |
end | ||
|
||
def resource_identifier_type_for(serializer) | ||
return serializer._type if serializer._type | ||
if ActiveModel::Serializer.config.jsonapi_resource_type == :singular | ||
serializer.object.class.model_name.singular | ||
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 know you didn't write this, but urgh 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. 👍 |
||
else | ||
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. Avoid |
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,11 @@ class Serializer | |
module Adapter | ||
class JsonApi | ||
class ResourceTypeConfigTest < Minitest::Test | ||
class ProfileTypeSerializer < ActiveModel::Serializer | ||
attributes :name | ||
type 'profile' | ||
end | ||
|
||
def setup | ||
@author = Author.new(id: 1, name: 'Steve K.') | ||
@author.bio = nil | ||
|
@@ -36,22 +41,29 @@ def with_jsonapi_resource_type type | |
end | ||
|
||
def test_config_plural | ||
with_adapter :json_api do | ||
with_jsonapi_resource_type :plural do | ||
hash = ActiveModel::SerializableResource.new(@comment).serializable_hash | ||
assert_equal('comments', hash[:data][:type]) | ||
end | ||
with_jsonapi_resource_type :plural do | ||
hash = serializable(@comment, adapter: :json_api).serializable_hash | ||
assert_equal('comments', hash[:data][:type]) | ||
end | ||
end | ||
|
||
def test_config_singular | ||
with_adapter :json_api do | ||
with_jsonapi_resource_type :singular do | ||
hash = ActiveModel::SerializableResource.new(@comment).serializable_hash | ||
assert_equal('comment', hash[:data][:type]) | ||
end | ||
with_jsonapi_resource_type :singular do | ||
hash = serializable(@comment, adapter: :json_api).serializable_hash | ||
assert_equal('comment', hash[:data][:type]) | ||
end | ||
end | ||
|
||
def test_explicit_type_value | ||
hash = serializable(@author, serializer: ProfileTypeSerializer, adapter: :json_api).serializable_hash | ||
assert_equal('profile', hash.fetch(:data).fetch(: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. Why using 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. Better error on failure B mobile phone
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. Then why not use it there? 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. Not at computer now, could be oversight, not important in any case B mobile phone
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. Agreed it's not important, but best to remain consistent. 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.
please |
||
end | ||
|
||
private | ||
|
||
def serializable(resource, options = {}) | ||
ActiveModel::SerializableResource.new(resource, options) | ||
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.
Awesome! 💯 Thanks for helping keep the public interface narrow!
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.
I'm not sure it's narrow. Underscored class-level accessor will be accessible anyways.
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.
well, basically we want to avoid creating internal instance methods that collide with attribute names.. less important in class methods, but still :) Right now, the serializer's internal instance methods have too wide an interface
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.
Yeah, right, I see that instance should be as clean as possible to let users use their names for attributes.