Skip to content

Commit

Permalink
defining json_key(root) as model class name
Browse files Browse the repository at this point in the history
  • Loading branch information
@joaomdmoura
joaomdmoura authored and João M. D. Moura committed Jul 23, 2015
1 parent 6266b6a commit 4187213
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion lib/active_model/serializer.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ def initialize(object, options = {})
end

def json_key
@root || self.class.root_name
@root || object.class.model_name.to_s.downcase
end

def id
Expand Down

5 comments on commit 4187213

@bolshakov
Copy link

Choose a reason for hiding this comment

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

Could you explain this change.

Before this change we used Serializer.root_name as json_key. Root name defined as follows:

    def self.root_name
      name.demodulize.underscore.sub(/_serializer$/, '') if name
    end

So for TestMovie and TestMovieSerializer:

method value
root_name test_movie
json_key testmovie

What is the aim of both methods?

@bolshakov
Copy link

Choose a reason for hiding this comment

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

It also strangely behaves with namespaces models:

class Admin::User
  include ActiveModel::Model
end 

Admin::User.model_name.to_s.downcase #=> admin::user

@dukex
Copy link

@dukex dukex commented on 4187213 Aug 5, 2015

Choose a reason for hiding this comment

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

@joaomdmoura Why this commit was necessary?

@timurvafin
Copy link

Choose a reason for hiding this comment

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

@joaomdmoura +1 for the questions.
Not sure there are other places where Serializer.root_name used right now.

@joaomdmoura
Copy link
Member Author

Choose a reason for hiding this comment

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

Just saw it today, sorry for taking so long ppl.

So, @bolshakov, on your example you simulated a best case scenario, but there are ppl using different serializers to serialize the same kind of object, in this case you would have different roots, which is kind of inconsistent, so after discussing with part o Rails team we decided to go for it.

We will still work on this in order to make it compatible with multiple conventions, it's already part of a PR.

I'm not saying we will keep it this way, we are always open to discuss it 😄

Please sign in to comment.