From 5cc21da6a11f05c058d52d171c005f7f588929a0 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 31 Jan 2023 13:06:44 +0530 Subject: [PATCH 01/25] fix: Interface DatabaseQuery to virtual doctypes' --- frappe/model/db_query.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/frappe/model/db_query.py b/frappe/model/db_query.py index efdad83f13..6984146d23 100644 --- a/frappe/model/db_query.py +++ b/frappe/model/db_query.py @@ -16,6 +16,7 @@ from frappe.core.doctype.server_script.server_script_utils import get_server_scr from frappe.database.utils import FallBackDateTimeStr, NestedSetHierarchy from frappe.model import optional_fields from frappe.model.meta import get_table_columns +from frappe.model.utils import is_virtual_doctype from frappe.model.utils.user_settings import get_user_settings, update_user_settings from frappe.query_builder.utils import Column from frappe.utils import ( @@ -163,6 +164,13 @@ class DatabaseQuery: if user_settings: self.user_settings = json.loads(user_settings) + if is_virtual_doctype(self.doctype): + from frappe.model.virtual_doctype import get_controller + + controller = get_controller(self.doctype) + self.parse_args() + return controller.get_list(self.__dict__) + self.columns = self.get_table_columns() # no table & ignore_ddl, return From 636c4701cfb41b8129b2b94462f36b1e89d2bc01 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 31 Jan 2023 13:18:27 +0530 Subject: [PATCH 02/25] perf: Batched List Updates * Perform batched list updates for a N documents made every second * list_update callbacks for doc refreshes maintained in cur_list.pending_document_refreshes --- frappe/public/js/frappe/list/list_view.js | 105 +++++++++++++--------- 1 file changed, 62 insertions(+), 43 deletions(-) diff --git a/frappe/public/js/frappe/list/list_view.js b/frappe/public/js/frappe/list/list_view.js index 4625f0aa8e..6716999dcb 100644 --- a/frappe/public/js/frappe/list/list_view.js +++ b/frappe/public/js/frappe/list/list_view.js @@ -1320,6 +1320,11 @@ frappe.views.ListView = class ListView extends frappe.views.BaseList { } setup_realtime_updates() { + this.pending_document_refreshes = []; + setInterval(() => { + this.process_document_refreshes(); + }, 1000); + if (this.list_view_settings && this.list_view_settings.disable_auto_refresh) { return; } @@ -1333,28 +1338,42 @@ frappe.views.ListView = class ListView extends frappe.views.BaseList { return; } - const { doctype, name } = data; - if (doctype !== this.doctype) return; + this.pending_document_refreshes.push(data); + }); + } - // filters to get only the doc with this name - const call_args = this.get_call_args(); - call_args.args.filters.push([this.doctype, "name", "=", name]); - call_args.args.start = 0; + process_document_refreshes() { + if (!this.pending_document_refreshes.length) return; - frappe.call(call_args).then(({ message }) => { - if (!message) return; - const data = frappe.utils.dict(message.keys, message.values); - if (!(data && data.length)) { - // this doc was changed and should not be visible - // in the listview according to filters applied - // let's remove it manually - this.data = this.data.filter((d) => d.name !== name); - this.render_list(); - return; - } + const names = this.pending_document_refreshes + .filter((d) => d.doctype === this.doctype) + .map((d) => d.name); + this.pending_document_refreshes = this.pending_document_refreshes.filter( + (d) => names.indexOf(d.name) === -1 + ); - const datum = data[0]; - const index = this.data.findIndex((d) => d.name === datum.name); + if (!names.length) return; + + // filters to get only the doc with this name + const call_args = this.get_call_args(); + call_args.args.filters.push([this.doctype, "name", "in", names]); + call_args.args.start = 0; + + frappe.call(call_args).then(({ message }) => { + if (!message) return; + const data = frappe.utils.dict(message.keys, message.values); + + if (!(data && data.length)) { + // this doc was changed and should not be visible + // in the listview according to filters applied + // let's remove it manually + this.data = this.data.filter((d) => names.indexOf(d.name) === -1); + this.render_list(); + return; + } + + data.forEach((datum) => { + const index = this.data.findIndex((doc) => doc.name === datum.name); if (index === -1) { // append new data @@ -1363,31 +1382,31 @@ frappe.views.ListView = class ListView extends frappe.views.BaseList { // update this data in place this.data[index] = datum; } - - this.data.sort((a, b) => { - const a_value = a[this.sort_by] || ""; - const b_value = b[this.sort_by] || ""; - - let return_value = 0; - if (a_value > b_value) { - return_value = 1; - } - - if (b_value > a_value) { - return_value = -1; - } - - if (this.sort_order === "desc") { - return_value = -return_value; - } - return return_value; - }); - this.toggle_result_area(); - this.render_list(); - if (this.$checks && this.$checks.length) { - this.set_rows_as_checked(); - } }); + + this.data.sort((a, b) => { + const a_value = a[this.sort_by] || ""; + const b_value = b[this.sort_by] || ""; + + let return_value = 0; + if (a_value > b_value) { + return_value = 1; + } + + if (b_value > a_value) { + return_value = -1; + } + + if (this.sort_order === "desc") { + return_value = -return_value; + } + return return_value; + }); + if (this.$checks && this.$checks.length) { + this.set_rows_as_checked(); + } + this.toggle_result_area(); + this.render_list(); }); } From 9d236fc2cc8e69e1002dc44c06b1b0193dac675a Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 31 Jan 2023 15:05:23 +0530 Subject: [PATCH 03/25] fix: handle missing is_virtual column via is_virtual_doctype --- frappe/model/db_query.py | 2 +- frappe/model/utils/__init__.py | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/frappe/model/db_query.py b/frappe/model/db_query.py index 6984146d23..af32df8c87 100644 --- a/frappe/model/db_query.py +++ b/frappe/model/db_query.py @@ -165,7 +165,7 @@ class DatabaseQuery: self.user_settings = json.loads(user_settings) if is_virtual_doctype(self.doctype): - from frappe.model.virtual_doctype import get_controller + from frappe.model.base_document import get_controller controller = get_controller(self.doctype) self.parse_args() diff --git a/frappe/model/utils/__init__.py b/frappe/model/utils/__init__.py index bf6804ad05..2935872fc7 100644 --- a/frappe/model/utils/__init__.py +++ b/frappe/model/utils/__init__.py @@ -129,5 +129,7 @@ def get_fetch_values(doctype, fieldname, value): @site_cache() -def is_virtual_doctype(doctype): - return frappe.db.get_value("DocType", doctype, "is_virtual") +def is_virtual_doctype(doctype: str): + if frappe.db.has_column("DocType", "is_virtual"): + return frappe.db.get_value("DocType", doctype, "is_virtual") + return False From dc940bac1d345c5448bc2077716fa3d41ca353b7 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 31 Jan 2023 17:07:04 +0530 Subject: [PATCH 04/25] fix: Pass all DatabaseQuery.execute params to virtual doctype's get_list Give parsed args higher priority in kwargs resolution --- frappe/model/db_query.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/frappe/model/db_query.py b/frappe/model/db_query.py index af32df8c87..23d1aac967 100644 --- a/frappe/model/db_query.py +++ b/frappe/model/db_query.py @@ -169,7 +169,15 @@ class DatabaseQuery: controller = get_controller(self.doctype) self.parse_args() - return controller.get_list(self.__dict__) + kwargs = { + "as_list": as_list, + "with_comment_count": with_comment_count, + "save_user_settings": save_user_settings, + "save_user_settings_fields": save_user_settings_fields, + "pluck": pluck, + "parent_doctype": parent_doctype, + } | self.__dict__ + return controller.get_list(kwargs) self.columns = self.get_table_columns() From ccb7c8cd80dcac2542b2b70a49052008291b0a35 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Wed, 1 Feb 2023 17:05:01 +0530 Subject: [PATCH 05/25] fix(desk): Filter out other doctypes on list_update event --- frappe/public/js/frappe/list/list_view.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/frappe/public/js/frappe/list/list_view.js b/frappe/public/js/frappe/list/list_view.js index 6716999dcb..c80f64e6d9 100644 --- a/frappe/public/js/frappe/list/list_view.js +++ b/frappe/public/js/frappe/list/list_view.js @@ -1330,6 +1330,10 @@ frappe.views.ListView = class ListView extends frappe.views.BaseList { } frappe.socketio.doctype_subscribe(this.doctype); frappe.realtime.on("list_update", (data) => { + if (data?.doctype !== this.doctype) { + return; + } + if (!frappe.get_doc(data?.doctype, data?.name)?.__unsaved) { frappe.model.remove_from_locals(data.doctype, data.name); } @@ -1345,9 +1349,7 @@ frappe.views.ListView = class ListView extends frappe.views.BaseList { process_document_refreshes() { if (!this.pending_document_refreshes.length) return; - const names = this.pending_document_refreshes - .filter((d) => d.doctype === this.doctype) - .map((d) => d.name); + const names = this.pending_document_refreshes.map((d) => d.name); this.pending_document_refreshes = this.pending_document_refreshes.filter( (d) => names.indexOf(d.name) === -1 ); From f8d7151e199a486465c771e875fe48b80846ba0d Mon Sep 17 00:00:00 2001 From: phot0n Date: Wed, 1 Feb 2023 17:08:25 +0530 Subject: [PATCH 06/25] fix: use sender from formatted email body for email queue --- frappe/email/doctype/email_queue/email_queue.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/email/doctype/email_queue/email_queue.py b/frappe/email/doctype/email_queue/email_queue.py index 56f7f6f5ea..41740281a8 100644 --- a/frappe/email/doctype/email_queue/email_queue.py +++ b/frappe/email/doctype/email_queue/email_queue.py @@ -39,7 +39,7 @@ class EmailQueue(Document): def set_recipients(self, recipients): self.set("recipients", []) for r in recipients: - self.append("recipients", {"recipient": r, "status": "Not Sent"}) + self.append("recipients", {"recipient": r.strip(), "status": "Not Sent"}) def on_trash(self): self.prevent_email_queue_delete() @@ -711,7 +711,7 @@ class QueueBuilder: "attachments": json.dumps(self.get_attachments()), "message_id": get_string_between("<", mail.msg_root["Message-Id"], ">"), "message": mail_to_string, - "sender": self.sender, + "sender": mail.sender, "reference_doctype": self.reference_doctype, "reference_name": self.reference_name, "add_unsubscribe_link": self._add_unsubscribe_link, From cc7141edfe74eaac316723b4f137aa5cfe30099b Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Wed, 1 Feb 2023 19:09:01 +0530 Subject: [PATCH 07/25] fix: Use debounce to process_document_refreshes Co-authored-by: Suraj Shetty <13928957+surajshetty3416@users.noreply.github.com> --- frappe/public/js/frappe/list/list_view.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/frappe/public/js/frappe/list/list_view.js b/frappe/public/js/frappe/list/list_view.js index c80f64e6d9..4e74710edf 100644 --- a/frappe/public/js/frappe/list/list_view.js +++ b/frappe/public/js/frappe/list/list_view.js @@ -1321,9 +1321,6 @@ frappe.views.ListView = class ListView extends frappe.views.BaseList { setup_realtime_updates() { this.pending_document_refreshes = []; - setInterval(() => { - this.process_document_refreshes(); - }, 1000); if (this.list_view_settings && this.list_view_settings.disable_auto_refresh) { return; @@ -1343,6 +1340,7 @@ frappe.views.ListView = class ListView extends frappe.views.BaseList { } this.pending_document_refreshes.push(data); + frappe.utils.debounce(this.process_document_refreshes.bind(this), 1000)(); }); } From 5236aeb4b0baa4436cea47a15a0616eb9eb16cf2 Mon Sep 17 00:00:00 2001 From: phot0n Date: Wed, 1 Feb 2023 20:12:02 +0530 Subject: [PATCH 08/25] test: email queue sender with always_use_account_name_as_sender_name and always_use_account_email_id_as_sender --- frappe/tests/test_email.py | 40 ++++++++++++++++++++++++++++++-------- 1 file changed, 32 insertions(+), 8 deletions(-) diff --git a/frappe/tests/test_email.py b/frappe/tests/test_email.py index de0fe00012..65910628db 100644 --- a/frappe/tests/test_email.py +++ b/frappe/tests/test_email.py @@ -3,9 +3,11 @@ import email import re +from unittest.mock import patch import frappe from frappe.email.doctype.email_account.test_email_account import TestEmailAccount +from frappe.email.doctype.email_queue.email_queue import QueueBuilder from frappe.tests.utils import FrappeTestCase test_dependencies = ["Email Account"] @@ -228,8 +230,37 @@ class TestEmail(FrappeTestCase): self.assertTrue("test1@example.com" in queue_recipients) self.assertEqual(len(queue_recipients), 2) + def test_sender(self): + def _patched_assertion(email_account, assertion): + with patch.object(QueueBuilder, "get_outgoing_email_account", return_value=email_account): + frappe.sendmail( + recipients=["test1@example.com"], + sender="admin@example.com", + subject="Test Email Queue", + message="This mail is queued!", + now=True, + ) + email_queue_sender = frappe.db.get_value("Email Queue", {"status": "Sent"}, "sender") + self.assertEqual(email_queue_sender, assertion) + + email_account = frappe.get_doc("Email Account", "_Test Email Account 1") + email_account.default_outgoing = 1 + + email_account.always_use_account_name_as_sender_name = 0 + email_account.always_use_account_email_id_as_sender = 0 + _patched_assertion(email_account, "admin@example.com") + + email_account.always_use_account_name_as_sender_name = 1 + _patched_assertion(email_account, "_Test Email Account 1 ") + + email_account.always_use_account_name_as_sender_name = 0 + email_account.always_use_account_email_id_as_sender = 1 + _patched_assertion(email_account, '"admin@example.com" ') + + email_account.always_use_account_name_as_sender_name = 1 + _patched_assertion(email_account, "_Test Email Account 1 ") + def test_unsubscribe(self): - from frappe.email.doctype.email_queue.email_queue import QueueBuilder from frappe.email.queue import unsubscribe unsubscribe(doctype="User", name="Administrator", email="test@example.com") @@ -322,10 +353,3 @@ class TestVerifiedRequests(FrappeTestCase): set_request(method="GET", path="?" + signed_url) self.assertTrue(verify_request()) frappe.local.request = None - - -if __name__ == "__main__": - import unittest - - frappe.connect() - unittest.main() From a0d1d1bd0ed80212fb1660d6e99c9bf84b26fe24 Mon Sep 17 00:00:00 2001 From: phot0n Date: Wed, 1 Feb 2023 21:37:23 +0530 Subject: [PATCH 09/25] test: fix test_unsubscribe --- frappe/email/queue.py | 2 +- frappe/tests/test_email.py | 17 ++++++----------- 2 files changed, 7 insertions(+), 12 deletions(-) diff --git a/frappe/email/queue.py b/frappe/email/queue.py index c990226682..ea975b532b 100755 --- a/frappe/email/queue.py +++ b/frappe/email/queue.py @@ -99,7 +99,7 @@ def get_unsubcribed_url( @frappe.whitelist(allow_guest=True) def unsubscribe(doctype, name, email): # unsubsribe from comments and communications - if not verify_request(): + if not frappe.flags.in_test and not verify_request(): return try: diff --git a/frappe/tests/test_email.py b/frappe/tests/test_email.py index 65910628db..84785b70f9 100644 --- a/frappe/tests/test_email.py +++ b/frappe/tests/test_email.py @@ -264,7 +264,6 @@ class TestEmail(FrappeTestCase): from frappe.email.queue import unsubscribe unsubscribe(doctype="User", name="Administrator", email="test@example.com") - self.assertTrue( frappe.db.get_value( "Email Unsubscribe", @@ -272,10 +271,6 @@ class TestEmail(FrappeTestCase): ) ) - before = frappe.db.sql("""select count(name) from `tabEmail Queue` where status='Not Sent'""")[ - 0 - ][0] - builder = QueueBuilder( recipients=["test@example.com", "test1@example.com"], sender="admin@example.com", @@ -285,13 +280,11 @@ class TestEmail(FrappeTestCase): message="This is mail is queued!", unsubscribe_message="Unsubscribe", ) - builder.process() - # this is sent async (?) - email_queue = frappe.db.sql( - """select name from `tabEmail Queue` where status='Not Sent'""", as_dict=1 - ) - self.assertEqual(len(email_queue), before + 1) + # don't send right now + builder.process() + + email_queue = frappe.db.get_value("Email Queue", {"status": "Not Sent"}) queue_recipients = [ r.recipient for r in frappe.db.sql( @@ -303,6 +296,8 @@ class TestEmail(FrappeTestCase): self.assertFalse("test@example.com" in queue_recipients) self.assertTrue("test1@example.com" in queue_recipients) self.assertEqual(len(queue_recipients), 1) + + frappe.get_doc("Email Queue", email_queue).send() self.assertTrue("Unsubscribe" in frappe.safe_decode(frappe.flags.sent_mail)) def test_image_parsing(self): From dad0e5cbba8224f4c3b6d215685f1d55bc9f05b1 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Wed, 1 Feb 2023 22:37:43 +0530 Subject: [PATCH 10/25] fix: drop table if exists for action and links closes https://github.com/frappe/frappe/issues/19712 --- frappe/database/mariadb/framework_mariadb.sql | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/frappe/database/mariadb/framework_mariadb.sql b/frappe/database/mariadb/framework_mariadb.sql index efeeaaf935..9507a48b91 100644 --- a/frappe/database/mariadb/framework_mariadb.sql +++ b/frappe/database/mariadb/framework_mariadb.sql @@ -116,6 +116,7 @@ CREATE TABLE `tabDocPerm` ( -- Table structure for table `tabDocType Action` -- +DROP TABLE IF EXISTS `tabDocType Action`; CREATE TABLE `tabDocType Action` ( `name` varchar(140) COLLATE utf8mb4_unicode_ci NOT NULL, `creation` datetime(6) DEFAULT NULL, @@ -137,9 +138,10 @@ CREATE TABLE `tabDocType Action` ( ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci ROW_FORMAT=DYNAMIC; -- --- Table structure for table `tabDocType Action` +-- Table structure for table `tabDocType Link` -- +DROP TABLE IF EXISTS `tabDocType Link`; CREATE TABLE `tabDocType Link` ( `name` varchar(140) COLLATE utf8mb4_unicode_ci NOT NULL, `creation` datetime(6) DEFAULT NULL, From d50f6fa7b47f0d5a72828242b8ee0e768793d79e Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 2 Feb 2023 13:42:29 +0530 Subject: [PATCH 11/25] test: cleanup test_create_virtual_doctype --- frappe/core/doctype/doctype/test_doctype.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/frappe/core/doctype/doctype/test_doctype.py b/frappe/core/doctype/doctype/test_doctype.py index e8226d4f9d..25dcc03ce9 100644 --- a/frappe/core/doctype/doctype/test_doctype.py +++ b/frappe/core/doctype/doctype/test_doctype.py @@ -552,13 +552,14 @@ class TestDocType(FrappeTestCase): self.assertRaises(InvalidFieldNameError, validate_links_table_fieldnames, doc) def test_create_virtual_doctype(self): - """Test virtual DOcTYpe.""" + """Test virtual DocType.""" virtual_doc = new_doctype("Test Virtual Doctype") virtual_doc.is_virtual = 1 - virtual_doc.insert() - virtual_doc.save() + virtual_doc.insert(ignore_if_duplicate=True) + virtual_doc.reload() doc = frappe.get_doc("DocType", "Test Virtual Doctype") + self.assertDictEqual(doc.as_dict(), virtual_doc.as_dict()) self.assertEqual(doc.is_virtual, 1) self.assertFalse(frappe.db.table_exists("Test Virtual Doctype")) From 5d3453eeb9eaacaa761ccfa26af68adea0a99d88 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 2 Feb 2023 13:43:31 +0530 Subject: [PATCH 12/25] refactor: Re-use DefaultOrderBy value as global constant --- frappe/database/database.py | 7 ++++--- frappe/database/query.py | 4 ++-- frappe/database/utils.py | 2 +- frappe/model/db_query.py | 6 +++--- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/frappe/database/database.py b/frappe/database/database.py index fe258be8d7..5fd379b9c5 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -20,6 +20,7 @@ import frappe.defaults import frappe.model.meta from frappe import _ from frappe.database.utils import ( + DefaultOrderBy, EmptyQueryValues, FallBackDateTimeStr, LazyMogrify, @@ -422,7 +423,7 @@ class Database: ignore=None, as_dict=False, debug=False, - order_by="KEEP_DEFAULT_ORDERING", + order_by=DefaultOrderBy, cache=False, for_update=False, *, @@ -492,7 +493,7 @@ class Database: ignore=None, as_dict=False, debug=False, - order_by="KEEP_DEFAULT_ORDERING", + order_by=DefaultOrderBy, update=None, cache=False, for_update=False, @@ -551,7 +552,7 @@ class Database: if (filters is not None) and (filters != doctype or doctype == "DocType"): try: if order_by: - order_by = "modified" if order_by == "KEEP_DEFAULT_ORDERING" else order_by + order_by = "modified" if order_by == DefaultOrderBy else order_by out = self._get_values_from_table( fields=fields, filters=filters, diff --git a/frappe/database/query.py b/frappe/database/query.py index 10423f9ca4..3bf6824ab4 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -10,7 +10,7 @@ from pypika.queries import QueryBuilder, Table import frappe from frappe import _ from frappe.database.operator_map import OPERATOR_MAP -from frappe.database.utils import get_doctype_name +from frappe.database.utils import DefaultOrderBy, get_doctype_name from frappe.query_builder import Criterion, Field, Order, functions from frappe.query_builder.functions import Function, SqlFunctions from frappe.query_builder.utils import PseudoColumnMapper @@ -314,7 +314,7 @@ class Engine: return _fields def apply_order_by(self, order_by: str | None): - if not order_by or order_by == "KEEP_DEFAULT_ORDERING": + if not order_by or order_by == DefaultOrderBy: return for declaration in order_by.split(","): if _order_by := declaration.strip(): diff --git a/frappe/database/utils.py b/frappe/database/utils.py index 304fd72be6..d1030ca6d7 100644 --- a/frappe/database/utils.py +++ b/frappe/database/utils.py @@ -17,7 +17,7 @@ QueryValues = tuple | list | dict | NoneType EmptyQueryValues = object() FallBackDateTimeStr = "0001-01-01 00:00:00.000000" - +DefaultOrderBy = "KEEP_DEFAULT_ORDERING" NestedSetHierarchy = ( "ancestors of", "descendants of", diff --git a/frappe/model/db_query.py b/frappe/model/db_query.py index 23d1aac967..c9789ae9bc 100644 --- a/frappe/model/db_query.py +++ b/frappe/model/db_query.py @@ -13,7 +13,7 @@ import frappe.permissions import frappe.share from frappe import _ from frappe.core.doctype.server_script.server_script_utils import get_server_script_map -from frappe.database.utils import FallBackDateTimeStr, NestedSetHierarchy +from frappe.database.utils import DefaultOrderBy, FallBackDateTimeStr, NestedSetHierarchy from frappe.model import optional_fields from frappe.model.meta import get_table_columns from frappe.model.utils import is_virtual_doctype @@ -73,7 +73,7 @@ class DatabaseQuery: or_filters=None, docstatus=None, group_by=None, - order_by="KEEP_DEFAULT_ORDERING", + order_by=DefaultOrderBy, limit_start=False, limit_page_length=None, as_list=False, @@ -888,7 +888,7 @@ class DatabaseQuery: def set_order_by(self, args): meta = frappe.get_meta(self.doctype) - if self.order_by and self.order_by != "KEEP_DEFAULT_ORDERING": + if self.order_by and self.order_by != DefaultOrderBy: args.order_by = self.order_by else: args.order_by = "" From c4061904da70b1daeb6b5d5df61d3c0878c6ec33 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 2 Feb 2023 13:45:17 +0530 Subject: [PATCH 13/25] test: Split DBQuery & ReportView API tests into separate cases --- frappe/tests/test_db_query.py | 177 +++++++++++++++++----------------- 1 file changed, 90 insertions(+), 87 deletions(-) diff --git a/frappe/tests/test_db_query.py b/frappe/tests/test_db_query.py index 1fa751662c..242b7eb02c 100644 --- a/frappe/tests/test_db_query.py +++ b/frappe/tests/test_db_query.py @@ -1,10 +1,13 @@ # Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors # License: MIT. See LICENSE import datetime +from unittest.mock import MagicMock, patch import frappe +from frappe.core.doctype.doctype.test_doctype import new_doctype from frappe.core.page.permission_manager.permission_manager import add, reset, update from frappe.custom.doctype.property_setter.property_setter import make_property_setter +from frappe.database.utils import DefaultOrderBy from frappe.desk.reportview import get_filters_cond from frappe.handler import execute_cmd from frappe.model.db_query import DatabaseQuery @@ -16,7 +19,7 @@ from frappe.utils.testutils import add_custom_field, clear_custom_fields test_dependencies = ["User", "Blog Post", "Blog Category", "Blogger"] -class TestReportview(FrappeTestCase): +class TestDBQuery(FrappeTestCase): def setUp(self): frappe.set_user("Administrator") @@ -726,89 +729,6 @@ class TestReportview(FrappeTestCase): self.assertEqual(users_unedited[0].modified, users_unedited[0].creation) self.assertNotEqual(users_edited[0].modified, users_edited[0].creation) - def test_reportview_get(self): - user = frappe.get_doc("User", "test@example.com") - add_child_table_to_blog_post() - - user_roles = frappe.get_roles() - user.remove_roles(*user_roles) - user.add_roles("Blogger") - - make_property_setter("Blog Post", "published", "permlevel", 1, "Int") - reset("Blog Post") - add("Blog Post", "Website Manager", 1) - update("Blog Post", "Website Manager", 1, "write", 1) - - frappe.set_user(user.name) - - frappe.local.request = frappe._dict() - frappe.local.request.method = "POST" - - frappe.local.form_dict = frappe._dict( - { - "doctype": "Blog Post", - "fields": ["published", "title", "`tabTest Child`.`test_field`"], - } - ) - - # even if * is passed, fields which are not accessible should be filtered out - response = execute_cmd("frappe.desk.reportview.get") - self.assertListEqual(response["keys"], ["title"]) - frappe.local.form_dict = frappe._dict( - { - "doctype": "Blog Post", - "fields": ["*"], - } - ) - - response = execute_cmd("frappe.desk.reportview.get") - self.assertNotIn("published", response["keys"]) - - frappe.set_user("Administrator") - user.add_roles("Website Manager") - frappe.set_user(user.name) - - frappe.set_user("Administrator") - - # Admin should be able to see access all fields - frappe.local.form_dict = frappe._dict( - { - "doctype": "Blog Post", - "fields": ["published", "title", "`tabTest Child`.`test_field`"], - } - ) - - response = execute_cmd("frappe.desk.reportview.get") - self.assertListEqual(response["keys"], ["published", "title", "test_field"]) - - # reset user roles - user.remove_roles("Blogger", "Website Manager") - user.add_roles(*user_roles) - - def test_reportview_get_aggregation(self): - # test aggregation based on child table field - frappe.local.form_dict = frappe._dict( - { - "doctype": "DocType", - "fields": """["`tabDocField`.`label` as field_label","`tabDocField`.`name` as field_name"]""", - "filters": "[]", - "order_by": "_aggregate_column desc", - "start": 0, - "page_length": 20, - "view": "Report", - "with_comment_count": 0, - "group_by": "field_label, field_name", - "aggregate_on_field": "columns", - "aggregate_on_doctype": "DocField", - "aggregate_function": "sum", - } - ) - - response = execute_cmd("frappe.desk.reportview.get") - self.assertListEqual( - response["keys"], ["field_label", "field_name", "_aggregate_column", "columns"] - ) - def test_cast_name(self): from frappe.core.doctype.doctype.test_doctype import new_doctype @@ -916,6 +836,91 @@ class TestReportview(FrappeTestCase): self.assertIn("ifnull", frappe.get_all("User", {"name": ("not in", [""])}, run=0)) +class TestReportView(FrappeTestCase): + def test_reportview_get(self): + user = frappe.get_doc("User", "test@example.com") + add_child_table_to_blog_post() + + user_roles = frappe.get_roles() + user.remove_roles(*user_roles) + user.add_roles("Blogger") + + make_property_setter("Blog Post", "published", "permlevel", 1, "Int") + reset("Blog Post") + add("Blog Post", "Website Manager", 1) + update("Blog Post", "Website Manager", 1, "write", 1) + + frappe.set_user(user.name) + + frappe.local.request = frappe._dict() + frappe.local.request.method = "POST" + + frappe.local.form_dict = frappe._dict( + { + "doctype": "Blog Post", + "fields": ["published", "title", "`tabTest Child`.`test_field`"], + } + ) + + # even if * is passed, fields which are not accessible should be filtered out + response = execute_cmd("frappe.desk.reportview.get") + self.assertListEqual(response["keys"], ["title"]) + frappe.local.form_dict = frappe._dict( + { + "doctype": "Blog Post", + "fields": ["*"], + } + ) + + response = execute_cmd("frappe.desk.reportview.get") + self.assertNotIn("published", response["keys"]) + + frappe.set_user("Administrator") + user.add_roles("Website Manager") + frappe.set_user(user.name) + + frappe.set_user("Administrator") + + # Admin should be able to see access all fields + frappe.local.form_dict = frappe._dict( + { + "doctype": "Blog Post", + "fields": ["published", "title", "`tabTest Child`.`test_field`"], + } + ) + + response = execute_cmd("frappe.desk.reportview.get") + self.assertListEqual(response["keys"], ["published", "title", "test_field"]) + + # reset user roles + user.remove_roles("Blogger", "Website Manager") + user.add_roles(*user_roles) + + def test_reportview_get_aggregation(self): + # test aggregation based on child table field + frappe.local.form_dict = frappe._dict( + { + "doctype": "DocType", + "fields": """["`tabDocField`.`label` as field_label","`tabDocField`.`name` as field_name"]""", + "filters": "[]", + "order_by": "_aggregate_column desc", + "start": 0, + "page_length": 20, + "view": "Report", + "with_comment_count": 0, + "group_by": "field_label, field_name", + "aggregate_on_field": "columns", + "aggregate_on_doctype": "DocField", + "aggregate_function": "sum", + } + ) + + response = execute_cmd("frappe.desk.reportview.get") + self.assertListEqual( + response["keys"], ["field_label", "field_name", "_aggregate_column", "columns"] + ) + + def add_child_table_to_blog_post(): child_table = frappe.get_doc( { @@ -939,7 +944,7 @@ def create_event(subject="_Test Event", starts_on=None): from frappe.utils import get_datetime - event = frappe.get_doc( + return frappe.get_doc( { "doctype": "Event", "subject": subject, @@ -948,8 +953,6 @@ def create_event(subject="_Test Event", starts_on=None): } ).insert(ignore_permissions=True) - return event - def create_nested_doctype(): if frappe.db.exists("DocType", "Nested DocType"): From fdff6351cda65aad0cef8a3ec08899c327ffe524 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 2 Feb 2023 13:45:35 +0530 Subject: [PATCH 14/25] test: Add test for DatabaseQuery for virtual doctypes --- frappe/tests/test_db_query.py | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/frappe/tests/test_db_query.py b/frappe/tests/test_db_query.py index 242b7eb02c..38bc469388 100644 --- a/frappe/tests/test_db_query.py +++ b/frappe/tests/test_db_query.py @@ -826,6 +826,33 @@ class TestDBQuery(FrappeTestCase): self.assertTrue(dashboard_settings) + def test_virtual_doctype(self): + """Test that virtual doctypes can be queried using get_all""" + + virtual_doctype = new_doctype("Virtual DocType") + virtual_doctype.is_virtual = 1 + virtual_doctype.insert(ignore_if_duplicate=True) + + class VirtualDocType: + @staticmethod + def get_list(args): + ... + + with patch("frappe.controllers", new={frappe.local.site: {"Virtual DocType": VirtualDocType}}): + VirtualDocType.get_list = MagicMock() + + frappe.get_all("Virtual DocType", filters={"name": "test"}, fields=["name"], limit=1) + + call_args = VirtualDocType.get_list.call_args[0][0] + VirtualDocType.get_list.assert_called_once() + self.assertIsInstance(call_args, dict) + self.assertEqual(call_args["doctype"], "Virtual DocType") + self.assertEqual(call_args["filters"], [["Virtual DocType", "name", "=", "test"]]) + self.assertEqual(call_args["fields"], ["name"]) + self.assertEqual(call_args["limit_page_length"], 1) + self.assertEqual(call_args["limit_start"], 0) + self.assertEqual(call_args["order_by"], DefaultOrderBy) + def test_coalesce_with_in_ops(self): self.assertNotIn("ifnull", frappe.get_all("User", {"name": ("in", ["a", "b"])}, run=0)) self.assertIn("ifnull", frappe.get_all("User", {"name": ("in", ["a", None])}, run=0)) From 07529ff1c3ad4aeb354c305483bc293cda2b72ce Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Thu, 2 Feb 2023 17:05:44 +0530 Subject: [PATCH 15/25] fix: Consider parenttype when renaming (#19901) --- frappe/model/rename_doc.py | 6 +++++- frappe/tests/test_rename_doc.py | 27 +++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/frappe/model/rename_doc.py b/frappe/model/rename_doc.py index 68b03dde55..420cbee091 100644 --- a/frappe/model/rename_doc.py +++ b/frappe/model/rename_doc.py @@ -397,7 +397,11 @@ def rename_doctype(doctype: str, old: str, new: str) -> None: def update_child_docs(old: str, new: str, meta: "Meta") -> None: # update "parent" for df in meta.get_table_fields(): - frappe.qb.update(df.options).set("parent", new).where(Field("parent") == old).run() + ( + frappe.qb.update(df.options) + .set("parent", new) + .where((Field("parent") == old) & (Field("parenttype") == meta.name)) + ).run() def update_link_field_values(link_fields: list[dict], old: str, new: str, doctype: str) -> None: diff --git a/frappe/tests/test_rename_doc.py b/frappe/tests/test_rename_doc.py index d85efa79bb..e48f908147 100644 --- a/frappe/tests/test_rename_doc.py +++ b/frappe/tests/test_rename_doc.py @@ -8,6 +8,7 @@ from random import choice, sample from unittest.mock import patch import frappe +from frappe.core.doctype.doctype.test_doctype import new_doctype from frappe.exceptions import DoesNotExistError, ValidationError from frappe.model.base_document import get_controller from frappe.model.rename_doc import ( @@ -271,3 +272,29 @@ class TestRenameDoc(FrappeTestCase): self.assertEqual(doc.name, new_name) self.available_documents.append(new_name) self.available_documents.remove(name) + + def test_parenttype(self): + child = new_doctype(istable=1).insert() + table_field = { + "label": "Test Table", + "fieldname": "test_table", + "fieldtype": "Table", + "options": child.name, + } + + parent_a = new_doctype(fields=[table_field], allow_rename=1, autoname="Prompt").insert() + parent_b = new_doctype(fields=[table_field], allow_rename=1, autoname="Prompt").insert() + + parent_a_instance = frappe.get_doc( + doctype=parent_a.name, test_table=[{"some_fieldname": "x"}], name="XYZ" + ).insert() + + parent_b_instance = frappe.get_doc( + doctype=parent_b.name, test_table=[{"some_fieldname": "x"}], name="XYZ" + ).insert() + + parent_b_instance.rename("ABC") + parent_a_instance.reload() + + self.assertEqual(len(parent_a_instance.test_table), 1) + self.assertEqual(len(parent_b_instance.test_table), 1) From 3f1deeba67cbf2f4adcb9dea32f2c4f8262cfb6e Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Thu, 2 Feb 2023 22:53:24 +0530 Subject: [PATCH 16/25] fix: can't sign out due to missing roles (#19905) --- frappe/__init__.py | 2 +- frappe/permissions.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index e32b04dccb..f7208035e5 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -570,7 +570,7 @@ def get_user(): def get_roles(username=None) -> list[str]: """Returns roles of current user.""" - if not local.session: + if not local.session or not local.session.user: return ["Guest"] import frappe.permissions diff --git a/frappe/permissions.py b/frappe/permissions.py index 2bee75d50c..91517e774f 100644 --- a/frappe/permissions.py +++ b/frappe/permissions.py @@ -413,7 +413,7 @@ def get_roles(user=None, with_standard=True): if not user: user = frappe.session.user - if user == "Guest": + if user == "Guest" or not user: return ["Guest"] def get(): From 47edc6317007203d4bf1b4422e34c3fb7d586fa8 Mon Sep 17 00:00:00 2001 From: Ritwik Puri Date: Fri, 3 Feb 2023 11:47:38 +0530 Subject: [PATCH 17/25] fix: support for different delimiter for timeline email linking (#19751) --- .../doctype/communication/communication.py | 28 +++++++++++-------- .../communication/test_communication.py | 23 +++++++-------- frappe/desk/form/load.py | 2 +- 3 files changed, 29 insertions(+), 24 deletions(-) diff --git a/frappe/core/doctype/communication/communication.py b/frappe/core/doctype/communication/communication.py index 9756bc73c0..6b948947a8 100644 --- a/frappe/core/doctype/communication/communication.py +++ b/frappe/core/doctype/communication/communication.py @@ -487,28 +487,32 @@ def parse_email(communication, email_strings): """ Parse email to add timeline links. When automatic email linking is enabled, an email from email_strings can contain - a doctype and docname ie in the format `admin+doctype+docname@example.com`, + a doctype and docname ie in the format `admin+doctype+docname@example.com` or `admin+doctype=docname@example.com`, the email is parsed and doctype and docname is extracted and timeline link is added. """ - if not frappe.get_all("Email Account", filters={"enable_automatic_linking": 1}): + if not frappe.db.get_value("Email Account", filters={"enable_automatic_linking": 1}): return - delimiter = "+" - for email_string in email_strings: if email_string: for email in email_string.split(","): - if delimiter in email: - email = email.split("@", 1)[0] - email_local_parts = email.split(delimiter) - if not len(email_local_parts) == 3: - continue - + email_username = email.split("@", 1)[0] + email_local_parts = email_username.split("+") + docname = doctype = None + if len(email_local_parts) == 3: doctype = unquote(email_local_parts[1]) docname = unquote(email_local_parts[2]) - if doctype and docname and frappe.db.exists(doctype, docname): - communication.add_link(doctype, docname) + elif len(email_local_parts) == 2: + document_parts = email_local_parts[1].split("=", 1) + if len(document_parts) != 2: + continue + + doctype = unquote(document_parts[0]) + docname = unquote(document_parts[1]) + + if doctype and docname and frappe.db.get_value(doctype, docname, ignore=True): + communication.add_link(doctype, docname) def get_email_without_link(email): diff --git a/frappe/core/doctype/communication/test_communication.py b/frappe/core/doctype/communication/test_communication.py index 5b208eaeb7..04e57f10cf 100644 --- a/frappe/core/doctype/communication/test_communication.py +++ b/frappe/core/doctype/communication/test_communication.py @@ -219,17 +219,17 @@ class TestCommunication(FrappeTestCase): self.assertIn(comm_note_2.name, data) def test_link_in_email(self): - frappe.delete_doc_if_exists("Note", "test document link in email") - create_email_account() - note = frappe.get_doc( - { - "doctype": "Note", - "title": "test document link in email", - "content": "test document link in email", - } - ).insert(ignore_permissions=True) + notes = {} + for i in range(2): + frappe.delete_doc_if_exists("Note", f"test document link in email {i}") + notes[i] = frappe.get_doc( + { + "doctype": "Note", + "title": f"test document link in email {i}", + } + ).insert(ignore_permissions=True) comm = frappe.get_doc( { @@ -237,14 +237,15 @@ class TestCommunication(FrappeTestCase): "communication_medium": "Email", "subject": "Document Link in Email", "sender": "comm_sender@example.com", - "recipients": f'comm_recipient+{quote("Note")}+{quote(note.name)}@example.com', + "recipients": f'comm_recipient+{quote("Note")}+{quote(notes[0].name)}@example.com,comm_recipient+{quote("Note")}={quote(notes[1].name)}@example.com', } ).insert(ignore_permissions=True) doc_links = [ (timeline_link.link_doctype, timeline_link.link_name) for timeline_link in comm.timeline_links ] - self.assertIn(("Note", note.name), doc_links) + self.assertIn(("Note", notes[0].name), doc_links) + self.assertIn(("Note", notes[1].name), doc_links) def test_parse_emails(self): emails = get_emails( diff --git a/frappe/desk/form/load.py b/frappe/desk/form/load.py index 3627f48109..42109f8863 100644 --- a/frappe/desk/form/load.py +++ b/frappe/desk/form/load.py @@ -403,7 +403,7 @@ def get_document_email(doctype, name): return None email = email.split("@") - return f"{email[0]}+{quote(doctype)}+{quote(cstr(name))}@{email[1]}" + return f"{email[0]}+{quote(doctype)}={quote(cstr(name))}@{email[1]}" def get_automatic_email_link(): From 5348dd1f2931abefa0fb65b1e63b8b609b32fd4f Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Fri, 3 Feb 2023 11:11:39 +0530 Subject: [PATCH 18/25] fix: Migration fails while inserting docfield When migrating base doctypes we need to insert docfield which triggers document naming rule code and document naming rule doesn't yet exists cause that's what we are trying to migrate. Fix: skip naming rule on bootstrapped doctypes. --- frappe/model/naming.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/frappe/model/naming.py b/frappe/model/naming.py index 29831451b0..d3b6fc7293 100644 --- a/frappe/model/naming.py +++ b/frappe/model/naming.py @@ -232,7 +232,11 @@ def set_naming_from_document_naming_rule(doc): """ Evaluate rules based on "Document Naming Series" doctype """ - if doc.doctype in log_types: + from frappe.model.base_document import DOCTYPES_FOR_DOCTYPE + + IGNORED_DOCTYPES = {*log_types, *DOCTYPES_FOR_DOCTYPE, "DefaultValue", "Patch Log"} + + if doc.doctype in IGNORED_DOCTYPES: return document_naming_rules = frappe.cache_manager.get_doctype_map( From 8d133ce32accfbc600d312484fbeae80029a0e2a Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Fri, 3 Feb 2023 12:40:24 +0530 Subject: [PATCH 19/25] fix: Ignore route conflict validations during migrate --- frappe/desk/utils.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/frappe/desk/utils.py b/frappe/desk/utils.py index 428ed95c02..77edf88d7a 100644 --- a/frappe/desk/utils.py +++ b/frappe/desk/utils.py @@ -9,16 +9,14 @@ def validate_route_conflict(doctype, name): Raises exception if name clashes with routes from other documents for /app routing """ + if frappe.flags.in_migrate: + return + all_names = [] for _doctype in ["Page", "Workspace", "DocType"]: - try: - all_names.extend( - [ - slug(d) for d in frappe.get_all(_doctype, pluck="name") if (doctype != _doctype and d != name) - ] - ) - except frappe.db.TableMissingError: - pass + all_names.extend( + [slug(d) for d in frappe.get_all(_doctype, pluck="name") if (doctype != _doctype and d != name)] + ) if slug(name) in all_names: frappe.msgprint(frappe._("Name already taken, please set a new name")) From 90c4543065f86f11b814d39d6e2c999bf4c7ae5a Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Fri, 3 Feb 2023 12:50:35 +0530 Subject: [PATCH 20/25] fix: Dont use cached controllers during migration --- frappe/model/base_document.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/model/base_document.py b/frappe/model/base_document.py index 2c9963ad23..43a1ac8d13 100644 --- a/frappe/model/base_document.py +++ b/frappe/model/base_document.py @@ -43,7 +43,7 @@ def get_controller(doctype): :param doctype: DocType name as string. """ - if frappe.local.dev_server: + if frappe.local.dev_server or frappe.flags.in_migrate: return import_controller(doctype) site_controllers = frappe.controllers.setdefault(frappe.local.site, {}) @@ -59,7 +59,7 @@ def import_controller(doctype): module_name = "Core" if doctype not in DOCTYPES_FOR_DOCTYPE: - meta = frappe.get_meta(doctype) + meta = frappe.get_meta(doctype, cached=not frappe.flags.in_migrate) if meta.custom: return NestedSet if meta.get("is_tree") else Document From b889bb5b5a41a7c8a49293eb3058159d68743d98 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Fri, 3 Feb 2023 13:06:09 +0530 Subject: [PATCH 21/25] fix: list view setting patch failures - make idempotent - ignore ordering (fails as it tries to query order which might not exist --- ...list_view_setting_to_list_view_settings.py | 39 +++++++++++-------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/frappe/patches/v13_0/rename_list_view_setting_to_list_view_settings.py b/frappe/patches/v13_0/rename_list_view_setting_to_list_view_settings.py index 2147a2da94..c7c8cbc724 100644 --- a/frappe/patches/v13_0/rename_list_view_setting_to_list_view_settings.py +++ b/frappe/patches/v13_0/rename_list_view_setting_to_list_view_settings.py @@ -5,22 +5,27 @@ import frappe def execute(): - if frappe.db.table_exists("List View Setting"): - if not frappe.db.table_exists("List View Settings"): - frappe.reload_doc("desk", "doctype", "List View Settings") + if not frappe.db.table_exists("List View Setting"): + return + if not frappe.db.exists("DocType", "List View Setting"): + return - existing_list_view_settings = frappe.get_all("List View Settings", as_list=True) - for list_view_setting in frappe.get_all( - "List View Setting", - fields=["disable_count", "disable_sidebar_stats", "disable_auto_refresh", "name"], - ): - name = list_view_setting.pop("name") - if name not in [x[0] for x in existing_list_view_settings]: - list_view_setting["doctype"] = "List View Settings" - list_view_settings = frappe.get_doc(list_view_setting) - # setting name here is necessary because autoname is set as prompt - list_view_settings.name = name - list_view_settings.insert() + frappe.reload_doc("desk", "doctype", "List View Settings") - frappe.delete_doc("DocType", "List View Setting", force=True) - frappe.db.commit() + existing_list_view_settings = frappe.get_all( + "List View Settings", as_list=True, order_by="modified" + ) + for list_view_setting in frappe.get_all( + "List View Setting", + fields=["disable_count", "disable_sidebar_stats", "disable_auto_refresh", "name"], + order_by="modified", + ): + name = list_view_setting.pop("name") + if name not in [x[0] for x in existing_list_view_settings]: + list_view_setting["doctype"] = "List View Settings" + list_view_settings = frappe.get_doc(list_view_setting) + # setting name here is necessary because autoname is set as prompt + list_view_settings.name = name + list_view_settings.insert() + + frappe.delete_doc("DocType", "List View Setting", force=True) From 75d092ef7dfcca82b6e2653e5118329acb94c061 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Fri, 3 Feb 2023 15:13:18 +0530 Subject: [PATCH 22/25] Revert "fix: Report sidebar must consider Permission Query" (#19921) --- frappe/boot.py | 26 +++++----------------- frappe/tests/test_boot.py | 46 +-------------------------------------- 2 files changed, 6 insertions(+), 66 deletions(-) diff --git a/frappe/boot.py b/frappe/boot.py index de3753f754..31e101aedc 100644 --- a/frappe/boot.py +++ b/frappe/boot.py @@ -3,16 +3,15 @@ """ bootstrap client session """ + import frappe import frappe.defaults import frappe.desk.desk_page from frappe.core.doctype.navbar_settings.navbar_settings import get_app_logo, get_navbar_settings -from frappe.database.utils import Query from frappe.desk.doctype.route_history.route_history import frequently_visited_links from frappe.desk.form.load import get_meta_bundle from frappe.email.inbox import get_email_accounts from frappe.model.base_document import get_controller -from frappe.model.db_query import DatabaseQuery from frappe.query_builder import DocType from frappe.query_builder.functions import Count from frappe.query_builder.terms import ParameterizedValueWrapper, SubQuery @@ -170,7 +169,6 @@ def get_user_pages_or_reports(parent, cache=False): parentTable = DocType(parent) # get pages or reports set on custom role - # must end in a WHERE clause for `_run_with_permission_query` pages_with_custom_roles = ( frappe.qb.from_(customRole) .from_(hasRole) @@ -184,8 +182,7 @@ def get_user_pages_or_reports(parent, cache=False): & (customRole[parent.lower()].isnotnull()) & (hasRole.role.isin(roles)) ) - ) - pages_with_custom_roles = _run_with_permission_query(pages_with_custom_roles, parent) + ).run(as_dict=True) for p in pages_with_custom_roles: has_role[p.name] = {"modified": p.modified, "title": p.title, "ref_doctype": p.ref_doctype} @@ -196,7 +193,6 @@ def get_user_pages_or_reports(parent, cache=False): .where(customRole[parent.lower()].isnotnull()) ) - # must end in a WHERE clause for `_run_with_permission_query` pages_with_standard_roles = ( frappe.qb.from_(hasRole) .from_(parentTable) @@ -212,7 +208,7 @@ def get_user_pages_or_reports(parent, cache=False): if parent == "Report": pages_with_standard_roles = pages_with_standard_roles.where(report.disabled == 0) - pages_with_standard_roles = _run_with_permission_query(pages_with_standard_roles, parent) + pages_with_standard_roles = pages_with_standard_roles.run(as_dict=True) for p in pages_with_standard_roles: if p.name not in has_role: @@ -226,13 +222,12 @@ def get_user_pages_or_reports(parent, cache=False): # pages with no role are allowed if parent == "Page": - # must end in a WHERE clause for `_run_with_permission_query` + pages_with_no_roles = ( frappe.qb.from_(parentTable) .select(parentTable.name, parentTable.modified, *columns) .where(no_of_roles == 0) - ) - pages_with_no_roles = _run_with_permission_query(pages_with_no_roles, parent) + ).run(as_dict=True) for p in pages_with_no_roles: if p.name not in has_role: @@ -253,17 +248,6 @@ def get_user_pages_or_reports(parent, cache=False): return has_role -def _run_with_permission_query(query: "Query", doctype: str) -> list[dict]: - """ - Adds Permission Query (Server Script) conditions and runs/executes modified query - Note: Works only if 'WHERE' is the last clause in the query - """ - permission_query = DatabaseQuery(doctype, frappe.session.user).get_permission_query_conditions() - if permission_query and frappe.session.user != "Administrator": - return frappe.db.sql(f"{query} AND {permission_query}", as_dict=True) - return query.run(as_dict=True) - - def load_translations(bootinfo): bootinfo["lang"] = frappe.lang bootinfo["__messages"] = get_messages_for_boot() diff --git a/frappe/tests/test_boot.py b/frappe/tests/test_boot.py index 232c379e08..0b688d6aee 100644 --- a/frappe/tests/test_boot.py +++ b/frappe/tests/test_boot.py @@ -1,5 +1,5 @@ import frappe -from frappe.boot import get_unseen_notes, get_user_pages_or_reports +from frappe.boot import get_unseen_notes from frappe.desk.doctype.note.note import mark_as_seen from frappe.tests.utils import FrappeTestCase @@ -26,47 +26,3 @@ class TestBootData(FrappeTestCase): mark_as_seen(note.name) unseen_notes = [d.title for d in get_unseen_notes()] self.assertListEqual(unseen_notes, []) - - def test_get_user_pages_or_reports_with_permission_query(self): - # Create a ToDo custom report with admin user - frappe.set_user("Administrator") - frappe.get_doc( - { - "doctype": "Report", - "ref_doctype": "ToDo", - "report_name": "Test Admin Report", - "report_type": "Report Builder", - "is_standard": "No", - } - ).insert() - - # Add permission query such that each user can only see their own custom reports - frappe.get_doc( - dict( - doctype="Server Script", - name="test_report_permission_query", - script_type="Permission Query", - reference_doctype="Report", - script="""conditions = f"(`tabReport`.is_standard = 'Yes' or `tabReport`.owner = '{frappe.session.user}')" - """, - ) - ).insert() - - # Create a ToDo custom report with test user - frappe.set_user("test@example.com") - frappe.get_doc( - { - "doctype": "Report", - "ref_doctype": "ToDo", - "report_name": "Test User Report", - "report_type": "Report Builder", - "is_standard": "No", - } - ).insert(ignore_permissions=True) - - get_user_pages_or_reports("Report") - allowed_reports = frappe.cache().get_value("has_role:Report", user=frappe.session.user) - - # Test user must not see admin user's report - self.assertNotIn("Test Admin Report", allowed_reports) - self.assertIn("Test User Report", allowed_reports) From 11f7e41994e9d2fd0384337222cb383c408c88ea Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Fri, 3 Feb 2023 13:23:45 +0530 Subject: [PATCH 23/25] fix: Show local variables in tracebacks during migration --- frappe/commands/site.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/frappe/commands/site.py b/frappe/commands/site.py index fbbdde8e03..c78bd4a7c5 100644 --- a/frappe/commands/site.py +++ b/frappe/commands/site.py @@ -592,6 +592,8 @@ def disable_user(context, email): @pass_context def migrate(context, skip_failing=False, skip_search_index=False): "Run patches, sync schema and rebuild files/translations" + from traceback_with_variables import activate_by_import + from frappe.migrate import SiteMigration for site in context.sites: From 95ad5c76965d5294d3be36e140f6ad65fa56ef06 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Fri, 3 Feb 2023 13:01:10 +0530 Subject: [PATCH 24/25] fix: Dont use meta for get_controller --- 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 43a1ac8d13..e3694d1baf 100644 --- a/frappe/model/base_document.py +++ b/frappe/model/base_document.py @@ -59,11 +59,11 @@ def import_controller(doctype): module_name = "Core" if doctype not in DOCTYPES_FOR_DOCTYPE: - meta = frappe.get_meta(doctype, cached=not frappe.flags.in_migrate) - if meta.custom: - return NestedSet if meta.get("is_tree") else Document - - module_name = meta.module + doctype_info = frappe.db.get_value("DocType", doctype, fieldname="*") + if doctype_info: + if doctype_info.custom: + return NestedSet if doctype_info.is_tree else Document + module_name = doctype_info.module module_path = None class_overrides = frappe.get_hooks("override_doctype_class") From 790b09f95fc3844bcc90314b984bf23568b28ff7 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Fri, 3 Feb 2023 22:25:27 +0530 Subject: [PATCH 25/25] feat: Allow clearing access logs --- frappe/core/doctype/access_log/access_log.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/frappe/core/doctype/access_log/access_log.py b/frappe/core/doctype/access_log/access_log.py index ca2909b970..c194f5d603 100644 --- a/frappe/core/doctype/access_log/access_log.py +++ b/frappe/core/doctype/access_log/access_log.py @@ -8,7 +8,13 @@ from frappe.utils import cstr class AccessLog(Document): - pass + @staticmethod + def clear_old_logs(days=30): + from frappe.query_builder import Interval + from frappe.query_builder.functions import Now + + table = frappe.qb.DocType("Access Log") + frappe.db.delete(table, filters=(table.modified < (Now() - Interval(days=days)))) @frappe.whitelist()