Rails core - Mar 2011 - ActionController#respond_with options don't work with :html format

Hi all, I''m wondering if this is a bug, or if I''m
misunderstanding the
code. I''m looking at the ActionController respond_with method, and
noticed that any options passed through don''t work for format.html.
For example:

class CommentsController < ApplicationController
  respond_to :html, :xml
  def new
    @comment = Comment.new
    respond_with(@comment, :status => :created)
  end
end

For html requests, the response code will be the standard :ok 200
code. But for xml request, or json, or any other acceptable request
mime type, the :created status will be returned.

The comments in actioncontroller/metal/responder.rb would lead me to
believe that this should work.

    # Display is just a shortcut to render a resource with the current
format.
    #
    #   display @user, :status => :ok
    #
    # For XML requests it''s equivalent to:
    #
    #   render :xml => @user, :status => :ok
    #
    # Options sent by the user are also used:
    #
    #   respond_with(@user, :status => :created)
    #   display(@user, :status => :ok)
    #
    # Results in:
    #
    #   render :xml => @user, :status => :created

I also was curious how this could possibly get through as a bug, so I
checked the test suite and it looks like it''s tested specifically with
a non-html request, which explains why the test would pass. But I''m
not sure why anyone would go out of their way to make the test
specifically request a non-standard mime-type unless it''s intentional
behavior. From test/controller/mime_responds_test.rb:

  def test_using_resource_with_status_and_location
    @request.accept = "text/html"
    post :using_resource_with_status_and_location
    assert @response.redirect?
    assert_equal "http://test.host/", @response.location

    @request.accept = "application/xml"
    get :using_resource_with_status_and_location
    assert_equal 201, @response.status
  end

So, should I add the test case and submit the patch, or should I
update the documentation to be clear that it doesn''t work for html
requests? Oh also, it''s not specific to just :status, it also
doesn''t
work for several other options.

On a related note, I tried using the #display method like in the
excerpted comments, but I get a wrong number of arguments error (2 for
0..1).

Thanks everyone!

-Steve Schwartz
@jangosteve

-- 
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com.
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.

> class CommentsController < ApplicationController
>  respond_to :html, :xml
>  def new
>    @comment = Comment.new
>    respond_with(@comment, :status => :created)
>  end
> end
It''s off topic, but this is a curious response code to send for new.
You haven''t created anything at all ;)
> For html requests, the response code will be the standard :ok 200
> code. But for xml request, or json, or any other acceptable request
> mime type, the :created status will be returned.
This kind of thing is to be expected because sending the
''right''
response code to a browser doesn''t lead to the correct behaviour from
a user''s perspective.  For example an API''s create action
sends a 201
response, however browsers want a 302 redirect.  You can see this kind
of thing here:

https://github.com/rails/rails/blob/master/actionpack/lib/action_controller/metal/responder.rb#L142-167
> So, should I add the test case and submit the patch, or should I
> update the documentation to be clear that it doesn''t work for html
> requests? Oh also, it''s not specific to just :status, it also
doesn''t
> work for several other options.
I believe it''s deliberate here, but if you can track down some errant
looking code, do raise a ticket with a patch.

> On a related note, I tried using the #display method like in the
> excerpted comments, but I get a wrong number of arguments error (2 for
> 0..1).
>
> Thanks everyone!
>
> -Steve Schwartz
> @jangosteve
>
> --
> You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
> To post to this group, send email to rubyonrails-core@googlegroups.com.
> To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com.
> For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.
>
>


-- 
Cheers

Koz

-- 
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com.
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.

On Mar 4, 2011, at 1:35 AM, Michael Koziarski wrote:
>> class CommentsController < ApplicationController
>>  respond_to :html, :xml
>>  def new
>>    @comment = Comment.new
>>    respond_with(@comment, :status => :created)
>>  end
>> end
>
> It''s off topic, but this is a curious response code to send for
new.
> You haven''t created anything at all ;)
Obviously, this isn''t my actual code, it was just the shortest working
example I could think of. Maybe :created was a bad example. Pretend I  
said :partial_content, or one of the other 50 status codes ;-)
>
>> For html requests, the response code will be the standard :ok 200
>> code. But for xml request, or json, or any other acceptable request
>> mime type, the :created status will be returned.
>
> This kind of thing is to be expected because sending the
''right''
> response code to a browser doesn''t lead to the correct behaviour
from
> a user''s perspective.  For example an API''s create action
sends a 201
> response, however browsers want a 302 redirect.  You can see this kind
> of thing here:
>
>
https://github.com/rails/rails/blob/master/actionpack/lib/action_controller/metal/responder.rb
> #L142-167
Not necessarily. My specific use-case is with sending an AJAX request  
(via the jquery-ujs) for an HTML snippet. The jQuery callbacks look at  
the response code, and so it is useful to have control over them  
without needing to resort to respond_to or format.html{ ... }.

Also, it''s not just :status, I can''t override :layout either.
This
doesn''t work for html requests either:

respond_with(@comment, :layout => :false)
>
>> So, should I add the test case and submit the patch, or should I
>> update the documentation to be clear that it doesn''t work for
html
>> requests? Oh also, it''s not specific to just :status, it also
doesn''t
>> work for several other options.
>
> I believe it''s deliberate here, but if you can track down some
errant
> looking code, do raise a ticket with a patch.
If it''s a bug, I have no problem writing the tests, fixing it, and  
submitting the patch. I just wanted to make sure there''s not some  
obvious reason I''m missing that this behavior would be intended.
>
>
>> On a related note, I tried using the #display method like in the
>> excerpted comments, but I get a wrong number of arguments error (2  
>> for
>> 0..1).
>>
>> Thanks everyone!
>>
>> -Steve Schwartz
>> @jangosteve
>>
>> --
>> You received this message because you are subscribed to the Google  
>> Groups "Ruby on Rails: Core" group.
>> To post to this group, send email to rubyonrails-core@googlegroups.com 
>> .
>> To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com
>> .
>> For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
>> .
>>
>>
>
>
>
> -- 
> Cheers
>
> Koz
-- 
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com.
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.

> Not necessarily. My specific use-case is with sending an AJAX request (via
> the jquery-ujs) for an HTML snippet. The jQuery callbacks look at the
> response code, and so it is useful to have control over them without
needing
> to resort to respond_to or format.html{ ... }.
I think your best bet will be to subclass the responders and remove
this ''browser vs api'' distinction.  We won''t be able
to remove those
without breaking lots of people''s apps.  Currently it''s a
feature that
you can safely say have code like this and have it redirect browsers
back to the index action.

respond_with(@comment, :status=>:created)



-- 
Cheers

Koz

-- 
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com.
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.

On Mar 4, 2011, at 2:08 AM, Michael Koziarski wrote:
> We won''t be able to remove those
> without breaking lots of people''s apps.  Currently it''s a
feature that
> you can safely say have code like this and have it redirect browsers
> back to the index action.
I see, that makes sense. At the very least, I''ll submit a patch that  
makes the :layout option work. After all, the html format is the only  
one that parameter makes sense for; if the developer puts :layout =>  
false (or some other template) in their respond_with options, it''s  
obvious they mean the html format.

respond_with(@comment, :layout => false)

Thanks Koz!

-- 
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com.
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.