Create DefaultSerializer so that as_json uses same interface. #207

stantona · 2013-02-10T17:44:59Z

When adding acive_model_serializers as a gem in an exiting project, I noticed that it broke existing functionality, particularly when responding with an array of objects:

respond_with users

where User overrides as_json.

I found that ArraySerializers was circumventing this method calling directly into serializable_hash.

I took the idea of an old pull request (that was never merged in) and applied DefaultSerializer which simply calls into object's as_json when serializable_hash is called. This also provides a common interface.

All tests pass.

I'm still not entirely sure if this is the right way of doing this, or what the implications are, but suffice to say this fixed my particular problem.

This is to ensure that PORO's as_json is called if no serializer is specified. Original behaviour was that serializable_hash was being called, overriding the as_json method.

steveklabnik · 2013-02-10T19:37:37Z

active_model_serializers.gemspec

@@ -20,5 +20,6 @@ Gem::Specification.new do |gem|
  gem.add_dependency 'activemodel', '>= 3.0'
  gem.add_development_dependency "rails", ">= 3.0"
  gem.add_development_dependency "pry"
+  gem.add_development_dependency "pry-nav"


Why'd this get added?

oversight, it helped with some debugging. I'll remove it.
On Feb 10, 2013 11:37 AM, "Steve Klabnik" notifications@github.com wrote:

In active_model_serializers.gemspec:

@@ -20,5 +20,6 @@ Gem::Specification.new do |gem|
gem.add_dependency 'activemodel', '>= 3.0'
gem.add_development_dependency "rails", ">= 3.0"
gem.add_development_dependency "pry"

gem.add_development_dependency "pry-nav"

Why'd this get added?

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/207/files#r2955823..

steveklabnik · 2013-02-10T19:38:18Z

All tests pass, although I had to rearrange the order of fields in a hash to make one particular test pass.

This kinda bugs me, though it may be an issue with the test, and not with your feature.

stantona · 2013-02-10T19:51:42Z

Yeah this bugs me too. Have to dig deeper here.
On Feb 10, 2013 11:38 AM, "Steve Klabnik" notifications@github.com wrote:

All tests pass, although I had to rearrange the order of fields in a hash
to make one particular test pass.

This kinda bugs me, though it may be an issue with the test, and not with
your feature.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/207#issuecomment-13359416..

stantona · 2013-02-11T15:14:00Z

@steveklabnik I removed the pry-nav gem in the gemspec. And after looking at the test again this morning, I realized that as suspected the order of the fields in the hash wasn't a problem. At one point I mistakenly thought I was dealing with strings and not hashes. I restored it to original order and all tests pass.

steveklabnik · 2013-02-11T18:02:48Z

Thanks! I wonder why the jruby tests are failing: https://travis-ci.org/rails-api/active_model_serializers/jobs/4723457

stantona · 2013-02-12T06:54:09Z

@steveklabnik hmm looks like it's failing when using Gemfile.edge. I get the same result on the master branch on my local machine. Need to dig deeper.

Merge remote-tracking branch 'fblee/master'

…_serializer Fix rendering nil with custom serializer

'embed_key' option to allow embedding by attributes other than IDs

steveklabnik · 2013-03-06T00:06:48Z

This doesn't merge cleanly any more, can you take a look? The Jruby thing shouldn't be an issue.

stantona · 2013-03-06T00:08:15Z

Sure I'll take a look sometime this evening. Thanks.

On 5 March 2013 16:06, Steve Klabnik notifications@github.com wrote:

This doesn't merge cleanly any more, can you take a look? The Jruby thing
shouldn't be an issue.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/207#issuecomment-14474157
.

Conflicts: lib/active_model/array_serializer.rb

stantona · 2013-03-06T16:53:31Z

@steveklabnik Merged.

steveklabnik · 2013-03-06T19:27:33Z

Now there's a bunch of test failures. :(

In the future, please rebase rather than merge master back in. ❤️

stantona · 2013-03-06T23:11:20Z

@steveklabnik Ahh, the test failed in earlier versions because of a line break. I fixed it and it seems to be good now. And yep, I'll rebase in the future, cheers.

Create DefaultSerializer so that as_json uses same interface.

steveklabnik · 2013-03-06T23:26:43Z

Thank you! Duck typing ftw! ❤️

LeeMallabone and others added 3 commits February 6, 2013 17:32

Add failing test to show behavior of custom serializer with nils.

d35999a

Create DefaultSerializer so that as_json uses same interface.

8f6218c

This is to ensure that PORO's as_json is called if no serializer is specified. Original behaviour was that serializable_hash was being called, overriding the as_json method.

Move DefaultSerializer from array_serializer.rb to serializer.rb

c4fcf96

steveklabnik reviewed Feb 10, 2013
View reviewed changes

stantona added 2 commits February 11, 2013 06:03

Remove pry-nav from gemspec.

e470b09

Restore order of fields in hash to original order.

b4616d7

Michi Huber and others added 5 commits February 26, 2013 09:57

test cases by fblee

9f93f34

Merge remote-tracking branch 'fblee/master'

serialization test: respect root settings when root object is nil

58a063e

Don't fail if object is nil, render null

c7a420d

Merge pull request #213 from michihuber/fix_rendering_nil_with_custom…

06c6587

…_serializer Fix rendering nil with custom serializer

Merge pull request #198 from jredburn/master

dc22579

'embed_key' option to allow embedding by attributes other than IDs

steveklabnik and others added 2 commits March 5, 2013 17:07

bumping to 0.7.0

fabdc62

Merge branch 'create-default-serializer' into HEAD

da26ebd

Conflicts: lib/active_model/array_serializer.rb

Remove line break on the conditional statement.

5a55a68

steveklabnik added a commit that referenced this pull request Mar 6, 2013

Merge pull request #207 from stantona/create-default-serializer

7b86838

Create DefaultSerializer so that as_json uses same interface.

steveklabnik merged commit 7b86838 into rails-api:master Mar 6, 2013

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create DefaultSerializer so that as_json uses same interface. #207

Create DefaultSerializer so that as_json uses same interface. #207

stantona commented Feb 10, 2013

steveklabnik Feb 10, 2013

stantona Feb 10, 2013

steveklabnik commented Feb 10, 2013

stantona commented Feb 10, 2013

stantona commented Feb 11, 2013

steveklabnik commented Feb 11, 2013

stantona commented Feb 12, 2013

steveklabnik commented Mar 6, 2013

stantona commented Mar 6, 2013

stantona commented Mar 6, 2013

steveklabnik commented Mar 6, 2013

stantona commented Mar 6, 2013

steveklabnik commented Mar 6, 2013

Create DefaultSerializer so that as_json uses same interface. #207

Create DefaultSerializer so that as_json uses same interface. #207

Conversation

stantona commented Feb 10, 2013

steveklabnik Feb 10, 2013

Choose a reason for hiding this comment

stantona Feb 10, 2013

Choose a reason for hiding this comment

steveklabnik commented Feb 10, 2013

stantona commented Feb 10, 2013

stantona commented Feb 11, 2013

steveklabnik commented Feb 11, 2013

stantona commented Feb 12, 2013

steveklabnik commented Mar 6, 2013

stantona commented Mar 6, 2013

stantona commented Mar 6, 2013

steveklabnik commented Mar 6, 2013

stantona commented Mar 6, 2013

steveklabnik commented Mar 6, 2013