From 7e6e7aa0556953d0a5a88a3304e83f71634f4ad7 Mon Sep 17 00:00:00 2001 From: Devin Slauenwhite Date: Fri, 13 Jan 2023 13:53:13 -0500 Subject: [PATCH 01/60] fix: ambiguous linked tables --- frappe/model/db_query.py | 32 ++++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/frappe/model/db_query.py b/frappe/model/db_query.py index 1d156d0d1a..2718dd1180 100644 --- a/frappe/model/db_query.py +++ b/frappe/model/db_query.py @@ -5,6 +5,7 @@ import copy import json import re +from collections import Counter from datetime import datetime import frappe @@ -57,6 +58,7 @@ class DatabaseQuery: self.doctype = doctype self.tables = [] self.link_tables = [] + self.linked_table_counter = Counter() self.conditions = [] self.or_conditions = [] self.fields = None @@ -242,7 +244,7 @@ class DatabaseQuery: # left join link tables for link in self.link_tables: - args.tables += f" {self.join} `tab{link.doctype}` on (`tab{link.doctype}`.`name` = {self.tables[0]}.`{link.fieldname}`)" + args.tables += f" {self.join} {link.table_name} {link.table_alias} on ({link.table_alias}.`name` = {self.tables[0]}.`{link.fieldname}`)" if self.grouped_or_conditions: self.conditions.append(f"({' or '.join(self.grouped_or_conditions)})") @@ -326,8 +328,10 @@ class DatabaseQuery: linked_field = frappe.get_meta(self.doctype).get_field(linked_fieldname) linked_doctype = linked_field.options if linked_field.fieldtype == "Link": - self.append_link_table(linked_doctype, linked_fieldname) - field = f"`tab{linked_doctype}`.`{fieldname}`" + linked_table = self.append_link_table(linked_doctype, linked_fieldname) + field = f"{linked_table.table_alias}.`{fieldname}`" + else: + field = f"`tab{linked_doctype}`.`{fieldname}`" if alias: field = f"{field} as {alias}" self.fields[self.fields.index(original_field)] = field @@ -437,6 +441,12 @@ class DatabaseQuery: table_name = field.split(".")[0] + # Check if table_name is a linked_table alias + for linked_table in self.link_tables: + if linked_table.table_alias == table_name: + table_name = linked_table.table_name + break + if table_name.lower().startswith("group_concat("): table_name = table_name[13:] if not table_name[0] == "`": @@ -452,14 +462,20 @@ class DatabaseQuery: self.check_read_permission(doctype) def append_link_table(self, doctype, fieldname): - for d in self.link_tables: - if d.doctype == doctype and d.fieldname == fieldname: - return + for linked_table in self.link_tables: + if linked_table.doctype == doctype and linked_table.fieldname == fieldname: + return linked_table self.check_read_permission(doctype) - self.link_tables.append( - frappe._dict(doctype=doctype, fieldname=fieldname, table_name=f"`tab{doctype}`") + self.linked_table_counter.update((doctype,)) + linked_table = frappe._dict( + doctype=doctype, + fieldname=fieldname, + table_name=f"`tab{doctype}`", + table_alias=f"`tab{doctype}_{self.linked_table_counter[doctype]}`", ) + self.link_tables.append(linked_table) + return linked_table def check_read_permission(self, doctype): if not self.flags.ignore_permissions and not frappe.has_permission( From af579e3192266e643b2a2419d55752a7d28e4551 Mon Sep 17 00:00:00 2001 From: Devin Slauenwhite Date: Fri, 13 Jan 2023 14:09:09 -0500 Subject: [PATCH 02/60] test: ambiguous linked tables --- frappe/tests/test_db_query.py | 57 +++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/frappe/tests/test_db_query.py b/frappe/tests/test_db_query.py index 1964792ea8..046f9a5dd5 100644 --- a/frappe/tests/test_db_query.py +++ b/frappe/tests/test_db_query.py @@ -917,6 +917,63 @@ class TestReportview(FrappeTestCase): self.assertIn("ifnull", frappe.get_all("User", {"name": ("not in", [])}, run=0)) self.assertIn("ifnull", frappe.get_all("User", {"name": ("not in", [""])}, run=0)) + def test_ambiguous_linked_tables(self): + from frappe.desk.reportview import get_list + + frappe.get_doc( + { + "doctype": "DocType", + "custom": 1, + "module": "Custom", + "name": "Related Todos", + "naming_rule": "Random", + "autoname": "hash", + "fields": [ + { + "label": "Todo One", + "fieldname": "todo_one", + "fieldtype": "Link", + "options": "ToDo", + "reqd": 1, + }, + { + "label": "Todo Two", + "fieldname": "todo_two", + "fieldtype": "Link", + "options": "ToDo", + "reqd": 1, + }, + ], + } + ).insert() + + todo_one = frappe.get_doc( + { + "doctype": "ToDo", + "description": "Todo One", + } + ).insert() + + todo_two = frappe.get_doc( + { + "doctype": "ToDo", + "description": "Todo Two", + } + ).insert() + + frappe.get_doc( + { + "doctype": "Related Todos", + "todo_one": todo_one.name, + "todo_two": todo_two.name, + } + ).insert() + + frappe.form_dict.doctype = "Related Todos" + data = get_list() + frappe.form_dict.doctype = "" + self.assertEqual(len(data), 1) + def add_child_table_to_blog_post(): child_table = frappe.get_doc( From 1fa9c1524022b7068ada03ecb65e7649cd43fee6 Mon Sep 17 00:00:00 2001 From: Devin Slauenwhite Date: Fri, 13 Jan 2023 14:39:21 -0500 Subject: [PATCH 03/60] fix(test): test no pymysql.err.OperationalError --- frappe/tests/test_db_query.py | 72 ++++++++++++++++++++--------------- 1 file changed, 42 insertions(+), 30 deletions(-) diff --git a/frappe/tests/test_db_query.py b/frappe/tests/test_db_query.py index 046f9a5dd5..b231a83b0e 100644 --- a/frappe/tests/test_db_query.py +++ b/frappe/tests/test_db_query.py @@ -918,34 +918,37 @@ class TestReportview(FrappeTestCase): self.assertIn("ifnull", frappe.get_all("User", {"name": ("not in", [""])}, run=0)) def test_ambiguous_linked_tables(self): - from frappe.desk.reportview import get_list + from frappe.desk.reportview import get - frappe.get_doc( - { - "doctype": "DocType", - "custom": 1, - "module": "Custom", - "name": "Related Todos", - "naming_rule": "Random", - "autoname": "hash", - "fields": [ - { - "label": "Todo One", - "fieldname": "todo_one", - "fieldtype": "Link", - "options": "ToDo", - "reqd": 1, - }, - { - "label": "Todo Two", - "fieldname": "todo_two", - "fieldtype": "Link", - "options": "ToDo", - "reqd": 1, - }, - ], - } - ).insert() + if not frappe.db.exists("DocType", "Related Todos"): + frappe.get_doc( + { + "doctype": "DocType", + "custom": 1, + "module": "Custom", + "name": "Related Todos", + "naming_rule": "Random", + "autoname": "hash", + "fields": [ + { + "label": "Todo One", + "fieldname": "todo_one", + "fieldtype": "Link", + "options": "ToDo", + "reqd": 1, + }, + { + "label": "Todo Two", + "fieldname": "todo_two", + "fieldtype": "Link", + "options": "ToDo", + "reqd": 1, + }, + ], + } + ).insert() + else: + frappe.db.delete("Related Todos") todo_one = frappe.get_doc( { @@ -970,9 +973,18 @@ class TestReportview(FrappeTestCase): ).insert() frappe.form_dict.doctype = "Related Todos" - data = get_list() - frappe.form_dict.doctype = "" - self.assertEqual(len(data), 1) + frappe.form_dict.fields = [ + "`tabRelated Todos`.`name`", + "`tabRelated Todos`.`todo_one`", + "`tabRelated Todos`.`todo_two`", + # because ToDo.show_title_as_field_link = 1 + "todo_one.description as todo_one_description", + "todo_two.description as todo_two_description", + ] + + # Shouldn't raise pymysql.err.OperationalError: (1066, "Not unique table/alias: 'tabToDo'") + data = get() + self.assertEqual(len(data["values"]), 1) def add_child_table_to_blog_post(): From 943a49aabce8f17879a637a565e80ea9045a6099 Mon Sep 17 00:00:00 2001 From: Devin Slauenwhite Date: Fri, 3 Mar 2023 11:28:36 -0500 Subject: [PATCH 04/60] fix: perm check on alias table name --- frappe/model/db_query.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/frappe/model/db_query.py b/frappe/model/db_query.py index 4143f05630..4821f6558e 100644 --- a/frappe/model/db_query.py +++ b/frappe/model/db_query.py @@ -62,6 +62,7 @@ class DatabaseQuery: self.doctype = doctype self.tables = [] self.link_tables = [] + self.linked_table_aliases = {} self.linked_table_counter = Counter() self.conditions = [] self.or_conditions = [] @@ -79,7 +80,7 @@ class DatabaseQuery: @property def query_tables(self): - return self.tables + [d.table_name for d in self.link_tables] + return self.tables + [d.table_alias for d in self.link_tables] def execute( self, @@ -508,6 +509,7 @@ class DatabaseQuery: table_name=f"`tab{doctype}`", table_alias=f"`tab{doctype}_{self.linked_table_counter[doctype]}`", ) + self.linked_table_aliases[linked_table.table_alias.replace("`", "")] = linked_table.table_name self.link_tables.append(linked_table) return linked_table @@ -657,7 +659,12 @@ class DatabaseQuery: # handle child / joined table fields elif "." in field: table, column = column.split(".", 1) - ch_doctype = table.replace("`", "").replace("tab", "", 1) + ch_doctype = table + + if ch_doctype in self.linked_table_aliases: + ch_doctype = self.linked_table_aliases[ch_doctype] + + ch_doctype = ch_doctype.replace("`", "").replace("tab", "", 1) if wrap_grave_quotes(table) in self.query_tables: permitted_child_table_fields = get_permitted_fields( From 1c86d6431f8a397b1d9364eaa17a1c1ea2f764de Mon Sep 17 00:00:00 2001 From: Devin Slauenwhite Date: Fri, 3 Mar 2023 11:46:57 -0500 Subject: [PATCH 05/60] fix: don't join on parenttype --- frappe/model/db_query.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/model/db_query.py b/frappe/model/db_query.py index 4821f6558e..17935c226c 100644 --- a/frappe/model/db_query.py +++ b/frappe/model/db_query.py @@ -488,7 +488,7 @@ class DatabaseQuery: table_name = table_name[13:] if not table_name[0] == "`": table_name = f"`{table_name}`" - if table_name not in self.query_tables: + if table_name not in self.query_tables and table_name not in self.linked_table_aliases.values(): self.append_table(table_name) def append_table(self, table_name): From f17e52088f988490bb7cf7c7586ed652ae20c3e3 Mon Sep 17 00:00:00 2001 From: Devin Slauenwhite Date: Fri, 3 Mar 2023 11:59:05 -0500 Subject: [PATCH 06/60] chore: linter --- frappe/model/db_query.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/frappe/model/db_query.py b/frappe/model/db_query.py index 17935c226c..394d3b8ccb 100644 --- a/frappe/model/db_query.py +++ b/frappe/model/db_query.py @@ -488,7 +488,9 @@ class DatabaseQuery: table_name = table_name[13:] if not table_name[0] == "`": table_name = f"`{table_name}`" - if table_name not in self.query_tables and table_name not in self.linked_table_aliases.values(): + if ( + table_name not in self.query_tables and table_name not in self.linked_table_aliases.values() + ): self.append_table(table_name) def append_table(self, table_name): From 4ee5c015c35fcb450e7f751edd5e8b533b974d02 Mon Sep 17 00:00:00 2001 From: Dirk van der Laarse Date: Wed, 14 Jun 2023 12:45:49 +0000 Subject: [PATCH 07/60] feat: setting for force web capture mode for camera uploads --- cypress/integration/control_attach.js | 45 +++++++++++++++++++ .../system_settings/system_settings.json | 9 +++- frappe/public/js/frappe/ui/capture.js | 11 +++-- 3 files changed, 61 insertions(+), 4 deletions(-) diff --git a/cypress/integration/control_attach.js b/cypress/integration/control_attach.js index 96b8c73b6e..d6086c9ed1 100644 --- a/cypress/integration/control_attach.js +++ b/cypress/integration/control_attach.js @@ -92,4 +92,49 @@ context("Attach Control", () => { cy.get('.actions-btn-group > .dropdown-menu [data-label="Delete"]').click(); cy.click_modal_primary_button("Yes"); }); + + it('Checking that "Camera" button in the "Attach" fieldtype does show if camera is available', () => { + //Navigating to the new form for the newly created doctype + let doctype = "Test Attach Control" + let dt_in_route = doctype.toLowerCase().replace(/ /g, "-"); + cy.visit(`/app/${dt_in_route}/new`, { + onBeforeLoad (win) { + // Mock "window.navigator.mediaDevices" property + // to return mock mediaDevices object + win.navigator.mediaDevices = { + ondevicechange: null + } + } + }); + cy.get("body").should("have.attr", "data-route", `Form/${doctype}/new-${dt_in_route}-1`); + cy.get("body").should("have.attr", "data-ajax-state", "complete"); + + //Clicking on the attach button which is displayed as part of creating a doctype with "Attach" fieldtype + cy.findByRole("button", { name: "Attach" }).click(); + + //Clicking on "Camera" button + cy.findByRole("button", { name: "Camera" }).should("exist"); + + }); + + it('Checking that "Camera" button in the "Attach" fieldtype does not show if no camera is available', () => { + //Navigating to the new form for the newly created doctype + let doctype = "Test Attach Control" + let dt_in_route = doctype.toLowerCase().replace(/ /g, "-"); + cy.visit(`/app/${dt_in_route}/new`, { + onBeforeLoad (win) { + // Delete "window.navigator.mediaDevices" property + delete win.navigator.mediaDevices + } + }); + cy.get("body").should("have.attr", "data-route", `Form/${doctype}/new-${dt_in_route}-1`); + cy.get("body").should("have.attr", "data-ajax-state", "complete"); + + //Clicking on the attach button which is displayed as part of creating a doctype with "Attach" fieldtype + cy.findByRole("button", { name: "Attach" }).click(); + + //Clicking on "Camera" button + cy.findByRole("button", { name: "Camera" }).should("not.exist"); + + }); }); diff --git a/frappe/core/doctype/system_settings/system_settings.json b/frappe/core/doctype/system_settings/system_settings.json index 5efe87da25..5d88a3e466 100644 --- a/frappe/core/doctype/system_settings/system_settings.json +++ b/frappe/core/doctype/system_settings/system_settings.json @@ -563,12 +563,19 @@ "fieldtype": "Link", "label": "Reset Password Template", "options": "Email Template" + }, + { + "default": "0", + "description": "When uploading files, force the use of the web-based image capture. If this is unchecked, the default behavior is to use the mobile native camera when use from a mobile is detected.", + "fieldname": "force_web_capture_mode_for_uploads", + "fieldtype": "Check", + "label": "Force web capture mode for uploads" } ], "icon": "fa fa-cog", "issingle": 1, "links": [], - "modified": "2023-05-25 13:02:54.808773", + "modified": "2023-06-14 11:19:33.429196", "modified_by": "Administrator", "module": "Core", "name": "System Settings", diff --git a/frappe/public/js/frappe/ui/capture.js b/frappe/public/js/frappe/ui/capture.js index bc7ce34149..72841885a0 100644 --- a/frappe/public/js/frappe/ui/capture.js +++ b/frappe/public/js/frappe/ui/capture.js @@ -76,11 +76,16 @@ frappe.ui.Capture = class { show() { this.build_dialog(); - if (frappe.is_mobile()) { - this.show_for_mobile(); - } else { + if (frappe.boot.sysdefaults.force_web_capture_mode_for_uploads) { this.show_for_desktop(); } + else { + if (frappe.is_mobile()) { + this.show_for_mobile(); + } else { + this.show_for_desktop(); + } + } } build_dialog() { From 93519703335a92b5e7be9acf271aa60ad4830ca9 Mon Sep 17 00:00:00 2001 From: Dirk van der Laarse Date: Wed, 14 Jun 2023 16:01:48 +0200 Subject: [PATCH 08/60] fix: cast sysdefaults setting to int --- frappe/public/js/frappe/ui/capture.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/public/js/frappe/ui/capture.js b/frappe/public/js/frappe/ui/capture.js index 72841885a0..8309ffeeb8 100644 --- a/frappe/public/js/frappe/ui/capture.js +++ b/frappe/public/js/frappe/ui/capture.js @@ -76,7 +76,7 @@ frappe.ui.Capture = class { show() { this.build_dialog(); - if (frappe.boot.sysdefaults.force_web_capture_mode_for_uploads) { + if (cint(frappe.boot.sysdefaults.force_web_capture_mode_for_uploads)) { this.show_for_desktop(); } else { From ac8466d3d8b217a30b79bbb6eda2c99be2a5e834 Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Wed, 19 Jul 2023 10:27:41 +0200 Subject: [PATCH 09/60] refactor(Geolocation): customize_draw_controls Extract method `customize_draw_controls` from `bind_leaflet_map` --- frappe/public/js/frappe/form/controls/geolocation.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/frappe/public/js/frappe/form/controls/geolocation.js b/frappe/public/js/frappe/form/controls/geolocation.js index 84b1989400..acff3d6ce8 100644 --- a/frappe/public/js/frappe/form/controls/geolocation.js +++ b/frappe/public/js/frappe/form/controls/geolocation.js @@ -35,6 +35,7 @@ frappe.ui.form.ControlGeolocation = class ControlGeolocation extends frappe.ui.f } make_map(value) { + this.customize_draw_controls(); this.bind_leaflet_map(); if (this.disabled) { this.map.dragging.disable(); @@ -113,7 +114,7 @@ frappe.ui.form.ControlGeolocation = class ControlGeolocation extends frappe.ui.f */ on_each_feature(feature, layer) {} - bind_leaflet_map() { + customize_draw_controls() { const circleToGeoJSON = L.Circle.prototype.toGeoJSON; L.Circle.include({ toGeoJSON: function () { @@ -138,6 +139,9 @@ frappe.ui.form.ControlGeolocation = class ControlGeolocation extends frappe.ui.f }); L.Icon.Default.imagePath = "/assets/frappe/images/leaflet/"; + } + + bind_leaflet_map() { this.map = L.map(this.map_id); this.map.setView(frappe.utils.map_defaults.center, frappe.utils.map_defaults.zoom); From 819f18acb47240a3bb25ca11139d051ca4b9f790 Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Wed, 19 Jul 2023 10:32:23 +0200 Subject: [PATCH 10/60] refactor(Geolocation): image path via map defaults Move the default path for icons to map_defaults instead of hardcoding it in the control --- frappe/public/js/frappe/form/controls/geolocation.js | 2 +- frappe/public/js/frappe/utils/utils.js | 1 + frappe/public/js/frappe/views/map/map_view.js | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/frappe/public/js/frappe/form/controls/geolocation.js b/frappe/public/js/frappe/form/controls/geolocation.js index acff3d6ce8..10ac21a5fb 100644 --- a/frappe/public/js/frappe/form/controls/geolocation.js +++ b/frappe/public/js/frappe/form/controls/geolocation.js @@ -138,7 +138,7 @@ frappe.ui.form.ControlGeolocation = class ControlGeolocation extends frappe.ui.f }, }); - L.Icon.Default.imagePath = "/assets/frappe/images/leaflet/"; + L.Icon.Default.imagePath = frappe.utils.map_defaults.image_path; } bind_leaflet_map() { diff --git a/frappe/public/js/frappe/utils/utils.js b/frappe/public/js/frappe/utils/utils.js index 4a8e519978..e4cae790f2 100644 --- a/frappe/public/js/frappe/utils/utils.js +++ b/frappe/public/js/frappe/utils/utils.js @@ -1203,6 +1203,7 @@ Object.assign(frappe.utils, { attribution: '© OpenStreetMap contributors', }, + image_path: "/assets/frappe/images/leaflet/", }, icon(icon_name, size = "sm", icon_class = "", icon_style = "", svg_class = "") { diff --git a/frappe/public/js/frappe/views/map/map_view.js b/frappe/public/js/frappe/views/map/map_view.js index e9bd71e3fc..b54938bc87 100644 --- a/frappe/public/js/frappe/views/map/map_view.js +++ b/frappe/public/js/frappe/views/map/map_view.js @@ -32,7 +32,7 @@ frappe.views.MapView = class MapView extends frappe.views.ListView { this.$result.html(`
`); - L.Icon.Default.imagePath = "/assets/frappe/images/leaflet/"; + L.Icon.Default.imagePath = frappe.utils.map_defaults.image_path; this.map = L.map(this.map_id).setView( frappe.utils.map_defaults.center, frappe.utils.map_defaults.zoom From 5ed4e45f525e2fa8a576df1be4630ba365250d48 Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Wed, 19 Jul 2023 10:36:19 +0200 Subject: [PATCH 11/60] refactor(Geolocation): provide less utils --- frappe/public/js/frappe/form/controls/geolocation.js | 2 +- frappe/public/js/frappe/views/map/map_view.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/public/js/frappe/form/controls/geolocation.js b/frappe/public/js/frappe/form/controls/geolocation.js index 10ac21a5fb..56a8d2a073 100644 --- a/frappe/public/js/frappe/form/controls/geolocation.js +++ b/frappe/public/js/frappe/form/controls/geolocation.js @@ -1,4 +1,4 @@ -frappe.provide("frappe.utils.utils"); +frappe.provide("frappe.utils"); frappe.ui.form.ControlGeolocation = class ControlGeolocation extends frappe.ui.form.ControlData { static horizontal = false; diff --git a/frappe/public/js/frappe/views/map/map_view.js b/frappe/public/js/frappe/views/map/map_view.js index b54938bc87..b86faf3edc 100644 --- a/frappe/public/js/frappe/views/map/map_view.js +++ b/frappe/public/js/frappe/views/map/map_view.js @@ -1,7 +1,7 @@ /** * frappe.views.MapView */ -frappe.provide("frappe.utils.utils"); +frappe.provide("frappe.utils"); frappe.provide("frappe.views"); frappe.views.MapView = class MapView extends frappe.views.ListView { From 8ff913b8adc052481cfbecf004e59fc35e96c97d Mon Sep 17 00:00:00 2001 From: David Arnold Date: Sun, 23 Jul 2023 19:32:04 -0500 Subject: [PATCH 12/60] build(deps): update bleach 3.3.0 -> 6.0.0 --- frappe/utils/html_utils.py | 92 ++++++++++++++++++++------------------ pyproject.toml | 2 +- 2 files changed, 50 insertions(+), 44 deletions(-) diff --git a/frappe/utils/html_utils.py b/frappe/utils/html_utils.py index a109af798b..d3c101349f 100644 --- a/frappe/utils/html_utils.py +++ b/frappe/utils/html_utils.py @@ -19,13 +19,14 @@ EMOJI_PATTERN = re.compile( def clean_html(html): import bleach + from bleach.css_sanitizer import CSSSanitizer if not isinstance(html, str): return html return bleach.clean( clean_script_and_style(html), - tags=[ + tags={ "div", "p", "br", @@ -42,9 +43,9 @@ def clean_html(html): "tbody", "td", "tr", - ], + }, attributes=[], - styles=["color", "border", "border-color"], + css_sanitizer=CSSSanitizer(allowed_css_properties=["color", "border", "border-color"]), strip=True, strip_comments=True, ) @@ -52,44 +53,13 @@ def clean_html(html): def clean_email_html(html): import bleach + from bleach.css_sanitizer import CSSSanitizer if not isinstance(html, str): return html - return bleach.clean( - clean_script_and_style(html), - tags=[ - "div", - "p", - "br", - "ul", - "ol", - "li", - "strong", - "b", - "em", - "i", - "u", - "a", - "table", - "thead", - "tbody", - "td", - "tr", - "th", - "pre", - "code", - "h1", - "h2", - "h3", - "h4", - "h5", - "h6", - "button", - "img", - ], - attributes=["border", "colspan", "rowspan", "src", "href", "style", "id"], - styles=[ + css_sanitizer = CSSSanitizer( + allowed_css_properties=[ "color", "border-color", "width", @@ -121,7 +91,43 @@ def clean_email_html(html): "text-align", "vertical-align", "display", - ], + ] + ) + + return bleach.clean( + clean_script_and_style(html), + tags={ + "div", + "p", + "br", + "ul", + "ol", + "li", + "strong", + "b", + "em", + "i", + "u", + "a", + "table", + "thead", + "tbody", + "td", + "tr", + "th", + "pre", + "code", + "h1", + "h2", + "h3", + "h4", + "h5", + "h6", + "button", + "img", + }, + attributes=["border", "colspan", "rowspan", "src", "href", "style", "id"], + css_sanitizer=css_sanitizer, protocols=["cid", "http", "https", "mailto", "data"], strip=True, strip_comments=True, @@ -146,6 +152,7 @@ def sanitize_html(html, linkify=False): Does not sanitize JSON, as it could lead to future problems """ import bleach + from bleach.css_sanitizer import CSSSanitizer from bs4 import BeautifulSoup if not isinstance(html, str): @@ -170,17 +177,16 @@ def sanitize_html(html, linkify=False): return name in acceptable_attributes attributes = {"*": attributes_filter, "svg": svg_attributes} - styles = bleach_allowlist.all_styles - strip_comments = False + css_sanitizer = CSSSanitizer(allowed_css_properties=bleach_allowlist.all_styles) # returns html with escaped tags, escaped orphan >, <, etc. escaped_html = bleach.clean( html, tags=tags, attributes=attributes, - styles=styles, - strip_comments=strip_comments, - protocols=["cid", "http", "https", "mailto"], + css_sanitizer=css_sanitizer, + strip_comments=False, + protocols={"cid", "http", "https", "mailto"}, ) return escaped_html diff --git a/pyproject.toml b/pyproject.toml index b2b890c24c..33e0ff9f1a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -28,7 +28,7 @@ dependencies = [ "Whoosh~=2.7.4", "beautifulsoup4~=4.12.2", "bleach-allowlist~=1.0.3", - "bleach~=3.3.0", + "bleach[css]~=6.0.0", "cairocffi==1.5.1", "chardet~=5.1.0", "croniter~=1.3.15", From a4385be70ce0f514a4d4b8712d29e04347597ab6 Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Mon, 24 Jul 2023 16:02:22 +0200 Subject: [PATCH 13/60] fix: pass email as value --- frappe/contacts/doctype/contact/test_contact.py | 2 ++ frappe/email/__init__.py | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/frappe/contacts/doctype/contact/test_contact.py b/frappe/contacts/doctype/contact/test_contact.py index 18f0d78732..b5f1c4bdf8 100644 --- a/frappe/contacts/doctype/contact/test_contact.py +++ b/frappe/contacts/doctype/contact/test_contact.py @@ -49,11 +49,13 @@ class TestContact(FrappeTestCase): # First time from database results = get_contact_list("_Test Supplier") self.assertEqual(results[0].label, "test_contact@example.com") + self.assertEqual(results[0].value, "test_contact@example.com") self.assertEqual(results[0].description, "_Test Contact For _Test Supplier") # Second time from cache results = get_contact_list("_Test Supplier") self.assertEqual(results[0].label, "test_contact@example.com") + self.assertEqual(results[0].value, "test_contact@example.com") self.assertEqual(results[0].description, "_Test Contact For _Test Supplier") diff --git a/frappe/email/__init__.py b/frappe/email/__init__.py index 463f54d7e0..666c726942 100644 --- a/frappe/email/__init__.py +++ b/frappe/email/__init__.py @@ -16,7 +16,7 @@ def get_contact_list(txt, page_length=20) -> list[dict]: if cached_contacts := get_cached_contacts(txt): return cached_contacts[:page_length] - fields = ["name", "first_name", "middle_name", "last_name", "company_name"] + fields = ["first_name", "middle_name", "last_name", "company_name"] contacts = frappe.get_list( "Contact", fields=fields + ["`tabContact Email`.email_id"], @@ -33,7 +33,7 @@ def get_contact_list(txt, page_length=20) -> list[dict]: # https://github.com/frappe/frappe/blob/6c6a89bcdd9454060a1333e23b855d0505c9ebc2/frappe/public/js/frappe/form/controls/autocomplete.js#L29-L35 result = [ frappe._dict( - value=d.name, + value=d.email_id, label=d.email_id, description=get_full_name(d.first_name, d.middle_name, d.last_name, d.company_name), ) From f775c014e49b5e824827ee1e81cd4e4c7c20ebe8 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 25 Jul 2023 13:06:49 +0530 Subject: [PATCH 14/60] chore: simple codeowners [skip ci] --- CODEOWNERS | 6 ------ 1 file changed, 6 deletions(-) diff --git a/CODEOWNERS b/CODEOWNERS index 861016710a..e636e6c9fb 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -4,9 +4,3 @@ # the repo. Unless a later match takes precedence, * @frappe/frappe-review-team -templates/ @surajshetty3416 -www/ @surajshetty3416 -patches/ @surajshetty3416 -data_import* @netchampfaris -core/ @surajshetty3416 -workspace @shariquerik From 7780670ae44c5359e6e404075d52e65600e960bd Mon Sep 17 00:00:00 2001 From: David Arnold Date: Tue, 25 Jul 2023 03:37:46 -0500 Subject: [PATCH 15/60] build(deps): update node redis client to v4 (#21797) * build(deps): update redis client to v4 in legacy mode * fix: node17+ - prefer ipv4 * chore: use redis client v4 api (async) and adapt error handling * fix: timeout by exiting if not in watch mode * fix: parse message before republishing --------- Co-authored-by: Ankush Menat --- esbuild/esbuild.js | 67 ++++++++++++++++++++++++---------------------- node_utils.js | 8 +++++- package.json | 2 +- realtime/index.js | 29 ++++++++++---------- yarn.lock | 61 +++++++++++++++++------------------------ 5 files changed, 82 insertions(+), 85 deletions(-) diff --git a/esbuild/esbuild.js b/esbuild/esbuild.js index 8937a03216..e4a226281b 100644 --- a/esbuild/esbuild.js +++ b/esbuild/esbuild.js @@ -89,12 +89,10 @@ const NODE_PATHS = [].concat( app_list.map((app) => path.resolve(get_app_path(app), "..")).filter(fs.existsSync) ); -execute() - .then(() => RUN_BUILD_COMMAND && run_build_command_for_apps(APPS)) - .catch((e) => { - console.error(e); - process.exit(1); - }); +execute().catch((e) => { + console.error(e); + process.exit(1); +}); if (WATCH_MODE) { // listen for open files in editor event @@ -127,6 +125,10 @@ async function execute() { for (const result of results) { await write_assets_json(result.metafile); } + RUN_BUILD_COMMAND && run_build_command_for_apps(APPS); + if (!WATCH_MODE) { + process.exit(0); + } } function build_assets_for_apps(apps, files) { @@ -418,18 +420,18 @@ async function write_assets_json(metafile) { }; } -function update_assets_json_in_cache() { +async function update_assets_json_in_cache() { // update assets_json cache in redis, so that it can be read directly by python - return new Promise((resolve) => { - let client = get_redis_subscriber("redis_cache"); - // handle error event to avoid printing stack traces - client.on("error", (_) => { - log_warn("Cannot connect to redis_cache to update assets_json"); - }); - client.del("assets_json", (err) => { - client.unref(); - resolve(); - }); + let client = get_redis_subscriber("redis_cache"); + // handle error event to avoid printing stack traces + try { + await client.connect(); + } catch (e) { + log_warn("Cannot connect to redis_cache to update assets_json"); + } + client.del("assets_json", (err) => { + client.unref(); + resolve(); }); } @@ -458,9 +460,11 @@ function run_build_command_for_apps(apps) { async function notify_redis({ error, success, changed_files }) { // notify redis which in turns tells socketio to publish this to browser let subscriber = get_redis_subscriber("redis_queue"); - subscriber.on("error", (_) => { + try { + await subscriber.connect(); + } catch (e) { log_warn("Cannot connect to redis_queue for browser events"); - }); + } let payload = null; if (error) { @@ -483,7 +487,7 @@ async function notify_redis({ error, success, changed_files }) { }; } - subscriber.publish( + await subscriber.publish( "events", JSON.stringify({ event: "build_event", @@ -492,21 +496,20 @@ async function notify_redis({ error, success, changed_files }) { ); } -function open_in_editor() { +async function open_in_editor() { let subscriber = get_redis_subscriber("redis_queue"); - subscriber.on("error", (_) => { + try { + await subscriber.connect(); + } catch (e) { log_warn("Cannot connect to redis_queue for open_in_editor events"); + } + subscriber.subscribe("open_in_editor", (file) => { + file = JSON.parse(file); + let file_path = path.resolve(file.file); + log("Opening file in editor:", file_path); + let launch = require("launch-editor"); + launch(`${file_path}:${file.line}:${file.column}`); }); - subscriber.on("message", (event, file) => { - if (event === "open_in_editor") { - file = JSON.parse(file); - let file_path = path.resolve(file.file); - log("Opening file in editor:", file_path); - let launch = require("launch-editor"); - launch(`${file_path}:${file.line}:${file.column}`); - } - }); - subscriber.subscribe("open_in_editor"); } function get_rebuilt_assets(prev_assets, new_assets) { diff --git a/node_utils.js b/node_utils.js index 14b6f3576b..38c9b1c1cc 100644 --- a/node_utils.js +++ b/node_utils.js @@ -1,8 +1,14 @@ const fs = require("fs"); const path = require("path"); -const redis = require("redis"); +const redis = require("@redis/client"); const bench_path = path.resolve(__dirname, "..", ".."); +const dns = require("dns"); + +// Since node17, node resolves to ipv6 unless system is configured otherwise. +// In Frappe context using ipv4 - 127.0.0.1 is fine. +dns.setDefaultResultOrder("ipv4first"); + function get_conf() { // defaults var conf = { diff --git a/package.json b/package.json index 2cd1e6894e..0794ae5d1d 100644 --- a/package.json +++ b/package.json @@ -22,6 +22,7 @@ "dependencies": { "@editorjs/editorjs": "^2.26.3", "@frappe/esbuild-plugin-postcss2": "^0.1.3", + "@redis/client": "^1.5.8", "@vue-flow/background": "^1.1.0", "@vue-flow/core": "^1.16.2", "@vue/component-compiler": "^4.2.4", @@ -63,7 +64,6 @@ "quill-image-resize": "^3.0.9", "quill-magic-url": "^3.0.0", "qz-tray": "^2.0.8", - "redis": "^3.1.1", "rtlcss": "^4.0.0", "sass": "^1.63.0", "showdown": "^2.1.0", diff --git a/realtime/index.js b/realtime/index.js index ecc6063ca4..c60ec66330 100644 --- a/realtime/index.js +++ b/realtime/index.js @@ -27,7 +27,8 @@ function on_connection(socket) { frappe_handlers(realtime, socket); // ESBUild "open in editor" on error - socket.on("open_in_editor", (data) => { + socket.on("open_in_editor", async (data) => { + await subscriber.connect(); subscriber.publish("open_in_editor", JSON.stringify(data)); }); } @@ -38,19 +39,19 @@ realtime.on("connection", on_connection); // Consume events sent from python via redis pub-sub channel. const subscriber = get_redis_subscriber(); -subscriber.on("message", function (_channel, message) { - message = JSON.parse(message); - - let namespace = "/" + message.namespace; - if (message.room) { - io.of(namespace).to(message.room).emit(message.event, message.message); - } else { - // publish to ALL sites only used for things like build event. - realtime.emit(message.event, message.message); - } -}); - -subscriber.subscribe("events"); +(async () => { + await subscriber.connect(); + subscriber.subscribe("events", (message) => { + message = JSON.parse(message); + let namespace = "/" + message.namespace; + if (message.room) { + io.of(namespace).to(message.room).emit(message.event, message.message); + } else { + // publish to ALL sites only used for things like build event. + realtime.emit(message.event, message.message); + } + }); +})(); // ======================= let port = conf.socketio_port; diff --git a/yarn.lock b/yarn.lock index e816b29656..f632157bbe 100644 --- a/yarn.lock +++ b/yarn.lock @@ -81,6 +81,15 @@ "@nodelib/fs.scandir" "2.1.5" fastq "^1.6.0" +"@redis/client@^1.5.8": + version "1.5.8" + resolved "https://registry.yarnpkg.com/@redis/client/-/client-1.5.8.tgz#a375ba7861825bd0d2dc512282b8bff7b98dbcb1" + integrity sha512-xzElwHIO6rBAqzPeVnCzgvrnBEcFL1P0w8P65VNLRkdVW8rOE58f52hdj0BDgmsdOm4f1EoXPZtH4Fh7M/qUpw== + dependencies: + cluster-key-slot "1.1.2" + generic-pool "3.9.0" + yallist "4.0.0" + "@socket.io/component-emitter@~3.1.0": version "3.1.0" resolved "https://registry.yarnpkg.com/@socket.io/component-emitter/-/component-emitter-3.1.0.tgz#96116f2a912e0c02817345b3c10751069920d553" @@ -592,6 +601,11 @@ clone@^2.1.1, clone@^2.1.2: resolved "https://registry.yarnpkg.com/clone/-/clone-2.1.2.tgz#1b7f4b9f591f1e8f83670401600345a02887435f" integrity sha512-3Pe/CF1Nn94hyhIYpjtiLhdCoEoz0DqQ+988E9gmeEdQZlojxnOb74wctFyuwWQHzqyf9X7C7MG8juUpqBJT8w== +cluster-key-slot@1.1.2: + version "1.1.2" + resolved "https://registry.yarnpkg.com/cluster-key-slot/-/cluster-key-slot-1.1.2.tgz#88ddaa46906e303b5de30d3153b7d9fe0a0c19ac" + integrity sha512-RMr0FhtfXemyinomL4hrWcYJxmX6deFdCxpJzhDttxgO1+bcCnkk+9drydLVDmAMG7NE6aN/fl4F7ucU/90gAA== + color-convert@^1.9.0: version "1.9.3" resolved "https://registry.yarnpkg.com/color-convert/-/color-convert-1.9.3.tgz#bb71850690e1f136567de629d2d5471deda4c1e8" @@ -973,11 +987,6 @@ delayed-stream@~1.0.0: resolved "https://registry.yarnpkg.com/delayed-stream/-/delayed-stream-1.0.0.tgz#df3ae199acadfb7d440aaae0b29e2272b24ec619" integrity sha512-ZySD7Nf91aLB0RxL4KGrKHBXl7Eds1DAmEdcoVawXnLD7SDhpNgtuII2aAkg7a7QS41jxPSZ17p4VdGnMHk3MQ== -denque@^1.5.0: - version "1.5.1" - resolved "https://registry.yarnpkg.com/denque/-/denque-1.5.1.tgz#07f670e29c9a78f8faecb2566a1e2c11929c5cbf" - integrity sha512-XwE+iZ4D6ZUB7mfYRMb5wByE8L74HCn30FBN7sWnXksWc1LO1bPDl67pBR9o/kC4z/xSNAwkMYcGgqDV3BE3Hw== - dezalgo@^1.0.4: version "1.0.4" resolved "https://registry.yarnpkg.com/dezalgo/-/dezalgo-1.0.4.tgz#751235260469084c132157dfa857f386d4c33d81" @@ -1457,6 +1466,11 @@ generic-names@^4.0.0: dependencies: loader-utils "^3.2.0" +generic-pool@3.9.0: + version "3.9.0" + resolved "https://registry.yarnpkg.com/generic-pool/-/generic-pool-3.9.0.tgz#36f4a678e963f4fdb8707eab050823abc4e8f5e4" + integrity sha512-hymDOu5B53XvN4QT9dBmZxPX4CWhBPPLguTZ9MMFeFa/Kg0xWVfylOVNlJji/E7yTZWFd/q9GO5TxDLq156D7g== + get-caller-file@^2.0.5: version "2.0.5" resolved "https://registry.yarnpkg.com/get-caller-file/-/get-caller-file-2.0.5.tgz#4f94412a82db32f36e3b0b9741f8a97feb031f7e" @@ -2874,33 +2888,6 @@ readdirp@~3.6.0: dependencies: picomatch "^2.2.1" -redis-commands@^1.7.0: - version "1.7.0" - resolved "https://registry.yarnpkg.com/redis-commands/-/redis-commands-1.7.0.tgz#15a6fea2d58281e27b1cd1acfb4b293e278c3a89" - integrity sha512-nJWqw3bTFy21hX/CPKHth6sfhZbdiHP6bTawSgQBlKOVRG7EZkfHbbHwQJnrE4vsQf0CMNE+3gJ4Fmm16vdVlQ== - -redis-errors@^1.0.0, redis-errors@^1.2.0: - version "1.2.0" - resolved "https://registry.yarnpkg.com/redis-errors/-/redis-errors-1.2.0.tgz#eb62d2adb15e4eaf4610c04afe1529384250abad" - integrity sha512-1qny3OExCf0UvUV/5wpYKf2YwPcOqXzkwKKSmKHiE6ZMQs5heeE/c8eXK+PNllPvmjgAbfnsbpkGZWy8cBpn9w== - -redis-parser@^3.0.0: - version "3.0.0" - resolved "https://registry.yarnpkg.com/redis-parser/-/redis-parser-3.0.0.tgz#b66d828cdcafe6b4b8a428a7def4c6bcac31c8b4" - integrity sha512-DJnGAeenTdpMEH6uAJRK/uiyEIH9WVsUmoLwzudwGJUwZPp80PDBWPHXSAGNPwNvIXAbe7MSUB1zQFugFml66A== - dependencies: - redis-errors "^1.0.0" - -redis@^3.1.1: - version "3.1.2" - resolved "https://registry.yarnpkg.com/redis/-/redis-3.1.2.tgz#766851117e80653d23e0ed536254677ab647638c" - integrity sha512-grn5KoZLr/qrRQVwoSkmzdbw6pwF+/rwODtrOr6vuBRiR/f3rjSTGupbF90Zpqm2oenix8Do6RV7pYEkGwlKkw== - dependencies: - denque "^1.5.0" - redis-commands "^1.7.0" - redis-errors "^1.2.0" - redis-parser "^3.0.0" - regexp.prototype.flags@^1.2.0, regexp.prototype.flags@^1.5.0: version "1.5.0" resolved "https://registry.yarnpkg.com/regexp.prototype.flags/-/regexp.prototype.flags-1.5.0.tgz#fe7ce25e7e4cca8db37b6634c8a2c7009199b9cb" @@ -3479,16 +3466,16 @@ y18n@^5.0.5: resolved "https://registry.yarnpkg.com/y18n/-/y18n-5.0.8.tgz#7f4934d0f7ca8c56f95314939ddcd2dd91ce1d55" integrity sha512-0pfFzegeDWJHJIAmTLRP2DwHjdF5s7jo9tuztdQxAhINCdvS+3nGINqPd00AphqJR/0LhANUS6/+7SCb98YOfA== +yallist@4.0.0, yallist@^4.0.0: + version "4.0.0" + resolved "https://registry.yarnpkg.com/yallist/-/yallist-4.0.0.tgz#9bb92790d9c0effec63be73519e11a35019a3a72" + integrity sha512-3wdGidZyq5PB084XLES5TpOSRA3wjXAlIWMhum2kRcv/41Sn2emQ0dycQW4uZXLejwKvg6EsvbdlVL+FYEct7A== + yallist@^2.1.2: version "2.1.2" resolved "https://registry.yarnpkg.com/yallist/-/yallist-2.1.2.tgz#1c11f9218f076089a47dd512f93c6699a6a81d52" integrity sha512-ncTzHV7NvsQZkYe1DW7cbDLm0YpzHmZF5r/iyP3ZnQtMiJ+pjzisCiMNI+Sj+xQF5pXhSHxSB3uDbsBTzY/c2A== -yallist@^4.0.0: - version "4.0.0" - resolved "https://registry.yarnpkg.com/yallist/-/yallist-4.0.0.tgz#9bb92790d9c0effec63be73519e11a35019a3a72" - integrity sha512-3wdGidZyq5PB084XLES5TpOSRA3wjXAlIWMhum2kRcv/41Sn2emQ0dycQW4uZXLejwKvg6EsvbdlVL+FYEct7A== - yaml@^1.10.2: version "1.10.2" resolved "https://registry.yarnpkg.com/yaml/-/yaml-1.10.2.tgz#2301c5ffbf12b467de8da2333a459e29e7920e4b" From c0d472e95ccf82986d4369247fcaa35fcbff6564 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Tue, 25 Jul 2023 16:21:59 +0530 Subject: [PATCH 16/60] refactor: Remove unnecessary code --- frappe/utils/html_utils.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/frappe/utils/html_utils.py b/frappe/utils/html_utils.py index d3c101349f..ca1cbc3a74 100644 --- a/frappe/utils/html_utils.py +++ b/frappe/utils/html_utils.py @@ -19,7 +19,6 @@ EMOJI_PATTERN = re.compile( def clean_html(html): import bleach - from bleach.css_sanitizer import CSSSanitizer if not isinstance(html, str): return html @@ -45,7 +44,6 @@ def clean_html(html): "tr", }, attributes=[], - css_sanitizer=CSSSanitizer(allowed_css_properties=["color", "border", "border-color"]), strip=True, strip_comments=True, ) From dff950a56d505899043a4f8b8499eb662087d092 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Tue, 25 Jul 2023 18:20:00 +0530 Subject: [PATCH 17/60] fix: Remove unused `resolve` in code fixes: https://github.com/frappe/frappe/actions/runs/5655856445/job/15321775151?pr=21782 --- esbuild/esbuild.js | 1 - 1 file changed, 1 deletion(-) diff --git a/esbuild/esbuild.js b/esbuild/esbuild.js index e4a226281b..e22613e0e9 100644 --- a/esbuild/esbuild.js +++ b/esbuild/esbuild.js @@ -431,7 +431,6 @@ async function update_assets_json_in_cache() { } client.del("assets_json", (err) => { client.unref(); - resolve(); }); } From 766e0b056b267defaffea1f3dd2ce7acb530c7be Mon Sep 17 00:00:00 2001 From: Corentin Flr <10946971+cogk@users.noreply.github.com> Date: Mon, 24 Jul 2023 16:12:48 +0200 Subject: [PATCH 18/60] fix(Form Builder): Fix label text font size --- .../public/js/form_builder/components/controls/CheckControl.vue | 2 +- .../public/js/form_builder/components/controls/DataControl.vue | 2 +- .../js/form_builder/components/controls/SelectControl.vue | 2 +- .../public/js/form_builder/components/controls/TextControl.vue | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/frappe/public/js/form_builder/components/controls/CheckControl.vue b/frappe/public/js/form_builder/components/controls/CheckControl.vue index 023bcebf66..fbdb76396d 100644 --- a/frappe/public/js/form_builder/components/controls/CheckControl.vue +++ b/frappe/public/js/form_builder/components/controls/CheckControl.vue @@ -6,7 +6,7 @@ let slots = useSlots();