Skip to content
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

Inheritance of serializer #1188

Closed
givigier opened this issue Sep 22, 2015 · 21 comments
Closed

Inheritance of serializer #1188

givigier opened this issue Sep 22, 2015 · 21 comments

Comments

@givigier
Copy link

class ApplicationSerializer < ActiveModel::Serializer
  attributes :id
  def id
    "test"
  end
end
class CustomerSerializer < ApplicationSerializer
end

I'm trying to use something like this on branch master AMS but it does not work. When I use CustomerSerializer, it does include the attribute :id. Is it possible to do this on AMS?

@bf4
Copy link
Member

bf4 commented Sep 22, 2015

Try defining read_attribute_for_serialization(attr) on your model and handle id there

Otherwise, we'll need some more information of

  • what you're trying to serializer and
  • with what data
  • what you expected
  • what you saw
  • what you tried
  • ruby flavor and version, rails version

@bf4
Copy link
Member

bf4 commented Sep 22, 2015

@givigier
Copy link
Author

Thanks @bf4. I could ask this on Stackoverflow. Could you close this issue please?

@beauby
Copy link
Contributor

beauby commented Sep 22, 2015

@givigier Closing for now, please post the link of the StackOverflow question here if you do open one.

@beauby beauby closed this as completed Sep 22, 2015
@bf4
Copy link
Member

bf4 commented Sep 22, 2015

(And link from your question here, as well.)

@givigier
Copy link
Author

@givigier
Copy link
Author

I guess that it's a bug. I'll try to explain exactly what I did.
I have two serializers, first is ApplicationSerializer and the second is CompanySerializer that inherits of ApplicationSerializer.

class ApplicationSerializer < ActiveModel::Serializer
  def id
    object.slug
  end
end
class V1::CompanySerializer < ApplicationSerializer
  attributes :id, :email
end

When I run on branch: '0-10-stable' I got:

{"id"=>"company-1", "email"=>"[email protected]"}

"company-1" is the slug of company with email [email protected]

When I run on branch: 'master' I got:

{"id"=>"1", "email"=>"[email protected]"}

In this case, slug is not setted as id. It also does not work when I try to use concerns like this:

module SerializerSluggable
  extend ActiveSupport::Concern

  def id
    object.slug
  end
end
require 'concerns/serializer_slugglable'
class V1::CompanySerializer <  ActiveModel::Serializer
  include SerializerSluggable

  attributes :id, :email
end

I got exactly the same result:

When I run on branch: '0-10-stable' I got:

{"id"=>"company-1", "email"=>"[email protected]"}

"company-1" is the slug of company with email [email protected]

When I run on branch: 'master' I got:

{"id"=>"1", "email"=>"[email protected]"}

My rails version is 4.2.2 and ruby version is 2.2.0.

@givigier
Copy link
Author

@bf4 @beauby is there any info i could post here to explain better?

@NullVoxPopuli
Copy link
Contributor

what happens if you do:

resource = SerializableResource.new(@company, serializer: V1::CompanySerializer)
resource.serializer.id

?

@givigier
Copy link
Author

resource = ActiveModel::SerializableResource.new(@company, serializer: V1::CompanySerializer)
resource.serializer.id

return NoMethodError: undefined method `id' for V1::CompanySerializer:Class

@NullVoxPopuli
Copy link
Contributor

well, that didn't do what I thought it would...

anyway,
I'll see if I can re-create your scenario in a test and debug a bit

@NullVoxPopuli NullVoxPopuli reopened this Sep 22, 2015
@bf4
Copy link
Member

bf4 commented Sep 22, 2015

I'm guessing the bug is in Serializer._attribute(*args) that conditionally defines attribute methods on the serializer , and that sonething is wrong in one of them

What's

ApplicationSerializer.instance_method(:id).source_location
ApplicationSerializer.instance_method(:id).source

And

CompanySerializer.instance_method(:id).source_location
CompanySerializer.instance_method(:id).source

And for each serializer

Serializer._attributes
serializer.attributes

On a SerializableResource you can call .serializer

@beauby
Copy link
Contributor

beauby commented Sep 22, 2015

As you can see here, if the method id is defined on the serializer, that is what will be used as id. In your case, it seems like the id method is not defined on the inherited serializer. Could you post the output of what @bf4 suggested? That would probably help solve the issue pretty quickly.

@givigier
Copy link
Author

class Company < ActiveRecord::Base
end

Company is an ActiveRecord model. It's not necessary to add read_attribute_for_serialization(attr) or am I wrong?

@beauby I added id on serializer too and it does not work 😢

class ApplicationSerializer < ActiveModel::Serializer
  def id
    object.slug
  end
end
class V1::CompanySerializer < ApplicationSerializer
  attributes :id, :email
end

It only works when I'm using branch: '0-10-stable', on master it's broken.

@beauby
Copy link
Contributor

beauby commented Sep 23, 2015

Company is an ActiveRecord model. It's not necessary to add read_attribute_for_serialization(attr) or am I wrong?

You're right about this.

In order to help you, we still need the output of:

ApplicationSerializer.instance_method(:id).source_location
ApplicationSerializer.instance_method(:id).source

CompanySerializer.instance_method(:id).source_location
CompanySerializer.instance_method(:id).source

ApplicationSerializer._attributes
ApplicationSerializer.attributes

CompanySerializer._attributes
CompanySerializer.attributes

@givigier
Copy link
Author

ApplicationSerializer.instance_method(:id).source_location

["/Users/givigier/Sites/agrid-api/app/serializers/application_serializer.rb", 4]

ApplicationSerializer.instance_method(:id).source

" def id\n object.slug\n end\n"

V1::CompanySerializer.instance_method(:id).source_location

["/Users/givigier/Sites/agrid-api/app/serializers/v1/company_serializer.rb", 9]

V1::CompanySerializer.instance_method(:id).source

" def id\n object.slug\n end\n"

ApplicationSerializer._attributes

[:id]

ApplicationSerializer.attributes

[]

V1::CompanySerializer._attributes

[:id, :email]

V1::CompanySerializer.attributes

[]

@beauby
Copy link
Contributor

beauby commented Sep 23, 2015

So what happens is that, in your parent serializer (ApplicationSerializer), you define the method id, which get inherited to CompanySerializer. However, when you latter call attributes :id, :email in CompanySerializer, this overrides the custom id method inherited from ApplicationSerializer, and replaces it with a stock def id; object.id; end.
This problem arrises only with the id attribute, because we somehow have to disable the check whether an id method already exists on the serializer when adding a stock one due to an attributes call (https://github.com/rails-api/active_model_serializers/blob/master/lib/active_model/serializer.rb#L65).
I'll investigate this more thoroughly, but in the meantime, you could use a concern that you would include at the end of your serializer definition.

@beauby
Copy link
Contributor

beauby commented Sep 23, 2015

@givigier Could you test #1195 and confirm it fixes your issue?

@givigier
Copy link
Author

It fixes the issue, thanks. I was going to try contributing but you fix fastly 👍

@givigier
Copy link
Author

@beauby Can I close the issue?

@beauby
Copy link
Contributor

beauby commented Sep 23, 2015

Sure 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants