Skip to content

Fix rule edit form deleting the rule instead of updating it - #95

Merged
akouryy merged 1 commit into
cookpad:masterfrom
akouryy:akouryy-prevent-accidental-rule-deletion
Jul 24, 2026
Merged

akouryy merged 1 commit into
cookpad:masterfrom
akouryy:akouryy-prevent-accidental-rule-deletion

Conversation

@akouryy

@akouryy akouryy commented Jul 24, 2026

Copy link
Copy Markdown
Member

Currently, submitting the rule edit form deletes the rule instead of updating it.

The Delete button is a button_to, which renders its own form, so this nests a form in another form.
In such cases browsers drop the inner <form> and make its hidden input _method=delete belong to the parent.
Therefore, every submission of the rule edit form sends a DELETE request.

To fix this, this PR moves the Delete and Transfer buttons out of the edit form.
A negative margin keeps them on the same row as the Update button, but is optional. If you don't care about visual compatibility, I can remove it.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes an unintended DELETE request when submitting the rule edit form by eliminating nested forms caused by using button_to (which generates its own <form>) inside the main edit <form>.

Changes:

  • Moves the “Transfer” link and “Delete” button_to out of the main rule edit form to prevent nested-form behavior from hijacking _method=delete.
  • Adjusts layout to keep action buttons visually aligned (currently via a negative top margin on the new action row).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

.column.is-2= button_to "Delete", rule_path(@rule), method: :delete, data: { confirm: %(Delete "#{@rule.name}"?) }, class: "button is-danger"

- if @rule.persisted?
.columns{style: "margin-top: -4rem"}

@nekketsuuu nekketsuuu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll leave the design up to you.

@akouryy
akouryy merged commit c40f1ed into cookpad:master Jul 24, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants