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

ActiveModelSerializers::Model successor initialized with string keys fix #1881

Merged

Conversation

yevhene
Copy link
Contributor

@yevhene yevhene commented Aug 15, 2016

Purpose

Fixing strange behavior of ActiveModelSerializers::Model successor when initialized with string keys hash

Changes

ActiveModelSerializers::Model is stringify keys on initialization

Additional helpful information

Example:

class User < ActiveModelSerializers::Model
  attr_accessor :name
end

class UserSerializer < ActiveModel::Serializer
  attribute :name
end

resource = User.new('key' => value)
serializer = UserSerializer.new(resource)
serializer.to_json #=> '{"name": null}'

@mention-bot
Copy link

@yevhene, thanks for your PR! By analyzing the annotation information on this pull request, we identified @bf4 and @dubadub to be potential reviewers

@NullVoxPopuli
Copy link
Contributor

this looks good. Simple, to the point. I actually didn't know why someone would want to do this, but I checked AR, and it supports this. So.. neat.

Could you add a changelog entry?

@bf4
Copy link
Member

bf4 commented Aug 16, 2016

@yevhene I'm pretty sure that behavior comes from ActiveModel::Model. Also, serializer.to_json is not the recommended way to serialize a model.

@yevhene
Copy link
Contributor Author

yevhene commented Aug 16, 2016

@bf4 Yes, this behavior is comes with ActiveModel::Model, but ActiveModelSerializers::Model overrides it for some reason. Problem not in to_json.

@yevhene
Copy link
Contributor Author

yevhene commented Aug 16, 2016

@NullVoxPopuli I was faced with this when I take raw data from database and wrap it with ActiveModelSerializers::Model descendant.

@yevhene yevhene force-pushed the model-initialized-with-strings-fix branch from 1fde9cc to 4480a08 Compare August 16, 2016 07:24
@yevhene
Copy link
Contributor Author

yevhene commented Aug 16, 2016

@NullVoxPopuli I updated the changelog.

@NullVoxPopuli
Copy link
Contributor

cool!, thanks!

@NullVoxPopuli NullVoxPopuli merged commit 1896e5a into rails-api:master Aug 16, 2016
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.

4 participants