From d8d8c8e54ee7cc96eb740bf5c756887f57d4d108 Mon Sep 17 00:00:00 2001 From: MitulDavid Date: Mon, 20 Sep 2021 21:11:47 +0530 Subject: [PATCH 01/68] ci: Code coverage for JS files --- .github/workflows/ui-tests.yml | 19 ++++++++++++++++++- .gitignore | 1 + cypress/plugins/index.js | 8 ++++---- cypress/support/index.js | 1 + frappe/commands/utils.py | 11 +++++++---- package.json | 8 ++++++-- 6 files changed, 37 insertions(+), 11 deletions(-) diff --git a/.github/workflows/ui-tests.yml b/.github/workflows/ui-tests.yml index 2a55546ec4..530b988919 100644 --- a/.github/workflows/ui-tests.yml +++ b/.github/workflows/ui-tests.yml @@ -122,12 +122,29 @@ jobs: DB: mariadb TYPE: ui + - name: Instrument Source Code + if: ${{ steps.check-build.outputs.build == 'strawberry' }} + run: cd ~/frappe-bench/apps/frappe/ && npx nyc instrument -x 'frappe/public/dist/**' -x 'frappe/public/js/lib/**' -x '**/*.bundle.js' --compact=false --in-place frappe + + - name: Build + if: ${{ steps.check-build.outputs.build == 'strawberry' }} + run: cd ~/frappe-bench/ && bench build --apps frappe + - name: Site Setup if: ${{ steps.check-build.outputs.build == 'strawberry' }} run: cd ~/frappe-bench/ && bench --site test_site execute frappe.utils.install.complete_setup_wizard - name: UI Tests if: ${{ steps.check-build.outputs.build == 'strawberry' }} - run: cd ~/frappe-bench/ && bench --site test_site run-ui-tests frappe --headless --parallel --ci-build-id $GITHUB_RUN_ID + run: cd ~/frappe-bench/ && bench --site test_site run-ui-tests frappe --with-coverage --headless --parallel --ci-build-id $GITHUB_RUN_ID env: CYPRESS_RECORD_KEY: 4a48f41c-11b3-425b-aa88-c58048fa69eb + + - name: Upload Coverage Data + if: ${{ steps.check-build.outputs.build == 'strawberry' }} + uses: codecov/codecov-action@v2 + with: + name: Cypress + fail_ci_if_error: true + directory: /home/runner/frappe-bench/apps/frappe/.cypress-coverage/ + verbose: true \ No newline at end of file diff --git a/.gitignore b/.gitignore index 1ff3122d70..c9dd8f38f3 100644 --- a/.gitignore +++ b/.gitignore @@ -67,6 +67,7 @@ coverage.xml *.cover .hypothesis/ .pytest_cache/ +.cypress-coverage # Translations *.mo diff --git a/cypress/plugins/index.js b/cypress/plugins/index.js index 07d9804a73..c08f9e594c 100644 --- a/cypress/plugins/index.js +++ b/cypress/plugins/index.js @@ -11,7 +11,7 @@ // This function is called when a project is opened or re-opened (e.g. due to // the project's config changing) -module.exports = () => { - // `on` is used to hook into various events Cypress emits - // `config` is the resolved Cypress config -}; +module.exports = (on, config) => { + require('@cypress/code-coverage/task')(on, config) + return config +} \ No newline at end of file diff --git a/cypress/support/index.js b/cypress/support/index.js index 1bee72d2ca..9cd770a31e 100644 --- a/cypress/support/index.js +++ b/cypress/support/index.js @@ -15,6 +15,7 @@ // Import commands.js using ES2015 syntax: import './commands'; +import '@cypress/code-coverage/support'; // Alternatively you can use CommonJS syntax: diff --git a/frappe/commands/utils.py b/frappe/commands/utils.py index b0151106db..bef558c859 100644 --- a/frappe/commands/utils.py +++ b/frappe/commands/utils.py @@ -592,9 +592,10 @@ def run_parallel_tests(context, app, build_number, total_builds, with_coverage=F @click.argument('app') @click.option('--headless', is_flag=True, help="Run UI Test in headless mode") @click.option('--parallel', is_flag=True, help="Run UI Test in parallel mode") +@click.option('--with-coverage', is_flag=True, help="Generate coverage report") @click.option('--ci-build-id') @pass_context -def run_ui_tests(context, app, headless=False, parallel=True, ci_build_id=None): +def run_ui_tests(context, app, headless=False, parallel=True, with_coverage=False, ci_build_id=None): "Run UI tests" site = get_site(context) app_base_path = os.path.abspath(os.path.join(frappe.get_app_path(app), '..')) @@ -604,6 +605,7 @@ def run_ui_tests(context, app, headless=False, parallel=True, ci_build_id=None): # override baseUrl using env variable site_env = f'CYPRESS_baseUrl={site_url}' password_env = f'CYPRESS_adminPassword={admin_password}' if admin_password else '' + coverage_env = f'CYPRESS_coverage={str(with_coverage).lower()}' os.chdir(app_base_path) @@ -611,22 +613,23 @@ def run_ui_tests(context, app, headless=False, parallel=True, ci_build_id=None): cypress_path = f"{node_bin}/cypress" plugin_path = f"{node_bin}/../cypress-file-upload" testing_library_path = f"{node_bin}/../@testing-library" + coverage_plugin_path = f"{node_bin}/../@cypress/code-coverage" # check if cypress in path...if not, install it. if not ( os.path.exists(cypress_path) and os.path.exists(plugin_path) and os.path.exists(testing_library_path) + and os.path.exists(coverage_plugin_path) and cint(subprocess.getoutput("npm view cypress version")[:1]) >= 6 ): # install cypress click.secho("Installing Cypress...", fg="yellow") - frappe.commands.popen("yarn add cypress@^6 cypress-file-upload@^5 @testing-library/cypress@^8 --no-lockfile") + frappe.commands.popen("yarn add cypress@^6 cypress-file-upload@^5 @testing-library/cypress@^8 @cypress/code-coverage@^3 --no-lockfile") # run for headless mode run_or_open = 'run --browser firefox --record' if headless else 'open' - command = '{site_env} {password_env} {cypress} {run_or_open}' - formatted_command = command.format(site_env=site_env, password_env=password_env, cypress=cypress_path, run_or_open=run_or_open) + formatted_command = f'{site_env} {password_env} {coverage_env} {cypress_path} {run_or_open}' if parallel: formatted_command += ' --parallel' diff --git a/package.json b/package.json index 2283a44533..8579b9a517 100644 --- a/package.json +++ b/package.json @@ -4,7 +4,8 @@ "build": "node esbuild", "production": "node esbuild --production", "watch": "node esbuild --watch", - "snyk-protect": "snyk protect" + "snyk-protect": "snyk protect", + "coverage:report": "npx nyc report --reporter=clover" }, "repository": { "type": "git", @@ -74,5 +75,8 @@ "rtlcss": "^3.2.1", "yargs": "^16.2.0" }, - "snyk": true + "snyk": true, + "nyc": { + "report-dir": ".cypress-coverage" + } } From b82162769e500bf00a7deb3cb4266af5d74a82e9 Mon Sep 17 00:00:00 2001 From: MitulDavid Date: Mon, 20 Sep 2021 21:13:08 +0530 Subject: [PATCH 02/68] test: Fix uncaught exception in dashboard_links.js --- cypress/integration/dashboard_links.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/cypress/integration/dashboard_links.js b/cypress/integration/dashboard_links.js index b77965ee1a..155f4986be 100644 --- a/cypress/integration/dashboard_links.js +++ b/cypress/integration/dashboard_links.js @@ -51,13 +51,12 @@ context('Dashboard links', () => { cur_frm.dashboard.data.reports = [ { 'label': 'Reports', - 'items': ['Permitted Documents For User'] + 'items': ['Website Analytics'] } ]; cur_frm.dashboard.render_report_links(); - cy.get('[data-report="Permitted Documents For User"]').contains('Permitted Documents For User').click(); - cy.findByText('Permitted Documents For User'); - cy.findByPlaceholderText('User').should("have.value", "Administrator"); + cy.get('[data-report="Website Analytics"]').contains('Website Analytics').click(); + cy.findByText('Website Analytics'); }); }); }); From 1e6b30b7117f22938873058a0973a423fb3fcb61 Mon Sep 17 00:00:00 2001 From: Aradhya-Tripathi Date: Thu, 26 Aug 2021 13:46:31 +0530 Subject: [PATCH 03/68] refactor: shifted build_conditions to query module --- frappe/database/query.py | 93 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 93 insertions(+) create mode 100644 frappe/database/query.py diff --git a/frappe/database/query.py b/frappe/database/query.py new file mode 100644 index 0000000000..492dce7216 --- /dev/null +++ b/frappe/database/query.py @@ -0,0 +1,93 @@ +from typing import Tuple, Union, List, Any +import frappe +import operator + + +class Query: + def __init__(self): + self.operator_map = { + "+": operator.add, + "=": operator.eq, + "-": operator.sub, + "!=": operator.ne, + "<": operator.lt, + ">": operator.gt, + "<=": operator.le, + ">=": operator.ge, + "in": self.func_in, + "not in": self.func_not_in, + "like": self.like, + "not like": self.not_like, + "regex": self.func_regex, + "between": self.func_between + } + + self.sql_functions = ["sum", "now", "interval"] + + @staticmethod + def like(key: str, value: Any): + """Wrapper method for `LIKE` + + Args: + key (str): field + value (ANy): criterion + + Returns: + [frappe.qb]: frappe.qb object with `LIKE` + """ + return frappe.qb.Field(key).like(value) + + @staticmethod + def func_in(key, value): + return frappe.qb.Field(key).isin(value) + + @staticmethod + def not_like(key, value): + return frappe.qb.Field(key).not_like(value) + + @staticmethod + def func_not_in(key, value): + return frappe.qb.Field(key).notin(value) + + @staticmethod + def func_regex(key, value): + return frappe.qb.Field(key).regex(value) + + @staticmethod + def func_between(key, value): + return frappe.qb.Field(key)[slice(*value)] + + @staticmethod + def get_func_obj(func: str): + return frappe.qb.functions(func) + + def build_conditions(self, filters, table): + """ + filters = {columns: condition} + frappe.qb.from_(table).where(conditions) + """ + + conditions = frappe.qb.from_(table) + + def _query(key): + nonlocal conditions + value = filters.get(key) + _operator = self.operator_map["="] + if isinstance(value, (list, tuple)): + if isinstance(value[1], (list, tuple)) or value[0] in list(self.operator_map.keys())[-4:]: + _operator = self.operator_map[value[0]] + conditions = conditions.where(_operator(key, value[1])) + else: + _operator = self.operator_map[value[0]] + conditions = conditions.where(_operator(frappe.qb.Field(key), value[1])) + else: + conditions = conditions.where(_operator(frappe.qb.Field(key), value)) + + if isinstance(filters, int) or isinstance(filters, str): + filters = {"name": str(filters)} + + if filters: + for f in filters: + _query(f) + + return conditions From f36808e4ccca7c6738c5e562a0cf7a8697348712 Mon Sep 17 00:00:00 2001 From: Aradhya-Tripathi Date: Thu, 26 Aug 2021 13:48:03 +0530 Subject: [PATCH 04/68] refactor: using Query for frappe.db.delete --- frappe/database/database.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/frappe/database/database.py b/frappe/database/database.py index 0ee11ea075..28f4560a6a 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -16,6 +16,7 @@ from frappe import _ from time import time from frappe.utils import now, getdate, cast, get_datetime, get_table_name from frappe.model.utils.link_count import flush_local_link_count +from .query import Query class Database(object): @@ -55,6 +56,7 @@ class Database(object): self.password = password or frappe.conf.db_password self.value_cache = {} + self.query = Query() def setup_type_map(self): pass @@ -984,9 +986,7 @@ class Database(object): """ values = () filters = filters or kwargs.get("conditions") - table = get_table_name(doctype) - query = f"DELETE FROM `{table}`" - + query = self.query.build_conditions(table=doctype, filters=filters).delete() if "debug" not in kwargs: kwargs["debug"] = debug From ae0c5ee291847685eebb76ec7af5ba07688dbc63 Mon Sep 17 00:00:00 2001 From: Aradhya-Tripathi Date: Wed, 1 Sep 2021 17:23:17 +0530 Subject: [PATCH 05/68] refactor: moved to frappe.qb in frappe.db.delete --- frappe/database/database.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/frappe/database/database.py b/frappe/database/database.py index 28f4560a6a..78bd79edcb 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -989,11 +989,7 @@ class Database(object): query = self.query.build_conditions(table=doctype, filters=filters).delete() if "debug" not in kwargs: kwargs["debug"] = debug - - if filters: - conditions, values = self.build_conditions(filters) - query = f"{query} WHERE {conditions}" - + return self.sql(query, values, **kwargs) def truncate(self, doctype: str): From a738f8ba7b9f499b762c9286b9ada4065c7c9122 Mon Sep 17 00:00:00 2001 From: Aradhya-Tripathi Date: Thu, 26 Aug 2021 18:51:54 +0530 Subject: [PATCH 06/68] wip: adding support for sql functions in frappe ORM --- frappe/database/query.py | 133 +++++++++++++++++++++++++++++---------- 1 file changed, 100 insertions(+), 33 deletions(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index 492dce7216..a5828be230 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -1,6 +1,7 @@ -from typing import Tuple, Union, List, Any -import frappe import operator +from typing import Any, Dict, List, Tuple, Union + +import frappe class Query: @@ -25,7 +26,7 @@ class Query: self.sql_functions = ["sum", "now", "interval"] @staticmethod - def like(key: str, value: Any): + def like(key: str, value: str) -> frappe.qb: """Wrapper method for `LIKE` Args: @@ -33,61 +34,127 @@ class Query: value (ANy): criterion Returns: - [frappe.qb]: frappe.qb object with `LIKE` + frappe.qb: `frappe.qb object with `LIKE` """ return frappe.qb.Field(key).like(value) @staticmethod - def func_in(key, value): + def func_in(key: str, value: Union[List, Tuple]) -> frappe.qb: + """Wrapper method for `IN` + + Args: + key (str): field + value (ANy): criterion + + Returns: + frappe.qb: `frappe.qb object with `IN` + """ return frappe.qb.Field(key).isin(value) @staticmethod - def not_like(key, value): + def not_like(key: str, value: str) -> frappe.qb: + """Wrapper method for `NOT LIKE` + + Args: + key (str): field + value (ANy): criterion + + Returns: + frappe.qb: `frappe.qb object with `NOT LIKE` + """ return frappe.qb.Field(key).not_like(value) @staticmethod - def func_not_in(key, value): + def func_not_in(key: str, value: Union[List, Tuple]): + """Wrapper method for `NOT IN` + + Args: + key (str): field + value (ANy): criterion + + Returns: + frappe.qb: `frappe.qb object with `NOT IN` + """ return frappe.qb.Field(key).notin(value) @staticmethod - def func_regex(key, value): + def func_regex(key: str, value: str) -> frappe.qb: + """Wrapper method for `REGEX` + + Args: + key (str): field + value (ANy): criterion + + Returns: + frappe.qb: `frappe.qb object with `REGEX` + """ return frappe.qb.Field(key).regex(value) @staticmethod - def func_between(key, value): + def func_between(key: str, value: Union[List, Tuple]) -> frappe.qb: + """Wrapper method for `BETWEEN` + + Args: + key (str): field + value (ANy): criterion + + Returns: + frappe.qb: `frappe.qb object with `BETWEEN` + """ return frappe.qb.Field(key)[slice(*value)] @staticmethod - def get_func_obj(func: str): + def get_func_obj(func: str) -> frappe.qb.functions: + """Wrapper method for generating custom functions + + Args: + key (str): field + value (ANy): criterion + + Returns: + frappe.qb.functions + """ return frappe.qb.functions(func) - def build_conditions(self, filters, table): - """ - filters = {columns: condition} - frappe.qb.from_(table).where(conditions) - """ + def make_function(self, key: Any, value: Union[int, str]): + return self.operator_map[value[0]](key, value[1]) - conditions = frappe.qb.from_(table) + def dict_query(self, filters: Dict[str, Union[str, int]], table: str): + conditions = frappe.qb.from_(table) + for key in filters: + value = filters.get(key) + _operator = self.operator_map["="] - def _query(key): - nonlocal conditions - value = filters.get(key) - _operator = self.operator_map["="] - if isinstance(value, (list, tuple)): - if isinstance(value[1], (list, tuple)) or value[0] in list(self.operator_map.keys())[-4:]: - _operator = self.operator_map[value[0]] - conditions = conditions.where(_operator(key, value[1])) + if not isinstance(key, str): + conditions = conditions.where(self.make_function(key, value)) + continue + if isinstance(value, (list, tuple)): + if isinstance(value[1], (list, tuple)) or value[0] in list(self.operator_map.keys())[-4:]: + _operator = self.operator_map[value[0]] + conditions = conditions.where(_operator(key, value[1])) + else: + _operator = self.operator_map[value[0]] + conditions = conditions.where(_operator(frappe.qb.Field(key), value[1])) else: - _operator = self.operator_map[value[0]] - conditions = conditions.where(_operator(frappe.qb.Field(key), value[1])) - else: - conditions = conditions.where(_operator(frappe.qb.Field(key), value)) + conditions = conditions.where(_operator(frappe.qb.Field(key), value)) + + return conditions + + def build_conditions(self, filters: Union[Dict[str, Union[str, int]], str, int], table: str) -> frappe.qb: + """Build conditions for sql query + + Args: + filters (Union[Dict[str, Union[str, int]], str, int]): conditions built from filters provided + table (str): DocType + + Returns: + frappe.qb: frappe.qb conditions object + """ + if isinstance(filters, list): + pass if isinstance(filters, int) or isinstance(filters, str): filters = {"name": str(filters)} - if filters: - for f in filters: - _query(f) - - return conditions + if filters and isinstance(filters, dict): + return self.dict_query(filters, table) \ No newline at end of file From a1ed8dc68f326d2c754128fdf58903b54abe1138 Mon Sep 17 00:00:00 2001 From: Aradhya-Tripathi Date: Thu, 26 Aug 2021 19:15:40 +0530 Subject: [PATCH 07/68] feat: added support for interval --- frappe/query_builder/functions.py | 1 + 1 file changed, 1 insertion(+) diff --git a/frappe/query_builder/functions.py b/frappe/query_builder/functions.py index 5ccb266945..10db78b659 100644 --- a/frappe/query_builder/functions.py +++ b/frappe/query_builder/functions.py @@ -1,4 +1,5 @@ from pypika.functions import * +from pypika import Interval, Index from frappe.query_builder.utils import ImportMapper, db_type_is from frappe.query_builder.custom import GROUP_CONCAT, STRING_AGG, MATCH, TO_TSVECTOR From 3fefc6a10d95b327c5ee8510810373861875aed7 Mon Sep 17 00:00:00 2001 From: Aradhya-Tripathi Date: Thu, 26 Aug 2021 19:15:58 +0530 Subject: [PATCH 08/68] fix: added delete without filters --- frappe/database/query.py | 133 +++++++++++++++++---------------------- 1 file changed, 59 insertions(+), 74 deletions(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index a5828be230..751397a872 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -7,34 +7,33 @@ import frappe class Query: def __init__(self): self.operator_map = { - "+": operator.add, - "=": operator.eq, - "-": operator.sub, - "!=": operator.ne, - "<": operator.lt, - ">": operator.gt, - "<=": operator.le, - ">=": operator.ge, - "in": self.func_in, - "not in": self.func_not_in, - "like": self.like, - "not like": self.not_like, - "regex": self.func_regex, - "between": self.func_between + "+": operator.add, + "=": operator.eq, + "-": operator.sub, + "!=": operator.ne, + "<": operator.lt, + ">": operator.gt, + "<=": operator.le, + ">=": operator.ge, + "in": self.func_in, + "not in": self.func_not_in, + "like": self.like, + "not like": self.not_like, + "regex": self.func_regex, + "between": self.func_between } - self.sql_functions = ["sum", "now", "interval"] @staticmethod def like(key: str, value: str) -> frappe.qb: """Wrapper method for `LIKE` Args: - key (str): field - value (ANy): criterion + key (str): field + value (ANy): criterion Returns: - frappe.qb: `frappe.qb object with `LIKE` + frappe.qb: `frappe.qb object with `LIKE` """ return frappe.qb.Field(key).like(value) @@ -43,11 +42,11 @@ class Query: """Wrapper method for `IN` Args: - key (str): field - value (ANy): criterion + key (str): field + value (ANy): criterion Returns: - frappe.qb: `frappe.qb object with `IN` + frappe.qb: `frappe.qb object with `IN` """ return frappe.qb.Field(key).isin(value) @@ -56,11 +55,11 @@ class Query: """Wrapper method for `NOT LIKE` Args: - key (str): field - value (ANy): criterion + key (str): field + value (ANy): criterion Returns: - frappe.qb: `frappe.qb object with `NOT LIKE` + frappe.qb: `frappe.qb object with `NOT LIKE` """ return frappe.qb.Field(key).not_like(value) @@ -69,11 +68,11 @@ class Query: """Wrapper method for `NOT IN` Args: - key (str): field - value (ANy): criterion + key (str): field + value (ANy): criterion Returns: - frappe.qb: `frappe.qb object with `NOT IN` + frappe.qb: `frappe.qb object with `NOT IN` """ return frappe.qb.Field(key).notin(value) @@ -82,11 +81,11 @@ class Query: """Wrapper method for `REGEX` Args: - key (str): field - value (ANy): criterion + key (str): field + value (ANy): criterion Returns: - frappe.qb: `frappe.qb object with `REGEX` + frappe.qb: `frappe.qb object with `REGEX` """ return frappe.qb.Field(key).regex(value) @@ -95,66 +94,52 @@ class Query: """Wrapper method for `BETWEEN` Args: - key (str): field - value (ANy): criterion + key (str): field + value (ANy): criterion Returns: - frappe.qb: `frappe.qb object with `BETWEEN` + frappe.qb: `frappe.qb object with `BETWEEN` """ return frappe.qb.Field(key)[slice(*value)] - @staticmethod - def get_func_obj(func: str) -> frappe.qb.functions: - """Wrapper method for generating custom functions - - Args: - key (str): field - value (ANy): criterion - - Returns: - frappe.qb.functions - """ - return frappe.qb.functions(func) - def make_function(self, key: Any, value: Union[int, str]): return self.operator_map[value[0]](key, value[1]) - def dict_query(self, filters: Dict[str, Union[str, int]], table: str): - conditions = frappe.qb.from_(table) - for key in filters: - value = filters.get(key) - _operator = self.operator_map["="] - - if not isinstance(key, str): - conditions = conditions.where(self.make_function(key, value)) - continue - if isinstance(value, (list, tuple)): - if isinstance(value[1], (list, tuple)) or value[0] in list(self.operator_map.keys())[-4:]: - _operator = self.operator_map[value[0]] - conditions = conditions.where(_operator(key, value[1])) - else: - _operator = self.operator_map[value[0]] - conditions = conditions.where(_operator(frappe.qb.Field(key), value[1])) - else: - conditions = conditions.where(_operator(frappe.qb.Field(key), value)) - + def dict_query(self, table: str, filters: Dict[str, Union[str, int]]=None): + conditions = frappe.qb.from_(table) + if not filters: return conditions - def build_conditions(self, filters: Union[Dict[str, Union[str, int]], str, int], table: str) -> frappe.qb: + for key in filters: + value = filters.get(key) + _operator = self.operator_map["="] + + if not isinstance(key, str): + conditions = conditions.where(self.make_function(key, value)) + continue + if isinstance(value, (list, tuple)): + if isinstance(value[1], (list, tuple)) or value[0] in list(self.operator_map.keys())[-4:]: + _operator = self.operator_map[value[0]] + conditions = conditions.where(_operator(key, value[1])) + else: + _operator = self.operator_map[value[0]] + conditions = conditions.where(_operator(frappe.qb.Field(key), value[1])) + else: + conditions = conditions.where(_operator(frappe.qb.Field(key), value)) + + return conditions + + def build_conditions(self, table: str, filters: Union[Dict[str, Union[str, int]], str, int]=None) -> frappe.qb: """Build conditions for sql query Args: - filters (Union[Dict[str, Union[str, int]], str, int]): conditions built from filters provided - table (str): DocType + filters (Union[Dict[str, Union[str, int]], str, int]): conditions built from filters provided + table (str): DocType Returns: - frappe.qb: frappe.qb conditions object + frappe.qb: frappe.qb conditions object """ - if isinstance(filters, list): - pass - if isinstance(filters, int) or isinstance(filters, str): filters = {"name": str(filters)} - if filters and isinstance(filters, dict): - return self.dict_query(filters, table) \ No newline at end of file + return self.dict_query(filters=filters, table=table) \ No newline at end of file From 3bd665ddf69734db78703fe1363504647721cf07 Mon Sep 17 00:00:00 2001 From: Aradhya-Tripathi Date: Sat, 28 Aug 2021 02:11:14 +0530 Subject: [PATCH 09/68] refactor: using frappe.qb for frappe.db.get_values --- frappe/database/database.py | 36 ++++++++++-------------------------- 1 file changed, 10 insertions(+), 26 deletions(-) diff --git a/frappe/database/database.py b/frappe/database/database.py index 78bd79edcb..2e7cd67bd0 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -79,7 +79,7 @@ class Database(object): pass def sql(self, query, values=(), as_dict = 0, as_list = 0, formatted = 0, - debug=0, ignore_ddl=0, as_utf8=0, auto_commit=0, update=None, explain=False): + debug=0, ignore_ddl=0, as_utf8=0, auto_commit=0, update=None, explain=False, return_query=False): """Execute a SQL query and fetch all rows. :param query: SQL query. @@ -92,7 +92,7 @@ class Database(object): :param as_utf8: Encode values as UTF 8. :param auto_commit: Commit after executing the query. :param update: Update this dict to all rows (if returned `as_dict`). - + :param return_query: Returns query with out executing it. Examples: # return customer names as dicts @@ -107,6 +107,8 @@ class Database(object): """ query = str(query) + if return_query: + return query if re.search(r'ifnull\(', query, flags=re.IGNORECASE): # replaces ifnull in query with coalesce query = re.sub(r'ifnull\(', 'coalesce(', query, flags=re.IGNORECASE) @@ -426,9 +428,8 @@ class Database(object): (doctype, filters, fieldname) in self.value_cache: return self.value_cache[(doctype, filters, fieldname)] - if not order_by: order_by = 'modified desc' - if isinstance(filters, list): + if not order_by: order_by = 'modified desc' out = self._get_value_for_many_names(doctype, filters, fieldname, debug=debug) else: @@ -441,6 +442,7 @@ class Database(object): if (filters is not None) and (filters!=doctype or doctype=="DocType"): try: + if not order_by: order_by = 'modified' out = self._get_values_from_table(fields, filters, doctype, as_dict, debug, order_by, update, for_update=for_update) except Exception as e: if ignore and (frappe.db.is_missing_column(e) or frappe.db.is_table_missing(e)): @@ -569,32 +571,14 @@ class Database(object): return self.get_single_value(*args, **kwargs) def _get_values_from_table(self, fields, filters, doctype, as_dict, debug, order_by=None, update=None, for_update=False): - fl = [] if isinstance(fields, (list, tuple)): - for f in fields: - if "(" in f or " as " in f: # function - fl.append(f) - else: - fl.append("`" + f + "`") - fl = ", ".join(fl) + query = self.query.build_conditions(table=doctype, filters=filters, orderby=order_by).select(*fields) else: - fl = fields if fields=="*": + query = self.query.build_conditions(table=doctype, filters=filters, orderby=order_by).select(fields) as_dict = True - - conditions, values = self.build_conditions(filters) - - order_by = ("order by " + order_by) if order_by else "" - - r = self.sql("select {fields} from `tab{doctype}` {where} {conditions} {order_by} {for_update}" - .format( - for_update = 'for update' if for_update else '', - fields = fl, - doctype = doctype, - where = "where" if conditions else "", - conditions = conditions, - order_by = order_by), - values, as_dict=as_dict, debug=debug, update=update) + print(query) + r = self.sql(query, as_dict=as_dict, debug=debug, update=update) return r From 1af8f5289f6a98909d23810da66ab9af169221e8 Mon Sep 17 00:00:00 2001 From: Aradhya-Tripathi Date: Sat, 28 Aug 2021 02:12:13 +0530 Subject: [PATCH 10/68] feat: added support for orderby --- frappe/database/query.py | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index 751397a872..4f5646f9a1 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -1,6 +1,8 @@ import operator from typing import Any, Dict, List, Tuple, Union +from frappe.query_builder import Order + import frappe @@ -105,7 +107,19 @@ class Query: def make_function(self, key: Any, value: Union[int, str]): return self.operator_map[value[0]](key, value[1]) - def dict_query(self, table: str, filters: Dict[str, Union[str, int]]=None): + def dict_query(self, table: str, filters: Dict[str, Union[str, int]] = None, + orderby:str = None, order:Order = None): + """Generate condition object using filters + + Args: + table (str): DocType + filters (Dict[str, Union[str, int]], optional): Conditions. Defaults to None. + orderby (str, optional): field to order by. Defaults to None. + order (Order, optional): order. Defaults to None. + + Returns: + condition: conditions object + """ conditions = frappe.qb.from_(table) if not filters: return conditions @@ -126,10 +140,13 @@ class Query: conditions = conditions.where(_operator(frappe.qb.Field(key), value[1])) else: conditions = conditions.where(_operator(frappe.qb.Field(key), value)) - + if orderby: + order = order if order else Order.desc + return conditions.orderby(orderby, order) return conditions - def build_conditions(self, table: str, filters: Union[Dict[str, Union[str, int]], str, int]=None) -> frappe.qb: + def build_conditions(self, table: str, filters: Union[Dict[str, Union[str, int]], str, int] = None, + orderby: str = None, order: Order = None) -> frappe.qb: """Build conditions for sql query Args: @@ -142,4 +159,4 @@ class Query: if isinstance(filters, int) or isinstance(filters, str): filters = {"name": str(filters)} - return self.dict_query(filters=filters, table=table) \ No newline at end of file + return self.dict_query(filters=filters, table=table, orderby=orderby, order=order) \ No newline at end of file From 601d75ad96dd2e1cb0dbc9aa839debf59f9973de Mon Sep 17 00:00:00 2001 From: Aradhya-Tripathi Date: Sat, 28 Aug 2021 02:13:21 +0530 Subject: [PATCH 11/68] refactor: modified pypika imports --- frappe/query_builder/builder.py | 7 +++++-- frappe/query_builder/functions.py | 1 - frappe/query_builder/utils.py | 4 ++-- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/frappe/query_builder/builder.py b/frappe/query_builder/builder.py index 1d9b9f0e40..0c78a7e89d 100644 --- a/frappe/query_builder/builder.py +++ b/frappe/query_builder/builder.py @@ -1,14 +1,17 @@ from pypika import MySQLQuery, Order, PostgreSQLQuery, terms from pypika.queries import Schema, Table from frappe.utils import get_table_name - - +from pypika.terms import Function class Base: terms = terms desc = Order.desc Schema = Schema Table = Table + @staticmethod + def functions(name: str, *args, **kwargs) -> Function: + return Function(name, *args, **kwargs) + @staticmethod def DocType(table_name: str, *args, **kwargs) -> Table: table_name = get_table_name(table_name) diff --git a/frappe/query_builder/functions.py b/frappe/query_builder/functions.py index 10db78b659..5ccb266945 100644 --- a/frappe/query_builder/functions.py +++ b/frappe/query_builder/functions.py @@ -1,5 +1,4 @@ from pypika.functions import * -from pypika import Interval, Index from frappe.query_builder.utils import ImportMapper, db_type_is from frappe.query_builder.custom import GROUP_CONCAT, STRING_AGG, MATCH, TO_TSVECTOR diff --git a/frappe/query_builder/utils.py b/frappe/query_builder/utils.py index c217d0975e..d915b85897 100644 --- a/frappe/query_builder/utils.py +++ b/frappe/query_builder/utils.py @@ -1,5 +1,5 @@ from enum import Enum -from typing import Any, Callable, Dict, get_type_hints +from typing import Any, Callable, Dict, Union, get_type_hints from importlib import import_module from pypika import Query @@ -26,7 +26,7 @@ class BuilderIdentificationFailed(Exception): def __init__(self): super().__init__("Couldn't guess builder") -def get_query_builder(type_of_db: str) -> Query: +def get_query_builder(type_of_db: str) -> Union[Postgres, MariaDB]: """[return the query builder object] Args: From ead9369586f6a036c6abdba1dc88ffcca44a2d7b Mon Sep 17 00:00:00 2001 From: Aradhya-Tripathi Date: Sat, 28 Aug 2021 02:23:53 +0530 Subject: [PATCH 12/68] fix(minor): fixed orderby in build_conditions --- frappe/database/query.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index 4f5646f9a1..03273c7a97 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -142,7 +142,7 @@ class Query: conditions = conditions.where(_operator(frappe.qb.Field(key), value)) if orderby: order = order if order else Order.desc - return conditions.orderby(orderby, order) + return conditions.orderby(orderby, order=order) return conditions def build_conditions(self, table: str, filters: Union[Dict[str, Union[str, int]], str, int] = None, From 9c3229018e28573c29a037b5e1c36d95a9c19388 Mon Sep 17 00:00:00 2001 From: Aradhya-Tripathi Date: Thu, 2 Sep 2021 00:00:24 +0530 Subject: [PATCH 13/68] refactor: moved count to use frappe.qb --- frappe/database/database.py | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/frappe/database/database.py b/frappe/database/database.py index 2e7cd67bd0..135637ccb9 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -572,10 +572,10 @@ class Database(object): def _get_values_from_table(self, fields, filters, doctype, as_dict, debug, order_by=None, update=None, for_update=False): if isinstance(fields, (list, tuple)): - query = self.query.build_conditions(table=doctype, filters=filters, orderby=order_by).select(*fields) + query = self.query.build_conditions(table=doctype, filters=filters, orderby=order_by, for_update=for_update).select(*fields) else: if fields=="*": - query = self.query.build_conditions(table=doctype, filters=filters, orderby=order_by).select(fields) + query = self.query.build_conditions(table=doctype, filters=filters, orderby=order_by, for_update=for_update).select(fields) as_dict = True print(query) r = self.sql(query, as_dict=as_dict, debug=debug, update=update) @@ -807,22 +807,19 @@ class Database(object): def count(self, dt, filters=None, debug=False, cache=False): """Returns `COUNT(*)` for given DocType and filters.""" + from frappe.query_builder.functions import Count if cache and not filters: cache_count = frappe.cache().get_value('doctype:count:{}'.format(dt)) if cache_count is not None: return cache_count + query = self.query.build_conditions(table=dt, filters=filters).select(Count("*")) if filters: - conditions, filters = self.build_conditions(filters) - count = self.sql("""select count(*) - from `tab%s` where %s""" % (dt, conditions), filters, debug=debug)[0][0] + count = self.sql(query, debug=debug)[0][0] return count else: - count = self.sql("""select count(*) - from `tab%s`""" % (dt,))[0][0] - + count = self.sql(query, debug=debug)[0][0] if cache: frappe.cache().set_value('doctype:count:{}'.format(dt), count, expires_in_sec = 86400) - return count def sum(self, dt, fieldname, filters=None): From 4e75489f4c7f4fc34690d3beb67dd4259621203e Mon Sep 17 00:00:00 2001 From: Aradhya-Tripathi Date: Fri, 3 Sep 2021 16:52:30 +0530 Subject: [PATCH 14/68] feat: added support for Criterion objects --- frappe/database/query.py | 100 +++++++++++++++++++++++++++++++++------ 1 file changed, 85 insertions(+), 15 deletions(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index 03273c7a97..32c65bce0e 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -1,9 +1,8 @@ import operator from typing import Any, Dict, List, Tuple, Union -from frappe.query_builder import Order - import frappe +from frappe.query_builder import Criterion, Order class Query: @@ -107,20 +106,91 @@ class Query: def make_function(self, key: Any, value: Union[int, str]): return self.operator_map[value[0]](key, value[1]) - def dict_query(self, table: str, filters: Dict[str, Union[str, int]] = None, - orderby:str = None, order:Order = None): - """Generate condition object using filters + @staticmethod + def change_orderby(order: str): + """Convert orderby to standart Order object + + Args: + order (str): Field, order + + Returns: + tuple: field, order + """ + order = order.split() + if order[1].lower() == "asc": + orderby, order = order[0], Order.asc + return orderby, order + orderby, order = order[0], Order.desc + return orderby, order + + @staticmethod + def get_condition(table: str, **kwargs) -> frappe.qb: + """Get initial table object Args: table (str): DocType - filters (Dict[str, Union[str, int]], optional): Conditions. Defaults to None. - orderby (str, optional): field to order by. Defaults to None. - order (Order, optional): order. Defaults to None. + + Returns: + frappe.qb: DocType with initial condition + """ + if kwargs.get("update"): + return frappe.qb.update(table) + if kwargs.get("into"): + return frappe.qb.into(table) + return frappe.qb.from_(table) + + def criterion_query(self, table: str, criterion: Criterion, **kwargs) -> frappe.qb: + """Generate filters from Criterion objects + + Args: + table (str): DocType + criterion (Criterion): Filters + + Returns: + frappe.qb: condition object + """ + condition = self.get_condition(table, **kwargs) + return condition.where(criterion) + + + def add_conditions(self, conditions: frappe.qb, **kwargs): + """Adding additional conditions + + Args: + conditions (frappe.qb): built conditions + + Returns: + conditions (frappe.qb): frappe.qb object + """ + if kwargs.get("orderby"): + orderby = kwargs.get("orderby") + order = kwargs.get("order") if kwargs.get("order") else Order.desc + if isinstance(orderby, str) and len(orderby.split()) > 1: + orderby, order = self.change_orderby(orderby) + conditions = conditions.orderby(orderby, order=order) + + if kwargs.get("limit"): + conditions = conditions.limit(kwargs.get("limit")) + + if kwargs.get("distinct"): + conditions = conditions.distinct() + + if kwargs.get("for_update"): + conditions = conditions.for_update() + + return conditions + + def dict_query(self, table: str, filters: Dict[str, Union[str, int]] = None, **kwargs): + """Build conditions using the given filters + + Args: + table (str): DocType + filters (Dict[str, Union[str, int]], optional): Filters. Defaults to None. Returns: condition: conditions object """ - conditions = frappe.qb.from_(table) + conditions = self.get_condition(table, **kwargs) if not filters: return conditions @@ -140,13 +210,10 @@ class Query: conditions = conditions.where(_operator(frappe.qb.Field(key), value[1])) else: conditions = conditions.where(_operator(frappe.qb.Field(key), value)) - if orderby: - order = order if order else Order.desc - return conditions.orderby(orderby, order=order) + conditions = self.add_conditions(conditions, **kwargs) return conditions - def build_conditions(self, table: str, filters: Union[Dict[str, Union[str, int]], str, int] = None, - orderby: str = None, order: Order = None) -> frappe.qb: + def build_conditions(self, table: str, filters: Union[Dict[str, Union[str, int]], str, int] = None, **kwargs) -> frappe.qb: """Build conditions for sql query Args: @@ -156,7 +223,10 @@ class Query: Returns: frappe.qb: frappe.qb conditions object """ + if isinstance(filters, Criterion): + return self.criterion_query(table, filters, **kwargs) + if isinstance(filters, int) or isinstance(filters, str): filters = {"name": str(filters)} - return self.dict_query(filters=filters, table=table, orderby=orderby, order=order) \ No newline at end of file + return self.dict_query(filters=filters, table=table, **kwargs) \ No newline at end of file From a726c0f7e8ffd1e66afdb2ce3533fa45af7dc938 Mon Sep 17 00:00:00 2001 From: Aradhya-Tripathi Date: Wed, 8 Sep 2021 02:33:27 +0530 Subject: [PATCH 15/68] feat: Added support for list of lists --- frappe/database/query.py | 49 ++++++++++++++++++++++++++++++---------- 1 file changed, 37 insertions(+), 12 deletions(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index 32c65bce0e..50ebc908c4 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -2,8 +2,7 @@ import operator from typing import Any, Dict, List, Tuple, Union import frappe -from frappe.query_builder import Criterion, Order - +from frappe.query_builder import Criterion, Order, Field class Query: def __init__(self): @@ -36,7 +35,7 @@ class Query: Returns: frappe.qb: `frappe.qb object with `LIKE` """ - return frappe.qb.Field(key).like(value) + return Field(key).like(value) @staticmethod def func_in(key: str, value: Union[List, Tuple]) -> frappe.qb: @@ -49,7 +48,7 @@ class Query: Returns: frappe.qb: `frappe.qb object with `IN` """ - return frappe.qb.Field(key).isin(value) + return Field(key).isin(value) @staticmethod def not_like(key: str, value: str) -> frappe.qb: @@ -62,7 +61,7 @@ class Query: Returns: frappe.qb: `frappe.qb object with `NOT LIKE` """ - return frappe.qb.Field(key).not_like(value) + return Field(key).not_like(value) @staticmethod def func_not_in(key: str, value: Union[List, Tuple]): @@ -75,7 +74,7 @@ class Query: Returns: frappe.qb: `frappe.qb object with `NOT IN` """ - return frappe.qb.Field(key).notin(value) + return Field(key).notin(value) @staticmethod def func_regex(key: str, value: str) -> frappe.qb: @@ -88,7 +87,7 @@ class Query: Returns: frappe.qb: `frappe.qb object with `REGEX` """ - return frappe.qb.Field(key).regex(value) + return Field(key).regex(value) @staticmethod def func_between(key: str, value: Union[List, Tuple]) -> frappe.qb: @@ -101,7 +100,7 @@ class Query: Returns: frappe.qb: `frappe.qb object with `BETWEEN` """ - return frappe.qb.Field(key)[slice(*value)] + return Field(key)[slice(*value)] def make_function(self, key: Any, value: Union[int, str]): return self.operator_map[value[0]](key, value[1]) @@ -180,8 +179,31 @@ class Query: return conditions - def dict_query(self, table: str, filters: Dict[str, Union[str, int]] = None, **kwargs): - """Build conditions using the given filters + def misc_query(self, table: str, filters: Union[List, Tuple] = None, **kwargs): + """Build conditions using the given Lists or Tuple filters + + Args: + table (str): DocType + filters (Union[List, Tuple], optional): Filters. Defaults to None. + """ + conditions = self.get_condition(table, **kwargs) + if not filters: + return conditions + if isinstance(filters, list): + for f in filters: + if not isinstance(f, (list, tuple)): + _operator = self.operator_map[filters[1]] + conditions = conditions.where(_operator(Field(filters[0]), Field(filters[2]))) + break + else: + _operator = self.operator_map[f[1]] + conditions = conditions.where(_operator(Field(f[0]), Field(f[2]))) + + conditions = self.add_conditions(conditions, **kwargs) + return conditions + + def dict_query(self, table: str, filters: Dict[str, Union[str, int]] = None, **kwargs) -> frappe.qb: + """Build conditions using the given dictionary filters Args: table (str): DocType @@ -207,9 +229,9 @@ class Query: conditions = conditions.where(_operator(key, value[1])) else: _operator = self.operator_map[value[0]] - conditions = conditions.where(_operator(frappe.qb.Field(key), value[1])) + conditions = conditions.where(_operator(Field(key), value[1])) else: - conditions = conditions.where(_operator(frappe.qb.Field(key), value)) + conditions = conditions.where(_operator(Field(key), value)) conditions = self.add_conditions(conditions, **kwargs) return conditions @@ -229,4 +251,7 @@ class Query: if isinstance(filters, int) or isinstance(filters, str): filters = {"name": str(filters)} + if isinstance(filters, (list, tuple)): + return self.misc_query(table, filters, **kwargs) + return self.dict_query(filters=filters, table=table, **kwargs) \ No newline at end of file From 1c2e470792cad3c18ce854409d48e3d01b633e53 Mon Sep 17 00:00:00 2001 From: Aradhya-Tripathi Date: Thu, 9 Sep 2021 16:35:49 +0530 Subject: [PATCH 16/68] fix: fixed strings and integer representation in build_conditions --- frappe/database/database.py | 4 ++-- frappe/database/query.py | 7 +++++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/frappe/database/database.py b/frappe/database/database.py index 135637ccb9..acc6f804b4 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -429,7 +429,7 @@ class Database(object): return self.value_cache[(doctype, filters, fieldname)] if isinstance(filters, list): - if not order_by: order_by = 'modified desc' + order_by = order_by or "modified desc" out = self._get_value_for_many_names(doctype, filters, fieldname, debug=debug) else: @@ -442,7 +442,7 @@ class Database(object): if (filters is not None) and (filters!=doctype or doctype=="DocType"): try: - if not order_by: order_by = 'modified' + order_by = order_by or "modified desc" out = self._get_values_from_table(fields, filters, doctype, as_dict, debug, order_by, update, for_update=for_update) except Exception as e: if ignore and (frappe.db.is_missing_column(e) or frappe.db.is_table_missing(e)): diff --git a/frappe/database/query.py b/frappe/database/query.py index 50ebc908c4..c0c1dcd850 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -193,11 +193,14 @@ class Query: for f in filters: if not isinstance(f, (list, tuple)): _operator = self.operator_map[filters[1]] - conditions = conditions.where(_operator(Field(filters[0]), Field(filters[2]))) + if not isinstance(filters[0], str): + conditions = self.make_function(filters[0], filters[2]) + break + conditions = conditions.where(_operator(Field(filters[0]), filters[2])) break else: _operator = self.operator_map[f[1]] - conditions = conditions.where(_operator(Field(f[0]), Field(f[2]))) + conditions = conditions.where(_operator(Field(f[0]), f[2])) conditions = self.add_conditions(conditions, **kwargs) return conditions From 73eb7806a85aacf641374fd356d08ebe87a2e2d8 Mon Sep 17 00:00:00 2001 From: Aradhya-Tripathi Date: Thu, 9 Sep 2021 18:05:44 +0530 Subject: [PATCH 17/68] refactor: removed aggregation from database.py refactor: moved aggregate to frappe.query --- .../package_release/package_release.py | 10 ++- frappe/database/database.py | 77 ------------------- frappe/tests/test_db.py | 6 -- frappe/utils/safe_exec.py | 7 +- 4 files changed, 8 insertions(+), 92 deletions(-) diff --git a/frappe/core/doctype/package_release/package_release.py b/frappe/core/doctype/package_release/package_release.py index 1fb8796882..fdad2edfc6 100644 --- a/frappe/core/doctype/package_release/package_release.py +++ b/frappe/core/doctype/package_release/package_release.py @@ -7,15 +7,19 @@ from frappe.modules.export_file import export_doc import os import subprocess + class PackageRelease(Document): def set_version(self): # set the next patch release by default + from frappe.query_builder.functions import Max + from frappe.query_builder import Field + if not self.major: - self.major = frappe.db.max('Package Release', 'major', dict(package=self.package)) + self.major = frappe.qb.from_("Package Release").where(Field(self.package) == "package").select(Max("major")).run()[0][0] or 0 if not self.minor: - self.minor = frappe.db.max('Package Release', 'minor', dict(package=self.package)) + self.minor = frappe.qb.from_("Package Release").where(Field(self.package) == "package").select(Max("minor")).run()[0][0] or 0 if not self.patch: - self.patch = frappe.db.max('Package Release', 'patch', dict(package=self.package)) + 1 + self.patch = frappe.qb.from_("Package Release").where(Field(self.package) == "package").select(Max("patch")).run()[0][0] or 1 def autoname(self): self.set_version() diff --git a/frappe/database/database.py b/frappe/database/database.py index acc6f804b4..2b7c52cd47 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -314,59 +314,6 @@ class Database(object): nres.append(nr) return nres - def build_conditions(self, filters): - """Convert filters sent as dict, lists to SQL conditions. filter's key - is passed by map function, build conditions like: - - * ifnull(`fieldname`, default_value) = %(fieldname)s - * `fieldname` [=, !=, >, >=, <, <=] %(fieldname)s - """ - conditions = [] - values = {} - def _build_condition(key): - """ - filter's key is passed by map function - build conditions like: - * ifnull(`fieldname`, default_value) = %(fieldname)s - * `fieldname` [=, !=, >, >=, <, <=] %(fieldname)s - """ - _operator = "=" - _rhs = " %(" + key + ")s" - value = filters.get(key) - values[key] = value - if isinstance(value, (list, tuple)): - # value is a tuple like ("!=", 0) - _operator = value[0].lower() - values[key] = value[1] - if isinstance(value[1], (tuple, list)): - # value is a list in tuple ("in", ("A", "B")) - _rhs = " ({0})".format(", ".join(self.escape(v) for v in value[1])) - del values[key] - - if _operator not in ["=", "!=", ">", ">=", "<", "<=", "like", "in", "not in", "not like"]: - _operator = "=" - - if "[" in key: - split_key = key.split("[") - condition = "coalesce(`" + split_key[0] + "`, " + split_key[1][:-1] + ") " \ - + _operator + _rhs - else: - condition = "`" + key + "` " + _operator + _rhs - - conditions.append(condition) - - if isinstance(filters, int): - # docname is a number, convert to string - filters = str(filters) - - if isinstance(filters, str): - filters = { "name": filters } - - for f in filters: - _build_condition(f) - - return " and ".join(conditions), values - def get(self, doctype, filters=None, as_dict=True, cache=False): """Returns `get_value` with fieldname='*'""" return self.get_value(doctype, filters, "*", as_dict=as_dict, cache=cache) @@ -822,30 +769,6 @@ class Database(object): frappe.cache().set_value('doctype:count:{}'.format(dt), count, expires_in_sec = 86400) return count - def sum(self, dt, fieldname, filters=None): - return self._get_aggregation('SUM', dt, fieldname, filters) - - def avg(self, dt, fieldname, filters=None): - return self._get_aggregation('AVG', dt, fieldname, filters) - - def min(self, dt, fieldname, filters=None): - return self._get_aggregation('MIN', dt, fieldname, filters) - - def max(self, dt, fieldname, filters=None): - return self._get_aggregation('MAX', dt, fieldname, filters) - - def _get_aggregation(self, function, dt, fieldname, filters=None): - if not self.has_column(dt, fieldname): - frappe.throw(frappe._('Invalid column'), self.InvalidColumnName) - - query = f'SELECT {function}({fieldname}) AS value FROM `tab{dt}`' - values = () - if filters: - conditions, values = self.build_conditions(filters) - query = f"{query} WHERE {conditions}" - - return self.sql(query, values)[0][0] or 0 - @staticmethod def format_date(date): return getdate(date).strftime("%Y-%m-%d") diff --git a/frappe/tests/test_db.py b/frappe/tests/test_db.py index 72bec78db7..20f38dc964 100644 --- a/frappe/tests/test_db.py +++ b/frappe/tests/test_db.py @@ -46,12 +46,6 @@ class TestDB(unittest.TestCase): def test_escape(self): frappe.db.escape("香港濟生堂製藥有限公司 - IT".encode("utf-8")) - def test_aggregation(self): - self.assertTrue(type(frappe.db.sum('DocField', 'permlevel', dict(parent=('like', 'doc')))) in (int, float)) - self.assertTrue(type(frappe.db.avg('DocField', 'permlevel')) in (int, float)) - self.assertTrue(type(frappe.db.min('DocField', 'permlevel')) in (int, float)) - self.assertTrue(type(frappe.db.max('DocField', 'permlevel')) in (int, float)) - def test_get_single_value(self): #setup values_dict = { diff --git a/frappe/utils/safe_exec.py b/frappe/utils/safe_exec.py index e18c498b3c..7ccd80e346 100644 --- a/frappe/utils/safe_exec.py +++ b/frappe/utils/safe_exec.py @@ -147,12 +147,7 @@ def get_safe_globals(): get_single_value = frappe.db.get_single_value, get_default = frappe.db.get_default, escape = frappe.db.escape, - sql = read_sql, - sum = frappe.db.sum, - avg = frappe.db.avg, - count = frappe.db.count, - min = frappe.db.min, - max = frappe.db.max + sql = read_sql ) if frappe.response: From c4f5db9f96788aa9058c44d50c260df1fee5bbb7 Mon Sep 17 00:00:00 2001 From: Aradhya-Tripathi Date: Mon, 13 Sep 2021 23:25:31 +0530 Subject: [PATCH 18/68] fix: fixed set_version in package_release --- frappe/core/doctype/package_release/package_release.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/frappe/core/doctype/package_release/package_release.py b/frappe/core/doctype/package_release/package_release.py index fdad2edfc6..30ad46c68e 100644 --- a/frappe/core/doctype/package_release/package_release.py +++ b/frappe/core/doctype/package_release/package_release.py @@ -15,11 +15,11 @@ class PackageRelease(Document): from frappe.query_builder import Field if not self.major: - self.major = frappe.qb.from_("Package Release").where(Field(self.package) == "package").select(Max("major")).run()[0][0] or 0 + self.major = frappe.qb.from_("Package Release").where(Field("package") == self.package).select(Max("major")).run()[0][0] or 0 if not self.minor: - self.minor = frappe.qb.from_("Package Release").where(Field(self.package) == "package").select(Max("minor")).run()[0][0] or 0 + self.minor = frappe.qb.from_("Package Release").where(Field("package") == self.package).select(Max("minor")).run()[0][0] or 0 if not self.patch: - self.patch = frappe.qb.from_("Package Release").where(Field(self.package) == "package").select(Max("patch")).run()[0][0] or 1 + self.patch = frappe.qb.from_("Package Release").where(Field("package") == self.package).select(Max("patch")).run()[0][0] or 1 def autoname(self): self.set_version() From 4f7446c722892216ef5049e672cdf056e16cefe3 Mon Sep 17 00:00:00 2001 From: Aradhya-Tripathi Date: Wed, 15 Sep 2021 23:54:09 +0530 Subject: [PATCH 19/68] refactor: changed return_query to run in database.py --- frappe/database/database.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/frappe/database/database.py b/frappe/database/database.py index 2b7c52cd47..3abcea735b 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -79,7 +79,7 @@ class Database(object): pass def sql(self, query, values=(), as_dict = 0, as_list = 0, formatted = 0, - debug=0, ignore_ddl=0, as_utf8=0, auto_commit=0, update=None, explain=False, return_query=False): + debug=0, ignore_ddl=0, as_utf8=0, auto_commit=0, update=None, explain=False, run=True): """Execute a SQL query and fetch all rows. :param query: SQL query. @@ -92,7 +92,7 @@ class Database(object): :param as_utf8: Encode values as UTF 8. :param auto_commit: Commit after executing the query. :param update: Update this dict to all rows (if returned `as_dict`). - :param return_query: Returns query with out executing it. + :param run: Returns query without excuting it if False. Examples: # return customer names as dicts @@ -107,7 +107,7 @@ class Database(object): """ query = str(query) - if return_query: + if not run: return query if re.search(r'ifnull\(', query, flags=re.IGNORECASE): # replaces ifnull in query with coalesce From 8c45157fea54f991ffe60cb46bcca95c8e7797a6 Mon Sep 17 00:00:00 2001 From: Aradhya-Tripathi Date: Fri, 17 Sep 2021 23:36:48 +0530 Subject: [PATCH 20/68] feat: added aggregation using build_conditions --- frappe/database/database.py | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/frappe/database/database.py b/frappe/database/database.py index 3abcea735b..d14b90d9ec 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -14,8 +14,10 @@ import frappe.model.meta from frappe import _ from time import time -from frappe.utils import now, getdate, cast, get_datetime, get_table_name +from frappe.utils import now, getdate, cast, get_datetime from frappe.model.utils.link_count import flush_local_link_count +from frappe.query_builder.functions import Count +from frappe.query_builder.functions import Min, Max, Avg, Sum from .query import Query @@ -92,7 +94,7 @@ class Database(object): :param as_utf8: Encode values as UTF 8. :param auto_commit: Commit after executing the query. :param update: Update this dict to all rows (if returned `as_dict`). - :param run: Returns query without excuting it if False. + :param run: Returns query without executing it if False. Examples: # return customer names as dicts @@ -376,7 +378,7 @@ class Database(object): return self.value_cache[(doctype, filters, fieldname)] if isinstance(filters, list): - order_by = order_by or "modified desc" + order_by = order_by or "modified_desc" out = self._get_value_for_many_names(doctype, filters, fieldname, debug=debug) else: @@ -389,7 +391,7 @@ class Database(object): if (filters is not None) and (filters!=doctype or doctype=="DocType"): try: - order_by = order_by or "modified desc" + order_by = order_by or "modified" out = self._get_values_from_table(fields, filters, doctype, as_dict, debug, order_by, update, for_update=for_update) except Exception as e: if ignore and (frappe.db.is_missing_column(e) or frappe.db.is_table_missing(e)): @@ -752,9 +754,20 @@ class Database(object): except Exception: return None + def min(self, dt, fieldname, filters=None, **kwargs): + return self.query.build_conditions(dt, filters=filters).select(Min(fieldname)).run(**kwargs)[0][0] or 0 + + def max(self, dt, fieldname, filters=None, **kwargs): + return self.query.build_conditions(dt, filters=filters).select(Max(fieldname)).run(**kwargs)[0][0] or 0 + + def avg(self, dt, fieldname, filters=None, **kwargs): + return self.query.build_conditions(dt, filters=filters).select(Avg(fieldname)).run(**kwargs)[0][0] or 0 + + def sum(self, dt, fieldname, filters=None, **kwargs): + return self.query.build_conditions(dt, filters=filters).select(Sum(fieldname)).run(**kwargs)[0][0] or 0 + def count(self, dt, filters=None, debug=False, cache=False): """Returns `COUNT(*)` for given DocType and filters.""" - from frappe.query_builder.functions import Count if cache and not filters: cache_count = frappe.cache().get_value('doctype:count:{}'.format(dt)) if cache_count is not None: From 0a09f93889d162fa21234bdde953540b27c454b3 Mon Sep 17 00:00:00 2001 From: Aradhya-Tripathi Date: Fri, 17 Sep 2021 23:37:34 +0530 Subject: [PATCH 21/68] refactor: moved non-instance specific functions out of the class --- frappe/database/query.py | 256 ++++++++++++++++++++------------------- 1 file changed, 133 insertions(+), 123 deletions(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index c0c1dcd850..e5a7eff498 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -4,140 +4,150 @@ from typing import Any, Dict, List, Tuple, Union import frappe from frappe.query_builder import Criterion, Order, Field -class Query: - def __init__(self): - self.operator_map = { - "+": operator.add, - "=": operator.eq, - "-": operator.sub, - "!=": operator.ne, - "<": operator.lt, - ">": operator.gt, - "<=": operator.le, - ">=": operator.ge, - "in": self.func_in, - "not in": self.func_not_in, - "like": self.like, - "not like": self.not_like, - "regex": self.func_regex, - "between": self.func_between - } + +def like(key: str, value: str) -> frappe.qb: + """Wrapper method for `LIKE` + + Args: + key (str): field + value (str): criterion + + Returns: + frappe.qb: `frappe.qb object with `LIKE` + """ + return Field(key).like(value) - @staticmethod - def like(key: str, value: str) -> frappe.qb: - """Wrapper method for `LIKE` +def func_in(key: str, value: Union[List, Tuple]) -> frappe.qb: + """Wrapper method for `IN` - Args: - key (str): field - value (ANy): criterion + Args: + key (str): field + value (Union[int, str]): criterion - Returns: - frappe.qb: `frappe.qb object with `LIKE` - """ - return Field(key).like(value) + Returns: + frappe.qb: `frappe.qb object with `IN` + """ + return Field(key).isin(value) - @staticmethod - def func_in(key: str, value: Union[List, Tuple]) -> frappe.qb: - """Wrapper method for `IN` - Args: - key (str): field - value (ANy): criterion +def not_like(key: str, value: str) -> frappe.qb: + """Wrapper method for `NOT LIKE` - Returns: - frappe.qb: `frappe.qb object with `IN` - """ - return Field(key).isin(value) + Args: + key (str): field + value (str): criterion - @staticmethod - def not_like(key: str, value: str) -> frappe.qb: - """Wrapper method for `NOT LIKE` + Returns: + frappe.qb: `frappe.qb object with `NOT LIKE` + """ + return Field(key).not_like(value) - Args: - key (str): field - value (ANy): criterion - Returns: - frappe.qb: `frappe.qb object with `NOT LIKE` - """ - return Field(key).not_like(value) +def func_not_in(key: str, value: Union[List, Tuple]): + """Wrapper method for `NOT IN` - @staticmethod - def func_not_in(key: str, value: Union[List, Tuple]): - """Wrapper method for `NOT IN` + Args: + key (str): field + value (Union[int, str]): criterion - Args: - key (str): field - value (ANy): criterion + Returns: + frappe.qb: `frappe.qb object with `NOT IN` + """ + return Field(key).notin(value) - Returns: - frappe.qb: `frappe.qb object with `NOT IN` - """ - return Field(key).notin(value) - @staticmethod - def func_regex(key: str, value: str) -> frappe.qb: - """Wrapper method for `REGEX` +def func_regex(key: str, value: str) -> frappe.qb: + """Wrapper method for `REGEX` - Args: - key (str): field - value (ANy): criterion + Args: + key (str): field + value (str): criterion - Returns: - frappe.qb: `frappe.qb object with `REGEX` - """ - return Field(key).regex(value) + Returns: + frappe.qb: `frappe.qb object with `REGEX` + """ + return Field(key).regex(value) - @staticmethod - def func_between(key: str, value: Union[List, Tuple]) -> frappe.qb: - """Wrapper method for `BETWEEN` - Args: - key (str): field - value (ANy): criterion +def func_between(key: str, value: Union[List, Tuple]) -> frappe.qb: + """Wrapper method for `BETWEEN` - Returns: - frappe.qb: `frappe.qb object with `BETWEEN` - """ - return Field(key)[slice(*value)] + Args: + key (str): field + value (Union[int, str]): criterion - def make_function(self, key: Any, value: Union[int, str]): - return self.operator_map[value[0]](key, value[1]) + Returns: + frappe.qb: `frappe.qb object with `BETWEEN` + """ + return Field(key)[slice(*value)] - @staticmethod - def change_orderby(order: str): - """Convert orderby to standart Order object +def make_function(key: Any, value: Union[int, str]): + """returns fucntion query - Args: - order (str): Field, order + Args: + key (Any): field + value (Union[int, str]): criterion - Returns: - tuple: field, order - """ - order = order.split() - if order[1].lower() == "asc": - orderby, order = order[0], Order.asc - return orderby, order - orderby, order = order[0], Order.desc + Returns: + frappe.qb: frappe.qb object + """ + return OPERATOR_MAP[value[0]](key, value[1]) + + +def change_orderby(order: str): + """Convert orderby to standart Order object + + Args: + order (str): Field, order + + Returns: + tuple: field, order + """ + order = order.split() + if order[1].lower() == "asc": + orderby, order = order[0], Order.asc return orderby, order + orderby, order = order[0], Order.desc + return orderby, order - @staticmethod - def get_condition(table: str, **kwargs) -> frappe.qb: - """Get initial table object - Args: - table (str): DocType +def get_condition(table: str, **kwargs) -> frappe.qb: + """Get initial table object - Returns: - frappe.qb: DocType with initial condition - """ - if kwargs.get("update"): - return frappe.qb.update(table) - if kwargs.get("into"): - return frappe.qb.into(table) - return frappe.qb.from_(table) + Args: + table (str): DocType + Returns: + frappe.qb: DocType with initial condition + """ + if kwargs.get("update"): + return frappe.qb.update(table) + if kwargs.get("into"): + return frappe.qb.into(table) + return frappe.qb.from_(table) + + +OPERATOR_MAP = { + "+": operator.add, + "=": operator.eq, + "-": operator.sub, + "!=": operator.ne, + "<": operator.lt, + ">": operator.gt, + "<=": operator.le, + ">=": operator.ge, + "in": func_in, + "not in": func_not_in, + "like": like, + "not like": not_like, + "regex": func_regex, + "between": func_between + } + + + +class Query: def criterion_query(self, table: str, criterion: Criterion, **kwargs) -> frappe.qb: """Generate filters from Criterion objects @@ -148,7 +158,7 @@ class Query: Returns: frappe.qb: condition object """ - condition = self.get_condition(table, **kwargs) + condition = get_condition(table, **kwargs) return condition.where(criterion) @@ -165,7 +175,7 @@ class Query: orderby = kwargs.get("orderby") order = kwargs.get("order") if kwargs.get("order") else Order.desc if isinstance(orderby, str) and len(orderby.split()) > 1: - orderby, order = self.change_orderby(orderby) + orderby, order = change_orderby(orderby) conditions = conditions.orderby(orderby, order=order) if kwargs.get("limit"): @@ -186,20 +196,20 @@ class Query: table (str): DocType filters (Union[List, Tuple], optional): Filters. Defaults to None. """ - conditions = self.get_condition(table, **kwargs) + conditions = get_condition(table, **kwargs) if not filters: return conditions if isinstance(filters, list): for f in filters: if not isinstance(f, (list, tuple)): - _operator = self.operator_map[filters[1]] + _operator = OPERATOR_MAP[filters[1]] if not isinstance(filters[0], str): - conditions = self.make_function(filters[0], filters[2]) + conditions = make_function(filters[0], filters[2]) break conditions = conditions.where(_operator(Field(filters[0]), filters[2])) break else: - _operator = self.operator_map[f[1]] + _operator = OPERATOR_MAP[f[1]] conditions = conditions.where(_operator(Field(f[0]), f[2])) conditions = self.add_conditions(conditions, **kwargs) @@ -213,25 +223,25 @@ class Query: filters (Dict[str, Union[str, int]], optional): Filters. Defaults to None. Returns: - condition: conditions object + frappe.qb: conditions object """ - conditions = self.get_condition(table, **kwargs) + conditions = get_condition(table, **kwargs) if not filters: return conditions for key in filters: value = filters.get(key) - _operator = self.operator_map["="] + _operator = OPERATOR_MAP["="] if not isinstance(key, str): - conditions = conditions.where(self.make_function(key, value)) + conditions = conditions.where(make_function(key, value)) continue if isinstance(value, (list, tuple)): - if isinstance(value[1], (list, tuple)) or value[0] in list(self.operator_map.keys())[-4:]: - _operator = self.operator_map[value[0]] + if isinstance(value[1], (list, tuple)) or value[0] in list(OPERATOR_MAP.keys())[-4:]: + _operator = OPERATOR_MAP[value[0]] conditions = conditions.where(_operator(key, value[1])) else: - _operator = self.operator_map[value[0]] + _operator = OPERATOR_MAP[value[0]] conditions = conditions.where(_operator(Field(key), value[1])) else: conditions = conditions.where(_operator(Field(key), value)) @@ -242,11 +252,11 @@ class Query: """Build conditions for sql query Args: - filters (Union[Dict[str, Union[str, int]], str, int]): conditions built from filters provided - table (str): DocType + filters (Union[Dict[str, Union[str, int]], str, int]): conditions in Dict + table (str): DocType Returns: - frappe.qb: frappe.qb conditions object + frappe.qb: frappe.qb conditions object """ if isinstance(filters, Criterion): return self.criterion_query(table, filters, **kwargs) @@ -257,4 +267,4 @@ class Query: if isinstance(filters, (list, tuple)): return self.misc_query(table, filters, **kwargs) - return self.dict_query(filters=filters, table=table, **kwargs) \ No newline at end of file + return self.dict_query(filters=filters, table=table, **kwargs) From 5ae95cd0013537a7defa42fb1adb394ffcf89c2e Mon Sep 17 00:00:00 2001 From: Aradhya-Tripathi Date: Sat, 18 Sep 2021 13:35:33 +0530 Subject: [PATCH 22/68] refactor: moved get_condition to query class --- frappe/database/query.py | 39 ++++++++++++++++++--------------------- 1 file changed, 18 insertions(+), 21 deletions(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index e5a7eff498..7d7de85646 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -112,22 +112,6 @@ def change_orderby(order: str): return orderby, order -def get_condition(table: str, **kwargs) -> frappe.qb: - """Get initial table object - - Args: - table (str): DocType - - Returns: - frappe.qb: DocType with initial condition - """ - if kwargs.get("update"): - return frappe.qb.update(table) - if kwargs.get("into"): - return frappe.qb.into(table) - return frappe.qb.from_(table) - - OPERATOR_MAP = { "+": operator.add, "=": operator.eq, @@ -146,8 +130,22 @@ OPERATOR_MAP = { } - class Query: + def get_condition(self, table: str, **kwargs) -> frappe.qb: + """Get initial table object + + Args: + table (str): DocType + + Returns: + frappe.qb: DocType with initial condition + """ + if kwargs.get("update"): + return frappe.qb.update(table) + if kwargs.get("into"): + return frappe.qb.into(table) + return frappe.qb.from_(table) + def criterion_query(self, table: str, criterion: Criterion, **kwargs) -> frappe.qb: """Generate filters from Criterion objects @@ -158,10 +156,9 @@ class Query: Returns: frappe.qb: condition object """ - condition = get_condition(table, **kwargs) + condition = self.get_condition(table, **kwargs) return condition.where(criterion) - def add_conditions(self, conditions: frappe.qb, **kwargs): """Adding additional conditions @@ -196,7 +193,7 @@ class Query: table (str): DocType filters (Union[List, Tuple], optional): Filters. Defaults to None. """ - conditions = get_condition(table, **kwargs) + conditions = self.get_condition(table, **kwargs) if not filters: return conditions if isinstance(filters, list): @@ -225,7 +222,7 @@ class Query: Returns: frappe.qb: conditions object """ - conditions = get_condition(table, **kwargs) + conditions = self.get_condition(table, **kwargs) if not filters: return conditions From 7f8128b27ca75d6a278c6e519c5f863e127968ae Mon Sep 17 00:00:00 2001 From: Aradhya-Tripathi Date: Mon, 20 Sep 2021 23:00:20 +0530 Subject: [PATCH 23/68] fix: added Column in aggregation --- frappe/database/database.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/frappe/database/database.py b/frappe/database/database.py index d14b90d9ec..d0483cbb8c 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -18,6 +18,7 @@ from frappe.utils import now, getdate, cast, get_datetime from frappe.model.utils.link_count import flush_local_link_count from frappe.query_builder.functions import Count from frappe.query_builder.functions import Min, Max, Avg, Sum +from frappe.query_builder.utils import Column from .query import Query @@ -755,16 +756,16 @@ class Database(object): return None def min(self, dt, fieldname, filters=None, **kwargs): - return self.query.build_conditions(dt, filters=filters).select(Min(fieldname)).run(**kwargs)[0][0] or 0 + return self.query.build_conditions(dt, filters=filters).select(Min(Column(fieldname))).run(**kwargs)[0][0] or 0 def max(self, dt, fieldname, filters=None, **kwargs): - return self.query.build_conditions(dt, filters=filters).select(Max(fieldname)).run(**kwargs)[0][0] or 0 + return self.query.build_conditions(dt, filters=filters).select(Max(Column(fieldname))).run(**kwargs)[0][0] or 0 def avg(self, dt, fieldname, filters=None, **kwargs): - return self.query.build_conditions(dt, filters=filters).select(Avg(fieldname)).run(**kwargs)[0][0] or 0 + return self.query.build_conditions(dt, filters=filters).select(Avg(Column(fieldname))).run(**kwargs)[0][0] or 0 def sum(self, dt, fieldname, filters=None, **kwargs): - return self.query.build_conditions(dt, filters=filters).select(Sum(fieldname)).run(**kwargs)[0][0] or 0 + return self.query.build_conditions(dt, filters=filters).select(Sum(Column(fieldname))).run(**kwargs)[0][0] or 0 def count(self, dt, filters=None, debug=False, cache=False): """Returns `COUNT(*)` for given DocType and filters.""" From cbe068c7ff0d3091f8c10b4d0212b70709f08955 Mon Sep 17 00:00:00 2001 From: Aradhya-Tripathi Date: Mon, 20 Sep 2021 23:00:52 +0530 Subject: [PATCH 24/68] feat: added aggregation in safe_exec --- frappe/utils/safe_exec.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/frappe/utils/safe_exec.py b/frappe/utils/safe_exec.py index 7ccd80e346..e593c09ec7 100644 --- a/frappe/utils/safe_exec.py +++ b/frappe/utils/safe_exec.py @@ -146,6 +146,11 @@ def get_safe_globals(): set_value = frappe.db.set_value, get_single_value = frappe.db.get_single_value, get_default = frappe.db.get_default, + count = frappe.db.count, + min = frappe.db.min, + max = frappe.db.max, + avg = frappe.db.avg, + sum = frappe.db.sum, escape = frappe.db.escape, sql = read_sql ) From e4724f06fcdcecf2e7c990ad4920a4b40409dc7d Mon Sep 17 00:00:00 2001 From: Aradhya-Tripathi Date: Mon, 20 Sep 2021 23:16:52 +0530 Subject: [PATCH 25/68] style: removed print statements --- frappe/database/database.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/frappe/database/database.py b/frappe/database/database.py index d0483cbb8c..4af7252cc1 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -527,7 +527,6 @@ class Database(object): if fields=="*": query = self.query.build_conditions(table=doctype, filters=filters, orderby=order_by, for_update=for_update).select(fields) as_dict = True - print(query) r = self.sql(query, as_dict=as_dict, debug=debug, update=update) return r @@ -907,7 +906,6 @@ class Database(object): query = self.query.build_conditions(table=doctype, filters=filters).delete() if "debug" not in kwargs: kwargs["debug"] = debug - return self.sql(query, values, **kwargs) def truncate(self, doctype: str): From f1d5d6a4e35f063415add7c309190ef12fd0d1bd Mon Sep 17 00:00:00 2001 From: Aradhya-Tripathi Date: Mon, 20 Sep 2021 23:43:52 +0530 Subject: [PATCH 26/68] ci: Trigger build From e8263245d2a85bf8545f5bd2d8fd4b3117238626 Mon Sep 17 00:00:00 2001 From: MitulDavid Date: Tue, 21 Sep 2021 08:47:35 +0530 Subject: [PATCH 27/68] style: Fix sider issues --- cypress/plugins/index.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cypress/plugins/index.js b/cypress/plugins/index.js index c08f9e594c..9720faa666 100644 --- a/cypress/plugins/index.js +++ b/cypress/plugins/index.js @@ -12,6 +12,6 @@ // the project's config changing) module.exports = (on, config) => { - require('@cypress/code-coverage/task')(on, config) - return config -} \ No newline at end of file + require('@cypress/code-coverage/task')(on, config); + return config; +}; \ No newline at end of file From eb16a09a5773eeb6a24a708fd89069042548b636 Mon Sep 17 00:00:00 2001 From: Mitul David <49085834+mituldavid@users.noreply.github.com> Date: Tue, 21 Sep 2021 20:26:14 +0530 Subject: [PATCH 28/68] ci: Upload coverage only if report is generated --- .github/workflows/ui-tests.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ui-tests.yml b/.github/workflows/ui-tests.yml index 530b988919..44316043cb 100644 --- a/.github/workflows/ui-tests.yml +++ b/.github/workflows/ui-tests.yml @@ -141,10 +141,10 @@ jobs: CYPRESS_RECORD_KEY: 4a48f41c-11b3-425b-aa88-c58048fa69eb - name: Upload Coverage Data - if: ${{ steps.check-build.outputs.build == 'strawberry' }} + if: ${{ steps.check-build.outputs.build == 'strawberry' && hashFiles('/home/runner/frappe-bench/apps/frappe/.cypress-coverage/clover.xml') != '' }} uses: codecov/codecov-action@v2 with: name: Cypress fail_ci_if_error: true directory: /home/runner/frappe-bench/apps/frappe/.cypress-coverage/ - verbose: true \ No newline at end of file + verbose: true From dd563001096f957722f708e829cfe5c28a802d21 Mon Sep 17 00:00:00 2001 From: leela Date: Wed, 22 Sep 2021 18:20:57 +0530 Subject: [PATCH 29/68] fix: date mismatches while displaying in short form --- frappe/public/js/frappe/utils/pretty_date.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/frappe/public/js/frappe/utils/pretty_date.js b/frappe/public/js/frappe/utils/pretty_date.js index 84fd276068..3ebe2c1ae2 100644 --- a/frappe/public/js/frappe/utils/pretty_date.js +++ b/frappe/public/js/frappe/utils/pretty_date.js @@ -25,11 +25,11 @@ function prettyDate(date, mini) { if (day_diff < 7) { return __("{0} d", [day_diff]); } else if (day_diff < 31) { - return __("{0} w", [Math.ceil(day_diff / 7)]); + return __("{0} w", [Math.floor(day_diff / 7)]); } else if (day_diff < 365) { - return __("{0} M", [Math.ceil(day_diff / 30)]); + return __("{0} M", [Math.floor(day_diff / 30)]); } else { - return __("{0} y", [Math.ceil(day_diff / 365)]); + return __("{0} y", [Math.floor(day_diff / 365)]); } } } else { @@ -54,15 +54,15 @@ function prettyDate(date, mini) { } else if (day_diff < 14) { return __("1 week ago"); } else if (day_diff < 31) { - return __("{0} weeks ago", [Math.ceil(day_diff / 7)]); + return __("{0} weeks ago", [Math.floor(day_diff / 7)]); } else if (day_diff < 62) { return __("1 month ago"); } else if (day_diff < 365) { - return __("{0} months ago", [Math.ceil(day_diff / 30)]); + return __("{0} months ago", [Math.floor(day_diff / 30)]); } else if (day_diff < 730) { return __("1 year ago"); } else { - return __("{0} years ago", [Math.ceil(day_diff / 365)]); + return __("{0} years ago", [Math.floor(day_diff / 365)]); } } } From eeae52bbf99cdb3218bc908dcd49d66c387e892d Mon Sep 17 00:00:00 2001 From: MitulDavid Date: Wed, 22 Sep 2021 19:07:32 +0530 Subject: [PATCH 30/68] ci: Use file-existence-action instead of hashFiles() --- .github/workflows/ui-tests.yml | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ui-tests.yml b/.github/workflows/ui-tests.yml index 44316043cb..2e8aca4e31 100644 --- a/.github/workflows/ui-tests.yml +++ b/.github/workflows/ui-tests.yml @@ -140,8 +140,14 @@ jobs: env: CYPRESS_RECORD_KEY: 4a48f41c-11b3-425b-aa88-c58048fa69eb + - name: Check If Coverage Report Exists + id: check_coverage + uses: andstor/file-existence-action@v1 + with: + files: "/home/runner/frappe-bench/apps/frappe/.cypress-coverage/clover.xml" + - name: Upload Coverage Data - if: ${{ steps.check-build.outputs.build == 'strawberry' && hashFiles('/home/runner/frappe-bench/apps/frappe/.cypress-coverage/clover.xml') != '' }} + if: ${{ steps.check-build.outputs.build == 'strawberry' && steps.check_coverage.outputs.files_exists == 'true' }} uses: codecov/codecov-action@v2 with: name: Cypress From b2a79ce91ed213fc241d8f6abb220c38e861805a Mon Sep 17 00:00:00 2001 From: Aradhya-Tripathi Date: Sun, 3 Oct 2021 01:14:59 +0530 Subject: [PATCH 31/68] refactor: removed redundant imports --- .../doctype/package_release/package_release.py | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/frappe/core/doctype/package_release/package_release.py b/frappe/core/doctype/package_release/package_release.py index 30ad46c68e..f981d0de93 100644 --- a/frappe/core/doctype/package_release/package_release.py +++ b/frappe/core/doctype/package_release/package_release.py @@ -6,20 +6,26 @@ from frappe.model.document import Document from frappe.modules.export_file import export_doc import os import subprocess +from frappe.query_builder.functions import Max class PackageRelease(Document): def set_version(self): # set the next patch release by default - from frappe.query_builder.functions import Max - from frappe.query_builder import Field - + doctype = frappe.qb.DocType("Package Release") if not self.major: - self.major = frappe.qb.from_("Package Release").where(Field("package") == self.package).select(Max("major")).run()[0][0] or 0 + self.major = frappe.qb.from_(doctype) \ + .where(doctype.package == self.package) \ + .select(Max(doctype.minor)).run()[0][0] or 0 + if not self.minor: - self.minor = frappe.qb.from_("Package Release").where(Field("package") == self.package).select(Max("minor")).run()[0][0] or 0 + self.minor = frappe.qb.from_(doctype) \ + .where(doctype.package == self.package) \ + .select(Max("minor")).run()[0][0] or 0 if not self.patch: - self.patch = frappe.qb.from_("Package Release").where(Field("package") == self.package).select(Max("patch")).run()[0][0] or 1 + self.patch = frappe.qb.from_(doctype) \ + .where(doctype.package == self.package) \ + .select(Max("patch")).run()[0][0] or 1 def autoname(self): self.set_version() From b798b96debb15304be660ac8442d919a6b475e64 Mon Sep 17 00:00:00 2001 From: MitulDavid Date: Mon, 4 Oct 2021 08:21:40 +0530 Subject: [PATCH 32/68] ci: Set Codecov flags to categorize coverage --- .github/workflows/server-mariadb-tests.yml | 3 ++- .github/workflows/server-postgres-tests.yml | 1 + .github/workflows/ui-tests.yml | 1 + codecov.yml | 12 +++++++++++- 4 files changed, 15 insertions(+), 2 deletions(-) diff --git a/.github/workflows/server-mariadb-tests.yml b/.github/workflows/server-mariadb-tests.yml index 0b187fc44c..2e50aa48f6 100644 --- a/.github/workflows/server-mariadb-tests.yml +++ b/.github/workflows/server-mariadb-tests.yml @@ -127,4 +127,5 @@ jobs: name: MariaDB fail_ci_if_error: true files: /home/runner/frappe-bench/sites/coverage.xml - verbose: true \ No newline at end of file + verbose: true + flags: server \ No newline at end of file diff --git a/.github/workflows/server-postgres-tests.yml b/.github/workflows/server-postgres-tests.yml index a5630121a4..2203b657ad 100644 --- a/.github/workflows/server-postgres-tests.yml +++ b/.github/workflows/server-postgres-tests.yml @@ -131,3 +131,4 @@ jobs: fail_ci_if_error: true files: /home/runner/frappe-bench/sites/coverage.xml verbose: true + flags: server diff --git a/.github/workflows/ui-tests.yml b/.github/workflows/ui-tests.yml index 2cbe4cce41..a9d331c44d 100644 --- a/.github/workflows/ui-tests.yml +++ b/.github/workflows/ui-tests.yml @@ -154,3 +154,4 @@ jobs: fail_ci_if_error: true directory: /home/runner/frappe-bench/apps/frappe/.cypress-coverage/ verbose: true + flags: ui-tests diff --git a/codecov.yml b/codecov.yml index 41b22001a5..b2dfa82fe1 100644 --- a/codecov.yml +++ b/codecov.yml @@ -9,5 +9,15 @@ coverage: threshold: 0.5% comment: - layout: "diff" + layout: "diff, flags" require_changes: true + +flags: + server: + paths: + - ".*\.py" + carryforward: true + ui-tests: + paths: + - ".*\.js" + carryforward: true \ No newline at end of file From cbbfdf33dea2e56cf8499a5d12bbd777cbbe12e0 Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Mon, 4 Oct 2021 17:49:21 +0530 Subject: [PATCH 33/68] fix: use site path instead of site name to generate DB name --- frappe/installer.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/frappe/installer.py b/frappe/installer.py index f0bf0cb51c..7234c9ba70 100755 --- a/frappe/installer.py +++ b/frappe/installer.py @@ -29,6 +29,10 @@ def _new_site( ): """Install a new Frappe site""" + from frappe.commands.scheduler import _is_scheduler_enabled + from frappe.utils import get_site_path, scheduler, touch_file + + if not force and os.path.exists(site): print("Site {0} already exists".format(site)) sys.exit(1) @@ -37,14 +41,11 @@ def _new_site( print("--no-mariadb-socket requires db_type to be set to mariadb.") sys.exit(1) - if not db_name: - import hashlib - db_name = "_" + hashlib.sha1(site.encode()).hexdigest()[:16] - frappe.init(site=site) - from frappe.commands.scheduler import _is_scheduler_enabled - from frappe.utils import get_site_path, scheduler, touch_file + if not db_name: + import hashlib + db_name = "_" + hashlib.sha1(get_site_path().encode()).hexdigest()[:16] try: # enable scheduler post install? From f2cf8981b5c5b4ba41093bab326a042c604fb74d Mon Sep 17 00:00:00 2001 From: Pruthvi Patel Date: Mon, 4 Oct 2021 17:59:34 +0530 Subject: [PATCH 34/68] fix: get abs path --- frappe/installer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/installer.py b/frappe/installer.py index 7234c9ba70..8b840ede46 100755 --- a/frappe/installer.py +++ b/frappe/installer.py @@ -45,7 +45,7 @@ def _new_site( if not db_name: import hashlib - db_name = "_" + hashlib.sha1(get_site_path().encode()).hexdigest()[:16] + db_name = "_" + hashlib.sha1(os.path.realpath(frappe.get_site_path()).encode()).hexdigest()[:16] try: # enable scheduler post install? From 9b19b41a552042cabfa9c98643cf53469041e24b Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Mon, 4 Oct 2021 19:56:59 +0200 Subject: [PATCH 35/68] feat: add context for "is (not) set" filter translations --- frappe/public/js/frappe/ui/filters/filter.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/public/js/frappe/ui/filters/filter.js b/frappe/public/js/frappe/ui/filters/filter.js index 6471fb47a5..2151e66236 100644 --- a/frappe/public/js/frappe/ui/filters/filter.js +++ b/frappe/public/js/frappe/ui/filters/filter.js @@ -536,8 +536,8 @@ frappe.ui.filter_utils = { if (condition === 'is') { df.fieldtype = 'Select'; df.options = [ - { label: __('Set'), value: 'set' }, - { label: __('Not Set'), value: 'not set' }, + { label: __('Set', null, 'Field value is set'), value: 'set' }, + { label: __('Not Set', null, 'Field value is not set'), value: 'not set' }, ]; } return; From d7f25ecb24705568ddcadb95c3fac258046f6f07 Mon Sep 17 00:00:00 2001 From: Aradhya-Tripathi Date: Tue, 5 Oct 2021 11:23:35 +0530 Subject: [PATCH 36/68] fix: fixed package_release --- frappe/core/doctype/package_release/package_release.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/frappe/core/doctype/package_release/package_release.py b/frappe/core/doctype/package_release/package_release.py index f981d0de93..d23ae917c4 100644 --- a/frappe/core/doctype/package_release/package_release.py +++ b/frappe/core/doctype/package_release/package_release.py @@ -23,9 +23,10 @@ class PackageRelease(Document): .where(doctype.package == self.package) \ .select(Max("minor")).run()[0][0] or 0 if not self.patch: - self.patch = frappe.qb.from_(doctype) \ + value = frappe.qb.from_(doctype) \ .where(doctype.package == self.package) \ - .select(Max("patch")).run()[0][0] or 1 + .select(Max("patch")).run()[0][0] or 0 + self.patch = value + 1 def autoname(self): self.set_version() From 985d3476bede817c7352257a92e95668614ceffb Mon Sep 17 00:00:00 2001 From: Aradhya-Tripathi Date: Tue, 5 Oct 2021 11:36:31 +0530 Subject: [PATCH 37/68] ci: trigger build From 25f694ab429308e83c9bcc4de194c5815e641360 Mon Sep 17 00:00:00 2001 From: pateljannat Date: Tue, 5 Oct 2021 12:55:07 +0530 Subject: [PATCH 38/68] docs: frappe school link in readme --- README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/README.md b/README.md index 6c2804d843..0ca60b9532 100644 --- a/README.md +++ b/README.md @@ -55,5 +55,9 @@ Full-stack web application framework that uses Python and MariaDB on the server For details and documentation, see the website [https://frappeframework.com](https://frappeframework.com) +## Learning + +1. [Frappe School](https://frappe.school) - Learn Frappe Framework and ERPNext from the various courses by the maintainers or from the community. + ### License This repository has been released under the [MIT License](LICENSE). From f22101a9a4aa6409054060821277ce77bc9e72d4 Mon Sep 17 00:00:00 2001 From: pateljannat Date: Tue, 5 Oct 2021 13:02:11 +0530 Subject: [PATCH 39/68] docs: making all headings same in readme --- README.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 0ca60b9532..6d1ad7b1c4 100644 --- a/README.md +++ b/README.md @@ -35,12 +35,12 @@ Full-stack web application framework that uses Python and MariaDB on the server side and a tightly integrated client side library. Built for [ERPNext](https://erpnext.com) -### Table of Contents +## Table of Contents * [Installation](https://frappeframework.com/docs/user/en/installation) * [Documentation](https://frappeframework.com/docs) * [License](#license) -### Installation +## Installation * [Install via Docker](https://github.com/frappe/frappe_docker) * [Install via Frappe Bench](https://github.com/frappe/bench) @@ -50,7 +50,7 @@ Full-stack web application framework that uses Python and MariaDB on the server 1. [Contribution Guidelines](https://github.com/frappe/erpnext/wiki/Contribution-Guidelines) 1. [Translations](https://translate.erpnext.com) -### Website +## Website For details and documentation, see the website [https://frappeframework.com](https://frappeframework.com) @@ -59,5 +59,5 @@ For details and documentation, see the website 1. [Frappe School](https://frappe.school) - Learn Frappe Framework and ERPNext from the various courses by the maintainers or from the community. -### License +## License This repository has been released under the [MIT License](LICENSE). From 8d48122633d3938951d35eacc79ad3e2cf81fdfe Mon Sep 17 00:00:00 2001 From: MitulDavid Date: Tue, 5 Oct 2021 13:15:46 +0530 Subject: [PATCH 40/68] ci: Fix Codecov configuration --- codecov.yml | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/codecov.yml b/codecov.yml index b2dfa82fe1..eeba1ff381 100644 --- a/codecov.yml +++ b/codecov.yml @@ -4,9 +4,17 @@ codecov: coverage: status: project: - default: + default: false + server: target: auto threshold: 0.5% + flags: + - server + ui-tests: + target: auto + threshold: 0.5% + flags: + - ui-tests comment: layout: "diff, flags" @@ -15,9 +23,9 @@ comment: flags: server: paths: - - ".*\.py" + - ".*\\.py" carryforward: true ui-tests: paths: - - ".*\.js" + - ".*\\.js" carryforward: true \ No newline at end of file From 25277e02da994dafe7bfe4da81f81ac6bdfd70e0 Mon Sep 17 00:00:00 2001 From: Aradhya-Tripathi Date: Tue, 5 Oct 2021 15:48:52 +0530 Subject: [PATCH 41/68] fix: Fixed get_values --- frappe/database/database.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/frappe/database/database.py b/frappe/database/database.py index 4af7252cc1..381247ffa2 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -20,6 +20,7 @@ from frappe.query_builder.functions import Count from frappe.query_builder.functions import Min, Max, Avg, Sum from frappe.query_builder.utils import Column from .query import Query +from pypika.terms import PseudoColumn class Database(object): @@ -521,11 +522,19 @@ class Database(object): return self.get_single_value(*args, **kwargs) def _get_values_from_table(self, fields, filters, doctype, as_dict, debug, order_by=None, update=None, for_update=False): + field_objects = [] + for field in fields: + if "(" in field or " as " in field: + field_objects.append(PseudoColumn(field)) + else: + field_objects.append(field) + criterion = self.query.build_conditions(table=doctype, filters=filters, orderby=order_by, for_update=for_update) + if isinstance(fields, (list, tuple)): - query = self.query.build_conditions(table=doctype, filters=filters, orderby=order_by, for_update=for_update).select(*fields) + query = criterion.select(*field_objects) else: if fields=="*": - query = self.query.build_conditions(table=doctype, filters=filters, orderby=order_by, for_update=for_update).select(fields) + query = criterion.select(fields) as_dict = True r = self.sql(query, as_dict=as_dict, debug=debug, update=update) From f0010439c77796879e1455997974849732176d3b Mon Sep 17 00:00:00 2001 From: Aradhya-Tripathi Date: Tue, 5 Oct 2021 15:54:27 +0530 Subject: [PATCH 42/68] style: formatted code --- frappe/database/database.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/frappe/database/database.py b/frappe/database/database.py index 381247ffa2..f07e0c38e3 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -523,11 +523,13 @@ class Database(object): def _get_values_from_table(self, fields, filters, doctype, as_dict, debug, order_by=None, update=None, for_update=False): field_objects = [] + for field in fields: if "(" in field or " as " in field: field_objects.append(PseudoColumn(field)) else: field_objects.append(field) + criterion = self.query.build_conditions(table=doctype, filters=filters, orderby=order_by, for_update=for_update) if isinstance(fields, (list, tuple)): From 4919d1ba627c43c8067a267b2cb042f1795d5616 Mon Sep 17 00:00:00 2001 From: Aradhya-Tripathi Date: Tue, 5 Oct 2021 16:02:51 +0530 Subject: [PATCH 43/68] feat: Added tests for get_value with sql functions --- frappe/tests/test_db.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/frappe/tests/test_db.py b/frappe/tests/test_db.py index 20f38dc964..2607bf7393 100644 --- a/frappe/tests/test_db.py +++ b/frappe/tests/test_db.py @@ -22,6 +22,8 @@ class TestDB(unittest.TestCase): self.assertNotEqual(frappe.db.get_value("User", {"name": ["!=", "Guest"]}), "Guest") self.assertEqual(frappe.db.get_value("User", {"name": ["<", "Adn"]}), "Administrator") self.assertEqual(frappe.db.get_value("User", {"name": ["<=", "Administrator"]}), "Administrator") + self.assertEqual(frappe.db.get_value("User", {}, ["Max(name)"]), "Guest") + self.assertEqual(frappe.db.get_value("User", {}, "Max(name)"), "Guest") self.assertEqual(frappe.db.sql("""SELECT name FROM `tabUser` WHERE name > 's' ORDER BY MODIFIED DESC""")[0][0], frappe.db.get_value("User", {"name": [">", "s"]})) From 162645d7037ad2029408eff4927b8c2384a2d149 Mon Sep 17 00:00:00 2001 From: Aradhya-Tripathi Date: Tue, 5 Oct 2021 16:23:37 +0530 Subject: [PATCH 44/68] fix: fixed get_value tests --- frappe/tests/test_db.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/tests/test_db.py b/frappe/tests/test_db.py index 2607bf7393..0d32a72756 100644 --- a/frappe/tests/test_db.py +++ b/frappe/tests/test_db.py @@ -22,8 +22,8 @@ class TestDB(unittest.TestCase): self.assertNotEqual(frappe.db.get_value("User", {"name": ["!=", "Guest"]}), "Guest") self.assertEqual(frappe.db.get_value("User", {"name": ["<", "Adn"]}), "Administrator") self.assertEqual(frappe.db.get_value("User", {"name": ["<=", "Administrator"]}), "Administrator") - self.assertEqual(frappe.db.get_value("User", {}, ["Max(name)"]), "Guest") - self.assertEqual(frappe.db.get_value("User", {}, "Max(name)"), "Guest") + self.assertEqual(frappe.db.get_value("User", {}, ["Max(name)"]), frappe.db.sql("SELECT Max(name) FROM tabUser")[0][0]) + self.assertEqual(frappe.db.get_value("User", {}, "Min(name)"), frappe.db.sql("SELECT Min(name) FROM tabUser")[0][0]) self.assertEqual(frappe.db.sql("""SELECT name FROM `tabUser` WHERE name > 's' ORDER BY MODIFIED DESC""")[0][0], frappe.db.get_value("User", {"name": [">", "s"]})) From dca6dd2750bcefa4acab2123806240cbd1f5bcc7 Mon Sep 17 00:00:00 2001 From: Shariq Ansari Date: Tue, 5 Oct 2021 17:03:17 +0530 Subject: [PATCH 45/68] fix: Changing the drag icon in list settings --- frappe/public/js/frappe/list/list_settings.js | 4 ++-- frappe/public/js/frappe/utils/utils.js | 4 ++-- frappe/public/scss/desk/list.scss | 4 ++++ 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/frappe/public/js/frappe/list/list_settings.js b/frappe/public/js/frappe/list/list_settings.js index 4877d6fbb7..3a82935a53 100644 --- a/frappe/public/js/frappe/list/list_settings.js +++ b/frappe/public/js/frappe/list/list_settings.js @@ -114,14 +114,14 @@ export default class ListSettings {
- + ${frappe.utils.icon("drag", "xs", "", "sortable-handle " + show_sortable_handle)}
${me.fields[idx].label}
- + ${frappe.utils.icon("delete", "xs")}
diff --git a/frappe/public/js/frappe/utils/utils.js b/frappe/public/js/frappe/utils/utils.js index f534dff1c6..0a80bb081c 100644 --- a/frappe/public/js/frappe/utils/utils.js +++ b/frappe/public/js/frappe/utils/utils.js @@ -1123,7 +1123,7 @@ Object.assign(frappe.utils, { } }, - icon(icon_name, size="sm", icon_class="") { + icon(icon_name, size="sm", icon_class="", svg_class="") { let size_class = ""; let icon_style = ""; if (typeof size == "object") { @@ -1131,7 +1131,7 @@ Object.assign(frappe.utils, { } else { size_class = `icon-${size}`; } - return ` + return ` `; }, diff --git a/frappe/public/scss/desk/list.scss b/frappe/public/scss/desk/list.scss index 1818f6d8b3..9c78dd0102 100644 --- a/frappe/public/scss/desk/list.scss +++ b/frappe/public/scss/desk/list.scss @@ -228,6 +228,10 @@ input.list-check-all, input.list-row-checkbox { z-index: 500; top: 0; } + + .sortable-handle { + cursor: -webkit-grabbing; + } } .list-items { From dc9f6a58c85ff2b02f5fc29930aca9eadbeb845f Mon Sep 17 00:00:00 2001 From: Shariq Ansari Date: Tue, 5 Oct 2021 17:26:52 +0530 Subject: [PATCH 46/68] fix: Resolved conflict --- frappe/public/js/frappe/list/list_settings.js | 2 +- frappe/public/js/frappe/utils/utils.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/public/js/frappe/list/list_settings.js b/frappe/public/js/frappe/list/list_settings.js index 3a82935a53..782a077a78 100644 --- a/frappe/public/js/frappe/list/list_settings.js +++ b/frappe/public/js/frappe/list/list_settings.js @@ -114,7 +114,7 @@ export default class ListSettings {
- ${frappe.utils.icon("drag", "xs", "", "sortable-handle " + show_sortable_handle)} + ${frappe.utils.icon("drag", "xs", "", "", "sortable-handle " + show_sortable_handle)}
${me.fields[idx].label} diff --git a/frappe/public/js/frappe/utils/utils.js b/frappe/public/js/frappe/utils/utils.js index 0a80bb081c..825bd8c1a1 100644 --- a/frappe/public/js/frappe/utils/utils.js +++ b/frappe/public/js/frappe/utils/utils.js @@ -1123,7 +1123,7 @@ Object.assign(frappe.utils, { } }, - icon(icon_name, size="sm", icon_class="", svg_class="") { + icon(icon_name, size="sm", icon_class="", icon_style="", svg_class="") { let size_class = ""; let icon_style = ""; if (typeof size == "object") { From f0e7605b65f145257cc560d8d6d715126929ea44 Mon Sep 17 00:00:00 2001 From: Shariq Ansari <30859809+shariquerik@users.noreply.github.com> Date: Wed, 6 Oct 2021 14:18:46 +0530 Subject: [PATCH 47/68] fix: Resolved redefine object issue on webform load (#14367) --- frappe/public/js/frappe/utils/datetime.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/frappe/public/js/frappe/utils/datetime.js b/frappe/public/js/frappe/utils/datetime.js index 0c6fea2986..99d47a6deb 100644 --- a/frappe/public/js/frappe/utils/datetime.js +++ b/frappe/public/js/frappe/utils/datetime.js @@ -242,18 +242,21 @@ Object.defineProperties(window, { get: function() { console.warn('Please use `frappe.datetime` instead of `dateutil`. It will be deprecated soon.'); return frappe.datetime; - } + }, + configurable: true }, 'date': { get: function() { console.warn('Please use `frappe.datetime` instead of `date`. It will be deprecated soon.'); return frappe.datetime; - } + }, + configurable: true }, 'get_today': { get: function() { console.warn('Please use `frappe.datetime.get_today` instead of `get_today`. It will be deprecated soon.'); return frappe.datetime.get_today; - } + }, + configurable: true } }); From 1d9000fd85346dbba2aae5850e07162bb9b445ef Mon Sep 17 00:00:00 2001 From: Sun Howwrongbum Date: Wed, 6 Oct 2021 14:21:03 +0530 Subject: [PATCH 48/68] feat: expose frappe.db.exists to Server Script --- frappe/utils/safe_exec.py | 1 + 1 file changed, 1 insertion(+) diff --git a/frappe/utils/safe_exec.py b/frappe/utils/safe_exec.py index 0d1574b49a..1751d41b66 100644 --- a/frappe/utils/safe_exec.py +++ b/frappe/utils/safe_exec.py @@ -147,6 +147,7 @@ def get_safe_globals(): set_value = frappe.db.set_value, get_single_value = frappe.db.get_single_value, get_default = frappe.db.get_default, + exists = frappe.db.exists, count = frappe.db.count, min = frappe.db.min, max = frappe.db.max, From d254ca2f505833c6d74a5302c283b574f4d8ae9d Mon Sep 17 00:00:00 2001 From: MitulDavid Date: Wed, 6 Oct 2021 17:37:20 +0530 Subject: [PATCH 49/68] fix: Make images in comments private --- frappe/core/doctype/file/file.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/frappe/core/doctype/file/file.py b/frappe/core/doctype/file/file.py index d9ecd85533..569c6ab23b 100755 --- a/frappe/core/doctype/file/file.py +++ b/frappe/core/doctype/file/file.py @@ -839,6 +839,7 @@ def extract_images_from_html(doc, content): doctype = doc.parenttype if doc.parent else doc.doctype name = doc.parent or doc.name + is_private = True if doctype == "Comment" else False _file = frappe.get_doc({ "doctype": "File", @@ -846,7 +847,8 @@ def extract_images_from_html(doc, content): "attached_to_doctype": doctype, "attached_to_name": name, "content": content, - "decode": False + "decode": False, + "is_private": is_private }) _file.save(ignore_permissions=True) file_url = _file.file_url From 34ba19a131765e8110b6e261f20199e93e2929e5 Mon Sep 17 00:00:00 2001 From: Anupam Date: Thu, 7 Oct 2021 10:27:47 +0530 Subject: [PATCH 50/68] fix: browser printing broken --- frappe/printing/page/print/print.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/printing/page/print/print.js b/frappe/printing/page/print/print.js index d3b1c69815..12a48eaa88 100644 --- a/frappe/printing/page/print/print.js +++ b/frappe/printing/page/print/print.js @@ -464,7 +464,7 @@ frappe.ui.form.PrintView = class { printit() { let me = this; - if (me.print_settings.enable_print_server) { + if (cint(me.print_settings.enable_print_server)) { if (localStorage.getItem('network_printer')) { me.print_by_server(); } else { From 5387e48a5995039d0aa284da943922a214134b1b Mon Sep 17 00:00:00 2001 From: Anupam Date: Thu, 7 Oct 2021 11:24:25 +0530 Subject: [PATCH 51/68] fix: network printer settings permission issue --- .../network_printer_settings.json | 12 +++++++++++- frappe/printing/page/print/print.js | 2 +- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/frappe/printing/doctype/network_printer_settings/network_printer_settings.json b/frappe/printing/doctype/network_printer_settings/network_printer_settings.json index cbef4b8ba4..11f1382225 100644 --- a/frappe/printing/doctype/network_printer_settings/network_printer_settings.json +++ b/frappe/printing/doctype/network_printer_settings/network_printer_settings.json @@ -41,10 +41,11 @@ ], "index_web_pages_for_search": 1, "links": [], - "modified": "2021-09-17 11:30:16.781655", + "modified": "2021-10-07 11:23:13.799402", "modified_by": "Administrator", "module": "Printing", "name": "Network Printer Settings", + "naming_rule": "Set by user", "owner": "Administrator", "permissions": [ { @@ -58,6 +59,15 @@ "role": "System Manager", "share": 1, "write": 1 + }, + { + "email": 1, + "export": 1, + "print": 1, + "read": 1, + "report": 1, + "role": "All", + "share": 1 } ], "sort_field": "modified", diff --git a/frappe/printing/page/print/print.js b/frappe/printing/page/print/print.js index 12a48eaa88..c1ab56b181 100644 --- a/frappe/printing/page/print/print.js +++ b/frappe/printing/page/print/print.js @@ -177,7 +177,7 @@ frappe.ui.form.PrintView = class { ); } - if (this.print_settings.enable_print_server) { + if (cint(this.print_settings.enable_print_server)) { this.page.add_menu_item(__('Select Network Printer'), () => this.network_printer_setting_dialog() ); From 3a5b967bfe5f178c3d948876beda8a596ff89f12 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 7 Oct 2021 12:45:18 +0530 Subject: [PATCH 52/68] docs: Update README * Fix Table of contents * Restructure and reword Resources section * Link code of conduct & security policy to README --- README.md | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 6d1ad7b1c4..2f9ed0c4dd 100644 --- a/README.md +++ b/README.md @@ -36,28 +36,28 @@ Full-stack web application framework that uses Python and MariaDB on the server side and a tightly integrated client side library. Built for [ERPNext](https://erpnext.com) ## Table of Contents -* [Installation](https://frappeframework.com/docs/user/en/installation) -* [Documentation](https://frappeframework.com/docs) +* [Installation](#installation) +* [Contributing](#contributing) +* [Resources](#resources) * [License](#license) ## Installation * [Install via Docker](https://github.com/frappe/frappe_docker) * [Install via Frappe Bench](https://github.com/frappe/bench) +* [Offical Documentation](https://frappeframework.com/docs/user/en/installation) ## Contributing +1. [Code of Conduct](CODE_OF_CONDUCT) 1. [Contribution Guidelines](https://github.com/frappe/erpnext/wiki/Contribution-Guidelines) +1. [Security Policy](SECURITY) 1. [Translations](https://translate.erpnext.com) -## Website +## Resources -For details and documentation, see the website -[https://frappeframework.com](https://frappeframework.com) - -## Learning - -1. [Frappe School](https://frappe.school) - Learn Frappe Framework and ERPNext from the various courses by the maintainers or from the community. +1. [frappeframework.com](https://frappeframework.com) - Official documentation of the Frappe Framework. +1. [frappe.school](https://frappe.school) - Pick from the various courses by the maintainers or from the community. ## License This repository has been released under the [MIT License](LICENSE). From b34178a3844ea863ffe297aace3a1694817ff023 Mon Sep 17 00:00:00 2001 From: gavin Date: Thu, 7 Oct 2021 12:51:44 +0530 Subject: [PATCH 53/68] docs: Fix links in README --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 2f9ed0c4dd..c0d9966340 100644 --- a/README.md +++ b/README.md @@ -49,9 +49,9 @@ Full-stack web application framework that uses Python and MariaDB on the server ## Contributing -1. [Code of Conduct](CODE_OF_CONDUCT) +1. [Code of Conduct](CODE_OF_CONDUCT.md) 1. [Contribution Guidelines](https://github.com/frappe/erpnext/wiki/Contribution-Guidelines) -1. [Security Policy](SECURITY) +1. [Security Policy](SECURITY.md) 1. [Translations](https://translate.erpnext.com) ## Resources From 0f73909d60ee5d60c5739b83e0bb60a6a0c45e5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20de=20Chazelles?= Date: Wed, 29 Sep 2021 16:40:58 +0200 Subject: [PATCH 54/68] fix: include 'Duration' in fieldtype to fetch (cherry picked from commit 06ad3b534c3131504dcf481be29cb8e1a78739ae) --- frappe/public/js/frappe/form/script_manager.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/public/js/frappe/form/script_manager.js b/frappe/public/js/frappe/form/script_manager.js index f960f4f767..f877a7cf8b 100644 --- a/frappe/public/js/frappe/form/script_manager.js +++ b/frappe/public/js/frappe/form/script_manager.js @@ -193,7 +193,7 @@ frappe.ui.form.ScriptManager = class ScriptManager { function setup_add_fetch(df) { if ((['Data', 'Read Only', 'Text', 'Small Text', 'Currency', 'Check', - 'Text Editor', 'Code', 'Link', 'Float', 'Int', 'Date', 'Select'].includes(df.fieldtype) || df.read_only==1) + 'Text Editor', 'Code', 'Link', 'Float', 'Int', 'Date', 'Select', 'Duration'].includes(df.fieldtype) || df.read_only==1) && df.fetch_from && df.fetch_from.indexOf(".")!=-1) { var parts = df.fetch_from.split("."); me.frm.add_fetch(parts[0], parts[1], df.fieldname); From 0879c35cbcff34a0784b64a4bb218cd96ac8ee83 Mon Sep 17 00:00:00 2001 From: Suraj Shetty <13928957+surajshetty3416@users.noreply.github.com> Date: Thu, 7 Oct 2021 15:50:34 +0530 Subject: [PATCH 55/68] chore: Only show server side coverage for now --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index c0d9966340..f8a1907da2 100644 --- a/README.md +++ b/README.md @@ -27,7 +27,7 @@ - +
From 6b91ade73c07dc1c070ed137cf54a29a3e7b0993 Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Thu, 7 Oct 2021 15:58:23 +0530 Subject: [PATCH 56/68] fix: `convert_dates_to_str` converts everything except `date` objects (#14340) Co-authored-by: Suraj Shetty <13928957+surajshetty3416@users.noreply.github.com> --- frappe/model/base_document.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/frappe/model/base_document.py b/frappe/model/base_document.py index 5605ac61ed..1826cca9a3 100644 --- a/frappe/model/base_document.py +++ b/frappe/model/base_document.py @@ -267,7 +267,12 @@ class BaseDocument(object): if isinstance(d[fieldname], list) and df.fieldtype not in table_fields: frappe.throw(_('Value for {0} cannot be a list').format(_(df.label))) - if convert_dates_to_str and isinstance(d[fieldname], (datetime.datetime, datetime.time, datetime.timedelta)): + if convert_dates_to_str and isinstance(d[fieldname], ( + datetime.datetime, + datetime.date, + datetime.time, + datetime.timedelta + )): d[fieldname] = str(d[fieldname]) if d[fieldname] == None and ignore_nulls: From 05cb2c16cd037a9b6110a45295a46bfe2c07295b Mon Sep 17 00:00:00 2001 From: MitulDavid Date: Thu, 7 Oct 2021 16:41:44 +0530 Subject: [PATCH 57/68] fix: Optional parameter for setting extracted image as private --- frappe/core/doctype/file/file.py | 3 +-- frappe/desk/form/utils.py | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/frappe/core/doctype/file/file.py b/frappe/core/doctype/file/file.py index 569c6ab23b..4df9ef3132 100755 --- a/frappe/core/doctype/file/file.py +++ b/frappe/core/doctype/file/file.py @@ -813,7 +813,7 @@ def extract_images_from_doc(doc, fieldname): doc.set(fieldname, content) -def extract_images_from_html(doc, content): +def extract_images_from_html(doc, content, is_private=False): frappe.flags.has_dataurl = False def _save_file(match): @@ -839,7 +839,6 @@ def extract_images_from_html(doc, content): doctype = doc.parenttype if doc.parent else doc.doctype name = doc.parent or doc.name - is_private = True if doctype == "Comment" else False _file = frappe.get_doc({ "doctype": "File", diff --git a/frappe/desk/form/utils.py b/frappe/desk/form/utils.py index a4dcee4ab3..bd04e1915c 100644 --- a/frappe/desk/form/utils.py +++ b/frappe/desk/form/utils.py @@ -66,7 +66,7 @@ def add_comment(reference_doctype, reference_name, content, comment_email, comme comment_type='Comment', comment_by=comment_by )) - doc.content = extract_images_from_html(doc, content) + doc.content = extract_images_from_html(doc, content, is_private=True) doc.insert(ignore_permissions=True) follow_document(doc.reference_doctype, doc.reference_name, frappe.session.user) From 8653b90ec6b522ea857b03681b15082e1ae41365 Mon Sep 17 00:00:00 2001 From: Shariq Ansari Date: Thu, 7 Oct 2021 16:44:20 +0530 Subject: [PATCH 58/68] fix: Replacing -webkit-grabbing with grabbing --- frappe/public/scss/desk/desktop.scss | 9 ++++----- frappe/public/scss/desk/list.scss | 3 ++- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/frappe/public/scss/desk/desktop.scss b/frappe/public/scss/desk/desktop.scss index 2ab6d98e20..6ab01a744c 100644 --- a/frappe/public/scss/desk/desktop.scss +++ b/frappe/public/scss/desk/desktop.scss @@ -164,12 +164,11 @@ body { .drag-handle { cursor: all-scroll; - cursor: -webkit-grabbing; + cursor: grabbing; &:active { + cursor: all-scroll; cursor: grabbing; - cursor: -moz-grabbing; - cursor: -webkit-grabbing; } } @@ -813,7 +812,7 @@ body { .drag-handle { cursor: all-scroll; - cursor: -webkit-grabbing; + cursor: grabbing; display: none; } @@ -966,7 +965,7 @@ body { .drag-handle { cursor: all-scroll; - cursor: -webkit-grabbing; + cursor: grabbing; } } } diff --git a/frappe/public/scss/desk/list.scss b/frappe/public/scss/desk/list.scss index 9c78dd0102..4456acabb3 100644 --- a/frappe/public/scss/desk/list.scss +++ b/frappe/public/scss/desk/list.scss @@ -230,7 +230,8 @@ input.list-check-all, input.list-row-checkbox { } .sortable-handle { - cursor: -webkit-grabbing; + cursor: all-scroll; + cursor: grabbing; } } From f7a8ee7756fb0b684293077e078144d95cd1b66c Mon Sep 17 00:00:00 2001 From: Rohit Waghchaure Date: Thu, 7 Oct 2021 16:50:12 +0530 Subject: [PATCH 59/68] fix: custom dialog box not working --- frappe/public/js/frappe/form/grid.js | 22 ++++++++++++---------- frappe/public/js/frappe/form/grid_row.js | 2 +- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/frappe/public/js/frappe/form/grid.js b/frappe/public/js/frappe/form/grid.js index 574543c3a6..1d302f5e1f 100644 --- a/frappe/public/js/frappe/form/grid.js +++ b/frappe/public/js/frappe/form/grid.js @@ -773,16 +773,18 @@ export default class Grid { } setup_user_defined_columns() { - let user_settings = frappe.get_user_settings(this.frm.doctype, 'GridView'); - if (user_settings && user_settings[this.doctype] && user_settings[this.doctype].length) { - this.user_defined_columns = user_settings[this.doctype].map(row => { - let column = frappe.meta.get_docfield(this.doctype, row.fieldname); - if (column) { - column.in_list_view = 1; - column.columns = row.columns; - return column; - } - }); + if (this.frm) { + let user_settings = frappe.get_user_settings(this.frm.doctype, 'GridView'); + if (user_settings && user_settings[this.doctype] && user_settings[this.doctype].length) { + this.user_defined_columns = user_settings[this.doctype].map(row => { + let column = frappe.meta.get_docfield(this.doctype, row.fieldname); + if (column) { + column.in_list_view = 1; + column.columns = row.columns; + return column; + } + }); + } } } diff --git a/frappe/public/js/frappe/form/grid_row.js b/frappe/public/js/frappe/form/grid_row.js index 20a04ee206..de174cf37f 100644 --- a/frappe/public/js/frappe/form/grid_row.js +++ b/frappe/public/js/frappe/form/grid_row.js @@ -497,7 +497,7 @@ export default class GridRow { } update_user_settings_for_grid() { - if (!this.selected_columns_for_grid) { + if (!this.selected_columns_for_grid || !this.frm) { return; } From 6886036002a7860ddb349eadd8d0428d288a40a6 Mon Sep 17 00:00:00 2001 From: Rohit Waghchaure Date: Thu, 7 Oct 2021 17:32:45 +0530 Subject: [PATCH 60/68] feat: option to set the width for the multi-select dialog box --- frappe/public/js/frappe/form/multi_select_dialog.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/frappe/public/js/frappe/form/multi_select_dialog.js b/frappe/public/js/frappe/form/multi_select_dialog.js index ba522a4085..37b7e08a80 100644 --- a/frappe/public/js/frappe/form/multi_select_dialog.js +++ b/frappe/public/js/frappe/form/multi_select_dialog.js @@ -70,6 +70,7 @@ frappe.ui.form.MultiSelectDialog = class MultiSelectDialog { this.dialog = new frappe.ui.Dialog({ title: title, fields: this.fields, + size: this.size, primary_action_label: this.primary_action_label || __("Get Items"), secondary_action_label: __("Make {0}", [__(this.doctype)]), primary_action: () => { @@ -135,7 +136,7 @@ frappe.ui.form.MultiSelectDialog = class MultiSelectDialog { this.get_child_result().then(r => { this.child_results = r.message || []; this.render_child_datatable(); - + this.$wrapper.addClass('hidden'); this.$child_wrapper.removeClass('hidden'); this.dialog.fields_dict.more_btn.$wrapper.hide(); From 42acb7647f1eab113485e05d68d84228826523b0 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 7 Oct 2021 20:04:04 +0530 Subject: [PATCH 61/68] ci: Upgrade Py37 to Py39 --- .github/workflows/patch-mariadb-tests.yml | 2 +- .github/workflows/publish-assets-develop.yml | 2 +- .github/workflows/publish-assets-releases.yml | 2 +- .github/workflows/server-mariadb-tests.yml | 2 +- .github/workflows/server-postgres-tests.yml | 2 +- .github/workflows/ui-tests.yml | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/patch-mariadb-tests.yml b/.github/workflows/patch-mariadb-tests.yml index 56137d0bea..8758c4e273 100644 --- a/.github/workflows/patch-mariadb-tests.yml +++ b/.github/workflows/patch-mariadb-tests.yml @@ -29,7 +29,7 @@ jobs: - name: Setup Python uses: actions/setup-python@v2 with: - python-version: 3.7 + python-version: '3.9' - name: Check if build should be run id: check-build diff --git a/.github/workflows/publish-assets-develop.yml b/.github/workflows/publish-assets-develop.yml index 85f3f7c3b0..f56d1460b5 100644 --- a/.github/workflows/publish-assets-develop.yml +++ b/.github/workflows/publish-assets-develop.yml @@ -18,7 +18,7 @@ jobs: node-version: 14 - uses: actions/setup-python@v2 with: - python-version: '3.7' + python-version: '3.9' - name: Set up bench and build assets run: | npm install -g yarn diff --git a/.github/workflows/publish-assets-releases.yml b/.github/workflows/publish-assets-releases.yml index a5cc1f8872..2582632fa0 100644 --- a/.github/workflows/publish-assets-releases.yml +++ b/.github/workflows/publish-assets-releases.yml @@ -21,7 +21,7 @@ jobs: python-version: '12.x' - uses: actions/setup-python@v2 with: - python-version: '3.7' + python-version: '3.9' - name: Set up bench and build assets run: | npm install -g yarn diff --git a/.github/workflows/server-mariadb-tests.yml b/.github/workflows/server-mariadb-tests.yml index 2e50aa48f6..588f357f26 100644 --- a/.github/workflows/server-mariadb-tests.yml +++ b/.github/workflows/server-mariadb-tests.yml @@ -38,7 +38,7 @@ jobs: - name: Setup Python uses: actions/setup-python@v2 with: - python-version: 3.7 + python-version: '3.9' - name: Check if build should be run id: check-build diff --git a/.github/workflows/server-postgres-tests.yml b/.github/workflows/server-postgres-tests.yml index 2203b657ad..78f379837b 100644 --- a/.github/workflows/server-postgres-tests.yml +++ b/.github/workflows/server-postgres-tests.yml @@ -41,7 +41,7 @@ jobs: - name: Setup Python uses: actions/setup-python@v2 with: - python-version: 3.7 + python-version: '3.9' - name: Check if build should be run id: check-build diff --git a/.github/workflows/ui-tests.yml b/.github/workflows/ui-tests.yml index a9d331c44d..fcc53ba59c 100644 --- a/.github/workflows/ui-tests.yml +++ b/.github/workflows/ui-tests.yml @@ -37,7 +37,7 @@ jobs: - name: Setup Python uses: actions/setup-python@v2 with: - python-version: 3.7 + python-version: '3.9' - name: Check if build should be run id: check-build From 3537316a2a6d6804c6418c6e4bfa1ececfccaa1c Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Thu, 7 Oct 2021 20:40:44 +0530 Subject: [PATCH 62/68] fix: Routing with hash in URL - Hash in document name should be encoded --- frappe/public/js/frappe/list/list_view.js | 2 +- frappe/public/js/frappe/router.js | 12 +++++------- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/frappe/public/js/frappe/list/list_view.js b/frappe/public/js/frappe/list/list_view.js index 1cf4b4c6ac..09072e106e 100644 --- a/frappe/public/js/frappe/list/list_view.js +++ b/frappe/public/js/frappe/list/list_view.js @@ -907,7 +907,7 @@ frappe.views.ListView = class ListView extends frappe.views.BaseList { return this.settings.get_form_link(doc); } - const docname = doc.name.match(/[%'"\s]/) + const docname = doc.name.match(/[%'"#\s]/) ? encodeURIComponent(doc.name) : doc.name; diff --git a/frappe/public/js/frappe/router.js b/frappe/public/js/frappe/router.js index 484f1ac911..349eadffb0 100644 --- a/frappe/public/js/frappe/router.js +++ b/frappe/public/js/frappe/router.js @@ -56,10 +56,6 @@ $('body').on('click', 'a', function(e) { return override(e.currentTarget.hash); } - if (frappe.router.is_app_route(e.currentTarget.pathname)) { - // target has "/app, this is a v2 style route. - return override(e.currentTarget.pathname + e.currentTarget.hash); - } }); frappe.router = { @@ -263,7 +259,9 @@ frappe.router = { return new Promise(resolve => { route = this.get_route_from_arguments(route); route = this.convert_from_standard_route(route); - const sub_path = this.make_url(route); + let sub_path = this.make_url(route); + // replace each # occurrences in the URL with encoded character except for last + // sub_path = sub_path.replace(/[#](?=.*[#])/g, "%23"); this.push_state(sub_path); setTimeout(() => { @@ -347,7 +345,7 @@ frappe.router = { return null; } else { a = String(a); - if (a && a.match(/[%'"\s\t]/)) { + if (a && a.match(/[%'"#\s\t]/)) { // if special chars, then encode a = encodeURIComponent(a); } @@ -374,7 +372,7 @@ frappe.router = { // return clean sub_path from hash or url // supports both v1 and v2 routing if (!route) { - route = window.location.pathname + window.location.hash + window.location.search; + route = window.location.pathname; if (route.includes('app#')) { // to support v1 route = window.location.hash; From f1c5fc4fe642fa8d88e6db01f4d6c6dda7e102b4 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Thu, 7 Oct 2021 21:50:08 +0530 Subject: [PATCH 63/68] feat: Shareable link for comments and communications - Also, highlight comment/communication that has been shared. --- .../js/frappe/form/footer/base_timeline.js | 8 +++-- .../js/frappe/form/footer/form_timeline.js | 36 ++++++++++++------- frappe/public/js/frappe/form/form.js | 10 +++++- .../form/templates/timeline_message_box.html | 14 ++++++++ frappe/public/js/frappe/utils/utils.js | 16 +++++++-- frappe/public/scss/common/css_variables.scss | 2 ++ frappe/public/scss/desk/dark.scss | 2 ++ frappe/public/scss/desk/global.scss | 13 +++++++ frappe/public/scss/desk/timeline.scss | 2 +- 9 files changed, 84 insertions(+), 19 deletions(-) diff --git a/frappe/public/js/frappe/form/footer/base_timeline.js b/frappe/public/js/frappe/form/footer/base_timeline.js index 702d964442..beeba16459 100644 --- a/frappe/public/js/frappe/form/footer/base_timeline.js +++ b/frappe/public/js/frappe/form/footer/base_timeline.js @@ -97,9 +97,13 @@ class BaseTimeline { } timeline_item.append(`
`); - timeline_item.find('.timeline-content').append(item.content); + let timeline_content = timeline_item.find('.timeline-content'); + timeline_content.append(item.content); if (!item.hide_timestamp && !item.is_card) { - timeline_item.find('.timeline-content').append(` - ${comment_when(item.creation)}`); + timeline_content.append(` - ${comment_when(item.creation)}`); + } + if (item.id) { + timeline_content.attr("id", item.id); } return timeline_item; } diff --git a/frappe/public/js/frappe/form/footer/form_timeline.js b/frappe/public/js/frappe/form/footer/form_timeline.js index b3feae3ee8..128bd355ad 100644 --- a/frappe/public/js/frappe/form/footer/form_timeline.js +++ b/frappe/public/js/frappe/form/footer/form_timeline.js @@ -96,6 +96,7 @@ class FormTimeline extends BaseTimeline { render_timeline_items() { super.render_timeline_items(); this.set_document_info(); + frappe.utils.bind_actions_with_object(this.timeline_items_wrapper, this); } set_document_info() { @@ -179,6 +180,7 @@ class FormTimeline extends BaseTimeline { is_card: true, content: this.get_communication_timeline_content(communication), doctype: "Communication", + id: `communication-${communication.name}`, name: communication.name }); }); @@ -246,6 +248,7 @@ class FormTimeline extends BaseTimeline { creation: comment.creation, is_card: true, doctype: "Comment", + id: `comment-${comment.name}`, name: comment.name, content: this.get_comment_timeline_content(comment), }; @@ -394,7 +397,7 @@ class FormTimeline extends BaseTimeline { } setup_reply(communication_box, communication_doc) { - let actions = communication_box.find('.actions'); + let actions = communication_box.find('.custom-actions'); let reply = $(`${frappe.utils.icon('reply', 'md')}`).click(() => { this.compose_mail(communication_doc); }); @@ -446,14 +449,16 @@ class FormTimeline extends BaseTimeline { let edit_wrapper = $(`
`).hide(); let edit_box = this.make_editable(edit_wrapper); let content_wrapper = comment_wrapper.find('.content'); - - let delete_button = $(); + let more_actions_wrapper = comment_wrapper.find('.more-actions'); if (frappe.model.can_delete("Comment")) { - delete_button = $(` - + const delete_option = $(` +
  • + + ${__("Delete")} + +
    • `).click(() => this.delete_comment(doc.name)); + more_actions_wrapper.find('.dropdown-menu').append(delete_option); } let dismiss_button = $(` @@ -493,15 +498,14 @@ class FormTimeline extends BaseTimeline { edit_button.toggle_edit_mode = () => { edit_button.edit_mode = !edit_button.edit_mode; edit_button.text(edit_button.edit_mode ? __('Save') : __('Edit')); - delete_button.toggle(!edit_button.edit_mode); + more_actions_wrapper.toggle(!edit_button.edit_mode); dismiss_button.toggle(edit_button.edit_mode); edit_wrapper.toggle(edit_button.edit_mode); content_wrapper.toggle(!edit_button.edit_mode); }; - - comment_wrapper.find('.actions').append(edit_button); - comment_wrapper.find('.actions').append(dismiss_button); - comment_wrapper.find('.actions').append(delete_button); + let actions_wrapper = comment_wrapper.find('.custom-actions'); + actions_wrapper.append(edit_button); + actions_wrapper.append(dismiss_button); } make_editable(container) { @@ -559,6 +563,14 @@ class FormTimeline extends BaseTimeline { }); }); } + + copy_link(ev) { + let doc_link = frappe.urllib.get_full_url( + frappe.utils.get_form_link(this.frm.doctype, this.frm.docname) + ); + let element_id = $(ev.currentTarget).closest(".timeline-content").attr("id"); + frappe.utils.copy_to_clipboard(`${doc_link}#${element_id}`); + } } export default FormTimeline; diff --git a/frappe/public/js/frappe/form/form.js b/frappe/public/js/frappe/form/form.js index 97473c3069..a095956dfe 100644 --- a/frappe/public/js/frappe/form/form.js +++ b/frappe/public/js/frappe/form/form.js @@ -480,7 +480,11 @@ frappe.ui.form.Form = class FrappeForm { this.layout.show_empty_form_message(); } - this.scroll_to_element(); + frappe.after_ajax(() => { + $(document).ready(() => { + this.scroll_to_element(); + }); + }); } set_first_tab_as_active() { @@ -598,6 +602,8 @@ frappe.ui.form.Form = class FrappeForm { this.validate_form_action(save_action, resolve); var after_save = function(r) { + // to remove hash from URL to avoid scroll after save + history.replaceState(null, null, ' '); if(!r.exc) { if (["Save", "Update", "Amend"].indexOf(save_action)!==-1) { frappe.utils.play_sound("click"); @@ -1195,6 +1201,8 @@ frappe.ui.form.Form = class FrappeForm { if (selector.length) { frappe.utils.scroll_to(selector); } + } else if (window.location.hash && $(window.location.hash).length) { + frappe.utils.scroll_to(window.location.hash, true, 200, null, null, true); } } diff --git a/frappe/public/js/frappe/form/templates/timeline_message_box.html b/frappe/public/js/frappe/form/templates/timeline_message_box.html index 3884918165..a38e36c7b5 100644 --- a/frappe/public/js/frappe/form/templates/timeline_message_box.html +++ b/frappe/public/js/frappe/form/templates/timeline_message_box.html @@ -63,6 +63,20 @@ {% } %} +
    +
    + + +
    diff --git a/frappe/public/js/frappe/utils/utils.js b/frappe/public/js/frappe/utils/utils.js index f534dff1c6..33dc447890 100644 --- a/frappe/public/js/frappe/utils/utils.js +++ b/frappe/public/js/frappe/utils/utils.js @@ -268,7 +268,8 @@ Object.assign(frappe.utils, {

    '); return content.html(); }, - scroll_to: function(element, animate=true, additional_offset, element_to_be_scrolled, callback) { + scroll_to: function(element, animate=true, additional_offset, + element_to_be_scrolled, callback, highlight_element=false) { if (frappe.flags.disable_auto_scroll) return; element_to_be_scrolled = element_to_be_scrolled || $("html, body"); @@ -291,11 +292,20 @@ Object.assign(frappe.utils, { } if (animate) { - element_to_be_scrolled.animate({ scrollTop: scroll_top }).promise().then(callback); + element_to_be_scrolled.animate({ + scrollTop: scroll_top + }, null, () => { + if (highlight_element) { + $(element).addClass('highlight'); + document.addEventListener("click", function() { + $(element).removeClass('highlight'); + }, {once: true}); + } + callback && callback(); + }); } else { element_to_be_scrolled.scrollTop(scroll_top); } - }, get_scroll_position: function(element, additional_offset) { let header_offset = $(".navbar").height() + $(".page-head:visible").height(); diff --git a/frappe/public/scss/common/css_variables.scss b/frappe/public/scss/common/css_variables.scss index 112238bfe5..ede3dd8ba9 100644 --- a/frappe/public/scss/common/css_variables.scss +++ b/frappe/public/scss/common/css_variables.scss @@ -209,6 +209,8 @@ --highlight-color: var(--gray-50); --yellow-highlight-color: var(--yellow-50); + --highlight-shadow: 1px 1px 10px var(--blue-50), 0px 0px 4px var(--blue-600); + // Border Sizes --border-radius-sm: 4px; --border-radius: 6px; diff --git a/frappe/public/scss/desk/dark.scss b/frappe/public/scss/desk/dark.scss index 7f0dfe73b8..35cdffc91a 100644 --- a/frappe/public/scss/desk/dark.scss +++ b/frappe/public/scss/desk/dark.scss @@ -75,6 +75,8 @@ --highlight-color: var(--gray-700); --yellow-highlight-color: var(--yellow-700); + --highlight-shadow: 1px 1px 10px var(--blue-900), 0px 0px 4px var(--blue-500); + // input --input-disabled-bg: none; diff --git a/frappe/public/scss/desk/global.scss b/frappe/public/scss/desk/global.scss index 8c646395e9..d157a43bc3 100644 --- a/frappe/public/scss/desk/global.scss +++ b/frappe/public/scss/desk/global.scss @@ -561,6 +561,19 @@ details > summary:focus { display: none; } +.highlight { + transition: 0.5s ease background-color; + box-shadow: var(--highlight-shadow) !important; +} + +.dropdown-menu.small { + font-size: var(--text-sm); + min-width: 140px; + .dropdown-item { + padding: var(--padding-xs); + } +} + // REDESIGN TODO: Handling of broken images? // img.no-image:before { // .img-background(); diff --git a/frappe/public/scss/desk/timeline.scss b/frappe/public/scss/desk/timeline.scss index a7e5d3dd9c..1861ee018b 100644 --- a/frappe/public/scss/desk/timeline.scss +++ b/frappe/public/scss/desk/timeline.scss @@ -117,7 +117,7 @@ $threshold: 34; .actions { display: flex; - > * { + > *:not(.indicator-pill) { color: var(--text-muted); } } From 59e0501cc7cd765cbefc2204df3aa4ec5bcbb94a Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Fri, 8 Oct 2021 08:58:52 +0530 Subject: [PATCH 64/68] test: Fix timeline tests --- cypress/integration/timeline.js | 7 ++++--- cypress/support/commands.js | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/cypress/integration/timeline.js b/cypress/integration/timeline.js index 6387485220..decde5c5de 100644 --- a/cypress/integration/timeline.js +++ b/cypress/integration/timeline.js @@ -43,13 +43,14 @@ context('Timeline', () => { cy.get('.timeline-content').should('contain', 'Testing Timeline 123'); //Deleting the added comment - cy.get('.actions > .btn > .icon').first().click(); + cy.get('.more-actions > .action-btn').click(); + cy.get('.more-actions .dropdown-item').contains('Delete').click(); cy.findByRole('button', {name: 'Yes'}).click(); cy.click_modal_primary_button('Yes'); //Deleting the added ToDo - cy.get('.menu-btn-group button').eq(1).click(); - cy.get('.menu-btn-group [data-label="Delete"]').click(); + cy.get('.menu-btn-group [data-original-title="Menu"]').click(); + cy.get('.menu-btn-group .dropdown-item').contains('Delete').click(); cy.findByRole('button', {name: 'Yes'}).click(); }); diff --git a/cypress/support/commands.js b/cypress/support/commands.js index 47c37a56a0..6484370946 100644 --- a/cypress/support/commands.js +++ b/cypress/support/commands.js @@ -353,5 +353,5 @@ Cypress.Commands.add('click_listview_primary_button', (btn_name) => { }); Cypress.Commands.add('click_timeline_action_btn', (btn_name) => { - cy.get('.timeline-content > .timeline-message-box > .justify-between > .actions > .btn').contains(btn_name).click(); + cy.get('.timeline-message-box .custom-actions > .btn').contains(btn_name).click(); }); \ No newline at end of file From 3a004d38dbfba2db7f1e8a9532c90c014d7a24d6 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Fri, 8 Oct 2021 09:39:31 +0530 Subject: [PATCH 65/68] test: Fix flaky test - Due to improper cleanup one of the test of Energy Point was failing --- frappe/email/doctype/notification/test_notification.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/frappe/email/doctype/notification/test_notification.py b/frappe/email/doctype/notification/test_notification.py index a086ded3fb..f05d35be3e 100644 --- a/frappe/email/doctype/notification/test_notification.py +++ b/frappe/email/doctype/notification/test_notification.py @@ -274,4 +274,7 @@ class TestNotification(unittest.TestCase): self.assertTrue('test2@example.com' in recipients) self.assertTrue('test1@example.com' in recipients) - + @classmethod + def tearDownClass(cls): + frappe.delete_doc_if_exists("Notification", "ToDo Status Update") + frappe.delete_doc_if_exists("Notification", "Contact Status Update") \ No newline at end of file From e6bbc698d07e46a8ec6455e990ec2bb001facdf4 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Fri, 8 Oct 2021 11:40:31 +0530 Subject: [PATCH 66/68] ci: fail CI if asset bundling fails (#14364) * ci: fail CI if asset bundling fails * chore: formatting Co-authored-by: Suraj Shetty <13928957+surajshetty3416@users.noreply.github.com> --- .github/helper/install.sh | 2 +- esbuild/esbuild.js | 5 ++++- frappe/build.py | 2 +- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/.github/helper/install.sh b/.github/helper/install.sh index 93189d2b1f..6c81d6298a 100644 --- a/.github/helper/install.sh +++ b/.github/helper/install.sh @@ -59,4 +59,4 @@ cd ../.. bench start & bench --site test_site reinstall --yes if [ "$TYPE" == "server" ]; then bench --site test_site_producer reinstall --yes; fi -bench build --app frappe +CI=Yes bench build --app frappe diff --git a/esbuild/esbuild.js b/esbuild/esbuild.js index 9074beae06..bf4436358e 100644 --- a/esbuild/esbuild.js +++ b/esbuild/esbuild.js @@ -104,6 +104,9 @@ async function execute() { log_error("There were some problems during build"); log(); log(chalk.dim(e.stack)); + if (process.env.CI) { + process.kill(process.pid); + } return; } @@ -528,4 +531,4 @@ function log_rebuilt_assets(prev_assets, new_assets) { log(" " + filename); } log(); -} \ No newline at end of file +} diff --git a/frappe/build.py b/frappe/build.py index 879d5ec432..05fa213018 100644 --- a/frappe/build.py +++ b/frappe/build.py @@ -246,7 +246,7 @@ def bundle(mode, apps=None, hard_link=False, make_copy=False, restore=False, ver check_node_executable() frappe_app_path = frappe.get_app_path("frappe", "..") - frappe.commands.popen(command, cwd=frappe_app_path, env=get_node_env()) + frappe.commands.popen(command, cwd=frappe_app_path, env=get_node_env(), raise_err=True) def watch(apps=None): From 7f24e10852adac4232afea489dc4d9b90c8bd104 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Fri, 8 Oct 2021 13:54:16 +0530 Subject: [PATCH 67/68] fix: Use promise to wait for all animations to complete --- 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 401d945b11..d79f86b084 100644 --- a/frappe/public/js/frappe/utils/utils.js +++ b/frappe/public/js/frappe/utils/utils.js @@ -294,7 +294,7 @@ Object.assign(frappe.utils, { if (animate) { element_to_be_scrolled.animate({ scrollTop: scroll_top - }, null, () => { + }).promise().then(() => { if (highlight_element) { $(element).addClass('highlight'); document.addEventListener("click", function() { From a1ddd25dff8a110c297e1210314ed089d9b35433 Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Fri, 8 Oct 2021 15:36:58 +0530 Subject: [PATCH 68/68] fix: unable to delete role permissions --- frappe/core/page/permission_manager/permission_manager.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/frappe/core/page/permission_manager/permission_manager.js b/frappe/core/page/permission_manager/permission_manager.js index 41cc900a97..6b427fdebf 100644 --- a/frappe/core/page/permission_manager/permission_manager.js +++ b/frappe/core/page/permission_manager/permission_manager.js @@ -325,15 +325,15 @@ frappe.PermissionEngine = class PermissionEngine { .attr("data-doctype", d.parent) .attr("data-role", d.role) .attr("data-permlevel", d.permlevel) - .click(function () { + .on("click", () => { return frappe.call({ module: "frappe.core", page: "permission_manager", method: "remove", args: { - doctype: $(this).attr("data-doctype"), - role: $(this).attr("data-role"), - permlevel: $(this).attr("data-permlevel") + doctype: d.parent, + role: d.role, + permlevel: d.permlevel }, callback: (r) => { if (r.exc) {