Return 303 on redirect for Turbo#5410
Closed
coder2000 wants to merge 2 commits intoheartcombo:mainfrom
Closed
Conversation
Turbo uses the DELETE verb directly when using `data-turbo-method: :delete` in rails so the redirect fails when returning the standard status of 302. This will return a 303 which will force the verb to change to GET. Should not affect any other usage with UJS.
fix: return status 303 for redirect
Contributor
|
This PR helped my issue. Thank you very much. However, I had to add def respond_to_on_destroy
# We actually need to hardcode this as Rails default responder doesn't
# 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), status: 303, notice: find_message(:signed_out) }
end
endAnd the same issue can happen def destroy
resource.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), status: 303, notice: find_message(:destroyed) }
end |
|
I think this is still relevant, as seen here #5458 |
carlosantoniodasilva
added a commit
that referenced
this pull request
Jan 31, 2023
Treat `:turbo_stream` request format as a navigational format, much like HTML, so Devise/responders can work properly. Allow configuring the `error_status` and `redirect_status` using the latest responders features, via a new custom Devise responder, so we can customize the both responses to match Hotwire/Turbo behavior, for example with `422 Unprocessable Entity` and `303 See Other`, respectively. The defaults aren't changing in Devise itself (yet), so it still responds on errors cases with `200 OK`, and redirects on non-GET requests with `302 Found`, but new apps are generated with the new statuses and existing apps can opt-in. Please note that these defaults might change in a future release of Devise. PRs/Issues references: #5545 #5529 #5516 #5499 #5487 #5467 #5440 #5410 #5340 #5542 #5530 #5519 #5513 #5478 #5468 #5463 #5458 #5448 #5446 #5439
4 tasks
carlosantoniodasilva
added a commit
that referenced
this pull request
Jan 31, 2023
Treat `:turbo_stream` request format as a navigational format, much like HTML, so Devise/responders can work properly. Allow configuring the `error_status` and `redirect_status` using the latest responders features, via a new custom Devise responder, so we can customize the both responses to match Hotwire/Turbo behavior, for example with `422 Unprocessable Entity` and `303 See Other`, respectively. The defaults aren't changing in Devise itself (yet), so it still responds on errors cases with `200 OK`, and redirects on non-GET requests with `302 Found`, but new apps are generated with the new statuses and existing apps can opt-in. Please note that these defaults might change in a future release of Devise. PRs/Issues references: #5545 #5529 #5516 #5499 #5487 #5467 #5440 #5410 #5340 #5542 #5530 #5519 #5513 #5478 #5468 #5463 #5458 #5448 #5446 #5439
carlosantoniodasilva
added a commit
that referenced
this pull request
Jan 31, 2023
Treat `:turbo_stream` request format as a navigational format, much like HTML, so Devise/responders can work properly. Allow configuring the `error_status` and `redirect_status` using the latest responders features, via a new custom Devise responder, so we can customize the both responses to match Hotwire/Turbo behavior, for example with `422 Unprocessable Entity` and `303 See Other`, respectively. The defaults aren't changing in Devise itself (yet), so it still responds on errors cases with `200 OK`, and redirects on non-GET requests with `302 Found`, but new apps are generated with the new statuses and existing apps can opt-in. Please note that these defaults might change in a future release of Devise. PRs/Issues references: #5545 #5529 #5516 #5499 #5487 #5467 #5440 #5410 #5340 #5542 #5530 #5519 #5513 #5478 #5468 #5463 #5458 #5448 #5446 #5439
carlosantoniodasilva
added a commit
that referenced
this pull request
Jan 31, 2023
Treat `:turbo_stream` request format as a navigational format, much like HTML, so Devise/responders can work properly. Allow configuring the `error_status` and `redirect_status` using the latest responders features, via a new custom Devise responder, so we can customize the both responses to match Hotwire/Turbo behavior, for example with `422 Unprocessable Entity` and `303 See Other`, respectively. The defaults aren't changing in Devise itself (yet), so it still responds on errors cases with `200 OK`, and redirects on non-GET requests with `302 Found`, but new apps are generated with the new statuses and existing apps can opt-in. Please note that these defaults might change in a future release of Devise. PRs/Issues references: #5545 #5529 #5516 #5499 #5487 #5467 #5440 #5410 #5340 #5542 #5530 #5519 #5513 #5478 #5468 #5463 #5458 #5448 #5446 #5439
Member
|
The main branch should contain all that's necessary for fully working with Turbo now, based on the changes here and a few others. A new version will be released soon, but feel free to test it out from the main branch in the meantime, and report back on any issues. Thanks. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Turbo uses the
DELETEverb directly so when redirecting with a 302 it retains that verb and the routing fails. This change will use status 303 which will force a change toGETwhich will allow proper redirecting after sign out.More information can be found in the following issues:
hotwired/turbo#84
hotwired/turbo-rails#259
rails/rails#43430