Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the submitter’s formmethod not being respected when using Rails 7 #button form helper #445

Merged
merged 1 commit into from Jun 20, 2023

Conversation

feliperaul
Copy link
Contributor

After upgrading turbo-rails from 7.1.0 to 7.3.0, code like this broke:

<%= form_with(...) do |f| %>
  <%= f.button formaction: destroy_all_articles_path, formmethod: :delete } do %>Delete all articles<% end %>

Before the upgrade, turbo correctly issued a request with method POST, and the body containing a _method key with the delete value:

image

This made Rails correctly route it with a delete verb on the backend.

After the upgrade, however, the _method key in the body is lost:

image

I could trace back this to https://github.com/hotwired/turbo-rails/pull/370/files from @seanpdoyle.

That code carefully looks for the submitter's formmethod attribute. However, there's an extra piece in this puzzle: Rails 7 ActionView::Helpers::FormBuilder#button has an override for formmethod if the submitter does not have the name or value attributes set, which is the default.

# File actionview/lib/action_view/helpers/form_helper.rb, line 2609
def button(value = nil, options = {}, &block)
  # ommited 

  formmethod = options[:formmethod]
  if formmethod.present? && !/post|get/i.match?(formmethod) && !options.key?(:name) && !options.key?(:value)
    options.merge! formmethod: :post, name: "_method", value: formmethod
  end

  @template.button_tag(value, options)
end

This means that if you use <%= f.button formmethod: :delete %>, it will generate a <button name="_method" value="delete" formmethod="post">.

So, on Rails 7, looking at the submitters formmethod is not enough, because the HTML in the page will already have swapped the formmethod defined by the user to post, setting the desired formmethod in the value field instead.

Therefore, this PR fixes the bug by checking if the submitter's name is already _method, in which case the submitter's value attribute will contain the actually desired method.

Lastly, I need help finishing the test suite. I couldn't understand exactly how to make so that the tests use the file app/javascript/turbo/fetch_requests.js file, because it seems to use the app/assets/javascripts/turbo.js file instead? And that file seems to be turbo + turbo-rails combined? Couldn't figure this one out!

@seanpdoyle Maybe you could help me finish this? Thanks in advance!

Mentioning hotwired/turbo#564 because it's related.

…ctionView::Helpers::FormBuilder#button formmethod option.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants