diff --git a/erpnext/patches.txt b/erpnext/patches.txt index 7177cd5c2f5..5df92026804 100644 --- a/erpnext/patches.txt +++ b/erpnext/patches.txt @@ -471,3 +471,4 @@ erpnext.patches.v16_0.update_order_qty_and_requested_qty_based_on_mr_and_po erpnext.patches.v16_0.complete_onboarding_steps_for_older_sites #2 erpnext.patches.v16_0.enable_serial_batch_setting erpnext.patches.v16_0.co_by_product_patch +erpnext.patches.v15_0.rename_italy_customer_name_fields diff --git a/erpnext/patches/v15_0/rename_italy_customer_name_fields.py b/erpnext/patches/v15_0/rename_italy_customer_name_fields.py new file mode 100644 index 00000000000..e56adc8765e --- /dev/null +++ b/erpnext/patches/v15_0/rename_italy_customer_name_fields.py @@ -0,0 +1,65 @@ +import frappe + + +def execute(): + """Rename Italy regional custom fields to avoid conflict with standard Customer fields. + + The Italy regional setup created custom fields 'first_name' and 'last_name' on Customer + which conflict with the standard read-only fields that fetch from customer_primary_contact. + This patch renames them to 'italy_customer_first_name' and 'italy_customer_last_name'. + """ + # Check if old fields exist and are the Italy regional ones + old_first_name_exists = frappe.db.exists("Custom Field", "Customer-first_name") + old_last_name_exists = frappe.db.exists("Custom Field", "Customer-last_name") + + is_italy_first_name = False + is_italy_last_name = False + + if old_first_name_exists: + field_doc = frappe.get_doc("Custom Field", "Customer-first_name") + is_italy_first_name = field_doc.depends_on and "customer_type" in field_doc.depends_on + + if old_last_name_exists: + field_doc = frappe.get_doc("Custom Field", "Customer-last_name") + is_italy_last_name = field_doc.depends_on and "customer_type" in field_doc.depends_on + + # If neither field is the Italy regional one, nothing to do + if not is_italy_first_name and not is_italy_last_name: + return + + # Step 1: Delete old Custom Field documents FIRST (to avoid duplicate field validation error) + if is_italy_first_name: + frappe.delete_doc("Custom Field", "Customer-first_name", force=True) + + if is_italy_last_name: + frappe.delete_doc("Custom Field", "Customer-last_name", force=True) + + # Step 2: Create the new fields and sync database schema + from erpnext.regional.italy.setup import make_custom_fields + + make_custom_fields(update=True) + + # Step 3: Migrate data from old columns to new columns (if old columns still exist in DB) + # Note: We do NOT drop the first_name/last_name columns because they are standard fields + # in Customer doctype (Read Only fields that fetch from customer_primary_contact). + # The Italy regional setup incorrectly created Custom Fields with the same names. + # We only migrate the data and leave the standard columns intact. + if is_italy_first_name and frappe.db.has_column("Customer", "first_name"): + frappe.db.sql( + """ + UPDATE `tabCustomer` + SET italy_customer_first_name = first_name + WHERE first_name IS NOT NULL AND first_name != '' + AND (italy_customer_first_name IS NULL OR italy_customer_first_name = '') + """ + ) + + if is_italy_last_name and frappe.db.has_column("Customer", "last_name"): + frappe.db.sql( + """ + UPDATE `tabCustomer` + SET italy_customer_last_name = last_name + WHERE last_name IS NOT NULL AND last_name != '' + AND (italy_customer_last_name IS NULL OR italy_customer_last_name = '') + """ + ) diff --git a/erpnext/regional/italy/e-invoice.xml b/erpnext/regional/italy/e-invoice.xml index 7c436a2b449..ed132d9998c 100644 --- a/erpnext/regional/italy/e-invoice.xml +++ b/erpnext/regional/italy/e-invoice.xml @@ -97,8 +97,8 @@ {%- if doc.customer_data.customer_type == "Individual" %} {{ doc.customer_data.fiscal_code }} - {{ doc.customer_data.first_name }} - {{ doc.customer_data.last_name }} + {{ doc.customer_data.italy_customer_first_name }} + {{ doc.customer_data.italy_customer_last_name }} {%- else %} diff --git a/erpnext/regional/italy/setup.py b/erpnext/regional/italy/setup.py index 9f9115ca12d..989f4da2251 100644 --- a/erpnext/regional/italy/setup.py +++ b/erpnext/regional/italy/setup.py @@ -232,7 +232,7 @@ def make_custom_fields(update=True): depends_on='eval:doc.customer_type=="Company"', ), dict( - fieldname="first_name", + fieldname="italy_customer_first_name", label="First Name", fieldtype="Data", insert_after="salutation", @@ -240,10 +240,10 @@ def make_custom_fields(update=True): depends_on='eval:doc.customer_type!="Company"', ), dict( - fieldname="last_name", + fieldname="italy_customer_last_name", label="Last Name", fieldtype="Data", - insert_after="first_name", + insert_after="italy_customer_first_name", print_hide=1, depends_on='eval:doc.customer_type!="Company"', ), diff --git a/erpnext/tests/test_italy_regional_patch.py b/erpnext/tests/test_italy_regional_patch.py new file mode 100644 index 00000000000..53034716d0e --- /dev/null +++ b/erpnext/tests/test_italy_regional_patch.py @@ -0,0 +1,259 @@ +"""Test for Italy regional patch: rename_italy_customer_name_fields. + +This test is completely DB-based to avoid dependencies on ERPNext test fixtures. +""" + +import unittest + +import frappe +from frappe.utils import now + + +class TestRenameItalyCustomerNameFields(unittest.TestCase): + """Test the patch that renames Italy regional custom fields on Customer.""" + + OLD_FIRST_NAME_FIELD = "Customer-first_name" + OLD_LAST_NAME_FIELD = "Customer-last_name" + NEW_FIRST_NAME_FIELD = "Customer-italy_customer_first_name" + NEW_LAST_NAME_FIELD = "Customer-italy_customer_last_name" + + @classmethod + def setUpClass(cls): + # Connect to the site + if not frappe.db: + frappe.connect() + cls.test_customer_name = "_Test Italy Patch Customer" + + def setUp(self): + """Set up test scenario: create old fields and test customer.""" + self._cleanup_fields() + self._cleanup_test_customer() + self._create_old_custom_fields_direct() + self._add_old_columns_to_db() + self._create_test_customer_direct() + + def tearDown(self): + """Clean up after test.""" + self._cleanup_test_customer() + self._cleanup_fields() + self._drop_old_columns_if_exist() + # Restore new fields from Italy setup + try: + from erpnext.regional.italy.setup import make_custom_fields + + make_custom_fields(update=True) + except (ImportError, AttributeError, ValueError) as e: + # Ignore setup failures in tearDown, but log for debugging + frappe.logger().warning(f"Failed to restore Italy setup in tearDown: {e}") + frappe.db.rollback() + + def _cleanup_fields(self): + """Remove both old and new custom fields.""" + for field_name in [ + self.OLD_FIRST_NAME_FIELD, + self.OLD_LAST_NAME_FIELD, + self.NEW_FIRST_NAME_FIELD, + self.NEW_LAST_NAME_FIELD, + ]: + if frappe.db.exists("Custom Field", field_name): + frappe.db.delete("Custom Field", {"name": field_name}) + + def _cleanup_test_customer(self): + """Remove test customer if exists.""" + if frappe.db.exists("Customer", self.test_customer_name): + # Delete directly from DB to avoid controller validation + frappe.db.delete("Customer", {"name": self.test_customer_name}) + + def _create_old_custom_fields_direct(self): + """Create the old custom fields directly in DB to bypass validation. + + This simulates the legacy state where Italy regional setup created + fields with names that now conflict with standard Customer fields. + """ + current_time = now() + + # Insert old first_name custom field directly + frappe.db.sql( + """ + INSERT INTO `tabCustom Field` + (name, creation, modified, modified_by, owner, docstatus, + dt, fieldname, fieldtype, label, insert_after, depends_on) + VALUES (%s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s) + """, + ( + self.OLD_FIRST_NAME_FIELD, + current_time, + current_time, + "Administrator", + "Administrator", + 0, + "Customer", + "first_name", + "Data", + "First Name", + "customer_name", + "eval:doc.customer_type == 'Individual'", + ), + ) + + # Insert old last_name custom field directly + frappe.db.sql( + """ + INSERT INTO `tabCustom Field` + (name, creation, modified, modified_by, owner, docstatus, + dt, fieldname, fieldtype, label, insert_after, depends_on) + VALUES (%s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s) + """, + ( + self.OLD_LAST_NAME_FIELD, + current_time, + current_time, + "Administrator", + "Administrator", + 0, + "Customer", + "last_name", + "Data", + "Last Name", + "first_name", + "eval:doc.customer_type == 'Individual'", + ), + ) + + frappe.db.commit() # nosemgrep: frappe-manual-commit -- required after raw SQL INSERT in test setup + + def _add_old_columns_to_db(self): + """Ensure old columns exist in the database table.""" + frappe.clear_cache() # Clear cache to get fresh column info + if not frappe.db.has_column("Customer", "first_name"): + frappe.db.sql_ddl("ALTER TABLE `tabCustomer` ADD COLUMN `first_name` VARCHAR(140)") + if not frappe.db.has_column("Customer", "last_name"): + frappe.db.sql_ddl("ALTER TABLE `tabCustomer` ADD COLUMN `last_name` VARCHAR(140)") + frappe.clear_cache() # Clear cache after adding columns + + def _drop_old_columns_if_exist(self): + """Drop old columns if they still exist.""" + frappe.clear_cache() # Clear cache to get fresh column info + try: + if frappe.db.has_column("Customer", "first_name"): + frappe.db.sql_ddl("ALTER TABLE `tabCustomer` DROP COLUMN `first_name`") + except frappe.db.InternalError as e: + # Column might already be dropped or locked + frappe.logger().debug(f"Could not drop first_name column: {e}") + try: + if frappe.db.has_column("Customer", "last_name"): + frappe.db.sql_ddl("ALTER TABLE `tabCustomer` DROP COLUMN `last_name`") + except frappe.db.InternalError as e: + # Column might already be dropped or locked + frappe.logger().debug(f"Could not drop last_name column: {e}") + frappe.clear_cache() # Clear cache after dropping columns + + def _create_test_customer_direct(self): + """Create a test customer directly in DB to avoid controller dependencies.""" + current_time = now() + + # Insert customer directly into DB + frappe.db.sql( + """ + INSERT INTO `tabCustomer` + (name, creation, modified, modified_by, owner, docstatus, + customer_name, customer_type, first_name, last_name) + VALUES (%s, %s, %s, %s, %s, %s, %s, %s, %s, %s) + """, + ( + self.test_customer_name, + current_time, + current_time, + "Administrator", + "Administrator", + 0, + self.test_customer_name, + "Individual", + "Mario", + "Rossi", + ), + ) + frappe.db.commit() # nosemgrep: frappe-manual-commit -- required after raw SQL INSERT in test setup + + def test_patch_renames_fields_and_migrates_data(self): + """Test that the patch renames fields and migrates data correctly.""" + # Verify old fields exist before patch + self.assertTrue(frappe.db.exists("Custom Field", self.OLD_FIRST_NAME_FIELD)) + self.assertTrue(frappe.db.exists("Custom Field", self.OLD_LAST_NAME_FIELD)) + + # Verify old data exists + old_first_name = frappe.db.get_value("Customer", self.test_customer_name, "first_name") + old_last_name = frappe.db.get_value("Customer", self.test_customer_name, "last_name") + self.assertEqual(old_first_name, "Mario") + self.assertEqual(old_last_name, "Rossi") + + # Execute the patch + from erpnext.patches.v15_0.rename_italy_customer_name_fields import execute + + execute() + + # Verify old Custom Field documents are deleted + self.assertFalse(frappe.db.exists("Custom Field", self.OLD_FIRST_NAME_FIELD)) + self.assertFalse(frappe.db.exists("Custom Field", self.OLD_LAST_NAME_FIELD)) + + # Verify new Custom Field documents exist + self.assertTrue(frappe.db.exists("Custom Field", self.NEW_FIRST_NAME_FIELD)) + self.assertTrue(frappe.db.exists("Custom Field", self.NEW_LAST_NAME_FIELD)) + + # Verify data was migrated to new columns + new_first_name = frappe.db.get_value("Customer", self.test_customer_name, "italy_customer_first_name") + new_last_name = frappe.db.get_value("Customer", self.test_customer_name, "italy_customer_last_name") + self.assertEqual(new_first_name, "Mario") + self.assertEqual(new_last_name, "Rossi") + + # Note: first_name/last_name columns are NOT dropped because they are + # standard Customer fields (Read Only, fetch from customer_primary_contact) + + def test_patch_skips_non_italy_fields(self): + """Test that the patch skips fields that are not Italy regional fields.""" + # Delete the Italy regional fields created in setUp + self._cleanup_fields() + self._drop_old_columns_if_exist() + self._cleanup_test_customer() + + current_time = now() + + # Create a custom field with same name but without Italy's depends_on + frappe.db.sql( + """ + INSERT INTO `tabCustom Field` + (name, creation, modified, modified_by, owner, docstatus, + dt, fieldname, fieldtype, label, insert_after) + VALUES (%s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s) + """, + ( + self.OLD_FIRST_NAME_FIELD, + current_time, + current_time, + "Administrator", + "Administrator", + 0, + "Customer", + "first_name", + "Data", + "First Name", + "customer_name", + ), + ) + frappe.db.commit() # nosemgrep: frappe-manual-commit -- required after raw SQL INSERT in test setup + + # Execute the patch + from erpnext.patches.v15_0.rename_italy_customer_name_fields import execute + + execute() + + # The non-Italy field should still exist (not renamed) + self.assertTrue(frappe.db.exists("Custom Field", self.OLD_FIRST_NAME_FIELD)) + + # Verify new Italy fields were NOT created (since this wasn't an Italy field) + self.assertFalse(frappe.db.exists("Custom Field", self.NEW_FIRST_NAME_FIELD)) + self.assertFalse(frappe.db.exists("Custom Field", self.NEW_LAST_NAME_FIELD)) + + +if __name__ == "__main__": + unittest.main()