Skip to content

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

Merged
merged 1 commit into from
Oct 2, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions lib/active_model/serializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@ def self.inherited(base)
super
end

def self.type(type)
self._type = type
end
Copy link
Member

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!

Copy link
Contributor Author

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.

Copy link
Member

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

Copy link
Contributor Author

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.


def self.attributes(*attrs)
attrs = attrs.first if attrs.first.class == Array

Expand Down Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

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

  • this can go up near the other class methods


def initialize(object, options = {})
self.object = object
Expand Down
1 change: 1 addition & 0 deletions lib/active_model/serializer/adapter/json_api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

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

I know you didn't write this, but urgh

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

else
Copy link
Contributor

Choose a reason for hiding this comment

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

Avoid unless with an else clause. Rewrite as an if with the positive clause first (https://github.com/bbatsov/ruby-style-guide#no-else-with-unless)

Expand Down
32 changes: 22 additions & 10 deletions test/adapter/json_api/resource_type_config_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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))
Copy link
Contributor

Choose a reason for hiding this comment

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

Why using fetch here instead of brackets?

Copy link
Member

Choose a reason for hiding this comment

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

Better error on failure

B mobile phone

On Oct 4, 2015, at 11:14 AM, Lucas Hosseini [email protected] wrote:

In test/adapter/json_api/resource_type_config_test.rb:

         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))
    
    Why using fetch here instead of brackets?


Reply to this email directly or view it on GitHub.

Copy link
Contributor

Choose a reason for hiding this comment

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

Then why not use it there?

Copy link
Member

Choose a reason for hiding this comment

The 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

On Oct 4, 2015, at 12:33 PM, Lucas Hosseini [email protected] wrote:

In test/adapter/json_api/resource_type_config_test.rb:

         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))
    
    Then why not use it there?


Reply to this email directly or view it on GitHub.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed it's not important, but best to remain consistent.

Copy link
Member

Choose a reason for hiding this comment

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

remain consistent

please

end

private

def serializable(resource, options = {})
ActiveModel::SerializableResource.new(resource, options)
end
end
end
end
Expand Down