Skip to content

Commit

Permalink
Apply changes for turbo_stream compatibility
Browse files Browse the repository at this point in the history
  • Loading branch information
JunichiIto committed Jan 11, 2023
1 parent 6d32d24 commit 2d1ae92
Show file tree
Hide file tree
Showing 16 changed files with 60 additions and 30 deletions.
2 changes: 1 addition & 1 deletion app/controllers/devise/confirmations_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ def create
if successfully_sent?(resource)
respond_with({}, location: after_resending_confirmation_instructions_path_for(resource_name))
else
respond_with(resource)
respond_with resource, status: :unprocessable_entity
end
end

Expand Down
6 changes: 3 additions & 3 deletions app/controllers/devise/passwords_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ def create
if successfully_sent?(resource)
respond_with({}, location: after_sending_reset_password_instructions_path_for(resource_name))
else
respond_with(resource)
respond_with resource, status: :unprocessable_entity
end
end

Expand All @@ -44,10 +44,10 @@ def update
else
set_flash_message!(:notice, :updated_not_active)
end
respond_with resource, location: after_resetting_password_path_for(resource)
respond_with resource, location: after_resetting_password_path_for(resource), status: :see_other
else
set_minimum_password_length
respond_with resource
respond_with resource, status: :unprocessable_entity
end
end

Expand Down
8 changes: 4 additions & 4 deletions app/controllers/devise/registrations_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def create
else
clean_up_passwords resource
set_minimum_password_length
respond_with resource
respond_with resource, status: :unprocessable_entity
end
end

Expand All @@ -53,11 +53,11 @@ def update
set_flash_message_for_update(resource, prev_unconfirmed_email)
bypass_sign_in resource, scope: resource_name if sign_in_after_change_password?

respond_with resource, location: after_update_path_for(resource)
respond_with resource, location: after_update_path_for(resource), status: :see_other
else
clean_up_passwords resource
set_minimum_password_length
respond_with resource
respond_with resource, status: :unprocessable_entity
end
end

Expand All @@ -67,7 +67,7 @@ def destroy
Devise.sign_out_all_scopes ? sign_out : sign_out(resource_name)
set_flash_message! :notice, :destroyed
yield resource if block_given?
respond_with_navigational(resource){ redirect_to after_sign_out_path_for(resource_name) }
respond_with_navigational(resource){ redirect_to after_sign_out_path_for(resource_name), status: :see_other }
end

# GET /resource/cancel
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/devise/sessions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ def respond_to_on_destroy
# support returning empty response on GET request
respond_to do |format|
format.all { head :no_content }
format.any(*navigational_formats) { redirect_to after_sign_out_path_for(resource_name) }
format.any(*navigational_formats) { redirect_to after_sign_out_path_for(resource_name), status: :see_other }
end
end
end
2 changes: 1 addition & 1 deletion app/controllers/devise/unlocks_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ def create
if successfully_sent?(resource)
respond_with({}, location: after_sending_unlock_instructions_path_for(resource))
else
respond_with(resource)
respond_with resource, status: :unprocessable_entity
end
end

Expand Down
2 changes: 1 addition & 1 deletion app/views/devise/registrations/edit.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,6 @@

<h3>Cancel my account</h3>

<p>Unhappy? <%= button_to "Cancel my account", registration_path(resource_name), data: { confirm: "Are you sure?" }, method: :delete %></p>
<p>Unhappy? <%= button_to "Cancel my account", registration_path(resource_name), data: { confirm: "Are you sure?", turbo_confirm: "Are you sure?" }, method: :delete %></p>

<%= link_to "Back", :back %>
2 changes: 1 addition & 1 deletion app/views/devise/shared/_links.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,6 @@

<%- if devise_mapping.omniauthable? %>
<%- resource_class.omniauth_providers.each do |provider| %>
<%= link_to "Sign in with #{OmniAuth::Utils.camelize(provider)}", omniauth_authorize_path(resource_name, provider), method: :post %><br />
<%= button_to "Sign in with #{OmniAuth::Utils.camelize(provider)}", omniauth_authorize_path(resource_name, provider), data: { turbo: false } %><br />
<% end %>
<% end %>
2 changes: 1 addition & 1 deletion lib/devise.rb
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ module Test

# Which formats should be treated as navigational.
mattr_accessor :navigational_formats
@@navigational_formats = ["*/*", :html]
@@navigational_formats = ["*/*", :html, :turbo_stream]

# When set to true, signing out a user signs out all other scopes.
mattr_accessor :sign_out_all_scopes
Expand Down
4 changes: 2 additions & 2 deletions lib/devise/failure_app.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ def respond
if http_auth?
http_auth
elsif warden_options[:recall]
recall
request_format == :turbo_stream ? redirect : recall
else
redirect
end
Expand Down Expand Up @@ -167,7 +167,7 @@ def scope_url
end

def skip_format?
%w(html */*).include? request_format.to_s
%w(html */* turbo_stream).include? request_format.to_s
end

# Choose whether we should respond in an HTTP authentication fashion,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,6 @@

<h3>Cancel my account</h3>

<p>Unhappy? <%= link_to "Cancel my account", registration_path(resource_name), data: { confirm: "Are you sure?" }, method: :delete %></p>
<p>Unhappy? <%= button_to "Cancel my account", registration_path(resource_name), data: { confirm: "Are you sure?", turbo_confirm: "Are you sure?" }, method: :delete %></p>

<%= link_to "Back", :back %>
19 changes: 19 additions & 0 deletions test/failure_app_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

require 'test_helper'
require 'ostruct'
require 'minitest/mock'

class FailureTest < ActiveSupport::TestCase
class RootFailureApp < Devise::FailureApp
Expand Down Expand Up @@ -65,6 +66,10 @@ def fake_engine
end
end

class SessionStorableFailureApp < Devise::FailureApp
def store_location_for(*); end
end

class RequestWithoutFlashSupport < ActionDispatch::Request
undef_method :flash
end
Expand Down Expand Up @@ -355,6 +360,20 @@ def call_failure(env_params = {})
assert_includes @response.third.body, 'Your account is not activated yet.'
end

test 'returns to the default redirect location for turbo_stream' do
Mime::Type.register "text/vnd.turbo-stream.html", :turbo_stream
env = {
"warden.options" => { recall: "devise/sessions#new", attempted_path: "/users/sign_in" },
"devise.mapping" => Devise.mappings[:user],
"warden" => stub_everything,
"formats" => Mime[:turbo_stream],
app: SessionStorableFailureApp
}
call_failure(env)
assert_equal 302, @response.first
assert_equal 'http://test.host/users/sign_in', @response.second['Location']
end

if Rails.application.config.respond_to?(:relative_url_root)
test 'calls the original controller with the proper environment considering the relative url root' do
swap Rails.application.config, relative_url_root: "/sample" do
Expand Down
4 changes: 2 additions & 2 deletions test/integration/authenticatable_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ class AuthenticationSanityTest < Devise::IntegrationTest

delete destroy_admin_session_path
assert_response :redirect
assert_redirected_to root_path
assert_see_other_to root_path

get root_path
assert_contain 'Signed out successfully'
Expand All @@ -129,7 +129,7 @@ class AuthenticationSanityTest < Devise::IntegrationTest
test 'unauthenticated admin set message on sign out' do
delete destroy_admin_session_path
assert_response :redirect
assert_redirected_to root_path
assert_see_other_to root_path

get root_path
assert_contain 'Signed out successfully'
Expand Down
12 changes: 8 additions & 4 deletions test/integration/omniauthable_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -126,15 +126,19 @@ def stub_action!(name)
end
end

test "generates a link to authenticate with provider" do
test "generates a button to authenticate with provider" do
visit "/users/sign_in"
assert_select "a[href=?][data-method='post']", "/users/auth/facebook", text: "Sign in with FaceBook"
assert_select "form[action=?]", "/users/auth/facebook" do
assert_select "input[value=?]", "Sign in with FaceBook"
end
end

test "generates a proper link when SCRIPT_NAME is set" do
test "generates a proper button when SCRIPT_NAME is set" do
header 'SCRIPT_NAME', '/q'
visit "/users/sign_in"
assert_select "a[href=?][data-method='post']", "/q/users/auth/facebook", text: "Sign in with FaceBook"
assert_select "form[action=?]", "/q/users/auth/facebook" do
assert_select "input[value=?]", "Sign in with FaceBook"
end
end

test "handles callback error parameter according to the specification" do
Expand Down
12 changes: 6 additions & 6 deletions test/integration/recoverable_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ def reset_password(options = {}, &block)
fill_in 'email', with: '[email protected]'
end

assert_response :success
assert_response :unprocessable_entity
assert_current_url '/users/password'
assert_have_selector "input[type=email][value='[email protected]']"
assert_contain 'not found'
Expand All @@ -102,7 +102,7 @@ def reset_password(options = {}, &block)
fill_in 'email', with: ' [email protected] '
end

assert_response :success
assert_response :unprocessable_entity
assert_current_url '/users/password'
assert_have_selector "input[type=email][value=' [email protected] ']"
assert_contain 'not found'
Expand Down Expand Up @@ -132,7 +132,7 @@ def reset_password(options = {}, &block)
fill_in 'email', with: '[email protected]'
end

assert_response :success
assert_response :unprocessable_entity
assert_current_url '/users/password'
assert_have_selector "input[type=email][value='[email protected]']"
assert_contain 'not found'
Expand All @@ -156,7 +156,7 @@ def reset_password(options = {}, &block)
user = create_user
reset_password reset_password_token: 'invalid_reset_password'

assert_response :success
assert_response :unprocessable_entity
assert_current_url '/users/password'
assert_have_selector '#error_explanation'
assert_contain %r{Reset password token(.*)invalid}
Expand All @@ -170,7 +170,7 @@ def reset_password(options = {}, &block)
fill_in 'Confirm new password', with: 'other_password'
end

assert_response :success
assert_response :unprocessable_entity
assert_current_url '/users/password'
assert_have_selector '#error_explanation'
assert_contain "Password confirmation doesn't match Password"
Expand All @@ -192,7 +192,7 @@ def reset_password(options = {}, &block)
request_forgot_password

reset_password { fill_in 'Confirm new password', with: 'other_password' }
assert_response :success
assert_response :unprocessable_entity
assert_have_selector '#error_explanation'
refute user.reload.valid_password?('987654321')

Expand Down
4 changes: 2 additions & 2 deletions test/integration/timeoutable_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ def last_request_at
delete destroy_user_session_path

assert_response :redirect
assert_redirected_to root_path
assert_see_other_to root_path
follow_redirect!
assert_contain 'Signed out successfully'
end
Expand All @@ -109,7 +109,7 @@ def last_request_at
follow_redirect!

assert_response :success
assert_contain 'Sign in'
assert_contain 'Log in'
refute warden.authenticated?(:user)
end

Expand Down
7 changes: 7 additions & 0 deletions test/support/http_method_compatibility.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,13 @@ class IntegrationTest < ActionDispatch::IntegrationTest
end
end
end

def assert_see_other_to(url)
assert_equal 303, @integration_session.status,
"Expected status to be 303, got #{@integration_session.status}"

assert_url url, @integration_session.headers["Location"]
end
end

class ControllerTestCase < ActionController::TestCase
Expand Down

0 comments on commit 2d1ae92

Please sign in to comment.