trailblazer / roar-rails

Use Roar's representers in Rails.
http://roar.apotomo.de
MIT License
235 stars 65 forks source link

ControllerAdditions::Render#render doesn't pass options like :callback through usefully to super #render #67

Closed antifun closed 9 years ago

antifun commented 10 years ago

This might be an expectation problem, but the following does not work as I thought it would. I want to use my representer to return an object as JSON, and following (old) conventions, if a callback is specified in my request, I assume I am to return a JSONP-suitable call of that function with my JSON-represented thing. However, I can't get the roar-rails rendering paths to do this. I have to resort to explicitly calling the representer and then using render rather than any of the respond_with fanciness that roar-rails patches.

class API::V1::PublicFeedsController < ApplicationController
  include Roar::Rails::ControllerAdditions
  # If this is included, #render is hijacked and won't go through normal ActionController processing
  #include Roar::Rails::ControllerAdditions::Render

  respond_to :json

  def show
    @thing = Thing.find(123)
    if params[:callback]
      # These do not work
      # option 1 -- respond_with @thing, represent_with: ::API::ThingRepresenter, callback: params['callback']
      # option 2 -- render json: @thing, represent_with: ::API::ThingRepresenter, callback: params['callback']
      #... but this does
      render json: @thing.extend(::API::ThingRepresenter), callback: params['callback']
    else
      # don't need callback so this works okay
      respond_with @thing, represent_with: ::API::ThingRepresenter
    end
end

I think the patched render needs to pass all the remaining options besides represent_with through to super, but my attempt to have it do so was not sufficient to combine the behaviors such that option 2 above was successful. If that's the right track, I can keep pursuing it.

apotonick commented 10 years ago

Yes, I'm very sorry, the #render patch was just a, well, patch, I knew it's gonna cause problems later.

Could you add a test to https://github.com/apotonick/roar-rails/blob/master/test/render_test.rb and implement it in https://github.com/apotonick/roar-rails/blob/master/lib/roar/rails/controller_additions.rb#L66 ? Thanks :heart: