-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
serializable_hash and as_json should take options = nil #965
serializable_hash and as_json should take options = nil #965
Conversation
per ActiveModel::Serialization#serializable_hash https://github.com/rails/rails/blob/96bb004fc6e67cdf1b873f11ad5f8efd06949797/activemodel/lib/active_model/serialization.rb def serializable_hash(options = nil) options ||= {} Otherwise, passing in nil to `as_json` or `serializable_hash` makes things blow up when passing nil into attributes
@@ -27,7 +28,7 @@ def serializable_hash(options = {}) | |||
end | |||
end | |||
else | |||
@hash[:data] = attributes_for_serializer(serializer, @options) | |||
@hash[:data] = attributes_for_serializer(serializer, options) | |||
add_resource_relationships(@hash[:data], serializer) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note there is some confusion in the method here between the options
to the serializable_hash and the @options
to the adapter, which I have resolved.
Failure seems transient, related to global state issues present in master https://travis-ci.org/rails-api/active_model_serializers/jobs/68187470 Possibly related to #961 |
options fedault valueserializable_hash and as_json
LGTM I'm merging it. |
@@ -15,10 +15,11 @@ def initialize(serializer, options = {}) | |||
end | |||
end | |||
|
|||
def serializable_hash(options = {}) | |||
def serializable_hash(options = nil) | |||
options = {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Introduced in rails-api#965, surfaced in rails-api#1041
per ActiveModel::Serialization#serializable_hash
https://github.com/rails/rails/blob/96bb004fc6e67cdf1b873f11ad5f8efd06949797/activemodel/lib/active_model/serialization.rb
Otherwise, passing in nil to
as_json
orserializable_hash
makes thingsblow up when passing nil into attributes
Extracted from #954