From 5700cf7befd7406977ae66381d070f1b4eedc8ca Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Thu, 10 Nov 2022 18:30:25 +0530 Subject: [PATCH 01/64] 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/64] 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 01fdb6a2412dad57587ae284553028d7ded8d5d0 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 10 Nov 2022 14:19:39 +0530 Subject: [PATCH 03/64] 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 04/64] 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 05/64] 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 06/64] 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 07/64] 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 08/64] 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 09/64] 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 10/64] 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 7f94e158ac9317c88dccdf517dd27007604c6f67 Mon Sep 17 00:00:00 2001 From: marination Date: Tue, 15 Nov 2022 14:38:56 +0530 Subject: [PATCH 11/64] fix: Consistency in `get_role_permissions` return value - Return value contains `if_owner` propert in object same as py - Elaborate code documentation --- frappe/public/js/frappe/model/perm.js | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/frappe/public/js/frappe/model/perm.js b/frappe/public/js/frappe/model/perm.js index 6931a2e2e7..f06c95194f 100644 --- a/frappe/public/js/frappe/model/perm.js +++ b/frappe/public/js/frappe/model/perm.js @@ -117,20 +117,38 @@ $.extend(frappe.perm, { }, get_role_permissions: (meta) => { + /** Returns a `dict` of evaluated Role Permissions like: + { + "read": 1, + "write": 0, + "if_owner": [if "if_owner" is enabled] + { + "read": 1, + "write": 0 + } + } */ let perm = [{ read: 0, permlevel: 0 }]; - // Returns a `dict` of evaluated Role Permissions + (meta.permissions || []).forEach((p) => { - // if user has this role let permlevel = cint(p.permlevel); if (!perm[permlevel]) { perm[permlevel] = {}; perm[permlevel]["permlevel"] = permlevel; } + // if user has this role if (frappe.user_roles.includes(p.role)) { frappe.perm.rights.forEach((right) => { let value = perm[permlevel][right] || p[right] || 0; - if (value) { + + if (p.if_owner && value) { + // if_owner is enabled for perm, + // construct perm object inside "if_owner" property + if (!perm[permlevel]["if_owner"]) { + perm[permlevel]["if_owner"] = {} + } + perm[permlevel]["if_owner"][right] = value; + } else if (value) { perm[permlevel][right] = value; } }); From cfc2dd44376d96c13f7eefbc769ff8f2663390fd Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Tue, 15 Nov 2022 14:50:38 +0530 Subject: [PATCH 12/64] 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 3a8fa6cbd5a29d7743e687a9cec96136c83749ee Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 15 Nov 2022 17:49:24 +0530 Subject: [PATCH 13/64] 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 92d9e7d61162b0186485c2c5684bc2e48d4b23ec Mon Sep 17 00:00:00 2001 From: marination Date: Tue, 15 Nov 2022 17:57:49 +0530 Subject: [PATCH 14/64] fix: Don't assign document level perms to doctype level permission store - If `doc` is passed to `has_perm`, checking for pre-stored doctype level perms is wrong - It gives back stale values that don't consider document level permissions and only change on reload (window property) - Get freshly evaluate perms at the doc level - If no `doc` is involved, doctype level custom window property can be used --- frappe/public/js/frappe/model/perm.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/frappe/public/js/frappe/model/perm.js b/frappe/public/js/frappe/model/perm.js index f06c95194f..b9112174e4 100644 --- a/frappe/public/js/frappe/model/perm.js +++ b/frappe/public/js/frappe/model/perm.js @@ -37,11 +37,13 @@ $.extend(frappe.perm, { has_perm: (doctype, permlevel, ptype, doc) => { if (!permlevel) permlevel = 0; - if (!frappe.perm.doctype_perm[doctype]) { - frappe.perm.doctype_perm[doctype] = frappe.perm.get_perm(doctype, doc); + if (!frappe.perm.doctype_perm[doctype] && !doc) { + frappe.perm.doctype_perm[doctype] = frappe.perm.get_perm(doctype); } - let perms = frappe.perm.doctype_perm[doctype]; + // if document object is passed, get fresh doc based perms + // (with ownership and user perms applied) else stale doctype perms + let perms = doc ? frappe.perm.get_perm(doctype, doc) : frappe.perm.doctype_perm[doctype]; if (!perms || !perms[permlevel]) return false; From 88e331e23687c89b73736eb72c980407e9db70c0 Mon Sep 17 00:00:00 2001 From: Maharshi Patel Date: Wed, 16 Nov 2022 12:55:25 +0530 Subject: [PATCH 15/64] 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 16/64] 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 17/64] 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 18/64] 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 19/64] 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 20/64] 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 21/64] 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 22/64] 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 23/64] 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 24/64] 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 25/64] 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 26/64] 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 27/64] 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 28/64] 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 29/64] 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 30/64] 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 31/64] 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 32/64] 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 33/64] 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 34/64] 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 35/64] 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 36/64] 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 37/64] 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 38/64] 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 ff1c9be33b0dc2846e3eb6d461cfd137999bad4f Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Fri, 18 Nov 2022 00:52:38 +0530 Subject: [PATCH 39/64] fix: correct logic for `if_owner` and refactor --- frappe/public/js/frappe/model/perm.js | 100 +++++++++++--------------- 1 file changed, 43 insertions(+), 57 deletions(-) diff --git a/frappe/public/js/frappe/model/perm.js b/frappe/public/js/frappe/model/perm.js index b9112174e4..524a0f5b1a 100644 --- a/frappe/public/js/frappe/model/perm.js +++ b/frappe/public/js/frappe/model/perm.js @@ -37,17 +37,14 @@ $.extend(frappe.perm, { has_perm: (doctype, permlevel, ptype, doc) => { if (!permlevel) permlevel = 0; - if (!frappe.perm.doctype_perm[doctype] && !doc) { - frappe.perm.doctype_perm[doctype] = frappe.perm.get_perm(doctype); - } // if document object is passed, get fresh doc based perms - // (with ownership and user perms applied) else stale doctype perms - let perms = doc ? frappe.perm.get_perm(doctype, doc) : frappe.perm.doctype_perm[doctype]; + // (with ownership and user perms applied) else cached doctype perms + const perms = doc + ? frappe.perm.get_perm(doctype, doc) + : (frappe.perm.doctype_perm[doctype] ??= frappe.perm.get_perm(doctype)); - if (!perms || !perms[permlevel]) return false; - - return !!perms[permlevel][ptype]; + return !!perms?.[permlevel]?.[ptype]; }, get_perm: (doctype, doc) => { @@ -63,56 +60,50 @@ $.extend(frappe.perm, { if (!meta) return perm; perm = frappe.perm.get_role_permissions(meta); + const base_perm = perm[0]; if (doc) { // apply user permissions via docinfo (which is processed server-side) let docinfo = frappe.model.get_docinfo(doctype, doc.name); if (docinfo && docinfo.permissions) { Object.keys(docinfo.permissions).forEach((ptype) => { - perm[0][ptype] = docinfo.permissions[ptype]; + base_perm[ptype] = docinfo.permissions[ptype]; }); } // if owner - if (!$.isEmptyObject(perm[0].if_owner)) { - if (doc.owner === user) { - $.extend(perm[0], perm[0].if_owner); - } else { - // not owner, remove permissions - $.each(perm[0].if_owner, (ptype) => { - if (perm[0].if_owner[ptype]) { - perm[0][ptype] = 0; - } - }); + if (doc.owner !== user) { + for (const right of frappe.perm.rights) { + if (base_perm[right] && !base_perm.rights_without_if_owner.has(right)) { + base_perm[right] = 0; + } } } // apply permissions from shared if (docinfo && docinfo.shared) { - for (let i = 0; i < docinfo.shared.length; i++) { - let s = docinfo.shared[i]; - if (s.user === user) { - perm[0]["read"] = perm[0]["read"] || s.read; - perm[0]["write"] = perm[0]["write"] || s.write; - perm[0]["submit"] = perm[0]["submit"] || s.submit; - perm[0]["share"] = perm[0]["share"] || s.share; + for (const s of docinfo.shared) { + if (s.user !== user) continue; - if (s.read) { - // also give print, email permissions if read - // and these permissions exist at level [0] - perm[0].email = - frappe.boot.user.can_email.indexOf(doctype) !== -1 ? 1 : 0; - perm[0].print = - frappe.boot.user.can_print.indexOf(doctype) !== -1 ? 1 : 0; - } + for (const right of ["read", "write", "submit", "share"]) { + if (!base_perm[right]) base_perm[right] = s[right]; + } + + if (s.read) { + // also give print, email permissions if read + // and these permissions exist at level [0] + base_perm.email = + frappe.boot.user.can_email.indexOf(doctype) !== -1 ? 1 : 0; + base_perm.print = + frappe.boot.user.can_print.indexOf(doctype) !== -1 ? 1 : 0; } } } } - if (frappe.model.can_read(doctype) && !perm[0].read) { + if (frappe.model.can_read(doctype) && !base_perm.read) { // read via sharing - perm[0].read = 1; + base_perm.read = 1; } return perm; @@ -123,35 +114,29 @@ $.extend(frappe.perm, { { "read": 1, "write": 0, - "if_owner": [if "if_owner" is enabled] - { - "read": 1, - "write": 0 - } - } */ + "rights_without_if_owner": {"read", "write"} // for permlevel 0 + } + */ + let perm = [{ read: 0, permlevel: 0 }]; (meta.permissions || []).forEach((p) => { - let permlevel = cint(p.permlevel); - if (!perm[permlevel]) { - perm[permlevel] = {}; - perm[permlevel]["permlevel"] = permlevel; + const permlevel = cint(p.permlevel); + const current_perm = (perm[permlevel] ??= { permlevel }); + + if (permlevel === 0) { + current_perm.rights_without_if_owner ??= new Set(); } // if user has this role if (frappe.user_roles.includes(p.role)) { frappe.perm.rights.forEach((right) => { - let value = perm[permlevel][right] || p[right] || 0; + if (!p[right]) return; - if (p.if_owner && value) { - // if_owner is enabled for perm, - // construct perm object inside "if_owner" property - if (!perm[permlevel]["if_owner"]) { - perm[permlevel]["if_owner"] = {} - } - perm[permlevel]["if_owner"][right] = value; - } else if (value) { - perm[permlevel][right] = value; + current_perm[right] = 1; + + if (permlevel === 0 && !p.if_owner) { + current_perm.rights_without_if_owner.add(right); } }); } @@ -189,7 +174,8 @@ $.extend(frappe.perm, { } } - if (perm[0].if_owner && perm[0].read) { + const base_perm = perm[0]; + if (base_perm.read && !base_perm.rights_without_if_owner.has("read")) { match_rules.push({ Owner: frappe.session.user }); } return match_rules; 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 40/64] 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 41/64] 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 42/64] 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 43/64] 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 44/64] 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); From 7bde45ae600aef54df473aa653b2046ea31d3e31 Mon Sep 17 00:00:00 2001 From: PeterG Date: Sun, 20 Nov 2022 10:13:34 +0000 Subject: [PATCH 45/64] [fix] clearer webform error --- frappe/website/doctype/web_form/web_form.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/frappe/website/doctype/web_form/web_form.py b/frappe/website/doctype/web_form/web_form.py index 03ba7c0880..cbd65e8822 100644 --- a/frappe/website/doctype/web_form/web_form.py +++ b/frappe/website/doctype/web_form/web_form.py @@ -577,6 +577,9 @@ def get_link_options(web_form_name, doctype, allow_read_on_all_link_options=Fals if not allow_read_on_all_link_options: limited_to_user = True + else: + print(vars(frappe.request)) + frappe.throw("You must be logged in to use this form.", frappe.PermissionError) else: for field in web_form_doc.web_form_fields: @@ -607,4 +610,4 @@ def get_link_options(web_form_name, doctype, allow_read_on_all_link_options=Fals return "\n".join([doc.value for doc in link_options]) else: - raise frappe.PermissionError(f"Not Allowed, {doctype}") + raise frappe.PermissionError(f"You don't have permission to access the {doctype} DocType.") From 4ffd454c0437d5f0546ccef71fe2bd5b85b17518 Mon Sep 17 00:00:00 2001 From: PeterG Date: Sun, 20 Nov 2022 16:07:07 +0545 Subject: [PATCH 46/64] [fix] clearer webform error --- frappe/website/doctype/web_form/web_form.py | 1 - 1 file changed, 1 deletion(-) diff --git a/frappe/website/doctype/web_form/web_form.py b/frappe/website/doctype/web_form/web_form.py index cbd65e8822..c674fa4cd2 100644 --- a/frappe/website/doctype/web_form/web_form.py +++ b/frappe/website/doctype/web_form/web_form.py @@ -578,7 +578,6 @@ def get_link_options(web_form_name, doctype, allow_read_on_all_link_options=Fals if not allow_read_on_all_link_options: limited_to_user = True else: - print(vars(frappe.request)) frappe.throw("You must be logged in to use this form.", frappe.PermissionError) else: From ed2804d1c5a30c23c17356df02d6753548878b1e Mon Sep 17 00:00:00 2001 From: marination Date: Mon, 21 Nov 2022 13:28:53 +0530 Subject: [PATCH 47/64] test: Cypress test to check basic `has_perm` and `get_perm` JS APIs - Test removes System Manager role for user and expects certain perms - On Kanban Board DocType, only System Manager is allowed to print/export/email/report/share - DocType level test only --- cypress/integration/permissions.js | 57 ++++++++++++++++++++++++++++++ frappe/tests/ui_test_helpers.py | 9 +++++ 2 files changed, 66 insertions(+) create mode 100644 cypress/integration/permissions.js diff --git a/cypress/integration/permissions.js b/cypress/integration/permissions.js new file mode 100644 index 0000000000..7a13239771 --- /dev/null +++ b/cypress/integration/permissions.js @@ -0,0 +1,57 @@ +context("Permissions API", () => { + before(() => { + cy.visit("/login"); + + cy.login("Administrator"); + cy.call("frappe.tests.ui_test_helpers.add_remove_role", { + action: "remove", + user: "frappe@example.com", + role: "System Manager", + }); + cy.call("logout"); + + cy.login("frappe@example.com"); + cy.visit("/app"); + }); + + it("Checks permissions via `has_perm` for Kanban Board DocType", () => { + cy.visit("/app/kanban-board/view/list"); + cy.window() + .its("frappe") + .then((frappe) => { + frappe.model.with_doctype("Kanban Board", function () { + // needed to make sure doc meta is loaded + expect(frappe.perm.has_perm("Kanban Board", 0, "read")).to.equal(true); + expect(frappe.perm.has_perm("Kanban Board", 0, "write")).to.equal(true); + expect(frappe.perm.has_perm("Kanban Board", 0, "print")).to.equal(false); + }); + }); + }); + + it("Checks permissions via `get_perm` for Kanban Board DocType", () => { + cy.visit("/app/kanban-board/view/list"); + cy.window() + .its("frappe") + .then((frappe) => { + frappe.model.with_doctype("Kanban Board", function () { + // needed to make sure doc meta is loaded + const perms = frappe.perm.get_perm("Kanban Board"); + expect(perms.read).to.equal(true); + expect(perms.write).to.equal(true); + expect(perms.rights_without_if_owner).to.include("read"); + }); + }); + }); + + after(() => { + cy.call("logout"); + + cy.login("Administrator"); + cy.call("frappe.tests.ui_test_helpers.add_remove_role", { + action: "add", + user: "frappe@example.com", + role: "System Manager", + }); + cy.call("logout"); + }); +}); diff --git a/frappe/tests/ui_test_helpers.py b/frappe/tests/ui_test_helpers.py index ecb6b4da97..9d32be229e 100644 --- a/frappe/tests/ui_test_helpers.py +++ b/frappe/tests/ui_test_helpers.py @@ -578,3 +578,12 @@ def create_kanban(): ], } ).insert() + + +@frappe.whitelist() +def add_remove_role(action, user, role): + user_doc = frappe.get_doc("User", user) + if action == "remove": + user_doc.remove_roles(role) + else: + user_doc.add_roles(role) From 8cbfc36f656c75e642db4217070a0631ea2686e0 Mon Sep 17 00:00:00 2001 From: PeterG Date: Mon, 21 Nov 2022 14:28:03 +0545 Subject: [PATCH 48/64] fix: update-password api call (#18943) [skip ci] --- frappe/www/update-password.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/www/update-password.html b/frappe/www/update-password.html index 04dfa5e097..586554efce 100644 --- a/frappe/www/update-password.html +++ b/frappe/www/update-password.html @@ -161,7 +161,7 @@ frappe.ready(function() { .text("{{ _('Invalid Password') }}"); }, 200: function(r) { - if (r.message && r.message.entropy) { + if (r.message && r.message.score) { var score = r.message.score, feedback = r.message.feedback; From 264c9dfad045771bd457cb5a638232826a397df3 Mon Sep 17 00:00:00 2001 From: PeterG Date: Mon, 21 Nov 2022 15:06:06 +0545 Subject: [PATCH 49/64] Update frappe/website/doctype/web_form/web_form.py Co-authored-by: Shariq Ansari <30859809+shariquerik@users.noreply.github.com> --- frappe/website/doctype/web_form/web_form.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/website/doctype/web_form/web_form.py b/frappe/website/doctype/web_form/web_form.py index c674fa4cd2..7ff707618a 100644 --- a/frappe/website/doctype/web_form/web_form.py +++ b/frappe/website/doctype/web_form/web_form.py @@ -609,4 +609,4 @@ def get_link_options(web_form_name, doctype, allow_read_on_all_link_options=Fals return "\n".join([doc.value for doc in link_options]) else: - raise frappe.PermissionError(f"You don't have permission to access the {doctype} DocType.") + raise frappe.PermissionError(_("You don't have permission to access the {0} DocType.").format(doctype)) From cfa9ed4fa0a26dcd14a580c5c3814dac6c4a11fd Mon Sep 17 00:00:00 2001 From: PeterG Date: Mon, 21 Nov 2022 15:06:17 +0545 Subject: [PATCH 50/64] Update frappe/website/doctype/web_form/web_form.py Co-authored-by: Shariq Ansari <30859809+shariquerik@users.noreply.github.com> --- frappe/website/doctype/web_form/web_form.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/website/doctype/web_form/web_form.py b/frappe/website/doctype/web_form/web_form.py index 7ff707618a..2bfa7e7133 100644 --- a/frappe/website/doctype/web_form/web_form.py +++ b/frappe/website/doctype/web_form/web_form.py @@ -578,7 +578,7 @@ def get_link_options(web_form_name, doctype, allow_read_on_all_link_options=Fals if not allow_read_on_all_link_options: limited_to_user = True else: - frappe.throw("You must be logged in to use this form.", frappe.PermissionError) + frappe.throw(_("You must be logged in to use this form."), frappe.PermissionError) else: for field in web_form_doc.web_form_fields: From e594c35a3adefd4e94887785316f1b9ca9cc2036 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 21 Nov 2022 14:47:05 +0530 Subject: [PATCH 51/64] fix: show fields without labels in print format builder (#18939) * fix: show fields without labels in print format builder * refactor: dont hardcode layout fields [skip ci] Co-authored-by: Shariq Ansari <30859809+shariquerik@users.noreply.github.com> Co-authored-by: Shariq Ansari <30859809+shariquerik@users.noreply.github.com> --- .../page/print_format_builder/print_format_builder.js | 5 +---- .../print_format_builder/print_format_builder_field.html | 2 +- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/frappe/printing/page/print_format_builder/print_format_builder.js b/frappe/printing/page/print_format_builder/print_format_builder.js index 4154861f84..657904f32e 100644 --- a/frappe/printing/page/print_format_builder/print_format_builder.js +++ b/frappe/printing/page/print_format_builder/print_format_builder.js @@ -280,10 +280,7 @@ frappe.PrintFormatBuilder = class PrintFormatBuilder { set_section(f.label); } else if (f.fieldtype === "Column Break") { set_column(); - } else if ( - !in_list(["Section Break", "Column Break", "Tab Break", "Fold"], f.fieldtype) && - f.label - ) { + } else if (!in_list(frappe.model.layout_fields, f.fieldtype)) { if (!column) set_column(); if (f.fieldtype === "Table") { diff --git a/frappe/printing/page/print_format_builder/print_format_builder_field.html b/frappe/printing/page/print_format_builder/print_format_builder_field.html index beb9de2f5a..90eb4f1f0a 100644 --- a/frappe/printing/page/print_format_builder/print_format_builder_field.html +++ b/frappe/printing/page/print_format_builder/print_format_builder_field.html @@ -34,7 +34,7 @@ - {{ __(field.label) }} + {{ __(field.label) || __(field.fieldname) }} ({%= __("Table") %}) From 4ec7d2dbbab91eaafd95cd03245b073053755d51 Mon Sep 17 00:00:00 2001 From: PeterG Date: Mon, 21 Nov 2022 15:11:18 +0545 Subject: [PATCH 52/64] feat(minor): redirect after login from NotPermittedPage (#18946) * redirect after login from NotPermittedPage --- frappe/website/page_renderers/not_permitted_page.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/frappe/website/page_renderers/not_permitted_page.py b/frappe/website/page_renderers/not_permitted_page.py index bd27150617..68d4efa939 100644 --- a/frappe/website/page_renderers/not_permitted_page.py +++ b/frappe/website/page_renderers/not_permitted_page.py @@ -14,9 +14,10 @@ class NotPermittedPage(TemplatePage): return True def render(self): + action = f"/login?redirect-to={frappe.request.path}" frappe.local.message_title = _("Not Permitted") frappe.local.response["context"] = dict( - indicator_color="red", primary_action="/login", primary_label=_("Login"), fullpage=True + indicator_color="red", primary_action=action, primary_label=_("Login"), fullpage=True ) self.set_standard_path("message") return super().render() From d529b2a64e3301f522e4a9b0b568b76af11b7e19 Mon Sep 17 00:00:00 2001 From: marination Date: Mon, 21 Nov 2022 15:32:39 +0530 Subject: [PATCH 53/64] chore: Cache Document level perms as well in `has_perm` - Cache documet level output of `get_perm` in `has_perm` as well --- frappe/public/js/frappe/model/perm.js | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/frappe/public/js/frappe/model/perm.js b/frappe/public/js/frappe/model/perm.js index 524a0f5b1a..66bdb3e02b 100644 --- a/frappe/public/js/frappe/model/perm.js +++ b/frappe/public/js/frappe/model/perm.js @@ -33,16 +33,26 @@ $.extend(frappe.perm, { "set_user_permissions", ], - doctype_perm: {}, + doc_perm: {}, has_perm: (doctype, permlevel, ptype, doc) => { if (!permlevel) permlevel = 0; - // if document object is passed, get fresh doc based perms - // (with ownership and user perms applied) else cached doctype perms - const perms = doc - ? frappe.perm.get_perm(doctype, doc) - : (frappe.perm.doctype_perm[doctype] ??= frappe.perm.get_perm(doctype)); + /** frappe.perm.doc_perm structure: + * { + * DocType: { + * "doctype_perm": {...perms}, + * : {...document perms}, + * : {...document perms}, + * } + * } + */ + // Cache doctype perms and document perms (with user perms and ownership) if absent + const doctype_perm = (frappe.perm.doc_perm[doctype] ??= {}); + const perms = (doctype_perm[doc ? doc.name : "doctype_perm"] ??= frappe.perm.get_perm( + doctype, + doc + )); return !!perms?.[permlevel]?.[ptype]; }, From 0937c962a700b500125ef27f625e58bcb2f67e3b Mon Sep 17 00:00:00 2001 From: marination Date: Mon, 21 Nov 2022 16:18:58 +0530 Subject: [PATCH 54/64] chore: `web_form.py` format via pre-commit --- frappe/website/doctype/web_form/web_form.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/frappe/website/doctype/web_form/web_form.py b/frappe/website/doctype/web_form/web_form.py index 2bfa7e7133..d102ac2fd8 100644 --- a/frappe/website/doctype/web_form/web_form.py +++ b/frappe/website/doctype/web_form/web_form.py @@ -609,4 +609,6 @@ def get_link_options(web_form_name, doctype, allow_read_on_all_link_options=Fals return "\n".join([doc.value for doc in link_options]) else: - raise frappe.PermissionError(_("You don't have permission to access the {0} DocType.").format(doctype)) + raise frappe.PermissionError( + _("You don't have permission to access the {0} DocType.").format(doctype) + ) From 4cd80bd27928e39bdc3569c22e3f5d6e69168c90 Mon Sep 17 00:00:00 2001 From: marination Date: Mon, 21 Nov 2022 16:41:37 +0530 Subject: [PATCH 55/64] fix: Avoid caching unsaved documents and secure whitelist decorator - Avoid caching documents like 'new-item-1', default to doctype perms instead - Use secure `whitelist_for_tests` decorator instead --- frappe/public/js/frappe/model/perm.js | 8 ++++---- frappe/tests/ui_test_helpers.py | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/frappe/public/js/frappe/model/perm.js b/frappe/public/js/frappe/model/perm.js index 66bdb3e02b..2cd553a43d 100644 --- a/frappe/public/js/frappe/model/perm.js +++ b/frappe/public/js/frappe/model/perm.js @@ -49,10 +49,10 @@ $.extend(frappe.perm, { */ // Cache doctype perms and document perms (with user perms and ownership) if absent const doctype_perm = (frappe.perm.doc_perm[doctype] ??= {}); - const perms = (doctype_perm[doc ? doc.name : "doctype_perm"] ??= frappe.perm.get_perm( - doctype, - doc - )); + const doc_exists = doc && !doc.__islocal; + + const perms = (doctype_perm[doc_exists ? doc.name : "doctype_perm"] ??= + frappe.perm.get_perm(doctype, doc)); return !!perms?.[permlevel]?.[ptype]; }, diff --git a/frappe/tests/ui_test_helpers.py b/frappe/tests/ui_test_helpers.py index aabcd28c8b..158f8988c7 100644 --- a/frappe/tests/ui_test_helpers.py +++ b/frappe/tests/ui_test_helpers.py @@ -581,7 +581,7 @@ def create_kanban(): ).insert() -@frappe.whitelist() +@whitelist_for_tests() def add_remove_role(action, user, role): user_doc = frappe.get_doc("User", user) if action == "remove": From 8003b907c132372516082c4611d2d109d89c129c Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Mon, 21 Nov 2022 17:11:34 +0530 Subject: [PATCH 56/64] chore: cache `role_perms` instead --- frappe/public/js/frappe/model/perm.js | 21 +++------------------ 1 file changed, 3 insertions(+), 18 deletions(-) diff --git a/frappe/public/js/frappe/model/perm.js b/frappe/public/js/frappe/model/perm.js index 2cd553a43d..eb2c24983f 100644 --- a/frappe/public/js/frappe/model/perm.js +++ b/frappe/public/js/frappe/model/perm.js @@ -33,27 +33,12 @@ $.extend(frappe.perm, { "set_user_permissions", ], - doc_perm: {}, + role_perms: {}, has_perm: (doctype, permlevel, ptype, doc) => { if (!permlevel) permlevel = 0; - /** frappe.perm.doc_perm structure: - * { - * DocType: { - * "doctype_perm": {...perms}, - * : {...document perms}, - * : {...document perms}, - * } - * } - */ - // Cache doctype perms and document perms (with user perms and ownership) if absent - const doctype_perm = (frappe.perm.doc_perm[doctype] ??= {}); - const doc_exists = doc && !doc.__islocal; - - const perms = (doctype_perm[doc_exists ? doc.name : "doctype_perm"] ??= - frappe.perm.get_perm(doctype, doc)); - + const perms = frappe.perm.get_perm(doctype, doc); return !!perms?.[permlevel]?.[ptype]; }, @@ -69,7 +54,7 @@ $.extend(frappe.perm, { if (!meta) return perm; - perm = frappe.perm.get_role_permissions(meta); + perm = frappe.perm.role_perms[doctype] ??= frappe.perm.get_role_permissions(meta); const base_perm = perm[0]; if (doc) { From 6e896aa412f777fddf436447d74a89ac2335bc07 Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Mon, 21 Nov 2022 17:17:18 +0530 Subject: [PATCH 57/64] fix: decorator usage --- frappe/tests/ui_test_helpers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/tests/ui_test_helpers.py b/frappe/tests/ui_test_helpers.py index 158f8988c7..05d3dedad6 100644 --- a/frappe/tests/ui_test_helpers.py +++ b/frappe/tests/ui_test_helpers.py @@ -581,7 +581,7 @@ def create_kanban(): ).insert() -@whitelist_for_tests() +@whitelist_for_tests def add_remove_role(action, user, role): user_doc = frappe.get_doc("User", user) if action == "remove": From e10a3d0b8dba86d6d1ae0dba16568f79c7f8e58b Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Mon, 21 Nov 2022 17:30:32 +0530 Subject: [PATCH 58/64] chore: move cheaper condition first --- frappe/public/js/frappe/model/perm.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/public/js/frappe/model/perm.js b/frappe/public/js/frappe/model/perm.js index eb2c24983f..1951a9d841 100644 --- a/frappe/public/js/frappe/model/perm.js +++ b/frappe/public/js/frappe/model/perm.js @@ -96,7 +96,7 @@ $.extend(frappe.perm, { } } - if (frappe.model.can_read(doctype) && !base_perm.read) { + if (!base_perm.read && frappe.model.can_read(doctype)) { // read via sharing base_perm.read = 1; } From 962d96c1a6ee7388191df6d728334a6a8a93dd79 Mon Sep 17 00:00:00 2001 From: Shariq Ansari Date: Mon, 21 Nov 2022 17:33:39 +0530 Subject: [PATCH 59/64] fix: only get fields which is not already in webform fields table --- frappe/website/doctype/web_form/web_form.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/website/doctype/web_form/web_form.js b/frappe/website/doctype/web_form/web_form.js index c494781f1f..28d33a5266 100644 --- a/frappe/website/doctype/web_form/web_form.js +++ b/frappe/website/doctype/web_form/web_form.js @@ -79,7 +79,7 @@ frappe.ui.form.on("Web Form", { .get_field("Web Form Field", "fieldtype") .options.split("\n"); - let added_fields = (frm.doc.fields || []).map((d) => d.fieldname); + let added_fields = (frm.doc.web_form_fields || []).map((d) => d.fieldname); get_fields_for_doctype(frm.doc.doc_type).then((fields) => { for (let df of fields) { From b8e7deea40927d94f6e4d8e5c214e5a7c2b0db0b Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Mon, 21 Nov 2022 18:06:08 +0530 Subject: [PATCH 60/64] chore: revert to `doctype_perm` cache for now --- frappe/public/js/frappe/model/perm.js | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/frappe/public/js/frappe/model/perm.js b/frappe/public/js/frappe/model/perm.js index 1951a9d841..fdd915ebfc 100644 --- a/frappe/public/js/frappe/model/perm.js +++ b/frappe/public/js/frappe/model/perm.js @@ -33,7 +33,7 @@ $.extend(frappe.perm, { "set_user_permissions", ], - role_perms: {}, + doctype_perm: {}, has_perm: (doctype, permlevel, ptype, doc) => { if (!permlevel) permlevel = 0; @@ -43,6 +43,17 @@ $.extend(frappe.perm, { }, get_perm: (doctype, doc) => { + // if document object is passed, get fresh doc based perms + // (with ownership and user perms applied) else cached doctype perms + + if (doc && !doc.__islocal) { + return frappe.perm._get_perm(doctype, doc); + } + + return (frappe.perm.doctype_perm[doctype] ??= frappe.perm._get_perm(doctype)); + }, + + _get_perm: (doctype, doc) => { let perm = [{ read: 0, permlevel: 0 }]; let meta = frappe.get_doc("DocType", doctype); @@ -54,7 +65,7 @@ $.extend(frappe.perm, { if (!meta) return perm; - perm = frappe.perm.role_perms[doctype] ??= frappe.perm.get_role_permissions(meta); + perm = frappe.perm.get_role_permissions(meta); const base_perm = perm[0]; if (doc) { From cf4d55855d7cf48cf271eac26d25d452788e90d4 Mon Sep 17 00:00:00 2001 From: Shariq Ansari Date: Mon, 21 Nov 2022 18:16:44 +0530 Subject: [PATCH 61/64] fix: added phone field in webform --- frappe/public/scss/website/web_form.scss | 24 ++++++++++++++----- .../web_form_field/web_form_field.json | 4 ++-- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/frappe/public/scss/website/web_form.scss b/frappe/public/scss/website/web_form.scss index e5a0093307..8ecf753384 100644 --- a/frappe/public/scss/website/web_form.scss +++ b/frappe/public/scss/website/web_form.scss @@ -109,15 +109,27 @@ .form-column { padding: 0 var(--padding-sm); - .frappe-control[data-fieldtype="Rating"] { - .like-disabled-input { - background-color: unset; - padding-left: 0px; + .frappe-control { + position: relative; - .rating { - cursor: default; + &[data-fieldtype="Rating"] { + .like-disabled-input { + background-color: unset; + padding-left: 0px; + + .rating { + cursor: default; + } } } + + .selected-phone { + top: calc(50% + 4px); + } + + .selected-color { + top: 6px; + } } &:first-child { diff --git a/frappe/website/doctype/web_form_field/web_form_field.json b/frappe/website/doctype/web_form_field/web_form_field.json index 4fb566be88..90ec99c3b8 100644 --- a/frappe/website/doctype/web_form_field/web_form_field.json +++ b/frappe/website/doctype/web_form_field/web_form_field.json @@ -39,7 +39,7 @@ "fieldtype": "Select", "in_list_view": 1, "label": "Fieldtype", - "options": "Attach\nAttach Image\nCheck\nCurrency\nColor\nData\nDate\nDatetime\nDuration\nFloat\nHTML\nInt\nLink\nPassword\nRating\nSelect\nSignature\nSmall Text\nText\nText Editor\nTable\nTime\nSection Break\nColumn Break\nPage Break" + "options": "Attach\nAttach Image\nCheck\nCurrency\nColor\nData\nDate\nDatetime\nDuration\nFloat\nHTML\nInt\nLink\nPassword\nPhone\nRating\nSelect\nSignature\nSmall Text\nText\nText Editor\nTable\nTime\nSection Break\nColumn Break\nPage Break" }, { "fieldname": "label", @@ -147,7 +147,7 @@ ], "istable": 1, "links": [], - "modified": "2022-08-22 17:22:39.026893", + "modified": "2022-11-21 17:41:52.139191", "modified_by": "Administrator", "module": "Website", "name": "Web Form Field", From 6479163899dedd2b3010be9a11f4ee71cc0e074a Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Mon, 21 Nov 2022 19:28:54 +0530 Subject: [PATCH 62/64] fix(file): set fieldname to table fieldname because files are always attached to parent --- frappe/public/js/frappe/form/controls/attach.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/frappe/public/js/frappe/form/controls/attach.js b/frappe/public/js/frappe/form/controls/attach.js index fc8aef72b8..bbd8129b8d 100644 --- a/frappe/public/js/frappe/form/controls/attach.js +++ b/frappe/public/js/frappe/form/controls/attach.js @@ -79,7 +79,11 @@ frappe.ui.form.ControlAttach = class ControlAttach extends frappe.ui.form.Contro if (this.frm) { options.doctype = this.frm.doctype; options.docname = this.frm.docname; - options.fieldname = this.df.fieldname; + if (this.grid) { + options.fieldname = this.grid.control.df.fieldname; + } else { + options.fieldname = this.df.fieldname; + } options.make_attachments_public = this.frm.meta.make_attachments_public; } From 45230cf938943b8cab627f81dfddbd223592574e Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Tue, 22 Nov 2022 00:00:37 +0530 Subject: [PATCH 63/64] fix: succinct version Co-authored-by: Shariq Ansari <30859809+shariquerik@users.noreply.github.com> --- frappe/public/js/frappe/form/controls/attach.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/frappe/public/js/frappe/form/controls/attach.js b/frappe/public/js/frappe/form/controls/attach.js index bbd8129b8d..0f40a0a829 100644 --- a/frappe/public/js/frappe/form/controls/attach.js +++ b/frappe/public/js/frappe/form/controls/attach.js @@ -79,11 +79,7 @@ frappe.ui.form.ControlAttach = class ControlAttach extends frappe.ui.form.Contro if (this.frm) { options.doctype = this.frm.doctype; options.docname = this.frm.docname; - if (this.grid) { - options.fieldname = this.grid.control.df.fieldname; - } else { - options.fieldname = this.df.fieldname; - } + options.fieldname = this.grid ? this.grid.df.fieldname : this.df.fieldname; options.make_attachments_public = this.frm.meta.make_attachments_public; } From eae918d545975d8752bbbc8c38641c95762f5408 Mon Sep 17 00:00:00 2001 From: Raffael Meyer <14891507+barredterra@users.noreply.github.com> Date: Tue, 22 Nov 2022 14:27:41 +0100 Subject: [PATCH 64/64] feat: contact full name (#18596) * feat: contact full name * patch: autocommit and don't update modified * patch: print progress while updating contacts * patch: don't overwrite existing full_name --- frappe/contacts/doctype/contact/contact.json | 20 ++++++++++--- frappe/contacts/doctype/contact/contact.py | 28 ++++++++++++++----- .../contacts/doctype/contact/test_contact.py | 13 +++++++++ frappe/patches.txt | 1 + frappe/patches/v15_0/set_contact_full_name.py | 25 +++++++++++++++++ 5 files changed, 76 insertions(+), 11 deletions(-) create mode 100644 frappe/patches/v15_0/set_contact_full_name.py diff --git a/frappe/contacts/doctype/contact/contact.json b/frappe/contacts/doctype/contact/contact.json index d342b2d794..c756a8ecb8 100644 --- a/frappe/contacts/doctype/contact/contact.json +++ b/frappe/contacts/doctype/contact/contact.json @@ -12,6 +12,7 @@ "first_name", "middle_name", "last_name", + "full_name", "email_id", "user", "address", @@ -52,8 +53,7 @@ "in_global_search": 1, "label": "First Name", "oldfieldname": "first_name", - "oldfieldtype": "Data", - "reqd": 1 + "oldfieldtype": "Data" }, { "bold": 1, @@ -243,6 +243,13 @@ "fieldname": "company_name", "fieldtype": "Data", "label": "Company Name" + }, + { + "fieldname": "full_name", + "fieldtype": "Data", + "hidden": 1, + "label": "Full Name", + "read_only": 1 } ], "icon": "fa fa-user", @@ -250,10 +257,12 @@ "image_field": "image", "index_web_pages_for_search": 1, "links": [], - "modified": "2020-08-27 14:12:09.906719", + "modified": "2022-10-27 10:40:50.097481", "modified_by": "Administrator", "module": "Contacts", "name": "Contact", + "name_case": "Title Case", + "naming_rule": "By script", "owner": "Administrator", "permissions": [ { @@ -379,6 +388,9 @@ "role": "All" } ], + "show_title_field_in_link": 1, "sort_field": "modified", - "sort_order": "ASC" + "sort_order": "ASC", + "states": [], + "title_field": "full_name" } diff --git a/frappe/contacts/doctype/contact/contact.py b/frappe/contacts/doctype/contact/contact.py index 1ed4307d8d..8540e1cfb8 100644 --- a/frappe/contacts/doctype/contact/contact.py +++ b/frappe/contacts/doctype/contact/contact.py @@ -11,10 +11,7 @@ from frappe.utils import cstr, has_gravatar class Contact(Document): def autoname(self): - # concat first and last name - self.name = " ".join( - filter(None, [cstr(self.get(f)).strip() for f in ["first_name", "last_name"]]) - ) + self.name = self._get_full_name() if frappe.db.exists("Contact", self.name): self.name = append_number_if_name_exists("Contact", self.name) @@ -25,6 +22,7 @@ class Contact(Document): break def validate(self): + self.full_name = self._get_full_name() self.set_primary_email() self.set_primary("phone") self.set_primary("mobile_no") @@ -128,6 +126,9 @@ class Contact(Document): if not primary_number_exists: setattr(self, fieldname, "") + def _get_full_name(self) -> str: + return get_full_name(self.first_name, self.middle_name, self.last_name, self.company_name) + def get_default_contact(doctype, name): """Returns default contact for the given doctype, name""" @@ -222,7 +223,7 @@ def contact_query(doctype, txt, searchfield, start, page_len, filters): return frappe.db.sql( """select - `tabContact`.name, `tabContact`.first_name, `tabContact`.last_name + `tabContact`.name, `tabContact`.full_name, `tabContact`.company_name from `tabContact`, `tabDynamic Link` where @@ -233,8 +234,8 @@ def contact_query(doctype, txt, searchfield, start, page_len, filters): `tabContact`.`{key}` like %(txt)s {mcond} order by - if(locate(%(_txt)s, `tabContact`.name), locate(%(_txt)s, `tabContact`.name), 99999), - `tabContact`.idx desc, `tabContact`.name + if(locate(%(_txt)s, `tabContact`.full_name), locate(%(_txt)s, `tabContact`.company_name), 99999), + `tabContact`.idx desc, `tabContact`.full_name limit %(start)s, %(page_len)s """.format( mcond=get_match_cond(doctype), key=searchfield ), @@ -327,3 +328,16 @@ def get_contacts_linked_from(doctype, docname, fields=None): return [] return frappe.get_list("Contact", fields=fields, filters={"name": ("in", contact_names)}) + + +def get_full_name( + first: str | None = None, + middle: str | None = None, + last: str | None = None, + company: str | None = None, +) -> str: + full_name = " ".join(filter(None, [cstr(f).strip() for f in [first, middle, last]])) + if not full_name and company: + full_name = company + + return full_name diff --git a/frappe/contacts/doctype/contact/test_contact.py b/frappe/contacts/doctype/contact/test_contact.py index 6f0fd4ae3f..e91e132258 100644 --- a/frappe/contacts/doctype/contact/test_contact.py +++ b/frappe/contacts/doctype/contact/test_contact.py @@ -1,6 +1,7 @@ # Copyright (c) 2017, Frappe Technologies and Contributors # License: MIT. See LICENSE import frappe +from frappe.contacts.doctype.contact.contact import get_full_name from frappe.tests.utils import FrappeTestCase test_dependencies = ["Contact", "Salutation"] @@ -31,6 +32,18 @@ class TestContact(FrappeTestCase): self.assertEqual(contact.phone, "+91 0000000002") self.assertEqual(contact.mobile_no, "+91 0000000003") + def test_get_full_name(self): + self.assertEqual(get_full_name(first="John"), "John") + self.assertEqual(get_full_name(last="Doe"), "Doe") + self.assertEqual(get_full_name(company="Doe Pvt Ltd"), "Doe Pvt Ltd") + self.assertEqual(get_full_name(first="John", last="Doe"), "John Doe") + self.assertEqual(get_full_name(first="John", middle="Jane"), "John Jane") + self.assertEqual(get_full_name(first="John", last="Doe", company="Doe Pvt Ltd"), "John Doe") + self.assertEqual( + get_full_name(first="John", middle="Jane", last="Doe", company="Doe Pvt Ltd"), + "John Jane Doe", + ) + def create_contact(name, salutation, emails=None, phones=None, save=True): doc = frappe.get_doc( diff --git a/frappe/patches.txt b/frappe/patches.txt index 278a351093..e8289a2790 100644 --- a/frappe/patches.txt +++ b/frappe/patches.txt @@ -215,3 +215,4 @@ frappe.patches.v14_0.drop_unused_indexes frappe.patches.v15_0.drop_modified_index frappe.patches.v14_0.add_manage_subscriptions_in_navbar_settings frappe.patches.v14_0.update_attachment_comment +frappe.patches.v15_0.set_contact_full_name diff --git a/frappe/patches/v15_0/set_contact_full_name.py b/frappe/patches/v15_0/set_contact_full_name.py new file mode 100644 index 0000000000..6dc3036f34 --- /dev/null +++ b/frappe/patches/v15_0/set_contact_full_name.py @@ -0,0 +1,25 @@ +import frappe +from frappe.contacts.doctype.contact.contact import get_full_name +from frappe.utils import update_progress_bar + + +def execute(): + """Set full name for all contacts""" + frappe.db.auto_commit_on_many_writes = 1 + + contacts = frappe.get_all( + "Contact", + fields=["name", "first_name", "middle_name", "last_name", "company_name"], + filters={"full_name": ("is", "not set")}, + as_list=True, + ) + total = len(contacts) + for idx, (name, first, middle, last, company) in enumerate(contacts): + update_progress_bar("Setting full name for contacts", idx, total) + frappe.db.set_value( + "Contact", + name, + "full_name", + get_full_name(first, middle, last, company), + update_modified=False, + )