Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Code cleanup for ActionDispatch::Flash#call #10580

Merged
merged 1 commit into from

5 participants

@julianvargasalvarez

The nested if was replaced by using presence which takes account for
the given hash when it is nil or when it is empty. The else was
removed because what it was doing was to assign to env[KEY] the
value it already had.

actionpack/lib/action_dispatch/middleware/flash.rb
@@ -243,15 +243,9 @@ def call(env)
session = Request::Session.find(env) || {}
flash_hash = env[KEY]
- if flash_hash
- if !flash_hash.empty? || session.key?('flash')
- session["flash"] = flash_hash.to_session_value
- new_hash = flash_hash.dup
- else
- new_hash = flash_hash
- end
-
- env[KEY] = new_hash
+ if !!flash_hash.presence || session.key?('flash')
@rafaelfranca Owner

Use flash_hash.present? instead

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@julianvargasalvarez julianvargasalvarez Code cleanup for ActionDispatch::Flash#call
The nested `if` was replaced by using `presence` which takes account for
the given hash when it is `nil` or when it is empty. The `else` was
removed because what it was doing was to assign to `env[KEY]` the
value it already had.
c43ca06
@julianvargasalvarez julianvargasalvarez commented on the diff
actionpack/lib/action_dispatch/middleware/flash.rb
@@ -243,15 +243,9 @@ def call(env)
session = Request::Session.find(env) || {}
flash_hash = env[KEY]
- if flash_hash
- if !flash_hash.empty? || session.key?('flash')
- session["flash"] = flash_hash.to_session_value
- new_hash = flash_hash.dup
- else
- new_hash = flash_hash
- end
-
- env[KEY] = new_hash
+ if flash_hash.present? || session.key?('flash')
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@guilleiguaran guilleiguaran merged commit 3a0e579 into from
@rubys

I'm getting: undefined methodto_session_value' for nil:NilClass`

Full traceback:

/home/rubys/git/rails/actionpack/lib/action_dispatch/middleware/flash.rb:247:in `call'
rack (1.5.2) lib/rack/session/abstract/id.rb:225:in `context'
rack (1.5.2) lib/rack/session/abstract/id.rb:220:in `call'
/home/rubys/git/rails/actionpack/lib/action_dispatch/middleware/cookies.rb:486:in `call'
/home/rubys/git/rails/activerecord/lib/active_record/query_cache.rb:36:in `call'
/home/rubys/git/rails/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb:626:in `call'
/home/rubys/git/rails/activerecord/lib/active_record/migration.rb:366:in `call'
/home/rubys/git/rails/actionpack/lib/action_dispatch/middleware/callbacks.rb:29:in `block in call'
/home/rubys/git/rails/activesupport/lib/active_support/callbacks.rb:418:in `_run__3834361723430503168__call__callbacks'
/home/rubys/git/rails/activesupport/lib/active_support/callbacks.rb:80:in `run_callbacks'
/home/rubys/git/rails/actionpack/lib/action_dispatch/middleware/callbacks.rb:27:in `call'
/home/rubys/git/rails/actionpack/lib/action_dispatch/middleware/reloader.rb:64:in `call'
/home/rubys/git/rails/actionpack/lib/action_dispatch/middleware/remote_ip.rb:76:in `call'
/home/rubys/git/rails/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb:17:in `call'
/home/rubys/git/rails/actionpack/lib/action_dispatch/middleware/show_exceptions.rb:30:in `call'
/home/rubys/git/rails/railties/lib/rails/rack/logger.rb:38:in `call_app'
/home/rubys/git/rails/railties/lib/rails/rack/logger.rb:21:in `block in call'
/home/rubys/git/rails/activesupport/lib/active_support/tagged_logging.rb:67:in `block in tagged'
/home/rubys/git/rails/activesupport/lib/active_support/tagged_logging.rb:25:in `tagged'
/home/rubys/git/rails/activesupport/lib/active_support/tagged_logging.rb:67:in `tagged'
/home/rubys/git/rails/railties/lib/rails/rack/logger.rb:21:in `call'
/home/rubys/git/rails/actionpack/lib/action_dispatch/middleware/request_id.rb:21:in `call'
rack (1.5.2) lib/rack/methodoverride.rb:21:in `call'
rack (1.5.2) lib/rack/runtime.rb:17:in `call'
/home/rubys/git/rails/activesupport/lib/active_support/cache/strategy/local_cache.rb:83:in `call'
rack (1.5.2) lib/rack/lock.rb:17:in `call'
/home/rubys/git/rails/actionpack/lib/action_dispatch/middleware/static.rb:64:in `call'
/home/rubys/git/rails/railties/lib/rails/engine.rb:511:in `call'
/home/rubys/git/rails/railties/lib/rails/application.rb:97:in `call'
rack (1.5.2) lib/rack/lock.rb:17:in `call'
rack (1.5.2) lib/rack/content_length.rb:14:in `call'
rack (1.5.2) lib/rack/handler/webrick.rb:60:in `service'
/home/rubys/.rvm/rubies/ruby-2.0.0-p0/lib/ruby/2.0.0/webrick/httpserver.rb:138:in `service'
/home/rubys/.rvm/rubies/ruby-2.0.0-p0/lib/ruby/2.0.0/webrick/httpserver.rb:94:in `run'
/home/rubys/.rvm/rubies/ruby-2.0.0-p0/lib/ruby/2.0.0/webrick/server.rb:295:in `block in start_thread'
Collaborator

Here is the PR to fix this. #10592

@guilleiguaran

@julianvargasalvarez this is causing also a failure in Railties test suite according to TravisCI (in asset tests): https://travis-ci.org/rails/rails/jobs/7107821, can you check this?

@julianvargasalvarez

Sure, I am already working on it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on May 12, 2013
  1. @julianvargasalvarez

    Code cleanup for ActionDispatch::Flash#call

    julianvargasalvarez authored
    The nested `if` was replaced by using `presence` which takes account for
    the given hash when it is `nil` or when it is empty. The `else` was
    removed because what it was doing was to assign to `env[KEY]` the
    value it already had.
This page is out of date. Refresh to see the latest.
Showing with 3 additions and 9 deletions.
  1. +3 −9 actionpack/lib/action_dispatch/middleware/flash.rb
View
12 actionpack/lib/action_dispatch/middleware/flash.rb
@@ -243,15 +243,9 @@ def call(env)
session = Request::Session.find(env) || {}
flash_hash = env[KEY]
- if flash_hash
- if !flash_hash.empty? || session.key?('flash')
- session["flash"] = flash_hash.to_session_value
- new_hash = flash_hash.dup
- else
- new_hash = flash_hash
- end
-
- env[KEY] = new_hash
+ if flash_hash.present? || session.key?('flash')
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ session["flash"] = flash_hash.to_session_value
+ env[KEY] = flash_hash.dup
end
if (!session.respond_to?(:loaded?) || session.loaded?) && # (reset_session uses {}, which doesn't implement #loaded?)
Something went wrong with that request. Please try again.