Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

When serialising a class, specify the type of any singular associations,... #7858

Closed
wants to merge 1 commit into from

6 participants

@cliochris

When serialising a class, specify the type of any singular associations, if necessary. Rails already correctly specifies the :type of any enumerable association (e.g. a has_many association), but made no attempt to do so for non-enumerables (e.g. a has_one association).

We must specify the :type of any polymorphic association. A has_one association to a class which uses single-table inheritance is an example of a polymorphic association.

Fixes #7471

@cliochris cliochris When serialising a class, specify the type of any singular associatio…
…ns, if

necessary. Rails already correctly specifies the :type of any enumerable
association (e.g. a has_many association), but made no attempt to do so for
non-enumerables (e.g. a has_one association).
We must specify the :type of any polymorphic association. A has_one
association to a class which uses single-table inheritance is an example of
a polymorphic association.
Fixes #7471
e7f7e35
@acapilleri

@cliochris thanks for you PR
cc / @rafaelfranca

@rafaelfranca rafaelfranca commented on the diff
activemodel/lib/active_model/serializers/xml.rb
@@ -140,7 +140,9 @@ def add_associations(association, records, opts)
end
else
merged_options[:root] = association.to_s
- records.to_xml(merged_options)
+ association_name = association.to_s.tableize.singularize
@rafaelfranca Owner

This need a refactoring.

association_name = association.to_s
record_class = record.class

merged_options[:root] = association_name

if record_class.to_s.underscore == association_name.tableize.singularize
  merged_options[:type] = record_class.name
end

records.to_xml(merged_options)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@rafaelfranca
Owner

Is this an issue in the master branch? If it is please open the pull request pointing to master and I'll backport to 3-2-stable.

@al2o3cr

A minor semantic issue with the commit message - describing a has_one that points to STI records as "polymorphic" is confusing, since isn't implemented as a polymorphic association (with :as / :polymorphic). Not sure what a better term would be, though...

@steveklabnik
Collaborator

Agreed. Just say STI instead.

@frodsan

@cliochris any news on this?

@cliochris

Sorry, I've been busy on other work. I'll try to get to this over the weekend. I understand @al2o3cr's comment about "polymorphic" being confusing, but note that it is NOT just STI that is affected, as shown in the unit test. On the other hand, STI is what lead me to the bug in the first place.

@cliochris

Sorry, unable to make further progress on this in 2012. Probably not a big deal as the issue has been outstanding since I reported it in February, 2011.

@steveklabnik
Collaborator

I will take a look at updating this, then. Thanks for letting us know. Leaving this open till I put my money where my mouth is ;)

@steveklabnik
Collaborator

I have submitted #8352 which supercedes this. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Oct 5, 2012
  1. @cliochris

    When serialising a class, specify the type of any singular associatio…

    cliochris authored
    …ns, if
    
    necessary. Rails already correctly specifies the :type of any enumerable
    association (e.g. a has_many association), but made no attempt to do so for
    non-enumerables (e.g. a has_one association).
    We must specify the :type of any polymorphic association. A has_one
    association to a class which uses single-table inheritance is an example of
    a polymorphic association.
    Fixes #7471
This page is out of date. Refresh to see the latest.
View
4 activemodel/lib/active_model/serializers/xml.rb
@@ -140,7 +140,9 @@ def add_associations(association, records, opts)
end
else
merged_options[:root] = association.to_s
- records.to_xml(merged_options)
+ association_name = association.to_s.tableize.singularize
@rafaelfranca Owner

This need a refactoring.

association_name = association.to_s
record_class = record.class

merged_options[:root] = association_name

if record_class.to_s.underscore == association_name.tableize.singularize
  merged_options[:type] = record_class.name
end

records.to_xml(merged_options)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ record_type = { :type => ((records.class.to_s.underscore == association_name) ? nil : records.class.name) }
+ records.to_xml merged_options.merge(record_type)
end
end
View
12 activemodel/test/cases/serializers/xml_serialization_test.rb
@@ -7,12 +7,12 @@ class Contact
extend ActiveModel::Naming
include ActiveModel::Serializers::Xml
- attr_accessor :address, :friends
+ attr_accessor :address, :friends, :contact
remove_method :attributes if method_defined?(:attributes)
def attributes
- instance_values.except("address", "friends")
+ instance_values.except("address", "friends", "contact")
end
end
@@ -57,6 +57,9 @@ def setup
@contact.address.state = "CA"
@contact.address.zip = 11111
@contact.friends = [Contact.new, Contact.new]
+ @related_contact = SerializableContact.new
+ @related_contact.name = "related"
+ @contact.contact = @related_contact
end
test "should serialize default root" do
@@ -205,4 +208,9 @@ def setup
assert_match %r{<friends>}, xml
assert_match %r{<friend>}, xml
end
+
+ test "association with sti" do
+ xml = @contact.to_xml(:include => :contact)
+ assert xml.include?(%(<contact type="SerializableContact">))
+ end
end
Something went wrong with that request. Please try again.