Merge branch 'v12-pre-release' into version-12

Author: nabinhait

.flake8

@@ -0,0 +1,32 @@
+[flake8]
+ignore =
+    E121,
+    E126,
+    E127,
+    E128,
+    E203,
+    E225,
+    E226,
+    E231,
+    E241,
+    E251,
+    E261,
+    E265,
+    E302,
+    E303,
+    E305,
+    E402,
+    E501,
+    E741,
+    W291,
+    W292,
+    W293,
+    W391,
+    W503,
+    W504,
+    F403,
+    B007,
+    B950,
+    W191,
+
+max-line-length = 200

cypress/integration/form.js

@@ -39,4 +39,21 @@ context('Form', () => {
 			list_view.filter_area.filter_list.clear_filters();
 		});
 	});
+	it('validates behaviour of Data options validations in child table', () => {
+		// test email validations for set_invalid controller
+		let website_input = 'website.in';
+		let expectBackgroundColor = 'rgb(255, 220, 220)';
+
+		cy.visit('/desk#Form/Contact/New Contact 1');
+		cy.get('.frappe-control[data-fieldname="email_ids"]').as('table');
+		cy.get('@table').find('button.grid-add-row').click();
+		cy.get('.grid-body .rows [data-fieldname="email_id"]').click();
+		cy.get('@table').find('input.input-with-feedback.form-control').as('email_input');
+		cy.get('@email_input').type(website_input, { waitForAnimations: false });
+		cy.fill_field('company_name', 'Test Company');
+		cy.get('@email_input').should($div => {
+			const style = window.getComputedStyle($div[0]);
+			expect(style.backgroundColor).to.equal(expectBackgroundColor);
+		});
+	});
 });

frappe/__init__.py

@@ -23,7 +23,7 @@
 	reload(sys)
 	sys.setdefaultencoding("utf-8")
 
-__version__ = '12.16.3'
+__version__ = '12.17.0'
 __title__ = "Frappe Framework"
 
 local = Local()
@@ -794,18 +794,19 @@ def get_meta_module(doctype):
 	return frappe.modules.load_doctype_module(doctype)
 
 def delete_doc(doctype=None, name=None, force=0, ignore_doctypes=None, for_reload=False,
-	ignore_permissions=False, flags=None, ignore_on_trash=False, ignore_missing=True):
+	ignore_permissions=False, flags=None, ignore_on_trash=False, ignore_missing=True, delete_permanently=False):
 	"""Delete a document. Calls `frappe.model.delete_doc.delete_doc`.
 
 	:param doctype: DocType of document to be delete.
 	:param name: Name of document to be delete.
 	:param force: Allow even if document is linked. Warning: This may lead to data integrity errors.
 	:param ignore_doctypes: Ignore if child table is one of these.
 	:param for_reload: Call `before_reload` trigger before deleting.
-	:param ignore_permissions: Ignore user permissions."""
+	:param ignore_permissions: Ignore user permissions.
+	:param delete_permanently: Do not create a Deleted Document for the document."""
 	import frappe.model.delete_doc
 	frappe.model.delete_doc.delete_doc(doctype, name, force, ignore_doctypes, for_reload,
-		ignore_permissions, flags, ignore_on_trash, ignore_missing)
+		ignore_permissions, flags, ignore_on_trash, ignore_missing, delete_permanently)
 
 def delete_doc_if_exists(doctype, name, force=0):
 	"""Delete document if exists."""

frappe/change_log/v12/v12_17_0.md

@@ -0,0 +1,9 @@
+## Version 12.17.0 Release Notes
+
+### Fixes & Enhancements
+
+- Handle empty conditions in s3 backup ([#11258](https://github.com/frappe/frappe/pull/11258))
+- Auto delete Prepared Reports permanently ([#12683](https://github.com/frappe/frappe/pull/12683))
+- Always validate file URLs (backport) ([#12708](https://github.com/frappe/frappe/pull/12708))
+- Email validation improvements ([#12670](https://github.com/frappe/frappe/pull/12670))
+- Average Chart compute inaccurate average with test cases ([#12686](https://github.com/frappe/frappe/pull/12686))

frappe/core/doctype/file/file.py

@@ -93,52 +93,89 @@ def validate(self):
 			self.set_file_name()
 			self.validate_duplicate_entry()
 			self.validate_attachment_limit()
+
 		self.validate_folder()
 
-		if not self.file_url and not self.flags.ignore_file_validate:
-			if not self.is_folder:
+		if self.is_folder:
+			self.file_url = ""
+		else:
+			self.validate_url()
+
+		self.file_size = frappe.form_dict.file_size or self.file_size
+
+	def validate_url(self):
+		if not self.file_url or self.file_url.startswith(("http://", "https://")):
+			if not self.flags.ignore_file_validate:
 				self.validate_file()
-			self.generate_content_hash()
 
-		self.validate_url()
+			return
 
-		if frappe.db.exists('File', {'name': self.name, 'is_folder': 0}):
-			old_file_url = self.file_url
-			if not self.is_folder and (self.is_private != self.db_get('is_private')):
-				private_files = frappe.get_site_path('private', 'files')
-				public_files = frappe.get_site_path('public', 'files')
+		# Probably an invalid web URL
+		if not self.file_url.startswith(("/files/", "/private/files/")):
+			frappe.throw(
+				_("URL must start with http:// or https://"),
+				title=_('Invalid URL')
+			)
+
+		# Ensure correct formatting and type
+		self.file_url = unquote(self.file_url)
+		self.is_private = cint(self.is_private)
+
+		self.handle_is_private_changed()
+
+		base_path = os.path.realpath(get_files_path(is_private=self.is_private))
+		if not os.path.realpath(self.get_full_path()).startswith(base_path):
+			frappe.throw(
+				_("The File URL you've entered is incorrect"),
+				title=_('Invalid File URL')
+			)
+
+	def handle_is_private_changed(self):
+		if not frappe.db.exists(
+			'File', {
+				'name': self.name,
+				'is_private': cint(not self.is_private)
+			}
+		):
+			return
 
-				file_name = self.file_url.split('/')[-1]
-				if not self.is_private:
-					shutil.move(os.path.join(private_files, file_name),
-						os.path.join(public_files, file_name))
+		old_file_url = self.file_url
 
-					self.file_url = "/files/{0}".format(file_name)
+		file_name = self.file_url.split('/')[-1]
+		private_file_path = frappe.get_site_path('private', 'files', file_name)
+		public_file_path = frappe.get_site_path('public', 'files', file_name)
 
-				else:
-					shutil.move(os.path.join(public_files, file_name),
-						os.path.join(private_files, file_name))
+		if self.is_private:
+			shutil.move(public_file_path, private_file_path)
+			url_starts_with = "/private/files/"
+		else:
+			shutil.move(private_file_path, public_file_path)
+			url_starts_with = "/files/"
 
-					self.file_url = "/private/files/{0}".format(file_name)
+		self.file_url = "{0}{1}".format(url_starts_with, file_name)
+		update_existing_file_docs(self)
 
-				update_existing_file_docs(self)
+		if (
+			not self.attached_to_doctype
+			or not self.attached_to_name
+			or not self.fetch_attached_to_field(old_file_url)
+		):
+			return
 
-			# update documents image url with new file url
-			if self.attached_to_doctype and self.attached_to_name:
-				if not self.attached_to_field:
-					field_name = None
-					reference_dict = frappe.get_doc(self.attached_to_doctype, self.attached_to_name).as_dict()
-					for key, value in reference_dict.items():
-						if value == old_file_url:
-							field_name = key
-							break
-					self.attached_to_field = field_name
-				if self.attached_to_field:
-					frappe.db.set_value(self.attached_to_doctype, self.attached_to_name,
-						self.attached_to_field, self.file_url)
-
-		if self.file_url and (self.is_private != self.file_url.startswith('/private')):
-			frappe.throw(_('Invalid file URL. Please contact System Administrator.'))
+		frappe.db.set_value(self.attached_to_doctype, self.attached_to_name,
+			self.attached_to_field, self.file_url)
+
+	def fetch_attached_to_field(self, old_file_url):
+		if self.attached_to_field:
+			return True
+
+		reference_dict = frappe.get_doc(
+			self.attached_to_doctype, self.attached_to_name).as_dict()
+
+		for key, value in reference_dict.items():
+			if value == old_file_url:
+				self.attached_to_field = key
+				return True
 
 	def validate_attachment_limit(self):
 		attachment_limit = 0
@@ -331,8 +368,13 @@ def exists_on_disk(self):
 
 	def get_content(self):
 		"""Returns [`file_name`, `content`] for given file name `fname`"""
+		if self.is_folder:
+			frappe.throw(_("Cannot get file contents of a Folder"))
+
 		if self.get('content'):
 			return self.content
+
+		self.validate_url()
 		file_path = self.get_full_path()
 
 		# read the file
@@ -419,17 +461,6 @@ def save_uploaded(self):
 		else:
 			raise Exception
 
-
-	def validate_url(self, df=None):
-		if self.file_url:
-			if not self.file_url.startswith(("http://", "https://", "/files/", "/private/files/")):
-				frappe.throw(_("URL must start with 'http://' or 'https://'"))
-				return
-
-			self.file_url = unquote(self.file_url)
-			self.file_size = frappe.form_dict.file_size or self.file_size
-
-
 	def get_uploaded_content(self):
 		# should not be unicode when reading a file, hence using frappe.form
 		if 'filedata' in frappe.form_dict:
@@ -692,7 +723,7 @@ def delete_file(path):
 			os.remove(path)
 
 
-def remove_file(fid=None, attached_to_doctype=None, attached_to_name=None, from_delete=False):
+def remove_file(fid=None, attached_to_doctype=None, attached_to_name=None, from_delete=False, delete_permanently=False):
 	"""Remove file and File entry"""
 	file_name = None
 	if not (attached_to_doctype and attached_to_name):
@@ -710,7 +741,7 @@ def remove_file(fid=None, attached_to_doctype=None, attached_to_name=None, from_
 		if not file_name:
 			file_name = frappe.db.get_value("File", fid, "file_name")
 		comment = doc.add_comment("Attachment Removed", _("Removed {0}").format(file_name))
-		frappe.delete_doc("File", fid, ignore_permissions=ignore_permissions)
+		frappe.delete_doc("File", fid, ignore_permissions=ignore_permissions, delete_permanently=delete_permanently)
 
 	return comment
 
@@ -719,17 +750,18 @@ def get_max_file_size():
 	return conf.get('max_file_size') or 10485760
 
 
-def remove_all(dt, dn, from_delete=False):
+def remove_all(dt, dn, from_delete=False, delete_permanently=False):
 	"""remove all files in a transaction"""
 	try:
 		for fid in frappe.db.sql_list("""select name from `tabFile` where
 			attached_to_doctype=%s and attached_to_name=%s""", (dt, dn)):
 			if from_delete:
 				# If deleting a doc, directly delete files
-				frappe.delete_doc("File", fid, ignore_permissions=True)
+				frappe.delete_doc("File", fid, ignore_permissions=True, delete_permanently=delete_permanently)
 			else:
 				# Removes file and adds a comment in the document it is attached to
-				remove_file(fid=fid, attached_to_doctype=dt, attached_to_name=dn, from_delete=from_delete)
+				remove_file(fid=fid, attached_to_doctype=dt, attached_to_name=dn,
+					from_delete=from_delete, delete_permanently=delete_permanently)
 	except Exception as e:
 		if e.args[0]!=1054: raise # (temp till for patched)
 

frappe/core/doctype/file/test_file.py

@@ -192,8 +192,6 @@ def tearDown(self):
 
 
 class TestFile(unittest.TestCase):
-
-
 	def setUp(self):
 		self.delete_test_data()
 		self.upload_file()
@@ -375,3 +373,20 @@ def test_website_user_file_permission(self):
 		# because user gets permission via has_web_form_permission
 		self.assertIsInstance(txt_file, File)
 		frappe.set_user('Administrator')
+
+	def test_parent_directory_validation_in_file_url(self):
+		file1 = frappe.get_doc({
+			"doctype": "File",
+			"file_name": 'parent_dir.txt',
+			"attached_to_doctype": "",
+			"attached_to_name": "",
+			"is_private": 1,
+			"content": test_content1}).insert()
+
+		file1.file_url = '/private/files/../test.txt'
+		self.assertRaises(frappe.exceptions.ValidationError, file1.save)
+
+		# No validation to see if file exists
+		file1.reload()
+		file1.file_url = '/private/files/parent_dir2.txt'
+		file1.save()

frappe/core/doctype/prepared_report/prepared_report.py

@@ -24,8 +24,6 @@ def before_insert(self):
 	def enqueue_report(self):
 		enqueue(run_background, prepared_report=self.name, timeout=6000)
 
-	def on_trash(self):
-		remove_all("Prepared Report", self.name)
 
 
 def run_background(prepared_report):
@@ -100,7 +98,7 @@ def delete_expired_prepared_reports():
 def delete_prepared_reports(reports):
 	reports = frappe.parse_json(reports)
 	for report in reports:
-		frappe.delete_doc('Prepared Report', report['name'], ignore_permissions=True)
+		frappe.delete_doc('Prepared Report', report['name'], ignore_permissions=True, delete_permanently=True)
 
 def create_json_gz_file(data, dt, dn):
 	# Storing data in CSV file causes information loss

frappe/core/doctype/system_settings/system_settings.json

@@ -59,8 +59,7 @@
   "enable_prepared_report_auto_deletion",
   "prepared_report_expiry_period",
   "chat",
-  "enable_chat",
-  "use_socketio_to_upload_file"
+  "enable_chat"
  ],
  "fields": [
   {
@@ -384,12 +383,6 @@
    "fieldtype": "Check",
    "label": "Enable Chat"
   },
-  {
-   "default": "1",
-   "fieldname": "use_socketio_to_upload_file",
-   "fieldtype": "Check",
-   "label": "Use socketio to upload file"
-  },
   {
    "fieldname": "column_break_21",
    "fieldtype": "Column Break"
@@ -404,7 +397,7 @@
   {
    "default": "7",
    "depends_on": "enable_prepared_report_auto_deletion",
-   "description": "System will automatically delete Prepared Reports after these many days since creation",
+   "description": "System will auto-delete Prepared Reports permanently after these many days since creation",
    "fieldname": "prepared_report_expiry_period",
    "fieldtype": "Int",
    "label": "Prepared Report Expiry Period (Days)"
@@ -424,7 +417,8 @@
  ],
  "icon": "fa fa-cog",
  "issingle": 1,
- "modified": "2020-07-26 15:57:34.197718",
+ "links": [],
+ "modified": "2021-03-25 17:54:32.668876",
  "modified_by": "Administrator",
  "module": "Core",
  "name": "System Settings",