Merge branch 'develop' into fix-espresso

frappe/desk/query_report.py

@@ -83,8 +83,8 @@ def generate_report_result(
 	columns, result, message, chart, report_summary, skip_total_row = ljust_list(res, 6)
 	columns = [get_column_as_dict(col) for col in (columns or [])]
 	report_column_names = [col["fieldname"] for col in columns]
-
 	# convert to list of dicts
+
 	result = normalize_result(result, columns)
 
 	if report.custom_columns:
@@ -230,6 +230,14 @@ def run(
 
 
 def add_custom_column_data(custom_columns, result):
+	doctype_names_from_custom_field = []
+	for column in custom_columns:
+		if len(column["fieldname"].split("-")) > 1:
+			# length greater than 1, means that the column is a custom field with confilicting fieldname
+			doctype_name = frappe.unscrub(column["fieldname"].split("-")[1])
+			doctype_names_from_custom_field.append(doctype_name)
+		column["fieldname"] = column["fieldname"].split("-")[0]
+
 	custom_column_data = get_data_for_custom_report(custom_columns, result)
 
 	for column in custom_columns:
@@ -247,6 +255,8 @@ def add_custom_column_data(custom_columns, result):
 				# possible if the row is empty
 				if not row_reference:
 					continue
+				if key[0] in doctype_names_from_custom_field:
+					column["fieldname"] = column.get("id")
 				row[column.get("fieldname")] = custom_column_data.get(key).get(row_reference)
 
 	return result
@@ -508,7 +518,6 @@ def get_data_for_custom_report(columns, result):
 			names = list(set(names))
 
 			doc_field_value_map[(doctype, fieldname)] = get_data_for_custom_field(doctype, fieldname, names)
-
 	return doc_field_value_map
 
 

frappe/oauth.py

@@ -539,20 +539,8 @@ def calculate_at_hash(access_token, hash_alg):
 
 
 def delete_oauth2_data():
-	# Delete Invalid Authorization Code and Revoked Token
-	commit_code, commit_token = False, False
-	code_list = frappe.get_all("OAuth Authorization Code", filters={"validity": "Invalid"})
-	token_list = frappe.get_all("OAuth Bearer Token", filters={"status": "Revoked"})
-	if len(code_list) > 0:
-		commit_code = True
-	if len(token_list) > 0:
-		commit_token = True
-	for code in code_list:
-		frappe.delete_doc("OAuth Authorization Code", code["name"])
-	for token in token_list:
-		frappe.delete_doc("OAuth Bearer Token", token["name"])
-	if commit_code or commit_token:
-		frappe.db.commit()
+	frappe.db.delete("OAuth Authorization Code", {"validity": "Invalid"})
+	frappe.db.delete("OAuth Bearer Token", {"status": "Revoked"})
 
 
 def get_client_scopes(client_id):

frappe/public/js/frappe/views/reports/query_report.js

@@ -1689,8 +1689,13 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList {
 							const insert_after_index = this.columns.findIndex(
 								(column) => column.label === values.insert_after
 							);
+
 							custom_columns.push({
-								fieldname: df.fieldname,
+								fieldname: this.columns
+									.map((column) => column.fieldname)
+									.includes(df.fieldname)
+									? df.fieldname + "-" + frappe.scrub(values.doctype)
+									: df.fieldname,
 								fieldtype: df.fieldtype,
 								label: df.label,
 								insert_after_index: insert_after_index,
@@ -1714,12 +1719,11 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList {
 									const custom_data = r.message;
 									const link_field =
 										this.doctype_field_map[values.doctype].fieldname;
-
 									this.add_custom_column(
 										custom_columns,
 										custom_data,
 										link_field,
-										values.field,
+										values,
 										insert_after_index
 									);
 									d.hide();
@@ -1800,13 +1804,25 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList {
 		}
 	}
 
-	add_custom_column(custom_column, custom_data, link_field, column_field, insert_after_index) {
+	add_custom_column(
+		custom_column,
+		custom_data,
+		link_field,
+		new_column_data,
+		insert_after_index
+	) {
 		const column = this.prepare_columns(custom_column);
+		const column_field = new_column_data.field;
 
 		this.columns.splice(insert_after_index + 1, 0, column[0]);
 
 		this.data.forEach((row) => {
-			row[column_field] = custom_data[row[link_field]];
+			if (column[0].fieldname.includes("-")) {
+				row[column_field + "-" + frappe.scrub(new_column_data.doctype)] =
+					custom_data[row[link_field]];
+			} else {
+				row[column_field] = custom_data[row[link_field]];
+			}
 		});
 
 		this.render_datatable();

frappe/tests/test_query_report.py

@@ -3,12 +3,21 @@
 
 import frappe
 import frappe.utils
-from frappe.desk.query_report import build_xlsx_data, export_query
+from frappe.core.doctype.doctype.test_doctype import new_doctype
+from frappe.desk.query_report import build_xlsx_data, export_query, run
 from frappe.tests.utils import FrappeTestCase
 from frappe.utils.xlsxutils import make_xlsx
 
 
 class TestQueryReport(FrappeTestCase):
+	@classmethod
+	def setUpClass(cls) -> None:
+		cls.enable_safe_exec()
+		return super().setUpClass()
+
+	def tearDown(self):
+		frappe.db.rollback()
+
 	def test_xlsx_data_with_multiple_datatypes(self):
 		"""Test exporting report using rows with multiple datatypes (list, dict)"""
 
@@ -110,3 +119,120 @@ class TestQueryReport(FrappeTestCase):
 						self.assertIn(column, row)
 
 		frappe.delete_doc("Report", REPORT_NAME, delete_permanently=True)
+
+	def test_report_for_duplicate_column_names(self):
+		"""Test report with duplicate column names"""
+
+		try:
+			fields = [
+				{"label": "First Name", "fieldname": "first_name", "fieldtype": "Data"},
+				{"label": "Last Name", "fieldname": "last_name", "fieldtype": "Data"},
+			]
+			docA = frappe.get_doc(
+				{
+					"doctype": "DocType",
+					"name": "Doc A",
+					"module": "Core",
+					"custom": 1,
+					"autoname": "field:first_name",
+					"fields": fields,
+					"permissions": [{"role": "System Manager"}],
+				}
+			).insert(ignore_if_duplicate=True)
+
+			docB = frappe.get_doc(
+				{
+					"doctype": "DocType",
+					"name": "Doc B",
+					"module": "Core",
+					"custom": 1,
+					"autoname": "field:last_name",
+					"fields": fields,
+					"permissions": [{"role": "System Manager"}],
+				}
+			).insert(ignore_if_duplicate=True)
+
+			for i in range(1, 3):
+				frappe.get_doc({"doctype": "Doc A", "first_name": f"John{i}", "last_name": "Doe"}).insert()
+				frappe.get_doc({"doctype": "Doc B", "last_name": f"Doe{i}", "first_name": "John"}).insert()
+
+			if not frappe.db.exists("Report", "Doc A Report"):
+				report = frappe.get_doc(
+					{
+						"doctype": "Report",
+						"ref_doctype": "Doc A",
+						"report_name": "Doc A Report",
+						"report_type": "Script Report",
+						"is_standard": "No",
+					}
+				).insert(ignore_permissions=True)
+			else:
+				report = frappe.get_doc("Report", "Doc A Report")
+
+			report.report_script = """
+result = [["Ritvik","Sardana", "Doe1"],["Shariq","Ansari", "Doe2"]]
+columns = [{
+			"label": "First Name",
+			"fieldname": "first_name",
+			"fieldtype": "Data",
+			"width": 180,
+		},
+		{
+			"label": "Last Name",
+			"fieldname": "last_name",
+			"fieldtype": "Data",
+			"width": 180,
+		},
+		{
+			"label": "Linked Field",
+			"fieldname": "linked_field",
+			"fieldtype": "Link",
+			"options": "Doc B",
+			"width": 180,
+		},
+	]
+
+data = columns, result
+				"""
+			report.save()
+
+			custom_columns = [
+				{
+					"fieldname": "first_name-Doc_B",
+					"fieldtype": "Data",
+					"label": "First Name",
+					"insert_after_index": 1,
+					"link_field": {"fieldname": "linked_field", "names": {}},
+					"doctype": "Doc B",
+					"width": 100,
+					"id": "first_name-Doc_B",
+					"name": "First Name",
+					"editable": False,
+					"compareValue": None,
+				},
+			]
+
+			response = run(
+				"Doc A Report",
+				filters={"user": "Administrator", "doctype": "Doc A"},
+				custom_columns=custom_columns,
+			)
+
+			self.assertListEqual(
+				["first_name", "last_name", "first_name-Doc_B", "linked_field"],
+				[d["fieldname"] for d in response["columns"]],
+			)
+
+			self.assertDictEqual(
+				{
+					"first_name": "Ritvik",
+					"last_name": "Sardana",
+					"linked_field": "Doe1",
+					"first_name-Doc_B": "John",
+				},
+				response["result"][0],
+			)
+
+		except Exception as e:
+			raise e
+			frappe.db.rollback()

frappe/utils/backups.py

@@ -15,6 +15,7 @@ from cryptography.fernet import Fernet
 
 # imports - module imports
 import frappe
+import frappe.utils
 from frappe import conf
 from frappe.utils import cint, get_file_size, get_url, now, now_datetime
 
@@ -343,12 +344,15 @@ class BackupGenerator:
 			backup_path = self.backup_path_files if folder == "public" else self.backup_path_private_files
 
 			if self.compress_files:
-				cmd_string = "tar cf - {1} | gzip > {0}"
+				cmd_string = "self=$$; ( tar cf - {1} || kill $self ) | gzip > {0}"
 			else:
 				cmd_string = "tar -cf {0} {1}"
 
 			frappe.utils.execute_in_shell(
-				cmd_string.format(backup_path, files_path), verbose=self.verbose, low_priority=True
+				cmd_string.format(backup_path, files_path),
+				verbose=self.verbose,
+				low_priority=True,
+				check_exit_code=True,
 			)
 
 	def copy_site_config(self):