Expand Flash Message Tests, Resolve Additional Issues, and simplify o…

…tp_set_flash_message (#96) * expand test coverage for flash messages to all currently tested controller actions; * add tests to confirm that invalid_refresh and could_not_confirm flash messages do not persist to next controller action (failing); * update invalid_refresh and could_not_confirm flash messages to render immediately too (resolves test failures); * simplify otp_set_flash_message method via existing Devise set_flash_message functionality;
wmlele · Sep 21, 2024 · 180b190 · 180b190
1 parent a75f920
commit 180b190
Show file tree

Hide file tree

Showing 9 changed files with 44 additions and 19 deletions.
diff --git a/app/controllers/devise_otp/devise/otp_credentials_controller.rb b/app/controllers/devise_otp/devise/otp_credentials_controller.rb
@@ -101,7 +101,7 @@ def done_valid_refresh
       end
 
       def failed_refresh
-        otp_set_flash_message :alert, :invalid_refresh
+        otp_set_flash_message :alert, :invalid_refresh, :now => true
         render :refresh
       end
 

diff --git a/app/controllers/devise_otp/devise/otp_tokens_controller.rb b/app/controllers/devise_otp/devise/otp_tokens_controller.rb
@@ -35,7 +35,7 @@ def update
           otp_set_flash_message :success, :successfully_updated
           redirect_to otp_token_path_for(resource)
         else
-          otp_set_flash_message :danger, :could_not_confirm
+          otp_set_flash_message :danger, :could_not_confirm, :now => true
           render :edit
         end
       end

diff --git a/lib/devise_otp_authenticatable/controllers/helpers.rb b/lib/devise_otp_authenticatable/controllers/helpers.rb
@@ -12,18 +12,8 @@ def authenticate_scope!
       #
       def otp_set_flash_message(key, kind, options = {})
         options[:scope] ||= "devise.otp.#{controller_name}"
-        options[:default] = Array(options[:default]).unshift(kind.to_sym)
-        options[:resource_name] = resource_name
-        options = devise_i18n_options(options) if respond_to?(:devise_i18n_options, true)
-        message = I18n.t("#{options[:resource_name]}.#{kind}", **options)
-
-        if message.present?
-          if options[:now]
-            flash.now[key] = message
-          else
-            flash[key] = message
-          end
-        end
+
+        set_flash_message(key, kind, options)
       end
 
       def otp_t

diff --git a/test/dummy/app/views/layouts/application.html.erb b/test/dummy/app/views/layouts/application.html.erb
@@ -8,11 +8,11 @@
 </head>
 <body>
 
-  <% if flash[:alert].present? %>
-    <div id="alert">
-      <%= flash[:alert] %>
-    </div>
-  <% end %>
+  <div id="alerts">
+    <% flash.keys.each do |key| %>
+      <%= content_tag :p, flash[key], :id => key %>
+    <% end %>
+  </div>
 
   <%= yield %>
 

diff --git a/test/integration/disable_token_test.rb b/test/integration/disable_token_test.rb
@@ -23,6 +23,9 @@ def teardown
     disable_otp
 
     assert page.has_content? "Disabled"
+    within "#alerts" do
+      assert page.has_content? 'Two-Factor Authentication has been disabled.'
+    end
 
     # logout
     sign_out

diff --git a/test/integration/enable_otp_form_test.rb b/test/integration/enable_otp_form_test.rb
@@ -20,6 +20,10 @@ def teardown
     assert_equal user_otp_token_path, current_path
     assert page.has_content?("Enabled")
 
+    within "#alerts" do
+      assert page.has_content? 'Your Two-Factor Authentication settings have been updated.'
+    end
+
     user.reload
     assert user.otp_enabled?
   end
@@ -37,6 +41,15 @@ def teardown
 
     user.reload
     assert_not user.otp_enabled?
+
+    within "#alerts" do
+      assert page.has_content? 'The Confirmation Code you entered did not match the QR code shown below.'
+    end
+
+    visit "/"
+    within "#alerts" do
+      assert !page.has_content?('The Confirmation Code you entered did not match the QR code shown below.')
+    end
   end
 
   test "a user should not be able enable their OTP authentication with a blank confirmation code" do
@@ -50,6 +63,10 @@ def teardown
 
     assert page.has_content?("To Enable Two-Factor Authentication")
 
+    within "#alerts" do
+      assert page.has_content? 'The Confirmation Code you entered did not match the QR code shown below.'
+    end
+
     user.reload
     assert_not user.otp_enabled?
   end

diff --git a/test/integration/persistence_test.rb b/test/integration/persistence_test.rb
@@ -36,6 +36,9 @@ def teardown
 
     click_link("Trust this browser")
     assert_text "Your browser is trusted."
+    within "#alerts" do
+      assert page.has_content? 'Your device is now trusted.'
+    end
     sign_out
 
     sign_user_in

diff --git a/test/integration/refresh_test.rb b/test/integration/refresh_test.rb
@@ -60,6 +60,15 @@ def teardown
     fill_in "user_refresh_password", with: "12345670"
     click_button "Continue..."
     assert_equal refresh_user_otp_credential_path, current_path
+
+    within "#alerts" do
+      assert page.has_content? 'Sorry, you provided the wrong credentials.'
+    end
+
+    visit "/"
+    within "#alerts" do
+      assert !page.has_content?('Sorry, you provided the wrong credentials.')
+    end
   end
 
   test "user should be finally be able to access their settings, and just password is enough" do

diff --git a/test/integration/reset_token_test.rb b/test/integration/reset_token_test.rb
@@ -23,6 +23,9 @@ def teardown
     reset_otp
 
     assert_equal "/users/otp/token/edit", current_path
+    within "#alerts" do
+      assert page.has_content? 'Your token secret has been reset. Please confirm your new token secret below.'
+    end
   end
 
   test "generates new token secrets" do