Merge pull request #486 from resilient-tech/mergify/bp/version-14-hot…

…fix/pr-485

refactor: separate modules for uninstall and custom field utils (backport #485)

india_compliance/gst_india/doctype/gst_settings/gst_settings.py

@@ -16,7 +16,8 @@
     _disable_api_promo,
     post_login,
 )
-from india_compliance.gst_india.utils import can_enable_api, toggle_custom_fields
+from india_compliance.gst_india.utils import can_enable_api
+from india_compliance.gst_india.utils.custom_fields import toggle_custom_fields
 
 
 class GSTSettings(Document):
@@ -56,7 +57,6 @@ def validate_gst_accounts(self):
         company_wise_account_types = {}
 
         for row in self.gst_accounts:
-
             # Validate Duplicate Accounts
             for fieldname in GST_ACCOUNT_FIELDS:
                 account = row.get(fieldname)

india_compliance/gst_india/setup/__init__.py

@@ -13,10 +13,8 @@
     SALES_REVERSE_CHARGE_FIELDS,
 )
 from india_compliance.gst_india.setup.property_setters import get_property_setters
-from india_compliance.gst_india.utils import get_data_file_path, toggle_custom_fields
-from india_compliance.patches.post_install.update_e_invoice_fields_and_logs import (
-    delete_custom_fields as _delete_custom_fields,
-)
+from india_compliance.gst_india.utils import get_data_file_path
+from india_compliance.gst_india.utils.custom_fields import toggle_custom_fields
 
 ITEM_VARIANT_FIELDNAMES = frozenset(("gst_hsn_code", "is_nil_exempt", "is_non_gst"))
 
@@ -31,51 +29,18 @@ def after_install():
     add_fields_to_item_variant_settings()
 
 
-def before_uninstall():
-    delete_custom_fields()
-    delete_property_setters()
-    remove_fields_from_item_variant_settings()
-
-
 def create_custom_fields():
     # Validation ignored for faster creation
     # Will not fail if a core field with same name already exists (!)
     # Will update a custom field if it already exists
-    _create_custom_fields(
-        _get_custom_fields_map(
-            CUSTOM_FIELDS,
-            SALES_REVERSE_CHARGE_FIELDS,
-            E_INVOICE_FIELDS,
-            E_WAYBILL_FIELDS,
-        ),
-        ignore_validate=True,
-    )
+    _create_custom_fields(get_all_custom_fields(), ignore_validate=True)
 
 
 def create_property_setters():
     for property_setter in get_property_setters():
         frappe.make_property_setter(property_setter)
 
 
-def delete_custom_fields():
-    _delete_custom_fields(
-        _get_custom_fields_map(
-            CUSTOM_FIELDS,
-            SALES_REVERSE_CHARGE_FIELDS,
-            E_INVOICE_FIELDS,
-            E_WAYBILL_FIELDS,
-        )
-    )
-
-
-def delete_property_setters():
-    for property_setter in get_property_setters():
-        keys_to_update = ["doc_type", "field_name", "property", "value"]
-        # Update the keys to match with the property setter fields
-        filters = dict(zip(keys_to_update, list(property_setter.values())))
-        frappe.db.delete("Property Setter", filters)
-
-
 def create_address_template():
     if frappe.db.exists("Address Template", "India"):
         return
@@ -224,28 +189,27 @@ def show_accounts_settings_override_warning():
     )
 
     click.secho(
-        "This is being set as Billing Address, since that's the correct "
-        "address for determining GST applicablility.",
+        (
+            "This is being set as Billing Address, since that's the correct "
+            "address for determining GST applicablility."
+        ),
         fg="yellow",
     )
 
 
-def _get_custom_fields_map(*custom_fields_list):
+def get_all_custom_fields():
     result = {}
 
-    for custom_fields in custom_fields_list:
+    for custom_fields in (
+        CUSTOM_FIELDS,
+        SALES_REVERSE_CHARGE_FIELDS,
+        E_INVOICE_FIELDS,
+        E_WAYBILL_FIELDS,
+    ):
         for doctypes, fields in custom_fields.items():
             if isinstance(fields, dict):
                 fields = [fields]
 
             result.setdefault(doctypes, []).extend(fields)
 
     return result
-
-
-def remove_fields_from_item_variant_settings():
-    settings = frappe.get_doc("Item Variant Settings")
-    settings.fields = [
-        row for row in settings.fields if row.field_name not in ITEM_VARIANT_FIELDNAMES
-    ]
-    settings.save()

india_compliance/gst_india/uninstall.py

@@ -0,0 +1,36 @@
+import frappe
+
+from india_compliance.gst_india.setup import (
+    ITEM_VARIANT_FIELDNAMES,
+    get_all_custom_fields,
+    get_property_setters,
+)
+from india_compliance.gst_india.utils.custom_fields import delete_custom_fields
+
+
+def before_uninstall():
+    delete_custom_fields(get_all_custom_fields())
+    delete_property_setters()
+    remove_fields_from_item_variant_settings()
+
+
+def delete_property_setters():
+    field_map = {
+        "doctype": "doc_type",
+        "fieldname": "field_name",
+    }
+
+    for property_setter in get_property_setters():
+        for key, fieldname in field_map.items():
+            if key in property_setter:
+                property_setter[fieldname] = property_setter.pop(key)
+
+        frappe.db.delete("Property Setter", property_setter)
+
+
+def remove_fields_from_item_variant_settings():
+    settings = frappe.get_doc("Item Variant Settings")
+    settings.fields = [
+        row for row in settings.fields if row.field_name not in ITEM_VARIANT_FIELDNAMES
+    ]
+    settings.save()

india_compliance/gst_india/utils/__init__.py

@@ -318,37 +318,6 @@ def get_all_gst_accounts(company):
     return accounts_list
 
 
-def toggle_custom_fields(custom_fields, show):
-    """
-    Show / hide custom fields
-
-    :param custom_fields: a dict like `{'Sales Invoice': [{fieldname: 'test', ...}]}`
-    :param show: True to show fields, False to hide
-    """
-
-    for doctypes, fields in custom_fields.items():
-        if isinstance(fields, dict):
-            # only one field
-            fields = [fields]
-
-        if isinstance(doctypes, str):
-            # only one doctype
-            doctypes = (doctypes,)
-
-        for doctype in doctypes:
-            frappe.db.set_value(
-                "Custom Field",
-                {
-                    "dt": doctype,
-                    "fieldname": ["in", [field["fieldname"] for field in fields]],
-                },
-                "hidden",
-                int(not show),
-            )
-
-            frappe.clear_cache(doctype=doctype)
-
-
 def parse_datetime(value, day_first=False):
     """Convert IST string to offset-naive system time"""
 
@@ -407,22 +376,6 @@ def get_titlecase_version(word, all_caps=False, **kwargs):
         return word
 
 
-def delete_old_fields(fields, doctypes):
-    if isinstance(fields, str):
-        fields = (fields,)
-
-    if isinstance(doctypes, str):
-        doctypes = (doctypes,)
-
-    frappe.db.delete(
-        "Custom Field",
-        {
-            "fieldname": ("in", fields),
-            "dt": ("in", doctypes),
-        },
-    )
-
-
 def is_api_enabled(settings=None):
     if not settings:
         settings = frappe.get_cached_value(

india_compliance/gst_india/utils/custom_fields.py

@@ -0,0 +1,74 @@
+import frappe
+
+
+def toggle_custom_fields(custom_fields, show):
+    """
+    Show / hide custom fields
+
+    :param custom_fields: a dict like `{'Sales Invoice': [{fieldname: 'test', ...}]}`
+    :param show: True to show fields, False to hide
+    """
+
+    for doctypes, fields in custom_fields.items():
+        if isinstance(fields, dict):
+            # only one field
+            fields = [fields]
+
+        if isinstance(doctypes, str):
+            # only one doctype
+            doctypes = (doctypes,)
+
+        for doctype in doctypes:
+            frappe.db.set_value(
+                "Custom Field",
+                {
+                    "dt": doctype,
+                    "fieldname": ["in", [field["fieldname"] for field in fields]],
+                },
+                "hidden",
+                int(not show),
+            )
+
+            frappe.clear_cache(doctype=doctype)
+
+
+def delete_old_fields(fieldnames, doctypes):
+    if isinstance(fieldnames, str):
+        fields = (fieldnames,)
+
+    if isinstance(doctypes, str):
+        doctypes = (doctypes,)
+
+    frappe.db.delete(
+        "Custom Field",
+        {
+            "fieldname": ("in", fields),
+            "dt": ("in", doctypes),
+        },
+    )
+
+
+def delete_custom_fields(custom_fields):
+    """
+    :param custom_fields: a dict like `{'Sales Invoice': [{fieldname: 'test', ...}]}`
+    """
+
+    for doctypes, fields in custom_fields.items():
+        if isinstance(fields, dict):
+            # only one field
+            fields = [fields]
+
+        if isinstance(doctypes, str):
+            # only one doctype
+            doctypes = (doctypes,)
+
+        for doctype in doctypes:
+            frappe.db.delete(
+                "Custom Field",
+                {
+                    "fieldname": ("in", [field["fieldname"] for field in fields]),
+                    "dt": doctype,
+                },
+            )
+
+            frappe.clear_cache(doctype=doctype)

india_compliance/income_tax_india/setup.py

@@ -1,14 +1,7 @@
 from frappe.custom.doctype.custom_field.custom_field import create_custom_fields
 
 from india_compliance.income_tax_india.constants.custom_fields import CUSTOM_FIELDS
-from india_compliance.patches.post_install.update_e_invoice_fields_and_logs import (
-    delete_custom_fields,
-)
 
 
 def after_install():
-    create_custom_fields(CUSTOM_FIELDS, update=True)
-
-
-def before_uninstall():
-    delete_custom_fields(CUSTOM_FIELDS)
+    create_custom_fields(CUSTOM_FIELDS, ignore_validate=True)

india_compliance/income_tax_india/uninstall.py

@@ -0,0 +1,6 @@
+from india_compliance.gst_india.utils.custom_fields import delete_custom_fields
+from india_compliance.income_tax_india.constants.custom_fields import CUSTOM_FIELDS
+
+
+def before_uninstall():
+    delete_custom_fields(CUSTOM_FIELDS)

india_compliance/patches/post_install/migrate_e_invoice_settings_to_gst_settings.py

@@ -5,7 +5,7 @@
 from frappe.utils.password import decrypt
 
 from india_compliance.gst_india.constants.custom_fields import E_INVOICE_FIELDS
-from india_compliance.gst_india.utils import toggle_custom_fields
+from india_compliance.gst_india.utils.custom_fields import toggle_custom_fields
 
 
 def execute():
@@ -39,8 +39,10 @@ def execute():
     if sbool(old_settings.enable):
         toggle_custom_fields(E_INVOICE_FIELDS, True)
         click.secho(
-            "Your e-Invoice Settings have been migrated to GST Settings."
-            " Please enable the e-Invoice API in GST Settings manually.\n",
+            (
+                "Your e-Invoice Settings have been migrated to GST Settings."
+                " Please enable the e-Invoice API in GST Settings manually.\n"
+            ),
             fg="yellow",
         )
 

india_compliance/patches/post_install/remove_old_fields.py

@@ -1,4 +1,4 @@
-from india_compliance.gst_india.utils import delete_old_fields
+from india_compliance.gst_india.utils.custom_fields import delete_old_fields
 
 
 def execute():

india_compliance/patches/post_install/set_default_gst_settings.py

@@ -4,7 +4,7 @@
 from india_compliance.gst_india.constants.custom_fields import (
     SALES_REVERSE_CHARGE_FIELDS,
 )
-from india_compliance.gst_india.utils import toggle_custom_fields
+from india_compliance.gst_india.utils.custom_fields import toggle_custom_fields
 
 # Enable setting only if transaction exists in last 3 years.
 POSTING_DATE_CONDITION = {

india_compliance/patches/post_install/set_gst_category.py

@@ -1,6 +1,6 @@
 import frappe
 
-from india_compliance.gst_india.utils import delete_old_fields
+from india_compliance.gst_india.utils.custom_fields import delete_old_fields
 
 
 def execute():

india_compliance/patches/post_install/setup_custom_fields_for_gst.py

@@ -1,6 +1,6 @@
 import frappe
 
-from india_compliance.gst_india.utils import delete_old_fields
+from india_compliance.gst_india.utils.custom_fields import delete_old_fields
 
 
 def execute():