Skip to content
Permalink
Browse files
Stringify the incoming hash in FlashHash 
Stringify the incoming as well to handle incoming symbol keys from
marshalled sessions
  • Loading branch information
@guilleiguaran @chancancode
guilleiguaran authored and chancancode committed Feb 11, 2014
1 parent a6ce984 commit a668beffd64106a1e1fedb71cc25eaaa11baf0c1
Showing 1 changed file with 3 additions and 1 deletion.
4 actionpack/lib/action_dispatch/middleware/flash.rb
View file
@@ -1,3 +1,5 @@
require 'active_support/core_ext/hash/keys'

module ActionDispatch
class Request < Rack::Request
# Access the contents of the flash. Use <tt>flash["notice"]</tt> to
@@ -94,7 +96,7 @@ def to_session_value

def initialize(flashes = {}, discard = []) #:nodoc:
@discard = Set.new(stringify_array(discard))
@flashes = flashes
@flashes = flashes.stringify_keys
@now = nil
end

11 comments on commit a668bef

@senny
Copy link
Member

@senny senny commented on a668bef Mar 4, 2014

Choose a reason for hiding this comment

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

Just upgraded my app to the RC and noticed that Symbol keys changed to String keys. Is this expected behavior? It should be documented in the upgrading guide at least. This will break apps for sure.

@kuldeepaggarwal
Copy link
Contributor

Choose a reason for hiding this comment

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

@senny This won't break any app, as if we try to access notice like flash[:notice], then

def [](k)
  @flash[k.to_s]
end

will save you... 😄

@guilleiguaran
Copy link
Member Author

Choose a reason for hiding this comment

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

@kuldeepaggarwal it breaks apps that were iterating over the keys and checking for specific keys, for example:

<% flash.keys.each do |key, value| %>
    <span class="<%= (key == :notice) ? 'green' : 'red' %>"><%= value %></span>
<% end %>

@senny
Copy link
Member

@senny senny commented on a668bef Mar 5, 2014

Choose a reason for hiding this comment

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

@kuldeepaggarwal flash access works but the keys need to be stored in some sort. Before they were symbols and now they are Strings. If you have applications that work with the internal flash representation (like the example of @guilleiguaran) you get different behavior than on Rails 4. I know there are many tutorials and even gems out there that suggest looping through the flash. So this is a compatibility issue.

It's easy to fix though so we need to make sure the user knows what the cause is an how to proceed.

@kuldeepaggarwal
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry I missed that...

@kuldeepaggarwal
Copy link
Contributor

Choose a reason for hiding this comment

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

@senny So should we update the guide?

@chancancode
Copy link
Member

@chancancode chancancode commented on a668bef Mar 9, 2014 via email

Choose a reason for hiding this comment

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

@chancancode
Copy link
Member

@chancancode chancancode commented on a668bef Mar 9, 2014 via email

Choose a reason for hiding this comment

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

@senny
Copy link
Member

@senny senny commented on a668bef Mar 9, 2014

Choose a reason for hiding this comment

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

i updated the upgrading guide. will take care of changelog/ release guide sync next week.

@kuldeepaggarwal
Copy link
Contributor

Choose a reason for hiding this comment

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

@senny 👍

@chancancode
Copy link
Member

@chancancode chancancode commented on a668bef Mar 9, 2014 via email

Choose a reason for hiding this comment

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

Please sign in to comment.