Use ERPNext's update_account_number for placeholder rename + per-bank trace
The 0.1.9 placeholder repurposing didn't take effect on the test machine (account tree still showed XXXX rows even though E-Taxes Bank Account got populated). The most likely culprit was the manual doc.save() + frappe.rename_doc combo: doc.save runs Account.validate, which fires validate_account_currency and validate_account_number — either can throw on an in-place placeholder edit, leaving the account_name updated but the doc name stale, which dodges the XXXX-pattern delete sweep too. Switch to ERPNext's canonical update_account_number helper. It does the rename atomically, handles the tree-doctype reparenting, and is the exact path the in-app rename UI uses. Currency is set with a direct db.set_value beforehand so we never go through the full validate stack. Also expanded "Jey Wizard trace: bank native done" to a JSON dump that records, for every bank in the cache: currency, whether a same-currency placeholder was available, the resulting GL name, and the action taken (reused_placeholder / created_fresh / skipped_closed / error). Plus the placeholder_index keys/counts as seen at the start of the loop. Next run will show exactly which step is failing if anything still goes wrong.
This commit is contained in:
parent
048ab04d7a
commit
830d621a87
|
|
@ -1 +1 @@
|
||||||
__version__ = "0.1.9"
|
__version__ = "0.1.10"
|
||||||
|
|
|
||||||
|
|
@ -317,18 +317,35 @@ def _materialize_native_banks(company):
|
||||||
skipped_closed = 0
|
skipped_closed = 0
|
||||||
created_by_currency = {}
|
created_by_currency = {}
|
||||||
created_gl_names = []
|
created_gl_names = []
|
||||||
|
bank_trace = [] # per-bank: action taken + result, dumped to Error Log at end
|
||||||
for acc in accounts:
|
for acc in accounts:
|
||||||
if (acc.get("status") or "").upper() == "C":
|
if (acc.get("status") or "").upper() == "C":
|
||||||
skipped_closed += 1
|
skipped_closed += 1
|
||||||
|
bank_trace.append({"iban": acc.get("number"), "action": "skipped_closed"})
|
||||||
continue
|
continue
|
||||||
|
cur_for_log = (acc.get("currency") or "AZN").strip() or "AZN"
|
||||||
|
ph_available_before = bool(_peek_placeholder(placeholder_index, cur_for_log))
|
||||||
try:
|
try:
|
||||||
gl = _materialize_one_bank(company, parent, acc, placeholder_index)
|
gl = _materialize_one_bank(company, parent, acc, placeholder_index)
|
||||||
if gl:
|
if gl:
|
||||||
created_gl_names.append(gl)
|
created_gl_names.append(gl)
|
||||||
cur = (acc.get("currency") or "AZN").strip() or "AZN"
|
created_by_currency.setdefault(cur_for_log, []).append(gl)
|
||||||
created_by_currency.setdefault(cur, []).append(gl)
|
|
||||||
created += 1
|
created += 1
|
||||||
|
bank_trace.append({
|
||||||
|
"iban": acc.get("number"),
|
||||||
|
"currency": cur_for_log,
|
||||||
|
"placeholder_was_available": ph_available_before,
|
||||||
|
"resulting_gl": gl,
|
||||||
|
"action": "reused_placeholder" if (ph_available_before and gl) else "created_fresh",
|
||||||
|
})
|
||||||
except Exception as exc:
|
except Exception as exc:
|
||||||
|
bank_trace.append({
|
||||||
|
"iban": acc.get("number"),
|
||||||
|
"currency": cur_for_log,
|
||||||
|
"placeholder_was_available": ph_available_before,
|
||||||
|
"action": "error",
|
||||||
|
"error": str(exc)[:300],
|
||||||
|
})
|
||||||
frappe.log_error(
|
frappe.log_error(
|
||||||
f"{acc.get('number')}: {exc}\n{traceback.format_exc()}",
|
f"{acc.get('number')}: {exc}\n{traceback.format_exc()}",
|
||||||
"Jey Wizard materialize (bank native)",
|
"Jey Wizard materialize (bank native)",
|
||||||
|
|
@ -340,8 +357,16 @@ def _materialize_native_banks(company):
|
||||||
# Success breadcrumb in Error Log so we can verify end-to-end run from the same
|
# Success breadcrumb in Error Log so we can verify end-to-end run from the same
|
||||||
# place where loader/cache traces live.
|
# place where loader/cache traces live.
|
||||||
frappe.log_error(
|
frappe.log_error(
|
||||||
f"company={company} materialised={created} skipped_closed={skipped_closed} "
|
json.dumps({
|
||||||
f"input_rows={len(accounts)} parent={parent}",
|
"company": company,
|
||||||
|
"materialised": created,
|
||||||
|
"skipped_closed": skipped_closed,
|
||||||
|
"input_rows": len(accounts),
|
||||||
|
"parent": parent,
|
||||||
|
"placeholder_index_keys": sorted(list(placeholder_index.keys())),
|
||||||
|
"placeholder_index_counts": {cur: len(items) for cur, items in placeholder_index.items()},
|
||||||
|
"banks": bank_trace,
|
||||||
|
}, ensure_ascii=False, indent=2),
|
||||||
"Jey Wizard trace: bank native done",
|
"Jey Wizard trace: bank native done",
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
@ -711,33 +736,32 @@ def _claim_placeholder(placeholder_index, currency):
|
||||||
return None
|
return None
|
||||||
|
|
||||||
|
|
||||||
|
def _peek_placeholder(placeholder_index, currency):
|
||||||
|
"""Like _claim_placeholder but doesn't mutate. Used by trace logging."""
|
||||||
|
if not placeholder_index:
|
||||||
|
return None
|
||||||
|
for ph in placeholder_index.get(currency, []):
|
||||||
|
if not ph["consumed"]:
|
||||||
|
return ph
|
||||||
|
return None
|
||||||
|
|
||||||
|
|
||||||
def _convert_placeholder_to_real(old_name, account_number, company, new_account_name, currency):
|
def _convert_placeholder_to_real(old_name, account_number, company, new_account_name, currency):
|
||||||
"""Repurpose an existing placeholder Account in place: update its
|
"""Repurpose an existing placeholder Account in place. Delegates the rename
|
||||||
account_name and account_currency, then rename to the canonical
|
to ERPNext's canonical update_account_number helper — that routine handles
|
||||||
"<account_number> - <account_name> - <abbr>" form. frappe.rename_doc
|
the tree-doctype rename, child-company sync, and final frappe.rename_doc
|
||||||
updates Link references in other docs (notably Company.default_bank_account
|
with force=True. Currency is updated separately via direct db write so we
|
||||||
and any GL entries) to the new name automatically.
|
don't go through Account.validate (which on a doc.save would re-fire all
|
||||||
|
hooks and historically choked on the AZ-CoA placeholder shape).
|
||||||
"""
|
"""
|
||||||
abbr = frappe.db.get_value("Company", company, "abbr") or ""
|
if currency:
|
||||||
doc = frappe.get_doc("Account", old_name)
|
frappe.db.set_value(
|
||||||
doc.account_name = new_account_name
|
"Account", old_name, "account_currency", currency, update_modified=False
|
||||||
doc.account_currency = currency
|
|
||||||
doc.flags.ignore_permissions = True
|
|
||||||
doc.flags.ignore_validate_update_after_submit = True
|
|
||||||
doc.save(ignore_permissions=True)
|
|
||||||
|
|
||||||
if account_number:
|
|
||||||
new_name = f"{account_number} - {new_account_name} - {abbr}"
|
|
||||||
else:
|
|
||||||
new_name = f"{new_account_name} - {abbr}"
|
|
||||||
|
|
||||||
if old_name != new_name and not frappe.db.exists("Account", new_name):
|
|
||||||
frappe.rename_doc(
|
|
||||||
"Account", old_name, new_name,
|
|
||||||
force=True, ignore_permissions=True, show_alert=False,
|
|
||||||
)
|
)
|
||||||
return new_name
|
from erpnext.accounts.doctype.account.account import update_account_number
|
||||||
return old_name
|
|
||||||
|
new_name = update_account_number(old_name, new_account_name, account_number)
|
||||||
|
return new_name or old_name
|
||||||
|
|
||||||
|
|
||||||
def _find_default_warehouse(company):
|
def _find_default_warehouse(company):
|
||||||
|
|
|
||||||
|
|
@ -10,7 +10,7 @@ frappe.provide("jey_wizard");
|
||||||
// Bump this string in every commit that changes wizard code. Displayed in the badge so
|
// Bump this string in every commit that changes wizard code. Displayed in the badge so
|
||||||
// we can tell at a glance which version is actually running on a given machine. Kept in
|
// we can tell at a glance which version is actually running on a given machine. Kept in
|
||||||
// sync with __version__ in jey_wizard/__init__.py.
|
// sync with __version__ in jey_wizard/__init__.py.
|
||||||
const JEY_WIZARD_VERSION = "0.1.9";
|
const JEY_WIZARD_VERSION = "0.1.10";
|
||||||
|
|
||||||
// Wipe Frappe + ERPNext default slides so their `before_load`/`after_load` listeners
|
// Wipe Frappe + ERPNext default slides so their `before_load`/`after_load` listeners
|
||||||
// don't try to mutate a wizard that isn't slide-based anymore.
|
// don't try to mutate a wizard that isn't slide-based anymore.
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue