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

Allow the default value of include_xyz? to be specified easily #116

Closed
GavinJoyce opened this issue Feb 4, 2015 · 4 comments · Fixed by #117
Closed

Allow the default value of include_xyz? to be specified easily #116

GavinJoyce opened this issue Feb 4, 2015 · 4 comments · Fixed by #117

Comments

@GavinJoyce
Copy link
Contributor

It's currently possible to do serializer.as_json(model, { include_description?: false }) which will exclude the description attribute. The default is to include the description.

It would be nice to be able to easily set the default to not include the attribute. Something like:

class PersonSerializer
    include RestPack::Serializer
    attributes :id, :name, { description: { include_by_default: false } }
end

serializer.as_json(model) would exclude the description
serializer.as_json(model, { include_description?: false }) would exclude the description
serializer.as_json(model, { include_description?: true }) would include the description

@GavinJoyce
Copy link
Contributor Author

@eugeneius, how about:

class PersonSerializer
    include RestPack::Serializer

    attributes :id, :name
    optional_attributes :description
end

or

class PersonSerializer
    include RestPack::Serializer

    attributes :id, :name
    optional :description
end

@eugeneius
Copy link
Collaborator

Adding them separately definitely reads better than flagging them inline as part of attributes 👍

@eugeneius
Copy link
Collaborator

What about using a similar API to mutations:

class PersonSerializer
  include RestPack::Serializer

  attributes :id, :name

  optional do
    attributes :description
  end
end

@GavinJoyce
Copy link
Contributor Author

That's nice however I think if I added support for that, the main API should become:

class PersonSerializer
  include RestPack::Serializer

  required do
    attributes :id, :name
  end

  optional do
    attributes :description
  end
end

I think I'll keep it simple for now, I've almost got a PR ready

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

Successfully merging a pull request may close this issue.

2 participants