From fd0564953b0d5dd32d563617b1894e48d0aa8c55 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 5 Apr 2022 15:30:46 +0530 Subject: [PATCH 01/28] fix(Kanban Board): Clear user settings & dpctype cache on_change Deletion of kanban boards may lead to 404s on clicking on the Kanban option in the views. This is due to the cache not being synced with the DB changes. Changed the hook to on_change since it gets triggered even in db_set. --- frappe/desk/doctype/kanban_board/kanban_board.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/frappe/desk/doctype/kanban_board/kanban_board.py b/frappe/desk/doctype/kanban_board/kanban_board.py index 97f529a061..a53045b096 100644 --- a/frappe/desk/doctype/kanban_board/kanban_board.py +++ b/frappe/desk/doctype/kanban_board/kanban_board.py @@ -12,8 +12,9 @@ class KanbanBoard(Document): def validate(self): self.validate_column_name() - def on_update(self): + def on_change(self): frappe.clear_cache(doctype=self.reference_doctype) + frappe.cache().delete_keys("_user_settings") def before_insert(self): for column in self.columns: From 540b34347c688b32ecc5538296f4c6a95dbab1ae Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 5 Apr 2022 15:39:29 +0530 Subject: [PATCH 02/28] fix: Don't update saved filters "silently" Blame on the filters would not be maintained since update_modified was unset. Also, this API allowed those without access to Kanban Boards to possibly pass "bad" filters and cause system to fail (possible DOS). Using client's set_value does a better job at tackling this. --- frappe/desk/doctype/kanban_board/kanban_board.py | 7 ------- frappe/public/js/frappe/views/kanban/kanban_view.js | 8 +------- 2 files changed, 1 insertion(+), 14 deletions(-) diff --git a/frappe/desk/doctype/kanban_board/kanban_board.py b/frappe/desk/doctype/kanban_board/kanban_board.py index a53045b096..ddb1b9ffec 100644 --- a/frappe/desk/doctype/kanban_board/kanban_board.py +++ b/frappe/desk/doctype/kanban_board/kanban_board.py @@ -239,10 +239,3 @@ def set_indicator(board_name, column_name, indicator): board.save() return board - - -@frappe.whitelist() -def save_filters(board_name, filters): - '''Save filters silently''' - frappe.db.set_value('Kanban Board', board_name, 'filters', - filters, update_modified=False) diff --git a/frappe/public/js/frappe/views/kanban/kanban_view.js b/frappe/public/js/frappe/views/kanban/kanban_view.js index 89d1d41836..5ce6da9d55 100644 --- a/frappe/public/js/frappe/views/kanban/kanban_view.js +++ b/frappe/public/js/frappe/views/kanban/kanban_view.js @@ -107,13 +107,7 @@ frappe.views.KanbanView = class KanbanView extends frappe.views.ListView { save_kanban_board_filters() { const filters = this.filter_area.get(); - frappe.call({ - method: 'frappe.desk.doctype.kanban_board.kanban_board.save_filters', - args: { - board_name: this.board_name, - filters: filters - } - }).then(r => { + frappe.db.set_value("Kanban Board", this.board_name, "filters", filters).then(r => { if (r.exc) { frappe.show_alert({ indicator: 'red', From 87c6cbeee2845e0b73b323afcf53c3d5143996e2 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 5 Apr 2022 17:30:17 +0530 Subject: [PATCH 03/28] fix: Check if kanban exists before switching route --- .../public/js/frappe/list/list_view_select.js | 26 ++++++++++++------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/frappe/public/js/frappe/list/list_view_select.js b/frappe/public/js/frappe/list/list_view_select.js index 54e88ea05b..475019d6c1 100644 --- a/frappe/public/js/frappe/list/list_view_select.js +++ b/frappe/public/js/frappe/list/list_view_select.js @@ -262,19 +262,27 @@ frappe.views.ListViewSelect = class ListViewSelect { setup_kanban_boards() { const last_opened_kanban = - frappe.model.user_settings[this.doctype]["Kanban"] && frappe.model.user_settings[this.doctype]["Kanban"] - .last_kanban_board; - if (last_opened_kanban) { - frappe.set_route( - "list", + ?.last_kanban_board; + + if (!last_opened_kanban) { + return frappe.views.KanbanView.show_kanban_dialog( this.doctype, - "kanban", - last_opened_kanban + true ); - } else { - frappe.views.KanbanView.show_kanban_dialog(this.doctype, true); } + frappe.db.exists("Kanban Board", last_opened_kanban).then(exists => { + if (exists) { + frappe.set_route( + "list", + this.doctype, + "kanban", + last_opened_kanban + ); + } else { + frappe.views.KanbanView.show_kanban_dialog(this.doctype, true); + } + }); } get_calendars() { From 13922966996d1013a3dcbdf937009800ef21adf5 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 5 Apr 2022 17:31:13 +0530 Subject: [PATCH 04/28] refactor: new Kanban dialog * Hide 'Save' button if no select fields exist * Show 'Customize Form' as primary button when no selects exist --- .../js/frappe/views/kanban/kanban_view.js | 43 +++++++++++-------- 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/frappe/public/js/frappe/views/kanban/kanban_view.js b/frappe/public/js/frappe/views/kanban/kanban_view.js index 5ce6da9d55..3bf3a16189 100644 --- a/frappe/public/js/frappe/views/kanban/kanban_view.js +++ b/frappe/public/js/frappe/views/kanban/kanban_view.js @@ -247,25 +247,36 @@ frappe.views.KanbanView.show_kanban_dialog = function (doctype, show_existing) { } function new_kanban_dialog(kanbans, show_existing) { + /* Kanban dialog can show either "Save" or "Customize Form" option depending if any Select fields exist in the DocType for Kanban creation + */ if (dialog) return dialog; - const fields = get_fields_for_dialog(kanbans.map(kanban => kanban.name), show_existing); - - let primary_action_label = __('Save'); + const dialog_fields = get_fields_for_dialog(kanbans.map(kanban => kanban.name), show_existing); + const select_fields = frappe.get_meta(doctype).fields.filter(df => { + return (df.fieldtype === 'Select') && (df.fieldname !== 'kanban_column') + }); + const to_save = select_fields.length > 0; + const primary_action_label = to_save ? __('Save') : __('Customize Form'); let primary_action = () => { - const values = dialog.get_values(); - if (!values.selected_kanban || values.selected_kanban == 'Create New Board') { - make_kanban_board(values.board_name, values.field_name, values.project) - .then(() => dialog.hide(), (err) => frappe.msgprint(err)); + if (to_save) { + const values = dialog.get_values(); + if (!values.selected_kanban || values.selected_kanban == 'Create New Board') { + make_kanban_board(values.board_name, values.field_name, values.project).then( + () => dialog.hide(), + (err) => frappe.msgprint(err) + ); + } else { + frappe.set_route(kanbans.find(kanban => kanban.name == values.selected_kanban).route); + } } else { - frappe.set_route(kanbans.find(kanban => kanban.name == values.selected_kanban).route); + frappe.set_route("Form", "Customize Form", {"doc_type": doctype}); } }; dialog = new frappe.ui.Dialog({ title: __('New Kanban Board'), - fields, + fields: dialog_fields, primary_action_label, primary_action }); @@ -274,6 +285,9 @@ frappe.views.KanbanView.show_kanban_dialog = function (doctype, show_existing) { function get_fields_for_dialog(kanban_options, show_existing = false) { kanban_options.push('Create New Board'); + const select_fields = frappe.get_meta(doctype).fields.filter(df => { + return df.fieldtype === 'Select' && df.fieldname !== 'kanban_column'; + }); let fields = [ { @@ -284,6 +298,7 @@ frappe.views.KanbanView.show_kanban_dialog = function (doctype, show_existing) { depends_on: `eval: ${show_existing}`, mandatory_depends_on: `eval: ${show_existing}`, options: kanban_options, + default: kanban_options[0] }, { fieldname: 'new_kanban_board_sb', @@ -309,13 +324,6 @@ frappe.views.KanbanView.show_kanban_dialog = function (doctype, show_existing) { }); } - const select_fields = - frappe.get_meta(doctype).fields - .filter(df => { - return df.fieldtype === 'Select' && - df.fieldname !== 'kanban_column'; - }); - if (select_fields.length > 0) { fields.push({ fieldtype: 'Select', @@ -333,9 +341,6 @@ frappe.views.KanbanView.show_kanban_dialog = function (doctype, show_existing) {

${__('No fields found that can be used as a Kanban Column. Use the Customize Form to add a Custom Field of type "Select".')}

- - ${__('Customize Form')} - ` }]; From 01e101d4b8c95bf868e41cac7ca5a65d47276735 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 12 Apr 2022 15:40:37 +0530 Subject: [PATCH 05/28] chore: Add typing and and style conflicts --- frappe/core/doctype/doctype/test_doctype.py | 49 ++++++++++++--------- frappe/model/document.py | 27 +++++++----- 2 files changed, 44 insertions(+), 32 deletions(-) diff --git a/frappe/core/doctype/doctype/test_doctype.py b/frappe/core/doctype/doctype/test_doctype.py index dc6d14b451..bc9a411ebd 100644 --- a/frappe/core/doctype/doctype/test_doctype.py +++ b/frappe/core/doctype/doctype/test_doctype.py @@ -1,6 +1,9 @@ # -*- coding: utf-8 -*- # Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors # License: MIT. See LICENSE +from typing import Dict, List, Optional +import unittest + import frappe import unittest from frappe.core.doctype.doctype.doctype import (UniqueFieldnameError, @@ -507,7 +510,7 @@ class TestDocType(unittest.TestCase): def test_autoincremented_doctype_transition(self): frappe.delete_doc("testy_autoinc_dt") - dt = new_doctype("testy_autoinc_dt", autoincremented=True).insert(ignore_permissions=True) + dt = new_doctype("testy_autoinc_dt", autoname="autoincrement").insert(ignore_permissions=True) dt.autoname = "hash" try: @@ -521,25 +524,31 @@ class TestDocType(unittest.TestCase): dt.delete(ignore_permissions=True) -def new_doctype(name, unique=0, depends_on='', fields=None, autoincremented=False): - doc = frappe.get_doc({ - "doctype": "DocType", - "module": "Core", - "custom": 1, - "fields": [{ - "label": "Some Field", - "fieldname": "some_fieldname", - "fieldtype": "Data", - "unique": unique, - "depends_on": depends_on, - }], - "permissions": [{ - "role": "System Manager", - "read": 1, - }], - "name": name, - "autoname": "autoincrement" if autoincremented else "" - }) +def new_doctype(name, unique: bool = False, depends_on: str = "", fields: Optional[List[Dict]] = None, **kwargs): + doc = frappe.get_doc( + { + "doctype": "DocType", + "module": "Core", + "custom": 1, + "fields": [ + { + "label": "Some Field", + "fieldname": "some_fieldname", + "fieldtype": "Data", + "unique": unique, + "depends_on": depends_on, + } + ], + "permissions": [ + { + "role": "System Manager", + "read": 1, + } + ], + "name": name, + **kwargs, + } + ) if fields: for f in fields: diff --git a/frappe/model/document.py b/frappe/model/document.py index 15e9c28d83..1d47957f67 100644 --- a/frappe/model/document.py +++ b/frappe/model/document.py @@ -3,6 +3,8 @@ import hashlib import json import time +from typing import List + from werkzeug.exceptions import NotFound import frappe @@ -21,8 +23,6 @@ from frappe.core.doctype.server_script.server_script_utils import run_server_scr from frappe.utils.data import get_absolute_url -# once_only validation -# methods def get_doc(*args, **kwargs): """returns a frappe.model.Document object. @@ -179,7 +179,7 @@ class Document(BaseDocument): if not self.has_permission(permtype): self.raise_no_permission_to(permlevel or permtype) - def has_permission(self, permtype="read", verbose=False): + def has_permission(self, permtype="read", verbose=False) -> bool: """Call `frappe.has_permission` if `self.flags.ignore_permissions` is not set. @@ -195,8 +195,15 @@ class Document(BaseDocument): frappe.flags.error_message = _('Insufficient Permission for {0}').format(self.doctype) raise frappe.PermissionError - def insert(self, ignore_permissions=None, ignore_links=None, ignore_if_duplicate=False, - ignore_mandatory=None, set_name=None, set_child_names=True): + def insert( + self, + ignore_permissions=None, + ignore_links=None, + ignore_if_duplicate=False, + ignore_mandatory=None, + set_name=None, + set_child_names=True, + ) -> "Document": """Insert the document in the database (as a new document). This will check for user permissions and execute `before_insert`, `validate`, `on_update`, `after_insert` methods if they are written. @@ -276,7 +283,7 @@ class Document(BaseDocument): """Wrapper for _save""" return self._save(*args, **kwargs) - def _save(self, ignore_permissions=None, ignore_version=None): + def _save(self, ignore_permissions=None, ignore_version=None) -> "Document": """Save the current document in the database in the **DocType**'s table or `tabSingles` (for single types). @@ -498,8 +505,7 @@ class Document(BaseDocument): self._save_passwords() self.validate_workflow() - children = self.get_all_children() - for d in children: + for d in self.get_all_children(): d._validate_data_fields() d._validate_selects() d._validate_non_negative() @@ -840,7 +846,7 @@ class Document(BaseDocument): frappe.throw(_("Cannot link cancelled document: {0}").format(msg), frappe.CancelledLinkError) - def get_all_children(self, parenttype=None): + def get_all_children(self, parenttype=None) -> List["Document"]: """Returns all children documents from **Table** type fields in a list.""" children = [] @@ -1399,6 +1405,3 @@ def execute_action(doctype, name, action, **kwargs): doc.add_comment('Comment', _('Action Failed') + '

' + msg) doc.notify_update() - - - From dfadf42d877e70c45746db45b0c43d822a1a933d Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 12 Apr 2022 19:05:06 +0530 Subject: [PATCH 06/28] test: Added tests for nestedset --- frappe/tests/test_nestedset.py | 235 +++++++++++++++++++++++++++++++++ 1 file changed, 235 insertions(+) create mode 100644 frappe/tests/test_nestedset.py diff --git a/frappe/tests/test_nestedset.py b/frappe/tests/test_nestedset.py new file mode 100644 index 0000000000..2d601ec185 --- /dev/null +++ b/frappe/tests/test_nestedset.py @@ -0,0 +1,235 @@ +# Copyright (c) 2022, Frappe Technologies Pvt. Ltd. and Contributors +# License: MIT. See LICENSE + +import frappe +from frappe.core.doctype.doctype.test_doctype import new_doctype +from frappe.query_builder import Field +from frappe.query_builder.functions import Max +from frappe.tests.utils import FrappeTestCase +from frappe.utils.nestedset import ( + NestedSetChildExistsError, + NestedSetInvalidMergeError, + NestedSetRecursionError, + get_descendants_of, + rebuild_tree, +) + +records = [ + { + "some_fieldname": "Root Node", + "parent_test_tree_doctype": None, + "is_group": 1, + }, + { + "some_fieldname": "Parent 1", + "parent_test_tree_doctype": "Root Node", + "is_group": 1, + }, + { + "some_fieldname": "Parent 2", + "parent_test_tree_doctype": "Root Node", + "is_group": 1, + }, + { + "some_fieldname": "Child 1", + "parent_test_tree_doctype": "Parent 1", + "is_group": 0, + }, + { + "some_fieldname": "Child 2", + "parent_test_tree_doctype": "Parent 1", + "is_group": 0, + }, + { + "some_fieldname": "Child 3", + "parent_test_tree_doctype": "Parent 2", + "is_group": 0, + }, +] + + +class NestedSetTestUtil: + def setup_test_doctype(self): + frappe.db.sql("delete from `tabDocType` where `name` = 'Test Tree DocType'") + frappe.db.sql_ddl("drop table if exists `tabTest Tree DocType`") + + self.tree_doctype = new_doctype( + "Test Tree DocType", is_tree=True, autoname="field:some_fieldname" + ) + self.tree_doctype.insert() + + for record in records: + d = frappe.new_doc("Test Tree DocType") + d.update(record) + d.insert() + + def teardown_test_doctype(self): + self.tree_doctype.delete() + frappe.db.sql_ddl("drop table if exists `tabTest Tree DocType`") + + def move_it_back(self): + parent_1 = frappe.get_doc("Test Tree DocType", "Parent 1") + parent_1.parent_test_tree_doctype = "Root Node" + parent_1.save() + + def get_no_of_children(self, record_name: str) -> int: + if not record_name: + return frappe.db.count("Test Tree DocType") + return len(get_descendants_of("Test Tree DocType", record_name, ignore_permissions=True)) + + +class TestNestedSet(FrappeTestCase): + @classmethod + def setUpClass(cls) -> None: + cls.nsu = NestedSetTestUtil() + cls.nsu.setup_test_doctype() + super().setUpClass() + + @classmethod + def tearDownClass(cls) -> None: + cls.nsu.teardown_test_doctype() + super().tearDownClass() + + def setUp(self) -> None: + frappe.db.rollback() + + def test_basic_tree(self): + global records + + min_lft = 1 + max_rgt = frappe.qb.from_("Test Tree DocType").select(Max(Field("rgt"))).run(pluck=True)[0] + + for record in records: + lft, rgt, parent_test_tree_doctype = frappe.db.get_value( + "Test Tree DocType", + record["some_fieldname"], + ["lft", "rgt", "parent_test_tree_doctype"], + ) + + if parent_test_tree_doctype: + parent_lft, parent_rgt = frappe.db.get_value( + "Test Tree DocType", parent_test_tree_doctype, ["lft", "rgt"] + ) + else: + # root + parent_lft = min_lft - 1 + parent_rgt = max_rgt + 1 + + self.assertTrue(lft) + self.assertTrue(rgt) + self.assertTrue(lft < rgt) + self.assertTrue(parent_lft < parent_rgt) + self.assertTrue(lft > parent_lft) + self.assertTrue(rgt < parent_rgt) + self.assertTrue(lft >= min_lft) + self.assertTrue(rgt <= max_rgt) + + no_of_children = self.nsu.get_no_of_children(record["some_fieldname"]) + self.assertTrue( + rgt == (lft + 1 + (2 * no_of_children)), + msg=(record, no_of_children, self.nsu.get_no_of_children(record["some_fieldname"])), + ) + + no_of_children = self.nsu.get_no_of_children(parent_test_tree_doctype) + self.assertTrue(parent_rgt == (parent_lft + 1 + (2 * no_of_children))) + + def test_recursion(self): + leaf_node = frappe.get_doc("Test Tree DocType", {"some_fieldname": "Parent 2"}) + leaf_node.parent_test_tree_doctype = "Child 3" + self.assertRaises(NestedSetRecursionError, leaf_node.save) + leaf_node.reload() + + def test_rebuild_tree(self): + rebuild_tree("Test Tree DocType", "parent_test_tree_doctype") + self.test_basic_tree() + + def test_move_group_into_another(self): + old_lft, old_rgt = frappe.db.get_value("Test Tree DocType", "Parent 2", ["lft", "rgt"]) + + parent_1 = frappe.get_doc("Test Tree DocType", "Parent 1") + lft, rgt = parent_1.lft, parent_1.rgt + + parent_1.parent_test_tree_doctype = "Parent 2" + parent_1.save() + self.test_basic_tree() + + # after move + new_lft, new_rgt = frappe.db.get_value("Test Tree DocType", "Parent 2", ["lft", "rgt"]) + + # lft should reduce + self.assertEqual(old_lft - new_lft, rgt - lft + 1) + + # adjacent siblings, hence rgt diff will be 0 + self.assertEqual(new_rgt - old_rgt, 0) + + self.nsu.move_it_back() + self.test_basic_tree() + + def test_move_leaf_into_another_group(self): + child_2 = frappe.get_doc("Test Tree DocType", "Child 2") + + # assert that child 2 is not already under parent 1 + parent_lft_old, parent_rgt_old = frappe.db.get_value( + "Test Tree DocType", "Parent 2", ["lft", "rgt"] + ) + self.assertTrue((parent_lft_old > child_2.lft) and (parent_rgt_old > child_2.rgt)) + + child_2.parent_test_tree_doctype = "Parent 2" + child_2.save() + self.test_basic_tree() + + # assert that child 2 is under parent 1 + parent_lft_new, parent_rgt_new = frappe.db.get_value( + "Test Tree DocType", "Parent 2", ["lft", "rgt"] + ) + self.assertFalse((parent_lft_new > child_2.lft) and (parent_rgt_new > child_2.rgt)) + + def test_delete_leaf(self): + global records + el = {"some_fieldname": "Child 1", "parent_test_tree_doctype": "Parent 1", "is_group": 0} + + child_1 = frappe.get_doc("Test Tree DocType", "Child 1") + child_1.delete() + records.remove(el) + + self.test_basic_tree() + + n = frappe.new_doc("Test Tree DocType") + n.update(el) + n.insert() + records.append(el) + + self.test_basic_tree() + + def test_delete_group(self): + # cannot delete group with child, but can delete leaf + with self.assertRaises(NestedSetChildExistsError): + frappe.delete_doc("Test Tree DocType", "Parent 1") + + def test_merge_groups(self): + global records + el = {"some_fieldname": "Parent 2", "parent_test_tree_doctype": "Root Node", "is_group": 1} + frappe.rename_doc("Test Tree DocType", "Parent 2", "Parent 1", merge=True) + records.remove(el) + self.test_basic_tree() + + def test_merge_leaves(self): + global records + el = {"some_fieldname": "Child 3", "parent_test_tree_doctype": "Parent 2", "is_group": 0} + + frappe.rename_doc( + "Test Tree DocType", + "Child 3", + "Child 2", + merge=True, + ) + records.remove(el) + self.test_basic_tree() + + def test_merge_leaf_into_group(self): + with self.assertRaises(NestedSetInvalidMergeError): + frappe.rename_doc("Test Tree DocType", "Child 1", "Parent 1", merge=True) + + def test_merge_group_into_leaf(self): + with self.assertRaises(NestedSetInvalidMergeError): + frappe.rename_doc("Test Tree DocType", "Parent 1", "Child 1", merge=True) From 70a8a49c9c9947d37b0dbec2eec9d7e9c33659c9 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 12 Apr 2022 19:06:35 +0530 Subject: [PATCH 07/28] fix: new_doctype API testing util Use kwargs instead of mapping defined kwargs to single actions --- frappe/tests/test_db.py | 2 +- frappe/tests/test_db_query.py | 2 +- frappe/tests/test_naming.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/frappe/tests/test_db.py b/frappe/tests/test_db.py index 624f346716..f722ad1d65 100644 --- a/frappe/tests/test_db.py +++ b/frappe/tests/test_db.py @@ -759,7 +759,7 @@ class TestDDLCommandsPost(unittest.TestCase): def test_sequence_table_creation(self): from frappe.core.doctype.doctype.test_doctype import new_doctype - dt = new_doctype("autoinc_dt_seq_test", autoincremented=True).insert(ignore_permissions=True) + dt = new_doctype("autoinc_dt_seq_test", autoname="autoincrement").insert(ignore_permissions=True) if frappe.db.db_type == "postgres": self.assertTrue( diff --git a/frappe/tests/test_db_query.py b/frappe/tests/test_db_query.py index 90b047b3cd..8bdd66a045 100644 --- a/frappe/tests/test_db_query.py +++ b/frappe/tests/test_db_query.py @@ -619,7 +619,7 @@ class TestReportview(unittest.TestCase): def test_cast_name(self): from frappe.core.doctype.doctype.test_doctype import new_doctype - dt = new_doctype("autoinc_dt_test", autoincremented=True).insert(ignore_permissions=True) + dt = new_doctype("autoinc_dt_test", autoname="autoincrement").insert(ignore_permissions=True) query = DatabaseQuery("autoinc_dt_test").execute( fields=["locate('1', `tabautoinc_dt_test`.`name`)", "`tabautoinc_dt_test`.`name`"], diff --git a/frappe/tests/test_naming.py b/frappe/tests/test_naming.py index e57d2ae4cd..33974e5d27 100644 --- a/frappe/tests/test_naming.py +++ b/frappe/tests/test_naming.py @@ -262,7 +262,7 @@ class TestNaming(unittest.TestCase): from frappe.core.doctype.doctype.test_doctype import new_doctype doctype = "autoinc_doctype" + frappe.generate_hash(length=5) - dt = new_doctype(doctype, autoincremented=True).insert(ignore_permissions=True) + dt = new_doctype(doctype, autoname="autoincrement").insert(ignore_permissions=True) for i in range(1, 20): self.assertEqual(frappe.new_doc(doctype).save(ignore_permissions=True).name, i) From 594e653347e3edef07d5074b7a4eb98ab48f1f37 Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Wed, 30 Mar 2022 10:15:34 +0530 Subject: [PATCH 08/28] refactor: improved `frappe._dict` --- frappe/__init__.py | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index 37c282f04a..e2c8fc5413 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -55,15 +55,9 @@ controllers = {} class _dict(dict): """dict like object that exposes keys as attributes""" - def __getattr__(self, key): - ret = self.get(key) - # "__deepcopy__" exception added to fix frappe#14833 via DFP - if not ret and key.startswith("__") and key != "__deepcopy__": - raise AttributeError() - return ret - - def __setattr__(self, key, value): - self[key] = value + __getattr__ = dict.get + __setattr__ = dict.__setitem__ + __delattr__ = dict.__delitem__ def __getstate__(self): return self @@ -73,11 +67,12 @@ class _dict(dict): def update(self, d): """update and return self -- the missing dict feature in python""" - super(_dict, self).update(d) + + super().update(d) return self def copy(self): - return _dict(dict(self).copy()) + return _dict(self) def _(msg, lang=None, context=None): From d2dfc8c3357682a1ad0366d5bbf85e5ce4c225f1 Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Mon, 4 Apr 2022 10:16:09 +0530 Subject: [PATCH 09/28] fix: define `__slots__` to avoid `__dict__` creation --- frappe/__init__.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index e2c8fc5413..10c8afbf23 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -55,20 +55,19 @@ controllers = {} class _dict(dict): """dict like object that exposes keys as attributes""" + __slots__ = () __getattr__ = dict.get __setattr__ = dict.__setitem__ __delattr__ = dict.__delitem__ + __setstate__ = dict.update def __getstate__(self): return self - def __setstate__(self, d): - self.update(d) - - def update(self, d): + def update(self, *args, **kwargs): """update and return self -- the missing dict feature in python""" - super().update(d) + super().update(*args, **kwargs) return self def copy(self): From 93051389b1cdf06fb379ff9d9bc70291fbbeca30 Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Fri, 8 Apr 2022 01:54:40 +0530 Subject: [PATCH 10/28] fix: use `isinstance` instead of looping over `__dict__` --- frappe/model/meta.py | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/frappe/model/meta.py b/frappe/model/meta.py index c363e63f4f..5031f7f901 100644 --- a/frappe/model/meta.py +++ b/frappe/model/meta.py @@ -134,24 +134,23 @@ class Meta(Document): def as_dict(self, no_nulls=False): def serialize(doc): out = {} - for key in doc.__dict__: - value = doc.__dict__.get(key) - + for key, value in doc.__dict__.items(): if isinstance(value, (list, tuple)): - if len(value) > 0 and hasattr(value[0], "__dict__"): - value = [serialize(d) for d in value] - else: + if not value or not isinstance(value[0], BaseDocument): # non standard list object, skip continue - if isinstance(value, (str, int, float, datetime, list, tuple)) or ( - not no_nulls and value is None + value = [serialize(d) for d in value] + + if ( + isinstance(value, (str, int, float, datetime, list, tuple)) + or (not no_nulls and value is None) ): out[key] = value # set empty lists for unset table fields for table_field in DOCTYPE_TABLE_FIELDS: - if not out.get(table_field.fieldname): + if out.get(table_field.fieldname) is None: out[table_field.fieldname] = [] return out From 38959637e75c9f1ed29de7fc79bf5d4e98a73e6c Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Wed, 13 Apr 2022 11:40:35 +0530 Subject: [PATCH 11/28] perf: move less expensive condition first Co-authored-by: gavin --- frappe/model/meta.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/model/meta.py b/frappe/model/meta.py index 5031f7f901..760182ac39 100644 --- a/frappe/model/meta.py +++ b/frappe/model/meta.py @@ -143,8 +143,8 @@ class Meta(Document): value = [serialize(d) for d in value] if ( - isinstance(value, (str, int, float, datetime, list, tuple)) - or (not no_nulls and value is None) + (not no_nulls and value is None) + or isinstance(value, (str, int, float, datetime, list, tuple)) ): out[key] = value From 8269e8dd4701b1e2385caafae37532e83c9633fb Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Wed, 13 Apr 2022 13:03:09 +0530 Subject: [PATCH 12/28] fix: ignore duplicate modules during install --- frappe/installer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/installer.py b/frappe/installer.py index 3ce2bdf293..b3a8bad589 100644 --- a/frappe/installer.py +++ b/frappe/installer.py @@ -579,7 +579,7 @@ def add_module_defs(app): d = frappe.new_doc("Module Def") d.app_name = app d.module_name = module - d.save(ignore_permissions=True) + d.insert(ignore_permissions=True, ignore_if_duplicate=True) def remove_missing_apps(): From 77879933ce113006dddf27e459972aff2dc9f5e8 Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Wed, 13 Apr 2022 13:21:01 +0530 Subject: [PATCH 13/28] style: make changes as per linter --- frappe/model/meta.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/frappe/model/meta.py b/frappe/model/meta.py index 760182ac39..407f7d0811 100644 --- a/frappe/model/meta.py +++ b/frappe/model/meta.py @@ -142,9 +142,8 @@ class Meta(Document): value = [serialize(d) for d in value] - if ( - (not no_nulls and value is None) - or isinstance(value, (str, int, float, datetime, list, tuple)) + if (not no_nulls and value is None) or isinstance( + value, (str, int, float, datetime, list, tuple) ): out[key] = value From dad19e2c3f4d28832b9fefc9c076ed88c28125a9 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Wed, 13 Apr 2022 13:51:51 +0530 Subject: [PATCH 14/28] fix: `required_apps` blocks app install --- frappe/installer.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/installer.py b/frappe/installer.py index b3a8bad589..634d6287f8 100644 --- a/frappe/installer.py +++ b/frappe/installer.py @@ -242,8 +242,8 @@ def install_app(name, verbose=False, set_as_patched=True): # install pre-requisites if app_hooks.required_apps: for app in app_hooks.required_apps: - name = parse_app_name(app) - install_app(name, verbose=verbose) + required_app = parse_app_name(app) + install_app(required_app, verbose=verbose) frappe.flags.in_install = name frappe.clear_cache() From 3ac6fd6ea47497aba772ff2a5f027856ecd61d8a Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Wed, 16 Feb 2022 14:31:41 +0530 Subject: [PATCH 15/28] fix: more flexible autocompletion api --- frappe/public/js/frappe/form/controls/code.js | 69 ++++++++++++------- 1 file changed, 44 insertions(+), 25 deletions(-) diff --git a/frappe/public/js/frappe/form/controls/code.js b/frappe/public/js/frappe/form/controls/code.js index 60805b75de..96243e8cec 100644 --- a/frappe/public/js/frappe/form/controls/code.js +++ b/frappe/public/js/frappe/form/controls/code.js @@ -54,17 +54,32 @@ frappe.ui.form.ControlCode = class ControlCode extends frappe.ui.form.ControlTex return this._autocompletions || []; }, set: (value) => { + let getter = value + if (typeof getter !== 'function') { + getter = () => value + } + if (!this._autocompletions) { + this._autocompletions = []; + } + this._autocompletions.push(getter); this.setup_autocompletion(); - this.df._autocompletions = value; } }); } - setup_autocompletion() { + setup_autocompletion(customGetCompletions) { if (this._autocompletion_setup) return; const ace = window.ace; - const get_autocompletions = () => this.df.autocompletions; + const get_autocompletions = () => { + let getters = this._autocompletions || []; + let completions = []; + for (let getter of getters) { + let values = getter() + completions.push(...values); + } + return completions; + } ace.config.loadModule("ace/ext/language_tools", langTools => { this.editor.setOptions({ @@ -73,31 +88,35 @@ frappe.ui.form.ControlCode = class ControlCode extends frappe.ui.form.ControlTex }); langTools.addCompleter({ - getCompletions: function(editor, session, pos, prefix, callback) { - if (prefix.length === 0) { - callback(null, []); - return; - } - let autocompletions = get_autocompletions(); - if (autocompletions.length) { - callback( - null, - autocompletions.map(a => { - if (typeof a === 'string') { - a = { value: a }; - } - return { - name: 'frappe', - value: a.value, - score: a.score - }; - }) - ); - } - } + getCompletions: customGetCompletions || getCompletions }); }); this._autocompletion_setup = true; + + function getCompletions(editor, session, pos, prefix, callback) { + if (prefix.length === 0) { + callback(null, []); + return; + } + let autocompletions = get_autocompletions(); + if (autocompletions.length) { + callback( + null, + autocompletions.map(a => { + if (typeof a === "string") { + a = { value: a }; + } + return { + name: "frappe", + value: a.value, + score: a.score, + meta: a.meta, + caption: a.caption + }; + }) + ); + } + } } refresh_height() { From f76542ffc44a9c431513a728c72240867424a6f4 Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Wed, 16 Feb 2022 14:31:53 +0530 Subject: [PATCH 16/28] fix: add image autocompletion in markdown fields --- frappe/public/js/frappe/form/form.js | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/frappe/public/js/frappe/form/form.js b/frappe/public/js/frappe/form/form.js index 6191e35073..b8a8195938 100644 --- a/frappe/public/js/frappe/form/form.js +++ b/frappe/public/js/frappe/form/form.js @@ -319,6 +319,25 @@ frappe.ui.form.Form = class FrappeForm { }); } + setup_image_autocompletions_in_markdown() { + this.fields.map(field => { + if (field.df.fieldtype === 'Markdown Editor') { + this.set_df_property(field.df.fieldname, 'autocompletions', () => { + let attachments = this.attachments.get_attachments(); + return attachments + .filter(file => frappe.utils.is_image_file(file.file_url)) + .map(file => { + return { + caption: 'image: ' + file.file_name, + value: `![](${file.file_url})`, + meta: 'image' + } + }); + }); + } + }) + } + // REFRESH refresh(docname) { @@ -533,6 +552,7 @@ frappe.ui.form.Form = class FrappeForm { // call onload post render for callbacks to be fired () => { if(this.cscript.is_onload) { + this.onload_post_render(); return this.script_manager.trigger("onload_post_render"); } }, @@ -560,6 +580,10 @@ frappe.ui.form.Form = class FrappeForm { }); } + onload_post_render() { + this.setup_image_autocompletions_in_markdown(); + } + set_first_tab_as_active() { this.layout.tabs[0] && this.layout.tabs[0].set_active(); From aa1fefee2851c9010591ffe844447d5165097f02 Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Wed, 16 Feb 2022 14:42:12 +0530 Subject: [PATCH 17/28] fix: pass context to autocompletions getter --- frappe/public/js/frappe/form/controls/code.js | 44 +++++++++---------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/frappe/public/js/frappe/form/controls/code.js b/frappe/public/js/frappe/form/controls/code.js index 96243e8cec..42168282a3 100644 --- a/frappe/public/js/frappe/form/controls/code.js +++ b/frappe/public/js/frappe/form/controls/code.js @@ -71,33 +71,21 @@ frappe.ui.form.ControlCode = class ControlCode extends frappe.ui.form.ControlTex if (this._autocompletion_setup) return; const ace = window.ace; - const get_autocompletions = () => { - let getters = this._autocompletions || []; - let completions = []; - for (let getter of getters) { - let values = getter() - completions.push(...values); - } - return completions; - } - ace.config.loadModule("ace/ext/language_tools", langTools => { - this.editor.setOptions({ - enableBasicAutocompletion: true, - enableLiveAutocompletion: true - }); - - langTools.addCompleter({ - getCompletions: customGetCompletions || getCompletions - }); - }); - this._autocompletion_setup = true; - - function getCompletions(editor, session, pos, prefix, callback) { + let getCompletions = (editor, session, pos, prefix, callback) => { if (prefix.length === 0) { callback(null, []); return; } + const get_autocompletions = () => { + let getters = this._autocompletions || []; + let completions = []; + for (let getter of getters) { + let values = getter({ editor, session, pos, prefix }); + completions.push(...values); + } + return completions; + } let autocompletions = get_autocompletions(); if (autocompletions.length) { callback( @@ -117,6 +105,18 @@ frappe.ui.form.ControlCode = class ControlCode extends frappe.ui.form.ControlTex ); } } + + ace.config.loadModule("ace/ext/language_tools", langTools => { + this.editor.setOptions({ + enableBasicAutocompletion: true, + enableLiveAutocompletion: true + }); + + langTools.addCompleter({ + getCompletions: customGetCompletions || getCompletions + }); + }); + this._autocompletion_setup = true; } refresh_height() { From 9c0223650c256d19ca9c4a9ffd16016747984530 Mon Sep 17 00:00:00 2001 From: Shariq Ansari <30859809+shariquerik@users.noreply.github.com> Date: Wed, 13 Apr 2022 14:20:56 +0530 Subject: [PATCH 18/28] fix: Email Dialog 'To' field is not full width (#16603) --- frappe/public/scss/common/modal.scss | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/frappe/public/scss/common/modal.scss b/frappe/public/scss/common/modal.scss index 0de34f4ae4..8e69a956e5 100644 --- a/frappe/public/scss/common/modal.scss +++ b/frappe/public/scss/common/modal.scss @@ -211,21 +211,18 @@ body.modal-open[style^="padding-right"] { display: flex; align-items: center; - .frappe-control { + .frappe-control:first-child { &[data-fieldname="sender"] { - flex: 1; - margin-bottom: 0px; + margin-right: 10px; } - &[data-fieldname="recipients"] { - margin-left: 10px; - } - &[data-fieldname="option_toggle_button"] { - margin-left: 10px; - margin-bottom: -24px; - button { - // same as form-control input - height: calc(1.5em + .75rem + 2px); - } + flex: 1; + } + .frappe-control:last-child { + margin-left: 10px; + margin-bottom: -24px; + button { + // same as form-control input + height: calc(1.5em + .75rem + 2px); } } } From ff1edcd217f4b25ebb9b466a6378ece2cccc1e4f Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Wed, 13 Apr 2022 16:55:33 +0530 Subject: [PATCH 19/28] feat: Drop images into Markdown fields --- .../frappe/form/controls/markdown_editor.js | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/frappe/public/js/frappe/form/controls/markdown_editor.js b/frappe/public/js/frappe/form/controls/markdown_editor.js index 5acf4bd467..0f723d626f 100644 --- a/frappe/public/js/frappe/form/controls/markdown_editor.js +++ b/frappe/public/js/frappe/form/controls/markdown_editor.js @@ -29,6 +29,8 @@ frappe.ui.form.ControlMarkdownEditor = class ControlMarkdownEditor extends frapp this.markdown_preview = $(`
`).hide(); this.markdown_container.append(this.markdown_preview); + + this.setup_image_drop(); } set_language() { @@ -53,4 +55,45 @@ frappe.ui.form.ControlMarkdownEditor = class ControlMarkdownEditor extends frapp set_disp_area(value) { this.disp_area && $(this.disp_area).text(value); } + + setup_image_drop() { + this.ace_editor_target.on('drop', e => { + e.stopPropagation(); + e.preventDefault(); + let { dataTransfer } = e.originalEvent; + if (!dataTransfer?.files?.length) { + return; + } + let files = dataTransfer.files; + if (!files[0].type.includes('image')) { + frappe.show_alert({ + message: __('You can only insert images in Markdown fields', [files[0].name]), + indicator: 'orange' + }); + return; + } + + new frappe.ui.FileUploader({ + dialog_title: __('Insert Image in Markdown'), + doctype: this.doctype, + docname: this.docname, + frm: this.frm, + files, + folder: 'Home/Attachments', + allow_multiple: false, + restrictions: { + allowed_file_types: ['image/*'] + }, + on_success: (file_doc) => { + if (this.frm) { + this.frm.attachments.attachment_uploaded(file_doc); + } + this.editor.session.insert( + this.editor.getCursorPosition(), + `![](${encodeURI(file_doc.file_url)})` + ); + } + }); + }); + } }; From 13cc0b9346a960604cee0eb2f537f47ae2932ef1 Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Wed, 13 Apr 2022 16:57:22 +0530 Subject: [PATCH 20/28] fix: FileUploader - New option: crop_image_aspect_ratio to force an aspect ratio during cropping - ImageCropper: Add aspect ratio buttons --- .../js/frappe/file_uploader/FileUploader.vue | 24 ++++-- .../js/frappe/file_uploader/ImageCropper.vue | 83 +++++++++++++++---- .../public/js/frappe/file_uploader/index.js | 20 +++-- .../public/js/frappe/form/controls/attach.js | 6 +- .../js/frappe/form/controls/attach_image.js | 1 - 5 files changed, 99 insertions(+), 35 deletions(-) diff --git a/frappe/public/js/frappe/file_uploader/FileUploader.vue b/frappe/public/js/frappe/file_uploader/FileUploader.vue index ff1afcdd3c..6c816c1115 100644 --- a/frappe/public/js/frappe/file_uploader/FileUploader.vue +++ b/frappe/public/js/frappe/file_uploader/FileUploader.vue @@ -36,7 +36,7 @@ ref="file_input" @change="on_file_input" :multiple="allow_multiple" - :accept="restrictions.allowed_file_types.join(', ')" + :accept="(restrictions.allowed_file_types || []).join(', ')" >
@@ -171,7 +171,8 @@ export default { default: () => ({ max_file_size: null, // 2048 -> 2KB max_number_of_files: null, - allowed_file_types: [] // ['image/*', 'video/*', '.jpg', '.gif', '.pdf'] + allowed_file_types: [], // ['image/*', 'video/*', '.jpg', '.gif', '.pdf'], + crop_image_aspect_ratio: null // 1, 16 / 9, 4 / 3, NaN (free) }) }, attach_doc_image: { @@ -203,7 +204,8 @@ export default { allow_web_link: true, google_drive_settings: { enabled: false - } + }, + wrapper_ready: false } }, created() { @@ -286,11 +288,12 @@ export default { .filter(this.check_restrictions) .map(file => { let is_image = file.type.startsWith('image'); + let size_kb = file.size / 1024; return { file_obj: file, cropper_file: file, crop_box_data: null, - optimize: this.attach_doc_image ? true : false, + optimize: size_kb > 200 && is_image && !file.type.includes('svg'), name: file.name, doc: null, progress: 0, @@ -303,12 +306,15 @@ export default { } }); this.files = this.files.concat(files); - if(this.files.length != 0 && this.attach_doc_image) { - this.toggle_image_cropper(0); + // if only one file is allowed and crop_image_aspect_ratio is set, open cropper immediately + if (this.files.length === 1 && !this.allow_multiple && this.restrictions.crop_image_aspect_ratio != null) { + if (!this.files[0].file_obj.type.includes('svg')) { + this.toggle_image_cropper(0); + } } }, check_restrictions(file) { - let { max_file_size, allowed_file_types } = this.restrictions; + let { max_file_size, allowed_file_types = [] } = this.restrictions; let mime_type = file.type; let extension = '.' + file.name.split('.').pop(); diff --git a/frappe/public/js/frappe/file_uploader/ImageCropper.vue b/frappe/public/js/frappe/file_uploader/ImageCropper.vue index 09b50390fe..0711cc7cc9 100644 --- a/frappe/public/js/frappe/file_uploader/ImageCropper.vue +++ b/frappe/public/js/frappe/file_uploader/ImageCropper.vue @@ -1,12 +1,39 @@ @@ -15,22 +42,31 @@ import Cropper from "cropperjs"; export default { name: "ImageCropper", - props: ["file", "attach_doc_image"], + props: ["file", "fixed_aspect_ratio"], data() { + let aspect_ratio = + this.fixed_aspect_ratio != null ? this.fixed_aspect_ratio : NaN; return { src: null, cropper: null, - image: null + image: null, + aspect_ratio }; }, + watch: { + aspect_ratio(value) { + if (this.cropper) { + this.cropper.setAspectRatio(value); + } + } + }, mounted() { if (window.FileReader) { let fr = new FileReader(); fr.onload = () => (this.src = fr.result); fr.readAsDataURL(this.file.cropper_file); } - aspect_ratio = this.attach_doc_image ? 1 : NaN; - crop_box = this.file.crop_box_data; + let crop_box = this.file.crop_box_data; this.image = this.$refs.image; this.image.onload = () => { this.cropper = new Cropper(this.image, { @@ -38,13 +74,31 @@ export default { scalable: false, viewMode: 1, data: crop_box, - aspectRatio: aspect_ratio + aspectRatio: this.aspect_ratio }); + window.cropper = this.cropper; }; }, computed: { - crop_button_text() { - return this.attach_doc_image ? "Upload" : "Crop"; + aspect_ratio_buttons() { + return [ + { + label: __("1:1"), + value: 1 + }, + { + label: __("4:3"), + value: 4 / 3 + }, + { + label: __("16:9"), + value: 16 / 9 + }, + { + label: __("Free"), + value: NaN + } + ]; } }, methods: { @@ -58,9 +112,6 @@ export default { }); this.file.file_obj = cropped_file_obj; this.$emit("toggle_image_cropper"); - if(this.attach_doc_image) { - this.$emit("upload_after_crop"); - } }, file_type); } } @@ -75,6 +126,8 @@ img { .image-cropper-actions { display: flex; - justify-content: flex-end; + align-items: center; + justify-content: space-between; + margin-top: var(--margin-md); } diff --git a/frappe/public/js/frappe/file_uploader/index.js b/frappe/public/js/frappe/file_uploader/index.js index ec90b19a1a..ab074c938d 100644 --- a/frappe/public/js/frappe/file_uploader/index.js +++ b/frappe/public/js/frappe/file_uploader/index.js @@ -10,11 +10,12 @@ export default class FileUploader { fieldname, files, folder, - restrictions, + restrictions = {}, upload_notes, allow_multiple, as_dataurl, disable_file_browser, + dialog_title, attach_doc_image, frm } = {}) { @@ -22,15 +23,11 @@ export default class FileUploader { frm && frm.attachments.max_reached(true); if (!wrapper) { - this.make_dialog(); + this.make_dialog(dialog_title); } else { this.wrapper = wrapper.get ? wrapper.get(0) : wrapper; } - if (attach_doc_image) { - restrictions.allowed_file_types = ['image/jpeg', 'image/png']; - } - this.$fileuploader = new Vue({ el: this.wrapper, render: h => h(FileUploaderComponent, { @@ -54,6 +51,10 @@ export default class FileUploader { this.uploader = this.$fileuploader.$children[0]; + if (!this.dialog) { + this.uploader.wrapper_ready = true; + } + this.uploader.$watch('files', (files) => { let all_private = files.every(file => file.private); if (this.dialog) { @@ -94,14 +95,17 @@ export default class FileUploader { return this.uploader.upload_files(); } - make_dialog() { + make_dialog(title) { this.dialog = new frappe.ui.Dialog({ - title: __('Upload'), + title: title || __('Upload'), primary_action_label: __('Upload'), primary_action: () => this.upload_files(), secondary_action_label: __('Set all private'), secondary_action: () => { this.uploader.toggle_all_private(); + }, + on_page_show: () => { + this.uploader.wrapper_ready = true; } }); diff --git a/frappe/public/js/frappe/form/controls/attach.js b/frappe/public/js/frappe/form/controls/attach.js index a91058a208..71ce2e8854 100644 --- a/frappe/public/js/frappe/form/controls/attach.js +++ b/frappe/public/js/frappe/form/controls/attach.js @@ -61,7 +61,8 @@ frappe.ui.form.ControlAttach = class ControlAttach extends frappe.ui.form.Contro } on_attach_doc_image() { this.set_upload_options(); - this.upload_options["attach_doc_image"] = true; + this.upload_options.restrictions.allowed_file_types = ['image/*']; + this.upload_options.restrictions.crop_image_aspect_ratio = 1; this.file_uploader = new frappe.ui.FileUploader(this.upload_options); } set_upload_options() { @@ -70,7 +71,8 @@ frappe.ui.form.ControlAttach = class ControlAttach extends frappe.ui.form.Contro on_success: file => { this.on_upload_complete(file); this.toggle_reload_button(); - } + }, + restrictions: {} }; if (this.frm) { diff --git a/frappe/public/js/frappe/form/controls/attach_image.js b/frappe/public/js/frappe/form/controls/attach_image.js index 7c24ec9551..0bae2e6241 100644 --- a/frappe/public/js/frappe/form/controls/attach_image.js +++ b/frappe/public/js/frappe/form/controls/attach_image.js @@ -19,7 +19,6 @@ frappe.ui.form.ControlAttachImage = class ControlAttachImage extends frappe.ui.f } set_upload_options() { super.set_upload_options(); - this.upload_options.restrictions = {}; this.upload_options.restrictions.allowed_file_types = ['image/*']; } }; From 03d542edce8dd87f956c26d583180e49126ba683 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Wed, 13 Apr 2022 17:32:58 +0530 Subject: [PATCH 21/28] fix!: Allow child table naming --- frappe/model/naming.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/frappe/model/naming.py b/frappe/model/naming.py index eb755851fb..9d1079d995 100644 --- a/frappe/model/naming.py +++ b/frappe/model/naming.py @@ -46,9 +46,6 @@ def set_new_name(doc): elif getattr(doc.meta, "issingle", False): doc.name = doc.doctype - elif getattr(doc.meta, "istable", False): - doc.name = make_autoname("hash", doc.doctype) - if not doc.name: set_naming_from_document_naming_rule(doc) From f4c363034e506580c556ffc2fa21ec921dd6ea0f Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Wed, 13 Apr 2022 18:40:32 +0530 Subject: [PATCH 22/28] style: formatting --- frappe/public/js/frappe/form/controls/code.js | 8 ++++---- frappe/public/js/frappe/form/form.js | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/frappe/public/js/frappe/form/controls/code.js b/frappe/public/js/frappe/form/controls/code.js index 42168282a3..1ed37d7d17 100644 --- a/frappe/public/js/frappe/form/controls/code.js +++ b/frappe/public/js/frappe/form/controls/code.js @@ -54,9 +54,9 @@ frappe.ui.form.ControlCode = class ControlCode extends frappe.ui.form.ControlTex return this._autocompletions || []; }, set: (value) => { - let getter = value + let getter = value; if (typeof getter !== 'function') { - getter = () => value + getter = () => value; } if (!this._autocompletions) { this._autocompletions = []; @@ -85,7 +85,7 @@ frappe.ui.form.ControlCode = class ControlCode extends frappe.ui.form.ControlTex completions.push(...values); } return completions; - } + }; let autocompletions = get_autocompletions(); if (autocompletions.length) { callback( @@ -104,7 +104,7 @@ frappe.ui.form.ControlCode = class ControlCode extends frappe.ui.form.ControlTex }) ); } - } + }; ace.config.loadModule("ace/ext/language_tools", langTools => { this.editor.setOptions({ diff --git a/frappe/public/js/frappe/form/form.js b/frappe/public/js/frappe/form/form.js index b8a8195938..0c8939cf5d 100644 --- a/frappe/public/js/frappe/form/form.js +++ b/frappe/public/js/frappe/form/form.js @@ -331,11 +331,11 @@ frappe.ui.form.Form = class FrappeForm { caption: 'image: ' + file.file_name, value: `![](${file.file_url})`, meta: 'image' - } + }; }); }); } - }) + }); } // REFRESH From 3cd047becfa2f1335320a4c5ec516f54787ab282 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Wed, 13 Apr 2022 19:22:46 +0530 Subject: [PATCH 23/28] fix: Obey force kwarg frappe.reload_doc --- frappe/modules/import_file.py | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/frappe/modules/import_file.py b/frappe/modules/import_file.py index 9cfbe163a5..00483bf6a5 100644 --- a/frappe/modules/import_file.py +++ b/frappe/modules/import_file.py @@ -107,7 +107,6 @@ def import_file_by_path( Returns: [bool]: True if import takes place. False if it wasn't imported. """ - frappe.flags.dt = frappe.flags.dt or [] try: docs = read_doc_from_file(path) except IOError: @@ -121,20 +120,19 @@ def import_file_by_path( docs = [docs] for doc in docs: - # modified timestamp in db, none if doctype's first import db_modified_timestamp = frappe.db.get_value(doc["doctype"], doc["name"], "modified") is_db_timestamp_latest = db_modified_timestamp and ( get_datetime(doc.get("modified")) <= get_datetime(db_modified_timestamp) ) - if not force or db_modified_timestamp: - try: - stored_hash = None - if doc["doctype"] == "DocType": + if not force and db_modified_timestamp: + stored_hash = None + if doc["doctype"] == "DocType": + try: stored_hash = frappe.db.get_value(doc["doctype"], doc["name"], "migration_hash") - except Exception: - frappe.flags.dt += [doc["doctype"]] + except Exception: + pass # if hash exists and is equal no need to update if stored_hash and stored_hash == calculated_hash: @@ -172,12 +170,6 @@ def import_file_by_path( return True -def is_timestamp_changed(doc): - # check if timestamps match - db_modified = frappe.db.get_value(doc["doctype"], doc["name"], "modified") - return not (db_modified and get_datetime(doc.get("modified")) == get_datetime(db_modified)) - - def read_doc_from_file(path): doc = None if os.path.exists(path): From 2fbf8c905f76cef2950cd173d06ea7bc410b0884 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Wed, 13 Apr 2022 20:44:10 +0530 Subject: [PATCH 24/28] fix: dont override local proxies (#16611) --- frappe/desk/doctype/system_console/system_console.py | 2 +- frappe/desk/form/load.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/desk/doctype/system_console/system_console.py b/frappe/desk/doctype/system_console/system_console.py index ffebfe2dd0..f1324403c3 100644 --- a/frappe/desk/doctype/system_console/system_console.py +++ b/frappe/desk/doctype/system_console/system_console.py @@ -13,7 +13,7 @@ class SystemConsole(Document): def run(self): frappe.only_for("System Manager") try: - frappe.debug_log = [] + frappe.local.debug_log = [] if self.type == "Python": safe_exec(self.console) self.output = "\n".join(frappe.debug_log) diff --git a/frappe/desk/form/load.py b/frappe/desk/form/load.py index 636b662a09..75cd403aac 100644 --- a/frappe/desk/form/load.py +++ b/frappe/desk/form/load.py @@ -57,7 +57,7 @@ def getdoc(doctype, name, user=None): doc.add_seen() set_link_titles(doc) if frappe.response.docs is None: - frappe.response = _dict({"docs": []}) + frappe.local.response = _dict({"docs": []}) frappe.response.docs.append(doc) From 5740dbb447211b03b92b503ef99e61840ee21e26 Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Thu, 14 Apr 2022 00:46:08 +0530 Subject: [PATCH 25/28] fix: handle attaching images in new form --- frappe/public/js/frappe/form/controls/markdown_editor.js | 2 +- frappe/public/js/frappe/form/sidebar/attachments.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/public/js/frappe/form/controls/markdown_editor.js b/frappe/public/js/frappe/form/controls/markdown_editor.js index 0f723d626f..774976abc6 100644 --- a/frappe/public/js/frappe/form/controls/markdown_editor.js +++ b/frappe/public/js/frappe/form/controls/markdown_editor.js @@ -85,7 +85,7 @@ frappe.ui.form.ControlMarkdownEditor = class ControlMarkdownEditor extends frapp allowed_file_types: ['image/*'] }, on_success: (file_doc) => { - if (this.frm) { + if (this.frm && !this.frm.is_new()) { this.frm.attachments.attachment_uploaded(file_doc); } this.editor.session.insert( diff --git a/frappe/public/js/frappe/form/sidebar/attachments.js b/frappe/public/js/frappe/form/sidebar/attachments.js index 0713d5dc43..3663cfd9a5 100644 --- a/frappe/public/js/frappe/form/sidebar/attachments.js +++ b/frappe/public/js/frappe/form/sidebar/attachments.js @@ -62,7 +62,7 @@ frappe.ui.form.Attachments = class Attachments { } get_attachments() { - return this.frm.get_docinfo().attachments; + return this.frm.get_docinfo().attachments || []; } add_attachment(attachment) { var file_name = attachment.file_name; From 168bf6d3e454cfe6a8ed3751d553900b5a072fef Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Thu, 14 Apr 2022 00:46:32 +0530 Subject: [PATCH 26/28] test: ui test for inserting image in markdown --- .../integration/control_markdown_editor.js | 22 +++++++++++++++++++ cypress/support/commands.js | 3 +++ 2 files changed, 25 insertions(+) create mode 100644 cypress/integration/control_markdown_editor.js diff --git a/cypress/integration/control_markdown_editor.js b/cypress/integration/control_markdown_editor.js new file mode 100644 index 0000000000..b527d854d4 --- /dev/null +++ b/cypress/integration/control_markdown_editor.js @@ -0,0 +1,22 @@ +context("Control Markdown Editor", () => { + before(() => { + cy.login(); + cy.visit("/app"); + }); + + it("should allow inserting images by drag and drop", () => { + cy.visit("/app/web-page/new"); + cy.fill_field("content_type", "Markdown", "Select"); + cy.get_field("main_section_md", "Markdown Editor").attachFile( + "sample_image.jpg", + { + subjectType: "drag-n-drop" + } + ); + cy.click_modal_primary_button("Upload"); + cy.get_field("main_section_md", "Markdown Editor").should( + "contain", + "![](/files/sample_image.jpg)" + ); + }); +}); diff --git a/cypress/support/commands.js b/cypress/support/commands.js index 37134f0cbc..636312376d 100644 --- a/cypress/support/commands.js +++ b/cypress/support/commands.js @@ -174,6 +174,9 @@ Cypress.Commands.add('get_field', (fieldname, fieldtype = 'Data') => { if (fieldtype === 'Code') { selector = `[data-fieldname="${fieldname}"] .ace_text-input`; } + if (fieldtype === 'Markdown Editor') { + selector = `[data-fieldname="${fieldname}"] .ace-editor-target`; + } return cy.get(selector).first(); }); From 39f8267a157c0aa2010f8699e80d003799ddc4f1 Mon Sep 17 00:00:00 2001 From: Shridhar Patil Date: Thu, 14 Apr 2022 09:51:41 +0530 Subject: [PATCH 27/28] feat: added support for data type json (#16187) > Please provide enough information so that others can review your pull request: Added json support for postgres and mariadb > Explain the **details** for making this change. What existing problem does the pull request solve? https://github.com/frappe/frappe/projects/4#card-50160428 > Screenshots/GIFs ![json](https://user-images.githubusercontent.com/11792643/156367383-8f8492c2-3817-449d-a2dd-c983eeadbb48.gif) --- **Previous attempts:** https://github.com/frappe/frappe/pull/8128 https://github.com/frappe/frappe/pull/7096 Docs: https://frappeframework.com/docs/v13/user/en/basics/doctypes/fieldtypes#json --- frappe/core/doctype/docfield/docfield.json | 4 +-- frappe/core/doctype/doctype/test_doctype.py | 29 +++++++++++++++++++ frappe/database/mariadb/database.py | 1 + frappe/database/postgres/database.py | 1 + frappe/model/__init__.py | 1 + frappe/model/base_document.py | 4 +++ .../public/js/frappe/form/controls/control.js | 1 + frappe/public/js/frappe/form/controls/json.js | 6 ++++ 8 files changed, 45 insertions(+), 2 deletions(-) create mode 100644 frappe/public/js/frappe/form/controls/json.js diff --git a/frappe/core/doctype/docfield/docfield.json b/frappe/core/doctype/docfield/docfield.json index 3267429298..9e9aaf489b 100644 --- a/frappe/core/doctype/docfield/docfield.json +++ b/frappe/core/doctype/docfield/docfield.json @@ -99,7 +99,7 @@ "label": "Type", "oldfieldname": "fieldtype", "oldfieldtype": "Select", - "options": "Autocomplete\nAttach\nAttach Image\nBarcode\nButton\nCheck\nCode\nColor\nColumn Break\nCurrency\nData\nDate\nDatetime\nDuration\nDynamic Link\nFloat\nFold\nGeolocation\nHeading\nHTML\nHTML Editor\nIcon\nImage\nInt\nLink\nLong Text\nMarkdown Editor\nPassword\nPercent\nRead Only\nRating\nSection Break\nSelect\nSignature\nSmall Text\nTab Break\nTable\nTable MultiSelect\nText\nText Editor\nTime", + "options": "Autocomplete\nAttach\nAttach Image\nBarcode\nButton\nCheck\nCode\nColor\nColumn Break\nCurrency\nData\nDate\nDatetime\nDuration\nDynamic Link\nFloat\nFold\nGeolocation\nHeading\nHTML\nHTML Editor\nIcon\nImage\nInt\nJSON\nLink\nLong Text\nMarkdown Editor\nPassword\nPercent\nRead Only\nRating\nSection Break\nSelect\nSignature\nSmall Text\nTab Break\nTable\nTable MultiSelect\nText\nText Editor\nTime", "reqd": 1, "search_index": 1 }, @@ -547,7 +547,7 @@ "index_web_pages_for_search": 1, "istable": 1, "links": [], - "modified": "2022-02-14 11:56:19.812863", + "modified": "2022-03-02 17:07:32.117897", "modified_by": "Administrator", "module": "Core", "name": "DocField", diff --git a/frappe/core/doctype/doctype/test_doctype.py b/frappe/core/doctype/doctype/test_doctype.py index 031cbb553f..7b4806da59 100644 --- a/frappe/core/doctype/doctype/test_doctype.py +++ b/frappe/core/doctype/doctype/test_doctype.py @@ -538,6 +538,35 @@ class TestDocType(unittest.TestCase): # cleanup dt.delete(ignore_permissions=True) + def test_json_field(self): + """Test json field.""" + import json + + json_doc = new_doctype( + "Test Json Doctype", + fields=[{"label": "json field", "fieldname": "test_json_field", "fieldtype": "JSON"}], + ) + json_doc.insert() + json_doc.save() + doc = frappe.get_doc("DocType", "Test Json Doctype") + for field in doc.fields: + if field.fieldname == "test_json_field": + self.assertEqual(field.fieldtype, "JSON") + break + + doc = frappe.get_doc( + {"doctype": "Test Json Doctype", "test_json_field": json.dumps({"hello": "world"})} + ) + doc.insert() + doc.save() + + test_json = frappe.get_doc("Test Json Doctype", doc.name) + + if isinstance(test_json.test_json_field, str): + test_json.test_json_field = json.loads(test_json.test_json_field) + + self.assertEqual(test_json.test_json_field["hello"], "world") + def new_doctype( name, unique: bool = False, depends_on: str = "", fields: Optional[List[Dict]] = None, **kwargs diff --git a/frappe/database/mariadb/database.py b/frappe/database/mariadb/database.py index dcb88d648b..1ae3fd8a61 100644 --- a/frappe/database/mariadb/database.py +++ b/frappe/database/mariadb/database.py @@ -54,6 +54,7 @@ class MariaDBDatabase(Database): "Duration": ("decimal", "21,9"), "Icon": ("varchar", self.VARCHAR_LEN), "Autocomplete": ("varchar", self.VARCHAR_LEN), + "JSON": ("json", ""), } def get_connection(self): diff --git a/frappe/database/postgres/database.py b/frappe/database/postgres/database.py index c34c20ee7b..228d0f48be 100644 --- a/frappe/database/postgres/database.py +++ b/frappe/database/postgres/database.py @@ -66,6 +66,7 @@ class PostgresDatabase(Database): "Duration": ("decimal", "21,9"), "Icon": ("varchar", self.VARCHAR_LEN), "Autocomplete": ("varchar", self.VARCHAR_LEN), + "JSON": ("json", ""), } def get_connection(self): diff --git a/frappe/model/__init__.py b/frappe/model/__init__.py index 6c9bab2dff..bd607e7119 100644 --- a/frappe/model/__init__.py +++ b/frappe/model/__init__.py @@ -37,6 +37,7 @@ data_fieldtypes = ( "Duration", "Icon", "Autocomplete", + "JSON", ) attachment_fieldtypes = ( diff --git a/frappe/model/base_document.py b/frappe/model/base_document.py index d73464cb06..4f2ddd3bb6 100644 --- a/frappe/model/base_document.py +++ b/frappe/model/base_document.py @@ -1,6 +1,7 @@ # Copyright (c) 2022, Frappe Technologies Pvt. Ltd. and Contributors # License: MIT. See LICENSE import datetime +import json import frappe from frappe import _ @@ -287,6 +288,9 @@ class BaseDocument(object): elif df.fieldtype == "Int" and not isinstance(d[fieldname], int): d[fieldname] = cint(d[fieldname]) + elif df.fieldtype == "JSON" and isinstance(d[fieldname], dict): + d[fieldname] = json.dumps(d[fieldname], sort_keys=True, indent=4, separators=(",", ": ")) + elif df.fieldtype in ("Currency", "Float", "Percent") and not isinstance(d[fieldname], float): d[fieldname] = flt(d[fieldname]) diff --git a/frappe/public/js/frappe/form/controls/control.js b/frappe/public/js/frappe/form/controls/control.js index 90e8697b1c..8db382dd91 100644 --- a/frappe/public/js/frappe/form/controls/control.js +++ b/frappe/public/js/frappe/form/controls/control.js @@ -39,6 +39,7 @@ import './multiselect_list'; import './rating'; import './duration'; import './icon'; +import './json'; frappe.ui.form.make_control = function (opts) { var control_class_name = "Control" + opts.df.fieldtype.replace(/ /g, ""); diff --git a/frappe/public/js/frappe/form/controls/json.js b/frappe/public/js/frappe/form/controls/json.js new file mode 100644 index 0000000000..ce2e0bd087 --- /dev/null +++ b/frappe/public/js/frappe/form/controls/json.js @@ -0,0 +1,6 @@ +frappe.ui.form.ControlJSON = class ControlCode extends frappe.ui.form.ControlCode { + set_language() { + this.editor.session.setMode('ace/mode/json'); + this.editor.setKeyboardHandler('ace/keyboard/vscode'); + } +}; From 5564f1e0072947fa92292ca3ec0bc6a7cd24b89c Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 14 Apr 2022 09:49:44 +0530 Subject: [PATCH 28/28] fix: Allow adding JSON fields via Customize Form --- frappe/custom/doctype/custom_field/custom_field.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/custom/doctype/custom_field/custom_field.json b/frappe/custom/doctype/custom_field/custom_field.json index 5632da2149..add6cbb828 100644 --- a/frappe/custom/doctype/custom_field/custom_field.json +++ b/frappe/custom/doctype/custom_field/custom_field.json @@ -123,7 +123,7 @@ "label": "Field Type", "oldfieldname": "fieldtype", "oldfieldtype": "Select", - "options": "Autocomplete\nAttach\nAttach Image\nBarcode\nButton\nCheck\nCode\nColor\nColumn Break\nCurrency\nData\nDate\nDatetime\nDuration\nDynamic Link\nFloat\nFold\nGeolocation\nHeading\nHTML\nHTML Editor\nIcon\nImage\nInt\nLink\nLong Text\nMarkdown Editor\nPassword\nPercent\nRead Only\nRating\nSection Break\nSelect\nSmall Text\nTable\nTable MultiSelect\nText\nText Editor\nTime\nSignature\nTab Break", + "options": "Autocomplete\nAttach\nAttach Image\nBarcode\nButton\nCheck\nCode\nColor\nColumn Break\nCurrency\nData\nDate\nDatetime\nDuration\nDynamic Link\nFloat\nFold\nGeolocation\nHeading\nHTML\nHTML Editor\nIcon\nImage\nInt\nJSON\nLink\nLong Text\nMarkdown Editor\nPassword\nPercent\nRead Only\nRating\nSection Break\nSelect\nSignature\nSmall Text\nTab Break\nTable\nTable MultiSelect\nText\nText Editor\nTime", "reqd": 1 }, { @@ -439,7 +439,7 @@ "idx": 1, "index_web_pages_for_search": 1, "links": [], - "modified": "2022-02-28 22:22:54.893269", + "modified": "2022-04-14 09:46:58.849765", "modified_by": "Administrator", "module": "Custom", "name": "Custom Field",