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() + diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index eb4424db30..bfc5ba845a 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.get('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: 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", diff --git a/frappe/core/doctype/file/file.py b/frappe/core/doctype/file/file.py index 3c89519faf..fa80d0ed6e 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: @@ -232,7 +204,6 @@ 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() self.call_delete_file() if not self.is_folder: self.add_comment_in_reference_doc('Attachment Removed', _("Removed {0}").format(self.file_name)) @@ -274,9 +245,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")) @@ -598,7 +566,6 @@ class File(NestedSet): 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({ @@ -619,12 +586,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, _ 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 e3c48e7469..be68f1fee2 100644 --- a/frappe/core/doctype/file/test_file.py +++ b/frappe/core/doctype/file/test_file.py @@ -183,7 +183,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]) @@ -212,11 +212,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): @@ -227,8 +224,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): @@ -251,8 +246,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): 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 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) 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; 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 diff --git a/frappe/tests/test_db_query.py b/frappe/tests/test_db_query.py index b1b12f642e..18c19cbf3e 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,74 @@ 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') + frappe.set_user('Administrator') + 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 +347,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)