Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Flash now on failure #26

Closed
wants to merge 3 commits into from

4 participants

@iain

Since the usual pattern is that a page gets rerendered after failure, the proper behavior would be to set the flash directly, so that it doesn't stick around if you give up and leave the page.

This is a redone version of my last pull request, with some improvements.

iain added some commits
@iain iain Set default of flash_now to :on_failure
Since the usual pattern is that a page gets rerendered after failure,
the proper behavior would be to set the flash directly, so that it
doesn't stick around if you give up and leave the page.
b706e7e
@iain iain Use a default for flash_now, enabling it to be set to false.
The previous behavior to never set flash.now can now be used again.
d36fc1a
@iain iain assert_not_flash_now should require that the flash is set
Otherwise it would return a false positive when the flash wasn't set at
all.
e98dd3c
@nadnerb

Seems to do the trick, merge please.

@did

I assume there has to be a reason why flash_now is not set by default to :on_failure. Am I right people from plataformatec ?

In my app, I write a custom responder and I re-define the set_flash_now? method within it like that:

    def set_flash_now?
      super || has_errors?
    end
@josevalim
Owner

This pull request was good. Could you please rebase? I know I am late to the party, but for some reason it seems I was not receiving notifications. :(

@iain iain referenced this pull request
Merged

Flash now on failure (rebase) #37

@josevalim josevalim closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jul 9, 2011
  1. @iain

    Set default of flash_now to :on_failure

    iain authored
    Since the usual pattern is that a page gets rerendered after failure,
    the proper behavior would be to set the flash directly, so that it
    doesn't stick around if you give up and leave the page.
  2. @iain

    Use a default for flash_now, enabling it to be set to false.

    iain authored
    The previous behavior to never set flash.now can now be used again.
  3. @iain

    assert_not_flash_now should require that the flash is set

    iain authored
    Otherwise it would return a false positive when the flash wasn't set at
    all.
This page is out of date. Refresh to see the latest.
Showing with 19 additions and 3 deletions.
  1. +2 −2 lib/responders/flash_responder.rb
  2. +17 −1 test/flash_responder_test.rb
View
4 lib/responders/flash_responder.rb
@@ -85,7 +85,7 @@ def initialize(controller, resources, options={})
@flash = options.delete(:flash)
@notice = options.delete(:notice)
@alert = options.delete(:alert)
- @flash_now = options.delete(:flash_now)
+ @flash_now = options.delete(:flash_now) { :on_failure }
end
def to_html
@@ -164,4 +164,4 @@ def flash_defaults_by_namespace(status) #:nodoc:
defaults << ""
end
end
-end
+end
View
18 test/flash_responder_test.rb
@@ -30,6 +30,11 @@ def another
respond_with(@resource, :notice => "Yes, notice this!", :alert => "Warning, warning!")
end
+ def flexible
+ options = params[:responder_options] || {}
+ respond_with(@resource, options)
+ end
+
protected
def interpolation_options
@@ -135,6 +140,16 @@ def test_sets_message_based_on_alert_key
assert_equal "Warning, warning!", flash[:alert]
end
+ def test_sets_flash_now_on_failure_by_default
+ post :another, :fail => true
+ assert_flash_now :alert
+ end
+
+ def test_never_set_flash_now
+ post :flexible, :fail => true, :responder_options => { :flash_now => false, :alert => "Warning" }
+ assert_not_flash_now :alert
+ end
+
# If we have flash.now, it's always marked as used.
def assert_flash_now(k)
assert flash.instance_variable_get(:@used).include?(k.to_sym),
@@ -142,6 +157,7 @@ def assert_flash_now(k)
end
def assert_not_flash_now(k)
+ assert flash.has_key?(k), "Expected #{k} to be set"
assert !flash.instance_variable_get(:@used).include?(k.to_sym),
"Expected #{k} to not be in flash.now, but it is."
end
@@ -169,4 +185,4 @@ def test_fallbacks_to_non_namespaced_controller_flash_message
delete :destroy
assert_equal "Successfully deleted the chosen address at Ocean Avenue", flash[:notice]
end
-end
+end
Something went wrong with that request. Please try again.