From 5700cf7befd7406977ae66381d070f1b4eedc8ca Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Thu, 10 Nov 2022 18:30:25 +0530 Subject: [PATCH 01/47] feat: Allow app_include_js and app_include_css via site config --- frappe/www/app.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/frappe/www/app.py b/frappe/www/app.py index ad7c69af6e..5ef59634b8 100644 --- a/frappe/www/app.py +++ b/frappe/www/app.py @@ -44,12 +44,15 @@ def get_context(context): boot_json = CLOSING_SCRIPT_TAG_PATTERN.sub("", boot_json) boot_json = json.dumps(boot_json) + include_js = hooks.get("app_include_js", []) + frappe.conf.get("app_include_js", []) + include_css = hooks.get("app_include_css", []) + frappe.conf.get("app_include_css", []) + context.update( { "no_cache": 1, "build_version": frappe.utils.get_build_version(), - "include_js": hooks["app_include_js"], - "include_css": hooks["app_include_css"], + "include_js": include_js, + "include_css": include_css, "layout_direction": "rtl" if is_rtl() else "ltr", "lang": frappe.local.lang, "sounds": hooks["sounds"], From e0a725025c298853a60afe5a48a7c35c648de620 Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Thu, 10 Nov 2022 18:47:26 +0530 Subject: [PATCH 02/47] test: for app_include_js and app_include_css --- frappe/tests/test_website.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/frappe/tests/test_website.py b/frappe/tests/test_website.py index 4cd39f4dd5..fc44064c18 100644 --- a/frappe/tests/test_website.py +++ b/frappe/tests/test_website.py @@ -333,6 +333,30 @@ class TestWebsite(FrappeTestCase): frappe.render_template(content, context), 'Test' ) + def test_app_include(self): + from frappe import hooks + + frappe.conf.update({"developer_mode": 1}) + frappe.set_user("Administrator") + hooks.app_include_js.append("test_app_include.js") + hooks.app_include_css.append("test_app_include.css") + frappe.conf.update({"app_include_js": ["test_app_include_via_site_config.js"]}) + frappe.conf.update({"app_include_css": ["test_app_include_via_site_config.css"]}) + + set_request(method="GET", path="/app") + content = get_response_content("/app") + self.assertIn('', content) + self.assertIn( + '', content + ) + self.assertIn('', content) + self.assertIn( + '', content + ) + frappe.conf.update({"developer_mode": 0}) + frappe.local.request = None + frappe.set_user("Guest") + def set_home_page_hook(key, value): from frappe import hooks From 5695346668d9c70becf292fdb57add54131eaa76 Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Mon, 14 Nov 2022 17:33:28 +0530 Subject: [PATCH 03/47] fix: allow mark tag in texteditor (#18871) --- frappe/utils/html_utils.py | 1 + 1 file changed, 1 insertion(+) diff --git a/frappe/utils/html_utils.py b/frappe/utils/html_utils.py index fa84170330..9885a8a8a9 100644 --- a/frappe/utils/html_utils.py +++ b/frappe/utils/html_utils.py @@ -277,6 +277,7 @@ acceptable_elements = [ "li", "m", "map", + "mark", "menu", "meter", "multicol", From 440825a372f1c533727b81782561a0a476214da9 Mon Sep 17 00:00:00 2001 From: gavin Date: Mon, 14 Nov 2022 18:15:38 +0530 Subject: [PATCH 04/47] refactor: which > find_executable (#18872) Use shutil from the standard library instead of distutils to find executables in PATH --- frappe/build.py | 3 +-- frappe/commands/utils.py | 7 ++++--- frappe/database/db_manager.py | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/frappe/build.py b/frappe/build.py index e66da4bd79..b74afa5d06 100644 --- a/frappe/build.py +++ b/frappe/build.py @@ -4,7 +4,6 @@ import os import re import shutil import subprocess -from distutils.spawn import find_executable from subprocess import getoutput from tempfile import mkdtemp, mktemp from urllib.parse import urlparse @@ -280,7 +279,7 @@ def check_node_executable(): warn = "⚠️ " if node_version.major < 14: click.echo(f"{warn} Please update your node version to 14") - if not find_executable("yarn"): + if not shutil.which("yarn"): click.echo(f"{warn} Please install yarn using below command and try again.\nnpm install -g yarn") click.echo() diff --git a/frappe/commands/utils.py b/frappe/commands/utils.py index e555f63f41..69f6f43ff3 100644 --- a/frappe/commands/utils.py +++ b/frappe/commands/utils.py @@ -2,7 +2,7 @@ import json import os import subprocess import sys -from distutils.spawn import find_executable +from shutil import which import click @@ -12,6 +12,7 @@ from frappe.coverage import CodeCoverage from frappe.exceptions import SiteNotSpecifiedError from frappe.utils import cint, update_progress_bar +find_executable = which # backwards compatibility DATA_IMPORT_DEPRECATION = ( "[DEPRECATED] The `import-csv` command used 'Data Import Legacy' which has been deprecated.\n" "Use `data-import` command instead to import data via 'Data Import'." @@ -525,7 +526,7 @@ def postgres(context): def _mariadb(): from frappe.database.mariadb.database import MariaDBDatabase - mysql = find_executable("mysql") + mysql = which("mysql") command = [ mysql, "--port", @@ -544,7 +545,7 @@ def _mariadb(): def _psql(): - psql = find_executable("psql") + psql = which("psql") subprocess.run([psql, "-d", frappe.conf.db_name]) diff --git a/frappe/database/db_manager.py b/frappe/database/db_manager.py index 3dddb7f862..5840158fa1 100644 --- a/frappe/database/db_manager.py +++ b/frappe/database/db_manager.py @@ -51,12 +51,12 @@ class DbManager: @staticmethod def restore_database(target, source, user, password): import os - from distutils.spawn import find_executable + from shutil import which from frappe.utils import make_esc esc = make_esc("$ ") - pv = find_executable("pv") + pv = which("pv") if pv: pipe = f"{pv} {source} |" From 2971abe517b2d98f35a75544db3ff3953da651a0 Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Tue, 15 Nov 2022 05:37:18 +0000 Subject: [PATCH 05/47] fix: remove middleware to clear `frappe.local` (#18874) --- frappe/app.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/frappe/app.py b/frappe/app.py index 0d7fdc1fe1..8f40fcfa82 100644 --- a/frappe/app.py +++ b/frappe/app.py @@ -5,7 +5,6 @@ import logging import os from werkzeug.exceptions import HTTPException, NotFound -from werkzeug.local import LocalManager from werkzeug.middleware.profiler import ProfilerMiddleware from werkzeug.middleware.shared_data import SharedDataMiddleware from werkzeug.wrappers import Request, Response @@ -25,13 +24,10 @@ from frappe.utils import get_site_name, sanitize_html from frappe.utils.error import make_error_snapshot from frappe.website.serve import get_response -local_manager = LocalManager(frappe.local) - _site = None _sites_path = os.environ.get("SITES_PATH", ".") -@local_manager.middleware @Request.application def application(request: Request): response = None From 01fdb6a2412dad57587ae284553028d7ded8d5d0 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 10 Nov 2022 14:19:39 +0530 Subject: [PATCH 06/47] chore: Remove unused realtime updates These events were added for supporting listeners in desk. The listeners have thus been removed and these are now unnecessary messages published to anyone landing on Frappe pages or on Desk. --- frappe/desk/notifications.py | 3 --- frappe/social/doctype/energy_point_log/energy_point_log.py | 1 - 2 files changed, 4 deletions(-) diff --git a/frappe/desk/notifications.py b/frappe/desk/notifications.py index 77d40f44d2..271f2b4074 100644 --- a/frappe/desk/notifications.py +++ b/frappe/desk/notifications.py @@ -155,8 +155,6 @@ def clear_notifications(user=None): else: cache.delete_key("notification_count:" + name) - frappe.publish_realtime("clear_notifications") - def clear_notification_config(user): frappe.cache().hdel("notification_config", user) @@ -164,7 +162,6 @@ def clear_notification_config(user): def delete_notification_count_for(doctype): frappe.cache().delete_key("notification_count:" + doctype) - frappe.publish_realtime("clear_notifications") def clear_doctype_notifications(doc, method=None, *args, **kwargs): diff --git a/frappe/social/doctype/energy_point_log/energy_point_log.py b/frappe/social/doctype/energy_point_log/energy_point_log.py index e888e9b53f..487f8225f5 100644 --- a/frappe/social/doctype/energy_point_log/energy_point_log.py +++ b/frappe/social/doctype/energy_point_log/energy_point_log.py @@ -37,7 +37,6 @@ class EnergyPointLog(Document): frappe.publish_realtime("energy_point_alert", message=alert_dict, user=self.user) frappe.cache().hdel("energy_points", self.user) - frappe.publish_realtime("update_points", after_commit=True) if self.type != "Review" and frappe.get_cached_value( "Notification Settings", self.user, "energy_points_system_notifications" From c86e1de38a5cdc987f59dc26bb640afa6668b1bb Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 10 Nov 2022 14:22:10 +0530 Subject: [PATCH 07/47] fix(recorder): Publish update only to Administrator --- frappe/recorder.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/frappe/recorder.py b/frappe/recorder.py index 9f6450b2b1..6df3077fa5 100644 --- a/frappe/recorder.py +++ b/frappe/recorder.py @@ -101,7 +101,9 @@ class Recorder: } frappe.cache().hset(RECORDER_REQUEST_SPARSE_HASH, self.uuid, request_data) frappe.publish_realtime( - event="recorder-dump-event", message=json.dumps(request_data, default=str) + event="recorder-dump-event", + message=json.dumps(request_data, default=str), + user="Administrator", ) self.mark_duplicates() From 5210ea593f0e01f6714ee94ef51bee4af2432cf3 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Mon, 14 Nov 2022 14:03:25 +0530 Subject: [PATCH 08/47] fix(socketio): Re-try thrice before trying to reconnect Set reconnectionAttempts to 3. If the server doesn't want to connect with the respective client (invalid origin, no cookie or sid transmitted) or is gone down, socketio client would retry connections indefinitely. This change limits retrying connection to just thrice every second. --- frappe/public/js/frappe/socketio_client.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/frappe/public/js/frappe/socketio_client.js b/frappe/public/js/frappe/socketio_client.js index 9e290ede0b..d091d3f5e7 100644 --- a/frappe/public/js/frappe/socketio_client.js +++ b/frappe/public/js/frappe/socketio_client.js @@ -17,14 +17,17 @@ frappe.socketio = { frappe.socketio.socket = io.connect(frappe.socketio.get_host(port), { secure: true, withCredentials: true, + reconnectionAttempts: 3, }); } else if (window.location.protocol == "http:") { frappe.socketio.socket = io.connect(frappe.socketio.get_host(port), { withCredentials: true, + reconnectionAttempts: 3, }); } else if (window.location.protocol == "file:") { frappe.socketio.socket = io.connect(window.localStorage.server, { withCredentials: true, + reconnectionAttempts: 3, }); } From 4de9c39bb8b9958dc41a426274a5bb2829246275 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Mon, 14 Nov 2022 17:06:25 +0530 Subject: [PATCH 09/47] refactor: SocketIO - Check request data in middleware - Authenticate each connection before allowing room access - Allow site room access only to System Users, restrict Website User & Guests to their respective user rooms Note: This doesn't check for roles / permissions --- frappe/realtime.py | 2 ++ socketio.js | 68 ++++++++++++++++++++++++---------------------- 2 files changed, 38 insertions(+), 32 deletions(-) diff --git a/frappe/realtime.py b/frappe/realtime.py index cac3078ac6..80ccf6ded2 100644 --- a/frappe/realtime.py +++ b/frappe/realtime.py @@ -122,8 +122,10 @@ def get_user_info(): from frappe.sessions import Session session = Session(None, resume=True).get_session_data() + return { "user": session.user, + "user_type": session.user_type, } diff --git a/socketio.js b/socketio.js index 30e931d6bf..d1fce081bb 100644 --- a/socketio.js +++ b/socketio.js @@ -14,47 +14,53 @@ const io = require("socket.io")(conf.socketio_port, { }, }); -// on socket connection -io.on("connection", function (socket) { +io.use((socket, next) => { if (get_hostname(socket.request.headers.host) != get_hostname(socket.request.headers.origin)) { + next(new Error("Invalid origin")); return; } if (!socket.request.headers.cookie) { + next(new Error("No cookie transmitted.")); return; } - const sid = cookie.parse(socket.request.headers.cookie).sid; - if (!sid) { + const cookies = cookie.parse(socket.request.headers.cookie); + + if (!cookies.sid) { + next(new Error("No sid transmitted.")); return; } + socket.sid = cookies.sid; + socket.user = cookies.user_id; - socket.user = cookie.parse(socket.request.headers.cookie).user_id; + request + .get(get_url(socket, "/api/method/frappe.realtime.get_user_info")) + .type("form") + .query({ + sid: socket.sid, + }) + .then((res) => { + console.log(`User ${res.body.message.user} found`); + socket.user = res.body.message.user; + socket.user_type = res.body.message.user_type; + }) + .catch((e) => { + next(new Error(`Unauthorized: ${e}`)); + return; + }); - let retries = 0; - let join_user_room = () => { - request - .get(get_url(socket, "/api/method/frappe.realtime.get_user_info")) - .type("form") - .query({ - sid: sid, - }) - .then((res) => { - const room = get_user_room(socket, res.body.message.user); - socket.join(room); - socket.join(get_site_room(socket)); - }) - .catch((e) => { - if (e.code === "ECONNREFUSED" && retries < 5) { - // retry after 1s - retries += 1; - return setTimeout(join_user_room, 1000); - } - log(`Unable to join user room. ${e}`); - }); - }; + next(); +}); - join_user_room(); +// on socket connection +io.on("connection", function (socket) { + const room = get_user_room(socket); + socket.join(room); + + if (socket.user == "System User") { + socket.join(get_site_room(socket)); + } socket.on("task_subscribe", function (task_id) { var room = get_task_room(socket, task_id); @@ -75,7 +81,6 @@ io.on("connection", function (socket) { socket.on("doc_subscribe", function (doctype, docname) { can_subscribe_doc({ socket, - sid, doctype, docname, callback: () => { @@ -93,7 +98,6 @@ io.on("connection", function (socket) { socket.on("doc_open", function (doctype, docname) { can_subscribe_doc({ socket, - sid, doctype, docname, callback: () => { @@ -210,7 +214,7 @@ function get_typing_room(socket, doctype, docname) { } function get_user_room(socket, user) { - return get_site_name(socket) + ":user:" + user; + return get_site_name(socket) + ":user:" + user || socket.user; } function get_site_room(socket) { @@ -261,7 +265,7 @@ function can_subscribe_doc(args) { .get(get_url(args.socket, "/api/method/frappe.realtime.can_subscribe_doc")) .type("form") .query({ - sid: args.sid, + sid: args.socket.sid, doctype: args.doctype, docname: args.docname, }) From 54bf617d09f50d02adbbceb6d6238150026b6da4 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Mon, 14 Nov 2022 18:00:32 +0530 Subject: [PATCH 10/47] perf(socketio): get_site_name The usages and number of conditions evaluated in the function called for some sort of a cache. If the site name is evaluated once, store it in the socket object! --- socketio.js | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/socketio.js b/socketio.js index d1fce081bb..716b37b5e6 100644 --- a/socketio.js +++ b/socketio.js @@ -226,21 +226,22 @@ function get_task_room(socket, task_id) { } function get_site_name(socket) { - var hostname_from_host = get_hostname(socket.request.headers.host); - - if (socket.request.headers["x-frappe-site-name"]) { - return get_hostname(socket.request.headers["x-frappe-site-name"]); + if (socket.site_name) { + return socket.site_name; + } else if (socket.request.headers["x-frappe-site-name"]) { + socket.site_name = get_hostname(socket.request.headers["x-frappe-site-name"]); } else if ( - ["localhost", "127.0.0.1"].indexOf(hostname_from_host) !== -1 && - conf.default_site + conf.default_site && + ["localhost", "127.0.0.1"].indexOf(get_hostname(socket.request.headers.host)) !== -1 ) { // from currentsite.txt since host is localhost - return conf.default_site; + socket.site_name = conf.default_site; } else if (socket.request.headers.origin) { - return get_hostname(socket.request.headers.origin); + socket.site_name = get_hostname(socket.request.headers.origin); } else { - return get_hostname(socket.request.headers.host); + socket.site_name = get_hostname(socket.request.headers.host); } + return socket.site_name; } function get_hostname(url) { From e97994f2116d674868bfb1f824b69a45b1a1e77e Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Mon, 14 Nov 2022 18:02:47 +0530 Subject: [PATCH 11/47] chore: Drop duplicate event method This particular definition was chosen since there was no corresponding subscribe method with the same key generation logic --- socketio.js | 1 - 1 file changed, 1 deletion(-) diff --git a/socketio.js b/socketio.js index 716b37b5e6..6b10cf9096 100644 --- a/socketio.js +++ b/socketio.js @@ -41,7 +41,6 @@ io.use((socket, next) => { sid: socket.sid, }) .then((res) => { - console.log(`User ${res.body.message.user} found`); socket.user = res.body.message.user; socket.user_type = res.body.message.user_type; }) From 9931c3af04491c293e7b882f9aebc2fddd9b9923 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 15 Nov 2022 12:40:40 +0530 Subject: [PATCH 12/47] refactor(socketio)!: list_update - Subscribe to list_update only for the list/report views that are opened - Check if user has read permission for doctype to subscribe to list updates --- frappe/public/js/frappe/list/list_view.js | 1 + frappe/public/js/frappe/socketio_client.js | 3 + .../js/frappe/views/reports/report_view.js | 1 + frappe/realtime.py | 75 +++++++++++-------- socketio.js | 40 +++++++++- 5 files changed, 89 insertions(+), 31 deletions(-) diff --git a/frappe/public/js/frappe/list/list_view.js b/frappe/public/js/frappe/list/list_view.js index 52d0026e37..a71cdf0a35 100644 --- a/frappe/public/js/frappe/list/list_view.js +++ b/frappe/public/js/frappe/list/list_view.js @@ -1315,6 +1315,7 @@ frappe.views.ListView = class ListView extends frappe.views.BaseList { if (this.list_view_settings && this.list_view_settings.disable_auto_refresh) { return; } + frappe.socketio.list_subscribe(this.doctype); frappe.realtime.on("list_update", (data) => { if (this.avoid_realtime_update()) { return; diff --git a/frappe/public/js/frappe/socketio_client.js b/frappe/public/js/frappe/socketio_client.js index d091d3f5e7..7bbd9fea76 100644 --- a/frappe/public/js/frappe/socketio_client.js +++ b/frappe/public/js/frappe/socketio_client.js @@ -133,6 +133,9 @@ frappe.socketio = { task_unsubscribe: function (task_id) { frappe.socketio.socket.emit("task_unsubscribe", task_id); }, + list_subscribe: function (doctype) { + frappe.socketio.socket.emit("list_update", doctype); + }, doc_subscribe: function (doctype, docname) { if (frappe.flags.doc_subscribe) { console.log("throttled"); diff --git a/frappe/public/js/frappe/views/reports/report_view.js b/frappe/public/js/frappe/views/reports/report_view.js index af7e518678..2e4f09caf8 100644 --- a/frappe/public/js/frappe/views/reports/report_view.js +++ b/frappe/public/js/frappe/views/reports/report_view.js @@ -56,6 +56,7 @@ frappe.views.ReportView = class ReportView extends frappe.views.ListView { if (this.list_view_settings?.disable_auto_refresh) { return; } + frappe.socketio.list_subscribe(this.doctype); frappe.realtime.on("list_update", (data) => this.on_update(data)); } diff --git a/frappe/realtime.py b/frappe/realtime.py index 80ccf6ded2..d843696c50 100644 --- a/frappe/realtime.py +++ b/frappe/realtime.py @@ -2,6 +2,7 @@ # License: MIT. See LICENSE import os +from contextlib import suppress import redis @@ -22,14 +23,14 @@ def publish_progress(percent, title=None, doctype=None, docname=None, descriptio def publish_realtime( - event=None, - message=None, - room=None, - user=None, - doctype=None, - docname=None, - task_id=None, - after_commit=False, + event: str = None, + message: dict = None, + room: str = None, + user: str = None, + doctype: str = None, + docname: str = None, + task_id: str = None, + after_commit: bool = False, ): """Publish real-time updates @@ -44,29 +45,29 @@ def publish_realtime( message = {} if event is None: - if getattr(frappe.local, "task_id", None): - event = "task_progress" - else: - event = "global" - - if event == "msgprint" and not user: + event = "task_progress" if frappe.local.task_id else "global" + elif event == "msgprint" and not user: user = frappe.session.user + elif event == "list_update": + doctype = doctype or message.get("doctype") + room = get_list_room(doctype) + + if not task_id and hasattr(frappe.local, "task_id"): + task_id = frappe.local.task_id if not room: - if not task_id and hasattr(frappe.local, "task_id"): - task_id = frappe.local.task_id - if task_id: - room = get_task_progress_room(task_id) - if not "task_id" in message: - message["task_id"] = task_id - after_commit = False + if "task_id" not in message: + message["task_id"] = task_id + room = get_task_progress_room(task_id) elif user: + # transmit to specific user: System, Website or Guest room = get_user_room(user) elif doctype and docname: room = get_doc_room(doctype, docname) else: + # This will be broadcasted to all Desk users room = get_site_room() if after_commit: @@ -83,13 +84,10 @@ def emit_via_redis(event, message, room): :param event: Event name, like `task_progress` etc. :param message: JSON message object. For async must contain `task_id` :param room: name of the room""" - r = get_redis_server() - try: + with suppress(redis.exceptions.ConnectionError): + r = get_redis_server() r.publish("events", frappe.as_json({"event": event, "message": message, "room": room})) - except redis.exceptions.ConnectionError: - # print(frappe.get_traceback()) - pass def get_redis_server(): @@ -117,6 +115,19 @@ def can_subscribe_doc(doctype, docname): return True +@frappe.whitelist(allow_guest=True) +def can_subscribe_list(doctype): + if os.environ.get("CI"): + return True + + from frappe.exceptions import PermissionError + + if not frappe.has_permission(user=frappe.session.user, doctype=doctype, ptype="read"): + raise PermissionError() + + return True + + @frappe.whitelist(allow_guest=True) def get_user_info(): from frappe.sessions import Session @@ -129,17 +140,21 @@ def get_user_info(): } +def get_list_room(doctype): + return f"{frappe.local.site}:list:{doctype}" + + def get_doc_room(doctype, docname): - return "".join([frappe.local.site, ":doc:", doctype, "/", cstr(docname)]) + return f"{frappe.local.site}:doc:{doctype}/{cstr(docname)}" def get_user_room(user): - return "".join([frappe.local.site, ":user:", user]) + return f"{frappe.local.site}:user:{user}" def get_site_room(): - return "".join([frappe.local.site, ":all"]) + return f"{frappe.local.site}:all" def get_task_progress_room(task_id): - return "".join([frappe.local.site, ":task_progress:", task_id]) + return f"{frappe.local.site}:task_progress:{task_id}" diff --git a/socketio.js b/socketio.js index 6b10cf9096..050e42bcd8 100644 --- a/socketio.js +++ b/socketio.js @@ -57,10 +57,20 @@ io.on("connection", function (socket) { const room = get_user_room(socket); socket.join(room); - if (socket.user == "System User") { + if (socket.user_type == "System User") { socket.join(get_site_room(socket)); } + socket.on("list_update", function (doctype) { + can_subscribe_list({ + socket, + doctype, + callback: () => { + socket.join(get_list_room(socket, doctype)); + }, + }); + }); + socket.on("task_subscribe", function (task_id) { var room = get_task_room(socket, task_id); socket.join(room); @@ -220,6 +230,10 @@ function get_site_room(socket) { return get_site_name(socket) + ":all"; } +function get_list_room(socket, doctype) { + return get_site_name(socket) + ":list:" + doctype; +} + function get_task_room(socket, task_id) { return get_site_name(socket) + ":task_progress:" + task_id; } @@ -284,6 +298,30 @@ function can_subscribe_doc(args) { }); } +function can_subscribe_list(args) { + if (!args) return; + if (!args.doctype) return; + request + .get(get_url(args.socket, "/api/method/frappe.realtime.can_subscribe_list")) + .type("form") + .query({ + sid: args.socket.sid, + doctype: args.doctype, + }) + .end(function (err, res) { + if (!res || res.status == 403 || err) { + if (err) { + log(err); + } + return false; + } else if (res.status == 200) { + args?.callback(err, res); + return true; + } + log("ERROR (can_subscribe_list): ", err, res); + }); +} + function send_users(args, action) { if (!(args && args.doctype && args.docname)) { return; From 97d2eab3e2620221b1762c005da99103f35ee35f Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 15 Nov 2022 13:05:01 +0530 Subject: [PATCH 13/47] refactor(socketio): docinfo_update - Rename event from `update_docinfo_for_{}_{}` to docinfo_update - Separate rooms for separate documents generated on requirement - Check if user has access to doc before sharing docinfo --- frappe/core/doctype/comment/comment.py | 4 +++- .../core/doctype/communication/communication.py | 4 +++- frappe/public/js/frappe/form/form.js | 7 +++---- frappe/public/js/frappe/socketio_client.js | 3 +++ frappe/realtime.py | 6 ++++++ socketio.js | 16 ++++++++++++++++ 6 files changed, 34 insertions(+), 6 deletions(-) diff --git a/frappe/core/doctype/comment/comment.py b/frappe/core/doctype/comment/comment.py index 3d886cf93b..9cad86ece1 100644 --- a/frappe/core/doctype/comment/comment.py +++ b/frappe/core/doctype/comment/comment.py @@ -44,8 +44,10 @@ class Comment(Document): return frappe.publish_realtime( - f"update_docinfo_for_{self.reference_doctype}_{self.reference_name}", + "docinfo_update", {"doc": self.as_dict(), "key": key, "action": action}, + doctype=self.reference_doctype, + docname=self.reference_name, after_commit=True, ) diff --git a/frappe/core/doctype/communication/communication.py b/frappe/core/doctype/communication/communication.py index bd85023025..9944961ca9 100644 --- a/frappe/core/doctype/communication/communication.py +++ b/frappe/core/doctype/communication/communication.py @@ -233,8 +233,10 @@ class Communication(Document, CommunicationEmailMixin): def notify_change(self, action): frappe.publish_realtime( - f"update_docinfo_for_{self.reference_doctype}_{self.reference_name}", + "docinfo_update", {"doc": self.as_dict(), "key": "communications", "action": action}, + doctype=self.reference_doctype, + docname=self.reference_name, after_commit=True, ) diff --git a/frappe/public/js/frappe/form/form.js b/frappe/public/js/frappe/form/form.js index 4f119f2551..78cdb03f6e 100644 --- a/frappe/public/js/frappe/form/form.js +++ b/frappe/public/js/frappe/form/form.js @@ -1942,10 +1942,9 @@ frappe.ui.form.Form = class FrappeForm { setup_docinfo_change_listener() { let doctype = this.doctype; let docname = this.docname; - let listener_name = `update_docinfo_for_${doctype}_${docname}`; - // to avoid duplicates - frappe.realtime.off(listener_name); - frappe.realtime.on(listener_name, ({ doc, key, action = "update" }) => { + + frappe.socketio.docinfo_subscribe(doctype, docname); + frappe.realtime.on("docinfo_update", ({ doc, key, action = "update" }) => { let doc_list = frappe.model.docinfo[doctype][docname][key] || []; if (action === "add") { frappe.model.docinfo[doctype][docname][key].push(doc); diff --git a/frappe/public/js/frappe/socketio_client.js b/frappe/public/js/frappe/socketio_client.js index 7bbd9fea76..e744a04c54 100644 --- a/frappe/public/js/frappe/socketio_client.js +++ b/frappe/public/js/frappe/socketio_client.js @@ -152,6 +152,9 @@ frappe.socketio = { frappe.socketio.socket.emit("doc_subscribe", doctype, docname); frappe.socketio.open_docs.push({ doctype: doctype, docname: docname }); }, + docinfo_subscribe: function (doctype, docname) { + frappe.socketio.socket.emit("docinfo_update", doctype, docname); + }, doc_unsubscribe: function (doctype, docname) { frappe.socketio.socket.emit("doc_unsubscribe", doctype, docname); frappe.socketio.open_docs = $.filter(frappe.socketio.open_docs, function (d) { diff --git a/frappe/realtime.py b/frappe/realtime.py index d843696c50..c1af250fcd 100644 --- a/frappe/realtime.py +++ b/frappe/realtime.py @@ -51,6 +51,8 @@ def publish_realtime( elif event == "list_update": doctype = doctype or message.get("doctype") room = get_list_room(doctype) + elif event == "docinfo_update": + room = get_docinfo_room(doctype, docname) if not task_id and hasattr(frappe.local, "task_id"): task_id = frappe.local.task_id @@ -148,6 +150,10 @@ def get_doc_room(doctype, docname): return f"{frappe.local.site}:doc:{doctype}/{cstr(docname)}" +def get_docinfo_room(doctype, docname): + return f"{frappe.local.site}:doc:{doctype}/{cstr(docname)}" + + def get_user_room(user): return f"{frappe.local.site}:user:{user}" diff --git a/socketio.js b/socketio.js index 050e42bcd8..5f6057c707 100644 --- a/socketio.js +++ b/socketio.js @@ -87,6 +87,18 @@ io.on("connection", function (socket) { send_existing_lines(task_id, socket); }); + socket.on("docinfo_update", function (doctype, docname) { + can_subscribe_doc({ + socket, + doctype, + docname, + callback: () => { + var room = get_docinfo_room(socket, doctype, docname); + socket.join(room); + }, + }); + }); + socket.on("doc_subscribe", function (doctype, docname) { can_subscribe_doc({ socket, @@ -214,6 +226,10 @@ function get_doc_room(socket, doctype, docname) { return get_site_name(socket) + ":doc:" + doctype + "/" + docname; } +function get_docinfo_room(socket, doctype, docname) { + return get_site_name(socket) + ":docinfo:" + doctype + "/" + docname; +} + function get_open_doc_room(socket, doctype, docname) { return get_site_name(socket) + ":open_doc:" + doctype + "/" + docname; } From cfc2dd44376d96c13f7eefbc769ff8f2663390fd Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Tue, 15 Nov 2022 14:50:38 +0530 Subject: [PATCH 14/47] test: patch get_hooks to bypass cache --- frappe/tests/test_website.py | 128 +++++++++++++++++++---------------- 1 file changed, 68 insertions(+), 60 deletions(-) diff --git a/frappe/tests/test_website.py b/frappe/tests/test_website.py index fc44064c18..2179f4cf13 100644 --- a/frappe/tests/test_website.py +++ b/frappe/tests/test_website.py @@ -67,24 +67,41 @@ class TestWebsite(FrappeTestCase): self.assertEqual(get_home_page(), "login") frappe.set_user("Administrator") + from frappe import get_hooks + + def patched_get_hooks(hook, value): + def wrapper(*args, **kwargs): + return_value = get_hooks(*args, **kwargs) + if args[0] == hook: + return_value = value + return return_value + + return wrapper + # test homepage via hooks clear_website_cache() - set_home_page_hook( - "get_website_user_home_page", "frappe.www._test._test_home_page.get_website_user_home_page" - ) - self.assertEqual(get_home_page(), "_test/_test_folder") + with patch.object( + frappe, + "get_hooks", + patched_get_hooks( + "get_website_user_home_page", ["frappe.www._test._test_home_page.get_website_user_home_page"] + ), + ): + self.assertEqual(get_home_page(), "_test/_test_folder") clear_website_cache() - set_home_page_hook("website_user_home_page", "login") - self.assertEqual(get_home_page(), "login") + with patch.object(frappe, "get_hooks", patched_get_hooks("website_user_home_page", ["login"])): + self.assertEqual(get_home_page(), "login") clear_website_cache() - set_home_page_hook("home_page", "about") - self.assertEqual(get_home_page(), "about") + with patch.object(frappe, "get_hooks", patched_get_hooks("home_page", ["about"])): + self.assertEqual(get_home_page(), "about") clear_website_cache() - set_home_page_hook("role_home_page", {"home-page-test": "home-page-test"}) - self.assertEqual(get_home_page(), "home-page-test") + with patch.object( + frappe, "get_hooks", patched_get_hooks("role_home_page", {"home-page-test": ["home-page-test"]}) + ): + self.assertEqual(get_home_page(), "home-page-test") def test_page_load(self): set_request(method="POST", path="login") @@ -196,24 +213,26 @@ class TestWebsite(FrappeTestCase): frappe.cache().delete_key("app_hooks") def test_custom_page_renderer(self): - import frappe.hooks + from frappe import get_hooks - frappe.hooks.page_renderer = ["frappe.tests.test_website.CustomPageRenderer"] - frappe.cache().delete_key("app_hooks") - set_request(method="GET", path="/custom") - response = get_response() - self.assertEqual(response.status_code, 3984) + def patched_get_hooks(*args, **kwargs): + return_value = get_hooks(*args, **kwargs) + if args and args[0] == "page_renderer": + return_value = ["frappe.tests.test_website.CustomPageRenderer"] + return return_value - set_request(method="GET", path="/new") - content = get_response_content() - self.assertIn("
Custom Page Response
", content) + with patch.object(frappe, "get_hooks", patched_get_hooks): + set_request(method="GET", path="/custom") + response = get_response() + self.assertEqual(response.status_code, 3984) - set_request(method="GET", path="/random") - response = get_response() - self.assertEqual(response.status_code, 404) + set_request(method="GET", path="/new") + content = get_response_content() + self.assertIn("
Custom Page Response
", content) - delattr(frappe.hooks, "page_renderer") - frappe.cache().delete_key("app_hooks") + set_request(method="GET", path="/random") + response = get_response() + self.assertEqual(response.status_code, 404) def test_printview_page(self): frappe.db.value_cache[("DocType", "Language", "name")] = (("Language",),) @@ -334,45 +353,34 @@ class TestWebsite(FrappeTestCase): ) def test_app_include(self): - from frappe import hooks + from frappe import get_hooks - frappe.conf.update({"developer_mode": 1}) - frappe.set_user("Administrator") - hooks.app_include_js.append("test_app_include.js") - hooks.app_include_css.append("test_app_include.css") - frappe.conf.update({"app_include_js": ["test_app_include_via_site_config.js"]}) - frappe.conf.update({"app_include_css": ["test_app_include_via_site_config.css"]}) + def patched_get_hooks(*args, **kwargs): + return_value = get_hooks(*args, **kwargs) + if isinstance(return_value, dict) and "app_include_js" in return_value: + return_value.app_include_js.append("test_app_include.js") + return_value.app_include_css.append("test_app_include.css") + return return_value - set_request(method="GET", path="/app") - content = get_response_content("/app") - self.assertIn('', content) - self.assertIn( - '', content - ) - self.assertIn('', content) - self.assertIn( - '', content - ) - frappe.conf.update({"developer_mode": 0}) - frappe.local.request = None - frappe.set_user("Guest") + with patch.object(frappe, "get_hooks", patched_get_hooks): + frappe.set_user("Administrator") + frappe.hooks.app_include_js.append("test_app_include.js") + frappe.hooks.app_include_css.append("test_app_include.css") + frappe.conf.update({"app_include_js": ["test_app_include_via_site_config.js"]}) + frappe.conf.update({"app_include_css": ["test_app_include_via_site_config.css"]}) - -def set_home_page_hook(key, value): - from frappe import hooks - - # reset home_page hooks - for hook in ( - "get_website_user_home_page", - "website_user_home_page", - "role_home_page", - "home_page", - ): - if hasattr(hooks, hook): - delattr(hooks, hook) - - setattr(hooks, key, value) - frappe.cache().delete_key("app_hooks") + set_request(method="GET", path="/app") + content = get_response_content("/app") + self.assertIn('', content) + self.assertIn( + '', content + ) + self.assertIn('', content) + self.assertIn( + '', content + ) + delattr(frappe.local, "request") + frappe.set_user("Guest") class CustomPageRenderer: From 425e4bf1b3dea2fd1daac6e07172850174f8ae08 Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Tue, 15 Nov 2022 10:49:02 +0000 Subject: [PATCH 15/47] fix(File): validate `attached_to_*` when saving (#18880) --- frappe/core/doctype/file/file.py | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/frappe/core/doctype/file/file.py b/frappe/core/doctype/file/file.py index 1518c72f95..0e28145c9a 100755 --- a/frappe/core/doctype/file/file.py +++ b/frappe/core/doctype/file/file.py @@ -78,21 +78,38 @@ class File(Document): self.validate_duplicate_entry() def validate(self): + if self.is_folder: + return + # Ensure correct formatting and type self.file_url = unquote(self.file_url) if self.file_url else "" + self.validate_attachment_references() + # when dict is passed to get_doc for creation of new_doc, is_new returns None # this case is handled inside handle_is_private_changed if not self.is_new() and self.has_value_changed("is_private"): self.handle_is_private_changed() - if not self.is_folder: - self.validate_file_path() - self.validate_file_url() - self.validate_file_on_disk() + self.validate_file_path() + self.validate_file_url() + self.validate_file_on_disk() self.file_size = frappe.form_dict.file_size or self.file_size + def validate_attachment_references(self): + if not self.attached_to_doctype: + return + + if self.attached_to_name and not isinstance(self.attached_to_name, str): + frappe.throw(_("Attached To Name must be a string"), TypeError) + + if not self.attached_to_field: + return + + if not frappe.get_meta(self.attached_to_doctype).has_field(self.attached_to_field): + frappe.throw(_("The fieldname you've specified in Attached To Field is invalid")) + def after_rename(self, *args, **kwargs): for successor in self.get_successors(): setup_folder_path(successor, self.name) From 9b90e620bcac12fdd271de32d6f948aaf89a7443 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 15 Nov 2022 17:16:30 +0530 Subject: [PATCH 16/47] chore: disable flaky test This is - flaky - difficult to find source of flake because of crazy tests - adds little value tbh [skip ci] --- frappe/tests/test_zbg_job_sanity_test.py | 27 ------------------------ 1 file changed, 27 deletions(-) delete mode 100644 frappe/tests/test_zbg_job_sanity_test.py diff --git a/frappe/tests/test_zbg_job_sanity_test.py b/frappe/tests/test_zbg_job_sanity_test.py deleted file mode 100644 index 19dc168c04..0000000000 --- a/frappe/tests/test_zbg_job_sanity_test.py +++ /dev/null @@ -1,27 +0,0 @@ -""" smoak tests to check that all registered background jobs execute without error. - -Note: Filename is intentional to run this test roughly at end. Don't change.""" - -import time - -import frappe -from frappe.core.doctype.rq_job.rq_job import RQJob, remove_failed_jobs -from frappe.tests.utils import FrappeTestCase, timeout - - -class TestScheduledJobSanity(FrappeTestCase): - def setUp(self): - remove_failed_jobs() - - @timeout(90) - def test_bg_jobs_run(self): - """Enqueue all scheduled jobs, wait for finish and verify that none failed.""" - for scheduled_job_type in frappe.get_all("Scheduled Job Type", pluck="name"): - frappe.get_doc("Scheduled Job Type", scheduled_job_type).enqueue(force=True) - - while RQJob.get_list({"filters": [["RQ Job", "status", "in", ("queued", "started")]]}): - time.sleep(0.5) - - # Check no failed, if failed print full details - failed_jobs = RQJob.get_list({"filters": [["RQ Job", "status", "=", "failed"]]}) - self.assertEqual(len(failed_jobs), 0, "Jobs failed: " + str(failed_jobs)) From 3a8fa6cbd5a29d7743e687a9cec96136c83749ee Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 15 Nov 2022 17:49:24 +0530 Subject: [PATCH 17/47] refactor(socketio): Use same room for doc & info events other changes - Name list room as doctype room for more generic use - avoid re-setting up listeners for generic events - discard docinfo_subscribe event --- frappe/public/js/frappe/form/form.js | 8 +++++++- frappe/public/js/frappe/model/model.js | 5 +++++ frappe/public/js/frappe/socketio_client.js | 6 +++--- frappe/realtime.py | 12 ++++-------- socketio.js | 22 +++------------------- 5 files changed, 22 insertions(+), 31 deletions(-) diff --git a/frappe/public/js/frappe/form/form.js b/frappe/public/js/frappe/form/form.js index 78cdb03f6e..60ec53250a 100644 --- a/frappe/public/js/frappe/form/form.js +++ b/frappe/public/js/frappe/form/form.js @@ -1943,7 +1943,12 @@ frappe.ui.form.Form = class FrappeForm { let doctype = this.doctype; let docname = this.docname; - frappe.socketio.docinfo_subscribe(doctype, docname); + frappe.socketio.doc_subscribe(doctype, docname); + + if (frappe.socketio.is_docinfo_listener_setup) { + return; + } + frappe.realtime.on("docinfo_update", ({ doc, key, action = "update" }) => { let doc_list = frappe.model.docinfo[doctype][docname][key] || []; if (action === "add") { @@ -1974,6 +1979,7 @@ frappe.ui.form.Form = class FrappeForm { this.timeline && this.timeline.refresh(); } }); + frappe.socketio.is_docinfo_listener_setup = true; } // Filters fields from the reference doctype and sets them as options for a Select field diff --git a/frappe/public/js/frappe/model/model.js b/frappe/public/js/frappe/model/model.js index 99553f470c..bcd6b297b2 100644 --- a/frappe/public/js/frappe/model/model.js +++ b/frappe/public/js/frappe/model/model.js @@ -79,6 +79,10 @@ $.extend(frappe.model, { init: function () { // setup refresh if the document is updated somewhere else + if (frappe.socketio.is_document_listener_setup) { + return; + } + frappe.realtime.on("doc_update", function (data) { var doc = locals[data.doctype] && locals[data.doctype][data.name]; @@ -104,6 +108,7 @@ $.extend(frappe.model, { } } }); + frappe.socketio.is_document_listener_setup = true; }, is_value_type: function (fieldtype) { diff --git a/frappe/public/js/frappe/socketio_client.js b/frappe/public/js/frappe/socketio_client.js index e744a04c54..3b84c514d9 100644 --- a/frappe/public/js/frappe/socketio_client.js +++ b/frappe/public/js/frappe/socketio_client.js @@ -3,6 +3,9 @@ frappe.socketio = { open_tasks: {}, open_docs: [], emit_queue: [], + is_docinfo_listener_setup: false, + is_document_listener_setup: false, + init: function (port = 3000) { if (frappe.boot.disable_async) { return; @@ -152,9 +155,6 @@ frappe.socketio = { frappe.socketio.socket.emit("doc_subscribe", doctype, docname); frappe.socketio.open_docs.push({ doctype: doctype, docname: docname }); }, - docinfo_subscribe: function (doctype, docname) { - frappe.socketio.socket.emit("docinfo_update", doctype, docname); - }, doc_unsubscribe: function (doctype, docname) { frappe.socketio.socket.emit("doc_unsubscribe", doctype, docname); frappe.socketio.open_docs = $.filter(frappe.socketio.open_docs, function (d) { diff --git a/frappe/realtime.py b/frappe/realtime.py index c1af250fcd..1ab3a74238 100644 --- a/frappe/realtime.py +++ b/frappe/realtime.py @@ -50,9 +50,9 @@ def publish_realtime( user = frappe.session.user elif event == "list_update": doctype = doctype or message.get("doctype") - room = get_list_room(doctype) + room = get_doctype_room(doctype) elif event == "docinfo_update": - room = get_docinfo_room(doctype, docname) + room = get_doc_room(doctype, docname) if not task_id and hasattr(frappe.local, "task_id"): task_id = frappe.local.task_id @@ -142,18 +142,14 @@ def get_user_info(): } -def get_list_room(doctype): - return f"{frappe.local.site}:list:{doctype}" +def get_doctype_room(doctype): + return f"{frappe.local.site}:doctype:{doctype}" def get_doc_room(doctype, docname): return f"{frappe.local.site}:doc:{doctype}/{cstr(docname)}" -def get_docinfo_room(doctype, docname): - return f"{frappe.local.site}:doc:{doctype}/{cstr(docname)}" - - def get_user_room(user): return f"{frappe.local.site}:user:{user}" diff --git a/socketio.js b/socketio.js index 5f6057c707..a6abeae87d 100644 --- a/socketio.js +++ b/socketio.js @@ -66,7 +66,7 @@ io.on("connection", function (socket) { socket, doctype, callback: () => { - socket.join(get_list_room(socket, doctype)); + socket.join(get_doctype_room(socket, doctype)); }, }); }); @@ -87,18 +87,6 @@ io.on("connection", function (socket) { send_existing_lines(task_id, socket); }); - socket.on("docinfo_update", function (doctype, docname) { - can_subscribe_doc({ - socket, - doctype, - docname, - callback: () => { - var room = get_docinfo_room(socket, doctype, docname); - socket.join(room); - }, - }); - }); - socket.on("doc_subscribe", function (doctype, docname) { can_subscribe_doc({ socket, @@ -226,10 +214,6 @@ function get_doc_room(socket, doctype, docname) { return get_site_name(socket) + ":doc:" + doctype + "/" + docname; } -function get_docinfo_room(socket, doctype, docname) { - return get_site_name(socket) + ":docinfo:" + doctype + "/" + docname; -} - function get_open_doc_room(socket, doctype, docname) { return get_site_name(socket) + ":open_doc:" + doctype + "/" + docname; } @@ -246,8 +230,8 @@ function get_site_room(socket) { return get_site_name(socket) + ":all"; } -function get_list_room(socket, doctype) { - return get_site_name(socket) + ":list:" + doctype; +function get_doctype_room(socket, doctype) { + return get_site_name(socket) + ":doctype:" + doctype; } function get_task_room(socket, task_id) { From 9fc330ea6c702515f4862e49cab0fae796420ef6 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 15 Nov 2022 18:45:51 +0530 Subject: [PATCH 18/47] Revert "fix: remove middleware to clear `frappe.local` (#18874)" (#18886) This reverts commit 2971abe517b2d98f35a75544db3ff3953da651a0. --- frappe/app.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/frappe/app.py b/frappe/app.py index 8f40fcfa82..0d7fdc1fe1 100644 --- a/frappe/app.py +++ b/frappe/app.py @@ -5,6 +5,7 @@ import logging import os from werkzeug.exceptions import HTTPException, NotFound +from werkzeug.local import LocalManager from werkzeug.middleware.profiler import ProfilerMiddleware from werkzeug.middleware.shared_data import SharedDataMiddleware from werkzeug.wrappers import Request, Response @@ -24,10 +25,13 @@ from frappe.utils import get_site_name, sanitize_html from frappe.utils.error import make_error_snapshot from frappe.website.serve import get_response +local_manager = LocalManager(frappe.local) + _site = None _sites_path = os.environ.get("SITES_PATH", ".") +@local_manager.middleware @Request.application def application(request: Request): response = None From 88e331e23687c89b73736eb72c980407e9db70c0 Mon Sep 17 00:00:00 2001 From: Maharshi Patel Date: Wed, 16 Nov 2022 12:55:25 +0530 Subject: [PATCH 19/47] fix: z-index for barcode and awesomeplete --- frappe/public/js/frappe/form/controls/data.js | 2 +- frappe/public/scss/common/awesomeplete.scss | 2 +- frappe/public/scss/common/controls.scss | 4 ++++ 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/frappe/public/js/frappe/form/controls/data.js b/frappe/public/js/frappe/form/controls/data.js index 5917a85bdb..0f34f151b7 100644 --- a/frappe/public/js/frappe/form/controls/data.js +++ b/frappe/public/js/frappe/form/controls/data.js @@ -77,7 +77,7 @@ frappe.ui.form.ControlData = class ControlData extends frappe.ui.form.ControlInp setup_url_field() { this.$wrapper.find(".control-input").append( - ` + ` ${frappe.utils.icon("link-url", "sm")} diff --git a/frappe/public/scss/common/awesomeplete.scss b/frappe/public/scss/common/awesomeplete.scss index 17f33d7e82..24e1e3a501 100644 --- a/frappe/public/scss/common/awesomeplete.scss +++ b/frappe/public/scss/common/awesomeplete.scss @@ -30,7 +30,7 @@ left: 0; margin: 0; padding: var(--padding-xs); - z-index: 1; + z-index: 2; min-width: 250px; &> li { diff --git a/frappe/public/scss/common/controls.scss b/frappe/public/scss/common/controls.scss index 0f42f6af9d..92bc996775 100644 --- a/frappe/public/scss/common/controls.scss +++ b/frappe/public/scss/common/controls.scss @@ -145,6 +145,10 @@ select.form-control { background-color: none; display: none; } + + .scan-icon { + z-index: 1; + } } .frappe-control:not([data-fieldtype='MultiSelectPills']):not([data-fieldtype='Table MultiSelect']) { From 45b0c3e28d1cec9958afb2417689c0f4eaa97060 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Wed, 16 Nov 2022 14:05:53 +0530 Subject: [PATCH 20/47] chore: remove dead code --- frappe/public/js/frappe/socketio_client.js | 5 ----- socketio.js | 13 ------------- 2 files changed, 18 deletions(-) diff --git a/frappe/public/js/frappe/socketio_client.js b/frappe/public/js/frappe/socketio_client.js index 3b84c514d9..d4f8fec87f 100644 --- a/frappe/public/js/frappe/socketio_client.js +++ b/frappe/public/js/frappe/socketio_client.js @@ -27,11 +27,6 @@ frappe.socketio = { withCredentials: true, reconnectionAttempts: 3, }); - } else if (window.location.protocol == "file:") { - frappe.socketio.socket = io.connect(window.localStorage.server, { - withCredentials: true, - reconnectionAttempts: 3, - }); } if (!frappe.socketio.socket) { diff --git a/socketio.js b/socketio.js index a6abeae87d..1bafa79a5c 100644 --- a/socketio.js +++ b/socketio.js @@ -84,7 +84,6 @@ io.on("connection", function (socket) { socket.on("progress_subscribe", function (task_id) { var room = get_task_room(socket, task_id); socket.join(room); - send_existing_lines(task_id, socket); }); socket.on("doc_subscribe", function (doctype, docname) { @@ -198,18 +197,6 @@ subscriber.on("message", function (_channel, message) { subscriber.subscribe("events"); -function send_existing_lines(task_id, socket) { - var room = get_task_room(socket, task_id); - subscriber.hgetall("task_log:" + task_id, function (_err, lines) { - io.to(room).emit("task_progress", { - task_id: task_id, - message: { - lines: lines, - }, - }); - }); -} - function get_doc_room(socket, doctype, docname) { return get_site_name(socket) + ":doc:" + doctype + "/" + docname; } From 2b7e4554c4e0dc6efeffecb5e699e1a666388190 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Wed, 16 Nov 2022 15:04:55 +0530 Subject: [PATCH 21/47] fix(desk): maintain realtime & cached data consistency - Clear docinfo_update callbacks before setting one; ensure only one active callback at any given point. - Remove document from local cache if list_update sent if not edited --- frappe/public/js/frappe/form/form.js | 22 ++++++++++++---------- frappe/public/js/frappe/list/list_view.js | 4 ++++ 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/frappe/public/js/frappe/form/form.js b/frappe/public/js/frappe/form/form.js index 60ec53250a..faebdf3493 100644 --- a/frappe/public/js/frappe/form/form.js +++ b/frappe/public/js/frappe/form/form.js @@ -1944,21 +1944,24 @@ frappe.ui.form.Form = class FrappeForm { let docname = this.docname; frappe.socketio.doc_subscribe(doctype, docname); - - if (frappe.socketio.is_docinfo_listener_setup) { - return; - } - + frappe.realtime.off("docinfo_update"); frappe.realtime.on("docinfo_update", ({ doc, key, action = "update" }) => { - let doc_list = frappe.model.docinfo[doctype][docname][key] || []; - if (action === "add") { - frappe.model.docinfo[doctype][docname][key].push(doc); + if ( + !doc.reference_doctype || + !doc.reference_name || + doc.reference_doctype !== doctype || + doc.reference_name !== docname + ) { + return; } - + let doc_list = frappe.model.docinfo[doctype][docname][key] || []; let docindex = doc_list.findIndex((old_doc) => { return old_doc.name === doc.name; }); + if (action === "add") { + frappe.model.docinfo[doctype][docname][key].push(doc); + } if (docindex > -1) { if (action === "update") { frappe.model.docinfo[doctype][docname][key].splice(docindex, 1, doc); @@ -1979,7 +1982,6 @@ frappe.ui.form.Form = class FrappeForm { this.timeline && this.timeline.refresh(); } }); - frappe.socketio.is_docinfo_listener_setup = true; } // Filters fields from the reference doctype and sets them as options for a Select field diff --git a/frappe/public/js/frappe/list/list_view.js b/frappe/public/js/frappe/list/list_view.js index a71cdf0a35..c0bd534140 100644 --- a/frappe/public/js/frappe/list/list_view.js +++ b/frappe/public/js/frappe/list/list_view.js @@ -1317,6 +1317,10 @@ frappe.views.ListView = class ListView extends frappe.views.BaseList { } frappe.socketio.list_subscribe(this.doctype); frappe.realtime.on("list_update", (data) => { + if (!frappe.get_doc(data?.doctype, data?.name)?.__unsaved) { + frappe.model.remove_from_locals(data.doctype, data.name); + } + if (this.avoid_realtime_update()) { return; } From 424a7d39bc455d7fd540e874105d11aa23795f9e Mon Sep 17 00:00:00 2001 From: Shariq Ansari Date: Wed, 16 Nov 2022 16:00:24 +0530 Subject: [PATCH 22/47] fix: webform read only field not working --- frappe/public/js/frappe/form/controls/base_input.js | 2 +- frappe/public/js/frappe/web_form/webform_script.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/public/js/frappe/form/controls/base_input.js b/frappe/public/js/frappe/form/controls/base_input.js index 030d35afdb..5910ae0dd7 100644 --- a/frappe/public/js/frappe/form/controls/base_input.js +++ b/frappe/public/js/frappe/form/controls/base_input.js @@ -79,7 +79,7 @@ frappe.ui.form.ControlInput = class ControlInput extends frappe.ui.form.Control if (me.frm) { me.value = frappe.model.get_value(me.doctype, me.docname, me.df.fieldname); } else if (me.doc) { - me.value = me.doc[me.df.fieldname]; + me.value = me.doc[me.df.fieldname] || ""; } if (me.can_write()) { diff --git a/frappe/public/js/frappe/web_form/webform_script.js b/frappe/public/js/frappe/web_form/webform_script.js index f24e10cff3..8a5d123d2f 100644 --- a/frappe/public/js/frappe/web_form/webform_script.js +++ b/frappe/public/js/frappe/web_form/webform_script.js @@ -55,7 +55,7 @@ frappe.ready(function () { function setup_fields(web_form_doc, doc_data) { web_form_doc.web_form_fields.forEach((df) => { df.is_web_form = true; - df.read_only = !web_form_doc.is_new && !web_form_doc.in_edit_mode; + df.read_only = df.read_only || (!web_form_doc.is_new && !web_form_doc.in_edit_mode); if (df.fieldtype === "Table") { df.get_data = () => { let data = []; From 0d5d2cf95c52f07660e91116e3684b831f2f6275 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Wed, 16 Nov 2022 16:15:31 +0530 Subject: [PATCH 23/47] ci: fix flake8 URL (#18895) --- .pre-commit-config.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 0783e94457..e976230244 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -54,8 +54,8 @@ repos: hooks: - id: isort - - repo: https://gitlab.com/pycqa/flake8 - rev: 3.9.2 + - repo: https://github.com/PyCQA/flake8 + rev: 5.0.4 hooks: - id: flake8 additional_dependencies: ['flake8-bugbear',] From 70633573c2b25c296666b8da719d5b53e0cd3ae8 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Wed, 16 Nov 2022 20:48:50 +0530 Subject: [PATCH 24/47] fix: dont convert row format if not required (#18900) --- frappe/installer.py | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/frappe/installer.py b/frappe/installer.py index 2a6c29a17f..0cd9b32063 100644 --- a/frappe/installer.py +++ b/frappe/installer.py @@ -3,8 +3,12 @@ import json import os +import re +import subprocess import sys from collections import OrderedDict +from contextlib import suppress +from shutil import which import click @@ -653,10 +657,22 @@ def convert_archive_content(sql_file_path): if frappe.conf.db_type == "mariadb": # ever since mariaDB 10.6, row_format COMPRESSED has been deprecated and removed # this step is added to ease restoring sites depending on older mariaDB servers + # This change was reverted by mariadb in 10.6.6 + # Ref: https://mariadb.com/kb/en/innodb-compressed-row-format/#read-only from pathlib import Path from frappe.utils import random_string + version = _guess_mariadb_version() + if not version or (version <= (10, 6, 0) or version >= (10, 6, 6)): + return + + click.secho( + "MariaDB version being used does not support ROW_FORMAT=COMPRESSED, " + "converting into DYNAMIC format.", + fg="yellow", + ) + old_sql_file_path = Path(f"{sql_file_path}_{random_string(10)}") sql_file_path = Path(sql_file_path) @@ -684,6 +700,20 @@ def extract_sql_gzip(sql_gz_path): return decompressed_file +def _guess_mariadb_version() -> tuple[int] | None: + # Using command-line because we *might* not have a connection yet and this command is required + # in non-interactive mode. + # Use db.sql("select version()") instead if connection is available. + with suppress(Exception): + mysql = which("mysql") + version_output = subprocess.getoutput(f"{mysql} --version") + version_regex = r"(?P\d+\.\d+\.\d+)-MariaDB" + + version = re.search(version_regex, version_output).group("version") + + return tuple(int(v) for v in version.split(".")) + + def extract_files(site_name, file_path): import shutil import subprocess From 96fee8c2935f1ca0b12650e9260a4425990b8d44 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Wed, 16 Nov 2022 21:53:49 +0530 Subject: [PATCH 25/47] feat: {site}:website room open to all users - Subscribe to room and pass messages without auth - Pass `room='website'` to publish_realtime to use - Pass discussions' comms through particular site's website room --- frappe/templates/discussions/discussions.js | 1 + .../website/doctype/discussion_reply/discussion_reply.py | 4 +++- socketio.js | 8 ++++++++ 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/frappe/templates/discussions/discussions.js b/frappe/templates/discussions/discussions.js index ad6d13e834..fc4c922a83 100644 --- a/frappe/templates/discussions/discussions.js +++ b/frappe/templates/discussions/discussions.js @@ -70,6 +70,7 @@ const show_new_topic_modal = (e) => { const setup_socket_io = () => { frappe.socketio.init(window.socketio_port || "9000"); + frappe.socketio.socket.emit("website"); frappe.socketio.socket.on("publish_message", (data) => { publish_message(data); }); diff --git a/frappe/website/doctype/discussion_reply/discussion_reply.py b/frappe/website/doctype/discussion_reply/discussion_reply.py index 1ac62d3b7d..9c7821b074 100644 --- a/frappe/website/doctype/discussion_reply/discussion_reply.py +++ b/frappe/website/doctype/discussion_reply/discussion_reply.py @@ -9,6 +9,7 @@ class DiscussionReply(Document): def on_update(self): frappe.publish_realtime( event="update_message", + room="website", message={"reply": frappe.utils.md_to_html(self.reply), "reply_name": self.name}, after_commit=True, ) @@ -41,6 +42,7 @@ class DiscussionReply(Document): frappe.publish_realtime( event="publish_message", + room="website", message={ "template": template, "topic_info": topic_info[0], @@ -53,7 +55,7 @@ class DiscussionReply(Document): def after_delete(self): frappe.publish_realtime( - event="delete_message", message={"reply_name": self.name}, after_commit=True + event="delete_message", room="website", message={"reply_name": self.name}, after_commit=True ) diff --git a/socketio.js b/socketio.js index 1bafa79a5c..c0b6b51584 100644 --- a/socketio.js +++ b/socketio.js @@ -61,6 +61,10 @@ io.on("connection", function (socket) { socket.join(get_site_room(socket)); } + socket.on("website", () => { + socket.join(get_website_room(socket)); + }); + socket.on("list_update", function (doctype) { can_subscribe_list({ socket, @@ -217,6 +221,10 @@ function get_site_room(socket) { return get_site_name(socket) + ":all"; } +function get_website_room(socket) { + return get_site_name(socket) + ":website"; +} + function get_doctype_room(socket, doctype) { return get_site_name(socket) + ":doctype:" + doctype; } From 16bd7a2d0b9749c6a9954ca33825ecb695183f76 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Wed, 16 Nov 2022 23:02:23 +0530 Subject: [PATCH 26/47] fix(socketio): Scoping & hoisting bugs Due to the previous logic, cookie data seemed inconsistent causing ghost sessions. --- socketio.js | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/socketio.js b/socketio.js index c0b6b51584..42549571e7 100644 --- a/socketio.js +++ b/socketio.js @@ -25,36 +25,33 @@ io.use((socket, next) => { return; } - const cookies = cookie.parse(socket.request.headers.cookie); + let cookies = cookie.parse(socket.request.headers.cookie); if (!cookies.sid) { next(new Error("No sid transmitted.")); return; } - socket.sid = cookies.sid; - socket.user = cookies.user_id; request .get(get_url(socket, "/api/method/frappe.realtime.get_user_info")) .type("form") .query({ - sid: socket.sid, + sid: cookies.sid, }) .then((res) => { socket.user = res.body.message.user; socket.user_type = res.body.message.user_type; + socket.sid = cookies.sid; + next(); }) .catch((e) => { next(new Error(`Unauthorized: ${e}`)); - return; }); - - next(); }); // on socket connection io.on("connection", function (socket) { - const room = get_user_room(socket); + const room = get_user_room(socket, socket.user); socket.join(room); if (socket.user_type == "System User") { From c3c1848b2abc0cdf233b59b4e410bd70d287ef3d Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Wed, 16 Nov 2022 23:04:46 +0530 Subject: [PATCH 27/47] fix: Restrict socket data to respective users after commit Fix conditions to bother only those who asked for the data: - Clear permissions cache only for updated users' data - Defer appropriate events until commit to avoid ghost events - Remove event unused by desk (and other apps) --- frappe/core/doctype/data_import/importer.py | 2 ++ frappe/core/doctype/user_permission/user_permission.py | 6 +++--- frappe/desk/doctype/notification_log/notification_log.py | 2 +- frappe/email/doctype/email_account/email_account.py | 6 ------ frappe/social/doctype/energy_point_log/energy_point_log.py | 4 +++- 5 files changed, 9 insertions(+), 11 deletions(-) diff --git a/frappe/core/doctype/data_import/importer.py b/frappe/core/doctype/data_import/importer.py index ea90b24a6f..20a8e7db9b 100644 --- a/frappe/core/doctype/data_import/importer.py +++ b/frappe/core/doctype/data_import/importer.py @@ -139,6 +139,7 @@ class Importer: "skipping": True, "data_import": self.data_import.name, }, + user=frappe.session.user, ) continue @@ -166,6 +167,7 @@ class Importer: "row_indexes": row_indexes, "eta": eta, }, + user=frappe.session.user, ) create_import_log( diff --git a/frappe/core/doctype/user_permission/user_permission.py b/frappe/core/doctype/user_permission/user_permission.py index 7020640da4..63c1f40512 100644 --- a/frappe/core/doctype/user_permission/user_permission.py +++ b/frappe/core/doctype/user_permission/user_permission.py @@ -18,11 +18,11 @@ class UserPermission(Document): def on_update(self): frappe.cache().hdel("user_permissions", self.user) - frappe.publish_realtime("update_user_permissions") + frappe.publish_realtime("update_user_permissions", user=self.user, after_commit=True) - def on_trash(self): # pylint: disable=no-self-use + def on_trash(self): frappe.cache().hdel("user_permissions", self.user) - frappe.publish_realtime("update_user_permissions") + frappe.publish_realtime("update_user_permissions", user=self.user, after_commit=True) def validate_user_permission(self): """checks for duplicate user permission records""" diff --git a/frappe/desk/doctype/notification_log/notification_log.py b/frappe/desk/doctype/notification_log/notification_log.py index 4d82932555..6367e0f45f 100644 --- a/frappe/desk/doctype/notification_log/notification_log.py +++ b/frappe/desk/doctype/notification_log/notification_log.py @@ -176,7 +176,7 @@ def mark_as_read(docname): @frappe.whitelist() def trigger_indicator_hide(): - frappe.publish_realtime("indicator_hide", user=frappe.session.user) + frappe.publish_realtime("indicator_hide", user=frappe.session.user, after_commit=True) def set_notifications_as_unseen(user): diff --git a/frappe/email/doctype/email_account/email_account.py b/frappe/email/doctype/email_account/email_account.py index c69d653c96..1db45604e1 100755 --- a/frappe/email/doctype/email_account/email_account.py +++ b/frappe/email/doctype/email_account/email_account.py @@ -486,12 +486,6 @@ class EmailAccount(Document): else: frappe.db.commit() - # notify if user is linked to account - if len(inbound_mails) > 0 and not frappe.local.flags.in_test: - frappe.publish_realtime( - "new_email", {"account": self.email_account_name, "number": len(inbound_mails)} - ) - if exceptions: raise Exception(frappe.as_json(exceptions)) diff --git a/frappe/social/doctype/energy_point_log/energy_point_log.py b/frappe/social/doctype/energy_point_log/energy_point_log.py index 487f8225f5..658d333c44 100644 --- a/frappe/social/doctype/energy_point_log/energy_point_log.py +++ b/frappe/social/doctype/energy_point_log/energy_point_log.py @@ -34,7 +34,9 @@ class EnergyPointLog(Document): def after_insert(self): alert_dict = get_alert_dict(self) if alert_dict: - frappe.publish_realtime("energy_point_alert", message=alert_dict, user=self.user) + frappe.publish_realtime( + "energy_point_alert", message=alert_dict, user=self.user, after_commit=True + ) frappe.cache().hdel("energy_points", self.user) From 190e01a5f394fd787115540616e5368f408e7c06 Mon Sep 17 00:00:00 2001 From: Athul Cyriac Ajay Date: Thu, 17 Nov 2022 11:18:20 +0530 Subject: [PATCH 28/47] fix: Force integer type in request.max_content_length --- frappe/app.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/app.py b/frappe/app.py index 0d7fdc1fe1..7927e834ed 100644 --- a/frappe/app.py +++ b/frappe/app.py @@ -112,7 +112,7 @@ def init_request(request): else: frappe.connect(set_admin_as_user=False) - request.max_content_length = frappe.local.conf.get("max_file_size") or 10 * 1024 * 1024 + request.max_content_length = int(frappe.local.conf.get("max_file_size")) or 10 * 1024 * 1024 make_form_dict(request) From 1f6f31fc972fd62b689c4c0495a98da5f9a11828 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Thu, 17 Nov 2022 11:21:51 +0530 Subject: [PATCH 29/47] refactor: int > cint --- frappe/app.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/app.py b/frappe/app.py index 7927e834ed..fc679aa44e 100644 --- a/frappe/app.py +++ b/frappe/app.py @@ -21,7 +21,7 @@ from frappe import _ from frappe.auth import SAFE_HTTP_METHODS, UNSAFE_HTTP_METHODS, HTTPRequest from frappe.core.doctype.comment.comment import update_comments_in_parent_after_request from frappe.middlewares import StaticDataMiddleware -from frappe.utils import get_site_name, sanitize_html +from frappe.utils import cint, get_site_name, sanitize_html from frappe.utils.error import make_error_snapshot from frappe.website.serve import get_response @@ -112,7 +112,7 @@ def init_request(request): else: frappe.connect(set_admin_as_user=False) - request.max_content_length = int(frappe.local.conf.get("max_file_size")) or 10 * 1024 * 1024 + request.max_content_length = cint(frappe.local.conf.get("max_file_size")) or 10 * 1024 * 1024 make_form_dict(request) From 64289308575fe2a96b4045a8d9cd13631d294bb3 Mon Sep 17 00:00:00 2001 From: Jannat Patel <31363128+pateljannat@users.noreply.github.com> Date: Thu, 17 Nov 2022 11:39:43 +0530 Subject: [PATCH 30/47] fix: security issue in discussions component (#18903) [skip ci] --- frappe/website/doctype/discussion_reply/discussion_reply.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/frappe/website/doctype/discussion_reply/discussion_reply.py b/frappe/website/doctype/discussion_reply/discussion_reply.py index 1ac62d3b7d..f4460160c1 100644 --- a/frappe/website/doctype/discussion_reply/discussion_reply.py +++ b/frappe/website/doctype/discussion_reply/discussion_reply.py @@ -59,4 +59,6 @@ class DiscussionReply(Document): @frappe.whitelist() def delete_message(reply_name): - frappe.delete_doc("Discussion Reply", reply_name, ignore_permissions=True) + owner = frappe.db.get_value("Discussion Reply", reply_name, "owner") + if owner == frappe.session.user: + frappe.delete_doc("Discussion Reply", reply_name) From c658d8cb1b32a2ac3b0fa1a84040864f2db6a532 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Thu, 17 Nov 2022 11:50:18 +0530 Subject: [PATCH 31/47] fix: ignore unpicklable hooks (#18902) If any custom app use import statement in hooks.py everything breaks. Hooks.py while being python file is still only supposed to be used for configuring. This PR ignores unpicklable members of hooks.py --- frappe/__init__.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index 84a27642a9..2d491ca068 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -1432,6 +1432,8 @@ def get_doc_hooks(): @request_cache def _load_app_hooks(app_name: str | None = None): + import types + hooks = {} apps = [app_name] if app_name else get_installed_apps(sort=True) @@ -1447,9 +1449,13 @@ def _load_app_hooks(app_name: str | None = None): if not request: raise SystemExit raise - for key in dir(app_hooks): + + def _is_valid_hook(obj): + return not isinstance(obj, (types.ModuleType, types.FunctionType, type)) + + for key, value in inspect.getmembers(app_hooks, predicate=_is_valid_hook): if not key.startswith("_"): - append_hook(hooks, key, getattr(app_hooks, key)) + append_hook(hooks, key, value) return hooks From f3c00c2bdcc963dc708716224d83de6b3686bf04 Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Thu, 17 Nov 2022 07:45:35 +0000 Subject: [PATCH 32/47] perf: dont fetch meta unless required (#18907) --- frappe/desk/search.py | 9 +++++++-- frappe/model/db_query.py | 6 +++--- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/frappe/desk/search.py b/frappe/desk/search.py index b820fabd6d..4843219179 100644 --- a/frappe/desk/search.py +++ b/frappe/desk/search.py @@ -206,11 +206,16 @@ def search_widget( ) order_by = f"_relevance, {order_by}" - ptype = "select" if frappe.only_has_select_perm(doctype) else "read" ignore_permissions = ( True if doctype == "DocType" - else (cint(ignore_user_permissions) and has_permission(doctype, ptype=ptype)) + else ( + cint(ignore_user_permissions) + and has_permission( + doctype, + ptype="select" if frappe.only_has_select_perm(doctype) else "read", + ) + ) ) values = frappe.get_list( diff --git a/frappe/model/db_query.py b/frappe/model/db_query.py index 3e6b8ec753..e689f91ddd 100644 --- a/frappe/model/db_query.py +++ b/frappe/model/db_query.py @@ -455,10 +455,10 @@ class DatabaseQuery: ) def check_read_permission(self, doctype): - ptype = "select" if frappe.only_has_select_perm(doctype) else "read" - if not self.flags.ignore_permissions and not frappe.has_permission( - doctype, ptype=ptype, parent_doctype=self.doctype + doctype, + ptype="select" if frappe.only_has_select_perm(doctype) else "read", + parent_doctype=self.doctype, ): frappe.flags.error_message = _("Insufficient Permission for {0}").format(frappe.bold(doctype)) raise frappe.PermissionError(doctype) From 8e0e2dad51d78c08a0b5b6dfc90f90a70a778571 Mon Sep 17 00:00:00 2001 From: Shariq Ansari Date: Thu, 17 Nov 2022 13:59:56 +0530 Subject: [PATCH 33/47] fix: (ctrl/cmd)+click for clickable divs --- frappe/public/js/frappe/router.js | 9 +++++++-- frappe/public/js/frappe/ui/page.js | 7 ++++++- frappe/public/js/frappe/ui/toolbar/awesome_bar.js | 4 ++++ .../public/js/frappe/widgets/quick_list_widget.js | 14 ++++++++++++-- frappe/public/js/frappe/widgets/shortcut_widget.js | 7 ++++++- 5 files changed, 35 insertions(+), 6 deletions(-) diff --git a/frappe/public/js/frappe/router.js b/frappe/public/js/frappe/router.js index 2005a46cfe..b13779bab2 100644 --- a/frappe/public/js/frappe/router.js +++ b/frappe/public/js/frappe/router.js @@ -12,6 +12,7 @@ frappe.route_history = []; frappe.view_factory = {}; frappe.view_factories = []; frappe.route_options = null; +frappe.open_in_new_tab = false; frappe.route_hooks = {}; $(window).on("hashchange", function (e) { @@ -347,8 +348,12 @@ frappe.router = { let sub_path = this.make_url(route); // replace each # occurrences in the URL with encoded character except for last // sub_path = sub_path.replace(/[#](?=.*[#])/g, "%23"); - this.push_state(sub_path); - + if (frappe.open_in_new_tab) { + window.open(sub_path, "_blank"); + frappe.open_in_new_tab = false; + } else { + this.push_state(sub_path); + } setTimeout(() => { frappe.after_ajax && frappe.after_ajax(() => { diff --git a/frappe/public/js/frappe/ui/page.js b/frappe/public/js/frappe/ui/page.js index e381f332a9..2cb4f41038 100644 --- a/frappe/public/js/frappe/ui/page.js +++ b/frappe/public/js/frappe/ui/page.js @@ -463,7 +463,12 @@ frappe.ui.Page = class Page { `); } - $link = $li.find("a").on("click", click); + $link = $li.find("a").on("click", (e) => { + if (e.ctrlKey || e.metaKey) { + frappe.open_in_new_tab = true; + } + return click(); + }); if (standard) { $li.appendTo(parent); diff --git a/frappe/public/js/frappe/ui/toolbar/awesome_bar.js b/frappe/public/js/frappe/ui/toolbar/awesome_bar.js index e8eaf25412..4a3c88403f 100644 --- a/frappe/public/js/frappe/ui/toolbar/awesome_bar.js +++ b/frappe/public/js/frappe/ui/toolbar/awesome_bar.js @@ -107,6 +107,10 @@ frappe.search.AwesomeBar = class AwesomeBar { if (item.onclick) { item.onclick(item.match); } else { + let event = o.originalEvent; + if (event.ctrlKey || event.metaKey) { + frappe.open_in_new_tab = true; + } frappe.set_route(item.route); } $input.val(""); diff --git a/frappe/public/js/frappe/widgets/quick_list_widget.js b/frappe/public/js/frappe/widgets/quick_list_widget.js index dbb26cb62c..a406075af2 100644 --- a/frappe/public/js/frappe/widgets/quick_list_widget.js +++ b/frappe/public/js/frappe/widgets/quick_list_widget.js @@ -161,7 +161,10 @@ export default class QuickListWidget extends Widget { $quick_list_item ); - $quick_list_item.click(() => { + $quick_list_item.click((e) => { + if (e.ctrlKey || e.metaKey) { + frappe.open_in_new_tab = true; + } frappe.set_route(`${frappe.utils.get_form_link(this.document_type, doc.name)}`); }); @@ -243,7 +246,14 @@ export default class QuickListWidget extends Widget { } let route = frappe.utils.generate_route({ type: "doctype", name: this.document_type }); this.see_all_button = $(` - ${__("View List")} +
${__("View List")}
`).appendTo(this.footer); + + this.see_all_button.click((e) => { + if (e.ctrlKey || e.metaKey) { + frappe.open_in_new_tab = true; + } + frappe.set_route(route); + }); } } diff --git a/frappe/public/js/frappe/widgets/shortcut_widget.js b/frappe/public/js/frappe/widgets/shortcut_widget.js index 5a4c7fdb88..d82f63a035 100644 --- a/frappe/public/js/frappe/widgets/shortcut_widget.js +++ b/frappe/public/js/frappe/widgets/shortcut_widget.js @@ -24,7 +24,7 @@ export default class ShortcutWidget extends Widget { } setup_events() { - this.widget.click(() => { + this.widget.click((e) => { if (this.in_customize_mode) return; let route = frappe.utils.generate_route({ @@ -40,6 +40,11 @@ export default class ShortcutWidget extends Widget { if (this.type == "DocType" && filters) { frappe.route_options = filters; } + + if (e.ctrlKey || e.metaKey) { + frappe.open_in_new_tab = true; + } + frappe.set_route(route); }); } From edf01ee1cd56147b5f57cf5345f943e82f258a1d Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Thu, 17 Nov 2022 14:43:03 +0530 Subject: [PATCH 34/47] fix(file): attached_to_name can be an integer (#18909) * fix(file): attached_to_name can be an integer incorrect validation introduces via https://github.com/frappe/frappe/pull/18880 * test(file): set correct fieldname * fix: check for str, int explicitly --- frappe/core/doctype/file/file.py | 4 ++-- frappe/core/doctype/file/test_file.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/frappe/core/doctype/file/file.py b/frappe/core/doctype/file/file.py index 0e28145c9a..0c21501589 100755 --- a/frappe/core/doctype/file/file.py +++ b/frappe/core/doctype/file/file.py @@ -101,8 +101,8 @@ class File(Document): if not self.attached_to_doctype: return - if self.attached_to_name and not isinstance(self.attached_to_name, str): - frappe.throw(_("Attached To Name must be a string"), TypeError) + if not self.attached_to_name or not isinstance(self.attached_to_name, (str, int)): + frappe.throw(_("Attached To Name must be a string or an integer"), frappe.ValidationError) if not self.attached_to_field: return diff --git a/frappe/core/doctype/file/test_file.py b/frappe/core/doctype/file/test_file.py index ed97c5683d..86bd69eb5f 100644 --- a/frappe/core/doctype/file/test_file.py +++ b/frappe/core/doctype/file/test_file.py @@ -85,7 +85,7 @@ class TestBase64File(FrappeTestCase): "doctype": "File", "file_name": "test_base64.txt", "attached_to_doctype": self.attached_to_doctype, - "attached_to_docname": self.attached_to_docname, + "attached_to_name": self.attached_to_docname, "content": self.test_content, "decode": True, } From 54a85d0636e350705e78b3725025f0dc8e2c6413 Mon Sep 17 00:00:00 2001 From: Rohit Waghchaure Date: Thu, 17 Nov 2022 15:05:51 +0530 Subject: [PATCH 35/47] fix: grid column indicators not working --- frappe/public/js/frappe/form/grid_row.js | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/frappe/public/js/frappe/form/grid_row.js b/frappe/public/js/frappe/form/grid_row.js index 4a30ad68e0..707796b378 100644 --- a/frappe/public/js/frappe/form/grid_row.js +++ b/frappe/public/js/frappe/form/grid_row.js @@ -649,13 +649,19 @@ export default class GridRow { this.search_columns = {}; this.grid.setup_visible_columns(); + let fields = + this.grid.user_defined_columns && this.grid.user_defined_columns.length > 0 + ? this.grid.user_defined_columns + : this.docfields; + this.grid.visible_columns.forEach((col, ci) => { // to get update df for the row - let df = this.docfields.find((field) => field.fieldname === col[0].fieldname); + let df = fields.find((field) => field.fieldname === col[0].fieldname); this.set_dependant_property(df); let colsize = col[1]; + let txt = this.doc ? frappe.format(this.doc[df.fieldname], df, null, this.doc) : __(df.label); @@ -1346,7 +1352,12 @@ export default class GridRow { } } refresh_field(fieldname, txt) { - let df = this.docfields.find((col) => { + let fields = + this.grid.user_defined_columns && this.grid.user_defined_columns.length > 0 + ? this.grid.user_defined_columns + : this.docfields; + + let df = fields.find((col) => { return col.fieldname === fieldname; }); From 862a5a398dd26c1226102b7cd42f3fafbbc8cdae Mon Sep 17 00:00:00 2001 From: gavin Date: Thu, 17 Nov 2022 15:18:48 +0530 Subject: [PATCH 36/47] fix(socketio): Revert irrelevant & unused changes Co-authored-by: Ankush Menat --- frappe/desk/doctype/notification_log/notification_log.py | 2 +- frappe/public/js/frappe/model/model.js | 5 ----- frappe/public/js/frappe/socketio_client.js | 2 -- frappe/realtime.py | 3 --- 4 files changed, 1 insertion(+), 11 deletions(-) diff --git a/frappe/desk/doctype/notification_log/notification_log.py b/frappe/desk/doctype/notification_log/notification_log.py index 6367e0f45f..4d82932555 100644 --- a/frappe/desk/doctype/notification_log/notification_log.py +++ b/frappe/desk/doctype/notification_log/notification_log.py @@ -176,7 +176,7 @@ def mark_as_read(docname): @frappe.whitelist() def trigger_indicator_hide(): - frappe.publish_realtime("indicator_hide", user=frappe.session.user, after_commit=True) + frappe.publish_realtime("indicator_hide", user=frappe.session.user) def set_notifications_as_unseen(user): diff --git a/frappe/public/js/frappe/model/model.js b/frappe/public/js/frappe/model/model.js index bcd6b297b2..99553f470c 100644 --- a/frappe/public/js/frappe/model/model.js +++ b/frappe/public/js/frappe/model/model.js @@ -79,10 +79,6 @@ $.extend(frappe.model, { init: function () { // setup refresh if the document is updated somewhere else - if (frappe.socketio.is_document_listener_setup) { - return; - } - frappe.realtime.on("doc_update", function (data) { var doc = locals[data.doctype] && locals[data.doctype][data.name]; @@ -108,7 +104,6 @@ $.extend(frappe.model, { } } }); - frappe.socketio.is_document_listener_setup = true; }, is_value_type: function (fieldtype) { diff --git a/frappe/public/js/frappe/socketio_client.js b/frappe/public/js/frappe/socketio_client.js index d4f8fec87f..1ac875544c 100644 --- a/frappe/public/js/frappe/socketio_client.js +++ b/frappe/public/js/frappe/socketio_client.js @@ -3,8 +3,6 @@ frappe.socketio = { open_tasks: {}, open_docs: [], emit_queue: [], - is_docinfo_listener_setup: false, - is_document_listener_setup: false, init: function (port = 3000) { if (frappe.boot.disable_async) { diff --git a/frappe/realtime.py b/frappe/realtime.py index 1ab3a74238..68809c63f3 100644 --- a/frappe/realtime.py +++ b/frappe/realtime.py @@ -119,9 +119,6 @@ def can_subscribe_doc(doctype, docname): @frappe.whitelist(allow_guest=True) def can_subscribe_list(doctype): - if os.environ.get("CI"): - return True - from frappe.exceptions import PermissionError if not frappe.has_permission(user=frappe.session.user, doctype=doctype, ptype="read"): From e78c74cbc16fdbdc07113528584bf5507cacc894 Mon Sep 17 00:00:00 2001 From: Shariq Ansari Date: Thu, 17 Nov 2022 16:19:35 +0530 Subject: [PATCH 37/47] feat: inline doc link for each field --- frappe/core/doctype/docfield/docfield.json | 11 +++++- frappe/public/icons/timeless/icons.svg | 6 +++ .../js/frappe/form/controls/base_input.js | 37 +++++++++++++------ .../public/js/frappe/form/controls/check.js | 1 + frappe/public/js/frappe/form/grid.js | 13 +++++++ 5 files changed, 54 insertions(+), 14 deletions(-) diff --git a/frappe/core/doctype/docfield/docfield.json b/frappe/core/doctype/docfield/docfield.json index 803ad3c140..dbdedf4bac 100644 --- a/frappe/core/doctype/docfield/docfield.json +++ b/frappe/core/doctype/docfield/docfield.json @@ -70,6 +70,7 @@ "columns", "column_break_22", "description", + "doc_url", "oldfieldname", "oldfieldtype" ], @@ -541,13 +542,19 @@ "fieldname": "is_virtual", "fieldtype": "Check", "label": "Virtual" + }, + { + "depends_on": "eval:!in_list([\"Tab Break\", \"Section Break\", \"Column Break\", \"Button\", \"HTML\"], doc.fieldtype)", + "fieldname": "doc_url", + "fieldtype": "Data", + "label": "Doc URL" } ], "idx": 1, "index_web_pages_for_search": 1, "istable": 1, "links": [], - "modified": "2022-04-19 12:27:28.641580", + "modified": "2022-11-17 14:14:39.404696", "modified_by": "Administrator", "module": "Core", "name": "DocField", @@ -557,4 +564,4 @@ "sort_field": "modified", "sort_order": "ASC", "states": [] -} +} \ No newline at end of file diff --git a/frappe/public/icons/timeless/icons.svg b/frappe/public/icons/timeless/icons.svg index af63d0c8ff..663eb6da48 100644 --- a/frappe/public/icons/timeless/icons.svg +++ b/frappe/public/icons/timeless/icons.svg @@ -859,6 +859,12 @@ + + + + + + ').appendTo(this.parent); } else { this.$wrapper = $( - '
\ -
\ -
\ - \ -
\ -
\ -
\ - \ -

\ -
\ -
\ -
' + `
+
+
+ + +
+
+
+ +

+
+
+
` ).appendTo(this.parent); } } @@ -104,6 +105,7 @@ frappe.ui.form.ControlInput = class ControlInput extends frappe.ui.form.Control me.set_description(); me.set_label(); + me.set_doc_url(); me.set_mandatory(me.value); me.set_bold(); me.set_required(); @@ -141,6 +143,17 @@ frappe.ui.form.ControlInput = class ControlInput extends frappe.ui.form.Control (icon ? ' ' : "") + __(this.df.label) || " "; this._label = this.df.label; } + + set_doc_url() { + if (!this.df?.doc_url) return; + + let $help = this.$wrapper.find("span.help"); + $help.empty(); + $(` + ${frappe.utils.icon("help", "sm")} + `).appendTo($help); + } + set_description(description) { if (description !== undefined) { this.df.description = description; diff --git a/frappe/public/js/frappe/form/controls/check.js b/frappe/public/js/frappe/form/controls/check.js index 91409ce4e0..46c09b6139 100644 --- a/frappe/public/js/frappe/form/controls/check.js +++ b/frappe/public/js/frappe/form/controls/check.js @@ -8,6 +8,7 @@ frappe.ui.form.ControlCheck = class ControlCheck extends frappe.ui.form.ControlD +

diff --git a/frappe/public/js/frappe/form/grid.js b/frappe/public/js/frappe/form/grid.js index 2ffba97efa..f6129de25f 100644 --- a/frappe/public/js/frappe/form/grid.js +++ b/frappe/public/js/frappe/form/grid.js @@ -62,6 +62,7 @@ export default class Grid { make() { let template = ` +

@@ -119,6 +120,7 @@ export default class Grid { this.wrapper = $(template).appendTo(this.parent); $(this.parent).addClass("form-group"); this.set_grid_description(); + this.set_doc_url(); frappe.utils.bind_actions_with_object(this.wrapper, this); @@ -148,6 +150,17 @@ export default class Grid { description_wrapper.hide(); } } + + set_doc_url() { + if (!this.df?.doc_url) return; + + let $help = $(this.parent).find("span.help"); + $help.empty(); + $(` + ${frappe.utils.icon("help", "sm")} + `).appendTo($help); + } + setup_grid_pagination() { this.grid_pagination = new GridPagination({ grid: this, From dcdc22b53f2b3c41a6ff2a94c08774ca7e31e37d Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Thu, 17 Nov 2022 16:23:57 +0530 Subject: [PATCH 38/47] build: pin pyopenssl (cherry picked from commit f70db1f5397e1b1795cc3ac957bb9b2519d4341f) # Conflicts: # pyproject.toml --- pyproject.toml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pyproject.toml b/pyproject.toml index 6506bfa13c..3a468f966b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -48,6 +48,11 @@ dependencies = [ "premailer~=3.8.0", "psutil~=5.9.1", "psycopg2-binary~=2.9.1", +<<<<<<< HEAD +======= + "pyasn1~=0.4.8", + "pyOpenSSL~=22.1.0", +>>>>>>> f70db1f539 (build: pin pyopenssl) "pycryptodome~=3.10.1", "pyotp~=2.6.0", "python-dateutil~=2.8.1", From 9dbccc98cb978be4eea0799bffccaf797db313c8 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Thu, 17 Nov 2022 16:47:42 +0530 Subject: [PATCH 39/47] chore: conflicts --- pyproject.toml | 4 ---- 1 file changed, 4 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 3a468f966b..f5ab8c94b2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -48,11 +48,7 @@ dependencies = [ "premailer~=3.8.0", "psutil~=5.9.1", "psycopg2-binary~=2.9.1", -<<<<<<< HEAD -======= - "pyasn1~=0.4.8", "pyOpenSSL~=22.1.0", ->>>>>>> f70db1f539 (build: pin pyopenssl) "pycryptodome~=3.10.1", "pyotp~=2.6.0", "python-dateutil~=2.8.1", From 40df601a738053950df4063d5575a45f6d3f8d41 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 17 Nov 2022 16:37:28 +0530 Subject: [PATCH 40/47] fix: Auto-add all users to website room Keep website room open for all - currently only used for discussions --- frappe/realtime.py | 4 ++++ frappe/templates/discussions/discussions.js | 1 - .../doctype/discussion_reply/discussion_reply.py | 10 +++++++--- socketio.js | 8 ++------ 4 files changed, 13 insertions(+), 10 deletions(-) diff --git a/frappe/realtime.py b/frappe/realtime.py index 68809c63f3..1d75f9d9e6 100644 --- a/frappe/realtime.py +++ b/frappe/realtime.py @@ -157,3 +157,7 @@ def get_site_room(): def get_task_progress_room(task_id): return f"{frappe.local.site}:task_progress:{task_id}" + + +def get_website_room(): + return f"{frappe.local.site}:website" diff --git a/frappe/templates/discussions/discussions.js b/frappe/templates/discussions/discussions.js index fc4c922a83..ad6d13e834 100644 --- a/frappe/templates/discussions/discussions.js +++ b/frappe/templates/discussions/discussions.js @@ -70,7 +70,6 @@ const show_new_topic_modal = (e) => { const setup_socket_io = () => { frappe.socketio.init(window.socketio_port || "9000"); - frappe.socketio.socket.emit("website"); frappe.socketio.socket.on("publish_message", (data) => { publish_message(data); }); diff --git a/frappe/website/doctype/discussion_reply/discussion_reply.py b/frappe/website/doctype/discussion_reply/discussion_reply.py index 5f4a8473d7..17903ace2d 100644 --- a/frappe/website/doctype/discussion_reply/discussion_reply.py +++ b/frappe/website/doctype/discussion_reply/discussion_reply.py @@ -3,13 +3,14 @@ import frappe from frappe.model.document import Document +from frappe.realtime import get_website_room class DiscussionReply(Document): def on_update(self): frappe.publish_realtime( event="update_message", - room="website", + room=get_website_room(), message={"reply": frappe.utils.md_to_html(self.reply), "reply_name": self.name}, after_commit=True, ) @@ -42,7 +43,7 @@ class DiscussionReply(Document): frappe.publish_realtime( event="publish_message", - room="website", + room=get_website_room(), message={ "template": template, "topic_info": topic_info[0], @@ -55,7 +56,10 @@ class DiscussionReply(Document): def after_delete(self): frappe.publish_realtime( - event="delete_message", room="website", message={"reply_name": self.name}, after_commit=True + event="delete_message", + room=get_website_room(), + message={"reply_name": self.name}, + after_commit=True, ) diff --git a/socketio.js b/socketio.js index 42549571e7..242100baae 100644 --- a/socketio.js +++ b/socketio.js @@ -51,17 +51,13 @@ io.use((socket, next) => { // on socket connection io.on("connection", function (socket) { - const room = get_user_room(socket, socket.user); - socket.join(room); + socket.join(get_user_room(socket, socket.user)); + socket.join(get_website_room(socket)); if (socket.user_type == "System User") { socket.join(get_site_room(socket)); } - socket.on("website", () => { - socket.join(get_website_room(socket)); - }); - socket.on("list_update", function (doctype) { can_subscribe_list({ socket, From c686671b159f1f7a99c16f0d50df9dc8a50abe8e Mon Sep 17 00:00:00 2001 From: Shariq Ansari Date: Thu, 17 Nov 2022 17:42:28 +0530 Subject: [PATCH 41/47] fix: set route options in local storage if open in new tab --- frappe/public/js/frappe/router.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/frappe/public/js/frappe/router.js b/frappe/public/js/frappe/router.js index b13779bab2..f6301085d7 100644 --- a/frappe/public/js/frappe/router.js +++ b/frappe/public/js/frappe/router.js @@ -349,6 +349,7 @@ frappe.router = { // replace each # occurrences in the URL with encoded character except for last // sub_path = sub_path.replace(/[#](?=.*[#])/g, "%23"); if (frappe.open_in_new_tab) { + localStorage["route_options"] = JSON.stringify(frappe.route_options); window.open(sub_path, "_blank"); frappe.open_in_new_tab = false; } else { @@ -498,6 +499,11 @@ frappe.router = { frappe.route_options = {}; } + if (localStorage.getItem("route_options")) { + frappe.route_options = JSON.parse(localStorage.getItem("route_options")); + localStorage.removeItem("route_options"); + } + let params = new URLSearchParams(query_string); for (const [key, value] of params) { frappe.route_options[key] = value; From f488a4953fb0b719c4b7098c0396f58818fb6a0d Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Thu, 17 Nov 2022 18:17:22 +0530 Subject: [PATCH 42/47] chore: validate ui test helpers at runtime (#18922) [skip ci] --- frappe/tests/ui_test_helpers.py | 59 +++++++++++++++++---------------- 1 file changed, 30 insertions(+), 29 deletions(-) diff --git a/frappe/tests/ui_test_helpers.py b/frappe/tests/ui_test_helpers.py index ecb6b4da97..e32b13a18a 100644 --- a/frappe/tests/ui_test_helpers.py +++ b/frappe/tests/ui_test_helpers.py @@ -5,7 +5,14 @@ from frappe.utils import add_to_date, now UI_TEST_USER = "frappe@example.com" -@frappe.whitelist() +def whitelist_for_tests(fn): + if frappe.request and not (frappe.flags.in_test or getattr(frappe.local, "dev_server", 0)): + frappe.throw("Cannot run UI tests. Use a development server with `bench start`") + + return frappe.whitelist()(fn) + + +@whitelist_for_tests def create_if_not_exists(doc): """Create records if they dont exist. Will check for uniqueness by checking if a record exists with these field value pairs @@ -13,9 +20,6 @@ def create_if_not_exists(doc): :param doc: dict of field value pairs. can be a list of dict for multiple records. """ - if not frappe.local.dev_server: - frappe.throw(_("This method can only be accessed in development"), frappe.PermissionError) - doc = frappe.parse_json(doc) if not isinstance(doc, list): @@ -38,7 +42,7 @@ def create_if_not_exists(doc): return names -@frappe.whitelist() +@whitelist_for_tests def create_todo_records(): frappe.db.truncate("ToDo") @@ -72,16 +76,13 @@ def create_todo_records(): ).insert() -@frappe.whitelist() +@whitelist_for_tests def clear_notes(): - if not frappe.local.dev_server: - frappe.throw(_("Not allowed"), frappe.PermissionError) - for note in frappe.get_all("Note", pluck="name"): frappe.delete_doc("Note", note, force=True) -@frappe.whitelist() +@whitelist_for_tests def create_communication_record(): doc = frappe.get_doc( { @@ -95,7 +96,7 @@ def create_communication_record(): return doc -@frappe.whitelist() +@whitelist_for_tests def setup_workflow(): from frappe.workflow.doctype.workflow.test_workflow import create_todo_workflow @@ -104,7 +105,7 @@ def setup_workflow(): frappe.clear_cache() -@frappe.whitelist() +@whitelist_for_tests def create_contact_phone_nos_records(): if frappe.get_all("Contact", {"first_name": "Test Contact"}): return @@ -116,7 +117,7 @@ def create_contact_phone_nos_records(): doc.insert() -@frappe.whitelist() +@whitelist_for_tests def create_doctype(name, fields): fields = frappe.parse_json(fields) if frappe.db.exists("DocType", name): @@ -133,7 +134,7 @@ def create_doctype(name, fields): ).insert() -@frappe.whitelist() +@whitelist_for_tests def create_child_doctype(name, fields): fields = frappe.parse_json(fields) if frappe.db.exists("DocType", name): @@ -151,7 +152,7 @@ def create_child_doctype(name, fields): ).insert() -@frappe.whitelist() +@whitelist_for_tests def create_contact_records(): if frappe.get_all("Contact", {"first_name": "Test Form Contact 1"}): return @@ -161,7 +162,7 @@ def create_contact_records(): insert_contact("Test Form Contact 3", "12345") -@frappe.whitelist() +@whitelist_for_tests def create_multiple_todo_records(): if frappe.get_all("ToDo", {"description": "Multiple ToDo 1"}): return @@ -177,7 +178,7 @@ def insert_contact(first_name, phone_number): doc.insert() -@frappe.whitelist() +@whitelist_for_tests def create_form_tour(): if frappe.db.exists("Form Tour", {"name": "Test Form Tour"}): return @@ -227,7 +228,7 @@ def create_form_tour(): tour.insert() -@frappe.whitelist() +@whitelist_for_tests def create_data_for_discussions(): web_page = create_web_page("Test page for discussions", "test-page-discussions", False) create_topic_and_reply(web_page) @@ -285,7 +286,7 @@ def create_topic_and_reply(web_page): reply.save() -@frappe.whitelist() +@whitelist_for_tests def update_webform_to_multistep(): if not frappe.db.exists("Web Form", "update-profile-duplicate"): doc = frappe.get_doc("Web Form", "edit-profile") @@ -297,7 +298,7 @@ def update_webform_to_multistep(): _doc.save() -@frappe.whitelist() +@whitelist_for_tests def update_child_table(name): doc = frappe.get_doc("DocType", name) if len(doc.fields) == 1: @@ -315,7 +316,7 @@ def update_child_table(name): doc.save() -@frappe.whitelist() +@whitelist_for_tests def insert_doctype_with_child_table_record(name): if frappe.get_all(name, {"title": "Test Grid Search"}): return @@ -361,7 +362,7 @@ def insert_doctype_with_child_table_record(name): doc.insert() -@frappe.whitelist() +@whitelist_for_tests def insert_translations(): translation = [ { @@ -395,7 +396,7 @@ def insert_translations(): frappe.get_doc(doc).insert() -@frappe.whitelist() +@whitelist_for_tests def create_blog_post(): blog_category = frappe.get_doc( @@ -449,7 +450,7 @@ def create_test_user(): user.save() -@frappe.whitelist() +@whitelist_for_tests def setup_tree_doctype(): frappe.delete_doc_if_exists("DocType", "Custom Tree", force=True) @@ -473,7 +474,7 @@ def setup_tree_doctype(): frappe.get_doc({"doctype": "Custom Tree", "tree": "All Trees"}).insert() -@frappe.whitelist() +@whitelist_for_tests def setup_image_doctype(): frappe.delete_doc_if_exists("DocType", "Custom Image", force=True) @@ -492,7 +493,7 @@ def setup_image_doctype(): ).insert() -@frappe.whitelist() +@whitelist_for_tests def setup_inbox(): frappe.db.sql("DELETE FROM `tabUser Email`") @@ -501,7 +502,7 @@ def setup_inbox(): user.save() -@frappe.whitelist() +@whitelist_for_tests def setup_default_view(view, force_reroute=None): frappe.delete_doc_if_exists("Property Setter", "Event-main-default_view") frappe.delete_doc_if_exists("Property Setter", "Event-main-force_re_route_to_default_view") @@ -532,13 +533,13 @@ def setup_default_view(view, force_reroute=None): ).insert() -@frappe.whitelist() +@whitelist_for_tests def create_note(): if not frappe.db.exists("Note", "Routing Test"): frappe.get_doc({"doctype": "Note", "title": "Routing Test"}).insert() -@frappe.whitelist() +@whitelist_for_tests def create_kanban(): if not frappe.db.exists("Custom Field", "Note-kanban"): frappe.get_doc( From dae4d57bed1daa002ba82df73dfb1cdf02157b29 Mon Sep 17 00:00:00 2001 From: Mohammad Hussain Nagaria <34810212+NagariaHussain@users.noreply.github.com> Date: Fri, 18 Nov 2022 13:15:34 +0530 Subject: [PATCH 43/47] fix: get_title must return string (#18926) * which was not the case with autoincrement name rule * only apparent when you set the "Image Field" in form settings --- frappe/public/js/frappe/form/form.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/public/js/frappe/form/form.js b/frappe/public/js/frappe/form/form.js index faebdf3493..767d71d991 100644 --- a/frappe/public/js/frappe/form/form.js +++ b/frappe/public/js/frappe/form/form.js @@ -1750,7 +1750,7 @@ frappe.ui.form.Form = class FrappeForm { if (this.meta.title_field) { return this.doc[this.meta.title_field]; } else { - return this.doc.name; + return String(this.doc.name); } } From 64d39ee18aef7e4726b91363730db67a3a0f8f85 Mon Sep 17 00:00:00 2001 From: Shariq Ansari Date: Fri, 18 Nov 2022 15:04:53 +0530 Subject: [PATCH 44/47] fix: dont show link for fields without label --- frappe/core/doctype/docfield/docfield.json | 8 ++++---- frappe/public/js/frappe/form/controls/base_input.js | 13 +++++++++++-- frappe/public/js/frappe/form/controls/signature.js | 1 + frappe/public/js/frappe/form/grid.js | 13 +++++++++++-- 4 files changed, 27 insertions(+), 8 deletions(-) diff --git a/frappe/core/doctype/docfield/docfield.json b/frappe/core/doctype/docfield/docfield.json index dbdedf4bac..f03c5506d3 100644 --- a/frappe/core/doctype/docfield/docfield.json +++ b/frappe/core/doctype/docfield/docfield.json @@ -70,7 +70,7 @@ "columns", "column_break_22", "description", - "doc_url", + "documentation_url", "oldfieldname", "oldfieldtype" ], @@ -545,9 +545,9 @@ }, { "depends_on": "eval:!in_list([\"Tab Break\", \"Section Break\", \"Column Break\", \"Button\", \"HTML\"], doc.fieldtype)", - "fieldname": "doc_url", - "fieldtype": "Data", - "label": "Doc URL" + "fieldname": "documentation_url", + "fieldtype": "Small Text", + "label": "Documentation URL" } ], "idx": 1, diff --git a/frappe/public/js/frappe/form/controls/base_input.js b/frappe/public/js/frappe/form/controls/base_input.js index da8f5df532..62f1c51f67 100644 --- a/frappe/public/js/frappe/form/controls/base_input.js +++ b/frappe/public/js/frappe/form/controls/base_input.js @@ -145,11 +145,20 @@ frappe.ui.form.ControlInput = class ControlInput extends frappe.ui.form.Control } set_doc_url() { - if (!this.df?.doc_url) return; + let unsupported_fieldtypes = frappe.model.no_value_type.filter( + (x) => frappe.model.table_fields.indexOf(x) === -1 + ); + + if ( + !this.df.label || + !this.df?.documentation_url || + in_list(unsupported_fieldtypes, this.df.fieldtype) + ) + return; let $help = this.$wrapper.find("span.help"); $help.empty(); - $(` + $(` ${frappe.utils.icon("help", "sm")} `).appendTo($help); } diff --git a/frappe/public/js/frappe/form/controls/signature.js b/frappe/public/js/frappe/form/controls/signature.js index 167695ac10..0cbc1f3c26 100644 --- a/frappe/public/js/frappe/form/controls/signature.js +++ b/frappe/public/js/frappe/form/controls/signature.js @@ -8,6 +8,7 @@ frappe.ui.form.ControlSignature = class ControlSignature extends frappe.ui.form. if (this.df.label) { $(this.wrapper).find("label").text(__(this.df.label)); } + this.set_doc_url(); frappe.require("/assets/frappe/js/lib/jSignature.min.js").then(() => { // make jSignature field diff --git a/frappe/public/js/frappe/form/grid.js b/frappe/public/js/frappe/form/grid.js index f6129de25f..d0c352d784 100644 --- a/frappe/public/js/frappe/form/grid.js +++ b/frappe/public/js/frappe/form/grid.js @@ -152,11 +152,20 @@ export default class Grid { } set_doc_url() { - if (!this.df?.doc_url) return; + let unsupported_fieldtypes = frappe.model.no_value_type.filter( + (x) => frappe.model.table_fields.indexOf(x) === -1 + ); + + if ( + !this.df.label || + !this.df?.documentation_url || + in_list(unsupported_fieldtypes, this.df.fieldtype) + ) + return; let $help = $(this.parent).find("span.help"); $help.empty(); - $(` + $(` ${frappe.utils.icon("help", "sm")} `).appendTo($help); } From 799e22a6cf394b357926dbed65b1a7c983c6990e Mon Sep 17 00:00:00 2001 From: Shariq Ansari Date: Fri, 18 Nov 2022 15:31:24 +0530 Subject: [PATCH 45/47] fix: update z-index --- frappe/public/scss/common/awesomeplete.scss | 2 +- frappe/public/scss/desk/global_search.scss | 4 ++-- frappe/public/scss/desk/page.scss | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/frappe/public/scss/common/awesomeplete.scss b/frappe/public/scss/common/awesomeplete.scss index 24e1e3a501..e8fb7dc6ad 100644 --- a/frappe/public/scss/common/awesomeplete.scss +++ b/frappe/public/scss/common/awesomeplete.scss @@ -30,7 +30,7 @@ left: 0; margin: 0; padding: var(--padding-xs); - z-index: 2; + z-index: 4; min-width: 250px; &> li { diff --git a/frappe/public/scss/desk/global_search.scss b/frappe/public/scss/desk/global_search.scss index 396b685d30..4ac797cf44 100644 --- a/frappe/public/scss/desk/global_search.scss +++ b/frappe/public/scss/desk/global_search.scss @@ -11,7 +11,7 @@ width: 100%; .modal-actions { - z-index: 4; + z-index: 5; top: 7px; } @@ -31,7 +31,7 @@ .search-icon { position: absolute; top: 15px; - z-index: 4; + z-index: 5; } } .search-results { diff --git a/frappe/public/scss/desk/page.scss b/frappe/public/scss/desk/page.scss index b0a24eed38..c799d8cc71 100644 --- a/frappe/public/scss/desk/page.scss +++ b/frappe/public/scss/desk/page.scss @@ -84,7 +84,7 @@ } .page-head { - z-index: 4; + z-index: 5; position: sticky; top: var(--navbar-height); background: var(--bg-color); From aa3794a489ac87b28dcb5d5aec0b523f78413759 Mon Sep 17 00:00:00 2001 From: Shariq Ansari Date: Fri, 18 Nov 2022 15:33:23 +0530 Subject: [PATCH 46/47] revert: unused css --- frappe/public/js/frappe/form/controls/data.js | 2 +- frappe/public/scss/common/controls.scss | 4 ---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/frappe/public/js/frappe/form/controls/data.js b/frappe/public/js/frappe/form/controls/data.js index 0f34f151b7..5917a85bdb 100644 --- a/frappe/public/js/frappe/form/controls/data.js +++ b/frappe/public/js/frappe/form/controls/data.js @@ -77,7 +77,7 @@ frappe.ui.form.ControlData = class ControlData extends frappe.ui.form.ControlInp setup_url_field() { this.$wrapper.find(".control-input").append( - ` + ` ${frappe.utils.icon("link-url", "sm")} diff --git a/frappe/public/scss/common/controls.scss b/frappe/public/scss/common/controls.scss index 92bc996775..0f42f6af9d 100644 --- a/frappe/public/scss/common/controls.scss +++ b/frappe/public/scss/common/controls.scss @@ -145,10 +145,6 @@ select.form-control { background-color: none; display: none; } - - .scan-icon { - z-index: 1; - } } .frappe-control:not([data-fieldtype='MultiSelectPills']):not([data-fieldtype='Table MultiSelect']) { From 9bbac0f84fdc9df852240ef4ba354168d6694879 Mon Sep 17 00:00:00 2001 From: Maharshi Patel Date: Fri, 18 Nov 2022 21:31:37 +0530 Subject: [PATCH 47/47] fix: update z-index considering sticky tabs --- frappe/public/scss/desk/global_search.scss | 4 ++-- frappe/public/scss/desk/page.scss | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/frappe/public/scss/desk/global_search.scss b/frappe/public/scss/desk/global_search.scss index 4ac797cf44..de5abb67c7 100644 --- a/frappe/public/scss/desk/global_search.scss +++ b/frappe/public/scss/desk/global_search.scss @@ -11,7 +11,7 @@ width: 100%; .modal-actions { - z-index: 5; + z-index: 6; top: 7px; } @@ -31,7 +31,7 @@ .search-icon { position: absolute; top: 15px; - z-index: 5; + z-index: 6; } } .search-results { diff --git a/frappe/public/scss/desk/page.scss b/frappe/public/scss/desk/page.scss index c799d8cc71..fc77cc2b05 100644 --- a/frappe/public/scss/desk/page.scss +++ b/frappe/public/scss/desk/page.scss @@ -84,7 +84,7 @@ } .page-head { - z-index: 5; + z-index: 6; position: sticky; top: var(--navbar-height); background: var(--bg-color);