mirror of
https://github.com/frappe/erpnext.git
synced 2026-05-16 03:29:16 +00:00
Revert "fix(regional): rename duplicate Customer fields in Italy setup" (#53409)
This commit is contained in:
@@ -471,4 +471,3 @@ 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.complete_onboarding_steps_for_older_sites #2
|
||||||
erpnext.patches.v16_0.enable_serial_batch_setting
|
erpnext.patches.v16_0.enable_serial_batch_setting
|
||||||
erpnext.patches.v16_0.co_by_product_patch
|
erpnext.patches.v16_0.co_by_product_patch
|
||||||
erpnext.patches.v15_0.rename_italy_customer_name_fields
|
|
||||||
|
|||||||
@@ -1,65 +0,0 @@
|
|||||||
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 = '')
|
|
||||||
"""
|
|
||||||
)
|
|
||||||
@@ -97,8 +97,8 @@
|
|||||||
{%- if doc.customer_data.customer_type == "Individual" %}
|
{%- if doc.customer_data.customer_type == "Individual" %}
|
||||||
<CodiceFiscale>{{ doc.customer_data.fiscal_code }}</CodiceFiscale>
|
<CodiceFiscale>{{ doc.customer_data.fiscal_code }}</CodiceFiscale>
|
||||||
<Anagrafica>
|
<Anagrafica>
|
||||||
<Nome>{{ doc.customer_data.italy_customer_first_name }}</Nome>
|
<Nome>{{ doc.customer_data.first_name }}</Nome>
|
||||||
<Cognome>{{ doc.customer_data.italy_customer_last_name }}</Cognome>
|
<Cognome>{{ doc.customer_data.last_name }}</Cognome>
|
||||||
</Anagrafica>
|
</Anagrafica>
|
||||||
{%- else %}
|
{%- else %}
|
||||||
<IdFiscaleIVA>
|
<IdFiscaleIVA>
|
||||||
|
|||||||
@@ -232,7 +232,7 @@ def make_custom_fields(update=True):
|
|||||||
depends_on='eval:doc.customer_type=="Company"',
|
depends_on='eval:doc.customer_type=="Company"',
|
||||||
),
|
),
|
||||||
dict(
|
dict(
|
||||||
fieldname="italy_customer_first_name",
|
fieldname="first_name",
|
||||||
label="First Name",
|
label="First Name",
|
||||||
fieldtype="Data",
|
fieldtype="Data",
|
||||||
insert_after="salutation",
|
insert_after="salutation",
|
||||||
@@ -240,10 +240,10 @@ def make_custom_fields(update=True):
|
|||||||
depends_on='eval:doc.customer_type!="Company"',
|
depends_on='eval:doc.customer_type!="Company"',
|
||||||
),
|
),
|
||||||
dict(
|
dict(
|
||||||
fieldname="italy_customer_last_name",
|
fieldname="last_name",
|
||||||
label="Last Name",
|
label="Last Name",
|
||||||
fieldtype="Data",
|
fieldtype="Data",
|
||||||
insert_after="italy_customer_first_name",
|
insert_after="first_name",
|
||||||
print_hide=1,
|
print_hide=1,
|
||||||
depends_on='eval:doc.customer_type!="Company"',
|
depends_on='eval:doc.customer_type!="Company"',
|
||||||
),
|
),
|
||||||
|
|||||||
@@ -1,259 +0,0 @@
|
|||||||
"""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()
|
|
||||||
Reference in New Issue
Block a user