"data-turbo-method: delete" should use POST with _method rather than DELETE

[stuart-leitch]

When an <a> link attributed with 'data-turbo-method: delete' is clicked turbo intercepts the click and issues a DELETE request to the server.

It's likely that the Rails controller will delete a record and then issue a redirect_to to a different page. The default status code for redirect_to is 302. This causes an issue with turbo, as it then issues the subsequent request while retaining the http method (i.e. DELETE). This can be avoided by explicitly issuing the redirect as a 303 e.g. redirect_to root_path, status: 303

An example sequence of requests illustrating the issue is:

Started GET "/articles/8" for ::1 at 2021-10-11 21:57:00 +0100
Completed 200 OK in 7ms (Views: 5.2ms | ActiveRecord: 0.1ms | Allocations: 3097)

Started DELETE "/articles/8" for ::1 at 2021-10-11 21:57:09 +0100
Redirected to http://localhost:3000/
Completed 302 Found in 7ms (ActiveRecord: 2.2ms | Allocations: 2079)

Started DELETE "/" for ::1 at 2021-10-11 21:57:09 +0100
ActionController::RoutingError (No route matches [DELETE] "/"):

Started GET "/articles/8" for ::1 at 2021-10-11 21:57:09 +0100
Completed 404 Not Found in 1ms (ActiveRecord: 0.1ms | Allocations: 870)

That DELETE "/" gives me the heebie-jeebies 😲.

We can't change the default redirect status in Rails, but don't want to have to remember to set the status code each time.

Proposed solution is to leave (base) turbo as is, and modify turbo-rails to use the POST w/_method approach. Other backends will need to respond with a 303. The POST w/_method approach is Rails specific.

See discussion here: https://github.com/rails/rails/pull/43430

I think the method convertLinkWithMethodClickToFormSubmission needs to be extended to handle this case. I'm really not sure how that will be done to avoid issues when base turbo is subsequently upgraded. Watching with interest!

[Intrepidd]

This could be a Turbo configuration option that is automatically turned on when using turbo-rails ?

[dhh]

I'm thinking something similar to what was done for confirm_method. So Turbo doesn't need to know about the Rails specific POST/_method trick.

[ghiculescu]

For reference, the data-turbo-confirm behaviour is added here https://github.com/hotwired/turbo/pull/379

[grncdr]

We can't change the default redirect status in Rails, but don't want to have to remember to set the status code each time.

What about a including a controller concern that overrides redirect_to if the request accepts Mime[:turbo_stream]?

This is what I'm using in an application I'm upgrading:

# frozen_string_literal: true

module Turbo
  module SafeRedirection
    extend ActiveSupport::Concern

    def redirect_to(url = {}, options = {})
      result = super

      if !request.get? && options[:turbo] != false && request.accepts.include?(Mime[:turbo_stream])
        self.status = 303
      end

      result
    end
  end
end

The above is adapted from the Turbo::Redirection concern recommended for UJS compatibility in the upgrading guide (which I'm also using) but I intend to leave it in place even after all UJS code is gone from my project.

Would it be reasonable to add that to turbo-rails and include it in ActionController::Base?

[ybakos]

Thanks for all things Rails. I'm excited to learn more about Hotwire etc.

I am observing the same behavior described in this issue, but with PATCH methods, so I thought I would add what I can.

I had this older Rails 5 app with the following routes:

  namespace :admin do
    resources :scholarships do
      patch 'publish', on: :member
      patch 'unpublish', on: :member

In my view, I generated "publish" and "unpublish" links like so:

= link_to 'unpublish', unpublish_admin_scholarship_path(@scholarship), method: :patch

= link_to 'publish', publish_admin_scholarship_path(@scholarship), method: :patch

Worked fine in Rails 5, the anchor tags incorporated the data-method="patch" property and Rails manages submitting a PATCH request to /admin/scholarships/42/publish and /admin/scholarships/42/unpublish.

I have custom actions in the controller:

  def update
    #...
  end

  def publish
    #...
  end

  def unpublish
    #...
  end

They do the usual "if update redirect here otherwise redirect there" idiom. Nothing fancy.

After upgrading to Rails 7, the links stopped working. I learned that we should now use data: { 'turbo-method': :patch } instead of method: :patch.

https://stackoverflow.com/questions/70474422/rails-7-link-to-with-method-delete-still-performs-get-request

https://github.com/heartcombo/devise/issues/5439

I am now witnessing weird behavior. The "unpublish" link half-works and the "publish" link does not. In the case of "publish," the log shows the request is sent to #update.

In the case of the "publish" link, the logs shows one PATCH request for #publish:

Started PATCH "/admin/scholarships/22/publish" for ::1 at 2022-02-07 22:50:38 -0800
Processing by Admin::ScholarshipsController#publish as TURBO_STREAM

Followed by twenty (!) more PATCH requests to #update. They look like this:

Started PATCH "/admin/scholarships/22" for ::1 at 2022-02-07 22:50:38 -0800
Processing by Admin::ScholarshipsController#update as TURBO_STREAM

Yes, there are twenty of them. In addition, the view does not update.

However, when I click the "unpublish" link, the view does update. That said, I see one PATCH request to #unpublish:

Started PATCH "/admin/scholarships/22/unpublish" for ::1 at 2022-02-07 22:53:24 -0800
Processing by Admin::ScholarshipsController#unpublish as TURBO_STREAM

Followed by one PATCH request to #update:

Started PATCH "/admin/scholarships/22" for ::1 at 2022-02-07 22:53:24 -0800
Processing by Admin::ScholarshipsController#update as TURBO_STREAM

Followed by a GET request which was generated by the logic in #publish

Started GET "/admin/scholarships/22" for ::1 at 2022-02-07 22:53:25 -0800
Processing by Admin::ScholarshipsController#show as HTML

Was I misinformed about replacing data-method="patch" with data: { 'turbo-method': :patch }?

Why does one link not work at all and generates 20 additional requests, while the other link kinda works and does not generate the 20 requests but still generates the extra PATCH #update request?

[dwightwatson]

@hfpp2012 You don't need to do that. You just replace data-confirm with data-turbo-confirm:data: {turbo_confirm: "Are you sure?}.

@ybakos What are the redirects like on those actions? Turbo will follow redirects and use the same HTTP method where Turbolinks would follow the redirect with HTTP GET. You may need to adjust the redirect to use status: :see_other.

[ybakos]

@dwightwatson They all redirect to the same view the link appears on. The source is below. But wait! Do I need to use respond_to blocks to play nicely with turbo out of the box? You'll notice that, in this app, I'm not. (!)

  def update
    @scholarship = Scholarship.find(params[:id])
    if @scholarship.updatable_by? current_user
      if @scholarship.update(scholarship_params)
        redirect_to [:admin, @scholarship], notice: 'Scholarship updated.'
      else
        render :edit
      end
    else
      redirect_to [:admin, @scholarship], alert: 'Could not update this scholarship.'
    end
  end

  def publish
    @scholarship = Scholarship.find(params[:id])
    if @scholarship.update(published: true)
      redirect_to [:admin, @scholarship], notice: 'Scholarship published, and is now visible to applicants.'
    else
      redirect_to [:admin, @scholarship], alert: 'Could not publish this scholarship.'
    end
  end

  def unpublish
    @scholarship = Scholarship.find(params[:id])
    if @scholarship.update(published: false)
      redirect_to [:admin, @scholarship], notice: 'Scholarship unpublished, and is not visible to applicants.'
    else
      redirect_to [:admin, @scholarship], alert: 'Could not unpublish this scholarship.'
    end
  end

[viktorianer]

You do not need respond_to, but you have to add status: :see_other to your redirect_to.

See this comment:

@hfpp2012 You don't need to do that. You just replace data-confirm with data-turbo-confirm:data: {turbo_confirm: "Are you sure?}.

@ybakos What are the redirects like on those actions? Turbo will follow redirects and use the same HTTP method where Turbolinks would follow the redirect with HTTP GET. You may need to adjust the redirect to use status: :see_other.

[brunoprietog]

Why can't the redirect status be changed to 303 as default when using turbo-rails? This is not intuitive at all for newbies.

[brunoprietog]

I think #370 solves this. Anyway, why can't make redirect_to redirect with 303 status instead of 302? Thanks

[Laykou]

I agree having an option to change redirect_to to use 303 - See Other by default would be great.

I see also Rails docs mentions this issue: https://api.rubyonrails.org/v7.0.3.1/classes/ActionController/Redirecting.html

If you are using XHR requests other than GET or POST and redirecting after the request then some browsers will follow the redirect using the original request method. This may lead to undesirable behavior such as a double DELETE. To work around this you can return a 303 See Other status code which will be followed using a GET request.