From c61ff3bced267ccd37c9f0d4b16c1dd32cd822dd Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Fri, 30 Aug 2019 19:25:36 +0530 Subject: [PATCH 01/18] fix: Remove NestedSet from File Pros: - Multiple file uploads wont result in a deadlock Cons: - Folder size cannot be computed --- frappe/core/doctype/file/file.py | 37 ++----------------- .../public/js/frappe/views/file/file_view.js | 2 +- 2 files changed, 4 insertions(+), 35 deletions(-) diff --git a/frappe/core/doctype/file/file.py b/frappe/core/doctype/file/file.py index 847e0cd267..71c4fcd965 100755 --- a/frappe/core/doctype/file/file.py +++ b/frappe/core/doctype/file/file.py @@ -26,6 +26,7 @@ from frappe.utils import get_hook_method, get_files_path, random_string, encode, from frappe import _ from frappe import conf from frappe.utils.nestedset import NestedSet +from frappe.model.document import Document from frappe.utils import strip from PIL import Image, ImageOps from six import StringIO, string_types @@ -42,8 +43,7 @@ class FolderNotEmpty(frappe.ValidationError): pass exclude_from_linked_with = True -class File(NestedSet): - nsm_parent_field = 'folder' +class File(Document): no_feed_on_delete = True def before_insert(self): @@ -72,8 +72,6 @@ class File(NestedSet): self.name = frappe.generate_hash("", 10) def after_insert(self): - self.update_parent_folder_size() - if not self.is_folder: self.add_comment_in_reference_doc('Attachment', _('Added {0}').format("{file_name}{icon}".format(**{ @@ -100,7 +98,6 @@ class File(NestedSet): self.validate_file() self.generate_content_hash() - self.set_folder_size() self.validate_url() if frappe.db.exists('File', {'name': self.name, 'is_folder': 0}): @@ -136,31 +133,6 @@ class File(NestedSet): frappe.db.set_value(self.attached_to_doctype, self.attached_to_name, self.attached_to_field, self.file_url) - - def set_folder_size(self): - """Set folder size if folder""" - if self.is_folder and not self.is_new(): - self.file_size = cint(self.get_folder_size()) - self.db_set('file_size', self.file_size) - - for folder in self.get_ancestors(): - frappe.db.set_value("File", folder, "file_size", self.get_folder_size(folder)) - - def get_folder_size(self, folder=None): - """Returns folder size for current folder""" - if not folder: - folder = self.name - - file_size = frappe.db.sql("""select ifnull(sum(file_size), 0) - from tabFile where folder=%s """, (folder))[0][0] - - return file_size - - def update_parent_folder_size(self): - """Update size of parent folder""" - if self.folder and not self.is_folder: # it not home - frappe.get_doc("File", self.folder).set_folder_size() - def set_folder_name(self): """Make parent folders if not exists based on reference doctype and name""" if self.attached_to_doctype and not self.folder: @@ -225,7 +197,7 @@ class File(NestedSet): if self.is_home_folder or self.is_attachments_folder: frappe.throw(_("Cannot delete Home and Attachments folders")) self.check_folder_is_empty() - super(File, self).on_trash() + # super(File, self).on_trash() self.call_delete_file() if not self.is_folder: self.add_comment_in_reference_doc('Attachment Removed', _("Removed {0}").format(self.file_name)) @@ -267,9 +239,6 @@ class File(NestedSet): return thumbnail_url - def after_delete(self): - self.update_parent_folder_size() - def check_folder_is_empty(self): """Throw exception if folder is not empty""" files = frappe.get_all("File", filters={"folder": self.name}, fields=("name", "file_name")) diff --git a/frappe/public/js/frappe/views/file/file_view.js b/frappe/public/js/frappe/views/file/file_view.js index 6ee37aec3d..5f9ca642f4 100644 --- a/frappe/public/js/frappe/views/file/file_view.js +++ b/frappe/public/js/frappe/views/file/file_view.js @@ -259,7 +259,7 @@ frappe.views.FileView = class FileView extends frappe.views.ListView { get_left_html(file) { file = this.prepare_datum(file); - const file_size = frappe.form.formatters.FileSize(file.file_size); + const file_size = file.file_size ? frappe.form.formatters.FileSize(file.file_size) : ''; const route_url = this.get_route_url(file); let created_on; From 92f8b6ac1b1e94081b0ae889c1bf25e59912c696 Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Sun, 1 Sep 2019 12:49:13 +0530 Subject: [PATCH 02/18] fix: Remove lft and rgt --- frappe/core/doctype/file/file.json | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/frappe/core/doctype/file/file.json b/frappe/core/doctype/file/file.json index 9bf7d03512..d9ab504db7 100644 --- a/frappe/core/doctype/file/file.json +++ b/frappe/core/doctype/file/file.json @@ -22,8 +22,6 @@ "column_break_10", "attached_to_name", "attached_to_field", - "lft", - "rgt", "old_parent", "content_hash", "uploaded_to_dropbox", @@ -145,18 +143,6 @@ "label": "Attached To Field", "read_only": 1 }, - { - "fieldname": "lft", - "fieldtype": "Int", - "hidden": 1, - "label": "lft" - }, - { - "fieldname": "rgt", - "fieldtype": "Int", - "hidden": 1, - "label": "rgt" - }, { "fieldname": "old_parent", "fieldtype": "Data", @@ -186,7 +172,7 @@ ], "icon": "fa fa-file", "idx": 1, - "modified": "2019-08-16 16:41:03.086023", + "modified": "2019-08-30 19:46:20.796453", "modified_by": "Administrator", "module": "Core", "name": "File", From db305d75687d0c6a2f270d67bcb4b8ac8940114a Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Sun, 1 Sep 2019 12:49:17 +0530 Subject: [PATCH 03/18] fix: File tests --- frappe/core/doctype/file/file.py | 15 +++++++++------ frappe/core/doctype/file/test_file.py | 13 +++---------- 2 files changed, 12 insertions(+), 16 deletions(-) diff --git a/frappe/core/doctype/file/file.py b/frappe/core/doctype/file/file.py index 71c4fcd965..e324a0d879 100755 --- a/frappe/core/doctype/file/file.py +++ b/frappe/core/doctype/file/file.py @@ -555,7 +555,6 @@ class File(Document): def on_doctype_update(): frappe.db.add_index("File", ["attached_to_doctype", "attached_to_name"]) - frappe.db.add_index("File", ["lft", "rgt"]) def make_home_folder(): home = frappe.get_doc({ @@ -576,12 +575,16 @@ def make_home_folder(): @frappe.whitelist() def get_breadcrumbs(folder): """returns name, file_name of parent folder""" - values = frappe.db.get_value("File", folder, ["lft", "rgt"], as_dict=True) - if not values: - frappe.throw(_("Folder {0} does not exist").format(folder)) + path = folder.split('/') - return frappe.db.sql("""select name, file_name from tabFile - where lft < %s and rgt > %s order by lft asc""", (values.lft, values.rgt), as_dict=1) + folders = [] + for i, p in enumerate(path): + indexes = range(0, i) + folder = '/'.join([path[i] for i in indexes]) + if folder: + folders.append(folder) + + return [frappe._dict(file_name=f) for f in folders] @frappe.whitelist() def create_new_folder(file_name, folder): diff --git a/frappe/core/doctype/file/test_file.py b/frappe/core/doctype/file/test_file.py index e9bf999bec..de97fbc162 100644 --- a/frappe/core/doctype/file/test_file.py +++ b/frappe/core/doctype/file/test_file.py @@ -171,7 +171,7 @@ class TestFile(unittest.TestCase): def delete_test_data(self): for f in frappe.db.sql('''select name, file_name from tabFile where - is_home_folder = 0 and is_attachments_folder = 0 order by rgt-lft asc'''): + is_home_folder = 0 and is_attachments_folder = 0 order by creation desc'''): frappe.delete_doc("File", f[0]) @@ -200,11 +200,8 @@ class TestFile(unittest.TestCase): def tests_after_upload(self): self.assertEqual(self.saved_folder, _("Home/Test Folder 1")) - - folder_size = frappe.db.get_value("File", _("Home/Test Folder 1"), "file_size") - saved_file_size = frappe.db.get_value("File", self.saved_name, "file_size") - - self.assertEqual(folder_size, saved_file_size) + file_folder = frappe.db.get_value("File", self.saved_name, "folder") + self.assertEqual(file_folder, _("Home/Test Folder 1")) def test_file_copy(self): @@ -215,8 +212,6 @@ class TestFile(unittest.TestCase): file = frappe.get_doc("File", {"file_name": "file_copy.txt"}) self.assertEqual(_("Home/Test Folder 2"), file.folder) - self.assertEqual(frappe.db.get_value("File", _("Home/Test Folder 2"), "file_size"), file.file_size) - self.assertEqual(frappe.db.get_value("File", _("Home/Test Folder 1"), "file_size"), 0) def test_folder_copy(self): @@ -239,8 +234,6 @@ class TestFile(unittest.TestCase): frappe.get_doc("File", file_copy_txt).delete() self.assertEqual(_("Home/Test Folder 1/Test Folder 3"), file.folder) - self.assertEqual(frappe.db.get_value("File", _("Home/Test Folder 1"), "file_size"), file.file_size) - self.assertEqual(frappe.db.get_value("File", _("Home/Test Folder 2"), "file_size"), 0) def test_default_folder(self): From 8676de02a17533d13ab645fecf814055f8c58c51 Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Sun, 1 Sep 2019 12:50:30 +0530 Subject: [PATCH 04/18] fix: Remove commented call --- frappe/core/doctype/file/file.py | 1 - 1 file changed, 1 deletion(-) diff --git a/frappe/core/doctype/file/file.py b/frappe/core/doctype/file/file.py index e324a0d879..b9973d848b 100755 --- a/frappe/core/doctype/file/file.py +++ b/frappe/core/doctype/file/file.py @@ -197,7 +197,6 @@ class File(Document): if self.is_home_folder or self.is_attachments_folder: frappe.throw(_("Cannot delete Home and Attachments folders")) self.check_folder_is_empty() - # super(File, self).on_trash() self.call_delete_file() if not self.is_folder: self.add_comment_in_reference_doc('Attachment Removed', _("Removed {0}").format(self.file_name)) From 4abb057a53ad260ca8130a5a26bf811f57da48e8 Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Tue, 3 Sep 2019 13:43:29 +0530 Subject: [PATCH 05/18] fix: Nested Set test cases --- frappe/tests/test_db_query.py | 201 ++++++++++++++++++---------------- 1 file changed, 108 insertions(+), 93 deletions(-) diff --git a/frappe/tests/test_db_query.py b/frappe/tests/test_db_query.py index b1b12f642e..28849618ee 100644 --- a/frappe/tests/test_db_query.py +++ b/frappe/tests/test_db_query.py @@ -185,37 +185,29 @@ class TestReportview(unittest.TestCase): self.assertTrue('date_diff' in data[0]) def test_nested_permission(self): - clear_user_permissions_for_doctype("File") - delete_test_file_hierarchy() # delete already existing folders - from frappe.core.doctype.file.file import create_new_folder frappe.set_user('Administrator') - - create_new_folder('level1-A', 'Home') - create_new_folder('level2-A', 'Home/level1-A') - create_new_folder('level2-B', 'Home/level1-A') - create_new_folder('level3-A', 'Home/level1-A/level2-A') - - create_new_folder('level1-B', 'Home') - create_new_folder('level2-A', 'Home/level1-B') + create_nested_doctype() + create_nested_doctype_records() + clear_user_permissions_for_doctype('Nested DocType') # user permission for only one root folder - add_user_permission('File', 'Home/level1-A', 'test2@example.com') + add_user_permission('Nested DocType', 'Level 1 A', 'test2@example.com') from frappe.core.page.permission_manager.permission_manager import update - update('File', 'All', 0, 'if_owner', 0) # to avoid if_owner filter + # to avoid if_owner filter + update('Nested DocType', 'All', 0, 'if_owner', 0) frappe.set_user('test2@example.com') - data = DatabaseQuery("File").execute() + data = DatabaseQuery('Nested DocType').execute() # children of root folder (for which we added user permission) should be accessible - self.assertTrue({"name": "Home/level1-A/level2-A"} in data) - self.assertTrue({"name": "Home/level1-A/level2-B"} in data) - self.assertTrue({"name": "Home/level1-A/level2-A/level3-A"} in data) + self.assertTrue({'name': 'Level 2 A'} in data) + self.assertTrue({'name': 'Level 2 A'} in data) # other folders should not be accessible - self.assertFalse({"name": "Home/level1-B"} in data) - self.assertFalse({"name": "Home/level1-B/level2-B"} in data) - update('File', 'All', 0, 'if_owner', 1) + self.assertFalse({'name': 'Level 1 B'} in data) + self.assertFalse({'name': 'Level 2 B'} in data) + update('Nested DocType', 'All', 0, 'if_owner', 1) frappe.set_user('Administrator') def test_filter_sanitizer(self): @@ -259,83 +251,75 @@ class TestReportview(unittest.TestCase): self.assertTrue('DefaultValue' in [d['name'] for d in out]) def test_of_not_of_descendant_ancestors(self): - clear_user_permissions_for_doctype("File") - delete_test_file_hierarchy() # delete already existing folders - from frappe.core.doctype.file.file import create_new_folder - - create_new_folder('level1-A', 'Home') - create_new_folder('level2-A', 'Home/level1-A') - create_new_folder('level2-B', 'Home/level1-A') - create_new_folder('level3-A', 'Home/level1-A/level2-A') - - create_new_folder('level1-B', 'Home') - create_new_folder('level2-A', 'Home/level1-B') + create_nested_doctype() + create_nested_doctype_records() + clear_user_permissions_for_doctype('Nested DocType') # in descendants filter - data = frappe.get_all('File', {'name': ('descendants of', 'Home/level1-A/level2-A')}) - self.assertTrue({"name": "Home/level1-A/level2-A/level3-A"} in data) + data = frappe.get_all('Nested DocType', {'name': ('descendants of', 'Level 2 A')}) + self.assertTrue({"name": "Level 3 A"} in data) - data = frappe.get_all('File', {'name': ('descendants of', 'Home/level1-A')}) - self.assertTrue({"name": "Home/level1-A/level2-A/level3-A"} in data) - self.assertTrue({"name": "Home/level1-A/level2-A"} in data) - self.assertTrue({"name": "Home/level1-A/level2-B"} in data) - self.assertFalse({"name": "Home/level1-B"} in data) - self.assertFalse({"name": "Home/level1-A"} in data) - self.assertFalse({"name": "Home"} in data) + data = frappe.get_all('Nested DocType', {'name': ('descendants of', 'Level 1 A')}) + self.assertTrue({"name": "Level 3 A"} in data) + self.assertTrue({"name": "Level 2 A"} in data) + self.assertFalse({"name": "Level 2 B"} in data) + self.assertFalse({"name": "Level 1 B"} in data) + self.assertFalse({"name": "Level 1 A"} in data) + self.assertFalse({"name": "Root"} in data) # in ancestors of filter - data = frappe.get_all('File', {'name': ('ancestors of', 'Home/level1-A/level2-A')}) - self.assertFalse({"name": "Home/level1-A/level2-A/level3-A"} in data) - self.assertFalse({"name": "Home/level1-A/level2-A"} in data) - self.assertFalse({"name": "Home/level1-A/level2-B"} in data) - self.assertFalse({"name": "Home/level1-B"} in data) - self.assertTrue({"name": "Home/level1-A"} in data) - self.assertTrue({"name": "Home"} in data) + data = frappe.get_all('Nested DocType', {'name': ('ancestors of', 'Level 2 A')}) + self.assertFalse({"name": "Level 3 A"} in data) + self.assertFalse({"name": "Level 2 A"} in data) + self.assertFalse({"name": "Level 2 B"} in data) + self.assertFalse({"name": "Level 1 B"} in data) + self.assertTrue({"name": "Level 1 A"} in data) + self.assertTrue({"name": "Root"} in data) - data = frappe.get_all('File', {'name': ('ancestors of', 'Home/level1-A')}) - self.assertFalse({"name": "Home/level1-A/level2-A/level3-A"} in data) - self.assertFalse({"name": "Home/level1-A/level2-A"} in data) - self.assertFalse({"name": "Home/level1-A/level2-B"} in data) - self.assertFalse({"name": "Home/level1-B"} in data) - self.assertFalse({"name": "Home/level1-A"} in data) - self.assertTrue({"name": "Home"} in data) + data = frappe.get_all('Nested DocType', {'name': ('ancestors of', 'Level 1 A')}) + self.assertFalse({"name": "Level 3 A"} in data) + self.assertFalse({"name": "Level 2 A"} in data) + self.assertFalse({"name": "Level 2 B"} in data) + self.assertFalse({"name": "Level 1 B"} in data) + self.assertFalse({"name": "Level 1 A"} in data) + self.assertTrue({"name": "Root"} in data) # not descendants filter - data = frappe.get_all('File', {'name': ('not descendants of', 'Home/level1-A/level2-A')}) - self.assertFalse({"name": "Home/level1-A/level2-A/level3-A"} in data) - self.assertTrue({"name": "Home/level1-A/level2-A"} in data) - self.assertTrue({"name": "Home/level1-A/level2-B"} in data) - self.assertTrue({"name": "Home/level1-A"} in data) - self.assertTrue({"name": "Home"} in data) + data = frappe.get_all('Nested DocType', {'name': ('not descendants of', 'Level 2 A')}) + self.assertFalse({"name": "Level 3 A"} in data) + self.assertTrue({"name": "Level 2 A"} in data) + self.assertTrue({"name": "Level 2 B"} in data) + self.assertTrue({"name": "Level 1 A"} in data) + self.assertTrue({"name": "Root"} in data) - data = frappe.get_all('File', {'name': ('not descendants of', 'Home/level1-A')}) - self.assertFalse({"name": "Home/level1-A/level2-A/level3-A"} in data) - self.assertFalse({"name": "Home/level1-A/level2-A"} in data) - self.assertFalse({"name": "Home/level1-A/level2-B"} in data) - self.assertTrue({"name": "Home/level1-B"} in data) - self.assertTrue({"name": "Home/level1-A"} in data) - self.assertTrue({"name": "Home"} in data) + data = frappe.get_all('Nested DocType', {'name': ('not descendants of', 'Level 1 A')}) + self.assertFalse({"name": "Level 3 A"} in data) + self.assertFalse({"name": "Level 2 A"} in data) + self.assertTrue({"name": "Level 2 B"} in data) + self.assertTrue({"name": "Level 1 B"} in data) + self.assertTrue({"name": "Level 1 A"} in data) + self.assertTrue({"name": "Root"} in data) # not ancestors of filter - data = frappe.get_all('File', {'name': ('not ancestors of', 'Home/level1-A/level2-A')}) - self.assertTrue({"name": "Home/level1-A/level2-A/level3-A"} in data) - self.assertTrue({"name": "Home/level1-A/level2-A"} in data) - self.assertTrue({"name": "Home/level1-A/level2-B"} in data) - self.assertTrue({"name": "Home/level1-B"} in data) - self.assertTrue({"name": "Home/level1-A"} not in data) - self.assertTrue({"name": "Home"} not in data) + data = frappe.get_all('Nested DocType', {'name': ('not ancestors of', 'Level 2 A')}) + self.assertTrue({"name": "Level 3 A"} in data) + self.assertTrue({"name": "Level 2 A"} in data) + self.assertTrue({"name": "Level 2 B"} in data) + self.assertTrue({"name": "Level 1 B"} in data) + self.assertTrue({"name": "Level 1 A"} not in data) + self.assertTrue({"name": "Root"} not in data) - data = frappe.get_all('File', {'name': ('not ancestors of', 'Home/level1-A')}) - self.assertTrue({"name": "Home/level1-A/level2-A/level3-A"} in data) - self.assertTrue({"name": "Home/level1-A/level2-A"} in data) - self.assertTrue({"name": "Home/level1-A/level2-B"} in data) - self.assertTrue({"name": "Home/level1-B"} in data) - self.assertTrue({"name": "Home/level1-A"} in data) - self.assertFalse({"name": "Home"} in data) + data = frappe.get_all('Nested DocType', {'name': ('not ancestors of', 'Level 1 A')}) + self.assertTrue({"name": "Level 3 A"} in data) + self.assertTrue({"name": "Level 2 A"} in data) + self.assertTrue({"name": "Level 2 B"} in data) + self.assertTrue({"name": "Level 1 B"} in data) + self.assertTrue({"name": "Level 1 A"} in data) + self.assertFalse({"name": "Root"} in data) - data = frappe.get_all('File', {'name': ('ancestors of', 'Home')}) + data = frappe.get_all('Nested DocType', {'name': ('ancestors of', 'Root')}) self.assertTrue(len(data) == 0) - self.assertTrue(len(frappe.get_all('File', {'name': ('not ancestors of', 'Home')})) == len(frappe.get_all('File'))) + self.assertTrue(len(frappe.get_all('Nested DocType', {'name': ('not ancestors of', 'Root')})) == len(frappe.get_all('Nested DocType'))) def test_is_set_is_not_set(self): @@ -364,14 +348,45 @@ def create_event(subject="_Test Event", starts_on=None): return event -def delete_test_file_hierarchy(): - files_to_delete = [ - 'Home/level1-A/level2-A/level3-A', - 'Home/level1-A/level2-A', - 'Home/level1-A/level2-B', - 'Home/level1-A', - 'Home/level1-B/level2-A', - 'Home/level1-B' +def create_nested_doctype(): + if frappe.db.exists('DocType', 'Nested DocType'): + return + + frappe.get_doc({ + 'doctype': 'DocType', + 'name': 'Nested DocType', + 'module': 'Custom', + 'is_tree': 1, + 'custom': 1, + 'autoname': 'Prompt', + 'fields': [ + {'label': 'Description', 'fieldname': 'description'} + ], + 'permissions': [ + {'role': 'Blogger'} + ] + }).insert() + +def create_nested_doctype_records(): + ''' + Create a structure like: + - Root + - Level 1 A + - Level 2 A + - Level 3 A + - Level 1 B + - Level 2 B + ''' + records = [ + {'name': 'Root', 'is_group': 1}, + {'name': 'Level 1 A', 'parent_nested_doctype': 'Root', 'is_group': 1}, + {'name': 'Level 2 A', 'parent_nested_doctype': 'Level 1 A', 'is_group': 1}, + {'name': 'Level 3 A', 'parent_nested_doctype': 'Level 2 A'}, + {'name': 'Level 1 B', 'parent_nested_doctype': 'Root', 'is_group': 1}, + {'name': 'Level 2 B', 'parent_nested_doctype': 'Level 1 B'}, ] - for file_name in files_to_delete: - frappe.delete_doc('File', file_name) + + for r in records: + d = frappe.new_doc('Nested DocType') + d.update(r) + d.insert(ignore_permissions=True, ignore_if_duplicate=True) From ef665c4c87efbd0852c8d0af263538590aeaa362 Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Tue, 10 Sep 2019 11:45:45 +0530 Subject: [PATCH 06/18] fix(pg test): get_list to get_all --- frappe/permissions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/permissions.py b/frappe/permissions.py index a27ba5ee75..a0d1677fac 100644 --- a/frappe/permissions.py +++ b/frappe/permissions.py @@ -423,7 +423,7 @@ def clear_user_permissions_for_doctype(doctype, user=None): filters = {'allow': doctype} if user: filters['user'] = user - user_permissions_for_doctype = frappe.db.get_list('User Permission', filters=filters) + user_permissions_for_doctype = frappe.db.get_all('User Permission', filters=filters) for d in user_permissions_for_doctype: frappe.delete_doc('User Permission', d.name) From 244bc16c27dd585dca9268257129612e3a34ae09 Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Tue, 10 Sep 2019 13:32:06 +0530 Subject: [PATCH 07/18] fix: Fallback to localhost for db_host --- frappe/database/postgres/setup_db.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/database/postgres/setup_db.py b/frappe/database/postgres/setup_db.py index 79c7c3a304..01a97178f9 100644 --- a/frappe/database/postgres/setup_db.py +++ b/frappe/database/postgres/setup_db.py @@ -17,7 +17,7 @@ def setup_database(force, source_sql, verbose): subprocess_env['PGPASSWORD'] = str(frappe.conf.db_password) # bootstrap db subprocess.check_output([ - 'psql', frappe.conf.db_name, '-h', frappe.conf.db_host, '-U', + 'psql', frappe.conf.db_name, '-h', frappe.conf.db_host or 'localhost', '-U', frappe.conf.db_name, '-f', os.path.join(os.path.dirname(__file__), 'framework_postgres.sql') ], env=subprocess_env) From 4811b64b1fefc980ca45a643821a4be8e24954cf Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Tue, 10 Sep 2019 13:32:23 +0530 Subject: [PATCH 08/18] fix(test): db.commit after creating table --- frappe/tests/test_db_query.py | 1 + 1 file changed, 1 insertion(+) diff --git a/frappe/tests/test_db_query.py b/frappe/tests/test_db_query.py index 28849618ee..6ddd152920 100644 --- a/frappe/tests/test_db_query.py +++ b/frappe/tests/test_db_query.py @@ -366,6 +366,7 @@ def create_nested_doctype(): {'role': 'Blogger'} ] }).insert() + frappe.db.commit() def create_nested_doctype_records(): ''' From 1eecc3dadb8ee1b44d9dc006292ff71e3ea1f4da Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Tue, 10 Sep 2019 13:34:48 +0530 Subject: [PATCH 09/18] Revert "fix(pg test): get_list to get_all" This reverts commit ef665c4c87efbd0852c8d0af263538590aeaa362. --- frappe/permissions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/permissions.py b/frappe/permissions.py index a0d1677fac..a27ba5ee75 100644 --- a/frappe/permissions.py +++ b/frappe/permissions.py @@ -423,7 +423,7 @@ def clear_user_permissions_for_doctype(doctype, user=None): filters = {'allow': doctype} if user: filters['user'] = user - user_permissions_for_doctype = frappe.db.get_all('User Permission', filters=filters) + user_permissions_for_doctype = frappe.db.get_list('User Permission', filters=filters) for d in user_permissions_for_doctype: frappe.delete_doc('User Permission', d.name) From 30a527b91123cc57bf5d45c731cdb91f59c4dfbb Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Tue, 10 Sep 2019 17:14:27 +0530 Subject: [PATCH 10/18] fix: Remove db.commit --- frappe/tests/test_db_query.py | 1 - 1 file changed, 1 deletion(-) diff --git a/frappe/tests/test_db_query.py b/frappe/tests/test_db_query.py index 6ddd152920..28849618ee 100644 --- a/frappe/tests/test_db_query.py +++ b/frappe/tests/test_db_query.py @@ -366,7 +366,6 @@ def create_nested_doctype(): {'role': 'Blogger'} ] }).insert() - frappe.db.commit() def create_nested_doctype_records(): ''' From 5bf4e1616f9956986b3d59781f4a8a7e2fa7b26c Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Tue, 10 Sep 2019 18:30:57 +0530 Subject: [PATCH 11/18] fix: Remove duplicate calls --- frappe/tests/test_db_query.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/frappe/tests/test_db_query.py b/frappe/tests/test_db_query.py index 28849618ee..d933140cbe 100644 --- a/frappe/tests/test_db_query.py +++ b/frappe/tests/test_db_query.py @@ -251,8 +251,6 @@ class TestReportview(unittest.TestCase): self.assertTrue('DefaultValue' in [d['name'] for d in out]) def test_of_not_of_descendant_ancestors(self): - create_nested_doctype() - create_nested_doctype_records() clear_user_permissions_for_doctype('Nested DocType') # in descendants filter From e74eb649b7cc8a4e09a15905e6f51fbfec4e29bc Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Wed, 11 Sep 2019 13:16:46 +0530 Subject: [PATCH 12/18] fix(test): Delete blog after test --- frappe/core/doctype/comment/test_comment.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/frappe/core/doctype/comment/test_comment.py b/frappe/core/doctype/comment/test_comment.py index 2adc5eb899..3cf8fbaa3f 100644 --- a/frappe/core/doctype/comment/test_comment.py +++ b/frappe/core/doctype/comment/test_comment.py @@ -53,5 +53,7 @@ class TestComment(unittest.TestCase): reference_name = test_blog.name ))), 0) + test_blog.delete() + From 01da83909daaba6d20f1d52e15bdc162ba912c05 Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Wed, 11 Sep 2019 13:17:00 +0530 Subject: [PATCH 13/18] fix(test): Check for mandatory manually --- frappe/core/doctype/doctype/doctype.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index e808cd3053..ef346d1992 100644 --- a/frappe/core/doctype/doctype/doctype.py +++ b/frappe/core/doctype/doctype/doctype.py @@ -586,9 +586,11 @@ class DocType(Document): if not self.is_tree: return self.add_nestedset_fields() - # set field as mandatory - field = self.meta.get_field('nsm_parent_field') - field.reqd = 1 + + if not self.nsm_parent_field: + field_label = frappe.bold(_("Parent Field (Tree)")) + frappe.throw(_("{0} is a mandatory field").format(field_label), frappe.MandatoryError) + # check if field is valid fieldnames = [df.fieldname for df in self.fields] if self.nsm_parent_field and self.nsm_parent_field not in fieldnames: From 7b9ee521eee29c9f36285e5df32bd8a70f91cdba Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Wed, 11 Sep 2019 15:58:36 +0530 Subject: [PATCH 14/18] fix: Use get_all instead of get_list --- frappe/permissions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/permissions.py b/frappe/permissions.py index a27ba5ee75..a0d1677fac 100644 --- a/frappe/permissions.py +++ b/frappe/permissions.py @@ -423,7 +423,7 @@ def clear_user_permissions_for_doctype(doctype, user=None): filters = {'allow': doctype} if user: filters['user'] = user - user_permissions_for_doctype = frappe.db.get_list('User Permission', filters=filters) + user_permissions_for_doctype = frappe.db.get_all('User Permission', filters=filters) for d in user_permissions_for_doctype: frappe.delete_doc('User Permission', d.name) From 2508ae13777434ef30f858b4451a1a4d0f91eec1 Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Wed, 18 Sep 2019 11:04:34 +0530 Subject: [PATCH 15/18] test: Set user Administrator --- frappe/tests/test_db_query.py | 1 + 1 file changed, 1 insertion(+) diff --git a/frappe/tests/test_db_query.py b/frappe/tests/test_db_query.py index d933140cbe..18c19cbf3e 100644 --- a/frappe/tests/test_db_query.py +++ b/frappe/tests/test_db_query.py @@ -251,6 +251,7 @@ class TestReportview(unittest.TestCase): self.assertTrue('DefaultValue' in [d['name'] for d in out]) def test_of_not_of_descendant_ancestors(self): + frappe.set_user('Administrator') clear_user_permissions_for_doctype('Nested DocType') # in descendants filter From c8a80a2e4d64e14d028db7f19a8b4a028c7b2042 Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Thu, 19 Sep 2019 15:41:17 +0530 Subject: [PATCH 16/18] fix: frappe.db.field_exists - field_exists didn't work correctly - The call to field_exists itself was wrong --- frappe/database/database.py | 5 ++++- frappe/model/base_document.py | 4 ++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/frappe/database/database.py b/frappe/database/database.py index 6702ce2a5e..412051c76f 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -758,7 +758,10 @@ class Database(object): def field_exists(self, dt, fn): """Return true of field exists.""" - return self.sql("select name from tabDocField where fieldname=%s and parent=%s", (dt, fn)) + return self.exists('DocField', { + 'fieldname': fn, + 'parent': dt + }) def table_exists(self, doctype): """Returns True if table for given doctype exists.""" diff --git a/frappe/model/base_document.py b/frappe/model/base_document.py index 7480970711..6eb2efce6b 100644 --- a/frappe/model/base_document.py +++ b/frappe/model/base_document.py @@ -38,8 +38,8 @@ def get_controller(doctype): or ["Core", False] if custom: - if frappe.db.field_exists(doctype, "is_tree"): - is_tree = frappe.db.get_value("DocType", doctype, ("is_tree"), cache=True) + if frappe.db.field_exists("DocType", "is_tree"): + is_tree = frappe.db.get_value("DocType", doctype, "is_tree", cache=True) else: is_tree = False _class = NestedSet if is_tree else Document From ef30490865bfb50313fcff8a1634d42dd3850292 Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Thu, 19 Sep 2019 16:14:19 +0530 Subject: [PATCH 17/18] style: Unused variable --- frappe/core/doctype/file/file.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/core/doctype/file/file.py b/frappe/core/doctype/file/file.py index abb69c83ea..fa80d0ed6e 100755 --- a/frappe/core/doctype/file/file.py +++ b/frappe/core/doctype/file/file.py @@ -589,7 +589,7 @@ def get_breadcrumbs(folder): path = folder.split('/') folders = [] - for i, p in enumerate(path): + for i, _ in enumerate(path): indexes = range(0, i) folder = '/'.join([path[i] for i in indexes]) if folder: From 3e390470aee3e2a89dc7da10ce515edc12c60ed3 Mon Sep 17 00:00:00 2001 From: Shivam Mishra Date: Thu, 19 Sep 2019 17:10:04 +0530 Subject: [PATCH 18/18] fix: retain doc.name --- frappe/public/js/frappe/web_form/web_form.js | 1 - 1 file changed, 1 deletion(-) diff --git a/frappe/public/js/frappe/web_form/web_form.js b/frappe/public/js/frappe/web_form/web_form.js index 216bf2d831..ca44dd65d0 100644 --- a/frappe/public/js/frappe/web_form/web_form.js +++ b/frappe/public/js/frappe/web_form/web_form.js @@ -107,7 +107,6 @@ export default class WebForm extends frappe.ui.FieldGroup { let for_payment = Boolean(this.accept_payment && !this.doc.paid); this.doc.doctype = this.doc_type; - this.doc.name = this.doc_name; this.doc.web_form_name = this.name; // Save