From e5cd30c5ab1d46479c0c152a6c048cb8a7f65427 Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Wed, 19 Aug 2026 17:12:16 +0530 Subject: [PATCH] fix(setup): handle Transaction Deletion Record CSV edge cases (#58300) Co-authored-by: Pedro Barbosa --- .../test_transaction_deletion_record.py | 52 +++++++++++++++++++ .../transaction_deletion_record.py | 25 +++++---- 2 files changed, 68 insertions(+), 9 deletions(-) diff --git a/erpnext/setup/doctype/transaction_deletion_record/test_transaction_deletion_record.py b/erpnext/setup/doctype/transaction_deletion_record/test_transaction_deletion_record.py index 79af85476d1..f51f918f7a2 100644 --- a/erpnext/setup/doctype/transaction_deletion_record/test_transaction_deletion_record.py +++ b/erpnext/setup/doctype/transaction_deletion_record/test_transaction_deletion_record.py @@ -163,6 +163,58 @@ class TestTransactionDeletionRecord(ERPNextTestSuite): # Task should have company field set self.assertIsNotNone(row.company_field, "Task should have company_field set after import") + def test_csv_import_tolerates_short_rows(self): + """Test that CSV rows omitting trailing columns are imported with an auto-detected company_field""" + company = "_Test Company 7" + + tdr = frappe.new_doc("Transaction Deletion Record") + tdr.company = company + tdr.insert() + + # Row omits the trailing company_field and child_doctypes columns; csv.DictReader fills + # them with None, which used to raise on None.strip(). + csv_content = "doctype_name,company_field,child_doctypes\nSales Invoice\n" + result = tdr.import_to_delete_template_method(csv_content) + tdr.reload() + + self.assertEqual(result["imported"], 1) + self.assertEqual(len(tdr.doctypes_to_delete), 1) + + row = tdr.doctypes_to_delete[0] + self.assertEqual(row.doctype_name, "Sales Invoice") + # company_field was not provided, so it should be auto-detected as "company" + self.assertEqual(row.company_field, "company") + + def test_csv_import_all_skipped_rows_preserves_existing_to_delete_list(self): + """Test that an all-skipped CSV import does not wipe an existing To Delete list""" + company = "_Test Company 7" + create_task(company) + + tdr = frappe.new_doc("Transaction Deletion Record") + tdr.company = company + tdr.insert() + tdr.generate_to_delete_list() + tdr.reload() + + original_doctype_names = [row.doctype_name for row in tdr.doctypes_to_delete] + self.assertGreater(len(original_doctype_names), 0) + + protected_doctype = "DocType" + nonexistent_doctype = "Nonexistent Doctype For Import Test" + csv_content = ( + "doctype_name,company_field,child_doctypes\n" + f"{protected_doctype},,\n" + f"{nonexistent_doctype},,\n" + ) + result = tdr.import_to_delete_template_method(csv_content) + + self.assertEqual(result["imported"], 0) + self.assertGreater(result["skipped"], 0) + + tdr.reload() + self.assertEqual(len(tdr.doctypes_to_delete), len(original_doctype_names)) + self.assertEqual([row.doctype_name for row in tdr.doctypes_to_delete], original_doctype_names) + def test_progress_tracking(self): """Test that deleted checkbox is marked when DocType deletion completes""" company = "_Test Company 7" diff --git a/erpnext/setup/doctype/transaction_deletion_record/transaction_deletion_record.py b/erpnext/setup/doctype/transaction_deletion_record/transaction_deletion_record.py index 82ef3bb766b..897cf6f6b19 100644 --- a/erpnext/setup/doctype/transaction_deletion_record/transaction_deletion_record.py +++ b/erpnext/setup/doctype/transaction_deletion_record/transaction_deletion_record.py @@ -510,15 +510,16 @@ class TransactionDeletionRecord(Document): if "doctype_name" not in (reader.fieldnames or []): frappe.throw(_("Invalid CSV format. Expected column: doctype_name")) - self.doctypes_to_delete = [] protected = _get_protected_doctypes_internal() - imported_count = 0 + imported_rows = [] skipped = [] for row in reader: - doctype_name = row.get("doctype_name", "").strip() - company_field = row.get("company_field", "").strip() or None + # csv.DictReader fills absent trailing fields with None, so the dict.get default + # never fires: coerce to "" before stripping. + doctype_name = (row.get("doctype_name") or "").strip() + company_field = (row.get("company_field") or "").strip() or None if not doctype_name: continue @@ -558,18 +559,24 @@ class TransactionDeletionRecord(Document): details = self._get_to_delete_row_infos(doctype_name, db_company_fields[0]) import_company_field = db_company_fields[0] - self.append( - "doctypes_to_delete", + imported_rows.append( { "doctype_name": doctype_name, "company_field": import_company_field, "document_count": details["document_count"], "child_doctypes": details["child_doctypes"], - }, + } ) - imported_count += 1 - self.save() + imported_count = len(imported_rows) + + # Only replace the existing To Delete list once we have at least one valid row, so an + # all-invalid CSV does not silently wipe a previously generated list. + if imported_rows: + self.doctypes_to_delete = [] + for imported_row in imported_rows: + self.append("doctypes_to_delete", imported_row) + self.save() if skipped: frappe.msgprint(