Respect redirect status code when recalling the action #5573
Conversation
This file contains 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
While introducing this on turbo, looks like no specific test was added, so this at least covers that a bit. It needs some conditional checks since not all supported Rails + Responders version work with the customization, so there's one test for the hardcoded status version too, which can be removed in the future.
carlosantoniodasilva
force-pushed
the
ca/failure-app-respect-redirect
branch
from
edeb5ce
to
e7d15f9
Compare
This was referenced Mar 20, 2023
carlosantoniodasilva
force-pushed
the
ca/failure-app-respect-redirect
branch
from
e7d15f9
to
73e2a61
Compare
It appears some people use the recall functionality with a redirect response, and Devise starting on version 4.9 was overriding that status code to the configured `error_status` for better Turbo support, which broke the redirect functionality / expectation. While I don't think it's really great usage of the recall functionality, or at least it was unexpected usage, it's been working like that basically forever where recalling would use the status code of the recalled action, so this at least keeps it more consistent with that behavior by respecting redirects and keeping that response as a redirect based on the configured status, which should also work with Turbo I believe, and makes this less of a breaking change. Closes #5570 Closes #5561 (it was closed previously, but related / closes with an actual change now.)
carlosantoniodasilva
force-pushed
the
ca/failure-app-respect-redirect
branch
from
73e2a61
to
89a0835
Compare
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.
It appears some people use the recall functionality with a redirect
response, and Devise starting on version 4.9 was overriding that status
code to the configured
error_statusfor better Turbo support, whichbroke the redirect functionality / expectation.
While I don't think it's really great usage of the recall functionality,
or at least it was unexpected usage, it's been working like that
basically forever where recalling would use the status code of the
recalled action, so this at least keeps it more consistent with that
behavior by respecting redirects and keeping that response as a redirect
based on the configured status, which should also work with Turbo I
believe, and makes this less of a breaking change.
Closes #5570
Closes #5561 (it was closed previously, but related / closes with an
actual change now.)