From 30659b9ee28e034c6eef7ba682eea5b302b6fb6b Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Thu, 11 Mar 2021 10:29:12 +0530 Subject: [PATCH] feat: Option to consider query string while writing website redirect - if match_with_query_string is true in a website redirect rule, system will try to match the regex with the result of route + query_string --- frappe/tests/test_website.py | 20 +++++++++++++------- frappe/website/redirect.py | 12 ++++++++---- frappe/website/serve.py | 28 ++++++++++++++++++---------- 3 files changed, 39 insertions(+), 21 deletions(-) diff --git a/frappe/tests/test_website.py b/frappe/tests/test_website.py index 91e08db457..ce165e36d9 100644 --- a/frappe/tests/test_website.py +++ b/frappe/tests/test_website.py @@ -51,7 +51,7 @@ class TestWebsite(unittest.TestCase): def test_page_load(self): set_request(method='POST', path='login') - response = render.render() + response = serve.get_response() self.assertEquals(response.status_code, 200) @@ -103,7 +103,8 @@ class TestWebsite(unittest.TestCase): frappe.hooks.website_redirects = [ dict(source=r'/testfrom', target=r'://testto1'), dict(source=r'/testfromregex.*', target=r'://testto2'), - dict(source=r'/testsub/(.*)', target=r'://testto3/\1') + dict(source=r'/testsub/(.*)', target=r'://testto3/\1'), + dict(source=r'/courses/course\?course=(.*)', target=r'/courses/\1', match_with_query_string=True), ] website_settings = frappe.get_doc('Website Settings') @@ -117,28 +118,33 @@ class TestWebsite(unittest.TestCase): frappe.cache().delete_key('website_redirects') set_request(method='GET', path='/testfrom') - response = render.render() + response = serve.get_response() self.assertEquals(response.status_code, 301) self.assertEquals(response.headers.get('Location'), r'://testto1') set_request(method='GET', path='/testfromregex/test') - response = render.render() + response = serve.get_response() self.assertEquals(response.status_code, 301) self.assertEquals(response.headers.get('Location'), r'://testto2') set_request(method='GET', path='/testsub/me') - response = render.render() + response = serve.get_response() self.assertEquals(response.status_code, 301) self.assertEquals(response.headers.get('Location'), r'://testto3/me') set_request(method='GET', path='/test404') - response = render.render() + response = serve.get_response() self.assertEquals(response.status_code, 404) set_request(method='GET', path='/testsource') - response = render.render() + response = serve.get_response() self.assertEquals(response.status_code, 301) self.assertEquals(response.headers.get('Location'), '/testtarget') + set_request(method='GET', path='/courses/course?course=data') + response = serve.get_response() + self.assertEquals(response.status_code, 301) + self.assertEquals(response.headers.get('Location'), '/courses/data') + delattr(frappe.hooks, 'website_redirects') frappe.cache().delete_key('app_hooks') diff --git a/frappe/website/redirect.py b/frappe/website/redirect.py index 73e3c21727..e66c0a3b7b 100644 --- a/frappe/website/redirect.py +++ b/frappe/website/redirect.py @@ -2,7 +2,7 @@ from __future__ import unicode_literals import re, frappe -def resolve_redirect(path): +def resolve_redirect(path, query_string=None): ''' Resolve redirects from hooks @@ -33,9 +33,13 @@ def resolve_redirect(path): for rule in redirects: pattern = rule['source'].strip('/ ') + '$' - if re.match(pattern, path): - redirect_to = re.sub(pattern, rule['target'], path) + path_to_match = path + if rule.get('match_with_query_string'): + path_to_match = path + '?' + frappe.safe_decode(query_string) + + if re.match(pattern, path_to_match): + redirect_to = re.sub(pattern, rule['target'], path_to_match) frappe.flags.redirect_location = redirect_to - frappe.cache().hset('website_redirects', path, redirect_to) + frappe.cache().hset('website_redirects', path_to_match, redirect_to) raise frappe.Redirect diff --git a/frappe/website/serve.py b/frappe/website/serve.py index b635d9615b..11e950573e 100644 --- a/frappe/website/serve.py +++ b/frappe/website/serve.py @@ -1,25 +1,33 @@ -import frappe -import os, mimetypes +import mimetypes +import os from werkzeug.wrappers import Response from werkzeug.wsgi import wrap_file -from frappe.website.render import (resolve_path, build_response) +import frappe +from frappe import _ +from frappe.utils import cint, cstr +from frappe.model.document import get_controller +from frappe.website.doctype.website_settings.website_settings import \ + get_website_settings from frappe.website.redirect import resolve_redirect -from frappe.website.router import (get_start_folders, get_doctypes_with_web_view, - get_page_info_from_web_page_with_dynamic_routes) -from frappe.website.utils import (get_home_page, can_cache, delete_page_cache, - get_toc, get_next_link) -from frappe.website.doctype.website_settings.website_settings import get_website_settings +from frappe.website.render import build_response, resolve_path +from frappe.website.router import (get_doctypes_with_web_view, + get_page_info_from_web_page_with_dynamic_routes, get_start_folders) +from frappe.website.utils import (can_cache, delete_page_cache, get_home_page, + get_next_link, get_toc) + def get_response(path=None, http_status_code=200): """render html page""" + query_string = None if not path: path = frappe.local.request.path + query_string = frappe.local.request.query_string try: path = path.strip('/ ') - resolve_redirect(path) + resolve_redirect(path, query_string) path = resolve_path(path) data = None @@ -513,7 +521,7 @@ class DocumentPage(BaseTemplatePage): if meta.is_published_field: condition_field = meta.is_published_field elif not meta.custom: - controller = get_controller(doctype) + controller = get_controller(meta.doctype) condition_field = controller.website.condition_field return condition_field