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

add require for active_support/json to fix #1656 #1657

Merged
merged 1 commit into from
Apr 4, 2016
Merged

add require for active_support/json to fix #1656 #1657

merged 1 commit into from
Apr 4, 2016

Conversation

andreaseger
Copy link
Contributor

Purpose

fixes the missing require "active_support/json" mentioned in #1656

Changes

  • remove explicit require "active_support/json" from test_helper.
  • add require "active_support/json" to lib/active_model_serializers.rb

Caveats

Given all the other rails stuff required in the tests simply removing the active_support json extension from the test won't make any tests fail.

Related GitHub issues

#1656

@bf4
Copy link
Member

bf4 commented Apr 4, 2016

Might adding a changelog entry? I'd also love if you could include code snippet here showing a failure fixed by the require.

@bf4
Copy link
Member

bf4 commented Apr 4, 2016

@andreaseger Sorry, should have been more careful in checking the original issue where you included a gist

require 'bundler/inline'

gemfile true do
  source 'https://rubygems.org'
  gem 'active_model_serializers', github: 'rails-api/active_model_serializers'
  gem 'activerecord', require: false
  gem 'mysql2'
end
print "==================================================\n\n"

class Shoe
  attr_accessor :id, :name, :vendor
  def initialize(args)
    args.each{ |k,v| send("#{k}=",v)}
  end
  alias read_attribute_for_serialization public_send
end

class ShoeSerializer < ActiveModel::Serializer
  type "shoe"

  attribute :id
  attribute :name
  attribute :vendor
end

shoe = Shoe.new(id: 3, name: "samba", vendor: "adidas")

require 'json'
print ActiveModelSerializers::SerializableResource.new(shoe,
                                                       adapter:   :json_api,
                                                      ).to_json

print "\n==================================================\n"
print ActiveModelSerializers::SerializableResource.new(shoe,
                                                       adapter:   :json_api,
                                                      ).as_json.to_json

require 'active_record'
ActiveRecord::Base.establish_connection(adapter: 'mysql2', database: 'test')
print "\n==================================================\n"
print ActiveModelSerializers::SerializableResource.new(shoe,
                                                       adapter:   :json_api,
                                                      ).to_json

__END__
==================================================

[active_model_serializers] Rendered ShoeSerializer with ActiveModelSerializers::Adapter::JsonApi (0.06ms)
"#<ActiveModelSerializers::Adapter::JsonApi:0x0055ad9b412820>"
==================================================
[active_model_serializers] Rendered ShoeSerializer with ActiveModelSerializers::Adapter::JsonApi (3.09ms)
{"data":{"id":"3","type":"shoe","attributes":{"name":"samba","vendor":"adidas"}}}
==================================================
[active_model_serializers] Rendered ShoeSerializer with ActiveModelSerializers::Adapter::JsonApi (0.34ms)
{"data":{"id":"3","type":"shoe","attributes":{"name":"samba","vendor":"adidas"}}}

@bf4 bf4 merged commit af2b38c into rails-api:master Apr 4, 2016
@@ -2,6 +2,7 @@
require 'active_support'
require 'active_support/core_ext/object/with_options'
require 'active_support/core_ext/string/inflections'
require 'active_support/json'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is https://github.com/rails/rails/blob/v5.0.0.beta3/activesupport/lib/active_support/json.rb

require 'active_support/json/decoding'
require 'active_support/json/encoding'

@bf4
Copy link
Member

bf4 commented Apr 5, 2016

I wonder if adding to_json(options) to the base adapter would have been sufficient

@andreaseger
Copy link
Contributor Author

I'm not familiar what other stuff is triggered by requiring active_support/json/encoding but for what it's worth this updated gist works as well: https://gist.github.com/andreaseger/84edad45b690b142c401a170b742aafb

If you think this would be a better way to solve this I can prepare another PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants