From dbd2aaf3c0130cea5388d1803688b71fb75a84e9 Mon Sep 17 00:00:00 2001 From: Wout Date: Wed, 18 Oct 2023 16:59:18 +0200 Subject: [PATCH] Feat make redirect status configurable (#1838) * feat: make default redirect status configurable * chore: use configured redirect status in turbolinks redirect module * test: add specs for globally configurable redirect status --- spec/lucky/action_redirect_spec.cr | 37 +++++++++++ src/lucky/redirectable.cr | 66 ++++++++++++++++---- src/lucky/redirectable_turbolinks_support.cr | 5 +- 3 files changed, 96 insertions(+), 12 deletions(-) diff --git a/spec/lucky/action_redirect_spec.cr b/spec/lucky/action_redirect_spec.cr index 4f232bfc1..27ccdf98a 100644 --- a/spec/lucky/action_redirect_spec.cr +++ b/spec/lucky/action_redirect_spec.cr @@ -51,6 +51,26 @@ describe Lucky::Action do should_redirect(action, to: "/somewhere", status: 301) end + it "redirects with a globally configured custom status" do + Lucky::Redirectable.temp_config(redirect_status: 303) do + action = RedirectAction.new(build_context, params) + action.redirect to: "/somewhere" + should_redirect(action, to: "/somewhere", status: 303) + + action = RedirectAction.new(build_context, params) + action.redirect to: RedirectAction.route + should_redirect(action, to: RedirectAction.path, status: 303) + + action = RedirectAction.new(build_context, params) + action.redirect to: RedirectAction + should_redirect(action, to: RedirectAction.path, status: 303) + + action = RedirectAction.new(build_context, params) + action.redirect to: ActionWithPrefix + should_redirect(action, to: "/prefix/redirect_test2", status: 303) + end + end + describe "#redirect_back" do it "redirects to referer if present" do request = build_request("POST") @@ -83,6 +103,23 @@ describe Lucky::Action do should_redirect(action, to: RedirectAction.path, status: 301) end + it "redirects back with the globally configured status code" do + Lucky::Redirectable.temp_config(redirect_status: 303) do + request = build_request("POST") + action = RedirectAction.new(build_context(request), params) + action.redirect_back fallback: "/fallback" + should_redirect(action, to: "/fallback", status: 303) + + action = RedirectAction.new(build_context, params) + action.redirect_back fallback: RedirectAction.route + should_redirect(action, to: RedirectAction.path, status: 303) + + action = RedirectAction.new(build_context, params) + action.redirect_back fallback: RedirectAction + should_redirect(action, to: RedirectAction.path, status: 303) + end + end + it "redirects to fallback if referer is external" do request = build_request("POST") request.headers["Referer"] = "https://external.com/coming/from" diff --git a/src/lucky/redirectable.cr b/src/lucky/redirectable.cr index 06b9bc968..16fee0479 100644 --- a/src/lucky/redirectable.cr +++ b/src/lucky/redirectable.cr @@ -14,22 +14,42 @@ # ``` # redirect to: Users::Index, status: 301 # -# # or use the built in enum value -# redirect to: Users::Index, status: :moved_permanently +# # or use the built-in enum value +# redirect to: Users::Index, status: HTTP::Status::MOVED_PERMANENTLY # ``` # -# You can find a list of all of the possible statuses [here](https://crystal-lang.org/api/latest/HTTP/Status.html). +# Alternatively, the status code can also be configured globally through the `redirect_status` setting: +# +# ``` +# Lucky::Redirectable.configure do |config| +# config.redirect_status = 303 +# +# # or using a built-in enum value +# config.redirect_status = HTTP::Status::SEE_OTHER.value +# end +# ``` +# +# You can find a list of all possible statuses [here](https://crystal-lang.org/api/latest/HTTP/Status.html). # # Internally, all the different methods in this module eventually use the # method that takes a `String`. However, it's recommended you pass a # `Lucky::Action` class if possible because it guarantees runtime safety. module Lucky::Redirectable + Habitat.create do + setting redirect_status : Int32 = HTTP::Status::FOUND.value + end + # Redirect back with a `Lucky::Action` fallback # # ``` # redirect_back fallback: Users::Index # ``` - def redirect_back(*, fallback : Lucky::Action.class, status = 302, allow_external = false) : Lucky::TextResponse + def redirect_back( + *, + fallback : Lucky::Action.class, + status = Lucky::Redirectable.settings.redirect_status, + allow_external = false + ) : Lucky::TextResponse redirect_back fallback: fallback.route, status: status, allow_external: allow_external end @@ -38,7 +58,12 @@ module Lucky::Redirectable # ``` # redirect_back fallback: Users::Show.with(user.id) # ``` - def redirect_back(*, fallback : Lucky::RouteHelper, status = 302, allow_external = false) : Lucky::TextResponse + def redirect_back( + *, + fallback : Lucky::RouteHelper, + status = Lucky::Redirectable.settings.redirect_status, + allow_external = false + ) : Lucky::TextResponse redirect_back fallback: fallback.path, status: status, allow_external: allow_external end @@ -47,7 +72,12 @@ module Lucky::Redirectable # ``` # redirect_back fallback: "/users", status: HTTP::Status::MOVED_PERMANENTLY # ``` - def redirect_back(*, fallback : String, status : HTTP::Status, allow_external = false) : Lucky::TextResponse + def redirect_back( + *, + fallback : String, + status : HTTP::Status, + allow_external = false + ) : Lucky::TextResponse redirect_back fallback: fallback, status: status.value, allow_external: allow_external end @@ -74,7 +104,12 @@ module Lucky::Redirectable # They can be explicitly allowed if necessary # # redirect_back fallback: "/home", allow_external: true - def redirect_back(*, fallback : String, status : Int32 = 302, allow_external : Bool = false) : Lucky::TextResponse + def redirect_back( + *, + fallback : String, + status : Int32 = Lucky::Redirectable.settings.redirect_status, + allow_external : Bool = false + ) : Lucky::TextResponse referer = request.headers["Referer"]? if referer && (allow_external || allowed_host?(referer)) @@ -89,7 +124,10 @@ module Lucky::Redirectable # ``` # redirect to: Users::Show.with(user.id), status: 301 # ``` - def redirect(to route : Lucky::RouteHelper, status = 302) : Lucky::TextResponse + def redirect( + to route : Lucky::RouteHelper, + status = Lucky::Redirectable.settings.redirect_status + ) : Lucky::TextResponse redirect to: route.path, status: status end @@ -98,14 +136,17 @@ module Lucky::Redirectable # ``` # redirect to: Users::Index # ``` - def redirect(to action : Lucky::Action.class, status = 302) : Lucky::TextResponse + def redirect( + to action : Lucky::Action.class, + status = Lucky::Redirectable.settings.redirect_status + ) : Lucky::TextResponse redirect to: action.route, status: status end # Redirect to the given path, with a human friendly status # # ``` - # redirect to: "/users", status: :moved_permanently + # redirect to: "/users", status: HTTP::Status::MOVED_PERMANENTLY # ``` def redirect(to path : String, status : HTTP::Status) : Lucky::TextResponse redirect(path, status.value) @@ -118,7 +159,10 @@ module Lucky::Redirectable # redirect to: "/users/1", status: 301 # ``` # Note: It's recommended to use the method above that accepts a human friendly version of the status - def redirect(to path : String, status : Int32 = 302) : Lucky::TextResponse + def redirect( + to path : String, + status : Int32 = Lucky::Redirectable.settings.redirect_status + ) : Lucky::TextResponse # flash messages are not consumed here, so keep them for the next action flash.keep context.response.headers.add "Location", path diff --git a/src/lucky/redirectable_turbolinks_support.cr b/src/lucky/redirectable_turbolinks_support.cr index 74457c573..473a6fd85 100644 --- a/src/lucky/redirectable_turbolinks_support.cr +++ b/src/lucky/redirectable_turbolinks_support.cr @@ -5,7 +5,10 @@ # but Lucky::ErrorAction not have pipe support module Lucky::RedirectableTurbolinksSupport # Overrides Lucky::Redirectable redirect's method - def redirect(to path : String, status : Int32 = 302) : Lucky::TextResponse + def redirect( + to path : String, + status : Int32 = Lucky::Redirectable.settings.redirect_status + ) : Lucky::TextResponse # flash messages are not consumed here, so keep them for the next action flash.keep if ajax? && request.method != "GET"