From 6d9d8e3e1703025acdf4a15b2b9140a9bbe54297 Mon Sep 17 00:00:00 2001 From: av-dev2 Date: Wed, 23 Sep 2026 23:37:55 +0300 Subject: [PATCH 1/7] feat: ask whether port expenses go to work in progress An Expense Accounting section on the Expenses tab: a checkbox, and the WIP and COGS accounts, which are only shown and only mandatory once it is ticked. The COGS account is held for releasing the expense when the container is invoiced, which is not built yet, so ticking this accumulates a WIP balance nothing clears. (cherry picked from commit 5f50a2264ae4a47cead76519f6768a599337b28d) --- .../icd_tz_settings/icd_tz_settings.json | 40 ++++++++++++++++++- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/icd_tz/icd_tz/doctype/icd_tz_settings/icd_tz_settings.json b/icd_tz/icd_tz/doctype/icd_tz_settings/icd_tz_settings.json index f9d51809..e10a5c7b 100644 --- a/icd_tz/icd_tz/doctype/icd_tz_settings/icd_tz_settings.json +++ b/icd_tz/icd_tz/doctype/icd_tz_settings/icd_tz_settings.json @@ -7,6 +7,7 @@ "editable_grid": 1, "engine": "InnoDB", "field_order": [ + "section_break_qvvx", "icd_code", "default_price_list", "column_break_8yjq8", @@ -28,6 +29,10 @@ "expenses_tab", "expense_defaults_section", "default_buying_price_list", + "wip_account", + "column_break_wip", + "enable_wip_for_expenses", + "cogs_account", "expense_pricing_criteria_section", "expense_types", "port_storage_section", @@ -181,6 +186,33 @@ "fieldtype": "Tab Break", "label": "Expenses" }, + { + "default": "0", + "description": "Book port expenses to a work in progress account instead of letting ERPNext choose the expense account. The COGS Account is held for releasing them when the container is invoiced.", + "fieldname": "enable_wip_for_expenses", + "fieldtype": "Check", + "label": "Enable WIP for Port Expenses" + }, + { + "depends_on": "enable_wip_for_expenses", + "fieldname": "wip_account", + "fieldtype": "Link", + "label": "WIP Account", + "mandatory_depends_on": "enable_wip_for_expenses", + "options": "Account" + }, + { + "fieldname": "column_break_wip", + "fieldtype": "Column Break" + }, + { + "depends_on": "enable_wip_for_expenses", + "fieldname": "cogs_account", + "fieldtype": "Link", + "label": "COGS Account", + "mandatory_depends_on": "enable_wip_for_expenses", + "options": "Account" + }, { "fieldname": "expense_defaults_section", "fieldtype": "Section Break", @@ -220,12 +252,16 @@ "fieldname": "port_storage_example", "fieldtype": "HTML", "options": "

Example

\n\n\n\n \n \n \n \n \n \n \n \n \n \n \n \n \n \n \n \n
ChargeFromTo
Single17
Double89999999
\n" + }, + { + "fieldname": "section_break_qvvx", + "fieldtype": "Section Break" } ], "index_web_pages_for_search": 1, "issingle": 1, "links": [], - "modified": "2026-09-22 12:00:00.000000", + "modified": "2026-09-23 23:35:21.302602", "modified_by": "Administrator", "module": "Icd Tz", "name": "ICD TZ Settings", @@ -247,4 +283,4 @@ "sort_order": "DESC", "states": [], "track_changes": 1 -} +} \ No newline at end of file From 00f9d1522044aa6e5069fedbaebb65198a93ba3f Mon Sep 17 00:00:00 2001 From: av-dev2 Date: Wed, 23 Sep 2026 23:37:56 +0300 Subject: [PATCH 2/7] fix: refuse a pair of WIP accounts that cannot be posted to mandatory_depends_on and the form filters reach the form only, not an import, a patch or the API. A group account, or an account of another company, fails far later at posting time, and two identical accounts would release an expense by posting it to itself. (cherry picked from commit ae9a0431d7dc935167623f5095be73755e36ec2f) --- .../icd_tz_settings/icd_tz_settings.py | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/icd_tz/icd_tz/doctype/icd_tz_settings/icd_tz_settings.py b/icd_tz/icd_tz/doctype/icd_tz_settings/icd_tz_settings.py index b86a53a6..1f80db37 100644 --- a/icd_tz/icd_tz/doctype/icd_tz_settings/icd_tz_settings.py +++ b/icd_tz/icd_tz/doctype/icd_tz_settings/icd_tz_settings.py @@ -20,6 +20,58 @@ def before_save(self): self.validate_storage_days() self.validate_expense_types() self.validate_port_storage_days() + self.validate_wip_accounts() + + def validate_wip_accounts(self): + """What the two accounts must be before expenses can be booked to work in progress + + The form makes them mandatory and filters out group accounts, but neither + reaches an import, a patch or the API, and a group account fails only later at + posting time. + """ + + if not self.enable_wip_for_expenses: + return + + missing = [ + self.meta.get_label(field) for field in ("wip_account", "cogs_account") if not self.get(field) + ] + if missing: + frappe.throw( + _("{0} is required while port expenses are booked to work in progress").format( + frappe.bold(" and ".join(missing)) + ), + title=_("WIP Accounts Not Set"), + ) + + if self.wip_account == self.cogs_account: + frappe.throw( + _("WIP Account and COGS Account must differ, or releasing an expense would post nothing"), + title=_("WIP Accounts Not Set"), + ) + + companies = set() + for field in ("wip_account", "cogs_account"): + account = frappe.get_cached_value( + "Account", self.get(field), ["is_group", "company"], as_dict=True + ) + if account.is_group: + frappe.throw( + _("{0} is a group account, which cannot be posted to").format( + frappe.bold(self.get(field)) + ), + title=_("WIP Accounts Not Set"), + ) + + companies.add(account.company) + + if len(companies) > 1: + frappe.throw( + _("WIP Account and COGS Account belong to different companies: {0}").format( + frappe.bold(", ".join(sorted(companies))) + ), + title=_("WIP Accounts Not Set"), + ) def validate_storage_days(self): storage_days = [] From 210613a0724e263f6629807d8ef827cfc9535e1b Mon Sep 17 00:00:00 2001 From: av-dev2 Date: Wed, 23 Sep 2026 23:38:03 +0300 Subject: [PATCH 3/7] feat: offer only accounts the WIP fields can post to Both account fields list the postable accounts of the user company, so a group account or another company's account is not offered in the first place. (cherry picked from commit 2d0cb25cecb251c1194ece2d0470549ee027b286) --- .../icd_tz/doctype/icd_tz_settings/icd_tz_settings.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/icd_tz/icd_tz/doctype/icd_tz_settings/icd_tz_settings.js b/icd_tz/icd_tz/doctype/icd_tz_settings/icd_tz_settings.js index 8af71205..e97c55fd 100644 --- a/icd_tz/icd_tz/doctype/icd_tz_settings/icd_tz_settings.js +++ b/icd_tz/icd_tz/doctype/icd_tz_settings/icd_tz_settings.js @@ -25,6 +25,17 @@ frappe.ui.form.on("ICD TZ Settings", { }; }); + for (const field of ["wip_account", "cogs_account"]) { + frm.set_query(field, () => { + return { + filters: { + is_group: 0, + company: frappe.defaults.get_user_default("Company"), + }, + }; + }); + } + frm.set_query("expense_item", "expense_types", () => { return { filters: { From bf7d98cff0ebe66187403b1dd0782010eced5e96 Mon Sep 17 00:00:00 2001 From: av-dev2 Date: Wed, 23 Sep 2026 23:38:18 +0300 Subject: [PATCH 4/7] feat: book a port expense order to work in progress Each order line carries the WIP account while the ICD books expenses there. It is left unset otherwise, so ERPNext keeps choosing the account from the item and company defaults exactly as before. An account belongs to one company and the settings hold one account, so another company is left alone rather than made unsavable. Where the ICD is deliberately creating a port expense order, a configured account that cannot serve the company is said out loud instead, since silence would send the expense back to the default and it would never be released. (cherry picked from commit 1caf6ecb155892bfb77fe9804fb041d51a70428d) --- icd_tz/icd_tz/api/purchase_order.py | 49 +++++++++++++++++++++++++++-- 1 file changed, 47 insertions(+), 2 deletions(-) diff --git a/icd_tz/icd_tz/api/purchase_order.py b/icd_tz/icd_tz/api/purchase_order.py index 0affe840..0fdf664f 100644 --- a/icd_tz/icd_tz/api/purchase_order.py +++ b/icd_tz/icd_tz/api/purchase_order.py @@ -87,6 +87,49 @@ def validate_no_draft_purchase_order(manifest: str, rows: list): ) +def get_wip_account(company: str) -> str | None: + """Work in progress account port expenses of this company are booked to, or None + + None means leave the expense account alone, so ERPNext keeps choosing it from the + item and company defaults as it always has. An account belongs to one company, and + the settings hold one account, so another company's expenses are left alone too + rather than made unsavable. + """ + + settings_doc = frappe.get_cached_doc("ICD TZ Settings") + if not settings_doc.enable_wip_for_expenses or not settings_doc.wip_account: + return None + + if frappe.get_cached_value("Account", settings_doc.wip_account, "company") != company: + return None + + return settings_doc.wip_account + + +def get_required_wip_account(company: str) -> str | None: + """The WIP account for a port expense order, or None while the ICD does not use one + + Where the ICD is deliberately creating a port expense order, a configured account + that cannot serve the company is said out loud rather than passed over, which is + what would otherwise send the expense quietly back to the ERPNext default. + """ + + settings_doc = frappe.get_cached_doc("ICD TZ Settings") + if not settings_doc.enable_wip_for_expenses: + return None + + wip_account = get_wip_account(company) + if not wip_account: + frappe.throw( + _("Port expenses are booked to work in progress, but {0} cannot be used for {1}").format( + frappe.bold(settings_doc.wip_account or _("no WIP Account")), frappe.bold(company) + ), + title=_("WIP Account Not Usable"), + ) + + return wip_account + + def build_purchase_order(manifest: str, buying_price_list: str, supplier: str, rows: list): """Draft order carrying one line per container, so cost lands on the right dimension""" @@ -94,6 +137,7 @@ def build_purchase_order(manifest: str, buying_price_list: str, supplier: str, r frappe.throw(_("Select a Supplier before creating the Purchase Order")) header = get_manifest_header(manifest) + wip_account = get_required_wip_account(header.company) purchase_order = frappe.new_doc("Purchase Order") purchase_order.update( @@ -110,7 +154,7 @@ def build_purchase_order(manifest: str, buying_price_list: str, supplier: str, r for row in rows: for container in row["containers"]: - add_container_line(purchase_order, manifest, row, container) + add_container_line(purchase_order, manifest, row, container, wip_account) purchase_order.flags.ignore_permissions = True purchase_order.insert() @@ -118,12 +162,13 @@ def build_purchase_order(manifest: str, buying_price_list: str, supplier: str, r return purchase_order -def add_container_line(purchase_order, manifest: str, row: dict, container: dict): +def add_container_line(purchase_order, manifest: str, row: dict, container: dict, wip_account: str | None): """One order line for one container, stamped with its accounting dimensions""" purchase_order.append( "items", { + "expense_account": wip_account, "item_code": row["item_code"], "qty": container["qty"], "rate": row["rate"], From 6a8aa0611d1cebdb56cb1c7414647c3e1833b8c6 Mon Sep 17 00:00:00 2001 From: av-dev2 Date: Wed, 23 Sep 2026 23:38:25 +0300 Subject: [PATCH 5/7] feat: hold a port expense invoice line on work in progress The mapper copies the account from the order the invoice was made from, but an invoice raised on its own has none and any invoice can be edited afterwards. A line that slipped onto another account would never be released to cost of goods sold. Only a configured port expense item on a container line is held. A container can be tagged on any purchase, and the rest of the invoice is ERPNext's to account for, a stock item under perpetual inventory above all. It runs before validate so ERPNext still has its say on every other line. (cherry picked from commit 4b36991178b0315b492aaf45bccdbcdd3920fc8e) --- icd_tz/icd_tz/api/purchase_invoice.py | 30 ++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/icd_tz/icd_tz/api/purchase_invoice.py b/icd_tz/icd_tz/api/purchase_invoice.py index 7c7d7078..b59b70d9 100644 --- a/icd_tz/icd_tz/api/purchase_invoice.py +++ b/icd_tz/icd_tz/api/purchase_invoice.py @@ -1,6 +1,34 @@ import frappe -from icd_tz.icd_tz.api.purchase_order import get_expense_coverage, set_rows +from icd_tz.icd_tz.api.purchase_order import ( + get_expense_coverage, + get_expense_items_by_type, + get_wip_account, + set_rows, +) + + +def set_wip_account(doc, method=None): + """Hold port expense lines on the work in progress account + + The mapper copies the account from the order the invoice was made from, but an + invoice raised on its own has none, and an invoice can be edited after it is made. + The expense is released to cost of goods sold when the container is invoiced, so a + line that slipped onto another account would never be released. + + Only a configured port expense item on a container line is held: a container can be + tagged on any purchase, and the rest of the invoice is ERPNext's to account for. + Runs before validate so ERPNext still has its say on every other line. + """ + + wip_account = get_wip_account(doc.company) + if not wip_account: + return + + expense_items = get_expense_items_by_type() + for item in doc.items: + if item.get("icd_container") and item.item_code in expense_items: + item.expense_account = wip_account def on_submit(doc, method): From b824b80d6aa0f5d1ee5a8c84a42d15ddc23965d1 Mon Sep 17 00:00:00 2001 From: av-dev2 Date: Wed, 23 Sep 2026 23:38:31 +0300 Subject: [PATCH 6/7] chore: run the WIP account rule before a purchase invoice validates Before validate rather than after, so ERPNext still assigns the accounts it owns and the against expense account it derives names what the invoice posts to. (cherry picked from commit 484fc9d0e76fdfda10c01cca844d8a1eed17235b) --- icd_tz/hooks.py | 1 + 1 file changed, 1 insertion(+) diff --git a/icd_tz/hooks.py b/icd_tz/hooks.py index 9d471445..3e96d1e2 100644 --- a/icd_tz/hooks.py +++ b/icd_tz/hooks.py @@ -155,6 +155,7 @@ "on_cancel": "icd_tz.icd_tz.api.purchase_order.on_cancel", }, "Purchase Invoice": { + "before_validate": "icd_tz.icd_tz.api.purchase_invoice.set_wip_account", "on_submit": "icd_tz.icd_tz.api.purchase_invoice.on_submit", "on_cancel": "icd_tz.icd_tz.api.purchase_invoice.on_cancel", }, From f6dbeb8e1cfad3c7bde36e1fb33a764955d39df1 Mon Sep 17 00:00:00 2001 From: av-dev2 Date: Wed, 23 Sep 2026 23:38:53 +0300 Subject: [PATCH 7/7] test: cover booking a port expense to work in progress Covers the account chosen and left alone by the setting, an order refused when the configured account cannot serve its company while an incidental document is left alone, the settings refusing a missing, group, repeated or cross company account, and an invoice holding only a configured expense line on a container. (cherry picked from commit 38d662bd939fdc14a6a7b2d47adce810eac42304) --- icd_tz/tests/test_expense_wip_account.py | 228 +++++++++++++++++++++++ 1 file changed, 228 insertions(+) create mode 100644 icd_tz/tests/test_expense_wip_account.py diff --git a/icd_tz/tests/test_expense_wip_account.py b/icd_tz/tests/test_expense_wip_account.py new file mode 100644 index 00000000..d434eed1 --- /dev/null +++ b/icd_tz/tests/test_expense_wip_account.py @@ -0,0 +1,228 @@ +# Copyright (c) 2026, elius mgani and Contributors +# See license.txt + +from types import SimpleNamespace + +import frappe +from frappe.tests.utils import FrappeTestCase + +from icd_tz.icd_tz.api.purchase_invoice import set_wip_account +from icd_tz.icd_tz.api.purchase_order import ( + add_container_line, + get_required_wip_account, + get_wip_account, +) + +test_ignore = ["Company", "Cost Center"] + +EXPENSE_ITEM = "_Test WIP Expense Item" + + +def get_accounts(company): + return frappe.get_all( + "Account", {"company": company, "is_group": 0, "root_type": "Expense"}, pluck="name", limit=2 + ) + + +def make_expense_item(): + if not frappe.db.exists("Item", EXPENSE_ITEM): + frappe.get_doc( + { + "doctype": "Item", + "item_code": EXPENSE_ITEM, + "item_name": EXPENSE_ITEM, + "item_group": frappe.db.get_value("Item Group", {"is_group": 0}, "name"), + "is_stock_item": 0, + } + ).insert(ignore_permissions=True) + + return EXPENSE_ITEM + + +class WipTestCase(FrappeTestCase): + def setUp(self): + self.company = frappe.db.get_value("Company", {}, "name") + accounts = get_accounts(self.company) + if len(accounts) < 2: + self.skipTest(f"{self.company} has fewer than two postable expense accounts") + + self.wip, self.cogs = accounts + self.other_company = frappe.db.get_value("Company", {"name": ("!=", self.company)}, "name") + settings_doc = frappe.get_single("ICD TZ Settings") + if settings_doc.expense_types: + self.item = settings_doc.expense_types[0].expense_item + else: + self.item = make_expense_item() + settings_doc.append("expense_types", {"expense_type": "Shore", "expense_item": self.item}) + settings_doc.flags.ignore_mandatory = True + settings_doc.save(ignore_permissions=True) + + def tearDown(self): + frappe.clear_document_cache("ICD TZ Settings", "ICD TZ Settings") + + def enable_wip(self, wip_account=None): + frappe.db.set_single_value( + "ICD TZ Settings", + { + "enable_wip_for_expenses": 1, + "wip_account": wip_account or self.wip, + "cogs_account": self.cogs, + }, + ) + + def disable_wip(self): + frappe.db.set_single_value("ICD TZ Settings", "enable_wip_for_expenses", 0) + + +class TestWipAccountSetting(WipTestCase): + """Which account port expenses are booked to""" + + def test_no_account_is_chosen_while_the_setting_is_off(self): + # ERPNext keeps choosing the expense account from the item and company defaults + self.disable_wip() + + self.assertIsNone(get_wip_account(self.company)) + get_required_wip_account(self.company) + + def test_the_wip_account_is_used_while_the_setting_is_on(self): + self.enable_wip() + + self.assertEqual(get_wip_account(self.company), self.wip) + + def test_another_company_is_left_alone_rather_than_blocked(self): + # one settings record holds one account, and an account belongs to one company + if not self.other_company: + self.skipTest("this test needs a second company") + + self.enable_wip() + + self.assertIsNone(get_wip_account(self.other_company)) + + def test_an_order_for_a_company_the_account_cannot_serve_is_refused(self): + if not self.other_company: + self.skipTest("this test needs a second company") + + self.enable_wip() + + self.assertRaises(frappe.ValidationError, get_required_wip_account, self.other_company) + + def test_an_order_is_refused_while_the_account_is_missing(self): + frappe.db.set_single_value("ICD TZ Settings", {"enable_wip_for_expenses": 1, "wip_account": None}) + + self.assertRaises(frappe.ValidationError, get_required_wip_account, self.company) + + def test_turning_it_on_without_the_accounts_is_refused(self): + settings_doc = frappe.get_single("ICD TZ Settings") + settings_doc.update({"enable_wip_for_expenses": 1, "wip_account": None, "cogs_account": None}) + + self.assertRaises(frappe.ValidationError, settings_doc.validate_wip_accounts) + + def test_the_two_accounts_must_differ(self): + settings_doc = frappe.get_single("ICD TZ Settings") + settings_doc.update({"enable_wip_for_expenses": 1, "wip_account": self.wip, "cogs_account": self.wip}) + + self.assertRaises(frappe.ValidationError, settings_doc.validate_wip_accounts) + + def test_two_accounts_of_different_companies_are_refused(self): + if not self.other_company: + self.skipTest("this test needs a second company") + + other_accounts = get_accounts(self.other_company) + if not other_accounts: + self.skipTest(f"{self.other_company} has no postable expense account") + + settings_doc = frappe.get_single("ICD TZ Settings") + settings_doc.update( + { + "enable_wip_for_expenses": 1, + "wip_account": self.wip, + "cogs_account": other_accounts[0], + } + ) + + self.assertRaises(frappe.ValidationError, settings_doc.validate_wip_accounts) + + def test_a_group_account_is_refused(self): + group = frappe.db.get_value("Account", {"company": self.company, "is_group": 1}, "name") + settings_doc = frappe.get_single("ICD TZ Settings") + settings_doc.update({"enable_wip_for_expenses": 1, "wip_account": group, "cogs_account": self.cogs}) + + self.assertRaises(frappe.ValidationError, settings_doc.validate_wip_accounts) + + +class TestOrderAndInvoiceLines(WipTestCase): + """Holding the expense on WIP from the order through to the invoice""" + + def make_order_line(self, wip_account): + purchase_order = frappe.new_doc("Purchase Order") + purchase_order.schedule_date = frappe.utils.nowdate() + row = { + "item_code": self.item, + "rate": 100, + "expense_type": "Shore", + "size": "20ft", + "cargo_type": None, + "destination": None, + "port": None, + } + container = { + "qty": 1, + "container_no": "TEST1234567", + "day_rows": [], + "icd_master_bl": "MBL-1", + "icd_container": "CNT-1", + } + add_container_line(purchase_order, "ICD-M-0001", row, container, wip_account) + + return purchase_order.items[0] + + def make_invoice(self, item_code, icd_container="CNT-1", expense_account="SOMETHING ELSE"): + # frappe._dict cannot carry an "items" attribute, it shadows the dict method + return SimpleNamespace( + company=self.company, + items=[ + frappe._dict( + item_code=item_code, icd_container=icd_container, expense_account=expense_account + ) + ], + ) + + def test_an_order_line_carries_the_wip_account(self): + self.assertEqual(self.make_order_line(self.wip).expense_account, self.wip) + + def test_an_order_line_carries_no_account_while_the_setting_is_off(self): + self.assertIsNone(self.make_order_line(None).expense_account) + + def test_an_invoice_line_for_a_port_expense_is_held_on_wip(self): + self.enable_wip() + doc = self.make_invoice(self.item) + + set_wip_account(doc) + + self.assertEqual(doc.items[0].expense_account, self.wip) + + def test_a_line_that_is_not_a_configured_expense_is_left_alone(self): + # a container can be tagged on any purchase, and the rest of that invoice is + # ERPNext's to account for, stock items above all + self.enable_wip() + doc = self.make_invoice("_Test Item") + + set_wip_account(doc) + + self.assertEqual(doc.items[0].expense_account, "SOMETHING ELSE") + + def test_a_line_with_no_container_is_left_alone(self): + self.enable_wip() + doc = self.make_invoice(self.item, icd_container=None) + + set_wip_account(doc) + + self.assertEqual(doc.items[0].expense_account, "SOMETHING ELSE") + + def test_an_invoice_is_left_alone_while_the_setting_is_off(self): + self.disable_wip() + doc = self.make_invoice(self.item) + + set_wip_account(doc) + + self.assertEqual(doc.items[0].expense_account, "SOMETHING ELSE")