From 3c41ec9e435091391b7d874341baa4acb139ac7b Mon Sep 17 00:00:00 2001 From: sokumon Date: Thu, 22 May 2025 12:14:39 +0530 Subject: [PATCH 01/66] fix: handle imap abort in email server --- frappe/email/receive.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/frappe/email/receive.py b/frappe/email/receive.py index 514db646f2..297feb3650 100644 --- a/frappe/email/receive.py +++ b/frappe/email/receive.py @@ -177,7 +177,7 @@ class EmailServer: for i, uid in enumerate(email_list[:100]): try: - self.retrieve_message(uid, i + 1) + self.retrieve_message(uid, i + 1, folder) except (_socket.timeout, LoginLimitExceeded): # get whatever messages were retrieved break @@ -258,7 +258,7 @@ class EmailServer: return match[0] if match else None - def retrieve_message(self, uid, msg_num): + def retrieve_message(self, uid, msg_num, folder): try: if cint(self.settings.use_imap): status, message = self.imap.uid("fetch", uid, "(BODY.PEEK[] BODY.PEEK[HEADER] FLAGS)") @@ -272,7 +272,9 @@ class EmailServer: except _socket.timeout: # propagate this error to break the loop raise - + except imaplib.IMAP4.abort: + self.connect() + self.get_messages(folder) except Exception as e: if self.has_login_limit_exceeded(e): raise LoginLimitExceeded(e) from e From 4fe4bb462d672ddad46eb719458633f4513a1432 Mon Sep 17 00:00:00 2001 From: sokumon Date: Fri, 23 May 2025 01:28:00 +0530 Subject: [PATCH 02/66] fix: add a retry limit to fetching messages --- frappe/email/receive.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/frappe/email/receive.py b/frappe/email/receive.py index 297feb3650..5e8a54764a 100644 --- a/frappe/email/receive.py +++ b/frappe/email/receive.py @@ -67,6 +67,8 @@ class EmailServer: """Wrapper for POP server to pull emails.""" def __init__(self, args=None): + self.retry_limit = 3 + self.retry_count = 0 self.settings = args or frappe._dict() def connect(self): @@ -273,8 +275,10 @@ class EmailServer: # propagate this error to break the loop raise except imaplib.IMAP4.abort: - self.connect() - self.get_messages(folder) + if self.retry_count < self.retry_limit: + self.connect() + self.get_messages(folder) + self.retry_count += 1 except Exception as e: if self.has_login_limit_exceeded(e): raise LoginLimitExceeded(e) from e From 5e91071f6dd6a6b311b7d4cdbb0c24c4bfe33324 Mon Sep 17 00:00:00 2001 From: "Abdallah A. Zaqout" <26047413+zaqoutabed@users.noreply.github.com> Date: Mon, 26 May 2025 16:33:34 +0300 Subject: [PATCH 03/66] feat: enable user to export chart data --- .../public/js/frappe/widgets/chart_widget.js | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/frappe/public/js/frappe/widgets/chart_widget.js b/frappe/public/js/frappe/widgets/chart_widget.js index 57f7a6e03e..e5a0f3fc65 100644 --- a/frappe/public/js/frappe/widgets/chart_widget.js +++ b/frappe/public/js/frappe/widgets/chart_widget.js @@ -311,6 +311,30 @@ export default class ChartWidget extends Widget { this.make_chart(); }, }, + { + label: __("Export {0}", [__(this.chart_doc.chart_name)]), + action: "action-export", + handler: () => { + let data = []; + const datasets = this.data?.datasets || []; + const labels = (this.data?.labels || []).map(label => label || "None"); + if (datasets.length > 0) { + datasets.forEach((element) => { + const name = element.name; + const values = element.values || []; + if (values.length > 0) { + data.push([name || this.chart_doc.chart_name]); + data.push(labels); + data.push(values); + } + }); + } else { + data.push([this.chart_doc.chart_name]); + data.push(labels); + } + frappe.tools.downloadify(data, null, this.chart_doc.chart_name); + }, + } ]; if (this.chart_doc.document_type) { From 4f2997bfae33a9d7eb46c699dc3cb3737462684a Mon Sep 17 00:00:00 2001 From: "Abdallah A. Zaqout" <26047413+zaqoutabed@users.noreply.github.com> Date: Tue, 27 May 2025 10:19:16 +0300 Subject: [PATCH 04/66] refactor: handle multi dataset --- .../public/js/frappe/widgets/chart_widget.js | 36 +++++++++++++------ 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/frappe/public/js/frappe/widgets/chart_widget.js b/frappe/public/js/frappe/widgets/chart_widget.js index e5a0f3fc65..c6d55fed2b 100644 --- a/frappe/public/js/frappe/widgets/chart_widget.js +++ b/frappe/public/js/frappe/widgets/chart_widget.js @@ -315,26 +315,42 @@ export default class ChartWidget extends Widget { label: __("Export {0}", [__(this.chart_doc.chart_name)]), action: "action-export", handler: () => { - let data = []; + const data = [[this.chart_doc.chart_name]]; + data.push([]); + data.push([]); + const datasets = this.data?.datasets || []; - const labels = (this.data?.labels || []).map(label => label || "None"); - if (datasets.length > 0) { + const labels = (this.data?.labels || []).map((label) => label || "None"); + if (datasets.length > 1) { + const csv_labels = []; + const csv_values = []; + labels.forEach((label, idx) => { + datasets.forEach((element) => { + csv_labels.push(`${element.name} (${label})`); + const values = element.values || []; + if(idx < values.length){ + csv_values.push(values[idx]); + } else { + csv_values.push(''); + } + }); + }); + data.push(["", ...csv_labels]); + data.push(["", ...csv_values]); + } else if (datasets.length === 1) { datasets.forEach((element) => { - const name = element.name; const values = element.values || []; if (values.length > 0) { - data.push([name || this.chart_doc.chart_name]); - data.push(labels); - data.push(values); + data.push(["", ...labels]); + data.push(["", ...values]); } }); } else { - data.push([this.chart_doc.chart_name]); - data.push(labels); + data.push(["", ...labels]); } frappe.tools.downloadify(data, null, this.chart_doc.chart_name); }, - } + }, ]; if (this.chart_doc.document_type) { From 23e44ae8eed369a1cad2bc3516a6a31df7b19988 Mon Sep 17 00:00:00 2001 From: "Abdallah A. Zaqout" <26047413+zaqoutabed@users.noreply.github.com> Date: Tue, 3 Jun 2025 12:19:59 +0300 Subject: [PATCH 05/66] refactor: fix pre-commit issues --- frappe/public/js/frappe/widgets/chart_widget.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/public/js/frappe/widgets/chart_widget.js b/frappe/public/js/frappe/widgets/chart_widget.js index c6d55fed2b..e48d84b7dc 100644 --- a/frappe/public/js/frappe/widgets/chart_widget.js +++ b/frappe/public/js/frappe/widgets/chart_widget.js @@ -328,10 +328,10 @@ export default class ChartWidget extends Widget { datasets.forEach((element) => { csv_labels.push(`${element.name} (${label})`); const values = element.values || []; - if(idx < values.length){ + if (idx < values.length) { csv_values.push(values[idx]); } else { - csv_values.push(''); + csv_values.push(""); } }); }); From bc63c17287b297f1852de0d892ea9e1f3cbb8d27 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Wed, 4 Jun 2025 13:22:37 +0530 Subject: [PATCH 06/66] perf: don't load doc before save for child tables (#32770) Consider this: ```python for row in doc.get_children(): row.db_set("amount", 0) ``` This sounds like it will do one write query for each row but it does 2 because of this unnecessary locking of child tables. --- frappe/model/document.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/model/document.py b/frappe/model/document.py index fad1bcbbed..d0061b8bfb 100644 --- a/frappe/model/document.py +++ b/frappe/model/document.py @@ -1396,7 +1396,7 @@ class Document(BaseDocument, DocRef): self.set("modified_by", frappe.session.user) # load but do not reload doc_before_save because before_change or on_change might expect it - if not self.get_doc_before_save(): + if not self.get_doc_before_save() and not self.meta.istable: self.load_doc_before_save() # to trigger notification on value change From 3d578300cb1cfa7b1bf54f8fa1fa7d19a58dfd18 Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Wed, 4 Jun 2025 13:43:00 +0530 Subject: [PATCH 07/66] fix: empty space between two sections (#32759) --- frappe/public/scss/desk/form.scss | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/frappe/public/scss/desk/form.scss b/frappe/public/scss/desk/form.scss index afdbc9c6df..81ee202d99 100644 --- a/frappe/public/scss/desk/form.scss +++ b/frappe/public/scss/desk/form.scss @@ -115,6 +115,11 @@ .form-section.card-section.hide-border { border-bottom: none; + padding-bottom: 0; + + &:not(:has(.section-head)) { + padding-top: 0; + } } .form-dashboard-section { From 57bb1abdeb4ba2c3a505e93fedfb10b6a2cd8a1d Mon Sep 17 00:00:00 2001 From: Saad Chaudhary <47004596+saadchaudharry@users.noreply.github.com> Date: Wed, 4 Jun 2025 13:53:48 +0530 Subject: [PATCH 08/66] fix(web_form): row number column in child tables were showing the wrong title * fix:Web Form Child Table: Incorrect title showing for 'row number' column #32331 * fix : code alignment with develop branch * fix : code alignment with develop branch --------- Co-authored-by: saadchaudhaary --- frappe/website/doctype/web_form/web_form.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/frappe/website/doctype/web_form/web_form.py b/frappe/website/doctype/web_form/web_form.py index 2e5d6e395f..0aa3dc6e3b 100644 --- a/frappe/website/doctype/web_form/web_form.py +++ b/frappe/website/doctype/web_form/web_form.py @@ -70,7 +70,6 @@ class WebForm(WebsiteGenerator): web_form_fields: DF.Table[WebFormField] website_sidebar: DF.Link | None # end: auto-generated types - website = frappe._dict(no_cache=1) def validate(self): @@ -334,7 +333,7 @@ def get_context(context): messages.append("Upload") messages.append("Last") messages.append("First") - messages.append("No.:Title of the 'row number' column") + messages.append("No.") # Phone Picker if any(field.fieldtype == "Phone" for field in self.web_form_fields): From b09e3c6f760da621ee20575359357669683fd5b5 Mon Sep 17 00:00:00 2001 From: Nabin Hait Date: Wed, 4 Jun 2025 18:24:25 +0530 Subject: [PATCH 09/66] fix: Get field currency based on default company if company is not available on doc (#32445) --- frappe/public/js/frappe/model/meta.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/public/js/frappe/model/meta.js b/frappe/public/js/frappe/model/meta.js index 9724b0729f..07b0f3bb44 100644 --- a/frappe/public/js/frappe/model/meta.js +++ b/frappe/public/js/frappe/model/meta.js @@ -287,7 +287,6 @@ $.extend(frappe.meta, { get_field_currency: function (df, doc) { var currency = frappe.boot.sysdefaults.currency || "USD"; if (!doc && cur_frm) doc = cur_frm.doc; - if (df && df.options) { if (df.options.indexOf(":") != -1) { var options = df.options.split(":"); @@ -299,7 +298,8 @@ $.extend(frappe.meta, { if (!docname && cur_frm) { docname = cur_frm.doc[options[1]]; } - } else { + } + if (!docname) { // Try to get default value, useful for cases like Company overridden in session defaults docname = frappe.defaults.get_user_default(options[1]); } From 670e46af4ad72f3e58ee5cbf9f6f046e2a605c46 Mon Sep 17 00:00:00 2001 From: Soham Kulkarni <77533095+sokumon@users.noreply.github.com> Date: Wed, 4 Jun 2025 18:44:35 +0530 Subject: [PATCH 10/66] fix(ui_tests): pin browser version in chrome to avoid mismatch error in parallel (#32774) --- .github/workflows/_base-ui-tests.yml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/.github/workflows/_base-ui-tests.yml b/.github/workflows/_base-ui-tests.yml index 88c22e28f1..7617a6abd6 100644 --- a/.github/workflows/_base-ui-tests.yml +++ b/.github/workflows/_base-ui-tests.yml @@ -92,13 +92,17 @@ jobs: -x '${{ github.event.repository.name }}/public/dist/**' \ -x '${{ github.event.repository.name }}/public/js/lib/**' \ -x '**/*.bundle.js' --compact=false --in-place ${{ github.event.repository.name }} - + - name: Site Setup run: | source ${GITHUB_WORKSPACE}/env/bin/activate bench --site test_site execute frappe.utils.install.complete_setup_wizard bench --site test_site execute frappe.tests.ui_test_helpers.create_test_user + - uses: browser-actions/setup-chrome@latest + - run: | + echo "BROWSER_PATH=$(which chrome)" >> $GITHUB_ENV + - name: Run Tests run: | source ${GITHUB_WORKSPACE}/env/bin/activate @@ -107,6 +111,7 @@ jobs: --with-coverage \ --headless \ --parallel \ + --browser ${{ env.BROWSER_PATH }} \ --ci-build-id $GITHUB_RUN_ID-$GITHUB_RUN_ATTEMPT env: CYPRESS_RECORD_KEY: 4a48f41c-11b3-425b-aa88-c58048fa69eb From ee9545e7f31aa3c9b4d0850613010fd579c50195 Mon Sep 17 00:00:00 2001 From: sokumon Date: Wed, 4 Jun 2025 13:21:08 +0530 Subject: [PATCH 11/66] fix: add h4 to h6 tags in text editor text dropdown --- frappe/public/js/frappe/form/controls/text_editor.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/public/js/frappe/form/controls/text_editor.js b/frappe/public/js/frappe/form/controls/text_editor.js index 61cc658b1f..582c6cace7 100644 --- a/frappe/public/js/frappe/form/controls/text_editor.js +++ b/frappe/public/js/frappe/form/controls/text_editor.js @@ -286,7 +286,7 @@ frappe.ui.form.ControlTextEditor = class ControlTextEditor extends frappe.ui.for get_toolbar_options() { return [ - [{ header: [1, 2, 3, false] }], + [{ header: [1, 2, 3, 4, 5, 6, false] }], [{ size: font_sizes }], ["bold", "italic", "underline", "strike", "clean"], [{ color: [] }, { background: [] }], From 2d1491881471ec75099c4d1bbbd13f807b6fc786 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Wed, 4 Jun 2025 19:15:27 +0530 Subject: [PATCH 12/66] fix!: Change count(cache=True) implmentation (#32779) This makes cache implementation uniform for all methods on db API. It's weird that this specific method was caching in redis, which defies expectations. --- frappe/database/database.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/frappe/database/database.py b/frappe/database/database.py index 1e8950775c..ce4326b4fb 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -1252,10 +1252,10 @@ class Database: def count(self, dt, filters=None, debug=False, cache=False, distinct: bool = True): """Return `COUNT(*)` for given DocType and filters.""" + cache_key = (dt, "COUNT(*)") if cache and not filters: - cache_count = frappe.cache.get_value(f"doctype:count:{dt}") - if cache_count is not None: - return cache_count + if cache_key in self.value_cache: + return self.value_cache[cache_key] count = frappe.qb.get_query( table=dt, filters=filters, @@ -1264,7 +1264,7 @@ class Database: validate_filters=True, ).run(debug=debug)[0][0] if not filters and cache: - frappe.cache.set_value(f"doctype:count:{dt}", count, expires_in_sec=86400) + self.value_cache[cache_key] = count return count def estimate_count(self, doctype: str) -> int: From 7d7d77d76223c639296207c54467a551543a64bb Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Wed, 4 Jun 2025 19:18:19 +0530 Subject: [PATCH 13/66] perf: misc small cruft (#32778) * perf: cache docstatus check for invalid links * perf: avoid querying if doctype is single * perf: cache is_single --- frappe/model/base_document.py | 4 +++- frappe/model/create_new.py | 2 +- frappe/model/meta.py | 13 ++++++------- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/frappe/model/base_document.py b/frappe/model/base_document.py index 5ccfbf0cce..071c1f8532 100644 --- a/frappe/model/base_document.py +++ b/frappe/model/base_document.py @@ -926,7 +926,9 @@ class BaseDocument: df.fieldname != "amended_from" and (is_submittable or self.meta.is_submittable) and frappe.get_meta(doctype).is_submittable - and DocStatus(frappe.db.get_value(doctype, docname, "docstatus") or 0).is_cancelled() + and DocStatus( + frappe.db.get_value(doctype, docname, "docstatus", cache=True) or 0 + ).is_cancelled() ): cancelled_links.append((df.fieldname, docname, get_msg(df, docname))) diff --git a/frappe/model/create_new.py b/frappe/model/create_new.py index 84fc2e1e5b..973d9fb619 100644 --- a/frappe/model/create_new.py +++ b/frappe/model/create_new.py @@ -41,7 +41,7 @@ def make_new_doc(doctype): doc["doctype"] = doctype doc["__islocal"] = 1 - if not frappe.model.meta.is_single(doctype): + if not getattr(doc.meta, "issingle", False): doc["__unsaved"] = 1 return doc diff --git a/frappe/model/meta.py b/frappe/model/meta.py index f12557d537..790bd1e586 100644 --- a/frappe/model/meta.py +++ b/frappe/model/meta.py @@ -38,10 +38,12 @@ from frappe.model.base_document import ( BaseDocument, ) from frappe.model.document import Document +from frappe.model.utils import is_single_doctype from frappe.model.workflow import get_workflow_name from frappe.modules import load_doctype_module from frappe.types import DocRef from frappe.utils import cached_property, cast, cint, cstr +from frappe.utils.caching import site_cache from frappe.utils.data import add_to_date, get_datetime DEFAULT_FIELD_LABELS = { @@ -806,13 +808,6 @@ class Meta(Document): ####### -def is_single(doctype): - try: - return frappe.db.get_value("DocType", doctype, "issingle") - except IndexError: - raise Exception("Cannot determine whether {} is single".format(doctype)) - - def get_parent_dt(dt): if not frappe.is_table(dt): return "" @@ -990,3 +985,7 @@ if typing.TYPE_CHECKING: class _Meta(Meta, DocType): pass + + +# backward compatibility +is_single = is_single_doctype From 8c0df08e8c35bf15c8799f06fa740f4ef37f5bc3 Mon Sep 17 00:00:00 2001 From: Corentin Forler <10946971+cogk@users.noreply.github.com> Date: Wed, 4 Jun 2025 16:41:03 +0200 Subject: [PATCH 14/66] fix(boot): Don't register app as "required" multiple times (#32773) --- frappe/boot.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/boot.py b/frappe/boot.py index 612e2f75be..cd51ff5464 100644 --- a/frappe/boot.py +++ b/frappe/boot.py @@ -128,7 +128,7 @@ def get_bootinfo(): def remove_apps_with_incomplete_dependencies(bootinfo): - remove_apps = [] + remove_apps = set() for app in bootinfo.setup_wizard_not_required_apps: if app in bootinfo.setup_wizard_completed_apps: @@ -142,7 +142,7 @@ def remove_apps_with_incomplete_dependencies(bootinfo): continue if required_app not in bootinfo.setup_wizard_completed_apps: - remove_apps.append(app) + remove_apps.add(app) for app in remove_apps: bootinfo.setup_wizard_not_required_apps.remove(app) From 051aba3049098791a53248751c7a95c464b696a1 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Wed, 4 Jun 2025 22:07:35 +0530 Subject: [PATCH 15/66] fix(DX): Fix type annotations for custom cached_property (#32785) + Avoid custom implementation on newer python versions entirely. --- frappe/utils/__init__.py | 73 ++++++++++++++++++++-------------------- 1 file changed, 36 insertions(+), 37 deletions(-) diff --git a/frappe/utils/__init__.py b/frappe/utils/__init__.py index 9f0ae2b567..174086101c 100644 --- a/frappe/utils/__init__.py +++ b/frappe/utils/__init__.py @@ -19,7 +19,7 @@ from collections.abc import ( ) from email.header import decode_header, make_header from email.utils import formataddr, parseaddr -from typing import TypeAlias, TypedDict +from typing import Any, Generic, TypeAlias, TypedDict from werkzeug.test import Client @@ -1153,46 +1153,45 @@ def safe_eval(code, eval_globals=None, eval_locals=None): return safe_eval(code, eval_globals, eval_locals) -class cached_property(functools.cached_property): - """ - A simpler `functools.cached_property` implementation without locks. - This isn't needed in Python 3.12+, since lock was removed in newer versions. - Hence, in those versions, it returns the `functools.cached_property` object. +cached_property = functools.cached_property +if sys.version_info.minor < 12: + T = TypeVar("T") - This does not prevent a possible race condition in multi-threaded usage. - The getter function could run more than once on the same instance, - with the latest run setting the cached value. If the cached property is - idempotent or otherwise not harmful to run more than once on an instance, - this is fine. If synchronization is needed, implement the necessary locking - inside the decorated getter function or around the cached property access. - """ + class cached_property(functools.cached_property, Generic[T]): + """ + A simpler `functools.cached_property` implementation without locks. + This isn't needed in Python 3.12+, since lock was removed in newer versions. + Hence, in those versions, it returns the `functools.cached_property` object. - def __new__(cls, func): - if sys.version_info.minor >= 12: - return functools.cached_property(func) + This does not prevent a possible race condition in multi-threaded usage. + The getter function could run more than once on the same instance, + with the latest run setting the cached value. If the cached property is + idempotent or otherwise not harmful to run more than once on an instance, + this is fine. If synchronization is needed, implement the necessary locking + inside the decorated getter function or around the cached property access. + """ - return super().__new__(cls) + def __init__(self, func: Callable[[Any], T]) -> T: + self.func = func + self.attrname = None + self.__doc__ = func.__doc__ + self.__module__ = func.__module__ - def __init__(self, func): - self.func = func - self.attrname = None - self.__doc__ = func.__doc__ - self.__module__ = func.__module__ + def __set_name__(self, owner, name): + if self.attrname is None: + self.attrname = name - def __set_name__(self, owner, name): - if self.attrname is None: - self.attrname = name + elif name != self.attrname: + raise TypeError( + "Cannot assign the same cached_property to two different names " + f"({self.attrname!r} and {name!r})." + ) - elif name != self.attrname: - raise TypeError( - "Cannot assign the same cached_property to two different names " - f"({self.attrname!r} and {name!r})." - ) + def __get__(self, instance, owner=None) -> T: + if instance is None: + return self - def __get__(self, instance, owner=None): - if instance is None: - return self - - value = self.func(instance) - instance.__dict__[self.attrname] = value - return value + value = self.func(instance) + instance.__dict__[self.attrname] = value + return value +# end: custom cached_property implementation From 9d82fe62ccb551fa93b2e15212af349cdbd95e7b Mon Sep 17 00:00:00 2001 From: Soham Kulkarni <77533095+sokumon@users.noreply.github.com> Date: Wed, 4 Jun 2025 23:50:28 +0530 Subject: [PATCH 16/66] ci: skip redis vuln check (#32787) --- .github/workflows/linters.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/linters.yml b/.github/workflows/linters.yml index 6c0b3d297c..1d3a9076e2 100644 --- a/.github/workflows/linters.yml +++ b/.github/workflows/linters.yml @@ -95,7 +95,7 @@ jobs: run: | pip install pip-audit cd ${GITHUB_WORKSPACE} - pip-audit --desc on . + pip-audit --desc on --ignore-vuln PYSEC-2023-312 . precommit: name: 'Pre-Commit' From 30c537665386cca8c02fbbc6ee8e01a3813aa58b Mon Sep 17 00:00:00 2001 From: Kit Rhodes Date: Thu, 5 Jun 2025 12:53:01 +1000 Subject: [PATCH 17/66] docs: Correct spelling of 'specified' in frappe/commands/utils.py (#32790) --- frappe/commands/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/commands/utils.py b/frappe/commands/utils.py index ea17a029e8..ed126dff24 100644 --- a/frappe/commands/utils.py +++ b/frappe/commands/utils.py @@ -175,7 +175,7 @@ def destroy_all_sessions(context: CliCtxObj, reason=None): @click.option("--format", "-f", type=click.Choice(["text", "json"]), default="text") @pass_context def show_config(context: CliCtxObj, format): - "Print configuration file to STDOUT in speified format" + "Print configuration file to STDOUT in specified format" if not context.sites: raise SiteNotSpecifiedError From fcaf17392331aec1ba98a8d986fed4dacc08be8c Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Fri, 23 May 2025 20:34:20 +0530 Subject: [PATCH 18/66] refactor: new lightweight test runner --- frappe/commands/testing.py | 238 +++++++++++++++++++++---------------- 1 file changed, 134 insertions(+), 104 deletions(-) diff --git a/frappe/commands/testing.py b/frappe/commands/testing.py index 4acc05c9f6..9e3f8ef29e 100644 --- a/frappe/commands/testing.py +++ b/frappe/commands/testing.py @@ -32,129 +32,153 @@ def main( debug: bool = False, debug_exceptions: tuple[Exception] | None = None, selected_categories: list[str] | None = None, + light: bool = False, ) -> None: """Main function to run tests""" - import logging + if light: + from frappe.testing import TestConfig + from frappe.testing.environment import _disable_scheduler_if_needed - from frappe.testing import ( - TestConfig, - TestRunner, - discover_all_tests, - discover_doctype_tests, - discover_module_tests, - ) - from frappe.testing.environment import _cleanup_after_tests, _initialize_test_environment - from frappe.tests.utils.generators import _clear_test_log - - _clear_test_log() - - if debug and not debug_exceptions: - debug_exceptions = (Exception,) - - testing_module_logger = logging.getLogger("frappe.testing") - testing_module_logger.setLevel(logging.DEBUG if verbose else logging.INFO) - start_time = time.time() - - # Check for mutually exclusive arguments - exclusive_args = [doctype, doctype_list_path, module_def, module] - if sum(arg is not None for arg in exclusive_args) > 1: - raise click.UsageError( - "Error: The following arguments are mutually exclusive: " - "doctype, doctype_list_path, module_def, and module. " - "Please specify only one of these." + test_config = TestConfig( + profile=profile, + failfast=failfast, + tests=tests, + case=case, + pdb_on_exceptions=debug_exceptions, + selected_categories=selected_categories or [], + skip_before_tests=skip_before_tests, ) + # init environment + frappe.init(site) + if not frappe.db: + frappe.connect() - # Prepare debug log message - debug_params = [] - for param_name in [ - "site", - "app", - "module", - "doctype", - "module_def", - "verbose", - "tests", - "force", - "profile", - "junit_xml_output", - "doctype_list_path", - "failfast", - "case", - "skip_before_tests", - "debug_exceptions", - "debug", - "selected_categories", - ]: - param_value = locals()[param_name] - if param_value is not None: - debug_params.append(f"{param_name}={param_value}") + # require db access + _disable_scheduler_if_needed() + frappe.clear_cache() - if debug_params: - click.secho(f"Starting test run with parameters: {', '.join(debug_params)}", fg="cyan", bold=True) - testing_module_logger.info(f"started with: {', '.join(debug_params)}") else: - click.secho("Starting test run with no specific parameters", fg="cyan", bold=True) - testing_module_logger.info("started with no specific parameters") - for handler in testing_module_logger.handlers: - if file := getattr(handler, "baseFilename", None): - click.secho( - f"View detailed logs{' (using --verbose)' if not verbose else ''}: {click.style(file, bold=True)}" + import logging + + from frappe.testing import ( + TestConfig, + TestRunner, + discover_all_tests, + discover_doctype_tests, + discover_module_tests, + ) + from frappe.testing.environment import _cleanup_after_tests, _initialize_test_environment + from frappe.tests.utils.generators import _clear_test_log + + _clear_test_log() + + if debug and not debug_exceptions: + debug_exceptions = (Exception,) + + testing_module_logger = logging.getLogger("frappe.testing") + testing_module_logger.setLevel(logging.DEBUG if verbose else logging.INFO) + start_time = time.time() + + # Check for mutually exclusive arguments + exclusive_args = [doctype, doctype_list_path, module_def, module] + if sum(arg is not None for arg in exclusive_args) > 1: + raise click.UsageError( + "Error: The following arguments are mutually exclusive: " + "doctype, doctype_list_path, module_def, and module. " + "Please specify only one of these." ) - test_config = TestConfig( - profile=profile, - failfast=failfast, - tests=tests, - case=case, - pdb_on_exceptions=debug_exceptions, - selected_categories=selected_categories or [], - skip_before_tests=skip_before_tests, - ) + # Prepare debug log message + debug_params = [] + for param_name in [ + "site", + "app", + "module", + "doctype", + "module_def", + "verbose", + "tests", + "force", + "profile", + "junit_xml_output", + "doctype_list_path", + "failfast", + "case", + "skip_before_tests", + "debug_exceptions", + "debug", + "selected_categories", + ]: + param_value = locals()[param_name] + if param_value is not None: + debug_params.append(f"{param_name}={param_value}") - _initialize_test_environment(site, test_config) + if debug_params: + click.secho(f"Starting test run with parameters: {', '.join(debug_params)}", fg="cyan", bold=True) + testing_module_logger.info(f"started with: {', '.join(debug_params)}") + else: + click.secho("Starting test run with no specific parameters", fg="cyan", bold=True) + testing_module_logger.info("started with no specific parameters") + for handler in testing_module_logger.handlers: + if file := getattr(handler, "baseFilename", None): + click.secho( + f"View detailed logs{' (using --verbose)' if not verbose else ''}: {click.style(file, bold=True)}" + ) - xml_output_file = _setup_xml_output(junit_xml_output) - - try: - # Create TestRunner instance - runner = TestRunner( - verbosity=2 if testing_module_logger.getEffectiveLevel() < logging.INFO else 1, - tb_locals=testing_module_logger.getEffectiveLevel() <= logging.INFO, - cfg=test_config, - buffer=not debug, # unfortunate as it messes up stdout/stderr output order + test_config = TestConfig( + profile=profile, + failfast=failfast, + tests=tests, + case=case, + pdb_on_exceptions=debug_exceptions, + selected_categories=selected_categories or [], + skip_before_tests=skip_before_tests, ) - if doctype or doctype_list_path: - doctype = _load_doctype_list(doctype_list_path) if doctype_list_path else doctype - discover_doctype_tests(doctype, runner, app, force) - elif module_def: - _run_module_def_tests(app, module_def, runner, force) - elif module: - discover_module_tests(module, runner, app) - else: - apps = [app] if app else frappe.get_installed_apps() - discover_all_tests(apps, runner) + _initialize_test_environment(site, test_config) - results = [] - for app, category, suite in runner.iterRun(): - click.secho( - f"\nRunning {suite.countTestCases()} {category} tests for {app}", fg="cyan", bold=True + xml_output_file = _setup_xml_output(junit_xml_output) + + try: + # Create TestRunner instance + runner = TestRunner( + verbosity=2 if testing_module_logger.getEffectiveLevel() < logging.INFO else 1, + tb_locals=testing_module_logger.getEffectiveLevel() <= logging.INFO, + cfg=test_config, + buffer=not debug, # unfortunate as it messes up stdout/stderr output order ) - results.append([app, category, runner.run(suite)]) - success = all(r.wasSuccessful() for _, _, r in results) - if not success: - sys.exit(1) + if doctype or doctype_list_path: + doctype = _load_doctype_list(doctype_list_path) if doctype_list_path else doctype + discover_doctype_tests(doctype, runner, app, force) + elif module_def: + _run_module_def_tests(app, module_def, runner, force) + elif module: + discover_module_tests(module, runner, app) + else: + apps = [app] if app else frappe.get_installed_apps() + discover_all_tests(apps, runner) - return results + results = [] + for app, category, suite in runner.iterRun(): + click.secho( + f"\nRunning {suite.countTestCases()} {category} tests for {app}", fg="cyan", bold=True + ) + results.append([app, category, runner.run(suite)]) - finally: - _cleanup_after_tests() - if xml_output_file: - xml_output_file.close() + success = all(r.wasSuccessful() for _, _, r in results) + if not success: + sys.exit(1) - end_time = time.time() - testing_module_logger.debug(f"Total test run time: {end_time - start_time:.3f} seconds") + return results + + finally: + _cleanup_after_tests() + if xml_output_file: + xml_output_file.close() + + end_time = time.time() + testing_module_logger.debug(f"Total test run time: {end_time - start_time:.3f} seconds") def _setup_xml_output(junit_xml_output): @@ -246,6 +270,7 @@ def _get_doctypes_for_module_def(app, module_def): default="all", help="Select test category to run", ) +@click.option("--light", is_flag=True, default=False) @pass_context def run_tests( context: CliCtxObj, @@ -263,6 +288,7 @@ def run_tests( failfast=False, case=None, test_category="all", + light=False, debug=False, ): """Run python unit-tests""" @@ -290,6 +316,9 @@ def run_tests( click.secho("Simply remove the flag.", fg="green") return + if light: + click.secho("Light test runner selected") + main( site, app, @@ -307,6 +336,7 @@ def run_tests( skip_before_tests=skip_before_tests, debug=debug, selected_categories=[] if test_category == "all" else test_category, + light=light, ) From 7e6b8fbf1c91da0d20ef2d1c8ca4d16a08d2db4b Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Sun, 25 May 2025 09:26:19 +0530 Subject: [PATCH 19/66] refactor: better code readibility --- frappe/commands/testing.py | 73 ++++++++++++++++++++++++++++++++++++++ frappe/testing/config.py | 19 ++++++++++ 2 files changed, 92 insertions(+) diff --git a/frappe/commands/testing.py b/frappe/commands/testing.py index 9e3f8ef29e..481f259d76 100644 --- a/frappe/commands/testing.py +++ b/frappe/commands/testing.py @@ -1,7 +1,11 @@ +import importlib import os import subprocess import sys import time +import unittest +from collections import deque +from pathlib import Path from typing import TYPE_CHECKING import click @@ -37,6 +41,7 @@ def main( """Main function to run tests""" if light: from frappe.testing import TestConfig + from frappe.testing.config import TestParameters from frappe.testing.environment import _disable_scheduler_if_needed test_config = TestConfig( @@ -48,6 +53,23 @@ def main( selected_categories=selected_categories or [], skip_before_tests=skip_before_tests, ) + + test_params = TestParameters( + site=site, + app=app, + module=module, + doctype=doctype, + module_def=module_def, + verbose=verbose, + tests=tests, + force=force, + profile=profile, + junit_xml_output=junit_xml_output, + doctype_list_path=doctype_list_path, + failfast=failfast, + case=case, + ) + # init environment frappe.init(site) if not frappe.db: @@ -57,6 +79,57 @@ def main( _disable_scheduler_if_needed() frappe.clear_cache() + # class LightTestRunner(unittest.TextTestRunner): + # def run(test): + # print(f"calling super no {test}") + # super().run(test) + # lightrunner = LightTestRunner() + + # Methods of this loader should be frappe specific + # it should respect parameters passed from command line + class FrappeTestLoader(unittest.TestLoader): + def __init__(self, params: TestParameters): + super(unittest.TestLoader, self).__init__() + self.params = params + + def discover( + self, + start_dir: str | None = None, + pattern: str | None = None, + top_level_dir: str | None = None, + ) -> unittest.TestSuite: + testsuite = unittest.TestSuite() + self.files = [] + apps = self.params.app or frappe.get_installed_apps() + for app in apps: + app_path = Path(frappe.get_app_path(app)) + for test_file in app_path.glob("**/test_*.py"): + module_name = f"{'.'.join(test_file.relative_to(app_path.parent).parent.parts)}.{test_file.stem}" + module = importlib.import_module(module_name) + self.files.append((test_file, module)) + + for file, module in self.files: + _s = unittest.defaultTestLoader.loadTestsFromModule(module) + suites_to_process = deque([_s]) + while suites_to_process: + suite = suites_to_process.popleft() + for elem in suite: + if elem.countTestCases(): + if isinstance(elem, unittest.TestSuite): + suites_to_process.append(elem) + elif isinstance(elem, unittest.TestCase): + if self.params.tests: + if elem._testMethodName in self.params.tests: + testsuite.addTest(elem) + else: + testsuite.addTest(elem) + + return testsuite + + suite = FrappeTestLoader(test_params).discover() + for x in suite: + print(x) + else: import logging diff --git a/frappe/testing/config.py b/frappe/testing/config.py index b44d165f12..eb6d47450a 100644 --- a/frappe/testing/config.py +++ b/frappe/testing/config.py @@ -12,3 +12,22 @@ class TestConfig: pdb_on_exceptions: tuple | None = None selected_categories: list[str] = field(default_factory=list) skip_before_tests: bool = False + + +@dataclass +class TestParameters: + """Configuration class for test runner""" + + site: str | None = None + app: str | None = None + module: str | None = None + doctype: str | None = None + module_def: str | None = None + verbose: bool = False + tests: tuple = () + force: bool = False + profile: bool = False + junit_xml_output: str | None = None + doctype_list_path: str | None = None + failfast: bool = False + case: str | None = None From c659f9021a70c1973a6eef18e16251b09eef25d6 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Wed, 28 May 2025 09:36:00 +0530 Subject: [PATCH 20/66] refactor: cleaner code with debugging statements --- frappe/commands/testing.py | 123 +++++++++++++++++++++---------------- 1 file changed, 70 insertions(+), 53 deletions(-) diff --git a/frappe/commands/testing.py b/frappe/commands/testing.py index 481f259d76..f0346a33d8 100644 --- a/frappe/commands/testing.py +++ b/frappe/commands/testing.py @@ -40,20 +40,10 @@ def main( ) -> None: """Main function to run tests""" if light: - from frappe.testing import TestConfig + from frappe.modules.utils import get_module_name from frappe.testing.config import TestParameters from frappe.testing.environment import _disable_scheduler_if_needed - test_config = TestConfig( - profile=profile, - failfast=failfast, - tests=tests, - case=case, - pdb_on_exceptions=debug_exceptions, - selected_categories=selected_categories or [], - skip_before_tests=skip_before_tests, - ) - test_params = TestParameters( site=site, app=app, @@ -79,56 +69,83 @@ def main( _disable_scheduler_if_needed() frappe.clear_cache() - # class LightTestRunner(unittest.TextTestRunner): - # def run(test): - # print(f"calling super no {test}") - # super().run(test) - # lightrunner = LightTestRunner() - - # Methods of this loader should be frappe specific - # it should respect parameters passed from command line class FrappeTestLoader(unittest.TestLoader): - def __init__(self, params: TestParameters): - super(unittest.TestLoader, self).__init__() - self.params = params + def recursive_load_suites_in_pymodule(self, suite): + suites_queue = deque([suite]) + while suites_queue: + suite = suites_queue.popleft() + for elem in suite: + if elem.countTestCases(): + if isinstance(elem, unittest.TestSuite): + suites_queue.append(elem) + elif isinstance(elem, unittest.TestCase): + if self.params.tests: + if elem._testMethodName in self.params.tests: + self.testsuite.addTest(elem) + else: + self.testsuite.addTest(elem) - def discover( - self, - start_dir: str | None = None, - pattern: str | None = None, - top_level_dir: str | None = None, - ) -> unittest.TestSuite: - testsuite = unittest.TestSuite() - self.files = [] - apps = self.params.app or frappe.get_installed_apps() + def load_testsuites_in_pymodule(self, file_modules): + for module in file_modules: + suite = unittest.defaultTestLoader.loadTestsFromModule(module) + self.recursive_load_suites_in_pymodule(suite) + + def load_pymodule_for_files(self, files: list): + """ + files: list of tuple of (Path, str) + """ + _file_modules = [] + for app_path, test_file in files: + module_name = ( + f"{'.'.join(test_file.relative_to(app_path.parent).parent.parts)}.{test_file.stem}" + ) + module = importlib.import_module(module_name) + _file_modules.append(module) + return _file_modules + + def get_files(self, apps: list) -> list: + files = [] for app in apps: app_path = Path(frappe.get_app_path(app)) for test_file in app_path.glob("**/test_*.py"): - module_name = f"{'.'.join(test_file.relative_to(app_path.parent).parent.parts)}.{test_file.stem}" - module = importlib.import_module(module_name) - self.files.append((test_file, module)) + files.append((app_path, test_file)) + return files - for file, module in self.files: - _s = unittest.defaultTestLoader.loadTestsFromModule(module) - suites_to_process = deque([_s]) - while suites_to_process: - suite = suites_to_process.popleft() - for elem in suite: - if elem.countTestCases(): - if isinstance(elem, unittest.TestSuite): - suites_to_process.append(elem) - elif isinstance(elem, unittest.TestCase): - if self.params.tests: - if elem._testMethodName in self.params.tests: - testsuite.addTest(elem) - else: - testsuite.addTest(elem) + def discover_tests(self, params: TestParameters) -> unittest.TestSuite: + self.params = params + self.testsuite = unittest.TestSuite() - return testsuite + if self.params.tests: + # handle --test; highest priority; will ignore --doctype and --app + files = self.get_files(frappe.get_installed_apps()) + file_pymodules = self.load_pymodule_for_files(files) + self.load_testsuites_in_pymodule(file_pymodules) - suite = FrappeTestLoader(test_params).discover() - for x in suite: - print(x) + elif self.params.doctype: + # handle --doctype; will ignore --app + module = frappe.get_cached_value("DocType", self.params.doctype, "module") + app = frappe.get_cached_value("Module Def", module, "app_name") + pymodule_name = get_module_name(self.params.doctype, module, "test_", app=app) + pymodule = importlib.import_module(pymodule_name) + self.load_testsuites_in_pymodule([pymodule]) + + elif self.params.app: + # handle --app + files = self.get_files([self.params.app]) + file_pymodules = self.load_pymodule_for_files(files) + self.load_testsuites_in_pymodule(file_pymodules) + + elif self.params.module: + # handle --module; supports --test as well + pymodule = importlib.import_module(self.params.module) + self.load_testsuites_in_pymodule([pymodule]) + + return self.testsuite + + suite = FrappeTestLoader().discover_tests(test_params) + print("Test Cases:", suite.countTestCases()) + res = unittest.TextTestRunner().run(suite) + print("Result:", res) else: import logging From 4640a78587c165120ee0ce0e9fe246b45b360fcd Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Thu, 29 May 2025 16:08:51 +0530 Subject: [PATCH 21/66] refactor: move loader to new fiel - barebones TestResult class - removed debugging statements --- frappe/commands/testing.py | 85 ++------------------------------------ frappe/testing/loader.py | 80 +++++++++++++++++++++++++++++++++++ frappe/testing/result.py | 45 ++++++++++++++++++++ 3 files changed, 129 insertions(+), 81 deletions(-) create mode 100644 frappe/testing/loader.py diff --git a/frappe/commands/testing.py b/frappe/commands/testing.py index f0346a33d8..68e516bd2b 100644 --- a/frappe/commands/testing.py +++ b/frappe/commands/testing.py @@ -1,17 +1,17 @@ -import importlib import os import subprocess import sys import time import unittest -from collections import deque -from pathlib import Path from typing import TYPE_CHECKING import click import frappe from frappe.commands import get_site, pass_context +from frappe.testing.config import TestParameters +from frappe.testing.loader import FrappeTestLoader +from frappe.testing.result import FrappeTestResult from frappe.utils.bench_helper import CliCtxObj if TYPE_CHECKING: @@ -40,8 +40,6 @@ def main( ) -> None: """Main function to run tests""" if light: - from frappe.modules.utils import get_module_name - from frappe.testing.config import TestParameters from frappe.testing.environment import _disable_scheduler_if_needed test_params = TestParameters( @@ -69,83 +67,8 @@ def main( _disable_scheduler_if_needed() frappe.clear_cache() - class FrappeTestLoader(unittest.TestLoader): - def recursive_load_suites_in_pymodule(self, suite): - suites_queue = deque([suite]) - while suites_queue: - suite = suites_queue.popleft() - for elem in suite: - if elem.countTestCases(): - if isinstance(elem, unittest.TestSuite): - suites_queue.append(elem) - elif isinstance(elem, unittest.TestCase): - if self.params.tests: - if elem._testMethodName in self.params.tests: - self.testsuite.addTest(elem) - else: - self.testsuite.addTest(elem) - - def load_testsuites_in_pymodule(self, file_modules): - for module in file_modules: - suite = unittest.defaultTestLoader.loadTestsFromModule(module) - self.recursive_load_suites_in_pymodule(suite) - - def load_pymodule_for_files(self, files: list): - """ - files: list of tuple of (Path, str) - """ - _file_modules = [] - for app_path, test_file in files: - module_name = ( - f"{'.'.join(test_file.relative_to(app_path.parent).parent.parts)}.{test_file.stem}" - ) - module = importlib.import_module(module_name) - _file_modules.append(module) - return _file_modules - - def get_files(self, apps: list) -> list: - files = [] - for app in apps: - app_path = Path(frappe.get_app_path(app)) - for test_file in app_path.glob("**/test_*.py"): - files.append((app_path, test_file)) - return files - - def discover_tests(self, params: TestParameters) -> unittest.TestSuite: - self.params = params - self.testsuite = unittest.TestSuite() - - if self.params.tests: - # handle --test; highest priority; will ignore --doctype and --app - files = self.get_files(frappe.get_installed_apps()) - file_pymodules = self.load_pymodule_for_files(files) - self.load_testsuites_in_pymodule(file_pymodules) - - elif self.params.doctype: - # handle --doctype; will ignore --app - module = frappe.get_cached_value("DocType", self.params.doctype, "module") - app = frappe.get_cached_value("Module Def", module, "app_name") - pymodule_name = get_module_name(self.params.doctype, module, "test_", app=app) - pymodule = importlib.import_module(pymodule_name) - self.load_testsuites_in_pymodule([pymodule]) - - elif self.params.app: - # handle --app - files = self.get_files([self.params.app]) - file_pymodules = self.load_pymodule_for_files(files) - self.load_testsuites_in_pymodule(file_pymodules) - - elif self.params.module: - # handle --module; supports --test as well - pymodule = importlib.import_module(self.params.module) - self.load_testsuites_in_pymodule([pymodule]) - - return self.testsuite - suite = FrappeTestLoader().discover_tests(test_params) - print("Test Cases:", suite.countTestCases()) - res = unittest.TextTestRunner().run(suite) - print("Result:", res) + res = unittest.TextTestRunner(resultclass=FrappeTestResult).run(suite) else: import logging diff --git a/frappe/testing/loader.py b/frappe/testing/loader.py new file mode 100644 index 0000000000..f36f8e28cd --- /dev/null +++ b/frappe/testing/loader.py @@ -0,0 +1,80 @@ +import importlib +import unittest +from collections import deque +from pathlib import Path + +import frappe +from frappe.modules.utils import get_module_name +from frappe.testing.config import TestParameters + + +class FrappeTestLoader(unittest.TestLoader): + def recursive_load_suites_in_pymodule(self, suite): + suites_queue = deque([suite]) + while suites_queue: + suite = suites_queue.popleft() + for elem in suite: + if elem.countTestCases(): + if isinstance(elem, unittest.TestSuite): + suites_queue.append(elem) + elif isinstance(elem, unittest.TestCase): + if self.params.tests: + if elem._testMethodName in self.params.tests: + self.testsuite.addTest(elem) + else: + self.testsuite.addTest(elem) + + def load_testsuites_in_pymodule(self, file_modules): + for module in file_modules: + suite = unittest.defaultTestLoader.loadTestsFromModule(module) + self.recursive_load_suites_in_pymodule(suite) + + def load_pymodule_for_files(self, files: list): + """ + files: list of tuple of (Path, str) + """ + _file_modules = [] + for app_path, test_file in files: + module_name = f"{'.'.join(test_file.relative_to(app_path.parent).parent.parts)}.{test_file.stem}" + module = importlib.import_module(module_name) + _file_modules.append(module) + return _file_modules + + def get_files(self, apps: list) -> list: + files = [] + for app in apps: + app_path = Path(frappe.get_app_path(app)) + for test_file in app_path.glob("**/test_*.py"): + files.append((app_path, test_file)) + return files + + def discover_tests(self, params: TestParameters) -> unittest.TestSuite: + self.params = params + self.testsuite = unittest.TestSuite() + + if self.params.tests: + # handle --test; highest priority; will ignore --doctype and --app + files = self.get_files(frappe.get_installed_apps()) + file_pymodules = self.load_pymodule_for_files(files) + self.load_testsuites_in_pymodule(file_pymodules) + + elif self.params.doctype: + # handle --doctype; will ignore --app + module = frappe.get_cached_value("DocType", self.params.doctype, "module") + app = frappe.get_cached_value("Module Def", module, "app_name") + pymodule_name = get_module_name(self.params.doctype, module, "test_", app=app) + pymodule = importlib.import_module(pymodule_name) + self.load_testsuites_in_pymodule([pymodule]) + + elif self.params.app: + # handle --app + files = self.get_files([self.params.app]) + file_pymodules = self.load_pymodule_for_files(files) + self.load_testsuites_in_pymodule(file_pymodules) + + elif self.params.module: + # handle --module; supports --test as well + pymodule = importlib.import_module(self.params.module) + self.load_testsuites_in_pymodule([pymodule]) + + return self.testsuite diff --git a/frappe/testing/result.py b/frappe/testing/result.py index 4aca1892a6..558790e8de 100644 --- a/frappe/testing/result.py +++ b/frappe/testing/result.py @@ -178,3 +178,48 @@ class TestResult(unittest.TextTestResult): SLOW_TEST_THRESHOLD = 2 + + +class FrappeTestResult(unittest.TextTestResult): + def __init__(self, stream, descriptions, verbosity): + super().__init__(stream, descriptions, verbosity) + + def startTest(self, test): + self.tb_locals = True + self._started_at = time.monotonic() + super().startTest(test) + super(unittest.TextTestResult, self).startTest(test) + test_class = unittest.util.strclass(test.__class__) + if not hasattr(self, "current_test_class") or self.current_test_class != test_class: + click.echo(f"\n{unittest.util.strclass(test.__class__)}") + self.current_test_class = test_class + + def getTestMethodName(self, test): + return test._testMethodName if hasattr(test, "_testMethodName") else str(test) + + def addSuccess(self, test): + super(unittest.TextTestResult, self).addSuccess(test) + elapsed = time.time() - self._started_at + threshold_passed = elapsed >= SLOW_TEST_THRESHOLD + elapsed = click.style(f" ({elapsed:.03}s)", fg="red") if threshold_passed else "" + click.echo(f" {click.style(' ✔ ', fg='green')} {self.getTestMethodName(test)}{elapsed}") + + def addError(self, test, err): + super(unittest.TextTestResult, self).addError(test, err) + click.echo(f" {click.style(' ✖ ', fg='red')} {self.getTestMethodName(test)}") + + def addFailure(self, test, err): + super(unittest.TextTestResult, self).addFailure(test, err) + click.echo(f" {click.style(' ✖ ', fg='red')} {self.getTestMethodName(test)}") + + def addSkip(self, test, reason): + super(unittest.TextTestResult, self).addSkip(test, reason) + click.echo(f" {click.style(' = ', fg='white')} {self.getTestMethodName(test)}") + + def addExpectedFailure(self, test, err): + super(unittest.TextTestResult, self).addExpectedFailure(test, err) + click.echo(f" {click.style(' ✖ ', fg='red')} {self.getTestMethodName(test)}") + + def addUnexpectedSuccess(self, test): + super(unittest.TextTestResult, self).addUnexpectedSuccess(test) + click.echo(f" {click.style(' ✔ ', fg='green')} {self.getTestMethodName(test)}") From 0a02603cfee232d325b8c60a0639c89528b8e077 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Thu, 29 May 2025 16:17:35 +0530 Subject: [PATCH 22/66] refactor: rename parameter and move code to function --- frappe/commands/testing.py | 255 +++++++++++++++++++------------------ 1 file changed, 129 insertions(+), 126 deletions(-) diff --git a/frappe/commands/testing.py b/frappe/commands/testing.py index 68e516bd2b..0512a82cb1 100644 --- a/frappe/commands/testing.py +++ b/frappe/commands/testing.py @@ -36,10 +36,10 @@ def main( debug: bool = False, debug_exceptions: tuple[Exception] | None = None, selected_categories: list[str] | None = None, - light: bool = False, + lightmode: bool = False, ) -> None: """Main function to run tests""" - if light: + if lightmode: from frappe.testing.environment import _disable_scheduler_if_needed test_params = TestParameters( @@ -57,141 +57,147 @@ def main( failfast=failfast, case=case, ) + run_tests_in_light_mode(test_params) + return - # init environment - frappe.init(site) - if not frappe.db: - frappe.connect() + import logging - # require db access - _disable_scheduler_if_needed() - frappe.clear_cache() + from frappe.testing import ( + TestConfig, + TestRunner, + discover_all_tests, + discover_doctype_tests, + discover_module_tests, + ) + from frappe.testing.environment import _cleanup_after_tests, _initialize_test_environment + from frappe.tests.utils.generators import _clear_test_log - suite = FrappeTestLoader().discover_tests(test_params) - res = unittest.TextTestRunner(resultclass=FrappeTestResult).run(suite) + _clear_test_log() + if debug and not debug_exceptions: + debug_exceptions = (Exception,) + + testing_module_logger = logging.getLogger("frappe.testing") + testing_module_logger.setLevel(logging.DEBUG if verbose else logging.INFO) + start_time = time.time() + + # Check for mutually exclusive arguments + exclusive_args = [doctype, doctype_list_path, module_def, module] + if sum(arg is not None for arg in exclusive_args) > 1: + raise click.UsageError( + "Error: The following arguments are mutually exclusive: " + "doctype, doctype_list_path, module_def, and module. " + "Please specify only one of these." + ) + + # Prepare debug log message + debug_params = [] + for param_name in [ + "site", + "app", + "module", + "doctype", + "module_def", + "verbose", + "tests", + "force", + "profile", + "junit_xml_output", + "doctype_list_path", + "failfast", + "case", + "skip_before_tests", + "debug_exceptions", + "debug", + "selected_categories", + ]: + param_value = locals()[param_name] + if param_value is not None: + debug_params.append(f"{param_name}={param_value}") + + if debug_params: + click.secho(f"Starting test run with parameters: {', '.join(debug_params)}", fg="cyan", bold=True) + testing_module_logger.info(f"started with: {', '.join(debug_params)}") else: - import logging - - from frappe.testing import ( - TestConfig, - TestRunner, - discover_all_tests, - discover_doctype_tests, - discover_module_tests, - ) - from frappe.testing.environment import _cleanup_after_tests, _initialize_test_environment - from frappe.tests.utils.generators import _clear_test_log - - _clear_test_log() - - if debug and not debug_exceptions: - debug_exceptions = (Exception,) - - testing_module_logger = logging.getLogger("frappe.testing") - testing_module_logger.setLevel(logging.DEBUG if verbose else logging.INFO) - start_time = time.time() - - # Check for mutually exclusive arguments - exclusive_args = [doctype, doctype_list_path, module_def, module] - if sum(arg is not None for arg in exclusive_args) > 1: - raise click.UsageError( - "Error: The following arguments are mutually exclusive: " - "doctype, doctype_list_path, module_def, and module. " - "Please specify only one of these." + click.secho("Starting test run with no specific parameters", fg="cyan", bold=True) + testing_module_logger.info("started with no specific parameters") + for handler in testing_module_logger.handlers: + if file := getattr(handler, "baseFilename", None): + click.secho( + f"View detailed logs{' (using --verbose)' if not verbose else ''}: {click.style(file, bold=True)}" ) - # Prepare debug log message - debug_params = [] - for param_name in [ - "site", - "app", - "module", - "doctype", - "module_def", - "verbose", - "tests", - "force", - "profile", - "junit_xml_output", - "doctype_list_path", - "failfast", - "case", - "skip_before_tests", - "debug_exceptions", - "debug", - "selected_categories", - ]: - param_value = locals()[param_name] - if param_value is not None: - debug_params.append(f"{param_name}={param_value}") + test_config = TestConfig( + profile=profile, + failfast=failfast, + tests=tests, + case=case, + pdb_on_exceptions=debug_exceptions, + selected_categories=selected_categories or [], + skip_before_tests=skip_before_tests, + ) - if debug_params: - click.secho(f"Starting test run with parameters: {', '.join(debug_params)}", fg="cyan", bold=True) - testing_module_logger.info(f"started with: {', '.join(debug_params)}") + _initialize_test_environment(site, test_config) + + xml_output_file = _setup_xml_output(junit_xml_output) + + try: + # Create TestRunner instance + runner = TestRunner( + verbosity=2 if testing_module_logger.getEffectiveLevel() < logging.INFO else 1, + tb_locals=testing_module_logger.getEffectiveLevel() <= logging.INFO, + cfg=test_config, + buffer=not debug, # unfortunate as it messes up stdout/stderr output order + ) + + if doctype or doctype_list_path: + doctype = _load_doctype_list(doctype_list_path) if doctype_list_path else doctype + discover_doctype_tests(doctype, runner, app, force) + elif module_def: + _run_module_def_tests(app, module_def, runner, force) + elif module: + discover_module_tests(module, runner, app) else: - click.secho("Starting test run with no specific parameters", fg="cyan", bold=True) - testing_module_logger.info("started with no specific parameters") - for handler in testing_module_logger.handlers: - if file := getattr(handler, "baseFilename", None): - click.secho( - f"View detailed logs{' (using --verbose)' if not verbose else ''}: {click.style(file, bold=True)}" - ) + apps = [app] if app else frappe.get_installed_apps() + discover_all_tests(apps, runner) - test_config = TestConfig( - profile=profile, - failfast=failfast, - tests=tests, - case=case, - pdb_on_exceptions=debug_exceptions, - selected_categories=selected_categories or [], - skip_before_tests=skip_before_tests, - ) - - _initialize_test_environment(site, test_config) - - xml_output_file = _setup_xml_output(junit_xml_output) - - try: - # Create TestRunner instance - runner = TestRunner( - verbosity=2 if testing_module_logger.getEffectiveLevel() < logging.INFO else 1, - tb_locals=testing_module_logger.getEffectiveLevel() <= logging.INFO, - cfg=test_config, - buffer=not debug, # unfortunate as it messes up stdout/stderr output order + results = [] + for app, category, suite in runner.iterRun(): + click.secho( + f"\nRunning {suite.countTestCases()} {category} tests for {app}", fg="cyan", bold=True ) + results.append([app, category, runner.run(suite)]) - if doctype or doctype_list_path: - doctype = _load_doctype_list(doctype_list_path) if doctype_list_path else doctype - discover_doctype_tests(doctype, runner, app, force) - elif module_def: - _run_module_def_tests(app, module_def, runner, force) - elif module: - discover_module_tests(module, runner, app) - else: - apps = [app] if app else frappe.get_installed_apps() - discover_all_tests(apps, runner) + success = all(r.wasSuccessful() for _, _, r in results) + if not success: + sys.exit(1) - results = [] - for app, category, suite in runner.iterRun(): - click.secho( - f"\nRunning {suite.countTestCases()} {category} tests for {app}", fg="cyan", bold=True - ) - results.append([app, category, runner.run(suite)]) + return results - success = all(r.wasSuccessful() for _, _, r in results) - if not success: - sys.exit(1) + finally: + _cleanup_after_tests() + if xml_output_file: + xml_output_file.close() - return results + end_time = time.time() + testing_module_logger.debug(f"Total test run time: {end_time - start_time:.3f} seconds") - finally: - _cleanup_after_tests() - if xml_output_file: - xml_output_file.close() - end_time = time.time() - testing_module_logger.debug(f"Total test run time: {end_time - start_time:.3f} seconds") +def run_tests_in_light_mode(test_params: TestParameters): + # init environment + frappe.init(test_params.site) + if not frappe.db: + frappe.connect() + + # disable scheduler + global scheduler_disabled_by_user + scheduler_disabled_by_user = frappe.utils.scheduler.is_scheduler_disabled(verbose=False) + if not scheduler_disabled_by_user: + frappe.utils.scheduler.disable_scheduler() + frappe.clear_cache() + + suite = FrappeTestLoader().discover_tests(test_params) + unittest.TextTestRunner(resultclass=FrappeTestResult).run(suite) def _setup_xml_output(junit_xml_output): @@ -283,7 +289,7 @@ def _get_doctypes_for_module_def(app, module_def): default="all", help="Select test category to run", ) -@click.option("--light", is_flag=True, default=False) +@click.option("--lightmode", is_flag=True, default=False) @pass_context def run_tests( context: CliCtxObj, @@ -301,7 +307,7 @@ def run_tests( failfast=False, case=None, test_category="all", - light=False, + lightmode=False, debug=False, ): """Run python unit-tests""" @@ -329,9 +335,6 @@ def run_tests( click.secho("Simply remove the flag.", fg="green") return - if light: - click.secho("Light test runner selected") - main( site, app, @@ -349,7 +352,7 @@ def run_tests( skip_before_tests=skip_before_tests, debug=debug, selected_categories=[] if test_category == "all" else test_category, - light=light, + lightmode=lightmode, ) From 0c6c6f50c4c84ba51fde1620f8c581d85d5f2802 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Thu, 29 May 2025 17:34:49 +0530 Subject: [PATCH 23/66] refactor: support failfast and minor changes --- frappe/commands/testing.py | 4 +--- frappe/testing/config.py | 2 -- frappe/testing/loader.py | 2 +- frappe/testing/result.py | 2 +- 4 files changed, 3 insertions(+), 7 deletions(-) diff --git a/frappe/commands/testing.py b/frappe/commands/testing.py index 0512a82cb1..9d2f2e67c7 100644 --- a/frappe/commands/testing.py +++ b/frappe/commands/testing.py @@ -40,8 +40,6 @@ def main( ) -> None: """Main function to run tests""" if lightmode: - from frappe.testing.environment import _disable_scheduler_if_needed - test_params = TestParameters( site=site, app=app, @@ -197,7 +195,7 @@ def run_tests_in_light_mode(test_params: TestParameters): frappe.clear_cache() suite = FrappeTestLoader().discover_tests(test_params) - unittest.TextTestRunner(resultclass=FrappeTestResult).run(suite) + unittest.TextTestRunner(failfast=test_params.failfast, resultclass=FrappeTestResult).run(suite) def _setup_xml_output(junit_xml_output): diff --git a/frappe/testing/config.py b/frappe/testing/config.py index eb6d47450a..272bb550ce 100644 --- a/frappe/testing/config.py +++ b/frappe/testing/config.py @@ -16,8 +16,6 @@ class TestConfig: @dataclass class TestParameters: - """Configuration class for test runner""" - site: str | None = None app: str | None = None module: str | None = None diff --git a/frappe/testing/loader.py b/frappe/testing/loader.py index f36f8e28cd..e8611b9ffa 100644 --- a/frappe/testing/loader.py +++ b/frappe/testing/loader.py @@ -26,7 +26,7 @@ class FrappeTestLoader(unittest.TestLoader): def load_testsuites_in_pymodule(self, file_modules): for module in file_modules: - suite = unittest.defaultTestLoader.loadTestsFromModule(module) + suite = self.loadTestsFromModule(module) self.recursive_load_suites_in_pymodule(suite) def load_pymodule_for_files(self, files: list): diff --git a/frappe/testing/result.py b/frappe/testing/result.py index 558790e8de..b2486da02a 100644 --- a/frappe/testing/result.py +++ b/frappe/testing/result.py @@ -199,7 +199,7 @@ class FrappeTestResult(unittest.TextTestResult): def addSuccess(self, test): super(unittest.TextTestResult, self).addSuccess(test) - elapsed = time.time() - self._started_at + elapsed = time.monotonic() - self._started_at threshold_passed = elapsed >= SLOW_TEST_THRESHOLD elapsed = click.style(f" ({elapsed:.03}s)", fg="red") if threshold_passed else "" click.echo(f" {click.style(' ✔ ', fg='green')} {self.getTestMethodName(test)}{elapsed}") From 31c4bbec7338113a45897da9db18693af03e1826 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Fri, 30 May 2025 16:44:38 +0530 Subject: [PATCH 24/66] refactor: move imports to defer memory usage --- frappe/commands/testing.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/frappe/commands/testing.py b/frappe/commands/testing.py index 9d2f2e67c7..df20556ff6 100644 --- a/frappe/commands/testing.py +++ b/frappe/commands/testing.py @@ -9,9 +9,6 @@ import click import frappe from frappe.commands import get_site, pass_context -from frappe.testing.config import TestParameters -from frappe.testing.loader import FrappeTestLoader -from frappe.testing.result import FrappeTestResult from frappe.utils.bench_helper import CliCtxObj if TYPE_CHECKING: @@ -40,6 +37,8 @@ def main( ) -> None: """Main function to run tests""" if lightmode: + from frappe.testing.config import TestParameters + test_params = TestParameters( site=site, app=app, @@ -181,7 +180,10 @@ def main( testing_module_logger.debug(f"Total test run time: {end_time - start_time:.3f} seconds") -def run_tests_in_light_mode(test_params: TestParameters): +def run_tests_in_light_mode(test_params): + from frappe.testing.loader import FrappeTestLoader + from frappe.testing.result import FrappeTestResult + # init environment frappe.init(test_params.site) if not frappe.db: From ddbaf09125130776654d145246df7b57c7bffd67 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Thu, 5 Jun 2025 09:55:05 +0530 Subject: [PATCH 25/66] fix: Standard field falsy comparisons in db_query (#32791) Extends the fix to standard fields. https://github.com/frappe/frappe/pull/32377/commits/e0f63a928f891d4733b4af9502be7daa86ffbfc3 --- frappe/model/db_query.py | 2 +- frappe/tests/test_db_query.py | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/frappe/model/db_query.py b/frappe/model/db_query.py index 81a9b982ad..a34dc8e58c 100644 --- a/frappe/model/db_query.py +++ b/frappe/model/db_query.py @@ -885,7 +885,7 @@ from {tables} df and (db_type := cstr(frappe.db.type_map.get(df.fieldtype, " ")[0])) and db_type in ("varchar", "text", "longtext", "smalltext", "json") - ): + ) or f.fieldname in ("owner", "modified_by", "parent", "parentfield", "parenttype"): value = cstr(f.value) fallback = "''" diff --git a/frappe/tests/test_db_query.py b/frappe/tests/test_db_query.py index e4d68e9f80..2f4644d766 100644 --- a/frappe/tests/test_db_query.py +++ b/frappe/tests/test_db_query.py @@ -1202,6 +1202,8 @@ class TestDBQuery(IntegrationTestCase): self.assertNotIn("\\'", query) self.assertNotIn("ifnull", query) self.assertFalse(frappe.get_all("DocField", {"name": None})) + self.assertFalse(frappe.get_all("DocField", {"parent": None})) + self.assertNotIn("0", frappe.get_all("DocField", {"parent": None}, run=0)) def test_ifnull_fallback_types(self): query = frappe.get_all("DocField", {"fieldname": ("!=", None)}, run=0) From 244a986cb9952fbb9ae827c6bc227fb8742bb5ea Mon Sep 17 00:00:00 2001 From: "Abdallah A. Zaqout" <26047413+zaqoutabed@users.noreply.github.com> Date: Thu, 5 Jun 2025 11:24:01 +0300 Subject: [PATCH 26/66] refactor: remove chart name from the label --- frappe/public/js/frappe/widgets/chart_widget.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/public/js/frappe/widgets/chart_widget.js b/frappe/public/js/frappe/widgets/chart_widget.js index e48d84b7dc..7130506ae0 100644 --- a/frappe/public/js/frappe/widgets/chart_widget.js +++ b/frappe/public/js/frappe/widgets/chart_widget.js @@ -312,7 +312,7 @@ export default class ChartWidget extends Widget { }, }, { - label: __("Export {0}", [__(this.chart_doc.chart_name)]), + label: __("Export"), action: "action-export", handler: () => { const data = [[this.chart_doc.chart_name]]; From 21308d3678984a49c15a7860f7a4725a57de847a Mon Sep 17 00:00:00 2001 From: sokumon Date: Thu, 5 Jun 2025 14:00:42 +0530 Subject: [PATCH 27/66] chore: bump datatable --- package.json | 2 +- yarn.lock | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/package.json b/package.json index 2d55997045..27e2716c62 100644 --- a/package.json +++ b/package.json @@ -52,7 +52,7 @@ "fast-deep-equal": "^2.0.1", "fast-glob": "^3.2.5", "frappe-charts": "^2.0.0-rc26", - "frappe-datatable": "1.18.4", + "frappe-datatable": "1.19.0", "frappe-gantt": "^0.6.0", "highlight.js": "^10.4.1", "html5-qrcode": "^2.3.8", diff --git a/yarn.lock b/yarn.lock index 2ea0ceccfb..798055edf4 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1431,10 +1431,10 @@ frappe-charts@^2.0.0-rc26: resolved "https://registry.yarnpkg.com/frappe-charts/-/frappe-charts-2.0.0-rc26.tgz#9632d620b92f2043cebd192a8899119f5715524b" integrity sha512-0vyXcwcekIeYA6pxCHGcRdG8llC6hpGR91nkbwRGSnBYMKomX2AQtfgTlIKMrE9nmAkewJeZsTx1scni8Ry0iA== -frappe-datatable@1.18.4: - version "1.18.4" - resolved "https://registry.yarnpkg.com/frappe-datatable/-/frappe-datatable-1.18.4.tgz#320b0fbb5039dd305be8c66f3a4fba73f6627469" - integrity sha512-nx8CUROmcb7svSPqtdAmPxcAI9GrOHXiwwz76/pVhsTBSsgqXqWHAyAIQzNDUQai2z2X5f0fYQNcjKZGKTyEqg== +frappe-datatable@1.19.0: + version "1.19.0" + resolved "https://registry.yarnpkg.com/frappe-datatable/-/frappe-datatable-1.19.0.tgz#43d9118603c6d1ecaab31aa21e8278694c17d849" + integrity sha512-DN7UIY8zrqagRV93mM4BS1tSNZ++8LT7E2cp3ZJEqeJeS/xQx9EVjhxAtQAEt2+QlDUL4hRYgXhRLH2Exz+K5w== dependencies: hyperlist "^1.0.0-beta" lodash "^4.17.5" From a3d5b4af77ecd255b615eb99b9b8ba0cbd6a1a75 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Thu, 5 Jun 2025 14:22:57 +0530 Subject: [PATCH 28/66] Revert "perf: cache docstatus check for invalid links" (#32799) This reverts commit be5c96acf22f208c4ec8fbcde92a721b0e84d561. --- frappe/model/base_document.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/frappe/model/base_document.py b/frappe/model/base_document.py index 071c1f8532..5ccfbf0cce 100644 --- a/frappe/model/base_document.py +++ b/frappe/model/base_document.py @@ -926,9 +926,7 @@ class BaseDocument: df.fieldname != "amended_from" and (is_submittable or self.meta.is_submittable) and frappe.get_meta(doctype).is_submittable - and DocStatus( - frappe.db.get_value(doctype, docname, "docstatus", cache=True) or 0 - ).is_cancelled() + and DocStatus(frappe.db.get_value(doctype, docname, "docstatus") or 0).is_cancelled() ): cancelled_links.append((df.fieldname, docname, get_msg(df, docname))) From 47a47a9b5d8e019e5cdce87c2868b82ea54ad405 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Wed, 4 Jun 2025 21:05:50 +0530 Subject: [PATCH 29/66] refactor!: Change internal datastructure of db.value_cache It's now a defaultdictionary of `[doctype][name/filters][fieldname]` This allows us to implement granular clearing and improve usage of this cache. --- frappe/database/database.py | 45 ++++++++++--------- frappe/deprecation_dumpster.py | 2 +- frappe/model/document.py | 2 +- frappe/tests/classes/context_managers.py | 2 - frappe/tests/classes/integration_test_case.py | 2 +- frappe/tests/test_website.py | 1 - frappe/utils/data.py | 6 +++ .../doctype/blog_post/test_blog_post.py | 4 -- 8 files changed, 32 insertions(+), 32 deletions(-) diff --git a/frappe/database/database.py b/frappe/database/database.py index ce4326b4fb..18ab1d2c94 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -34,7 +34,16 @@ from frappe.exceptions import DoesNotExistError, ImplicitCommitError from frappe.monitor import get_trace_id from frappe.query_builder import Case from frappe.query_builder.functions import Count -from frappe.utils import CallbackManager, cint, get_datetime, get_table_name, getdate, now, sbool +from frappe.utils import ( + CallbackManager, + cint, + get_datetime, + get_table_name, + getdate, + now, + recursive_defaultdict, + sbool, +) from frappe.utils import cast as cast_fieldtype if TYPE_CHECKING: @@ -112,7 +121,7 @@ class Database: self.transaction_writes = 0 self.auto_commit_on_many_writes = 0 - self.value_cache = {} + self.value_cache = recursive_defaultdict() self.logger = frappe.logger("database") self.logger.setLevel("WARNING") @@ -615,11 +624,8 @@ class Database: user = frappe.db.get_values("User", "test@example.com", "*")[0] """ out = None - cache_key = None - if cache and isinstance(filters, str): - cache_key = (doctype, filters, fieldname) - if cache_key in self.value_cache: - return self.value_cache[cache_key] + if cache and isinstance(filters, str) and fieldname in self.value_cache[doctype][filters]: + return self.value_cache[doctype][filters][fieldname] if distinct: order_by = None @@ -701,8 +707,8 @@ class Database: distinct=distinct, ) - if cache and cache_key: - self.value_cache[cache_key] = out + if cache and isinstance(filters, str): + self.value_cache[doctype][filters][fieldname] = out return out @@ -858,8 +864,7 @@ class Database: frappe.qb.into("Singles").columns("doctype", "field", "value").insert(*singles_data).run(debug=debug) frappe.clear_document_cache(doctype, doctype) - if doctype in self.value_cache: - del self.value_cache[doctype] + self.value_cache.pop(doctype, None) def get_single_value(self, doctype: str, fieldname: str, cache: bool = True): """Get property of Single DocType. Cache locally by default @@ -872,10 +877,6 @@ class Database: # Get the default value of the company from the Global Defaults doctype. company = frappe.db.get_single_value('Global Defaults', 'default_company') """ - - if doctype not in self.value_cache: - self.value_cache[doctype] = {} - if cache and fieldname in self.value_cache[doctype]: return self.value_cache[doctype][fieldname] @@ -975,8 +976,7 @@ class Database: query.run(debug=debug) - if dt in self.value_cache: - del self.value_cache[dt] + self.value_cache.pop(dt, None) def bulk_update( self, @@ -1252,10 +1252,10 @@ class Database: def count(self, dt, filters=None, debug=False, cache=False, distinct: bool = True): """Return `COUNT(*)` for given DocType and filters.""" - cache_key = (dt, "COUNT(*)") - if cache and not filters: - if cache_key in self.value_cache: - return self.value_cache[cache_key] + cache_key = "COUNT(*)" + if cache and not filters and cache_key in self.value_cache[dt]: + return self.value_cache[dt][cache_key] + count = frappe.qb.get_query( table=dt, filters=filters, @@ -1263,8 +1263,9 @@ class Database: distinct=distinct, validate_filters=True, ).run(debug=debug)[0][0] + if not filters and cache: - self.value_cache[cache_key] = count + self.value_cache[dt][cache_key] = count return count def estimate_count(self, doctype: str) -> int: diff --git a/frappe/deprecation_dumpster.py b/frappe/deprecation_dumpster.py index 686fbc57c1..91aac5526f 100644 --- a/frappe/deprecation_dumpster.py +++ b/frappe/deprecation_dumpster.py @@ -576,7 +576,7 @@ def get_tests_CompatFrappeTestCase(): traceback.print_stack(limit=10) def _rollback_db(): - frappe.db.value_cache = {} + frappe.db.value_cache.clear() frappe.db.rollback() def _restore_thread_locals(flags): diff --git a/frappe/model/document.py b/frappe/model/document.py index d0061b8bfb..b55fdafd2a 100644 --- a/frappe/model/document.py +++ b/frappe/model/document.py @@ -706,7 +706,7 @@ class Document(BaseDocument, DocRef): ) if self.doctype in frappe.db.value_cache: - del frappe.db.value_cache[self.doctype] + frappe.db.value_cache.pop(self.doctype, None) def set_user_and_timestamp(self): self._original_modified = self.modified diff --git a/frappe/tests/classes/context_managers.py b/frappe/tests/classes/context_managers.py index c9c1b0024d..4ee9fd628e 100644 --- a/frappe/tests/classes/context_managers.py +++ b/frappe/tests/classes/context_managers.py @@ -88,8 +88,6 @@ def change_settings(doctype, settings_dict=None, /, commit=False, **settings) -> for key, value in settings_dict.items(): setattr(settings, key, value) settings.save(ignore_permissions=True) - # singles are cached by default, clear to avoid flake - frappe.db.value_cache[settings] = {} if commit: frappe.db.commit() yield diff --git a/frappe/tests/classes/integration_test_case.py b/frappe/tests/classes/integration_test_case.py index a089f2e0cd..ecb2a47e29 100644 --- a/frappe/tests/classes/integration_test_case.py +++ b/frappe/tests/classes/integration_test_case.py @@ -183,7 +183,7 @@ def _commit_watcher(): def _rollback_db(): - frappe.db.value_cache = {} + frappe.db.value_cache.clear() frappe.db.rollback() diff --git a/frappe/tests/test_website.py b/frappe/tests/test_website.py index 1309579f36..47c9d7548b 100644 --- a/frappe/tests/test_website.py +++ b/frappe/tests/test_website.py @@ -252,7 +252,6 @@ class TestWebsite(IntegrationTestCase): self.assertEqual(response.status_code, 404) def test_printview_page(self): - frappe.db.value_cache[("DocType", "Language", "name")] = (("Language",),) frappe.set_user("Administrator") content = get_response_content("/Language/ru") self.assertIn('
', content) diff --git a/frappe/utils/data.py b/frappe/utils/data.py index 94fa3c642e..12dbe8cbb3 100644 --- a/frappe/utils/data.py +++ b/frappe/utils/data.py @@ -12,6 +12,7 @@ import re import time import typing from code import compile_command +from collections import defaultdict from enum import Enum from functools import lru_cache from typing import Any, Literal, Optional, TypeVar @@ -2711,6 +2712,11 @@ def mock(type, size=1, locale="en"): return squashify(results) +# Recursive default dict with arbitrary levels of nesting +def recursive_defaultdict(): + return defaultdict(recursive_defaultdict) + + # This is used in test to count memory overhead of default imports. def _get_rss_memory_usage(): import psutil diff --git a/frappe/website/doctype/blog_post/test_blog_post.py b/frappe/website/doctype/blog_post/test_blog_post.py index ec9698bb6d..8697d0f9db 100644 --- a/frappe/website/doctype/blog_post/test_blog_post.py +++ b/frappe/website/doctype/blog_post/test_blog_post.py @@ -74,9 +74,6 @@ class TestBlogPost(IntegrationTestCase): category_page_link = next(iter(soup.find_all("a", href=re.compile(blog.blog_category)))) category_page_url = category_page_link["href"] - cached_value = frappe.db.value_cache.get(("DocType", "Blog Post", "name")) - frappe.db.value_cache[("DocType", "Blog Post", "name")] = (("Blog Post",),) - # Visit the category page (by following the link found in above stage) set_request(path=category_page_url) category_page_response = get_response() @@ -85,7 +82,6 @@ class TestBlogPost(IntegrationTestCase): self.assertIn(blog.title, category_page_html) # Cleanup - frappe.db.value_cache[("DocType", "Blog Post", "name")] = cached_value frappe.delete_doc("Blog Post", blog.name) frappe.delete_doc("Blog Category", blog.blog_category) From f1a03200ab61f74f94907240a2a7c1b0eebf59ba Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Wed, 4 Jun 2025 16:07:23 +0530 Subject: [PATCH 30/66] fix: clear db.value_cache when clearing doctype cache --- frappe/__init__.py | 2 ++ frappe/database/database.py | 4 ---- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index 39ae573211..d4f335f052 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -977,6 +977,8 @@ def get_document_cache_key(doctype: str, name: str): def clear_document_cache(doctype: str, name: str | None = None) -> None: + frappe.db.value_cache.pop(doctype, None) + def clear_in_redis(): if name is not None: cache.delete_value(get_document_cache_key(doctype, name)) diff --git a/frappe/database/database.py b/frappe/database/database.py index 18ab1d2c94..1501edef77 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -864,8 +864,6 @@ class Database: frappe.qb.into("Singles").columns("doctype", "field", "value").insert(*singles_data).run(debug=debug) frappe.clear_document_cache(doctype, doctype) - self.value_cache.pop(doctype, None) - def get_single_value(self, doctype: str, fieldname: str, cache: bool = True): """Get property of Single DocType. Cache locally by default @@ -976,8 +974,6 @@ class Database: query.run(debug=debug) - self.value_cache.pop(dt, None) - def bulk_update( self, doctype: str, From c434f4ca1436bf944a031b1e8bb1370cff9a486d Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Wed, 4 Jun 2025 16:04:17 +0530 Subject: [PATCH 31/66] Revert "Revert "perf: cache fetch from value too (#32700)" (#32708)" This reverts commit 8a3f65338cae40c77fe967105165510dd73cf8c3. --- frappe/model/base_document.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/frappe/model/base_document.py b/frappe/model/base_document.py index 5ccfbf0cce..aeb9e0fb65 100644 --- a/frappe/model/base_document.py +++ b/frappe/model/base_document.py @@ -889,16 +889,16 @@ class BaseDocument: # cache a single value type values = _dict(name=frappe.db.get_value(doctype, docname, "name", cache=True)) else: - values_to_fetch = ["name"] + [ - _df.fetch_from.split(".")[-1] for _df in fields_to_fetch - ] + values_to_fetch = ( + "name", + *(_df.fetch_from.split(".")[-1] for _df in fields_to_fetch), + ) # fallback to dict with field_to_fetch=None if link field value is not found # (for compatibility, `values` must have same data type) empty_values = _dict({value: None for value in values_to_fetch}) - # don't cache if fetching other values too values = ( - frappe.db.get_value(doctype, docname, values_to_fetch, as_dict=True) + frappe.db.get_value(doctype, docname, values_to_fetch, as_dict=True, cache=True) or empty_values ) From 4dd21f175ea79d401180bb4a3d666ab264585f1e Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Thu, 5 Jun 2025 14:26:17 +0530 Subject: [PATCH 32/66] perf: Avoid seprately fetching docstatus --- frappe/model/base_document.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/frappe/model/base_document.py b/frappe/model/base_document.py index aeb9e0fb65..f7a7cd5cd8 100644 --- a/frappe/model/base_document.py +++ b/frappe/model/base_document.py @@ -887,10 +887,13 @@ class BaseDocument: if not meta.get("is_virtual"): if not fields_to_fetch: # cache a single value type - values = _dict(name=frappe.db.get_value(doctype, docname, "name", cache=True)) + values = frappe.db.get_value( + doctype, docname, ("name", "docstatus"), cache=True, as_dict=True + ) else: values_to_fetch = ( "name", + "docstatus", *(_df.fetch_from.split(".")[-1] for _df in fields_to_fetch), ) @@ -926,7 +929,7 @@ class BaseDocument: df.fieldname != "amended_from" and (is_submittable or self.meta.is_submittable) and frappe.get_meta(doctype).is_submittable - and DocStatus(frappe.db.get_value(doctype, docname, "docstatus") or 0).is_cancelled() + and DocStatus(values.docstatus or 0).is_cancelled() ): cancelled_links.append((df.fieldname, docname, get_msg(df, docname))) From 3c0678efac9372e528896c888ab5c372c3e10ee6 Mon Sep 17 00:00:00 2001 From: Soham Kulkarni <77533095+sokumon@users.noreply.github.com> Date: Thu, 5 Jun 2025 17:37:24 +0530 Subject: [PATCH 33/66] fix(mobile): broken UI/ UX issues (#32577) * fix: add a way to go back to list view from form * fix: show the correct logo while routing * fix(mobile): make report view look decent * fix: only show the back button if no previous route * fix(mobile): broken timeline view * fix(mobile): broken timeline actions * fix(mobile): fix timeline content * fix: minify duration in mobile view * fix(mobile): align timeline dot * fix(mobile): toggle visiblity of standard filters in list view * fix: dont translate template string * fix(minor): dont wrap the list count --- frappe/public/icons/lucide.svg | 8 ++++ frappe/public/icons/timeless/icons.svg | 32 +++++++++---- .../js/frappe/form/footer/form_timeline.js | 42 ++++++++++++---- .../form/templates/timeline_message_box.html | 48 +++++++++++-------- frappe/public/js/frappe/list/base_list.js | 34 +++++++++++++ frappe/public/js/frappe/list/list_view.js | 3 +- frappe/public/js/frappe/router.js | 2 +- frappe/public/js/frappe/ui/apps_switcher.js | 2 +- frappe/public/js/frappe/ui/sidebar.js | 1 + frappe/public/js/frappe/ui/toolbar/toolbar.js | 22 +++++++++ frappe/public/js/frappe/views/formview.js | 1 + .../js/frappe/views/reports/report_view.js | 6 +-- frappe/public/scss/desk/report.scss | 15 +++--- frappe/public/scss/desk/timeline.scss | 15 ++++++ 14 files changed, 180 insertions(+), 51 deletions(-) diff --git a/frappe/public/icons/lucide.svg b/frappe/public/icons/lucide.svg index b9bbb03158..0e5eab1451 100644 --- a/frappe/public/icons/lucide.svg +++ b/frappe/public/icons/lucide.svg @@ -8,6 +8,14 @@ + + + + + + + + diff --git a/frappe/public/icons/timeless/icons.svg b/frappe/public/icons/timeless/icons.svg index b8dbde9d94..b8de8f59bc 100644 --- a/frappe/public/icons/timeless/icons.svg +++ b/frappe/public/icons/timeless/icons.svg @@ -60,6 +60,14 @@ Tip: use lucide.svg in /icons for all downloaded icons + + + + + + + + @@ -396,7 +404,11 @@ Tip: use lucide.svg in /icons for all downloaded icons - + + + + + @@ -728,14 +740,6 @@ Tip: use lucide.svg in /icons for all downloaded icons - - - - - - - - @@ -978,10 +982,20 @@ Tip: use lucide.svg in /icons for all downloaded icons + + + + + + + + + + diff --git a/frappe/public/js/frappe/form/footer/form_timeline.js b/frappe/public/js/frappe/form/footer/form_timeline.js index f541733616..7a398357a4 100644 --- a/frappe/public/js/frappe/form/footer/form_timeline.js +++ b/frappe/public/js/frappe/form/footer/form_timeline.js @@ -529,10 +529,28 @@ class FormTimeline extends BaseTimeline { ).click(() => { this.compose_mail(communication_doc, true); }); - actions.append(reply); - actions.append(reply_all); + if (frappe.is_mobile()) { + this.add_dropdown_item(communication_box, [reply, reply_all]); + } else { + actions.append(reply); + actions.append(reply_all); + } + } + add_dropdown_item(timeline_box, menu_items) { + let more_actions = timeline_box.find(".more-actions > .dropdown-menu > li"); + menu_items.forEach((m) => { + let action_name = m[0].classList[1]; + let formatted_action_name = + String(action_name).charAt(0).toUpperCase() + String(action_name).slice(1); + m.empty(); + m.append( + __("{0}", [frappe.utils.to_title_case(formatted_action_name.replace("-", " "))]) + ); + m.removeClass(); + m.addClass("dropdown-item"); + more_actions.append(m); + }); } - compose_mail(communication_doc = null, reply_all = false) { const args = { doc: this.frm.doc, @@ -646,11 +664,11 @@ class FormTimeline extends BaseTimeline { let edit_button = $(); let current_user = frappe.session.user; if (["Administrator", doc.owner].includes(current_user)) { - edit_button = $(` +
`) + .prependTo(this.$filter_list_wrapper.find(".filter-selector")) + .on("click", function () { + me.toggle_standard_filter(); + }); + let children = list_view.page.page_form.children(); + list_view.page.page_form.append(children.get().reverse()); + } + + toggle_standard_filter() { + if (this.standard_filters_visible) { + this.standard_filters_visible = false; + this.standard_filters_wrapper.hide(); + } else { + this.standard_filters_visible = true; + this.standard_filters_wrapper.show(); + } + let icon_name = !this.standard_filters_visible ? "funnel-plus" : "funnel-x"; + this.$filter_list_wrapper + .find(".filter-toggle") + .find("use") + .attr("href", `#icon-${icon_name}`); } setup() { diff --git a/frappe/public/js/frappe/list/list_view.js b/frappe/public/js/frappe/list/list_view.js index 763b1b7466..363e5f0b68 100644 --- a/frappe/public/js/frappe/list/list_view.js +++ b/frappe/public/js/frappe/list/list_view.js @@ -661,6 +661,7 @@ frappe.views.ListView = class ListView extends frappe.views.BaseList { ); $count.tooltip({ delay: { show: 600, hide: 100 }, trigger: "hover" }); $count.css("cursor", "pointer"); + $count.css("white-space", "nowrap"); $count.on("click", () => { me.count_upper_bound = 0; $count.off("click"); @@ -718,7 +719,7 @@ frappe.views.ListView = class ListView extends frappe.views.BaseList { .join(""); const right_html = ` - +