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

Document and implement options to serializable_hash #1643

Open
bf4 opened this issue Mar 31, 2016 · 4 comments
Open

Document and implement options to serializable_hash #1643

bf4 opened this issue Mar 31, 2016 · 4 comments

Comments

@bf4
Copy link
Member

bf4 commented Mar 31, 2016

Expected behavior vs actual behavior

The only things that we should ever explicitly pass to to_json and friends are the options that go to ActiveModel::Serialization and ActiveModel::Serializers::JSON

i.e. only, except, methods, include

The reason is that render json: resource, options essentially results in the Rails Renderer logic below.

resource = ActiveModel::Serializable_resource.new(resource, options)
resource.to_json(options) unless resource.is_a?(String)

This continues work from #1494

Related, #1572 fixes a case where we accidentally are using options passing into serializable_hash

Details

This is how AMS fits into rendering and why I'm thinking this way

render json: record, options
# |-> https://github.com/rails/rails/blob/v5.0.0.beta3/actionpack/lib/action_controller/metal/renderers.rb#L151-L152
  _render_with_renderer_json(record, options)
+  # |-> https://github.com/rails-api/active_model_serializers/blob/d30aa4c44fe004821be49d3427b56973f95c4984/lib/action_controller/serialization.rb#L46-L52
+    record = ActiveModel::Serializable_resource.new(record, options)
    # |-> https://github.com/rails/rails/blob/v5.0.0.beta3/actionpack/lib/action_controller/metal/renderers.rb#L159
      record.to_json(options) unless record.is_a?(String)
      # |-> https://github.com/rails/rails/blob/v5.0.0.beta3/activemodel/lib/active_model/serializers/json.rb#L88-L92
        record.as_json(options)
        # |-> https://github.com/rails/rails/blob/v5.0.0.beta3/activemodel/lib/active_model/serialization.rb#L124-L137
          record.serializable_hash(options)

where serializalbe_hash is where the options only, except, methods, include are defined, and as_json defines the option root and uses include_root_in_json.

@groyoh
Copy link
Member

groyoh commented Apr 2, 2016

What is the goal of the methods options?

@bf4
Copy link
Member Author

bf4 commented Apr 3, 2016

Goal? It's been in Rails json serialization since 2011. It's just calls that method.

@bf4
Copy link
Member Author

bf4 commented Apr 13, 2016

Closed by #1678

@bf4
Copy link
Member Author

bf4 commented Aug 5, 2016

Re-opened since those options aren't actually handled

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

2 participants