From a574c1ba88cd76d74065bfbcbf2c9c78dd208d0b Mon Sep 17 00:00:00 2001 From: saxenabhishek Date: Thu, 2 Dec 2021 23:20:40 +0530 Subject: [PATCH 01/36] chore: patching ValueWrapper --- frappe/query_builder/terms.py | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 frappe/query_builder/terms.py diff --git a/frappe/query_builder/terms.py b/frappe/query_builder/terms.py new file mode 100644 index 0000000000..c221dcb28e --- /dev/null +++ b/frappe/query_builder/terms.py @@ -0,0 +1,27 @@ +from typing import Any, Optional, Dict +from pypika.terms import ValueWrapper +from pypika.utils import format_alias_sql + + +class NamedParameterWrapper(): + def __init__(self, parameters: Dict[str, Any]): + self.parameters = parameters + + def update_parameters(self, param_key: Any, param_value: Any, **kwargs): + self.parameters[param_key[1:]] = param_value + + def get_sql(self, **kwargs): + return f'@param{len(self.parameters) + 1}' + + +class ParameterizedValueWrapper(ValueWrapper): + def get_sql(self, quote_char: Optional[str] = None, secondary_quote_char: str = "'", param_wrapper= None, **kwargs: Any) -> str: + if param_wrapper is None: + sql = self.get_value_sql(quote_char=quote_char, secondary_quote_char=secondary_quote_char, **kwargs) + return format_alias_sql(sql, self.alias, quote_char=quote_char, **kwargs) + else: + value_sql = self.get_value_sql(quote_char=quote_char, **kwargs) + param_sql = param_wrapper.get_sql(**kwargs) + param_wrapper.update_parameters(param_key=param_sql, param_value=value_sql, **kwargs) + + return format_alias_sql(param_sql, self.alias, quote_char=quote_char, **kwargs) \ No newline at end of file From 9fdacedfc80889c81c4887c2f2f2581e5d0d56f6 Mon Sep 17 00:00:00 2001 From: saxenabhishek Date: Thu, 2 Dec 2021 23:23:25 +0530 Subject: [PATCH 02/36] feat: sanitise frappe.qb --- frappe/query_builder/__init__.py | 5 +++++ frappe/query_builder/terms.py | 8 ++++---- frappe/query_builder/utils.py | 14 ++++++++++---- 3 files changed, 19 insertions(+), 8 deletions(-) diff --git a/frappe/query_builder/__init__.py b/frappe/query_builder/__init__.py index 9c7432142f..06d499678f 100644 --- a/frappe/query_builder/__init__.py +++ b/frappe/query_builder/__init__.py @@ -1,2 +1,7 @@ +from frappe.query_builder.terms import ParameterizedValueWrapper +import pypika + +pypika.terms.ValueWrapper = ParameterizedValueWrapper + from pypika import * from frappe.query_builder.utils import Column, DocType, get_query_builder, patch_query_execute, patch_query_aggregation diff --git a/frappe/query_builder/terms.py b/frappe/query_builder/terms.py index c221dcb28e..c09d9595fb 100644 --- a/frappe/query_builder/terms.py +++ b/frappe/query_builder/terms.py @@ -1,4 +1,4 @@ -from typing import Any, Optional, Dict +from typing import Any, List, Optional, Dict from pypika.terms import ValueWrapper from pypika.utils import format_alias_sql @@ -8,10 +8,10 @@ class NamedParameterWrapper(): self.parameters = parameters def update_parameters(self, param_key: Any, param_value: Any, **kwargs): - self.parameters[param_key[1:]] = param_value + self.parameters[param_key[2:-2]] = param_value def get_sql(self, **kwargs): - return f'@param{len(self.parameters) + 1}' + return f'%(param{len(self.parameters) + 1})s' class ParameterizedValueWrapper(ValueWrapper): @@ -20,7 +20,7 @@ class ParameterizedValueWrapper(ValueWrapper): sql = self.get_value_sql(quote_char=quote_char, secondary_quote_char=secondary_quote_char, **kwargs) return format_alias_sql(sql, self.alias, quote_char=quote_char, **kwargs) else: - value_sql = self.get_value_sql(quote_char=quote_char, **kwargs) + value_sql = self.get_value_sql(quote_char=quote_char, **kwargs) if not isinstance(self.value,int) else self.value param_sql = param_wrapper.get_sql(**kwargs) param_wrapper.update_parameters(param_key=param_sql, param_value=value_sql, **kwargs) diff --git a/frappe/query_builder/utils.py b/frappe/query_builder/utils.py index a7f52df012..7922825725 100644 --- a/frappe/query_builder/utils.py +++ b/frappe/query_builder/utils.py @@ -10,6 +10,7 @@ import frappe from .builder import MariaDB, Postgres from pypika.terms import PseudoColumn +from frappe.query_builder.terms import NamedParameterWrapper class db_type_is(Enum): MARIADB = "mariadb" @@ -53,12 +54,16 @@ def patch_query_execute(): This excludes the use of `frappe.db.sql` method while executing the query object """ - def execute_query(query, *args, **kwargs): - query = str(query) + query, params = prepare_query(query) + return frappe.db.sql(query, params, *args, **kwargs) + + def prepare_query(query): + params = {} + query = query.get_sql(param_wrapper = NamedParameterWrapper(params)) if frappe.flags.in_safe_exec and not query.lower().strip().startswith("select"): raise frappe.PermissionError('Only SELECT SQL allowed in scripting') - return frappe.db.sql(query, *args, **kwargs) + return query, params query_class = get_attr(str(frappe.qb).split("'")[1]) builder_class = get_type_hints(query_class._builder).get('return') @@ -67,6 +72,7 @@ def patch_query_execute(): raise BuilderIdentificationFailed builder_class.run = execute_query + builder_class.walk = prepare_query def patch_query_aggregation(): @@ -77,4 +83,4 @@ def patch_query_aggregation(): frappe.qb.max = _max frappe.qb.min = _min frappe.qb.avg = _avg - frappe.qb.sum = _sum \ No newline at end of file + frappe.qb.sum = _sum From 6120b4b3c1dbf17bc783be16ba41a5c73d5c5df1 Mon Sep 17 00:00:00 2001 From: saxenabhishek Date: Sat, 4 Dec 2021 20:12:48 +0530 Subject: [PATCH 03/36] fix: extend named parameters to frappe.qb.function --- frappe/query_builder/__init__.py | 3 ++- frappe/query_builder/terms.py | 28 +++++++++++++++++++++++++--- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/frappe/query_builder/__init__.py b/frappe/query_builder/__init__.py index 06d499678f..bf7be84c51 100644 --- a/frappe/query_builder/__init__.py +++ b/frappe/query_builder/__init__.py @@ -1,7 +1,8 @@ -from frappe.query_builder.terms import ParameterizedValueWrapper +from frappe.query_builder.terms import ParameterizedValueWrapper, ParameterizedFunction import pypika pypika.terms.ValueWrapper = ParameterizedValueWrapper +pypika.terms.Function = ParameterizedFunction from pypika import * from frappe.query_builder.utils import Column, DocType, get_query_builder, patch_query_execute, patch_query_aggregation diff --git a/frappe/query_builder/terms.py b/frappe/query_builder/terms.py index c09d9595fb..2032cd8497 100644 --- a/frappe/query_builder/terms.py +++ b/frappe/query_builder/terms.py @@ -1,5 +1,6 @@ -from typing import Any, List, Optional, Dict -from pypika.terms import ValueWrapper +from typing import Any, Dict, Optional + +from pypika.terms import Function, ValueWrapper from pypika.utils import format_alias_sql @@ -23,5 +24,26 @@ class ParameterizedValueWrapper(ValueWrapper): value_sql = self.get_value_sql(quote_char=quote_char, **kwargs) if not isinstance(self.value,int) else self.value param_sql = param_wrapper.get_sql(**kwargs) param_wrapper.update_parameters(param_key=param_sql, param_value=value_sql, **kwargs) + return format_alias_sql(param_sql, self.alias, quote_char=quote_char, **kwargs) - return format_alias_sql(param_sql, self.alias, quote_char=quote_char, **kwargs) \ No newline at end of file + +class ParameterizedFunction(Function): + def get_sql(self, **kwargs: Any) -> str: + with_alias = kwargs.pop("with_alias", False) + with_namespace = kwargs.pop("with_namespace", False) + quote_char = kwargs.pop("quote_char", None) + dialect = kwargs.pop("dialect", None) + param_wrapper = kwargs.pop("param_wrapper", None) + + function_sql = self.get_function_sql(with_namespace=with_namespace, quote_char=quote_char, param_wrapper=param_wrapper, dialect=dialect) + + if self.schema is not None: + function_sql = "{schema}.{function}".format( + schema=self.schema.get_sql(quote_char=quote_char, dialect=dialect, **kwargs), + function=function_sql, + ) + + if with_alias: + return format_alias_sql(function_sql, self.alias, quote_char=quote_char, **kwargs) + + return function_sql From aa855afe089d209d2a4796c8497d21ed5d12578a Mon Sep 17 00:00:00 2001 From: saxenabhishek Date: Mon, 6 Dec 2021 12:12:56 +0530 Subject: [PATCH 04/36] test: test for patches through walk --- frappe/tests/test_query_builder.py | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/frappe/tests/test_query_builder.py b/frappe/tests/test_query_builder.py index 7a0935a63b..1d63d2041c 100644 --- a/frappe/tests/test_query_builder.py +++ b/frappe/tests/test_query_builder.py @@ -2,7 +2,7 @@ import unittest from typing import Callable import frappe -from frappe.query_builder.functions import GroupConcat, Match +from frappe.query_builder.functions import Coalesce, GroupConcat, Match from frappe.query_builder.utils import db_type_is @@ -49,6 +49,25 @@ class TestBuilderBase(object): self.assertIsInstance(query.run, Callable) self.assertIsInstance(data, list) + def test_walk(self): + DocType = frappe.qb.DocType('DocType') + query = ( + frappe.qb.from_(DocType) + .select(DocType.name) + .where((DocType.owner == "Administrator' --") + & (Coalesce(DocType.search_fields == "subject")) + ) + ) + self.assertTrue("walk" in dir(query)) + query, params = query.walk() + + self.assertIn("%(param1)s", query) + self.assertIn("%(param2)s", query) + self.assertIn("param1",params) + self.assertEqual(params["param1"],"Administrator' --") + self.assertEqual(params["param2"],"subject") + + @run_only_if(db_type_is.MARIADB) class TestBuilderMaria(unittest.TestCase, TestBuilderBase): def test_adding_tabs_in_from(self): @@ -59,7 +78,6 @@ class TestBuilderMaria(unittest.TestCase, TestBuilderBase): "SELECT * FROM `__Auth`", frappe.qb.from_("__Auth").select("*").get_sql() ) - @run_only_if(db_type_is.POSTGRES) class TestBuilderPostgres(unittest.TestCase, TestBuilderBase): def test_adding_tabs_in_from(self): From 99defea410080b20c1ded5792f4c8d7900d64972 Mon Sep 17 00:00:00 2001 From: Suraj Shetty <13928957+surajshetty3416@users.noreply.github.com> Date: Tue, 7 Dec 2021 16:40:45 +0530 Subject: [PATCH 05/36] fix: Explicitly ignore semgrep warning --- frappe/query_builder/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/query_builder/utils.py b/frappe/query_builder/utils.py index 7922825725..2767e90242 100644 --- a/frappe/query_builder/utils.py +++ b/frappe/query_builder/utils.py @@ -56,7 +56,7 @@ def patch_query_execute(): """ def execute_query(query, *args, **kwargs): query, params = prepare_query(query) - return frappe.db.sql(query, params, *args, **kwargs) + return frappe.db.sql(query, params, *args, **kwargs) # nosemgrep def prepare_query(query): params = {} From 4f7c8d066ad76ac39ac7bbd9666ce58c5fd5e24c Mon Sep 17 00:00:00 2001 From: saxenabhishek Date: Tue, 7 Dec 2021 20:55:38 +0530 Subject: [PATCH 06/36] feat: build constant value cols --- frappe/query_builder/custom.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/frappe/query_builder/custom.py b/frappe/query_builder/custom.py index 5aaed463d9..978c9858d3 100644 --- a/frappe/query_builder/custom.py +++ b/frappe/query_builder/custom.py @@ -1,7 +1,8 @@ -from typing import Optional +from typing import Any, Optional from pypika.functions import DistinctOptionFunction -from pypika.utils import builder +from pypika.terms import Term +from pypika.utils import builder, format_alias_sql import frappe @@ -81,3 +82,11 @@ class TO_TSVECTOR(DistinctOptionFunction): text (str): [ the text string that we match it against ] """ self._PLAINTO_TSQUERY = text + + +class ConstantColumn(Term): + def __init__(self, name: str) -> None: + self.name = name + + def get_sql(self, quote_char: Optional[str] = None, **kwargs: Any) -> str: + return format_alias_sql('"{name}"'.format(name=self.name), self.alias, quote_char=quote_char, **kwargs) From 9a393e5654c17011c86b0badf44b2e3c24c40de4 Mon Sep 17 00:00:00 2001 From: saxenabhishek Date: Wed, 8 Dec 2021 15:43:22 +0530 Subject: [PATCH 07/36] fix: postgres compatibility --- frappe/query_builder/custom.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/frappe/query_builder/custom.py b/frappe/query_builder/custom.py index 978c9858d3..65eccac838 100644 --- a/frappe/query_builder/custom.py +++ b/frappe/query_builder/custom.py @@ -2,7 +2,7 @@ from typing import Any, Optional from pypika.functions import DistinctOptionFunction from pypika.terms import Term -from pypika.utils import builder, format_alias_sql +from pypika.utils import builder, format_alias_sql, format_quotes import frappe @@ -85,8 +85,15 @@ class TO_TSVECTOR(DistinctOptionFunction): class ConstantColumn(Term): + alias = None + def __init__(self, name: str) -> None: self.name = name def get_sql(self, quote_char: Optional[str] = None, **kwargs: Any) -> str: - return format_alias_sql('"{name}"'.format(name=self.name), self.alias, quote_char=quote_char, **kwargs) + return format_alias_sql( + format_quotes(self.name,kwargs.get("secondary_quote_char") or ""), + self.alias or self.name, + quote_char=quote_char, + **kwargs + ) From a71795afec8d4bb0132c527dbe7e330c5fbf0b5c Mon Sep 17 00:00:00 2001 From: saxenabhishek Date: Wed, 8 Dec 2021 15:46:30 +0530 Subject: [PATCH 08/36] docs: ConstantColumn --- frappe/query_builder/custom.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/frappe/query_builder/custom.py b/frappe/query_builder/custom.py index 65eccac838..c86132e44d 100644 --- a/frappe/query_builder/custom.py +++ b/frappe/query_builder/custom.py @@ -86,14 +86,19 @@ class TO_TSVECTOR(DistinctOptionFunction): class ConstantColumn(Term): alias = None - - def __init__(self, name: str) -> None: - self.name = name + + def __init__(self, value: str) -> None: + """[ Returns a pseudo column with a constant value in all the rows] + + Args: + value (str): [ Value of the column ] + """ + self.value = value def get_sql(self, quote_char: Optional[str] = None, **kwargs: Any) -> str: return format_alias_sql( - format_quotes(self.name,kwargs.get("secondary_quote_char") or ""), - self.alias or self.name, + format_quotes(self.value, kwargs.get("secondary_quote_char") or ""), + self.alias or self.value, quote_char=quote_char, **kwargs ) From d7b2822ce80d859c6a820253bad12202785e73ae Mon Sep 17 00:00:00 2001 From: Summayya Date: Wed, 8 Dec 2021 18:01:07 +0530 Subject: [PATCH 09/36] refactor: move rnl arrow to common variable file --- frappe/public/scss/common/css_variables.scss | 3 +++ frappe/public/scss/desk/css_variables.scss | 3 --- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/frappe/public/scss/common/css_variables.scss b/frappe/public/scss/common/css_variables.scss index ede3dd8ba9..a14c19af2a 100644 --- a/frappe/public/scss/common/css_variables.scss +++ b/frappe/public/scss/common/css_variables.scss @@ -225,4 +225,7 @@ --checkbox-right-margin: var(--margin-xs); --checkbox-size: 14px; --checkbox-focus-shadow: 0 0 0 2px var(--gray-300); + + --right-arrow-svg: url("data: image/svg+xml;utf8, "); + --left-arrow-svg: url("data: image/svg+xml;utf8, "); } diff --git a/frappe/public/scss/desk/css_variables.scss b/frappe/public/scss/desk/css_variables.scss index 255d0a91e4..4a2a27f8d1 100644 --- a/frappe/public/scss/desk/css_variables.scss +++ b/frappe/public/scss/desk/css_variables.scss @@ -63,7 +63,4 @@ $input-height: 28px !default; // skeleton --skeleton-bg: var(--gray-100); - --right-arrow-svg: url("data: image/svg+xml;utf8, "); - - --left-arrow-svg: url("data: image/svg+xml;utf8, "); } From 0f08c53c7cf2ea0897f532c99a37158c7d38c172 Mon Sep 17 00:00:00 2001 From: Summayya Date: Wed, 8 Dec 2021 18:02:03 +0530 Subject: [PATCH 10/36] refactor: breadcrumb style for portal pages --- frappe/public/scss/website/index.scss | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/frappe/public/scss/website/index.scss b/frappe/public/scss/website/index.scss index 58f5ca79c6..606415ed97 100644 --- a/frappe/public/scss/website/index.scss +++ b/frappe/public/scss/website/index.scss @@ -112,9 +112,30 @@ } .breadcrumb { - padding: 0; + padding-left: 0; + padding-right: 0; font-size: $font-size-sm; - background-color: $breadcrumb-bg; +} + +.breadcrumb-item { + + .breadcrumb-item + { + font-size: $font-size-sm; + &::before { + content: #{"/*!rtl:var(--left-arrow-svg);*/"}var(--right-arrow-svg); + display: inline-block; + } + } + a { + color: var(--text-color) + } + + li.disabled { + a { + color: var(--text-muted); + pointer-events: none; + } + } } a.card { From 3baa09401e1d3abd8ff14b88b7b0a8b53257b87a Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Thu, 9 Dec 2021 09:59:19 +0530 Subject: [PATCH 11/36] ci: update apt cache before install (#15226) --- .github/helper/install.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/helper/install.sh b/.github/helper/install.sh index 454cc89694..19a7c68e19 100644 --- a/.github/helper/install.sh +++ b/.github/helper/install.sh @@ -17,7 +17,7 @@ if [ "$TYPE" == "server" ]; then fi if [ "$DB" == "mariadb" ];then - sudo apt install mariadb-client-10.3 + sudo apt update && sudo apt install mariadb-client-10.3 mysql --host 127.0.0.1 --port 3306 -u root -e "SET GLOBAL character_set_server = 'utf8mb4'"; mysql --host 127.0.0.1 --port 3306 -u root -e "SET GLOBAL collation_server = 'utf8mb4_unicode_ci'"; From f589342c7061f6eefca30534676712fd0995b1f5 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Wed, 8 Dec 2021 22:35:27 +0530 Subject: [PATCH 12/36] fix: missing content in communication composer `message` is used as args in some places and `txt` in another. Supporting both since there's no clear interface to this arguments for this component. --- frappe/public/js/frappe/views/communication.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/frappe/public/js/frappe/views/communication.js b/frappe/public/js/frappe/views/communication.js index beeea40c5e..1ed79a9758 100755 --- a/frappe/public/js/frappe/views/communication.js +++ b/frappe/public/js/frappe/views/communication.js @@ -351,7 +351,7 @@ frappe.views.CommunicationComposer = class { } async set_values_from_last_edited_communication() { - if (this.txt) return; + if (this.txt || this.message) return; const last_edited = this.get_last_edited_communication(); if (!last_edited.content) return; @@ -713,7 +713,7 @@ frappe.views.CommunicationComposer = class { async set_content() { if (this.content_set) return; - let message = this.txt || ""; + let message = this.txt || this.message || ""; if (!message && this.frm) { const { doctype, docname } = this.frm; message = await localforage.getItem(doctype + docname) || ""; @@ -727,7 +727,7 @@ frappe.views.CommunicationComposer = class { const SALUTATION_END_COMMENT = ""; if (this.real_name && !message.includes(SALUTATION_END_COMMENT)) { - this.message = ` + message = `

${__('Dear {0},', [this.real_name], 'Salutation in new email')},

${SALUTATION_END_COMMENT}
${message} From 5b6012d0d3944153bb3bb8156c21482a1a8583da Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Wed, 8 Dec 2021 22:52:02 +0530 Subject: [PATCH 13/36] feat: copy error to clipboard Currently error can be reported if email is setup but other than that only way to find error info is use browser's console. This change adds a secondary button to copy error info to clipboard. --- frappe/public/js/frappe/request.js | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/frappe/public/js/frappe/request.js b/frappe/public/js/frappe/request.js index adc3bb5626..eff0391338 100644 --- a/frappe/public/js/frappe/request.js +++ b/frappe/public/js/frappe/request.js @@ -481,6 +481,24 @@ frappe.request.report_error = function(xhr, request_opts) { exc = ""; } + const copy_markdown_to_clipboard = () => { + const code_block = snippet => '```\n' + snippet + '\n```'; + const traceback_info = [ + '### App Versions', + code_block(JSON.stringify(frappe.boot.versions, null, "\t")), + '### Route', + code_block(frappe.get_route_str()), + '### Trackeback', + code_block(exc), + '### Request Data', + code_block(JSON.stringify(request_opts, null, "\t")), + '### Response Data', + code_block(JSON.stringify(data, null, '\t')), + ].join("\n"); + frappe.utils.copy_to_clipboard(traceback_info); + }; + + var show_communication = function() { var error_report_message = [ '
Please type some additional information that could help us reproduce this issue:
', @@ -532,6 +550,11 @@ frappe.request.report_error = function(xhr, request_opts) { frappe.msgprint(__('Support Email Address Not Specified')); } frappe.error_dialog.hide(); + }, + secondary_action_label: __('Copy error to clipboard'), + secondary_action: () => { + copy_markdown_to_clipboard(); + frappe.error_dialog.hide(); } }); frappe.error_dialog.wrapper.classList.add('msgprint-dialog'); From ecdb8932864ced64c9969fe351b492cabdfd10da Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Wed, 8 Dec 2021 23:01:59 +0530 Subject: [PATCH 14/36] fix: copy to clipboard strips off newlines input element isn't suitable for this hack, switch to textarea for supporting newlines in text to be copied. --- frappe/public/js/frappe/utils/utils.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/public/js/frappe/utils/utils.js b/frappe/public/js/frappe/utils/utils.js index 5f546c22da..47aba903b9 100644 --- a/frappe/public/js/frappe/utils/utils.js +++ b/frappe/public/js/frappe/utils/utils.js @@ -957,7 +957,7 @@ Object.assign(frappe.utils, { return decoded; }, copy_to_clipboard(string) { - let input = $(""); + let input = $("