Docs: Update changelog for 0.10.3

[jaredbeck]

I hope this will help someone else to upgrade.

Demonstration of breaking change

class Banana < ApplicationRecord
end
# There is no BananaSerializer

0.10.2

render json: @banana # => {"id":1,"name":"a banana"}

0.10.3

render json: @banana # => {}

[mention-bot]

@jaredbeck, thanks for your PR! By analyzing the history of the files in this pull request, we identified @bf4, @groyoh and @CorainChicago to be potential reviewers.

[bf4]

@jaredbeck Thanks for the bug report. That would be a good thing to get fixed before 0.10.5

[jaredbeck]

@jaredbeck Thanks for the bug report. That would be a good thing to get fixed before 0.10.5

Oh! I didn't know I was writing a bug report! I thought this change was intentional. Maybe we could add still add it to the changelog, but under a "Known issues" heading instead of under "Breaking changes"?

[bf4]

Yup!

[bf4]

@jaredbeck was there a reason you thought it was intentional?

[jaredbeck]

Yup!

OK, I'll push a new commit with that change.

@jaredbeck was there a reason you thought it was intentional?

Nah, no reason.

[jaredbeck]

@jaredbeck was there a reason you thought it was intentional?

Nah, no reason.

Oh wait, yeah, there was a reason. The docs (howto/upgrade_from_0_8_to_0_10.md) say:

No default serializer when serializer doesn't exist
https://github.com/rails-api/active_model_serializers/blob/master/docs/howto/upgrade_from_0_8_to_0_10.md

[jaredbeck]

Maybe we could add still add it to the changelog, but under a "Known issues" heading instead of under "Breaking changes"?

Yup!

OK, I'll push a new commit with that change.

New commit pushed, thanks.

[bf4]

@jaredbeck

ah, what that should mean, is that we just call as_json on anything that doesn't have a serializer.

# From a comment in https://github.com/rails-api/active_model_serializers/pull/2026/files
     # BUG: per #2027, JSON API resource relationships are only id and type, and hence either
     # *require* a serializer or we need to be a little clever about figuring out the id/type.
     # In either case, returning the raw virtual value will almost always be incorrect.
     options[:virtual_value] = object.try(:as_json) || object

[bf4]

And see https://github.com/rails-api/active_model_serializers/tree/a9d533d916e64c8fb585a3b19c2cbbc8405a6f05#primitive-handling

[jaredbeck]

ah, what that should mean, is that we just call as_json on anything that doesn't have a serializer.

I've added that, thanks.

[bf4]

If you could add a failing test, that would probably make the case easier to understand. I have an idea what might have causes the regression, but haven't tried to duplicate it.

[bf4]

Calling as_json when no serializer is found isn't new. What's new, is that you've describe a regression/change in render json: MyModel.new

[jaredbeck]

I got the test suite running, but I'm having trouble writing a test that reproduces this issue. Here's what I tried.

diff --git a/test/action_controller/serialization_test.rb b/test/action_controller/serialization_test.rb
index dfd72b4..07fc4c2 100644
--- a/test/action_controller/serialization_test.rb
+++ b/test/action_controller/serialization_test.rb
@@ -5,6 +5,10 @@ module ActionController
     class ImplicitSerializerTest < ActionController::TestCase
       class ImplicitSerializationTestController < ActionController::Base
         include SerializationTesting
+        def render_model_without_serializer
+          render json: ModelWithoutSerializer.new(id: 1, name: 'Alice')
+        end
+
         def render_using_implicit_serializer
           @profile = Profile.new(name: 'Name 1', description: 'Description 1', comments: 'Comments 1')
           render json: @profile
@@ -144,6 +148,13 @@ module ActionController
 
       tests ImplicitSerializationTestController
 
+      def test_render_model_without_serializer
+        get :render_model_without_serializer
+        expected = { id: 1, name: 'Alice' }
+        assert_equal 'application/json', @response.content_type
+        assert_equal expected.to_json, @response.body
+      end
+
       # We just have Null for now, this will change
       def test_render_using_implicit_serializer
         get :render_using_implicit_serializer
diff --git a/test/fixtures/poro.rb b/test/fixtures/poro.rb
index 6245ad2..563703c 100644
--- a/test/fixtures/poro.rb
+++ b/test/fixtures/poro.rb
@@ -223,3 +223,12 @@ class PaginatedSerializer < ActiveModel::Serializer::CollectionSerializer
     'paginated'
   end
 end
+
+# This model has no serializer. Specifically, there is no constant defined
+# named `ModelWithoutSerializerSerializer`
+class ModelWithoutSerializer < Model
+  attributes :id, :name
+end
+if defined?(ModelWithoutSerializerSerializer)
+  fail 'Expected to be undefined: ModelWithoutSerializerSerializer'
+end

Maybe this is only a problem in 0.10.3 and was fixed in 0.10.4? Or maybe there's something wrong with my test?

[bf4]

@jaredbeck maybe better to make an AR class first to test comparison? What Rails and Ruby versions was this on? What JSON version?

[bf4]

I tried making a test, but couldn't duplicate on earlier versions bundle exec m test/action_controller/regression_test.rb

require "test_helper"

module ActionController
  module Serialization
    class ImplicitSerializerTest < ActionController::TestCase
      class PostWithoutSerializer < ActiveRecord::Base
        self.table_name = :posts
      end
      class ImplicitSerializationTestController < ActionController::Base
        def render_record_without_serializer
          @post = PostWithoutSerializer.new(title: "Title", body: "Body")
          render json: @post
        end
      end

      tests ImplicitSerializationTestController

      def test_record_without_serializer
        with_adapter :json do
          get :render_record_without_serializer
        end

        expected = {}
        PostWithoutSerializer.column_names.each do |field| expected[field] ||= nil end
        expected.update(
          title: "Title",
          body: "Body"
        )

        assert_equal "application/json", @response.content_type
        assert_equal expected.to_json, @response.body
      end
    end
  end
end

I also failed to find a failure via bisecting git bisect run bundle check || bundle >/dev/null; COVERAGE_MINIMUM=0.0 bundle exec ruby -Ilib:test test/action_controller/regression_test.rb

or even

git rev-list --reverse v0.10.2..v0.10.4 | while read rev; do echo "Running $rev"; bundle check || bundle >/dev/null; COVERAGE_MINIMUM=0.0 bundle exec ruby -Ilib:test test/action_controller/regression_test.rb || exit 1; done

[jaredbeck]

Benjamin, thanks for looking into this. I'm convinced now that my initial description was over-simplistic; there's something else going on. For now, I have a work-around: always define a serializer, and that's good enough for now. I'm going to close this, thanks again for your help.

[bf4]

@jaredbeck I'll be happy to help debug if it's reproducible, if you want to screen share/ pair sometime.

[jaredbeck]

@jaredbeck I'll be happy to help debug if it's reproducible, if you want to screen share/ pair sometime.

Thanks Benjamin, that's very generous. I can probably find a point in my app's commit history where it's reproducible, but let me spend some time making sure it's not my app doing something weird before I take up any more of your time.