Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Allow default options for `as_json` method on models #3172

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants
Contributor

nhocki commented Sep 30, 2011

When you've got an AR Model and you override the as_json method, you should be able to add default options to the renderer, like this:

class User < ActiveRecord::Base
  def as_json(options = {})
    super(options.merge(:except => [:password_digest]))
  end
end

This was not possible before this commit. See the added test case.

Thanks

Fixing `as_json` method for ActiveRecord models.
When you've got an AR Model and you override the `as_json` method,
you should be able to add default options to the renderer, like this:

    class User < ActiveRecord::Base
      def as_json(options = {})
        super(options.merge(:except => [:password_digest]))
      end
    end

This was not possible before this commit. See the added test case.
Contributor

josevalim commented Sep 30, 2011

Thanks, I have been meaning to do this myself, but I believe that we should not do it on 3-1-stable. There is also code that could be improved by assuming a hash is always given.

Contributor

nhocki commented Sep 30, 2011

I think a hash should be assumed. It's how you actually add options to the renderer. I would be more than happy to look into some code that could be improved if you gave me a small hint (this is my first patch, so I'm kind of lost in the source-code).

Also, where do you think this should be merged?

Owner

guilleiguaran commented Sep 30, 2011

should be merged in master branch :)

Contributor

nhocki commented Sep 30, 2011

It would be great for this to make it to Rails 3.1.1 though (that's the reason I chose to fetch the 3-1-stable branch :P). I'll send the pull request to master later today :-)

Contributor

nhocki commented Sep 30, 2011

I didn't know how to change the target branch, so I sent the pull request #3175

Thanks!

@nhocki nhocki closed this Sep 30, 2011

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