fix(regional): rename duplicate Customer fields in Italy setup (backport #50921) (#53397)

* fix(regional): rename duplicate Customer fields in Italy setup (#50921)

Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
(cherry picked from commit c6efc403cd)

# Conflicts:
#	erpnext/patches.txt

* chore: resolve conflicts

* chore: resolve conflicts

Removed obsolete patches for older versions.

---------

Co-authored-by: Solede <lorenzo.caldara@gmail.com>
Co-authored-by: Mihir Kandoi <kandoimihir@gmail.com>

erpnext/patches.txt

@@ -432,3 +432,4 @@ erpnext.patches.v16_0.set_ordered_qty_in_quotation_item
 erpnext.patches.v15_0.replace_http_with_https_in_sales_partner
 erpnext.patches.v16_0.add_portal_redirects
 erpnext.patches.v16_0.update_order_qty_and_requested_qty_based_on_mr_and_po
+erpnext.patches.v15_0.rename_italy_customer_name_fields

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 = '')
+		"""
+		)

erpnext/regional/italy/e-invoice.xml

@@ -97,8 +97,8 @@
         {%- if doc.customer_data.customer_type == "Individual" %}
           <CodiceFiscale>{{ doc.customer_data.fiscal_code }}</CodiceFiscale>
           <Anagrafica>
-            <Nome>{{ doc.customer_data.first_name }}</Nome>
-            <Cognome>{{ doc.customer_data.last_name }}</Cognome>
+            <Nome>{{ doc.customer_data.italy_customer_first_name }}</Nome>
+            <Cognome>{{ doc.customer_data.italy_customer_last_name }}</Cognome>
           </Anagrafica>
         {%- else %}
           <IdFiscaleIVA>

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"',
 			),

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()