vassili/seitime-frappe
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 1

Merge pull request #27352 from cogk/refactor-move-validations-to-before-insert

Browse source 
refactor(File): Move validations to before_insert
This commit is contained in:
Akhil Narang 2024-08-21 16:42:30 +05:30 • committed by GitHub
commit 1d0f15033a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 47 additions and 4 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

13
frappe/core/doctype/file/file.py
View file

@ -91,6 +91,7 @@ class File(Document):
def before_insert(self):
self.set_folder_name()
self.set_is_private()
self.set_file_name()
self.validate_attachment_limit()
self.set_file_type()
@ -106,12 +107,11 @@ class File(Document):
self.flags.new_file = True
frappe.db.after_rollback.add(self.on_rollback)
self.validate_duplicate_entry() # Hash is generated in save_file
def after_insert(self):
if not self.is_folder:
self.create_attachment_record()
self.set_is_private()
self.set_file_name()
self.validate_duplicate_entry()
def validate(self):
if self.is_folder:
@ -470,7 +470,12 @@ class File(Document):
"""If file not attached to any other record, delete it"""
on_disk_file_not_shared = self.content_hash and not frappe.get_all(
"File",
filters={"content_hash": self.content_hash, "name": ["!=", self.name]},
filters={
"content_hash": self.content_hash,
"name": ["!=", self.name],
# NOTE: Some old Files might share file_urls while not sharing the is_private value
# "is_private": self.is_private,
},
limit=1,
)
if on_disk_file_not_shared:

38
frappe/core/doctype/file/test_file.py
View file

@ -920,3 +920,41 @@ class TestGuestFileAndAttachments(FrappeTestCase):
).insert(ignore_permissions=True)
self.assertFalse(file.is_downloadable())
def test_private_remains_private_even_if_same_hash(self):
file_name = "test" + frappe.generate_hash()
content = file_name.encode()
doc_pub: "File" = frappe.new_doc("File") # type: ignore
doc_pub.file_url = f"/files/{file_name}.txt"
doc_pub.content = content
doc_pub.save()
doc_pri: "File" = frappe.new_doc("File") # type: ignore
doc_pri.file_url = f"/private/files/{file_name}.txt"
doc_pri.is_private = False
doc_pri.content = content
doc_pri.save()
doc_pub.reload()
doc_pri.reload()
self.assertEqual(doc_pub.is_private, 0)
self.assertEqual(doc_pri.is_private, 1)
self.assertEqual(doc_pub.file_url, f"/files/{file_name}.txt")
self.assertEqual(doc_pri.file_url, f"/private/files/{file_name}.txt")
self.assertEqual(doc_pub.get_content(), content)
self.assertEqual(doc_pri.get_content(), content)
# Deleting a public File should not delete the private File's disk file
doc_pub.delete()
self.assertTrue(os.path.exists(doc_pri.get_full_path()))
# TODO: Migrate existing Files that have a mismatch between `is_private` and `file_url` prefix?
# self.assertFalse(os.path.exists(doc_pub.get_full_path()))
self.assertEqual(doc_pri.get_content(), content)
doc_pri.delete()
self.assertFalse(os.path.exists(doc_pri.get_full_path()))