Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Respect redirect status code when recalling the action #5573

Merged
merged 2 commits into from
Mar 30, 2023

Conversation

carlosantoniodasilva
Copy link
Member

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.)

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.
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 carlosantoniodasilva force-pushed the ca/failure-app-respect-redirect branch from 73e2a61 to 89a0835 Compare March 20, 2023 21:18
@carlosantoniodasilva carlosantoniodasilva marked this pull request as ready for review March 23, 2023 20:11
@carlosantoniodasilva carlosantoniodasilva merged commit 8dbe5b2 into main Mar 30, 2023
@carlosantoniodasilva carlosantoniodasilva deleted the ca/failure-app-respect-redirect branch March 30, 2023 20:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
1 participant