Merge pull request #5548 from heartcombo/ca-turbo

Integrate with Hotwire/Turbo by configuring error and response statuses
heartcombo · Feb 9, 2023 · 8e2e3f6 · 8e2e3f6
2 parents d0f0853 + 31c4f31
commit 8e2e3f6
Show file tree

Hide file tree

Showing 25 changed files with 146 additions and 32 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,27 @@
 
 * enhancements
   * Add support for Ruby 3.1/3.2.
+  * Add support for Hotwire + Turbo, default in Rails 7+.
+    * Devise uses the latest `responders` version (v3.1.0 or higher), which allows configuring the status used for validation error responses (`error_status`) and for redirects after POST/PUT/PATCH/DELETE requests (`redirect_status`). For backwards compatibility, Devise keeps `error_status` as `:ok` which returns a `200 OK` response, and `redirect_status` to `:found` which returns a `302 Found` response, but you can configure it to return `422 Unprocessable Entity` and `303 See Other` respectively, to match the behavior expected by Hotwire/Turbo:
+
+      ```ruby
+      # config/initializers/devise.rb
+      Devise.setup do |config|
+        # ...
+        config.responder.error_status = :unprocessable_entity
+        config.responder.redirect_status = :see_other
+        # ...
+      end
+      ```
+
+      These configs are already generated by default with new apps, and existing apps may opt-in as described above. Trying to set these with an older version of `responders` will issue a warning and have no effect, so please upgrade the `responders` version if you're upgrading Devise for this integration. Note that these defaults may change in future versions of Devise, to better match the Rails + Hotwire/Turbo defaults across the board.
+    * If you have a custom responder set on your application and expect it to affect Devise as well, you may need to override the Devise responder entirely with `config.responder = MyApplicationResponder`, so that it uses your custom one. The main reason Devise uses a custom responder is to be able to configure the statuses as described above, but you can also change that config on your own responder if you want. Check the `responders` readme for more info on that.
+    * If you have created a custom responder and/or failure app just to customize responses for better Hotwire/Turbo integration, they should no longer be necessary.
+    * `:turbo_stream` is now treated as a navigational format, so it works like HTML navigation when using Turbo. Note: if you relied on `:turbo_stream` to be treated as a non-navigational format before, you can reconfigure your `navigational_formats` in the Devise initializer file to exclude it.
+    * OmniAuth "Sign in with" links were changed to buttons that generate HTML forms with method=POST, instead of using link + method=POST that required rails-ujs to work. Since rails-ujs is no longer the default for new Rails apps, this allows the OmniAuth buttons to work in any scenario, with or without rails-ujs and/or Turbo. This only affects apps that are using the default `devise/shared/_links.html.erb` partial from Devise with OmniAuth enabled.
+    * The "Cancel my account" button was changed to include the `data-turbo-confirm` option, so that it works with both rails-ujs and Turbo by default.
+    * Devise does not provide "sign out" links/buttons in its shared views, but if you're using `sign_out_via` with `:delete` (the default), and are using links with `method: :delete`, those need to be updated with `data: { turbo_method: :delete }` instead for Turbo.
+    * Check [this upgrade guide](https://github.com/heartcombo/devise/wiki/How-To:-Upgrade-to-Devise-4.9.0-[Hotwire-Turbo-integration]) for more detailed information.
 
 ### 4.8.1 - 2021-12-16
 

diff --git a/Gemfile b/Gemfile
@@ -11,7 +11,7 @@ gem "rdoc"
 
 gem "rails-controller-testing", github: "rails/rails-controller-testing"
 
-gem "responders", "~> 3.0"
+gem "responders", "~> 3.1"
 
 group :test do
   gem "nokogiri", "< 1.13"

diff --git a/Gemfile.lock b/Gemfile.lock
@@ -10,7 +10,7 @@ GIT
 PATH
   remote: .
   specs:
-    devise (4.8.1)
+    devise (4.9.0.alpha)
       bcrypt (~> 3.0)
       orm_adapter (~> 0.1)
       railties (>= 4.1.0)
@@ -189,9 +189,9 @@ GEM
     rake (13.0.6)
     rdoc (6.5.0)
       psych (>= 4.0.0)
-    responders (3.0.1)
-      actionpack (>= 5.0)
-      railties (>= 5.0)
+    responders (3.1.0)
+      actionpack (>= 5.2)
+      railties (>= 5.2)
     rexml (3.2.5)
     ruby-openid (2.9.2)
     ruby2_keywords (0.0.5)
@@ -231,7 +231,7 @@ DEPENDENCIES
   rails (~> 7.0.0)
   rails-controller-testing!
   rdoc
-  responders (~> 3.0)
+  responders (~> 3.1)
   rexml
   sqlite3 (~> 1.4)
   timecop

diff --git a/README.md b/README.md
@@ -474,6 +474,36 @@ Please note: You will still need to add `devise_for` in your routes in order to
 devise_for :users, skip: :all
 ```
 
+### Hotwire/Turbo
+
+Devise integrates with Hotwire/Turbo by treating such requests as navigational, and configuring certain responses for errors and redirects to match the expected behavior. New apps are generated with the following response configuration by default, and existing apps may opt-in by adding the config to their Devise initializers:
+
+```ruby
+Devise.setup do |config|
+  # ...
+  # When using Devise with Hotwire/Turbo, the http status for error responses
+  # and some redirects must match the following. The default in Devise for existing
+  # apps is `200 OK` and `302 Found respectively`, but new apps are generated with
+  # these new defaults that match Hotwire/Turbo behavior.
+  # Note: These might become the new default in future versions of Devise.
+  config.responder.error_status = :unprocessable_entity
+  config.responder.redirect_status = :see_other
+end
+```
+
+**Important**: these custom responses require the `responders` gem version to be `3.1.0` or higher, please make sure you update it if you're going to use this configuration. Check [this upgrade guide](https://github.com/heartcombo/devise/wiki/How-To:-Upgrade-to-Devise-4.9.0-[Hotwire-Turbo-integration]) for more info.
+
+_Note_: the above statuses configuration may become the default for Devise in a future release.
+
+There are a couple other changes you might need to make in your app to work with Hotwire/Turbo, if you're migrating from rails-ujs:
+
+* The `data-confirm` option that adds a confirmation modal to buttons/forms before submission needs to change to `data-turbo-confirm`, so that Turbo handles those appropriately.
+* The `data-method` option that sets the request method for link submissions needs to change to `data-turbo-method`. This is not necessary for `button_to` or `form`s since Turbo can handle those.
+
+If you're setting up Devise to sign out via `:delete`, and you're using links (instead of buttons wrapped in a form) to sign out with the `method: :delete` option, they will need to be updated as described above. (Devise does not provide sign out links/buttons in its shared views.)
+
+Make sure to inspect your views looking for those, and change appropriately.
+
 ### I18n
 
 Devise uses flash messages with I18n, in conjunction with the flash keys :notice and :alert. To customize your app, you can set up your locale file:

diff --git a/app/controllers/devise/confirmations_controller.rb b/app/controllers/devise/confirmations_controller.rb
@@ -27,6 +27,7 @@ def show
       set_flash_message!(:notice, :confirmed)
       respond_with_navigational(resource){ redirect_to after_confirmation_path_for(resource_name, resource) }
     else
+      # TODO: use `error_status` when the default changes to `:unprocessable_entity`.
       respond_with_navigational(resource.errors, status: :unprocessable_entity){ render :new }
     end
   end

diff --git a/app/controllers/devise/registrations_controller.rb b/app/controllers/devise/registrations_controller.rb
@@ -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: Devise.responder.redirect_status }
   end
 
   # GET /resource/cancel

diff --git a/app/controllers/devise/sessions_controller.rb b/app/controllers/devise/sessions_controller.rb
@@ -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: Devise.responder.redirect_status }
     end
   end
 end
diff --git a/app/controllers/devise/unlocks_controller.rb b/app/controllers/devise/unlocks_controller.rb
@@ -29,6 +29,7 @@ def show
       set_flash_message! :notice, :unlocked
       respond_with_navigational(resource){ redirect_to after_unlock_path_for(resource) }
     else
+      # TODO: use `error_status` when the default changes to `:unprocessable_entity`.
       respond_with_navigational(resource.errors, status: :unprocessable_entity){ render :new }
     end
   end

diff --git a/app/controllers/devise_controller.rb b/app/controllers/devise_controller.rb
@@ -15,6 +15,7 @@ class DeviseController < Devise.parent_controller.constantize
   end
 
   prepend_before_action :assert_is_devise_resource!
+  self.responder = Devise.responder
   respond_to :html if mimes_for_respond_to.empty?
 
   # Override prefixes to consider the scoped view.

diff --git a/app/views/devise/registrations/edit.html.erb b/app/views/devise/registrations/edit.html.erb
@@ -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 %>
diff --git a/app/views/devise/shared/_error_messages.html.erb b/app/views/devise/shared/_error_messages.html.erb
@@ -1,5 +1,5 @@
 <% if resource.errors.any? %>
-  <div id="error_explanation">
+  <div id="error_explanation" data-turbo-cache="false">
     <h2>
       <%= I18n.t("errors.messages.not_saved",
                  count: resource.errors.count,

diff --git a/app/views/devise/shared/_links.html.erb b/app/views/devise/shared/_links.html.erb
@@ -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 %>
diff --git a/devise.gemspec b/devise.gemspec
@@ -32,4 +32,11 @@ Gem::Specification.new do |s|
   s.add_dependency("bcrypt", "~> 3.0")
   s.add_dependency("railties", ">= 4.1.0")
   s.add_dependency("responders")
+
+  s.post_install_message = %q{
+[DEVISE] Please review the [changelog] and [upgrade guide] for more info on Hotwire / Turbo integration.
+
+  [changelog] https://github.com/heartcombo/devise/blob/main/CHANGELOG.md
+  [upgrade guide] https://github.com/heartcombo/devise/wiki/How-To:-Upgrade-to-Devise-4.9.0-%5BHotwire-Turbo-integration%5D
+  }
 end
diff --git a/gemfiles/Gemfile-rails-6-0 b/gemfiles/Gemfile-rails-6-0
@@ -9,7 +9,7 @@ gem "rdoc"
 
 gem "rails-controller-testing", github: "rails/rails-controller-testing"
 
-gem "responders", "~> 3.0"
+gem "responders", "~> 3.1"
 
 group :test do
   gem "nokogiri", "< 1.13"

diff --git a/gemfiles/Gemfile-rails-6-1 b/gemfiles/Gemfile-rails-6-1
@@ -9,7 +9,7 @@ gem "rdoc"
 
 gem "rails-controller-testing", github: "rails/rails-controller-testing"
 
-gem "responders", "~> 3.0"
+gem "responders", "~> 3.1"
 
 if RUBY_VERSION >= "3.1"
   gem "net-smtp", require: false

diff --git a/gemfiles/Gemfile-rails-main b/gemfiles/Gemfile-rails-main
@@ -9,7 +9,7 @@ gem "rdoc"
 
 gem "rails-controller-testing", github: "rails/rails-controller-testing"
 
-gem "responders", "~> 3.0"
+gem "responders", "~> 3.1"
 
 group :test do
   gem "nokogiri", "< 1.13"

diff --git a/lib/devise.rb b/lib/devise.rb
@@ -23,6 +23,7 @@ module Devise
   module Controllers
     autoload :Helpers,        'devise/controllers/helpers'
     autoload :Rememberable,   'devise/controllers/rememberable'
+    autoload :Responder,      'devise/controllers/responder'
     autoload :ScopedViews,    'devise/controllers/scoped_views'
     autoload :SignInOut,      'devise/controllers/sign_in_out'
     autoload :StoreLocation,  'devise/controllers/store_location'
@@ -217,7 +218,16 @@ module Test
 
   # Which formats should be treated as navigational.
   mattr_accessor :navigational_formats
-  @@navigational_formats = ["*/*", :html]
+  @@navigational_formats = ["*/*", :html, :turbo_stream]
+
+  # The default responder used by Devise, used to customize status codes with:
+  #
+  #   `config.responder.error_status`
+  #   `config.responder.redirect_status`
+  #
+  # Can be replaced by a custom application responder.
+  mattr_accessor :responder
+  @@responder = Devise::Controllers::Responder
 
   # When set to true, signing out a user signs out all other scopes.
   mattr_accessor :sign_out_all_scopes

diff --git a/lib/devise/controllers/responder.rb b/lib/devise/controllers/responder.rb
@@ -0,0 +1,35 @@
+# frozen_string_literal: true
+
+module Devise
+  module Controllers
+    # Custom Responder to configure default statuses that only apply to Devise,
+    # and allow to integrate more easily with Hotwire/Turbo.
+    class Responder < ActionController::Responder
+      if respond_to?(:error_status=) && respond_to?(:redirect_status=)
+        self.error_status = :ok
+        self.redirect_status = :found
+      else
+        # TODO: remove this support for older Rails versions, which aren't supported by Turbo
+        # and/or responders. It won't allow configuring a custom response, but it allows Devise
+        # to use these methods and defaults across the implementation more easily.
+        def self.error_status
+          :ok
+        end
+
+        def self.redirect_status
+          :found
+        end
+
+        def self.error_status=(*)
+          warn "[DEVISE] Setting the error status on the Devise responder has no effect with this " \
+            "version of `responders`, please make sure you're using a newer version. Check the changelog for more info."
+        end
+
+        def self.redirect_status=(*)
+          warn "[DEVISE] Setting the redirect status on the Devise responder has no effect with this " \
+            "version of `responders`, please make sure you're using a newer version. Check the changelog for more info."
+        end
+      end
+    end
+  end
+end
diff --git a/lib/devise/failure_app.rb b/lib/devise/failure_app.rb
@@ -71,7 +71,9 @@ def recall
       end
 
       flash.now[:alert] = i18n_message(:invalid) if is_flashing_format?
-      self.response = recall_app(warden_options[:recall]).call(request.env)
+      self.response = recall_app(warden_options[:recall]).call(request.env).tap { |response|
+        response[0] = Rack::Utils.status_code(Devise.responder.error_status)
+      }
     end
 
     def redirect
@@ -167,7 +169,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,

diff --git a/lib/devise/version.rb b/lib/devise/version.rb
@@ -1,5 +1,5 @@
 # frozen_string_literal: true
 
 module Devise
-  VERSION = "4.8.1".freeze
+  VERSION = "4.9.0.alpha".freeze
 end
diff --git a/lib/generators/templates/devise.rb b/lib/generators/templates/devise.rb
@@ -256,14 +256,14 @@
 
   # ==> Navigation configuration
   # Lists the formats that should be treated as navigational. Formats like
-  # :html, should redirect to the sign in page when the user does not have
+  # :html should redirect to the sign in page when the user does not have
   # access, but formats like :xml or :json, should return 401.
   #
   # If you have any extra navigational formats, like :iphone or :mobile, you
   # should add them to the navigational formats lists.
   #
   # The "*/*" below is required to match Internet Explorer requests.
-  # config.navigational_formats = ['*/*', :html]
+  # config.navigational_formats = ['*/*', :html, :turbo_stream]
 
   # The default HTTP method used to sign out a resource. Default is :delete.
   config.sign_out_via = :delete
@@ -296,12 +296,14 @@
   # so you need to do it manually. For the users scope, it would be:
   # config.omniauth_path_prefix = '/my_engine/users/auth'
 
-  # ==> Turbolinks configuration
-  # If your app is using Turbolinks, Turbolinks::Controller needs to be included to make redirection work correctly:
-  #
-  # ActiveSupport.on_load(:devise_failure_app) do
-  #   include Turbolinks::Controller
-  # end
+  # ==> Hotwire/Turbo configuration
+  # When using Devise with Hotwire/Turbo, the http status for error responses
+  # and some redirects must match the following. The default in Devise for existing
+  # apps is `200 OK` and `302 Found respectively`, but new apps are generated with
+  # these new defaults that match Hotwire/Turbo behavior.
+  # Note: These might become the new default in future versions of Devise.
+  config.responder.error_status = :unprocessable_entity
+  config.responder.redirect_status = :see_other
 
   # ==> Configuration for :registerable
 

diff --git a/lib/generators/templates/simple_form_for/registrations/edit.html.erb b/lib/generators/templates/simple_form_for/registrations/edit.html.erb
@@ -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 %>
diff --git a/test/integration/omniauthable_test.rb b/test/integration/omniauthable_test.rb
@@ -128,13 +128,17 @@ def stub_action!(name)
 
   test "generates a link 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=?][method=post]", "/users/auth/facebook" do
+      assert_select "input[type=submit][value=?]", "Sign in with FaceBook"
+    end
   end
 
   test "generates a proper link 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=?][method=post]", "/q/users/auth/facebook" do
+      assert_select "input[type=submit][value=?]", "Sign in with FaceBook"
+    end
   end
 
   test "handles callback error parameter according to the specification" do

diff --git a/test/integration/timeoutable_test.rb b/test/integration/timeoutable_test.rb
@@ -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
 

diff --git a/test/support/integration.rb b/test/support/integration.rb
@@ -61,8 +61,8 @@ def sign_in_as_admin(options = {}, &block)
   # account Middleware redirects.
   #
   def assert_redirected_to(url)
-    assert_includes [301, 302], @integration_session.status,
-           "Expected status to be 301 or 302, got #{@integration_session.status}"
+    assert_includes [301, 302, 303], @integration_session.status,
+           "Expected status to be 301, 302, or 303, got #{@integration_session.status}"
 
     assert_url url, @integration_session.headers["Location"]
   end