Skip to content
This repository

ActiveModel::Serializers::Xml should extend ActiveModel::Naming #7874

Closed
wants to merge 1 commit into from

4 participants

Omer Jakobinsky Justin George Francesco Rodríguez Jeremy Kemper
Omer Jakobinsky

Assuming a native Ruby class named Foo which includes only the following module ActiveModel::Serializers::Xml. If we call #to_xml on an instance of Foo we would get the following exception NoMethodError (undefined method 'model_name'...

One way to solve this issue would be to also make sure Foo also extend ActiveModel::Naming. Another way, which appear to be more robust, and more importantly, consistent with the rest of the codebase (i.e. already used in ActiveModel::Serializers::JSON) would be extend ActiveModel::Naming the moment ActiveModel::Serializers::Xml is included.

Justin George

Can you add a test for this? looks like a good patch otherwise.

Francesco Rodríguez
Collaborator

Indeed. This is failing in master.

[16] pry(main)> class User2
[16] pry(main)*   include ActiveModel::Serializers::Xml
[16] pry(main)*   attr_accessor :name
[16] pry(main)*   
[16] pry(main)*   def attributes
[16] pry(main)*     instance_values
[16] pry(main)*   end  
[16] pry(main)* end  
=> nil
[17] pry(main)> user2 = User2.new
=> #<User2:0x007fccdaab6db0>
[18] pry(main)> user2.name = 'Francesco'
=> "Francesco"
[19] pry(main)> user2.to_xml
NoMethodError: undefined method `model_name' for User2:Class
from /Users/frodsan/Code/rails/activemodel/lib/active_model/serializers/xml.rb:78:in `serialize'

[6] pry(main)> class User
[6] pry(main)*   include ActiveModel::Serializers::JSON
[6] pry(main)*   attr_accessor :name
[6] pry(main)*   
[6] pry(main)*   def attributes
[6] pry(main)*     instance_values
[6] pry(main)*   end  
[6] pry(main)* end  
=> nil
[7] pry(main)> user = User.new
=> #<User:0x007fccdab83f90>
[8] pry(main)> user.name = 'Francesco'
=> "Francesco"
[9] pry(main)> user.to_json
=> "{\"name\":\"Francesco\"}"

@Omer please, add a test for this.

Francesco Rodríguez
Collaborator

This was done in 94f6b3d.

Jeremy Kemper jeremy closed this October 26, 2012
Omer Jakobinsky

Sorry it took me so long to reply. The important thing is that a fix has been merged.

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

Showing 1 unique commit by 1 author.

Oct 08, 2012
Omer Jakobinsky ActiveModel::Serializers::Xml should extend ActiveModel::Naming e68b53b
This page is out of date. Refresh to see the latest.
4  activemodel/lib/active_model/serializers/xml.rb
@@ -10,6 +10,10 @@ module Xml
10 10
       extend ActiveSupport::Concern
11 11
       include ActiveModel::Serialization
12 12
 
  13
+      included do
  14
+        extend ActiveModel::Naming
  15
+      end
  16
+
13 17
       class Serializer #:nodoc:
14 18
         class Attribute #:nodoc:
15 19
           attr_reader :name, :value, :type
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.