From ff4dca3e16125731b241e9f79c2db10be1891f0b Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Fri, 22 Dec 2023 12:22:48 +0530 Subject: [PATCH] fix(redirect): make the status codes a `select` field instead of `int` Drop mandatory, assume sane defaults The current implementation broke old users of redirects like helpdesk app Signed-off-by: Akhil Narang --- frappe/tests/test_website.py | 11 ++++++++++- .../website_route_redirect.json | 7 +++---- .../website_route_redirect/website_route_redirect.py | 2 +- frappe/website/path_resolver.py | 2 +- 4 files changed, 15 insertions(+), 7 deletions(-) diff --git a/frappe/tests/test_website.py b/frappe/tests/test_website.py index 2ab4ee8a0e..4e768e5467 100644 --- a/frappe/tests/test_website.py +++ b/frappe/tests/test_website.py @@ -180,7 +180,11 @@ class TestWebsite(FrappeTestCase): website_settings = frappe.get_doc("Website Settings") website_settings.append( "route_redirects", - {"source": "/testsource", "target": "/testtarget", "redirect_http_status": 301}, + {"source": "/testsource", "target": "/testtarget"}, + ) + website_settings.append( + "route_redirects", + {"source": "/testdoc307", "target": "/testtarget", "redirect_http_status": 307}, ) website_settings.save() @@ -208,6 +212,11 @@ class TestWebsite(FrappeTestCase): self.assertEqual(response.status_code, 301) self.assertEqual(response.headers.get("Location"), "/testtarget") + set_request(method="GET", path="/testdoc307") + response = get_response() + self.assertEqual(response.status_code, 307) + self.assertEqual(response.headers.get("Location"), "/testtarget") + set_request(method="GET", path="/courses/course?course=data") response = get_response() self.assertEqual(response.status_code, 301) diff --git a/frappe/website/doctype/website_route_redirect/website_route_redirect.json b/frappe/website/doctype/website_route_redirect/website_route_redirect.json index 30ace0b8ba..d105f11053 100644 --- a/frappe/website/doctype/website_route_redirect/website_route_redirect.json +++ b/frappe/website/doctype/website_route_redirect/website_route_redirect.json @@ -26,15 +26,14 @@ { "default": "301", "fieldname": "redirect_http_status", - "fieldtype": "Int", + "fieldtype": "Select", "label": "Redirect HTTP Status", - "options": "301\n302\n307\n308", - "reqd": 1 + "options": "301\n302\n307\n308" } ], "istable": 1, "links": [], - "modified": "2023-12-13 12:09:50.726082", + "modified": "2023-12-22 12:21:28.436139", "modified_by": "Administrator", "module": "Website", "name": "Website Route Redirect", diff --git a/frappe/website/doctype/website_route_redirect/website_route_redirect.py b/frappe/website/doctype/website_route_redirect/website_route_redirect.py index 616c8f28ac..06f29117e2 100644 --- a/frappe/website/doctype/website_route_redirect/website_route_redirect.py +++ b/frappe/website/doctype/website_route_redirect/website_route_redirect.py @@ -17,7 +17,7 @@ class WebsiteRouteRedirect(Document): parent: DF.Data parentfield: DF.Data parenttype: DF.Data - redirect_http_status: DF.Int + redirect_http_status: DF.Literal["301", "302", "307", "308"] source: DF.SmallText target: DF.SmallText # end: auto-generated types diff --git a/frappe/website/path_resolver.py b/frappe/website/path_resolver.py index 0ff1a8f0e3..fefec2b4eb 100644 --- a/frappe/website/path_resolver.py +++ b/frappe/website/path_resolver.py @@ -135,7 +135,7 @@ def resolve_redirect(path, query_string=None): if match: redirect_to = re.sub(pattern, rule["target"], path_to_match) frappe.flags.redirect_location = redirect_to - status_code = rule.get("redirect_http_status", 301) + status_code = rule.get("redirect_http_status") or 301 frappe.cache.hset( "website_redirects", path_to_match, {"path": redirect_to, "status_code": status_code} )