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

Specify type of singular assication during serialization #8352

Merged
merged 1 commit into from Nov 28, 2012

Conversation

steveklabnik
Copy link
Member

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 STI association. A has_one
association to a class which uses single-table inheritance is an example of
this type of association.

Fixes #7471

@steveklabnik
Copy link
Member Author

@rafaelfranca I did a different refactoring than you did, mine is even simpler, I think. I used the 1.9 hash syntax, let me know if you want it in 1.8 so you can cherry-pick a backport.

records.to_xml(merged_options)

unless records.class.to_s.underscore == association.to_s
merged_options[:type] = records.class.name
Copy link
Member

Choose a reason for hiding this comment

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

wrong, indentation

Copy link
Member Author

Choose a reason for hiding this comment

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

duuuuuur

@rafaelfranca
Copy link
Member

Why you removed all these tests?

assert_match %r{<address>}, xml
assert_match %r{<apt-number type="integer">}, xml
test "association with sti" do
xml = @contact.to_xml(:include => :contact)
Copy link
Member

Choose a reason for hiding this comment

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

Ruby 1.9 syntax

@steveklabnik
Copy link
Member Author

Because I suck at git. Ugh. I'll fix that now.

@@ -238,7 +241,6 @@ def to_ary
xml = @contact.to_xml :dasherize => false, :include => :address, :indent => 0
assert_match %r{<apt_number type="integer">}, xml
end

Copy link
Member

Choose a reason for hiding this comment

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

This line should not be removed

Copy link
Member Author

Choose a reason for hiding this comment

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

Already fixed

@rafaelfranca
Copy link
Member

Sorry, I forgot to ask the CHANGELOG entry

@steveklabnik
Copy link
Member Author

Doing it now. I figured I'd wait till you were happy with the patch itself.

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 STI association. A has_one
association to a class which uses single-table inheritance is an example of
this type of association.

Fixes rails#7471
@steveklabnik
Copy link
Member Author

Done.

rafaelfranca added a commit that referenced this pull request Nov 28, 2012
Specify type of singular assication during serialization
@rafaelfranca rafaelfranca merged commit 4c99d08 into rails:master Nov 28, 2012
@rafaelfranca
Copy link
Member

❤️ 💚 💙 💛 💜

@steveklabnik
Copy link
Member Author

🤘 ❤️

@steveklabnik
Copy link
Member Author

Should I open a PR to backport are are you going to?

@yegct
Copy link
Contributor

yegct commented Nov 29, 2012

Thank you guys very much for your work here. I appreciate it.

@steveklabnik
Copy link
Member Author

Thanks for kicking it off!

rafaelfranca added a commit that referenced this pull request Nov 29, 2012
Specify type of singular assication during serialization
Conflicts:
	activemodel/CHANGELOG.md
	activemodel/test/cases/serializers/xml_serialization_test.rb
@rafaelfranca
Copy link
Member

I already did yesterday but forget to push. Done now.

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.

Rails 3.2.8 ActiveModel::Serializers::Xml::Serializer ignores has_one sti
3 participants