From 2a80bb01ac1a41d5e5fcd70c7a37f62c5c93e8a9 Mon Sep 17 00:00:00 2001 From: "Patrick.St" <72972659+pstuhlmueller@users.noreply.github.com> Date: Tue, 21 Feb 2023 16:13:07 +0100 Subject: [PATCH 01/39] fix: Incorrect use of the Walrus operator Incorrect use of the Walrus operator leads to unintended behavior for if-condition: "None" will be appended to cc. --- frappe/core/doctype/communication/mixins.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/core/doctype/communication/mixins.py b/frappe/core/doctype/communication/mixins.py index 7b6427d1c2..7b34208019 100644 --- a/frappe/core/doctype/communication/mixins.py +++ b/frappe/core/doctype/communication/mixins.py @@ -70,7 +70,7 @@ class CommunicationEmailMixin: if include_sender: cc.append(self.sender_mailid) if is_inbound_mail_communcation: - if (doc_owner := self.get_owner()) not in frappe.STANDARD_USERS: + if (doc_owner := self.get_owner()) and (doc_owner not in frappe.STANDARD_USERS): cc.append(doc_owner) cc = set(cc) - {self.sender_mailid} cc.update(self.get_assignees()) From 841557338b69d455a683ce4f02e8c76a12016b49 Mon Sep 17 00:00:00 2001 From: "Patrick.St" <72972659+pstuhlmueller@users.noreply.github.com> Date: Tue, 21 Feb 2023 16:24:02 +0100 Subject: [PATCH 02/39] fix: sending mails to unintended recipients as cc Security vulnerability: Unintentionally, all incoming emails are sent as CC to all users in a ToDo as "allocated_to" with the status "Open" --- frappe/core/doctype/communication/mixins.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/frappe/core/doctype/communication/mixins.py b/frappe/core/doctype/communication/mixins.py index 7b34208019..22b7e8a0fc 100644 --- a/frappe/core/doctype/communication/mixins.py +++ b/frappe/core/doctype/communication/mixins.py @@ -216,7 +216,11 @@ class CommunicationEmailMixin: "reference_name": self.reference_name, "reference_type": self.reference_doctype, } - return ToDo.get_owners(filters) + + if self.reference_doctype == "ToDo" and self.reference_name != None: + return ToDo.get_owners(filters) + else: + return [] @staticmethod def filter_thread_notification_disbled_users(emails): From f089e9108e5d71ab1fc2061ebe77e06f2b743135 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 27 Mar 2023 15:58:43 +0530 Subject: [PATCH 03/39] fix: hard link environment variable (#20467) On docker based deploys symlinking inside volume doesn't work. [skip ci] --- frappe/commands/utils.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/frappe/commands/utils.py b/frappe/commands/utils.py index f41cca3c57..01de67eb05 100644 --- a/frappe/commands/utils.py +++ b/frappe/commands/utils.py @@ -24,7 +24,11 @@ EXTRA_ARGS_CTX = {"ignore_unknown_options": True, "allow_extra_args": True} @click.option("--app", help="Build assets for app") @click.option("--apps", help="Build assets for specific apps") @click.option( - "--hard-link", is_flag=True, default=False, help="Copy the files instead of symlinking" + "--hard-link", + is_flag=True, + default=False, + help="Copy the files instead of symlinking", + envvar="FRAPPE_HARD_LINK_ASSETS", ) @click.option( "--make-copy", From 32dbbb47bfc5fa89647befd1b3cdf21f5c27b9f9 Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Mon, 27 Mar 2023 17:03:20 +0530 Subject: [PATCH 04/39] feat: redis cache decorator (#20452) * feat: redis cache decorator * fix: review changes * fix: remove unintentional changes * fix: remove unintentional changes * refactor: cleanup and simplify code for redis AIs suck * fix: bug * test: redis cache * fix: remove unused import * feat: make redis cache user specific redis cache utils already support this, extending so everyone can use it * feat: support @redis_cache without params * test: flake in request site cache test --------- Co-authored-by: Ankush Menat --- frappe/tests/test_caching.py | 79 +++++++++++++++++++++++++++++++++-- frappe/utils/caching.py | 36 ++++++++++++++++ frappe/utils/redis_wrapper.py | 4 ++ 3 files changed, 115 insertions(+), 4 deletions(-) diff --git a/frappe/tests/test_caching.py b/frappe/tests/test_caching.py index d1de587d0d..4faade331c 100644 --- a/frappe/tests/test_caching.py +++ b/frappe/tests/test_caching.py @@ -4,7 +4,7 @@ from unittest.mock import MagicMock import frappe from frappe.tests.test_api import FrappeAPITestCase from frappe.tests.utils import FrappeTestCase -from frappe.utils.caching import request_cache, site_cache +from frappe.utils.caching import redis_cache, request_cache, site_cache CACHE_TTL = 4 external_service = MagicMock(return_value=30) @@ -82,13 +82,84 @@ class TestSiteCache(FrappeAPITestCase): api_with_ttl = f"{module}.ping_with_ttl" api_without_ttl = f"{module}.ping" - start = time.monotonic() for _ in range(5): self.get(f"/api/method/{api_with_ttl}") self.get(f"/api/method/{api_without_ttl}") - end = time.monotonic() self.assertEqual(register_with_external_service.call_count, 2) - time.sleep(CACHE_TTL - (end - start)) + time.sleep(CACHE_TTL) self.get(f"/api/method/{api_with_ttl}") self.assertEqual(register_with_external_service.call_count, 3) + + +class TestRedisCache(FrappeAPITestCase): + def test_redis_cache(self): + function_call_count = 0 + + @redis_cache(ttl=CACHE_TTL) + def calculate_area(radius: float) -> float: + nonlocal function_call_count + function_call_count += 1 + return 3.14 * radius**2 + + self.assertEqual(calculate_area(10), 314) + self.assertEqual(function_call_count, 1) + self.assertEqual(calculate_area(10), 314) + self.assertEqual(function_call_count, 1) + + time.sleep(CACHE_TTL) + self.assertEqual(calculate_area(10), 314) + self.assertEqual(function_call_count, 2) + + calculate_area.clear_cache() + self.assertEqual(calculate_area(10), 314) + self.assertEqual(function_call_count, 3) + calculate_area.clear_cache() + + def test_redis_cache_without_params(self): + function_call_count = 0 + + @redis_cache + def calculate_area(radius: float) -> float: + nonlocal function_call_count + function_call_count += 1 + return 3.14 * radius**2 + + calculate_area.clear_cache() + self.assertEqual(calculate_area(10), 314) + self.assertEqual(function_call_count, 1) + + calculate_area.clear_cache() + self.assertEqual(calculate_area(10), 314) + self.assertEqual(function_call_count, 2) + + calculate_area.clear_cache() + + def test_redis_cache_diff_args(self): + function_call_count = 0 + + @redis_cache(ttl=CACHE_TTL) + def calculate_area(radius: float) -> float: + nonlocal function_call_count + function_call_count += 1 + return 3.14 * radius**2 + + self.assertEqual(calculate_area(10), 314) + self.assertEqual(function_call_count, 1) + self.assertEqual(calculate_area(100), 31400) + self.assertEqual(function_call_count, 2) + + self.assertEqual(calculate_area(5), 25 * 3.14) + self.assertEqual(function_call_count, 3) + + calculate_area(10) + # from cache now + self.assertEqual(function_call_count, 3) + + calculate_area(radius=10) + # args, kwargs are treated differently + self.assertEqual(function_call_count, 4) + + calculate_area(radius=10) + # kwargs should hit cache too + self.assertEqual(function_call_count, 4) diff --git a/frappe/utils/caching.py b/frappe/utils/caching.py index a2c9496098..007582f25f 100644 --- a/frappe/utils/caching.py +++ b/frappe/utils/caching.py @@ -128,3 +128,39 @@ def site_cache(ttl: int | None = None, maxsize: int | None = None) -> Callable: return time_cache_wrapper(ttl) return time_cache_wrapper + + +def redis_cache(ttl: int | None = 3600, user: str | bool | None = None) -> Callable: + """Decorator to cache method calls and its return values in Redis + + args: + ttl: time to expiry in seconds, defaults to 1 hour + user: `true` should cache be specific to session user. + """ + + def wrapper(func: Callable = None) -> Callable: + + func_key = f"{func.__module__}.{func.__qualname__}" + + def clear_cache(): + frappe.cache().delete_keys(func_key) + + func.clear_cache = clear_cache + func.ttl = ttl if not callable(ttl) else 3600 + + @wraps(func) + def redis_cache_wrapper(*args, **kwargs): + func_call_key = func_key + str(__generate_request_cache_key(args, kwargs)) + if frappe.cache().exists(func_call_key): + return frappe.cache().get_value(func_call_key, user=user) + else: + val = func(*args, **kwargs) + ttl = getattr(func, "ttl", 3600) + frappe.cache().set_value(func_call_key, val, expires_in_sec=ttl, user=user) + return val + + return redis_cache_wrapper + + if callable(ttl): + return wrapper(ttl) + return wrapper diff --git a/frappe/utils/redis_wrapper.py b/frappe/utils/redis_wrapper.py index ea91299cfc..3b335b2c1d 100644 --- a/frappe/utils/redis_wrapper.py +++ b/frappe/utils/redis_wrapper.py @@ -195,6 +195,10 @@ class RedisWrapper(redis.Redis): except redis.exceptions.ConnectionError: return False + def exists(self, *names: str, user=None, shared=None) -> int: + names = [self.make_key(n, user=user, shared=shared) for n in names] + return super().exists(*names) + def hgetall(self, name): value = super().hgetall(self.make_key(name)) return {key: pickle.loads(value) for key, value in value.items()} From 69b22f3af244d52a3f50fb1839ae257b4420e81e Mon Sep 17 00:00:00 2001 From: Rutwik Hiwalkar <50401596+rutwikhdev@users.noreply.github.com> Date: Tue, 28 Mar 2023 00:01:42 +0530 Subject: [PATCH 05/39] chore: add ascending order to querybuilder (#20471) --- frappe/query_builder/builder.py | 1 + 1 file changed, 1 insertion(+) diff --git a/frappe/query_builder/builder.py b/frappe/query_builder/builder.py index 5a7b06221f..535284f34d 100644 --- a/frappe/query_builder/builder.py +++ b/frappe/query_builder/builder.py @@ -12,6 +12,7 @@ from frappe.utils import get_table_name class Base: terms = terms desc = Order.desc + asc = Order.asc Schema = Schema Table = Table From dc4509796d8284803d661125cc1a96bb10300a0b Mon Sep 17 00:00:00 2001 From: Samuel Danieli <23150094+scdanieli@users.noreply.github.com> Date: Tue, 28 Mar 2023 08:17:56 +0200 Subject: [PATCH 06/39] fix: suggested email ids in New Email dialog (#20319) * chore: enhance UX of New Email dialog * do not show contacts without an email * use name as value, y? if several contacts use the same email address, the entry will be displayed several times, but always with the same description, which leads to confusion - using name as value makes the entries distinguishable * chore: ignore semgrep Rewriting the query is not in the scope of this PR. * chore: keep semgrep failing on raw query [skip ci] * fix: use email_id as value * Revert "fix: use email_id as value" This reverts commit e4c44e2094ddb9b525bc34d400642dcee5656096. * chore: comment confusing code --------- Co-authored-by: Raffael Meyer <14891507+barredterra@users.noreply.github.com> Co-authored-by: Ankush Menat --- frappe/email/__init__.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/frappe/email/__init__.py b/frappe/email/__init__.py index 1f6af4a3e7..486db2a784 100644 --- a/frappe/email/__init__.py +++ b/frappe/email/__init__.py @@ -11,7 +11,7 @@ def sendmail_to_system_managers(subject, content): @frappe.whitelist() def get_contact_list(txt, page_length=20) -> list[dict]: - """Returns contacts (from autosuggest)""" + """Return email ids for a multiselect field.""" if cached_contacts := get_cached_contacts(txt): return cached_contacts[:page_length] @@ -19,11 +19,14 @@ def get_contact_list(txt, page_length=20) -> list[dict]: reportview_conditions = build_match_conditions("Contact") match_conditions = f"and {reportview_conditions}" if reportview_conditions else "" + # The multiselect field will store the `label` as the selected value. + # The `value` is just used as a unique key to distinguish between the options. + # https://github.com/frappe/frappe/blob/6c6a89bcdd9454060a1333e23b855d0505c9ebc2/frappe/public/js/frappe/form/controls/autocomplete.js#L29-L35 out = frappe.db.sql( - f"""select email_id as value, + f"""select name as value, email_id as label, concat(first_name, ifnull(concat(' ',last_name), '' )) as description from tabContact - where name like %(txt)s or email_id like %(txt)s + where (name like %(txt)s or email_id like %(txt)s) and email_id != '' {match_conditions} limit %(page_length)s""", {"txt": f"%{txt}%", "page_length": page_length}, From 229dcb3c9123ebaf5417e5959f25f7a21e50231e Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 28 Mar 2023 12:07:13 +0530 Subject: [PATCH 07/39] fix: pin pymysql to avoid breaking behaviour (#20475) ``` File "/home/ankush/benches/develop/apps/frappe/frappe/database/database.py", line 920, in get_default d = self.get_defaults(key, parent) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/ankush/benches/develop/apps/frappe/frappe/database/database.py", line 936, in get_defaults defaults = frappe.defaults.get_defaults_for(parent) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/ankush/benches/develop/apps/frappe/frappe/defaults.py", line 222, in get_defaults_for .run(as_dict=True) ^^^^^^^^^^^^^^^^^ File "/home/ankush/benches/develop/apps/frappe/frappe/query_builder/utils.py", line 85, in execute_query return frappe.db.sql(query, params, *args, **kwargs) # nosemgrep ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/ankush/benches/develop/apps/frappe/frappe/database/database.py", line 264, in sql self.log_query(query, values, debug, explain) File "/home/ankush/benches/develop/apps/frappe/frappe/database/mariadb/database.py", line 203, in log_query self.last_query = query = self._cursor._last_executed ^^^^^^^^^^^^^^^^^^^^^^^^^^^ AttributeError: 'Cursor' object has no attribute '_last_executed'. Did you mean: '_check_executed'? ``` --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 837ea4624a..c984ca7e91 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -16,7 +16,7 @@ dependencies = [ "Jinja2~=3.1.2", "Pillow~=9.3.0", "PyJWT~=2.4.0", - "PyMySQL~=1.0.2", + "PyMySQL==1.0.2", "PyPDF2~=2.1.0", "PyPika~=0.48.9", "PyQRCode~=1.2.1", From 024faff02505cc5764d822ecab718814ea75f242 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 28 Mar 2023 13:04:27 +0530 Subject: [PATCH 08/39] build: bump pymysql (#20478) Actual fix for this bandaid fix: https://github.com/frappe/frappe/pull/20475 Keeping pymysql hard pinned until we have better way to get last full query. --- frappe/database/mariadb/database.py | 2 +- pyproject.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/database/mariadb/database.py b/frappe/database/mariadb/database.py index 7f9846d6c4..43540956e0 100644 --- a/frappe/database/mariadb/database.py +++ b/frappe/database/mariadb/database.py @@ -200,7 +200,7 @@ class MariaDBDatabase(MariaDBConnectionUtil, MariaDBExceptionUtil, Database): return db_size[0].get("database_size") def log_query(self, query, values, debug, explain): - self.last_query = query = self._cursor._last_executed + self.last_query = query = self._cursor._executed self._log_query(query, debug, explain) return self.last_query diff --git a/pyproject.toml b/pyproject.toml index c984ca7e91..48903f3163 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -16,7 +16,7 @@ dependencies = [ "Jinja2~=3.1.2", "Pillow~=9.3.0", "PyJWT~=2.4.0", - "PyMySQL==1.0.2", + "PyMySQL==1.0.3", "PyPDF2~=2.1.0", "PyPika~=0.48.9", "PyQRCode~=1.2.1", From 6bda014406addb89aac3a17c68c997abf01b9db7 Mon Sep 17 00:00:00 2001 From: Devin Slauenwhite Date: Tue, 28 Mar 2023 05:04:45 -0400 Subject: [PATCH 09/39] fix: install cypress plugins in frappe namespace (#20459) --- frappe/commands/utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/commands/utils.py b/frappe/commands/utils.py index 01de67eb05..03374986d4 100644 --- a/frappe/commands/utils.py +++ b/frappe/commands/utils.py @@ -912,7 +912,7 @@ def run_ui_tests( os.chdir(app_base_path) - node_bin = subprocess.getoutput("yarn bin") + node_bin = subprocess.getoutput("(cd ../frappe && yarn bin)") cypress_path = f"{node_bin}/cypress" drag_drop_plugin_path = f"{node_bin}/../@4tw/cypress-drag-drop" real_events_plugin_path = f"{node_bin}/../cypress-real-events" @@ -939,7 +939,7 @@ def run_ui_tests( "@cypress/code-coverage@^3", ] ) - frappe.commands.popen(f"yarn add {packages} --no-lockfile") + frappe.commands.popen(f"(cd ../frappe && yarn add {packages} --no-lockfile)") # run for headless mode run_or_open = "run --browser chrome --record" if headless else "open" From e756d417901b755ff2e65de97dddca06d25ee658 Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Tue, 28 Mar 2023 17:24:20 +0530 Subject: [PATCH 10/39] chore: add insights banner (#20487) * chore: add insights banner * chore: cleanup message and title [skip ci] --------- Co-authored-by: Ankush Menat --- frappe/public/js/frappe/list/list_sidebar.js | 41 ++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/frappe/public/js/frappe/list/list_sidebar.js b/frappe/public/js/frappe/list/list_sidebar.js index f845f3a375..fe5f0b810f 100644 --- a/frappe/public/js/frappe/list/list_sidebar.js +++ b/frappe/public/js/frappe/list/list_sidebar.js @@ -38,6 +38,8 @@ frappe.views.ListSidebar = class ListSidebar { this.reload_stats(); }); } + + this.add_insights_banner(); } setup_views() { @@ -239,4 +241,43 @@ frappe.views.ListSidebar = class ListSidebar { this.sidebar.find(".stat-no-records").remove(); this.get_stats(); } + + add_insights_banner() { + try { + if (this.list_view.view != "Report") { + return; + } + + if (localStorage.getItem("show_insights_banner") == "false") { + return; + } + + if (this.insights_banner) { + this.insights_banner.remove(); + } + + const message = "Get more insights from your data with Frappe Insights."; + const link = "https://frappe.io/s/insights"; + const cta = "Get Frappe Insights"; + + this.insights_banner = $(` +
+
+ ${message} +
+
+ ${cta} -> +
+
+ + + +
+
+ `).appendTo(this.sidebar); + } catch (error) { + console.error(error); + } + } }; From 63338a3806281a2933fac7fb158b730c1c64bca9 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 28 Mar 2023 17:33:05 +0530 Subject: [PATCH 11/39] fix: date field shouldn't be formatted for TZ (#20486) Date fields aren't timezone aware. --- frappe/public/js/frappe/form/formatters.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/public/js/frappe/form/formatters.js b/frappe/public/js/frappe/form/formatters.js index 1d555c3603..14f01b6607 100644 --- a/frappe/public/js/frappe/form/formatters.js +++ b/frappe/public/js/frappe/form/formatters.js @@ -186,7 +186,7 @@ frappe.form.formatters = { return value; } if (value) { - value = frappe.datetime.str_to_user(value); + value = frappe.datetime.str_to_user(value, false, true); // handle invalid date if (value === "Invalid date") { value = null; From 90f8f945b4f830d458ad453f81fc95fa3dfe9f75 Mon Sep 17 00:00:00 2001 From: Marica Date: Tue, 28 Mar 2023 18:13:37 +0530 Subject: [PATCH 12/39] feat: Disable Sharing globally (#20318) * feat: Disable Sharing globally - Checkbox in System Settings - If disabled, avoid share UI render - Share APIs return None (non-obstructing) if share APIs are invoked * feat: Settings checkbox must toggle share permission globally - Treat feature like a perm toggler. Essentially noone is allowed to explicity share anything - Implicit sharing via `ignore_share_permissions` is allowed. Devs can decide where sharing should happen under the hood - UI is made read only and not hidden. Users must see who doc is already shared with - Make sure perm APIs used by share feature return false if sharing is disabled - Rename checkbox to `Disable Document Sharing` * test: (server side) Impact of disabling sharing on APIs - Also, fix missed system setting rename in `assign_to` * fix: Inform assigner if assignee lacks perms and sharing is disabled - misc: readable conditions * fix: throw instead of msgprint * fix: Typo and appropriate message for `throw` --------- Co-authored-by: Ankush Menat --- frappe/core/doctype/docshare/test_docshare.py | 65 ++++++++++++++++++- .../system_settings/system_settings.json | 11 +++- frappe/desk/form/assign_to.py | 13 +++- frappe/permissions.py | 3 + frappe/public/js/frappe/model/model.js | 6 ++ 5 files changed, 92 insertions(+), 6 deletions(-) diff --git a/frappe/core/doctype/docshare/test_docshare.py b/frappe/core/doctype/docshare/test_docshare.py index b874042d15..5a2a8da936 100644 --- a/frappe/core/doctype/docshare/test_docshare.py +++ b/frappe/core/doctype/docshare/test_docshare.py @@ -4,7 +4,7 @@ import frappe import frappe.share from frappe.automation.doctype.auto_repeat.test_auto_repeat import create_submittable_doctype -from frappe.tests.utils import FrappeTestCase +from frappe.tests.utils import FrappeTestCase, change_settings test_dependencies = ["User"] @@ -139,3 +139,66 @@ class TestDocShare(FrappeTestCase): test_doc.reload() self.assertTrue(test_doc.has_permission("read")) + + @change_settings("System Settings", {"disable_document_sharing": 1}) + def test_share_disabled_add(self): + "Test if user loses share access on disabling share globally." + frappe.share.add("Event", self.event.name, self.user, share=1) # Share as admin + frappe.set_user(self.user) + + # User does not have share access although given to them + self.assertFalse(self.event.has_permission("share")) + self.assertRaises( + frappe.PermissionError, frappe.share.add, "Event", self.event.name, "test1@example.com" + ) + + @change_settings("System Settings", {"disable_document_sharing": 1}) + def test_share_disabled_add_with_ignore_permissions(self): + frappe.share.add("Event", self.event.name, self.user, share=1) + frappe.set_user(self.user) + + # User does not have share access although given to them + self.assertFalse(self.event.has_permission("share")) + + # Test if behaviour is consistent for developer overrides + frappe.share.add_docshare( + "Event", self.event.name, "test1@example.com", flags={"ignore_share_permission": True} + ) + + @change_settings("System Settings", {"disable_document_sharing": 1}) + def test_share_disabled_set_permission(self): + frappe.share.add("Event", self.event.name, self.user, share=1) + frappe.set_user(self.user) + + # User does not have share access although given to them + self.assertFalse(self.event.has_permission("share")) + self.assertRaises( + frappe.PermissionError, + frappe.share.set_permission, + "Event", + self.event.name, + "test1@example.com", + "read", + ) + + @change_settings("System Settings", {"disable_document_sharing": 1}) + def test_share_disabled_assign_to(self): + """ + Assigning a document to a user without access must not share the document, + if sharing disabled. + """ + from frappe.desk.form.assign_to import add, get + + frappe.share.add("Event", self.event.name, self.user, share=1) + frappe.set_user(self.user) + + # Assign to 'test1@example.com' + add({"doctype": "Event", "name": self.event.name, "assign_to": ["test1@example.com"]}) + + # Check if assigned to 'test1@example.com' + assignments = get(dict(doctype="Event", name=self.event.name)) + self.assertEqual(len(assignments), 1) + + # Check if not shared with 'test1@example.com' + shared_users = [x.user for x in frappe.share.get_users("Event", self.event.name)] + self.assertNotIn("test1@example.com", shared_users) diff --git a/frappe/core/doctype/system_settings/system_settings.json b/frappe/core/doctype/system_settings/system_settings.json index 8ebcb493de..102a0a76c2 100644 --- a/frappe/core/doctype/system_settings/system_settings.json +++ b/frappe/core/doctype/system_settings/system_settings.json @@ -13,6 +13,7 @@ "time_zone", "enable_onboarding", "setup_complete", + "disable_document_sharing", "date_and_number_format", "date_format", "time_format", @@ -528,12 +529,18 @@ "fieldtype": "Select", "label": "Rounding Method", "options": "Banker's Rounding (legacy)\nBanker's Rounding\nCommercial Rounding" + }, + { + "default": "0", + "fieldname": "disable_document_sharing", + "fieldtype": "Check", + "label": "Disable Document Sharing" } ], "icon": "fa fa-cog", "issingle": 1, "links": [], - "modified": "2023-03-10 12:23:45.248125", + "modified": "2023-03-14 11:30:56.465653", "modified_by": "Administrator", "module": "Core", "name": "System Settings", @@ -552,4 +559,4 @@ "sort_order": "ASC", "states": [], "track_changes": 1 -} +} \ No newline at end of file diff --git a/frappe/desk/form/assign_to.py b/frappe/desk/form/assign_to.py index 72265dce1f..2b17d38371 100644 --- a/frappe/desk/form/assign_to.py +++ b/frappe/desk/form/assign_to.py @@ -93,10 +93,17 @@ def add(args=None): doc = frappe.get_doc(args["doctype"], args["name"]) - # if assignee does not have permissions, share + # if assignee does not have permissions, share or inform if not frappe.has_permission(doc=doc, user=assign_to): - frappe.share.add(doc.doctype, doc.name, assign_to) - shared_with_users.append(assign_to) + if frappe.get_system_settings("disable_document_sharing"): + msg = _("User {0} is not permitted to access this document.").format(frappe.bold(assign_to)) + msg += "
" + _( + "As document sharing is disabled, please give them the required permissions before assigning." + ) + frappe.throw(msg, title=_("Missing Permission")) + else: + frappe.share.add(doc.doctype, doc.name, assign_to) + shared_with_users.append(assign_to) # make this document followed by assigned user if frappe.get_cached_value("User", assign_to, "follow_assigned_documents"): diff --git a/frappe/permissions.py b/frappe/permissions.py index 75a940233e..97badea500 100644 --- a/frappe/permissions.py +++ b/frappe/permissions.py @@ -77,6 +77,9 @@ def has_permission( if user == "Administrator": return True + if ptype == "share" and frappe.get_system_settings("disable_document_sharing"): + return False + if not doc and hasattr(doctype, "doctype"): # first argument can be doc or doctype doc = doctype diff --git a/frappe/public/js/frappe/model/model.js b/frappe/public/js/frappe/model/model.js index ebf1ff3eac..6c6f617275 100644 --- a/frappe/public/js/frappe/model/model.js +++ b/frappe/public/js/frappe/model/model.js @@ -447,6 +447,12 @@ $.extend(frappe.model, { }, can_share: function (doctype, frm) { + let disable_sharing = cint(frappe.sys_defaults.disable_document_sharing); + + if (disable_sharing && frappe.session.user !== "Administrator") { + return false; + } + if (frm) { return frm.perm[0].share === 1; } From 513a209d531ecccc0f143a8cd13376e51032a6fa Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Wed, 29 Mar 2023 09:25:20 +0530 Subject: [PATCH 13/39] test: fix test case to modified behaviour We throw instead of showing warning now --- frappe/core/doctype/docshare/test_docshare.py | 17 ++++++----------- frappe/tests/utils.py | 4 ++-- 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/frappe/core/doctype/docshare/test_docshare.py b/frappe/core/doctype/docshare/test_docshare.py index 5a2a8da936..e080b0d4ff 100644 --- a/frappe/core/doctype/docshare/test_docshare.py +++ b/frappe/core/doctype/docshare/test_docshare.py @@ -187,18 +187,13 @@ class TestDocShare(FrappeTestCase): Assigning a document to a user without access must not share the document, if sharing disabled. """ - from frappe.desk.form.assign_to import add, get + from frappe.desk.form.assign_to import add frappe.share.add("Event", self.event.name, self.user, share=1) frappe.set_user(self.user) - # Assign to 'test1@example.com' - add({"doctype": "Event", "name": self.event.name, "assign_to": ["test1@example.com"]}) - - # Check if assigned to 'test1@example.com' - assignments = get(dict(doctype="Event", name=self.event.name)) - self.assertEqual(len(assignments), 1) - - # Check if not shared with 'test1@example.com' - shared_users = [x.user for x in frappe.share.get_users("Event", self.event.name)] - self.assertNotIn("test1@example.com", shared_users) + self.assertRaises( + frappe.ValidationError, + add, + {"doctype": "Event", "name": self.event.name, "assign_to": ["test1@example.com"]}, + ) diff --git a/frappe/tests/utils.py b/frappe/tests/utils.py index fe95960518..2cdcfb5643 100644 --- a/frappe/tests/utils.py +++ b/frappe/tests/utils.py @@ -143,7 +143,7 @@ def change_settings(doctype, settings_dict): # change setting for key, value in settings_dict.items(): setattr(settings, key, value) - settings.save() + settings.save(ignore_permissions=True) # singles are cached by default, clear to avoid flake frappe.db.value_cache[settings] = {} yield # yield control to calling function @@ -153,7 +153,7 @@ def change_settings(doctype, settings_dict): settings = frappe.get_doc(doctype) for key, value in previous_settings.items(): setattr(settings, key, value) - settings.save() + settings.save(ignore_permissions=True) def timeout(seconds=30, error_message="Test timed out."): From 6096e45b36108364e7a155501a5478aa3f894751 Mon Sep 17 00:00:00 2001 From: Raffael Meyer <14891507+barredterra@users.noreply.github.com> Date: Wed, 29 Mar 2023 07:01:01 +0200 Subject: [PATCH 14/39] test: switch tests to supported methods (#20494) * fix: switch tests to supported methods get_fetch_fields, update_linked_doctypes * test: semantic assertions * test: fixup deprecation tests imports --------- Co-authored-by: Ankush Menat --- frappe/tests/test_rename_doc.py | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/frappe/tests/test_rename_doc.py b/frappe/tests/test_rename_doc.py index e48f908147..3f67bc4a1f 100644 --- a/frappe/tests/test_rename_doc.py +++ b/frappe/tests/test_rename_doc.py @@ -9,14 +9,9 @@ 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.exceptions import DoesNotExistError from frappe.model.base_document import get_controller -from frappe.model.rename_doc import ( - bulk_rename, - get_fetch_fields, - update_document_title, - update_linked_doctypes, -) +from frappe.model.rename_doc import bulk_rename, update_document_title from frappe.modules.utils import get_doc_path from frappe.tests.utils import FrappeTestCase from frappe.utils import add_to_date, now @@ -255,14 +250,16 @@ class TestRenameDoc(FrappeTestCase): ) def test_deprecated_utils(self): + from frappe.model.rename_doc import get_fetch_fields, update_linked_doctypes + stdout = StringIO() with redirect_stdout(stdout), patch_db(["set_value"]): get_fetch_fields("User", "ToDo", ["Activity Log"]) - self.assertTrue("Function frappe.model.rename_doc.get_fetch_fields" in stdout.getvalue()) + self.assertIn("Function frappe.model.rename_doc.get_fetch_fields", stdout.getvalue()) update_linked_doctypes("User", "ToDo", "str", "str") - self.assertTrue("Function frappe.model.rename_doc.update_linked_doctypes" in stdout.getvalue()) + self.assertIn("Function frappe.model.rename_doc.update_linked_doctypes", stdout.getvalue()) def test_doc_rename_method(self): name = choice(self.available_documents) From 45c86e2ff843f4dfdd35666c224f3b19845fbd8b Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Wed, 29 Mar 2023 12:49:28 +0530 Subject: [PATCH 15/39] fix: nestedset rename (#20498) --- frappe/model/rename_doc.py | 13 ++++++++----- frappe/tests/test_nestedset.py | 7 +++++++ 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/frappe/model/rename_doc.py b/frappe/model/rename_doc.py index 420cbee091..3908365291 100644 --- a/frappe/model/rename_doc.py +++ b/frappe/model/rename_doc.py @@ -488,6 +488,9 @@ def update_options_for_fieldtype(fieldtype: str, old: str, new: str) -> None: if frappe.conf.developer_mode: for name in frappe.get_all("DocField", filters={"options": old}, pluck="parent"): + if name in (old, new): + continue + doctype = frappe.get_doc("DocType", name) save = False for f in doctype.fields: @@ -496,11 +499,11 @@ def update_options_for_fieldtype(fieldtype: str, old: str, new: str) -> None: save = True if save: doctype.save() - else: - DocField = frappe.qb.DocType("DocField") - frappe.qb.update(DocField).set(DocField.options, new).where( - (DocField.fieldtype == fieldtype) & (DocField.options == old) - ).run() + + DocField = frappe.qb.DocType("DocField") + frappe.qb.update(DocField).set(DocField.options, new).where( + (DocField.fieldtype == fieldtype) & (DocField.options == old) + ).run() frappe.qb.update(CustomField).set(CustomField.options, new).where( (CustomField.fieldtype == fieldtype) & (CustomField.options == old) diff --git a/frappe/tests/test_nestedset.py b/frappe/tests/test_nestedset.py index 182831b680..ef63fb66c2 100644 --- a/frappe/tests/test_nestedset.py +++ b/frappe/tests/test_nestedset.py @@ -8,6 +8,7 @@ from frappe.core.doctype.doctype.test_doctype import new_doctype from frappe.query_builder import Field from frappe.query_builder.functions import Max from frappe.tests.utils import FrappeTestCase +from frappe.utils import random_string from frappe.utils.nestedset import ( NestedSetChildExistsError, NestedSetInvalidMergeError, @@ -213,6 +214,12 @@ class TestNestedSet(FrappeTestCase): remove_subtree("Test Tree DocType", "Parent 2") self.test_basic_tree() + def test_rename_nestedset(self): + doctype = new_doctype(is_tree=True).insert() + + # Rename doctype + frappe.rename_doc("DocType", doctype.name, "Test " + random_string(10), force=True) + def test_merge_groups(self): global records el = {"some_fieldname": "Parent 2", "parent_test_tree_doctype": "Root Node", "is_group": 1} From 40ad98359896bfdbe263fc7238334ad7e11e023f Mon Sep 17 00:00:00 2001 From: Raffael Meyer <14891507+barredterra@users.noreply.github.com> Date: Wed, 29 Mar 2023 09:31:32 +0200 Subject: [PATCH 16/39] feat(contact template): clickable email ids and phone numbers, less labels (#20247) * refactor(contact template): use for .. of loop * refactor(contact template): clickable phone numbers and email ids * refactor(contact template): less labels * fix: escape phone and email ids --- .../frappe/form/templates/contact_list.html | 58 ++++++++++--------- 1 file changed, 30 insertions(+), 28 deletions(-) diff --git a/frappe/public/js/frappe/form/templates/contact_list.html b/frappe/public/js/frappe/form/templates/contact_list.html index e5fdc3f88e..c4cc08a549 100644 --- a/frappe/public/js/frappe/form/templates/contact_list.html +++ b/frappe/public/js/frappe/form/templates/contact_list.html @@ -1,49 +1,51 @@
-{% for(var i=0, l=contact_list.length; i

- {%= contact_list[i].first_name %} {%= contact_list[i].last_name %} - {% if(contact_list[i].is_primary_contact) { %} + {%= contact.first_name %} {%= contact.last_name %} + {% if(contact.is_primary_contact) { %}  ({%= __("Primary") %}) {% } %} - {% if(contact_list[i].designation){ %} - – {%= contact_list[i].designation %} + {% if(contact.designation){ %} + – {%= contact.designation %} {% } %} - {%= __("Edit") %}

- {% if (contact_list[i].phones || contact_list[i].email_ids) { %} + {% if (contact.phone || contact.mobile_no || contact.phone_nos.length > 0) { %}

- {% if(contact_list[i].phone) { %} - {%= __("Phone") %}: {%= contact_list[i].phone %} ({%= __("Primary") %})
+ {% if(contact.phone) { %} + {%= frappe.utils.escape_html(contact.phone) %} · {%= __("Primary Phone") %}
{% endif %} - {% if(contact_list[i].mobile_no) { %} - {%= __("Mobile No") %}: {%= contact_list[i].mobile_no %} ({%= __("Primary") %})
+ {% if(contact.mobile_no) { %} + {%= frappe.utils.escape_html(contact.mobile_no) %} · {%= __("Primary Mobile") %}
{% endif %} - {% if(contact_list[i].phone_nos) { %} - {% for(var j=0, k=contact_list[i].phone_nos.length; j - {% } %} - {% endif %} -

-

- {% if(contact_list[i].email_id) { %} - {%= __("Email") %}: {%= contact_list[i].email_id %} ({%= __("Primary") %})
- {% endif %} - {% if(contact_list[i].email_ids) { %} - {% for(var j=0, k=contact_list[i].email_ids.length; j + {% if(contact.phone_nos) { %} + {% for(const phone_no of contact.phone_nos) { %} + {%= frappe.utils.escape_html(phone_no.phone) %}
{% } %} {% endif %}

{% endif %} + {% if (contact.email_id || contact.email_ids.length > 0) { %}

- {% if (contact_list[i].address) { %} - {%= __("Address") %}: {%= contact_list[i].address %}
- {% endif %} + {% if(contact.email_id) { %} + {%= frappe.utils.escape_html(contact.email_id) %} · {%= __("Primary Email") %}
+ {% endif %} + {% if(contact.email_ids) { %} + {% for(const email_id of contact.email_ids) { %} + {%= frappe.utils.escape_html(email_id.email_id) %}
+ {% } %} + {% endif %}

+ {% endif %} + {% if (contact.address) { %} +

+ {%= contact.address %} +

+ {% endif %} {% } %} {% if(!contact_list.length) { %} @@ -51,4 +53,4 @@ {% } %}

-

\ No newline at end of file +

From a56ea73b7d048c1536bc9957ba6e9cde2d1ddc3f Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Thu, 30 Mar 2023 11:52:53 +0530 Subject: [PATCH 17/39] fix: escape HTML instead of sanitizing --- frappe/www/printview.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/frappe/www/printview.py b/frappe/www/printview.py index 9fdc77a2ba..38a0409e5f 100644 --- a/frappe/www/printview.py +++ b/frappe/www/printview.py @@ -11,7 +11,7 @@ import frappe from frappe import _, get_module_path from frappe.core.doctype.access_log.access_log import make_access_log from frappe.core.doctype.document_share_key.document_share_key import is_expired -from frappe.utils import cint, sanitize_html, strip_html +from frappe.utils import cint, escape_html, strip_html from frappe.utils.jinja_globals import is_rtl if TYPE_CHECKING: @@ -27,12 +27,11 @@ def get_context(context): """Build context for print""" if not ((frappe.form_dict.doctype and frappe.form_dict.name) or frappe.form_dict.doc): return { - "body": sanitize_html( - """

Error

+ "body": f""" +

Error

Parameters doctype and name required

- 
%s
""" - % repr(frappe.form_dict) - ) + 
{escape_html(frappe.as_json(frappe.form_dict, indent=2))}
+ """ } if frappe.form_dict.doc: From 9758781f809a60fb1f1c3c29ad746f98478d6a3d Mon Sep 17 00:00:00 2001 From: Raffael Meyer <14891507+barredterra@users.noreply.github.com> Date: Thu, 30 Mar 2023 13:40:44 +0200 Subject: [PATCH 18/39] fix: bulk update using doc method, check perms (#20522) * fix: bulk update * fix: always check permission --- .../desk/doctype/bulk_update/bulk_update.js | 49 +++++++------------ .../desk/doctype/bulk_update/bulk_update.py | 32 ++++++------ 2 files changed, 34 insertions(+), 47 deletions(-) diff --git a/frappe/desk/doctype/bulk_update/bulk_update.js b/frappe/desk/doctype/bulk_update/bulk_update.js index 017eee1480..d8a2b89cf3 100644 --- a/frappe/desk/doctype/bulk_update/bulk_update.js +++ b/frappe/desk/doctype/bulk_update/bulk_update.js @@ -16,38 +16,27 @@ frappe.ui.form.on("Bulk Update", { if (!frm.doc.update_value) { frappe.throw(__('Field "value" is mandatory. Please specify value to be updated')); } else { - frappe - .call({ - method: "frappe.desk.doctype.bulk_update.bulk_update.update", - args: { - doctype: frm.doc.document_type, - field: frm.doc.field, - value: frm.doc.update_value, - condition: frm.doc.condition, - limit: frm.doc.limit, - }, - }) - .then((r) => { - let failed = r.message; - if (!failed) failed = []; + frm.call("bulk_update").then((r) => { + let failed = r.message; + if (!failed) failed = []; - if (failed.length && !r._server_messages) { - frappe.throw( - __("Cannot update {0}", [ - failed.map((f) => (f.bold ? f.bold() : f)).join(", "), - ]) - ); - } else { - frappe.msgprint({ - title: __("Success"), - message: __("Updated Successfully"), - indicator: "green", - }); - } + if (failed.length && !r._server_messages) { + frappe.throw( + __("Cannot update {0}", [ + failed.map((f) => (f.bold ? f.bold() : f)).join(", "), + ]) + ); + } else { + frappe.msgprint({ + title: __("Success"), + message: __("Updated Successfully"), + indicator: "green", + }); + } - frappe.hide_progress(); - frm.save(); - }); + frappe.hide_progress(); + frm.save(); + }); } }); }, diff --git a/frappe/desk/doctype/bulk_update/bulk_update.py b/frappe/desk/doctype/bulk_update/bulk_update.py index 5521d9583f..535be8155f 100644 --- a/frappe/desk/doctype/bulk_update/bulk_update.py +++ b/frappe/desk/doctype/bulk_update/bulk_update.py @@ -10,26 +10,24 @@ from frappe.utils.scheduler import is_scheduler_inactive class BulkUpdate(Document): - pass + @frappe.whitelist() + def bulk_update(self): + self.check_permission("write") + limit = self.limit if self.limit and cint(self.limit) < 500 else 500 + condition = "" + if self.condition: + if ";" in self.condition: + frappe.throw(_("; not allowed in condition")) -@frappe.whitelist() -def update(doctype, field, value, condition="", limit=500): - if not limit or cint(limit) > 500: - limit = 500 + condition = f" where {self.condition}" - if condition: - condition = " where " + condition - - if ";" in condition: - frappe.throw(_("; not allowed in condition")) - - docnames = frappe.db.sql_list( - f"""select name from `tab{doctype}`{condition} limit {limit} offset 0""" - ) - data = {} - data[field] = value - return submit_cancel_or_update_docs(doctype, docnames, "update", data) + docnames = frappe.db.sql_list( + f"""select name from `tab{self.document_type}`{condition} limit {limit} offset 0""" + ) + return submit_cancel_or_update_docs( + self.document_type, docnames, "update", {self.field: self.update_value} + ) @frappe.whitelist() From 2cb492561a2ba790241b081be507b252d8061fac Mon Sep 17 00:00:00 2001 From: Himanshu Shivhare Date: Thu, 30 Mar 2023 17:57:00 +0530 Subject: [PATCH 19/39] fix(ux): correct email account setup path in error message (#20513) --- frappe/core/doctype/communication/email.py | 2 +- frappe/email/doctype/email_account/email_account.py | 2 +- frappe/email/smtp.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/frappe/core/doctype/communication/email.py b/frappe/core/doctype/communication/email.py index 2e199e014d..1733b7b716 100755 --- a/frappe/core/doctype/communication/email.py +++ b/frappe/core/doctype/communication/email.py @@ -165,7 +165,7 @@ def _make( if not comm.get_outgoing_email_account(): frappe.throw( _( - "Unable to send mail because of a missing email account. Please setup default Email Account from Setup > Email > Email Account" + "Unable to send mail because of a missing email account. Please setup default Email Account from Settings > Email Account" ), exc=frappe.OutgoingEmailError, ) diff --git a/frappe/email/doctype/email_account/email_account.py b/frappe/email/doctype/email_account/email_account.py index 2e5dbe2e24..faf28afdb3 100755 --- a/frappe/email/doctype/email_account/email_account.py +++ b/frappe/email/doctype/email_account/email_account.py @@ -325,7 +325,7 @@ class EmailAccount(Document): if _raise_error: frappe.throw( - _("Please setup default Email Account from Setup > Email > Email Account"), + _("Please setup default Email Account from Settings > Email Account"), frappe.OutgoingEmailError, ) diff --git a/frappe/email/smtp.py b/frappe/email/smtp.py index 028b21b0ae..5e1b5ef296 100644 --- a/frappe/email/smtp.py +++ b/frappe/email/smtp.py @@ -70,7 +70,7 @@ class SMTPServer: if not self.server: frappe.msgprint( _( - "Email Account not setup. Please create a new Email Account from Setup > Email > Email Account" + "Email Account not setup. Please create a new Email Account from Settings > Email Account" ), raise_exception=frappe.OutgoingEmailError, ) From aab37e0a6ca59727edf7c48ee9bc64d09677fcea Mon Sep 17 00:00:00 2001 From: Suraj Shetty <13928957+surajshetty3416@users.noreply.github.com> Date: Fri, 31 Mar 2023 13:31:18 +0530 Subject: [PATCH 20/39] fix: Check if reference_name is set --- frappe/core/doctype/communication/mixins.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/core/doctype/communication/mixins.py b/frappe/core/doctype/communication/mixins.py index 22b7e8a0fc..52ea93d829 100644 --- a/frappe/core/doctype/communication/mixins.py +++ b/frappe/core/doctype/communication/mixins.py @@ -217,7 +217,7 @@ class CommunicationEmailMixin: "reference_type": self.reference_doctype, } - if self.reference_doctype == "ToDo" and self.reference_name != None: + if self.reference_doctype and self.reference_name: return ToDo.get_owners(filters) else: return [] From c509983ca4d4450609e6960b858148635bf54950 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Fri, 31 Mar 2023 13:37:55 +0530 Subject: [PATCH 21/39] build: bump redis version https://github.com/redis/redis-py/releases --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 48903f3163..daa0748e5f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -57,7 +57,7 @@ dependencies = [ "python-dateutil~=2.8.1", "pytz==2022.1", "rauth~=0.7.3", - "redis~=4.3.4", + "redis~=4.5.4", "hiredis~=2.0.0", "requests-oauthlib~=1.3.0", "requests~=2.27.1", From 7a92a604e0686e1679c6df3ecce7a9f38e0c1506 Mon Sep 17 00:00:00 2001 From: Suraj Shetty <13928957+surajshetty3416@users.noreply.github.com> Date: Fri, 31 Mar 2023 14:03:49 +0530 Subject: [PATCH 22/39] style: Fix formatting --- frappe/email/smtp.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/frappe/email/smtp.py b/frappe/email/smtp.py index 5e1b5ef296..3b22bc4ce4 100644 --- a/frappe/email/smtp.py +++ b/frappe/email/smtp.py @@ -69,9 +69,7 @@ class SMTPServer: if not self.server: frappe.msgprint( - _( - "Email Account not setup. Please create a new Email Account from Settings > Email Account" - ), + _("Email Account not setup. Please create a new Email Account from Settings > Email Account"), raise_exception=frappe.OutgoingEmailError, ) From 5ad9350b14ada372ed4a528d3988178218c87ebc Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Fri, 31 Mar 2023 14:37:51 +0530 Subject: [PATCH 23/39] fix: Handle JsBarcode exceptions (#20533) JsBarcode exceptions prevent entire page from loading. Instead of that catch error and show it in helpbox so user can correct the barcode if required. Steps to reproduce: 1. Add barcode field 2. Set barcode type in options 3. add invalid barcode and save (cherry picked from commit 57d40b2614068c13b4b14f42438b46b22c0f5533) --- frappe/public/js/frappe/form/controls/barcode.js | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/frappe/public/js/frappe/form/controls/barcode.js b/frappe/public/js/frappe/form/controls/barcode.js index c130ecc039..a819384773 100644 --- a/frappe/public/js/frappe/form/controls/barcode.js +++ b/frappe/public/js/frappe/form/controls/barcode.js @@ -27,6 +27,7 @@ frappe.ui.form.ControlBarcode = class ControlBarcode extends frappe.ui.form.Cont let svg = value; let barcode_value = ""; + this.set_empty_description(); if (value && value.startsWith(" Date: Fri, 31 Mar 2023 17:42:13 +0530 Subject: [PATCH 24/39] perf: Faster address query with explicit joins This querry is particularly problamatic when there are OR conditions due to shared docs. ```sql ANALYZE SELECT `tabAddress`.name, `tabAddress`.city, `tabAddress`.country FROM `tabAddress` join `tabDynamic Link` on (`tabDynamic Link`.parent = `tabAddress`.name AND `tabDynamic Link`.parenttype = 'Address') WHERE `tabDynamic Link`.link_doctype = 'Customer' AND `tabDynamic Link`.link_name = 'CUSTOMER NAME' AND coalesce(`tabAddress`.disabled, 0) = 0 AND (`tabAddress`.`country` like '%%' OR `tabAddress`.`state` like '%%' OR `tabAddress`.`name` like '%%' OR `tabAddress`.`name` like '%%') AND ((((coalesce(`tabAddress`.`branch`, '')='' OR `tabAddress`.`branch` in ('something'))))) OR (`tabAddress`.name in ('SOME SHARED DOC')) ORDER BY if(locate('', `tabAddress`.name), locate('', `tabAddress`.name), 99999), `tabAddress`.idx DESC, `tabAddress`.name LIMIT 0, 20; ``` **Before:** Never terminates and reads entire table with millions of lines :LOL: **After:** Reads ~100 rows max. ``` *************************** 1. row *************************** id: 1 select_type: SIMPLE table: tabDynamic Link type: index_merge possible_keys: parent,link_doctype_link_name_index,link_name key: link_name,link_doctype_link_name_index,parent key_len: 563,1126,563 ref: NULL rows: 40 r_rows: 79.00 filtered: 100.00 r_filtered: 100.00 Extra: Using union(intersect(link_name,link_doctype_link_name_index),parent); Using where; Using temporary; Using filesort *************************** 2. row *************************** id: 1 select_type: SIMPLE table: tabAddress type: eq_ref possible_keys: PRIMARY,selco_branch key: PRIMARY key_len: 562 ref: _900a733bdc3bb9ed.tabDynamic Link.parent rows: 1 r_rows: 1.00 filtered: 100.00 r_filtered: 100.00 Extra: Using where 2 rows in set (0.001 sec) ``` > Please provide enough information so that others can review your pull request: > Explain the **details** for making this change. What existing problem does the pull request solve? > Screenshots/GIFs --- frappe/contacts/doctype/address/address.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/frappe/contacts/doctype/address/address.py b/frappe/contacts/doctype/address/address.py index 5fe22eb7f2..77dee10615 100644 --- a/frappe/contacts/doctype/address/address.py +++ b/frappe/contacts/doctype/address/address.py @@ -254,10 +254,10 @@ def address_query(doctype, txt, searchfield, start, page_len, filters): """select `tabAddress`.name, `tabAddress`.city, `tabAddress`.country from - `tabAddress`, `tabDynamic Link` + `tabAddress` + join `tabDynamic Link` + on (`tabDynamic Link`.parent = `tabAddress`.name and `tabDynamic Link`.parenttype = 'Address') where - `tabDynamic Link`.parent = `tabAddress`.name and - `tabDynamic Link`.parenttype = 'Address' and `tabDynamic Link`.link_doctype = %(link_doctype)s and `tabDynamic Link`.link_name = %(link_name)s and ifnull(`tabAddress`.disabled, 0) = 0 and From fba3497ba0f2c549453d6cae3c6cc553494a7a22 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Fri, 31 Mar 2023 19:04:11 +0530 Subject: [PATCH 25/39] test: address query --- .../contacts/doctype/address/test_address.py | 30 ++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/frappe/contacts/doctype/address/test_address.py b/frappe/contacts/doctype/address/test_address.py index 1d11c5efef..d30b0f9f78 100644 --- a/frappe/contacts/doctype/address/test_address.py +++ b/frappe/contacts/doctype/address/test_address.py @@ -1,7 +1,9 @@ # Copyright (c) 2015, Frappe Technologies and Contributors # License: MIT. See LICENSE +from functools import partial + import frappe -from frappe.contacts.doctype.address.address import get_address_display +from frappe.contacts.doctype.address.address import address_query, get_address_display from frappe.tests.utils import FrappeTestCase @@ -28,3 +30,29 @@ class TestAddress(FrappeTestCase): address = frappe.get_list("Address")[0].name display = get_address_display(frappe.get_doc("Address", address).as_dict()) self.assertTrue(display) + + def test_address_query(self): + def query(doctype="Address", txt="", searchfield="name", start=0, page_len=20, filters=None): + if filters is None: + filters = {"link_doctype": "User", "link_name": "Administrator"} + return address_query(doctype, txt, searchfield, start, page_len, filters) + + frappe.get_doc( + { + "address_type": "Billing", + "address_line1": "1", + "city": "Mumbai", + "state": "Maharashtra", + "country": "India", + "doctype": "Address", + "links": [ + { + "link_doctype": "User", + "link_name": "Administrator", + } + ], + } + ).insert() + + self.assertGreaterEqual(len(query(txt="admin")), 1) + self.assertEqual(len(query(txt="what_zyx")), 0) From ae3b3ebb174d4709f01f002014ebaa0930632aab Mon Sep 17 00:00:00 2001 From: Raffael Meyer <14891507+barredterra@users.noreply.github.com> Date: Sat, 1 Apr 2023 18:52:03 +0200 Subject: [PATCH 26/39] chore: remove excessive whitespace (#20544) to make the linter happy [skip ci] --- frappe/core/doctype/communication/mixins.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/core/doctype/communication/mixins.py b/frappe/core/doctype/communication/mixins.py index 52ea93d829..24b6a8fafb 100644 --- a/frappe/core/doctype/communication/mixins.py +++ b/frappe/core/doctype/communication/mixins.py @@ -216,7 +216,7 @@ class CommunicationEmailMixin: "reference_name": self.reference_name, "reference_type": self.reference_doctype, } - + if self.reference_doctype and self.reference_name: return ToDo.get_owners(filters) else: From b72ec114eeed6de1bc77cedc9b5cc540ae3dd769 Mon Sep 17 00:00:00 2001 From: Shariq Ansari Date: Sun, 2 Apr 2023 14:33:35 +0530 Subject: [PATCH 27/39] fix(UI): align link cards & charts on workspace --- frappe/public/scss/desk/desktop.scss | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/frappe/public/scss/desk/desktop.scss b/frappe/public/scss/desk/desktop.scss index 41eaa4777d..cef6e2e52a 100644 --- a/frappe/public/scss/desk/desktop.scss +++ b/frappe/public/scss/desk/desktop.scss @@ -208,7 +208,6 @@ body { // Overrides for each widgets &.dashboard-widget-box { min-height: 240px; - padding: var(--padding-md) var(--padding-lg); .filter-chart { background-color: var(--control-bg); @@ -238,13 +237,16 @@ body { } .widget-head { - padding: var(--padding-sm); display: flex; justify-content: space-between; flex-wrap: wrap; gap: 6px; } + .widget-body { + padding-top: 7px; + } + .widget-control { display: flex; align-items: center; @@ -560,7 +562,7 @@ body { } &.links-widget-box { - padding: 18px 12px; + padding: 12px 7px; .link-item { display: flex; From 10dcbc5ecf2aa22b128a71a52061eb3a1c3da5f7 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sun, 2 Apr 2023 15:10:29 +0530 Subject: [PATCH 28/39] fix: fix address query for postgres refer https://github.com/frappe/frappe/pull/20537#ref-pullrequest-1645575433 --- frappe/contacts/doctype/address/address.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/contacts/doctype/address/address.py b/frappe/contacts/doctype/address/address.py index 77dee10615..a1ecbdd4a5 100644 --- a/frappe/contacts/doctype/address/address.py +++ b/frappe/contacts/doctype/address/address.py @@ -266,7 +266,7 @@ def address_query(doctype, txt, searchfield, start, page_len, filters): order by if(locate(%(_txt)s, `tabAddress`.name), locate(%(_txt)s, `tabAddress`.name), 99999), `tabAddress`.idx desc, `tabAddress`.name - limit %(start)s, %(page_len)s """.format( + limit %(page_len)s offset %(start)s""".format( mcond=get_match_cond(doctype), search_condition=search_condition, condition=condition or "", From f206b1582e31108efdd90893270b57ab259aace5 Mon Sep 17 00:00:00 2001 From: Marica Date: Sun, 2 Apr 2023 15:14:40 +0530 Subject: [PATCH 29/39] test: Kanban Test fails to remove System Manager role (#20505) * fix: Kanban Test fails to remove System Manager role - As there's only one user with System Manager role, role removal is reverted - Add another user with this role, so that role removal on test user works * chore: Reuse `create_test_user` util * fix: user switching in kanban test --------- Co-authored-by: barredterra <14891507+barredterra@users.noreply.github.com> --- cypress/integration/kanban.js | 19 +++++++++++-------- frappe/tests/ui_test_helpers.py | 9 ++++++--- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/cypress/integration/kanban.js b/cypress/integration/kanban.js index 04a72a9436..f14c991c7c 100644 --- a/cypress/integration/kanban.js +++ b/cypress/integration/kanban.js @@ -98,15 +98,17 @@ context("Kanban Board", () => { }); it("Checks if Kanban Board edits are blocked for non-System Manager and non-owner of the Board", () => { - // create admin kanban board - cy.call("frappe.tests.ui_test_helpers.create_todo", { description: "Frappe User ToDo" }); - cy.switch_to_user("Administrator"); - cy.call("frappe.tests.ui_test_helpers.create_admin_kanban"); - // remove sys manager - cy.remove_role("frappe@example.com", "System Manager"); - cy.switch_to_user("frappe@example.com"); + const noSystemManager = "nosysmanager@example.com"; + cy.call("frappe.tests.ui_test_helpers.create_test_user", { + username: noSystemManager, + }); + cy.remove_role(noSystemManager, "System Manager"); + cy.call("frappe.tests.ui_test_helpers.create_todo", { description: "Frappe User ToDo" }); + cy.call("frappe.tests.ui_test_helpers.create_admin_kanban"); + + cy.switch_to_user(noSystemManager); cy.visit("/app/todo/view/kanban/Admin Kanban"); @@ -122,7 +124,8 @@ context("Kanban Board", () => { // Column actions should be hidden (dropdown for 'Archive' and indicators) cy.get(".kanban .column-options").should("have.length", 0); - cy.add_role("frappe@example.com", "System Manager"); + cy.switch_to_user("Administrator"); + cy.call("frappe.client.delete", { doctype: "User", name: noSystemManager }); }); after(() => { diff --git a/frappe/tests/ui_test_helpers.py b/frappe/tests/ui_test_helpers.py index 2846b33eb9..4d3e3ec11f 100644 --- a/frappe/tests/ui_test_helpers.py +++ b/frappe/tests/ui_test_helpers.py @@ -426,12 +426,15 @@ def create_blog_post(): return doc -def create_test_user(): - if frappe.db.exists("User", UI_TEST_USER): +@whitelist_for_tests +def create_test_user(username=None): + name = username or UI_TEST_USER + + if frappe.db.exists("User", name): return user = frappe.new_doc("User") - user.email = UI_TEST_USER + user.email = name user.first_name = "Frappe" user.new_password = frappe.local.conf.admin_password user.send_welcome_email = 0 From 5fff6698ada2d313493f6e50d4c2acfc889c9e8e Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sun, 2 Apr 2023 15:23:22 +0530 Subject: [PATCH 30/39] fix: use `develop` as branch name for new apps dont ask me why --- frappe/tests/test_boilerplate.py | 4 ++++ frappe/utils/boilerplate.py | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/frappe/tests/test_boilerplate.py b/frappe/tests/test_boilerplate.py index 999c74592e..0f58e84df4 100644 --- a/frappe/tests/test_boilerplate.py +++ b/frappe/tests/test_boilerplate.py @@ -8,6 +8,7 @@ import unittest from io import StringIO from unittest.mock import patch +import git import yaml import frappe @@ -134,6 +135,9 @@ class TestBoilerPlate(unittest.TestCase): self.check_parsable_python_files(new_app_dir) + app_repo = git.Repo(new_app_dir) + self.assertEqual(app_repo.active_branch.name, "develop") + def test_create_app_without_git_init(self): app_name = "test_app_no_git" diff --git a/frappe/utils/boilerplate.py b/frappe/utils/boilerplate.py index 1cd57f4695..2e8a5088ed 100644 --- a/frappe/utils/boilerplate.py +++ b/frappe/utils/boilerplate.py @@ -150,7 +150,7 @@ def _create_app_boilerplate(dest, hooks, no_git=False): f.write(frappe.as_unicode(gitignore_template.format(app_name=hooks.app_name))) # initialize git repository - app_repo = git.Repo.init(app_directory) + app_repo = git.Repo.init(app_directory, initial_branch="develop") app_repo.git.add(A=True) app_repo.index.commit("feat: Initialize App") From 3d626dd36488433e9dd5a72f0bcfa8b0da54c02c Mon Sep 17 00:00:00 2001 From: Wolfram Schmidt Date: Mon, 3 Apr 2023 07:53:12 +0200 Subject: [PATCH 31/39] chore: Update de.csv (#20546) alligned singular and plural. Has effect on DocType naming of DocType Notification! --- frappe/translations/de.csv | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/translations/de.csv b/frappe/translations/de.csv index c60fcbb41e..af8ac7bcb8 100644 --- a/frappe/translations/de.csv +++ b/frappe/translations/de.csv @@ -1720,11 +1720,11 @@ Note: Changing the Page Name will break previous URL to this page.,"Hinweis: Wen Note: Multiple sessions will be allowed in case of mobile device,Hinweis: Mehrere Sitzungen wird im Falle einer mobilen Gerät erlaubt sein, Nothing to show,Nichts anzuzeigen, Nothing to update,Nichts zu aktualisieren, -Notification,Mitteilung, +Notification,Benachrichtigung, Notification Recipient,Benachrichtigungsempfänger, Notification Tones,Benachrichtigungstöne, Notifications,Benachrichtigungen, -Notifications and bulk mails will be sent from this outgoing server.,Hinweise und Massen-E-Mails werden von diesem Postausgangsserver versendet., +Notifications and bulk mails will be sent from this outgoing server.,Benachrichtigungen und Massen-E-Mails werden von diesem Postausgangsserver versendet., Notify Users On Every Login,Benutzer bei jeder Anmeldung benachrichtigen, Notify if unreplied,"Benachrichtigen, wenn unbeantwortet", Notify if unreplied for (in mins),"Benachrichtigen, wenn unbeantwortet für (in Minuten)", From 492fdbeec44d1fa164eb750ba1cd804b2caa7a78 Mon Sep 17 00:00:00 2001 From: Wolfram Schmidt Date: Mon, 3 Apr 2023 07:53:34 +0200 Subject: [PATCH 32/39] chore: Update de.csv (#20545) added translations related to Navbar-Settings. Cleanup. --- frappe/translations/de.csv | 3 +++ 1 file changed, 3 insertions(+) diff --git a/frappe/translations/de.csv b/frappe/translations/de.csv index af8ac7bcb8..1ef1bf1a9b 100644 --- a/frappe/translations/de.csv +++ b/frappe/translations/de.csv @@ -1383,6 +1383,7 @@ Inverse,Invertieren, Is,Ist, Is Attachments Folder,Ist Ordner für Anhänge, Is Child Table,Ist Untertabelle, +Is Custom,Ist benutzerdefiniert, Is Custom Field,Ist benutzerdefiniertes Feld, Is First Startup,Ist Erstes Startup, Is Folder,Ist Ordner, @@ -1613,6 +1614,7 @@ Naming,Bezeichnung, Naming Rule, Benennungsregel, "Naming Options:\n
  1. field:[fieldname] - By Field
  2. naming_series: - By Naming Series (field called naming_series must be present
  3. Prompt - Prompt user for a name
  4. [series] - Series by prefix (separated by a dot); for example PRE.#####
    5. \n
  5. format:EXAMPLE-{MM}morewords{fieldname1}-{fieldname2}-{#####} - Replace all braced words (fieldnames, date words (DD, MM, YY), series) with their value. Outside braces, any characters can be used.
","Namensoptionen:
  1. Feld: [Feldname] - Nach Feld
  2. naming_series: - Nach der Namensreihe (das Feld naming_series muss vorhanden sein)
  3. Eingabeaufforderung - Benutzer nach einem Namen fragen
  4. [Serie] - Reihe nach Präfix (getrennt durch einen Punkt); zum Beispiel PRE. #####
  5. Format: BEISPIEL- {MM} morewords {Feldname1} - {Feldname2} - {#####} - Ersetzt alle verspannten Wörter (Feldnamen, Datumsworte (DD, MM, YY), Serien) durch ihren Wert. Außerhalb von Klammern können beliebige Zeichen verwendet werden.
", Naming Series mandatory,Nummernkreis zwingend erforderlich, +Navigation Settings,Navigationseinstellungen, Nested set error. Please contact the Administrator.,Schachtelfehler. Bitte den Administrator kontaktieren., New Activity,Neue Aktivität, New Chat,Neuer Chat, @@ -2323,6 +2325,7 @@ Show more details,Weiteres, Show only errors,Zeige nur Fehler, "Show title in browser window as ""Prefix - title""","Diesen Eintrag im Browser-Fenster als ""Präfix - Titel"" anzeigen", Showing only Numeric fields from Report,Nur numerische Felder aus Bericht anzeigen, +Sidebar,Seitenleiste, Sidebar Items,Elemente der Seitenleiste, Sidebar Settings,Sidebar-Einstellungen, Sidebar and Comments,Sidebar und Kommentare, From 62a1f234fe2945cb94ab3ae3d924b1166e434fa5 Mon Sep 17 00:00:00 2001 From: Wolfram Schmidt Date: Mon, 3 Apr 2023 07:53:49 +0200 Subject: [PATCH 33/39] chore: Update de.csv (#20547) added missing DocType translation. This used to be Naming Series (Nummernkreis) https://doku.phamos.eu/books/erpnext-benutzerhandbuch-v14/page/dokumentenbenennungseinstellungen-document-naming-settings --- frappe/translations/de.csv | 1 + 1 file changed, 1 insertion(+) diff --git a/frappe/translations/de.csv b/frappe/translations/de.csv index 1ef1bf1a9b..6ef7faa337 100644 --- a/frappe/translations/de.csv +++ b/frappe/translations/de.csv @@ -64,6 +64,7 @@ Delivery Status,Lieferstatus, Department,Abteilung, Details,Details, Document Name,Dokumentenname, +Document Naming Settings,Dokumentenbenennungseinstellungen, Document Status,Dokumentenstatus, Document Type,Dokumententyp, Domain,Domäne, From 75973fd22a6935e862afd48cb23bbec7ebacf199 Mon Sep 17 00:00:00 2001 From: Wolfram Schmidt Date: Mon, 3 Apr 2023 07:53:59 +0200 Subject: [PATCH 34/39] chore: Update de.csv (#20548) translations for new feature to crop images on upload. Crop (cutting) is in conflict with crop (Agricultural) but as this module is not part of ERPNext-Standard installation. This should not matter for now. --- frappe/translations/de.csv | 1 + 1 file changed, 1 insertion(+) diff --git a/frappe/translations/de.csv b/frappe/translations/de.csv index 6ef7faa337..2dd903ee93 100644 --- a/frappe/translations/de.csv +++ b/frappe/translations/de.csv @@ -52,6 +52,7 @@ Content,Inhalt, Content Type,Inhaltstyp, Create,Erstellen, Created By,Erstellt von, +Crop,Zuschneiden, Current,Laufend, Custom HTML,Benutzerdefiniertes HTML, Custom?,Benutzerdefiniert?, From a1aaed0a5f23b8a52e0f075ca953e4102679102e Mon Sep 17 00:00:00 2001 From: Sabu Siyad Date: Mon, 3 Apr 2023 11:26:49 +0530 Subject: [PATCH 35/39] feat(util): `get_table_name`: wrap in backticks (#20553) --- frappe/utils/__init__.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/frappe/utils/__init__.py b/frappe/utils/__init__.py index 5d1aed259a..ef32ff5653 100644 --- a/frappe/utils/__init__.py +++ b/frappe/utils/__init__.py @@ -1031,8 +1031,13 @@ def groupby_metric(iterable: dict[str, list], key: str): return records -def get_table_name(table_name: str) -> str: - return f"tab{table_name}" if not table_name.startswith("__") else table_name +def get_table_name(table_name: str, wrap_in_backticks: bool = False) -> str: + name = f"tab{table_name}" if not table_name.startswith("__") else table_name + + if wrap_in_backticks: + return f"`{name}`" + + return name def squashify(what): From fa32b610d661bae0ca935cf3a4f7c3f5c3ecac66 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 3 Apr 2023 14:47:45 +0530 Subject: [PATCH 36/39] fix: rewrite query for postgres (#20557) --- frappe/contacts/doctype/address/address.py | 6 +++++- frappe/contacts/doctype/address/test_address.py | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/frappe/contacts/doctype/address/address.py b/frappe/contacts/doctype/address/address.py index a1ecbdd4a5..965425019c 100644 --- a/frappe/contacts/doctype/address/address.py +++ b/frappe/contacts/doctype/address/address.py @@ -264,7 +264,11 @@ def address_query(doctype, txt, searchfield, start, page_len, filters): ({search_condition}) {mcond} {condition} order by - if(locate(%(_txt)s, `tabAddress`.name), locate(%(_txt)s, `tabAddress`.name), 99999), + case + when locate(%(_txt)s, `tabAddress`.name) != 0 + then locate(%(_txt)s, `tabAddress`.name) + else 99999 + end, `tabAddress`.idx desc, `tabAddress`.name limit %(page_len)s offset %(start)s""".format( mcond=get_match_cond(doctype), diff --git a/frappe/contacts/doctype/address/test_address.py b/frappe/contacts/doctype/address/test_address.py index d30b0f9f78..ecb95f9e0c 100644 --- a/frappe/contacts/doctype/address/test_address.py +++ b/frappe/contacts/doctype/address/test_address.py @@ -54,5 +54,5 @@ class TestAddress(FrappeTestCase): } ).insert() - self.assertGreaterEqual(len(query(txt="admin")), 1) + self.assertGreaterEqual(len(query(txt="Admin")), 1) self.assertEqual(len(query(txt="what_zyx")), 0) From 06580bdbff9a8f86709e52c82afe0cb9da2dc1d4 Mon Sep 17 00:00:00 2001 From: Daizy Modi Date: Mon, 3 Apr 2023 15:02:05 +0530 Subject: [PATCH 37/39] fix: allow `reset_otp_secret` only if Two Factor Auth is enabled (#20506) * fix: display `Reset OTP Secret` button only if Two factor Auth is enabled * fix: added validations and fetched value from cached doc * fix: linter changes --- frappe/core/doctype/user/user.js | 5 ++++- frappe/twofactor.py | 18 +++++++++++++----- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/frappe/core/doctype/user/user.js b/frappe/core/doctype/user/user.js index 413dd07dc4..918a9ee37c 100644 --- a/frappe/core/doctype/user/user.js +++ b/frappe/core/doctype/user/user.js @@ -219,7 +219,10 @@ frappe.ui.form.on("User", { }); } - if (frappe.session.user == doc.name || frappe.user.has_role("System Manager")) { + if ( + cint(frappe.boot.sysdefaults.enable_two_factor_auth) && + (frappe.session.user == doc.name || frappe.user.has_role("System Manager")) + ) { frm.add_custom_button( __("Reset OTP Secret"), function () { diff --git a/frappe/twofactor.py b/frappe/twofactor.py index 8ad02f0b5a..c4292b0533 100644 --- a/frappe/twofactor.py +++ b/frappe/twofactor.py @@ -450,12 +450,20 @@ def disable(): @frappe.whitelist() -def reset_otp_secret(user): +def reset_otp_secret(user: str): if frappe.session.user != user: frappe.only_for("System Manager", message=True) - otp_issuer = frappe.db.get_single_value("System Settings", "otp_issuer_name") - user_email = frappe.db.get_value("User", user, "email") + settings = frappe.get_cached_doc("System Settings") + + if not settings.enable_two_factor_auth: + frappe.throw( + _("You have to enable Two Factor Auth from System Settings."), + title=_("Enable Two Factor Auth"), + ) + + otp_issuer = settings.otp_issuer_name or "Frappe Framework" + user_email = frappe.get_cached_value("User", user, "email") clear_default(user + "_otplogin") clear_default(user + "_otpsecret") @@ -463,10 +471,10 @@ def reset_otp_secret(user): email_args = { "recipients": user_email, "sender": None, - "subject": _("OTP Secret Reset - {0}").format(otp_issuer or "Frappe Framework"), + "subject": _("OTP Secret Reset - {0}").format(otp_issuer), "message": _( "

Your OTP secret on {0} has been reset. If you did not perform this reset and did not request it, please contact your System Administrator immediately.

" - ).format(otp_issuer or "Frappe Framework"), + ).format(otp_issuer), "delayed": False, "retry": 3, } From d2bde19b5c967b3980ce5175eb74e70e9e74eac4 Mon Sep 17 00:00:00 2001 From: Daizy Modi Date: Mon, 3 Apr 2023 15:03:37 +0530 Subject: [PATCH 38/39] fix: removed unnecessary usage of `@frappe.whitelist` (#20503) --- frappe/core/doctype/server_script/server_script.py | 1 - 1 file changed, 1 deletion(-) diff --git a/frappe/core/doctype/server_script/server_script.py b/frappe/core/doctype/server_script/server_script.py index dc502e4683..17cddee1e8 100644 --- a/frappe/core/doctype/server_script/server_script.py +++ b/frappe/core/doctype/server_script/server_script.py @@ -169,7 +169,6 @@ class ServerScript(Document): return items -@frappe.whitelist() def setup_scheduler_events(script_name, frequency): """Creates or Updates Scheduled Job Type documents based on the specified script name and frequency From 3db1c1aea00cbc79559a73c5cc9638f99ea07cc8 Mon Sep 17 00:00:00 2001 From: Daizy Modi Date: Mon, 3 Apr 2023 15:04:46 +0530 Subject: [PATCH 39/39] fix: allowed only POST and PUT methods in `rename_doc` (#20504) --- frappe/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index 332224f989..9d7befe2d1 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -1274,7 +1274,7 @@ def reload_doc( return frappe.modules.reload_doc(module, dt, dn, force=force, reset_permissions=reset_permissions) -@whitelist() +@whitelist(methods=["POST", "PUT"]) def rename_doc( doctype: str, old: str,