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

Keep namespace in record_type #437

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

johnvuko
Copy link

@johnvuko johnvuko commented Aug 2, 2019

Hi,
this PR is about to keep the namespace in record_type. I believe it should be like this because the namespace is an important part of the class name and without it the record_type is not assured to be unique like it's supposed to be.
We intensively use namespace and we have a few classes with the same name in different namespace and so it can create some confusion for our frontend developers.

I think it should at least be a global option, and I don't want to set set_type on every serializer and record_type on every relation (by the way it's strange to have to set again record_type on relations).

@stas
Copy link
Contributor

stas commented Aug 3, 2019

I'd like to disagree here. I think it's pretty common to have namespace scoped serializers/models and not reflect it in the record type.

I think it should at least be a global option, and I don't want to set set_type on every serializer and record_type on every relation

You don't have to set it for every serializer if you create a base serializer class which you inherit for the rest of your implemented serializers.

Consider something like this:

# app/serializers/base_serializer.rb
class BaseSerializer
  include FastJsonapi::ObjectSerializer
  def self.set_type
    super # or change it for your use-case
  end
  # ... any other customization ...
end

next

# app/serializers/tag_serializer.rb
class TagSerializer < BaseSerializer
  # ...
end

(by the way it's strange to have to set again record_type on relations)

It's a small trade-off which helps improve the performance a lot.

@johnvuko
Copy link
Author

johnvuko commented Aug 3, 2019

Hi,
for the base class that's kind of what I did, except I have overridden the reflected_record_type method.

For my case we have a project with around 200 models / tables and the namespace is used to classify / clarify models.
Like we have a model Supplier::Profile and a Restaurant::Profile (which have no common logic) and our frontend developers read the type field from the JSON which supposed to be unique by model.

The type field is supposed to identify uniquely the resource model, I don't know how you use namespace but it means you could have to 2 serializers with the same type, which doesn't follow JSON API specification from what I understand.

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.

2 participants