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

False positive for Rails/ActionControllerFlashBeforeRender when redirecting in ensure #915

Closed
dacook opened this issue Jan 17, 2023 · 2 comments

Comments

@dacook
Copy link

dacook commented Jan 17, 2023

When using an ensure block to redirect (see below controller code), this cop raises an error.
I think it's different, but could be related to: #909

  def destroy
    if label_icon.destroy
      flash[:success] = "Label Icon deleted: #{label_icon.name}."
    end
  rescue ActiveRecord::InvalidForeignKey => e
    flash[:error] = "Cannot delete #{label_icon.name}, remove from all packing categories first." 
  ensure
    redirect_to admin_label_icons_path # <- This action never renders, because of this redirect
  end

Expected behavior

Rubocop pass

Actual behavior

C: [Correctable] Rails/ActionControllerFlashBeforeRender: Use flash.now before render.
    flash[:error] = "Cannot delete #{label_icon.name}, remove from all packing categories first."
    ^^^^^

Steps to reproduce the problem

Run rubocop on the above code.

RuboCop version

bundle exec rubocop -V
1.39.0 (using Parser 3.1.2.1, rubocop-ast 1.23.0, running on ruby 3.1.2) [x86_64-darwin19]
  - rubocop-rails 2.17.3
@zvkemp
Copy link

zvkemp commented Jan 30, 2023

There is a similar false positive when using a conditional followed by rescue:

  context 'when using `flash` before a `redirect_to` in a conditional branch followed by render & rescue' do
    it 'does not register an offense' do
      expect_no_offenses(<<~RUBY)
        class HomeController < ApplicationController
          def create
            if condition?
              flash[:notice] = "success"
              redirect_to login_path
            else
              flash.now[:error] = "conditional error"
              render :new
            end
          rescue ActionController::ParameterMissing
            flash.now[:form_errors] = "param error"
            render :new
          end
        end
      RUBY
    end
  end

@koic
Copy link
Member

koic commented Mar 29, 2023

This issue has been resolved by latest RuboCop Rails. Please upgrade to the latest version of RuboCop Rails. Thank you.

@koic koic closed this as completed Mar 29, 2023
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

No branches or pull requests

3 participants