Skip to content

Fix DoubleRenderError in update action and simplify display_errors - #700

Open
Solus90 wants to merge 1 commit into
RefugeRestrooms:developfrom
Solus90:fix/update-double-render-and-display-errors
Open

Fix DoubleRenderError in update action and simplify display_errors#700
Solus90 wants to merge 1 commit into
RefugeRestrooms:developfrom
Solus90:fix/update-double-render-and-display-errors

Conversation

@Solus90

@Solus90 Solus90 commented Mar 2, 2026

Copy link
Copy Markdown

What

This PR fixes two bugs in RestroomsController:

Bug 1: AbstractController::DoubleRenderError in update

In the update action, when a restroom fails validation during an edit, the code called both render 'edit' and then fell through to the unconditional redirect_to @restroom below it. This raises an AbstractController::DoubleRenderError at runtime.

Before:

else
  display_errors
  render 'edit'
end

redirect_to @restroom  # always executes — crashes when render already called

After:

else
  display_errors
  render 'edit'
  return  # prevents fall-through to redirect_to
end

redirect_to @restroom

Bug 2: Redundant iteration in display_errors

The display_errors helper iterated over @restroom.errors.each do but had no block parameter. On every iteration it simply overwrote the same flash[:alert] key with the same value — a no-op loop that just sets the same string repeatedly and then discards it.

Before:

@restroom.errors.each do
  flash[:alert] = I18n.t('restroom.flash.field')
end

After:

flash[:alert] = I18n.t('restroom.flash.field')

Impact

  • Editing an existing restroom and submitting invalid data would previously crash with a DoubleRenderError instead of showing the edit form with an error message.
  • No behaviour change for valid submissions or voting actions.

In the  action, when restroom validation fails,
was called but execution continued to the unconditional  below it, raising an AbstractController::DoubleRenderError.
Added an explicit  after  to prevent this.

In , the  block had no block
parameter and simply overwrote the same flash alert key on every
iteration. Simplified to set the flash message once directly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant