diff --git a/erpnext/accounts/doctype/account/chart_of_accounts/verified/philippines.json b/erpnext/accounts/doctype/account/chart_of_accounts/verified/philippines.json index ea3977711a7..1cad6a97d0c 100644 --- a/erpnext/accounts/doctype/account/chart_of_accounts/verified/philippines.json +++ b/erpnext/accounts/doctype/account/chart_of_accounts/verified/philippines.json @@ -406,8 +406,7 @@ "Customer Deposits": { "account_number": "2500", "is_group": 0, - "root_type": "Liability", - "account_type": "Payable" + "root_type": "Liability" } }, "Non Current Liabilities": { diff --git a/erpnext/accounts/doctype/bank_account/bank_account.py b/erpnext/accounts/doctype/bank_account/bank_account.py index aced4258526..30dbb013be8 100644 --- a/erpnext/accounts/doctype/bank_account/bank_account.py +++ b/erpnext/accounts/doctype/bank_account/bank_account.py @@ -115,7 +115,7 @@ def get_party_bank_account(party_type, party): ) -def get_default_company_bank_account(company, party_type, party): +def get_default_company_bank_account(company, party_type, party, ignore_permissions=True): default_company_bank_account = frappe.db.get_value(party_type, party, "default_bank_account") if default_company_bank_account: if company != frappe.get_cached_value("Bank Account", default_company_bank_account, "company"): @@ -126,6 +126,14 @@ def get_default_company_bank_account(company, party_type, party): "Bank Account", {"company": company, "is_company_account": 1, "is_default": 1} ) + if not ignore_permissions: + default_company_bank_account = ( + default_company_bank_account + if default_company_bank_account + and frappe.get_cached_doc("Bank Account", default_company_bank_account).has_permission("select") + else None + ) + return default_company_bank_account diff --git a/erpnext/accounts/doctype/bank_transaction/test_bank_transaction.py b/erpnext/accounts/doctype/bank_transaction/test_bank_transaction.py index 4294c4462b1..05a9c055078 100644 --- a/erpnext/accounts/doctype/bank_transaction/test_bank_transaction.py +++ b/erpnext/accounts/doctype/bank_transaction/test_bank_transaction.py @@ -115,6 +115,36 @@ class TestBankTransaction(FrappeTestCase): self.assertEqual(bank_transaction.unallocated_amount, 1700) self.assertEqual(bank_transaction.payment_entries, []) + # Amending a reconciled payment entry must not carry over its clearance date + def test_clearance_date_cleared_on_amend(self): + bank_transaction = frappe.get_doc( + "Bank Transaction", + dict(description="1512567 BG/000003025 OPSKATTUZWXXX AT776000000098709849 Herr G"), + ) + payment = frappe.get_doc("Payment Entry", dict(party="Mr G", paid_amount=1700)) + vouchers = json.dumps( + [ + { + "payment_doctype": "Payment Entry", + "payment_name": payment.name, + "amount": bank_transaction.unallocated_amount, + } + ] + ) + reconcile_vouchers(bank_transaction.name, vouchers) + + self.assertTrue(frappe.db.get_value("Payment Entry", payment.name, "clearance_date")) + + payment.reload() + payment.cancel() + + amended = frappe.copy_doc(payment) + amended.amended_from = payment.name + amended.docstatus = 0 + amended.insert() + + self.assertFalse(amended.clearance_date) + # Check if ERPNext can correctly filter a linked payments based on the debit/credit amount def test_debit_credit_output(self): bank_transaction = frappe.get_doc( diff --git a/erpnext/accounts/doctype/exchange_rate_revaluation/exchange_rate_revaluation.js b/erpnext/accounts/doctype/exchange_rate_revaluation/exchange_rate_revaluation.js index eeda531c4d6..a2e5e54a776 100644 --- a/erpnext/accounts/doctype/exchange_rate_revaluation/exchange_rate_revaluation.js +++ b/erpnext/accounts/doctype/exchange_rate_revaluation/exchange_rate_revaluation.js @@ -22,17 +22,27 @@ frappe.ui.form.on("Exchange Rate Revaluation", { refresh: function (frm) { if (frm.doc.docstatus == 1) { frappe.call({ - method: "check_journal_entry_condition", + method: "check_journal_and_reversal", doc: frm.doc, callback: function (r) { if (r.message) { - frm.add_custom_button( - __("Journal Entries"), - function () { - return frm.events.make_jv(frm); - }, - __("Create") - ); + if (!r.message.journals_posted) { + frm.add_custom_button( + __("Journal Entries"), + function () { + return frm.events.make_jv(frm); + }, + __("Create") + ); + } else if (!r.message.reversals_posted) { + frm.add_custom_button( + __("Reversal Journal Entries"), + function () { + return frm.events.make_reverse_journal(frm); + }, + __("Create") + ); + } } }, }); @@ -100,6 +110,14 @@ frappe.ui.form.on("Exchange Rate Revaluation", { }, }); }, + make_reverse_journal: function (frm) { + frappe.call({ + method: "make_reverse_journal", + doc: frm.doc, + freeze: true, + freeze_message: __("Reversing Journals..."), + }); + }, }); frappe.ui.form.on("Exchange Rate Revaluation Account", { diff --git a/erpnext/accounts/doctype/exchange_rate_revaluation/exchange_rate_revaluation.py b/erpnext/accounts/doctype/exchange_rate_revaluation/exchange_rate_revaluation.py index 41249662624..b87e8e00951 100644 --- a/erpnext/accounts/doctype/exchange_rate_revaluation/exchange_rate_revaluation.py +++ b/erpnext/accounts/doctype/exchange_rate_revaluation/exchange_rate_revaluation.py @@ -8,7 +8,7 @@ from frappe.model.document import Document from frappe.model.meta import get_field_precision from frappe.query_builder import Criterion, Order from frappe.query_builder.functions import NullIf, Sum -from frappe.utils import flt, get_link_to_form +from frappe.utils import flt, get_link_to_form, nowdate import erpnext from erpnext.accounts.doctype.journal_entry.journal_entry import get_balance_on @@ -90,25 +90,31 @@ class ExchangeRateRevaluation(Document): ) def on_cancel(self): - self.ignore_linked_doctypes = "GL Entry" + self.ignore_linked_doctypes = ["GL Entry", "Payment Ledger Entry"] @frappe.whitelist() - def check_journal_entry_condition(self): + def check_journal_and_reversal(self): exchange_gain_loss_account = self.get_for_unrealized_gain_loss_account() + journals_posted = False + reversals_posted = False + + je = qb.DocType("Journal Entry") jea = qb.DocType("Journal Entry Account") journals = ( - qb.from_(jea) - .select(jea.parent) + qb.from_(je) + .join(jea) + .on(je.name == jea.parent) + .select(je.name) .distinct() .where( (jea.reference_type == "Exchange Rate Revaluation") & (jea.reference_name == self.name) & (jea.docstatus == 1) + & (je.reversal_of.isnull()) # omit journals that have reversals ) - .run() + .run(pluck="name") ) - if journals: gle = qb.DocType("GL Entry") total_amt = ( @@ -123,12 +129,31 @@ class ExchangeRateRevaluation(Document): .run() ) - if total_amt and total_amt[0][0] != self.total_gain_loss: - return True + if total_amt and total_amt[0][0] == self.total_gain_loss: + journals_posted = True else: - return False + journals_posted = False - return True + # reverse journals + reverse_journals = ( + qb.from_(je) + .join(jea) + .on(je.name == jea.parent) + .select(je.name) + .where( + (jea.reference_type == "Exchange Rate Revaluation") + & (jea.reference_name == self.name) + & (jea.docstatus == 1) + & (je.reversal_of.notnull()) + ) + .run(pluck="name") + ) + if reverse_journals: + reversals_posted = True + else: + reversals_posted = False + + return {"journals_posted": journals_posted, "reversals_posted": reversals_posted} def fetch_and_calculate_accounts_data(self): accounts = self.get_accounts_data() @@ -342,6 +367,7 @@ class ExchangeRateRevaluation(Document): @frappe.whitelist() def make_jv_entries(self): + frappe.has_permission("Journal Entry", "write", throw=True) zero_balance_jv = self.make_jv_for_zero_balance() if zero_balance_jv: frappe.msgprint( @@ -571,6 +597,38 @@ class ExchangeRateRevaluation(Document): journal_entry.save() return journal_entry + @frappe.whitelist() + def make_reverse_journal(self): + frappe.has_permission("Journal Entry", "write", throw=True) + je = qb.DocType("Journal Entry") + jea = qb.DocType("Journal Entry Account") + journals = ( + qb.from_(je) + .join(jea) + .on(je.name == jea.parent) + .select(je.name) + .distinct() + .where( + (jea.reference_type == "Exchange Rate Revaluation") + & (jea.reference_name == self.name) + & (jea.docstatus == 1) + & (je.reversal_of.isnull()) # omit journals that have reversals + ) + .run(pluck="name") + ) + if journals: + from erpnext.accounts.doctype.journal_entry.journal_entry import make_reverse_journal_entry + + for x in journals: + reversal = make_reverse_journal_entry(x) + reversal.posting_date = nowdate() + reversal.submit() + frappe.msgprint( + _("Revaluation journal for {0} has been created: {1}").format( + frappe.bold(x), get_link_to_form("Journal Entry", reversal.name) + ) + ) + def calculate_exchange_rate_using_last_gle(company, account, party_type, party): """ diff --git a/erpnext/accounts/doctype/exchange_rate_revaluation/test_exchange_rate_revaluation.py b/erpnext/accounts/doctype/exchange_rate_revaluation/test_exchange_rate_revaluation.py index 3eef6ab3832..4329b6078ec 100644 --- a/erpnext/accounts/doctype/exchange_rate_revaluation/test_exchange_rate_revaluation.py +++ b/erpnext/accounts/doctype/exchange_rate_revaluation/test_exchange_rate_revaluation.py @@ -130,7 +130,8 @@ class TestExchangeRateRevaluation(AccountsTestMixin, FrappeTestCase): err = err.save().submit() # Create JV for ERR - self.assertTrue(err.check_journal_entry_condition()) + ret = err.check_journal_and_reversal() + self.assertFalse(ret.get("journals_posted")) err_journals = err.make_jv_entries() je = frappe.get_doc("Journal Entry", err_journals.get("zero_balance_jv")) je = je.submit() @@ -213,7 +214,8 @@ class TestExchangeRateRevaluation(AccountsTestMixin, FrappeTestCase): err = err.save().submit() # Create JV for ERR - self.assertTrue(err.check_journal_entry_condition()) + ret = err.check_journal_and_reversal() + self.assertFalse(ret.get("journals_posted")) err_journals = err.make_jv_entries() je = frappe.get_doc("Journal Entry", err_journals.get("zero_balance_jv")) je = je.submit() @@ -287,3 +289,83 @@ class TestExchangeRateRevaluation(AccountsTestMixin, FrappeTestCase): for key, _val in expected_data.items(): self.assertEqual(expected_data.get(key), account_details.get(key)) + + @change_settings( + "Accounts Settings", + {"allow_multi_currency_invoices_against_single_party_account": 1, "allow_stale": 0}, + ) + def test_05_revaluation_journal_reversal(self): + """ + Test reversing of revaluation journals + """ + si = create_sales_invoice( + item=self.item, + company=self.company, + customer=self.customer, + debit_to=self.debtors_usd, + posting_date=today(), + parent_cost_center=self.cost_center, + cost_center=self.cost_center, + rate=100, + price_list_rate=100, + do_not_submit=1, + ) + si.currency = "USD" + si.conversion_rate = 80 + si.save().submit() + + err = frappe.new_doc("Exchange Rate Revaluation") + err.company = self.company + err.posting_date = today() + err.fetch_and_calculate_accounts_data() + self.assertEqual(len(err.accounts), 1) + err.save().submit() + + gain_loss_account = err.get_for_unrealized_gain_loss_account() + usd_account = err.accounts[0].account + old_balance = err.accounts[0].balance_in_base_currency + new_balance = err.accounts[0].new_balance_in_base_currency + total_gain_loss = err.total_gain_loss + + # Create JV for ERR + ret = err.check_journal_and_reversal() + self.assertFalse(ret.get("journals_posted")) + err_journals = err.make_jv_entries() + je = frappe.get_doc("Journal Entry", err_journals.get("revaluation_jv")) + je = je.submit() + + je.reload() + self.assertEqual(je.voucher_type, "Exchange Rate Revaluation") + self.assertEqual(len(je.accounts), 3) + expected = [ + (usd_account, new_balance, 0.0, 100.0, 0.0), + (usd_account, 0.0, old_balance, 0.0, 100.0), + (gain_loss_account, 0.0, total_gain_loss, 0.0, total_gain_loss), + ] + actual = [] + for acc in je.accounts: + actual.append( + ( + acc.account, + acc.debit, + acc.credit, + acc.debit_in_account_currency, + acc.credit_in_account_currency, + ) + ) + self.assertEqual(expected, actual) + + # Assert reversals are not posted + ret = err.check_journal_and_reversal() + self.assertTrue(ret.get("journals_posted")) + self.assertFalse(ret.get("reversals_posted")) + + err.make_reverse_journal() + ret = err.check_journal_and_reversal() + self.assertTrue(ret.get("journals_posted")) + self.assertTrue(ret.get("reversals_posted")) + + reverse_jv = frappe.db.get_all( + "Journal Entry", filters={"reversal_of": err_journals.get("revaluation_jv")}, pluck="name" + ) + self.assertIsNotNone(reverse_jv) diff --git a/erpnext/accounts/doctype/journal_entry/journal_entry.js b/erpnext/accounts/doctype/journal_entry/journal_entry.js index ae3ee00e535..232c33d4def 100644 --- a/erpnext/accounts/doctype/journal_entry/journal_entry.js +++ b/erpnext/accounts/doctype/journal_entry/journal_entry.js @@ -40,6 +40,10 @@ frappe.ui.form.on("Journal Entry", { }, refresh: function (frm) { + if (frm.doc.reversal_of && (frm.is_new() || frm.doc.docstatus == 0)) { + frm.set_read_only(); + } + erpnext.toggle_naming_series(); if (frm.doc.docstatus > 0) { diff --git a/erpnext/accounts/doctype/journal_entry/journal_entry.py b/erpnext/accounts/doctype/journal_entry/journal_entry.py index aa048a71ff2..762585601e5 100644 --- a/erpnext/accounts/doctype/journal_entry/journal_entry.py +++ b/erpnext/accounts/doctype/journal_entry/journal_entry.py @@ -21,6 +21,7 @@ from erpnext.accounts.doctype.repost_accounting_ledger.repost_accounting_ledger from erpnext.accounts.doctype.tax_withholding_category.tax_withholding_category import ( get_party_tax_withholding_details, ) +from erpnext.accounts.general_ledger import validate_opening_entry_against_pcv from erpnext.accounts.party import get_party_account from erpnext.accounts.utils import ( cancel_exchange_gain_loss_journal, @@ -123,6 +124,9 @@ class JournalEntry(AccountsController): if not self.is_opening: self.is_opening = "No" + if self.is_opening == "Yes": + validate_opening_entry_against_pcv(self.company) + self.clearance_date = None self.validate_party() diff --git a/erpnext/accounts/doctype/journal_entry/journal_entry_list.js b/erpnext/accounts/doctype/journal_entry/journal_entry_list.js index 9d6e87392e5..45d29d08477 100644 --- a/erpnext/accounts/doctype/journal_entry/journal_entry_list.js +++ b/erpnext/accounts/doctype/journal_entry/journal_entry_list.js @@ -1,12 +1,15 @@ frappe.listview_settings["Journal Entry"] = { - add_fields: ["voucher_type", "posting_date", "total_debit", "company", "user_remark"], + add_fields: ["voucher_type", "posting_date", "total_debit", "company", "user_remark", "reversal_of"], get_indicator: function (doc) { if (doc.docstatus == 0) { return [__("Draft", "red", "docstatus,=,0")]; } else if (doc.docstatus == 2) { return [__("Cancelled", "grey", "docstatus,=,2")]; - } else { - return [__(doc.voucher_type), "blue", "voucher_type,=," + doc.voucher_type]; + } else if (doc.docstatus === 1) { + if (doc.reversal_of && doc.voucher_type == "Exchange Rate Revaluation") { + return [__("Reversal Of Exchange Rate Revaluation"), "blue"]; + } + return [__(doc.voucher_type), "blue", `voucher_type,=,${doc.voucher_type}`]; } }, }; diff --git a/erpnext/accounts/doctype/opening_invoice_creation_tool/opening_invoice_creation_tool.js b/erpnext/accounts/doctype/opening_invoice_creation_tool/opening_invoice_creation_tool.js index 4938e6690e5..eeac8b1ebc3 100644 --- a/erpnext/accounts/doctype/opening_invoice_creation_tool/opening_invoice_creation_tool.js +++ b/erpnext/accounts/doctype/opening_invoice_creation_tool/opening_invoice_creation_tool.js @@ -24,15 +24,22 @@ frappe.ui.form.on("Opening Invoice Creation Tool", { setTimeout( () => { frm.doc.import_in_progress = false; - frm.clear_table("invoices"); - frm.refresh_fields(); frm.page.clear_indicator(); frm.dashboard.hide_progress(); - if (frm.doc.invoice_type == "Sales") { - frappe.msgprint(__("Opening Sales Invoices have been created.")); + if (!data.errors) { + frm.clear_table("invoices"); + frm.refresh_fields(); + const message = + frm.doc.invoice_type == "Sales" + ? __("Opening Sales Invoice(s) have been created.") + : __("Opening Purchase Invoice(s) have been created."); + frappe.show_alert({ + message: message, + indicator: "green", + }); } else { - frappe.msgprint(__("Opening Purchase Invoices have been created.")); + frm.refresh_fields(); } }, 1500, @@ -75,29 +82,31 @@ frappe.ui.form.on("Opening Invoice Creation Tool", { }, setup_company_filters: function (frm) { - frm.set_query("cost_center", "invoices", function (doc, cdt, cdn) { - return { - filters: { - company: doc.company, - }, - }; + frm.events.apply_company_query_filter(frm, "cost_center", "invoices", { is_group: 0 }); + frm.events.apply_company_query_filter(frm, "project", "invoices"); + frm.events.apply_company_query_filter(frm, "project"); + frm.events.apply_company_query_filter(frm, "cost_center", undefined, { is_group: 0 }); + frm.events.apply_company_query_filter(frm, "temporary_opening_account", "invoices", { + account_type: "Temporary", + is_group: 0, }); + }, - frm.set_query("cost_center", function (doc) { + apply_company_query_filter: function (frm, field_name, child_doctype = null, filters = {}) { + const query = function (doc) { return { filters: { company: doc.company, + ...filters, }, }; - }); + }; - frm.set_query("temporary_opening_account", "invoices", function (doc, cdt, cdn) { - return { - filters: { - company: doc.company, - }, - }; - }); + if (child_doctype) { + frm.set_query(field_name, child_doctype, query); + } else { + frm.set_query(field_name, query); + } }, company: function (frm) { @@ -121,10 +130,7 @@ frappe.ui.form.on("Opening Invoice Creation Tool", { }, invoice_type: function (frm) { - $.each(frm.doc.invoices, (idx, row) => { - row.party_type = frm.doc.invoice_type == "Sales" ? "Customer" : "Supplier"; - row.party = ""; - }); + frm.clear_table("invoices"); frm.refresh_fields(); }, @@ -165,7 +171,19 @@ frappe.ui.form.on("Opening Invoice Creation Tool", { }); frappe.ui.form.on("Opening Invoice Creation Tool Item", { - invoices_add: (frm) => { + invoices_add: (frm, cdt, cdn) => { + const row = frappe.get_doc(cdt, cdn); + const field_copy = []; + + ["project", "cost_center"].forEach((fieldname) => { + if (frm.doc[fieldname]) { + frappe.model.set_value(cdt, cdn, fieldname, frm.doc[fieldname]); + } else { + field_copy.push(fieldname); + } + }); + + frm.script_manager.copy_from_first_row("invoices", row, field_copy); frm.trigger("update_invoice_table"); }, }); diff --git a/erpnext/accounts/doctype/opening_invoice_creation_tool/opening_invoice_creation_tool.py b/erpnext/accounts/doctype/opening_invoice_creation_tool/opening_invoice_creation_tool.py index 4003f533ded..48551eab3b2 100644 --- a/erpnext/accounts/doctype/opening_invoice_creation_tool/opening_invoice_creation_tool.py +++ b/erpnext/accounts/doctype/opening_invoice_creation_tool/opening_invoice_creation_tool.py @@ -122,6 +122,17 @@ class OpeningInvoiceCreationTool(Document): if not row.get(scrub(d)): frappe.throw(mandatory_error_msg.format(row.idx, d, self.invoice_type)) + self.validate_temporary_opening_account(row) + + def validate_temporary_opening_account(self, row): + account_type = frappe.get_cached_value("Account", row.temporary_opening_account, "account_type") + if account_type != "Temporary": + frappe.throw( + _("Row #{0}: {1} account is not of type {2}").format( + row.idx, row.temporary_opening_account, "Temporary" + ) + ) + def get_invoices(self): invoices = [] for row in self.invoices: @@ -191,6 +202,7 @@ class OpeningInvoiceCreationTool(Document): "description": row.item_name or "Opening Invoice Item", income_expense_account_field: row.temporary_opening_account, "cost_center": cost_center, + "project": row.get("project") or self.get("project"), } ) @@ -254,22 +266,32 @@ class OpeningInvoiceCreationTool(Document): def start_import(invoices): errors = 0 names = [] + total = len(invoices) for idx, d in enumerate(invoices): + # Scope each invoice to a savepoint so a failure only undoes that invoice. + # A plain rollback() would discard the whole transaction — including invoices + # imported earlier in this batch and the error logs of earlier failures (the + # latter only survive on mariadb because the Error Log table is MyISAM; on + # postgres they would be lost). Rolling back to a savepoint keeps both. + savepoint = f"opening_invoice_{frappe.generate_hash(length=8)}" + frappe.db.savepoint(savepoint) + is_last = idx == total - 1 try: invoice_number = None if d.invoice_number: invoice_number = d.invoice_number - publish(idx, len(invoices), d.doctype) doc = frappe.get_doc(d) doc.flags.ignore_mandatory = True doc.insert(set_name=invoice_number) doc.submit() frappe.db.commit() names.append(doc.name) + publish(idx, total, d.doctype, errors=errors if is_last else None) except Exception: errors += 1 frappe.db.rollback() doc.log_error("Opening invoice creation failed") + publish(idx, total, d.doctype, errors=errors if is_last else None) if errors: frappe.msgprint( _("You had {} errors while creating opening invoices. Check {} for more details").format( @@ -281,7 +303,7 @@ def start_import(invoices): return names -def publish(index, total, doctype): +def publish(index, total, doctype, errors=None): frappe.publish_realtime( "opening_invoice_creation_progress", dict( @@ -289,6 +311,7 @@ def publish(index, total, doctype): message=_("Creating {} out of {} {}").format(index + 1, total, doctype), count=index + 1, total=total, + errors=errors, ), user=frappe.session.user, ) diff --git a/erpnext/accounts/doctype/opening_invoice_creation_tool/test_opening_invoice_creation_tool.py b/erpnext/accounts/doctype/opening_invoice_creation_tool/test_opening_invoice_creation_tool.py index bfab823f495..9bb69cacd33 100644 --- a/erpnext/accounts/doctype/opening_invoice_creation_tool/test_opening_invoice_creation_tool.py +++ b/erpnext/accounts/doctype/opening_invoice_creation_tool/test_opening_invoice_creation_tool.py @@ -3,6 +3,7 @@ import frappe from frappe.tests.utils import FrappeTestCase +from frappe.utils import add_days, today from erpnext.accounts.doctype.accounting_dimension.test_accounting_dimension import ( create_dimension, @@ -11,6 +12,7 @@ from erpnext.accounts.doctype.accounting_dimension.test_accounting_dimension imp from erpnext.accounts.doctype.opening_invoice_creation_tool.opening_invoice_creation_tool import ( get_temporary_opening_account, ) +from erpnext.projects.doctype.project.test_project import make_project test_dependencies = ["Customer", "Supplier", "Accounting Dimension"] @@ -27,21 +29,26 @@ class TestOpeningInvoiceCreationTool(FrappeTestCase): self, invoice_type="Sales", company=None, - party_1=None, - party_2=None, - invoice_number=None, + invoices=None, + project=None, + cost_center=None, department=None, + return_doc=False, ): doc = frappe.get_single("Opening Invoice Creation Tool") args = get_opening_invoice_creation_dict( invoice_type=invoice_type, company=company, - party_1=party_1, - party_2=party_2, - invoice_number=invoice_number, + invoices=invoices, + project=project, + cost_center=cost_center, department=department, ) doc.update(args) + + if return_doc: + return doc + return doc.make_invoices() def test_opening_sales_invoice_creation(self): @@ -50,8 +57,8 @@ class TestOpeningInvoiceCreationTool(FrappeTestCase): self.assertEqual(len(invoices), 2) expected_value = { "keys": ["customer", "outstanding_amount", "status"], - 0: ["_Test Customer", 300, "Overdue"], - 1: ["_Test Customer 1", 250, "Overdue"], + 0: ["_Test Customer", 200, "Overdue"], + 1: ["_Test Customer 1", 200, "Overdue"], } self.check_expected_values(invoices, expected_value) @@ -68,48 +75,34 @@ class TestOpeningInvoiceCreationTool(FrappeTestCase): for field_idx, field in enumerate(expected_value["keys"]): self.assertEqual(si.get(field, ""), expected_value[invoice_idx][field_idx]) + def test_opening_invoice_requires_temporary_account_type(self): + doc = self.make_invoices(company="_Test Opening Invoice Company", return_doc=True) + doc.invoices[0].temporary_opening_account = "Sales - _TOIC" + self.assertRaises(frappe.ValidationError, doc.make_invoices) + def test_opening_purchase_invoice_creation(self): invoices = self.make_invoices(invoice_type="Purchase", company="_Test Opening Invoice Company") self.assertEqual(len(invoices), 2) expected_value = { "keys": ["supplier", "outstanding_amount", "status"], - 0: ["_Test Supplier", 300, "Overdue"], - 1: ["_Test Supplier 1", 250, "Overdue"], + 0: ["_Test Supplier", 200, "Overdue"], + 1: ["_Test Supplier 1", 200, "Overdue"], } self.check_expected_values(invoices, expected_value, "Purchase") def test_opening_sales_invoice_creation_with_missing_debit_account(self): - company = "_Test Opening Invoice Company" party_1, party_2 = make_customer("Customer A"), make_customer("Customer B") - old_default_receivable_account = frappe.db.get_value("Company", company, "default_receivable_account") - frappe.db.set_value("Company", company, "default_receivable_account", "") + old_default_receivable_account = frappe.db.get_value( + "Company", "_Test Opening Invoice Company", "default_receivable_account" + ) + frappe.db.set_value("Company", "_Test Opening Invoice Company", "default_receivable_account", "") - if not frappe.db.exists("Cost Center", "_Test Opening Invoice Company - _TOIC"): - cc = frappe.get_doc( - { - "doctype": "Cost Center", - "cost_center_name": "_Test Opening Invoice Company", - "is_group": 1, - "company": "_Test Opening Invoice Company", - } - ) - cc.insert(ignore_mandatory=True) - cc2 = frappe.get_doc( - { - "doctype": "Cost Center", - "cost_center_name": "Main", - "is_group": 0, - "company": "_Test Opening Invoice Company", - "parent_cost_center": cc.name, - } - ) - cc2.insert() - - frappe.db.set_value("Company", company, "cost_center", "Main - _TOIC") - - self.make_invoices(company="_Test Opening Invoice Company", party_1=party_1, party_2=party_2) + self.make_invoices( + company="_Test Opening Invoice Company", + invoices=[{"party": party_1}, {"party": party_2}], + ) # Check if missing debit account error raised error_log = frappe.db.exists( @@ -119,74 +112,113 @@ class TestOpeningInvoiceCreationTool(FrappeTestCase): self.assertTrue(error_log) # teardown - frappe.db.set_value("Company", company, "default_receivable_account", old_default_receivable_account) - - def test_renaming_of_invoice_using_invoice_number_field(self): - company = "_Test Opening Invoice Company" - party_1, party_2 = make_customer("Customer A"), make_customer("Customer B") - self.make_invoices( - company=company, party_1=party_1, party_2=party_2, invoice_number="TEST-NEW-INV-11" + frappe.db.set_value( + "Company", + "_Test Opening Invoice Company", + "default_receivable_account", + old_default_receivable_account, ) - sales_inv1 = frappe.get_all("Sales Invoice", filters={"customer": "Customer A"})[0].get("name") - sales_inv2 = frappe.get_all("Sales Invoice", filters={"customer": "Customer B"})[0].get("name") - self.assertEqual(sales_inv1, "TEST-NEW-INV-11") + def test_renaming_of_invoice_using_invoice_number_field(self): + party_1, party_2 = make_customer("Customer A"), make_customer("Customer B") + inv_num = f"TEST-NEW-INV-{frappe.generate_hash(length=8)}" + invoices = self.make_invoices( + company="_Test Opening Invoice Company", + invoices=[ + {"party": party_1, "invoice_number": inv_num}, + {"party": party_2}, + ], + ) - # teardown - for inv in [sales_inv1, sales_inv2]: - doc = frappe.get_doc("Sales Invoice", inv) - doc.cancel() + self.assertEqual(invoices[0], inv_num) def test_opening_invoice_with_accounting_dimension(self): invoices = self.make_invoices( invoice_type="Sales", company="_Test Opening Invoice Company", department="Sales - _TOIC" ) - expected_value = { - "keys": ["customer", "outstanding_amount", "status", "department"], - 0: ["_Test Customer", 300, "Overdue", "Sales - _TOIC"], - 1: ["_Test Customer 1", 250, "Overdue", "Sales - _TOIC"], - } - self.check_expected_values(invoices, expected_value, invoice_type="Sales") + for invoice in invoices: + self.assertEqual(frappe.db.get_value("Sales Invoice", invoice, "department"), "Sales - _TOIC") - def tearDown(self): + def test_opening_entry_project_linking(self): + doc = self.make_invoices( + company="_Test Opening Invoice Company", invoice_type="Sales", return_doc=True + ) + project_1 = make_project( + {"project_name": "Test Opening Invoice projecty 01", "company": "_Test Opening Invoice Company"} + ) + project_2 = make_project( + {"project_name": "Test Opening Invoice projecty 02", "company": "_Test Opening Invoice Company"} + ) + doc.invoices[0].project = project_1.name + doc.invoices[1].project = project_2.name + invoices = doc.make_invoices() + sales_invoice_1 = frappe.get_doc("Sales Invoice", invoices[0]) + sales_invoice_2 = frappe.get_doc("Sales Invoice", invoices[1]) + + self.assertEqual(sales_invoice_1.items[0].project, project_1.name) + self.assertEqual(sales_invoice_2.items[0].project, project_2.name) + + @classmethod + def tearDownClass(cls): disable_dimension() + super().tearDownClass() def get_opening_invoice_creation_dict(**args): party = "Customer" if args.get("invoice_type", "Sales") == "Sales" else "Supplier" company = args.get("company", "_Test Company") + default_invoices = [] + default_invoice_rows = [ + { + "qty": 1.0, + "outstanding_amount": 200, + "party": f"_Test {party}", + "item_name": "Opening Item", + "due_date": add_days(today(), -10), + "posting_date": add_days(today(), -15), + "temporary_opening_account": get_temporary_opening_account(company), + }, + { + "qty": 1.0, + "outstanding_amount": 200, + "party": f"_Test {party} 1", + "item_name": "Opening Item", + "due_date": add_days(today(), -10), + "posting_date": add_days(today(), -15), + "temporary_opening_account": get_temporary_opening_account(company), + }, + ] + + for row in args.get("invoices") or default_invoice_rows: + default_invoices.append( + { + "qty": row.get("qty") or 1.0, + "outstanding_amount": row.get("outstanding_amount") or 200, + "party": row.get("party") or f"_Test {party}", + "item_name": row.get("item_name") or "Opening Item", + "due_date": row.get("due_date") or add_days(today(), -10), + "posting_date": row.get("posting_date") or add_days(today(), -15), + "temporary_opening_account": row.get("temporary_opening_account") + or get_temporary_opening_account(company), + "invoice_number": row.get("invoice_number"), + "project": row.get("project"), + "cost_center": row.get("cost_center"), + } + ) invoice_dict = frappe._dict( { "company": company, "invoice_type": args.get("invoice_type", "Sales"), - "invoices": [ - { - "qty": 1.0, - "outstanding_amount": 300, - "party": args.get("party_1") or f"_Test {party}", - "item_name": "Opening Item", - "due_date": "2016-09-10", - "posting_date": "2016-09-05", - "temporary_opening_account": get_temporary_opening_account(company), - "invoice_number": args.get("invoice_number"), - }, - { - "qty": 2.0, - "outstanding_amount": 250, - "party": args.get("party_2") or f"_Test {party} 1", - "item_name": "Opening Item", - "due_date": "2016-09-10", - "posting_date": "2016-09-05", - "temporary_opening_account": get_temporary_opening_account(company), - "invoice_number": None, - }, - ], + "project": args.get("project"), + "cost_center": args.get("cost_center"), + "invoices": default_invoices, } ) invoice_dict.update(args) + invoice_dict.invoices = default_invoices return invoice_dict diff --git a/erpnext/accounts/doctype/opening_invoice_creation_tool_item/opening_invoice_creation_tool_item.json b/erpnext/accounts/doctype/opening_invoice_creation_tool_item/opening_invoice_creation_tool_item.json index ed8ff7c0f7a..f8aa72fbf21 100644 --- a/erpnext/accounts/doctype/opening_invoice_creation_tool_item/opening_invoice_creation_tool_item.json +++ b/erpnext/accounts/doctype/opening_invoice_creation_tool_item/opening_invoice_creation_tool_item.json @@ -19,7 +19,8 @@ "qty", "accounting_dimensions_section", "cost_center", - "dimension_col_break" + "dimension_col_break", + "project" ], "fields": [ { @@ -79,6 +80,7 @@ "fieldtype": "Currency", "in_list_view": 1, "label": "Outstanding Amount", + "options": "Company:company:default_currency", "reqd": 1 }, { @@ -111,11 +113,17 @@ "fieldname": "invoice_number", "fieldtype": "Data", "label": "Invoice Number" + }, + { + "fieldname": "project", + "fieldtype": "Link", + "label": "Project", + "options": "Project" } ], "istable": 1, "links": [], - "modified": "2022-03-21 19:31:45.382656", + "modified": "2026-07-03 15:17:11.938499", "modified_by": "Administrator", "module": "Accounts", "name": "Opening Invoice Creation Tool Item", @@ -126,4 +134,4 @@ "sort_order": "DESC", "states": [], "track_changes": 1 -} \ No newline at end of file +} diff --git a/erpnext/accounts/doctype/opening_invoice_creation_tool_item/opening_invoice_creation_tool_item.py b/erpnext/accounts/doctype/opening_invoice_creation_tool_item/opening_invoice_creation_tool_item.py index bc48300286f..38df591b727 100644 --- a/erpnext/accounts/doctype/opening_invoice_creation_tool_item/opening_invoice_creation_tool_item.py +++ b/erpnext/accounts/doctype/opening_invoice_creation_tool_item/opening_invoice_creation_tool_item.py @@ -25,6 +25,7 @@ class OpeningInvoiceCreationToolItem(Document): party: DF.DynamicLink party_type: DF.Link | None posting_date: DF.Date | None + project: DF.Link | None qty: DF.Data | None temporary_opening_account: DF.Link | None # end: auto-generated types diff --git a/erpnext/accounts/doctype/payment_entry/payment_entry.py b/erpnext/accounts/doctype/payment_entry/payment_entry.py index 148dd4edcc5..815bd0fd6f2 100644 --- a/erpnext/accounts/doctype/payment_entry/payment_entry.py +++ b/erpnext/accounts/doctype/payment_entry/payment_entry.py @@ -2691,6 +2691,9 @@ def get_party_details(company, party_type, party, date, cost_center=None): if not frappe.db.exists(party_type, party): frappe.throw(_("{0} {1} does not exist").format(_(party_type), party)) + ptype = "select" if frappe.only_has_select_perm(party_type) else "read" + frappe.has_permission(party_type, ptype, party, throw=True) + party_account = get_party_account(party_type, party, company) account_currency = get_account_currency(party_account) account_balance = ( @@ -2707,7 +2710,7 @@ def get_party_details(company, party_type, party, date, cost_center=None): ) if party_type in ["Customer", "Supplier"]: party_bank_account = get_party_bank_account(party_type, party) - bank_account = get_default_company_bank_account(company, party_type, party) + bank_account = get_default_company_bank_account(company, party_type, party, ignore_permissions=False) return { "party_account": party_account, diff --git a/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py b/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py index 9ec4e0a073a..d286c6513f4 100644 --- a/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py +++ b/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py @@ -796,10 +796,17 @@ class PaymentReconciliation(Document): def reconcile_dr_cr_note(dr_cr_notes, company, active_dimensions=None): + allocated_amount_precision = get_field_precision( + frappe.get_meta("Payment Reconciliation Allocation").get_field("allocated_amount") + ) for inv in dr_cr_notes: if ( - abs(frappe.db.get_value(inv.voucher_type, inv.voucher_no, "outstanding_amount")) - < inv.allocated_amount + flt( + abs(frappe.db.get_value(inv.voucher_type, inv.voucher_no, "outstanding_amount")) + - inv.allocated_amount, + allocated_amount_precision, + ) + < 0 ): frappe.throw( _("{0} has been modified after you pulled it. Please pull it again.").format(inv.voucher_type) diff --git a/erpnext/accounts/doctype/payment_reconciliation/test_payment_reconciliation.py b/erpnext/accounts/doctype/payment_reconciliation/test_payment_reconciliation.py index b7d8fb44853..a727e4ab894 100644 --- a/erpnext/accounts/doctype/payment_reconciliation/test_payment_reconciliation.py +++ b/erpnext/accounts/doctype/payment_reconciliation/test_payment_reconciliation.py @@ -155,6 +155,7 @@ class TestPaymentReconciliation(FrappeTestCase): sinv = create_sales_invoice( qty=qty, rate=rate, + posting_date=posting_date, company=self.company, customer=self.customer, item_code=self.item, @@ -2146,7 +2147,7 @@ class TestPaymentReconciliation(FrappeTestCase): pr.reconcile() si.reload() - self.assertEqual(si.status, "Partly Paid") + self.assertEqual(si.status, "Overdue") # check PR tool output post reconciliation self.assertEqual(len(pr.get("invoices")), 1) self.assertEqual(pr.get("invoices")[0].get("outstanding_amount"), 120) @@ -2540,6 +2541,76 @@ class TestPaymentReconciliation(FrappeTestCase): self.assertEqual(flt(pr.allocation[0].difference_amount), 5000.0) pr.reconcile() + def test_cr_note_split_across_invoices_floating_point_precision(self): + """Regression: when a credit note is split across multiple invoices, floating-point + arithmetic (150 - 8.45 - 90.72 = 50.83000000000001) must not cause reconcile() to fail. + + The test environment rounds INR totals to whole rupees (smallest_currency_fraction_value=0), + so the invoices are created with round-number totals (100, 200, 100) and then partially paid + down to the decimal outstanding amounts (8.45, 90.72, 72.57) via payment entries. + """ + from erpnext.accounts.doctype.payment_entry.payment_entry import get_payment_entry + + # Create invoices on different posting dates to control sort-order in Payment Reconciliation + # (invoices are sorted by posting_date ascending, so si_a is processed first). + # Processing order 8.45 → 90.72 → 72.57 produces the float chain: + # 150 - 8.45 = 141.55 → 141.55 - 90.72 = 50.83000000000001 + # The last allocation row will therefore carry allocated_amount = 50.83000000000001. + si_a = self.create_sales_invoice(qty=1, rate=100, posting_date=add_days(nowdate(), -2)) + si_b = self.create_sales_invoice(qty=1, rate=200, posting_date=add_days(nowdate(), -1)) + si_c = self.create_sales_invoice(qty=1, rate=100, posting_date=nowdate()) + + # Partially pay each invoice so the remaining outstanding is a clean decimal value. + # INR rounds the invoice total to a whole rupee, so we achieve decimal outstandings + # by subtracting a decimal-valued payment from the integer total: + # 100 - 91.55 = 8.45 + # 200 - 109.28 = 90.72 + # 100 - 27.43 = 72.57 + for si, partial_paid in ((si_a, 91.55), (si_b, 109.28), (si_c, 27.43)): + pe = get_payment_entry(si.doctype, si.name) + pe.paid_amount = partial_paid + pe.received_amount = partial_paid + pe.references[0].allocated_amount = partial_paid + pe.save().submit() + + cr_note = self.create_sales_invoice( + qty=-1, rate=150, posting_date=nowdate(), do_not_save=True, do_not_submit=True + ) + cr_note.is_return = 1 + cr_note = cr_note.save().submit() + + pr = self.create_payment_reconciliation() + # Widen date range so all three invoices (oldest is -2 days) are fetched + pr.from_invoice_date = add_days(nowdate(), -2) + pr.to_invoice_date = nowdate() + pr.from_payment_date = nowdate() + pr.to_payment_date = nowdate() + + pr.get_unreconciled_entries() + self.assertEqual(len(pr.invoices), 3) + self.assertEqual(len(pr.payments), 1) + + invoices = [x.as_dict() for x in pr.invoices] + payments = [x.as_dict() for x in pr.payments] + pr.allocate_entries(frappe._dict({"invoices": invoices, "payments": payments})) + + # Credit note (150) covers all of si_a (8.45) and si_b (90.72), then partially si_c + self.assertEqual(len(pr.allocation), 3) + last_row = pr.allocation[-1] + # Last allocated amount should be ~50.83 (possibly 50.83000000000001 due to float arithmetic) + self.assertAlmostEqual(flt(last_row.allocated_amount), 50.83, places=2) + + # reconcile() must not raise "has been modified after you pulled it" due to float imprecision + pr.reconcile() + + si_a.reload() + si_b.reload() + si_c.reload() + self.assertEqual(si_a.outstanding_amount, 0) + self.assertEqual(si_b.outstanding_amount, 0) + # si_c is only partially settled: 72.57 - 50.83 = 21.74 + self.assertAlmostEqual(si_c.outstanding_amount, 21.74, places=2) + def make_customer(customer_name, currency=None): if not frappe.db.exists("Customer", customer_name): diff --git a/erpnext/accounts/doctype/payment_request/payment_request.py b/erpnext/accounts/doctype/payment_request/payment_request.py index f13569e0d9b..01de1e34e21 100644 --- a/erpnext/accounts/doctype/payment_request/payment_request.py +++ b/erpnext/accounts/doctype/payment_request/payment_request.py @@ -367,6 +367,7 @@ class PaymentRequest(Document): bank_amount=bank_amount, created_from_payment_request=True, ) + payment_entry.set_missing_ref_details(force=True) payment_entry.update( { diff --git a/erpnext/accounts/doctype/payment_request/test_payment_request.py b/erpnext/accounts/doctype/payment_request/test_payment_request.py index df28b623488..9f92e9f4f09 100644 --- a/erpnext/accounts/doctype/payment_request/test_payment_request.py +++ b/erpnext/accounts/doctype/payment_request/test_payment_request.py @@ -618,6 +618,22 @@ class TestPaymentRequest(FrappeTestCase): pi.load_from_db() self.assertEqual(pr_2.grand_total, pi.outstanding_amount) + def test_payment_entry_reference_details_fetched_from_invoice(self): + pi = make_purchase_invoice(currency="INR", qty=1, rate=94500) + pi.submit() + + pr = make_payment_request(dt="Purchase Invoice", dn=pi.name, mute_email=1, submit_doc=0, return_doc=1) + pr.grand_total = 94000 + pr.submit() + + pe = pr.create_payment_entry(submit=False) + + self.assertEqual(pe.references[0].reference_name, pi.name) + self.assertEqual(pe.references[0].total_amount, pi.grand_total) + self.assertEqual(pe.references[0].outstanding_amount, pi.outstanding_amount) + self.assertEqual(pe.references[0].allocated_amount, 94000) + self.assertEqual(pe.paid_amount, 94000) + def test_consider_journal_entry_and_return_invoice(self): from erpnext.accounts.doctype.journal_entry.test_journal_entry import make_journal_entry diff --git a/erpnext/accounts/doctype/period_closing_voucher/test_period_closing_voucher.py b/erpnext/accounts/doctype/period_closing_voucher/test_period_closing_voucher.py index e4e31a9adf4..e9bad6d7494 100644 --- a/erpnext/accounts/doctype/period_closing_voucher/test_period_closing_voucher.py +++ b/erpnext/accounts/doctype/period_closing_voucher/test_period_closing_voucher.py @@ -379,12 +379,15 @@ class TestPeriodClosingVoucher(unittest.TestCase): self.make_period_closing_voucher(posting_date="2021-03-31") - # Passed posting_date is after PCV end date, so cancellation should not fail. - make_reverse_gl_entries( - voucher_type="Journal Entry", - voucher_no=jv.name, - posting_date="2022-01-01", - ) + frappe.db.set_single_value("Accounts Settings", "acc_frozen_upto", "2021-12-31") + + try: + make_reverse_gl_entries( + voucher_type="Journal Entry", + voucher_no=jv.name, + ) + finally: + frappe.db.set_single_value("Accounts Settings", "acc_frozen_upto", None) totals_after_cancel = frappe.db.sql( """ diff --git a/erpnext/accounts/doctype/pricing_rule/pricing_rule.py b/erpnext/accounts/doctype/pricing_rule/pricing_rule.py index cc090df9270..4fab7fc1121 100644 --- a/erpnext/accounts/doctype/pricing_rule/pricing_rule.py +++ b/erpnext/accounts/doctype/pricing_rule/pricing_rule.py @@ -156,6 +156,24 @@ class PricingRule(Document): if len(values) != len(set(values)): frappe.throw(_("Duplicate {0} found in the table").format(self.apply_on)) + if self.apply_on == "Item Code": + self.validate_template_with_variant(values) + + def validate_template_with_variant(self, item_codes): + # throws if a template and its variant both exist in one rule + variants = frappe.get_all( + "Item", + filters={"name": ("in", item_codes), "variant_of": ("in", item_codes)}, + fields=["name", "variant_of"], + ) + if variants: + variant = variants[0] + frappe.throw( + _("Variant {0} and its template {1} cannot both be added to the same Pricing Rule").format( + frappe.bold(variant.name), frappe.bold(variant.variant_of) + ) + ) + def validate_mandatory(self): if self.has_priority and not self.priority: throw(_("Priority is mandatory"), frappe.MandatoryError, _("Please Set Priority")) diff --git a/erpnext/accounts/doctype/pricing_rule/test_pricing_rule.py b/erpnext/accounts/doctype/pricing_rule/test_pricing_rule.py index 123c17f9b75..b5b464b05d9 100644 --- a/erpnext/accounts/doctype/pricing_rule/test_pricing_rule.py +++ b/erpnext/accounts/doctype/pricing_rule/test_pricing_rule.py @@ -336,6 +336,31 @@ class TestPricingRule(FrappeTestCase): details = get_item_details(args) self.assertEqual(details.get("discount_percentage"), 17.5) + def test_pricing_rule_with_template_and_its_variant(self): + if not frappe.db.exists("Item", "Test Variant PRT"): + variant = frappe.new_doc("Item") + variant.item_code = "Test Variant PRT" + variant.item_name = "Test Variant PRT" + variant.item_group = "_Test Item Group" + variant.is_stock_item = 1 + variant.variant_of = "_Test Variant Item" + variant.stock_uom = "_Test UOM" + variant.append("attributes", {"attribute": "Test Size", "attribute_value": "Medium"}) + variant.insert() + + rule = frappe.new_doc("Pricing Rule") + rule.title = "_Test Pricing Rule Template Variant" + rule.apply_on = "Item Code" + rule.currency = "USD" + rule.selling = 1 + rule.rate_or_discount = "Discount Percentage" + rule.discount_percentage = 10 + rule.company = "_Test Company" + rule.append("items", {"item_code": "_Test Variant Item"}) + rule.append("items", {"item_code": "Test Variant PRT"}) + + self.assertRaises(frappe.ValidationError, rule.insert) + def test_pricing_rule_for_stock_qty(self): test_record = { "doctype": "Pricing Rule", diff --git a/erpnext/accounts/doctype/process_period_closing_voucher/process_period_closing_voucher.py b/erpnext/accounts/doctype/process_period_closing_voucher/process_period_closing_voucher.py index b5ca3331b71..6315560b89f 100644 --- a/erpnext/accounts/doctype/process_period_closing_voucher/process_period_closing_voucher.py +++ b/erpnext/accounts/doctype/process_period_closing_voucher/process_period_closing_voucher.py @@ -86,50 +86,55 @@ class ProcessPeriodClosingVoucher(Document): cancel_pcv_processing(self.name) +def initialize_parallel_threads(docname: str): + threads = 4 + timeout = frappe.db.get_single_value("Accounts Settings", "pcv_job_timeout") or 3600 + ppcvd = qb.DocType("Process Period Closing Voucher Detail") + + frappe.db.set_value("Process Period Closing Voucher", docname, "status", "Running") + + if normal_balances := ( + qb.from_(ppcvd) + .select(ppcvd.name, ppcvd.processing_date, ppcvd.report_type, ppcvd.parentfield) + .where(ppcvd.parent.eq(docname) & ppcvd.status.eq("Queued")) + .orderby(ppcvd.parentfield, ppcvd.idx, ppcvd.processing_date) + .limit(threads) + .for_update(skip_locked=True) + .run(as_dict=True) + ): + if not is_scheduler_inactive(): + for x in normal_balances: + frappe.db.set_value( + "Process Period Closing Voucher Detail", + x.name, + "status", + "Running", + ) + frappe.enqueue( + method="erpnext.accounts.doctype.process_period_closing_voucher.process_period_closing_voucher.process_individual_date", + queue="long", + timeout=timeout, + is_async=True, + enqueue_after_commit=True, + docname=docname, + row_name=x.name, + date=x.processing_date, + report_type=x.report_type, + parentfield=x.parentfield, + ) + # keep transaction on PPCV and PPCVD short + # prevents concurrency errors - REPEATABLE READ + if not frappe.in_test: + frappe.db.commit() # nosemgrep + else: + frappe.db.set_value("Process Period Closing Voucher", docname, "status", "Completed") + + @frappe.whitelist() def start_pcv_processing(docname: str): if frappe.db.get_value("Process Period Closing Voucher", docname, "status") in ["Queued", "Running"]: frappe.has_permission("Process Period Closing Voucher", "write", doc=docname, throw=True) - frappe.db.set_value("Process Period Closing Voucher", docname, "status", "Running") - - timeout = frappe.db.get_single_value("Accounts Settings", "pcv_job_timeout") or 3600 - - ppcvd = qb.DocType("Process Period Closing Voucher Detail") - if normal_balances := ( - qb.from_(ppcvd) - .select(ppcvd.processing_date, ppcvd.report_type, ppcvd.parentfield) - .where(ppcvd.parent.eq(docname) & ppcvd.status.eq("Queued")) - .orderby(ppcvd.parentfield, ppcvd.idx, ppcvd.processing_date) - .limit(4) - .for_update(skip_locked=True) - .run(as_dict=True) - ): - if not is_scheduler_inactive(): - for x in normal_balances: - frappe.db.set_value( - "Process Period Closing Voucher Detail", - { - "processing_date": x.processing_date, - "parent": docname, - "report_type": x.report_type, - "parentfield": x.parentfield, - }, - "status", - "Running", - ) - frappe.enqueue( - method="erpnext.accounts.doctype.process_period_closing_voucher.process_period_closing_voucher.process_individual_date", - queue="long", - timeout=timeout, - is_async=True, - enqueue_after_commit=True, - docname=docname, - date=x.processing_date, - report_type=x.report_type, - parentfield=x.parentfield, - ) - else: - frappe.db.set_value("Process Period Closing Voucher", docname, "status", "Completed") + initialize_parallel_threads(docname) @frappe.whitelist() @@ -247,11 +252,11 @@ def get_gle_for_closing_account(pcv, dimension_balance, dimensions): @frappe.whitelist() def schedule_next_date(docname: str): timeout = frappe.db.get_single_value("Accounts Settings", "pcv_job_timeout") or 3600 - ppcvd = qb.DocType("Process Period Closing Voucher Detail") + if to_process := ( qb.from_(ppcvd) - .select(ppcvd.processing_date, ppcvd.report_type, ppcvd.parentfield) + .select(ppcvd.name, ppcvd.processing_date, ppcvd.report_type, ppcvd.parentfield) .where(ppcvd.parent.eq(docname) & ppcvd.status.eq("Queued")) .orderby(ppcvd.parentfield, ppcvd.idx, ppcvd.processing_date) .limit(1) @@ -261,15 +266,15 @@ def schedule_next_date(docname: str): if not is_scheduler_inactive(): frappe.db.set_value( "Process Period Closing Voucher Detail", - { - "processing_date": to_process[0].processing_date, - "parent": docname, - "report_type": to_process[0].report_type, - "parentfield": to_process[0].parentfield, - }, + to_process[0].name, "status", "Running", ) + # keep transaction on PPCV and PPCVD short + # prevents concurrency errors - REPEATABLE READ + if not frappe.in_test: + frappe.db.commit() # nosemgrep + frappe.enqueue( method="erpnext.accounts.doctype.process_period_closing_voucher.process_period_closing_voucher.process_individual_date", queue="long", @@ -277,6 +282,7 @@ def schedule_next_date(docname: str): is_async=True, enqueue_after_commit=True, docname=docname, + row_name=to_process[0].name, date=to_process[0].processing_date, report_type=to_process[0].report_type, parentfield=to_process[0].parentfield, @@ -441,6 +447,11 @@ def summarize_and_post_ledger_entries(docname): make_closing_entries(closing_entries, pcv.name, pcv.company, pcv.period_end_date) + # keep transaction on PPCV and PPCVD short + # prevents concurrency errors - REPEATABLE READ + if not frappe.in_test: + frappe.db.commit() # nosemgrep + frappe.db.set_value("Period Closing Voucher", pcv.name, "gle_processing_status", "Completed") frappe.db.set_value("Process Period Closing Voucher", docname, "status", "Completed") @@ -526,10 +537,10 @@ def build_dimension_wise_balance_dict(gl_entries): return dimension_balances -def process_individual_date(docname: str, date, report_type, parentfield): +def process_individual_date(docname: str, row_name, date, report_type, parentfield): current_date_status = frappe.db.get_value( "Process Period Closing Voucher Detail", - {"processing_date": date, "report_type": report_type, "parentfield": parentfield}, + row_name, "status", ) if current_date_status != "Running": @@ -576,17 +587,20 @@ def process_individual_date(docname: str, date, report_type, parentfield): # save results frappe.db.set_value( "Process Period Closing Voucher Detail", - {"processing_date": date, "parent": docname, "report_type": report_type, "parentfield": parentfield}, + row_name, "closing_balance", frappe.json.dumps(res), ) frappe.db.set_value( "Process Period Closing Voucher Detail", - {"processing_date": date, "parent": docname, "report_type": report_type, "parentfield": parentfield}, + row_name, "status", "Completed", ) + # commit heavy computation before touching PPCV or PPCVD + if not frappe.in_test: + frappe.db.commit() # nosemgrep # chain call schedule_next_date(docname) diff --git a/erpnext/accounts/doctype/process_period_closing_voucher_detail/process_period_closing_voucher_detail.py b/erpnext/accounts/doctype/process_period_closing_voucher_detail/process_period_closing_voucher_detail.py index f3a8302ac5b..0e0b905c96a 100644 --- a/erpnext/accounts/doctype/process_period_closing_voucher_detail/process_period_closing_voucher_detail.py +++ b/erpnext/accounts/doctype/process_period_closing_voucher_detail/process_period_closing_voucher_detail.py @@ -1,7 +1,7 @@ # Copyright (c) 2025, Frappe Technologies Pvt. Ltd. and contributors # For license information, please see license.txt -# import frappe +import frappe from frappe.model.document import Document @@ -24,3 +24,10 @@ class ProcessPeriodClosingVoucherDetail(Document): # end: auto-generated types pass + + +def on_doctype_update(): + frappe.db.add_index( + "Process Period Closing Voucher Detail", + ["parent", "status", "parentfield", "idx", "processing_date"], + ) diff --git a/erpnext/accounts/doctype/process_statement_of_accounts/process_statement_of_accounts.html b/erpnext/accounts/doctype/process_statement_of_accounts/process_statement_of_accounts.html index cd1e357e3bc..c60de4c29c7 100644 --- a/erpnext/accounts/doctype/process_statement_of_accounts/process_statement_of_accounts.html +++ b/erpnext/accounts/doctype/process_statement_of_accounts/process_statement_of_accounts.html @@ -13,7 +13,7 @@ {% endif %} -

{{ _("GENERAL LEDGER") }}

+

{{ _("STATEMENT OF ACCOUNTS") }}

{% if filters.party[0] == filters.party_name[0] %}
{{ _("Customer: ") }} {{ filters.party_name[0] }}
diff --git a/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.json b/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.json index 88428e57879..8e9da9baa9d 100644 --- a/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.json +++ b/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.json @@ -1397,8 +1397,10 @@ "fetch_from": "supplier.represents_company", "fieldname": "represents_company", "fieldtype": "Link", + "ignore_user_permissions": 1, "label": "Represents Company", - "options": "Company" + "options": "Company", + "read_only": 1 }, { "depends_on": "eval:doc.update_stock && doc.is_internal_supplier", @@ -1660,7 +1662,7 @@ "idx": 204, "is_submittable": 1, "links": [], - "modified": "2026-03-17 20:44:00.221219", + "modified": "2026-07-12 23:54:21.263951", "modified_by": "Administrator", "module": "Accounts", "name": "Purchase Invoice", diff --git a/erpnext/accounts/general_ledger.py b/erpnext/accounts/general_ledger.py index 599173c99f5..a1cabc1f3a5 100644 --- a/erpnext/accounts/general_ledger.py +++ b/erpnext/accounts/general_ledger.py @@ -697,13 +697,15 @@ def make_reverse_gl_entries( partial_cancel=partial_cancel, ) validate_accounting_period(gl_entries) - check_freezing_date(gl_entries[0]["posting_date"], adv_adj) is_opening = any(d.get("is_opening") == "Yes" for d in gl_entries) - # For reverse entries, use the posting_date parameter if provided and valid - # Otherwise fall back to original posting_date - validation_date = posting_date if posting_date else gl_entries[0]["posting_date"] + if immutable_ledger_enabled: + validation_date = posting_date or frappe.form_dict.get("posting_date") or getdate() + else: + validation_date = posting_date if posting_date else gl_entries[0]["posting_date"] + + check_freezing_date(validation_date, adv_adj) validate_against_pcv(is_opening, validation_date, gl_entries[0]["company"]) if partial_cancel: @@ -770,7 +772,7 @@ def make_reverse_gl_entries( if immutable_ledger_enabled: new_gle["is_cancelled"] = 0 - new_gle["posting_date"] = frappe.form_dict.get("posting_date") or getdate() + new_gle["posting_date"] = posting_date or frappe.form_dict.get("posting_date") or getdate() elif posting_date: new_gle["posting_date"] = posting_date @@ -802,13 +804,24 @@ def check_freezing_date(posting_date, adv_adj=False): ) -def validate_against_pcv(is_opening, posting_date, company): - if is_opening and frappe.db.exists("Period Closing Voucher", {"docstatus": 1, "company": company}): +def validate_opening_entry_against_pcv(company): + if frappe.db.exists("Period Closing Voucher", {"docstatus": 1, "company": company}): frappe.throw( - _("Opening Entry can not be created after Period Closing Voucher is created."), + _( + "A Period Closing Voucher is already submitted and an Opening Entry can no longer be created. {0} to learn more." + ).format( + '' + + _("Read the docs") + + "" + ), title=_("Invalid Opening Entry"), ) + +def validate_against_pcv(is_opening, posting_date, company): + if is_opening: + validate_opening_entry_against_pcv(company) + # Local import so you don't have to touch file-level imports from frappe.query_builder.functions import Max diff --git a/erpnext/accounts/party.py b/erpnext/accounts/party.py index b39c5a7dc62..2edba2e5c5d 100644 --- a/erpnext/accounts/party.py +++ b/erpnext/accounts/party.py @@ -431,6 +431,17 @@ def get_party_account(party_type, party=None, company=None, include_advance=Fals Will first search in party (Customer / Supplier) record, if not found, will search in group (Customer Group / Supplier Group), finally will return default.""" + + def account_perm_check(account): + ptype = "select" if frappe.only_has_select_perm("Account") else "read" + if frappe.has_permission("Account", ptype, account): + return + + # Using custom message to prevent data leak in case of `apply_strict_permission` is enabled. + frappe.throw( + _("User don't have permissions to select/read this account."), exc=frappe.PermissionError + ) + if not party_type: frappe.throw(_("Party Type is mandatory")) if not company: @@ -441,46 +452,51 @@ def get_party_account(party_type, party=None, company=None, include_advance=Fals "default_receivable_account" if party_type == "Customer" else "default_payable_account" ) - return frappe.get_cached_value("Company", company, default_account_name) - - account = frappe.db.get_value( - "Party Account", {"parenttype": party_type, "parent": party, "company": company}, "account" - ) - - if not account and party_type in ["Customer", "Supplier"]: - party_group_doctype = "Customer Group" if party_type == "Customer" else "Supplier Group" - group = frappe.get_cached_value(party_type, party, scrub(party_group_doctype)) + account = frappe.get_cached_value("Company", company, default_account_name) + else: account = frappe.db.get_value( - "Party Account", - {"parenttype": party_group_doctype, "parent": group, "company": company}, - "account", + "Party Account", {"parenttype": party_type, "parent": party, "company": company}, "account" ) - if not account and party_type in ["Customer", "Supplier"]: - default_account_name = ( - "default_receivable_account" if party_type == "Customer" else "default_payable_account" - ) - account = frappe.get_cached_value("Company", company, default_account_name) + if not account and party_type in ["Customer", "Supplier"]: + party_group_doctype = "Customer Group" if party_type == "Customer" else "Supplier Group" + group = frappe.get_cached_value(party_type, party, scrub(party_group_doctype)) + account = frappe.db.get_value( + "Party Account", + {"parenttype": party_group_doctype, "parent": group, "company": company}, + "account", + ) - existing_gle_currency = get_party_gle_currency(party_type, party, company) - if existing_gle_currency: - if account: - account_currency = frappe.get_cached_value("Account", account, "account_currency") - if (account and account_currency != existing_gle_currency) or not account: - account = get_party_gle_account(party_type, party, company) + if not account and party_type in ["Customer", "Supplier"]: + default_account_name = ( + "default_receivable_account" if party_type == "Customer" else "default_payable_account" + ) + account = frappe.get_cached_value("Company", company, default_account_name) - # get default account on the basis of party type - if not account: - account_type = frappe.get_cached_value("Party Type", party_type, "account_type") - default_account_name = "default_" + account_type.lower() + "_account" - account = frappe.get_cached_value("Company", company, default_account_name) + existing_gle_currency = get_party_gle_currency(party_type, party, company) + if existing_gle_currency: + if account: + account_currency = frappe.get_cached_value("Account", account, "account_currency") + if (account and account_currency != existing_gle_currency) or not account: + account = get_party_gle_account(party_type, party, company) - if include_advance and party_type in ["Customer", "Supplier", "Student"]: + # get default account on the basis of party type + if not account: + account_type = frappe.get_cached_value("Party Type", party_type, "account_type") + default_account_name = "default_" + account_type.lower() + "_account" + account = frappe.get_cached_value("Company", company, default_account_name) + + if account: + account_perm_check(account) + + if include_advance and party and party_type in ["Customer", "Supplier", "Student"]: advance_account = get_party_advance_account(party_type, party, company) + if advance_account: + account_perm_check(advance_account) return [account, advance_account] - else: - return [account] + + return [account] return account diff --git a/erpnext/accounts/report/accounts_receivable/accounts_receivable.py b/erpnext/accounts/report/accounts_receivable/accounts_receivable.py index e83311647b2..42b3991194b 100644 --- a/erpnext/accounts/report/accounts_receivable/accounts_receivable.py +++ b/erpnext/accounts/report/accounts_receivable/accounts_receivable.py @@ -263,10 +263,12 @@ class ReceivablePayableReport: # Build and use a separate row for Employee Advances. # This allows Payments or Journals made against Emp Advance to be processed. - if ( - not row - and ple.against_voucher_type == "Employee Advance" - and self.filters.handle_employee_advances + if not row and ( + (ple.against_voucher_type == "Employee Advance" and self.filters.handle_employee_advances) + or ( + ple.against_voucher_type == "Exchange Rate Revaluation" + and self.filters.for_revaluation_journals + ) ): _d = self.build_voucher_dict(ple) _d.voucher_type = ple.against_voucher_type diff --git a/erpnext/controllers/accounts_controller.py b/erpnext/controllers/accounts_controller.py index 0d3f13dde8c..365e481890f 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -141,6 +141,26 @@ class AccountsController(TransactionBase): if self.doctype in relevant_docs: self.set_payment_schedule() + def before_insert(self): + self.clear_clearance_date_on_amend() + + def clear_clearance_date_on_amend(self): + """Drop the bank reconciliation clearance date copied over while amending. + + The framework copies `no_copy` fields when amending, so a reconciled + voucher would carry a stale clearance date into its amendment even though + the linked bank transaction gets unreconciled on cancellation. + """ + if not self.get("amended_from"): + return + + if self.meta.has_field("clearance_date"): + self.clearance_date = None + + for payment in self.get("payments") or []: + if payment.meta.has_field("clearance_date"): + payment.clearance_date = None + def remove_bundle_for_non_stock_invoices(self): has_sabb = False if self.doctype in ("Sales Invoice", "Purchase Invoice") and not self.update_stock: diff --git a/erpnext/controllers/item_variant.py b/erpnext/controllers/item_variant.py index c2c620950af..a05ff7f3b7c 100644 --- a/erpnext/controllers/item_variant.py +++ b/erpnext/controllers/item_variant.py @@ -177,6 +177,68 @@ def update_variant_attribute_values(item_attribute): frappe.flags.attribute_values = None +def get_attribute_abbr_renames(item_attribute): + """Return the set of (current) attribute values whose abbreviation was renamed.""" + if item_attribute.numeric_values: + return set() + + db_value = item_attribute.get_doc_before_save() + if not db_value: + return set() + + old_abbrs = {d.name: d.abbr for d in db_value.item_attribute_values} + changed_values = set() + + for row in item_attribute.item_attribute_values: + if row.name in old_abbrs and old_abbrs[row.name] != row.abbr: + changed_values.add(row.attribute_value) + + return changed_values + + +def update_variant_item_codes_for_abbr_renames(item_attribute): + """Rebuild item_code/item_name of variant Items affected by a renamed Item Attribute abbreviation.""" + changed_values = get_attribute_abbr_renames(item_attribute) + if not changed_values: + return + + item_variant_table = frappe.qb.DocType("Item Variant Attribute") + variant_names = ( + frappe.qb.from_(item_variant_table) + .select(item_variant_table.parent) + .where(item_variant_table.attribute == item_attribute.name) + .where(item_variant_table.attribute_value.isin(list(changed_values))) + .distinct() + .run(pluck=True) + ) + + for variant_name in variant_names: + rename_variant_item_code(variant_name) + + +def rename_variant_item_code(variant_name): + """Recompute a variant's item_code/item_name from its template and current attribute abbreviations, + renaming the Item if it has changed.""" + variant = frappe.get_doc("Item", variant_name) + if not variant.variant_of: + return + + template = frappe.get_cached_doc("Item", variant.variant_of) + + new_code = frappe._dict({"item_code": None, "item_name": None, "attributes": variant.attributes}) + make_variant_item_code(template.item_code, template.item_name, new_code) + + if not new_code.item_code or new_code.item_code == variant.item_code: + return + + frappe.rename_doc("Item", variant.item_code, new_code.item_code) + + # Keep item_name in lockstep with item_code: both are derived from the same abbreviation, so + # item_name is always rebuilt here too, even if it had since been customized away from that pattern. + if new_code.item_name and new_code.item_name != variant.item_name: + frappe.db.set_value("Item", new_code.item_code, "item_name", new_code.item_name) + + def validate_item_attribute_value(attributes_list, attribute, attribute_value, item, from_variant=True): allow_rename_attribute_value = frappe.db.get_single_value( "Item Variant Settings", "allow_rename_attribute_value" diff --git a/erpnext/controllers/status_updater.py b/erpnext/controllers/status_updater.py index c695d17e80f..05011a6b9d9 100644 --- a/erpnext/controllers/status_updater.py +++ b/erpnext/controllers/status_updater.py @@ -158,7 +158,8 @@ status_map = { "Pick List": [ ["Draft", None], ["Open", "eval:self.docstatus == 1"], - ["Completed", "stock_entry_exists"], + ["Completed", "is_fully_transferred"], + ["Partially Transferred", "is_partially_transferred"], [ "Partly Delivered", "eval:self.purpose == 'Delivery' and self.delivery_status == 'Partly Delivered'", diff --git a/erpnext/controllers/stock_controller.py b/erpnext/controllers/stock_controller.py index f58c831922a..c48eb2bd620 100644 --- a/erpnext/controllers/stock_controller.py +++ b/erpnext/controllers/stock_controller.py @@ -263,6 +263,10 @@ class StockController(AccountsController): parent_details = self.get_parent_details_for_packed_items() for row in self.get(table_name): + item_code = row.get("rm_item_code") or row.get("item_code") + if not item_code or not self.is_serial_batch_item(item_code): + continue + if ( not via_landed_cost_voucher and row.serial_and_batch_bundle diff --git a/erpnext/controllers/trends.py b/erpnext/controllers/trends.py index f8e152f5299..28ff84c83fd 100644 --- a/erpnext/controllers/trends.py +++ b/erpnext/controllers/trends.py @@ -361,13 +361,24 @@ def based_wise_columns_query(based_on, trans): # based_on_cols, based_on_select, based_on_group_by, addl_tables if based_on == "Item": - based_on_details["based_on_cols"] = ["Item:Link/Item:120", "Item Name:Data:120"] + based_on_details["based_on_cols"] = [ + {"label": _("Item"), "fieldtype": "Link", "options": "Item", "width": 120, "fieldname": "item"}, + {"label": _("Item Name"), "fieldtype": "Data", "width": 120, "fieldname": "item_name"}, + ] based_on_details["based_on_select"] = "t2.item_code, t2.item_name," based_on_details["based_on_group_by"] = "t2.item_code" based_on_details["addl_tables"] = "" elif based_on == "Item Group": - based_on_details["based_on_cols"] = ["Item Group:Link/Item Group:120"] + based_on_details["based_on_cols"] = [ + { + "label": _("Item Group"), + "fieldtype": "Link", + "options": "Item Group", + "width": 120, + "fieldname": "item_group", + } + ] based_on_details["based_on_select"] = "t2.item_group," based_on_details["based_on_group_by"] = "t2.item_group" based_on_details["addl_tables"] = "" @@ -375,32 +386,80 @@ def based_wise_columns_query(based_on, trans): elif based_on == "Customer": if trans == "Quotation": based_on_details["based_on_cols"] = [ - "Party:Link/Customer:120", - "Party Name:Data:120", - "Territory:Link/Territory:120", + { + "label": _("Party"), + "fieldtype": "Link", + "options": "Customer", + "width": 120, + "fieldname": "party", + }, + {"label": _("Party Name"), "fieldtype": "Data", "width": 120, "fieldname": "party_name"}, + { + "label": _("Territory"), + "fieldtype": "Link", + "options": "Territory", + "width": 120, + "fieldname": "territory", + }, ] based_on_details["based_on_select"] = "t1.party_name, t1.customer_name, t1.territory," else: based_on_details["based_on_cols"] = [ - "Customer:Link/Customer:120", - "Customer Name:Data:120", - "Territory:Link/Territory:120", + { + "label": _("Customer"), + "fieldtype": "Link", + "options": "Customer", + "width": 120, + "fieldname": "customer", + }, + { + "label": _("Customer Name"), + "fieldtype": "Data", + "width": 120, + "fieldname": "customer_name", + }, + { + "label": _("Territory"), + "fieldtype": "Link", + "options": "Territory", + "width": 120, + "fieldname": "territory", + }, ] based_on_details["based_on_select"] = "t1.customer, t1.customer_name, t1.territory," based_on_details["based_on_group_by"] = "t1.party_name" if trans == "Quotation" else "t1.customer" based_on_details["addl_tables"] = "" elif based_on == "Customer Group": - based_on_details["based_on_cols"] = ["Customer Group:Link/Customer Group"] + based_on_details["based_on_cols"] = [ + { + "label": _("Customer Group"), + "fieldtype": "Link", + "options": "Customer Group", + "fieldname": "customer_group", + } + ] based_on_details["based_on_select"] = "t1.customer_group," based_on_details["based_on_group_by"] = "t1.customer_group" based_on_details["addl_tables"] = "" elif based_on == "Supplier": based_on_details["based_on_cols"] = [ - "Supplier:Link/Supplier:120", - "Supplier Name:Data:120", - "Supplier Group:Link/Supplier Group:140", + { + "label": _("Supplier"), + "fieldtype": "Link", + "options": "Supplier", + "width": 120, + "fieldname": "supplier", + }, + {"label": _("Supplier Name"), "fieldtype": "Data", "width": 120, "fieldname": "supplier_name"}, + { + "label": _("Supplier Group"), + "fieldtype": "Link", + "options": "Supplier Group", + "width": 140, + "fieldname": "supplier_group", + }, ] based_on_details["based_on_select"] = "t1.supplier, t1.supplier_name, t3.supplier_group," based_on_details["based_on_group_by"] = "t1.supplier" @@ -408,26 +467,58 @@ def based_wise_columns_query(based_on, trans): based_on_details["addl_tables_relational_cond"] = " and t1.supplier = t3.name" elif based_on == "Supplier Group": - based_on_details["based_on_cols"] = ["Supplier Group:Link/Supplier Group:140"] + based_on_details["based_on_cols"] = [ + { + "label": _("Supplier Group"), + "fieldtype": "Link", + "options": "Supplier Group", + "width": 140, + "fieldname": "supplier_group", + } + ] based_on_details["based_on_select"] = "t3.supplier_group," based_on_details["based_on_group_by"] = "t3.supplier_group" based_on_details["addl_tables"] = ",`tabSupplier` t3" based_on_details["addl_tables_relational_cond"] = " and t1.supplier = t3.name" elif based_on == "Territory": - based_on_details["based_on_cols"] = ["Territory:Link/Territory:120"] + based_on_details["based_on_cols"] = [ + { + "label": _("Territory"), + "fieldtype": "Link", + "options": "Territory", + "width": 120, + "fieldname": "territory", + } + ] based_on_details["based_on_select"] = "t1.territory," based_on_details["based_on_group_by"] = "t1.territory" based_on_details["addl_tables"] = "" elif based_on == "Project": if trans in ["Sales Invoice", "Delivery Note", "Sales Order"]: - based_on_details["based_on_cols"] = ["Project:Link/Project:120"] + based_on_details["based_on_cols"] = [ + { + "label": _("Project"), + "fieldtype": "Link", + "options": "Project", + "width": 120, + "fieldname": "project", + } + ] based_on_details["based_on_select"] = "t1.project," based_on_details["based_on_group_by"] = "t1.project" based_on_details["addl_tables"] = "" elif trans in ["Purchase Order", "Purchase Invoice", "Purchase Receipt"]: - based_on_details["based_on_cols"] = ["Project:Link/Project:120"] + based_on_details["based_on_cols"] = [ + { + "label": _("Project"), + "fieldtype": "Link", + "options": "Project", + "width": 120, + "fieldname": "project", + } + ] based_on_details["based_on_select"] = "t2.project," based_on_details["based_on_group_by"] = "t2.project" based_on_details["addl_tables"] = "" @@ -435,7 +526,15 @@ def based_wise_columns_query(based_on, trans): frappe.throw(_("Project-wise data is not available for Quotation")) based_on_details["based_on_select"] += "t4.default_currency as currency," - based_on_details["based_on_cols"].append("Currency:Link/Currency:120") + based_on_details["based_on_cols"].append( + { + "label": _("Currency"), + "fieldtype": "Link", + "options": "Currency", + "width": 120, + "fieldname": "currency", + } + ) based_on_details["addl_tables"] += ", `tabCompany` t4" based_on_details["addl_tables_relational_cond"] = ( based_on_details.get("addl_tables_relational_cond", "") + " and t1.company = t4.name" @@ -446,6 +545,14 @@ def based_wise_columns_query(based_on, trans): def group_wise_column(group_by): if group_by: - return [group_by + ":Link/" + group_by + ":120"] + return [ + { + "label": _(group_by), + "fieldtype": "Link", + "options": group_by, + "width": 120, + "fieldname": frappe.scrub(group_by), + } + ] else: return [] diff --git a/erpnext/crm/doctype/crm_settings/crm_settings.js b/erpnext/crm/doctype/crm_settings/crm_settings.js index 0fb695a3da4..ef71437be49 100644 --- a/erpnext/crm/doctype/crm_settings/crm_settings.js +++ b/erpnext/crm/doctype/crm_settings/crm_settings.js @@ -2,6 +2,35 @@ // For license information, please see license.txt frappe.ui.form.on("CRM Settings", { - // refresh: function(frm) { - // } + refresh: function (frm) { + const flag = frm.events.calculate_visiblity_flag(frm); + + frm.set_df_property("allowed_users", "hidden", !flag); + frm.set_df_property("allowed_users", "reqd", flag); + }, + + enable_frappe_crm_data_synchronization: function (frm) { + const flag = frm.events.calculate_visiblity_flag(frm); + + if (flag) { + frappe.show_alert( + __("Allowed Users is required for data synchronization from remote Frappe CRM site.") + ); + } + + /* + make allowed_users field visible and mandatory if enable_frappe_crm_data_synchronization + is set and crm app is not installed. + */ + + frm.set_df_property("allowed_users", "hidden", !flag); + frm.set_df_property("allowed_users", "reqd", flag); + }, + + calculate_visiblity_flag: function (frm) { + const crm_sync_enabled = frm.doc.enable_frappe_crm_data_synchronization; + const is_crm_installed = cint(frappe.utils.get_installed_apps().includes("crm")); + + return crm_sync_enabled && !is_crm_installed; + }, }); diff --git a/erpnext/crm/doctype/crm_settings/crm_settings.json b/erpnext/crm/doctype/crm_settings/crm_settings.json index 8822dd7ea02..3539da5b7cb 100644 --- a/erpnext/crm/doctype/crm_settings/crm_settings.json +++ b/erpnext/crm/doctype/crm_settings/crm_settings.json @@ -120,9 +120,9 @@ "fieldtype": "Column Break" }, { - "depends_on": "eval:doc.enable_frappe_crm_data_synchronization === 1;", "fieldname": "allowed_users", "fieldtype": "Table MultiSelect", + "hidden": 1, "label": "Allowed Users", "options": "Frappe CRM Allowed User", "permlevel": 1 @@ -139,7 +139,7 @@ "index_web_pages_for_search": 1, "issingle": 1, "links": [], - "modified": "2026-06-22 01:26:13.474915", + "modified": "2026-07-01 01:09:16.461470", "modified_by": "Administrator", "module": "CRM", "name": "CRM Settings", diff --git a/erpnext/crm/doctype/crm_settings/crm_settings.py b/erpnext/crm/doctype/crm_settings/crm_settings.py index 7ca341adb77..379c55ae5b3 100644 --- a/erpnext/crm/doctype/crm_settings/crm_settings.py +++ b/erpnext/crm/doctype/crm_settings/crm_settings.py @@ -6,6 +6,8 @@ from frappe import _ from frappe.custom.doctype.custom_field.custom_field import create_custom_fields from frappe.model.document import Document +from erpnext.crm.frappe_crm_api import is_crm_installed + class CRMSettings(Document): # begin: auto-generated types @@ -46,13 +48,16 @@ class CRMSettings(Document): ) def validate_allowed_users(self): - if self.enable_frappe_crm_data_synchronization and not self.allowed_users: + if self.enable_frappe_crm_data_synchronization and not (is_crm_installed() or self.allowed_users): frappe.throw( _( "Please add atleast one user on Allowed Users to allow Data Synchronization from Frappe CRM site." ) ) + if self.enable_frappe_crm_data_synchronization and is_crm_installed() and self.allowed_users: + frappe.throw(_("Allowed Users is not required as Frappe CRM is already installed on the site.")) + def before_save(self): self.clear_allowed_users() diff --git a/erpnext/crm/frappe_crm_api.py b/erpnext/crm/frappe_crm_api.py index 5db9b7dc652..ddd974663dc 100644 --- a/erpnext/crm/frappe_crm_api.py +++ b/erpnext/crm/frappe_crm_api.py @@ -1,5 +1,6 @@ import json +import click import frappe from frappe import _ @@ -150,7 +151,9 @@ def create_customer(customer_data=None): for field in CUSTOMER_ALLOWED_FIELDS: if customer_data.get(field) is not None: customer.set(field, customer_data.get(field)) - customer.insert(ignore_permissions=True) + + # If CRM is installed on the site, User Permission cannot be ignored while saving Customer Records. + customer.insert(ignore_permissions=not is_crm_installed()) customer_name = customer.name contacts = json.loads(customer_data.get("contacts")) @@ -169,6 +172,10 @@ def validate_frappe_crm_sync(): _("Frappe CRM data synchronization is not enabled on ERPNext. Contact System Manager of ERPNext.") ) + # Skip allowed_users validation if CRM is installed on the site. + if is_crm_installed(): + return + allowed_users = [d.user for d in CRMSettings.allowed_users] if frappe.session.user not in allowed_users: @@ -178,3 +185,35 @@ def validate_frappe_crm_sync(): ), exc=frappe.PermissionError, ) + + +def is_crm_installed(): + return "crm" in frappe.get_installed_apps() + + +def remove_allowed_users_on_crm_install(): + try: + CRMSettings = frappe.get_single("CRM Settings") + + if not CRMSettings.enable_frappe_crm_data_synchronization: + return + + CRMSettings.allowed_users = [] + CRMSettings.save() + click.secho("Removed 'Allowed Users' from CRM Settings.") + except Exception: + click.secho("'Allowed Users' from CRM Settings couldn't be cleared.") + + +def disable_frappe_crm_data_synchronization_on_crm_uninstall(): + try: + CRMSettings = frappe.get_single("CRM Settings") + + if not CRMSettings.enable_frappe_crm_data_synchronization: + return + + CRMSettings.enable_frappe_crm_data_synchronization = 0 + CRMSettings.save() + click.secho("'Enable Frappe CRM Data Synchronization' on CRM Settings has been disabled.") + except Exception: + click.secho("'Enable Frappe CRM Data Synchronization' on CRM Settings could not be disabled.") diff --git a/erpnext/crm/utils.py b/erpnext/crm/utils.py index 8e6574bde4d..06d97b5173f 100644 --- a/erpnext/crm/utils.py +++ b/erpnext/crm/utils.py @@ -166,6 +166,7 @@ def get_open_todos(ref_doctype, ref_docname): "allocated_to", "date", ], + order_by="date asc", ) @@ -190,6 +191,7 @@ def get_open_events(ref_doctype, ref_docname): & (event_link.reference_docname == ref_docname) & (event.status == "Open") ) + .orderby(event.starts_on) ) data = query.run(as_dict=True) diff --git a/erpnext/hooks.py b/erpnext/hooks.py index a1c64b60377..118f047f19c 100644 --- a/erpnext/hooks.py +++ b/erpnext/hooks.py @@ -61,6 +61,9 @@ before_install = [ ] after_install = "erpnext.setup.install.after_install" +after_app_install = "erpnext.setup.install.after_app_install" +after_app_uninstall = "erpnext.setup.install.after_app_uninstall" + boot_session = "erpnext.startup.boot.boot_session" notification_config = "erpnext.startup.notifications.get_notification_config" get_help_messages = "erpnext.utilities.activation.get_help_messages" diff --git a/erpnext/manufacturing/doctype/bom/bom.js b/erpnext/manufacturing/doctype/bom/bom.js index e83f7e232ad..88a6c3bad66 100644 --- a/erpnext/manufacturing/doctype/bom/bom.js +++ b/erpnext/manufacturing/doctype/bom/bom.js @@ -441,7 +441,11 @@ frappe.ui.form.on("BOM", { }, routing(frm) { - if (frm.doc.routing && frm.doc.with_operations && !frm.doc.operations.length) { + // Refetch operations whenever the routing is (re)selected, so that + // changing the routing - e.g. on a new BOM version copied from another + // BOM - replaces the operations with those of the newly selected routing + // instead of keeping the old ones. + if (frm.doc.routing && frm.doc.with_operations) { frappe.call({ doc: frm.doc, method: "get_routing", diff --git a/erpnext/manufacturing/doctype/work_order/test_work_order.py b/erpnext/manufacturing/doctype/work_order/test_work_order.py index 2679d6e29fe..0d421ed4a63 100644 --- a/erpnext/manufacturing/doctype/work_order/test_work_order.py +++ b/erpnext/manufacturing/doctype/work_order/test_work_order.py @@ -687,6 +687,28 @@ class TestWorkOrder(FrappeTestCase): ste = make_stock_entry(wo_order.name, "Material Transfer for Manufacture", wo_order.qty) self.assertEqual(ste.get("items")[0].get("cost_center"), "_Test Cost Center - _TC") + @change_settings("Manufacturing Settings", {"make_serial_no_batch_from_work_order": 0}) + def test_cost_center_for_manufacture_falls_back_to_item_group_default(self): + # "_Test Item Group" is master data with buying_cost_center already set to + # "_Test Cost Center 2 - _TC" for "_Test Company"; only the FG item and its + # BOM need to be created, since no existing item in that group has one. + fg_item = make_item( + "_Test FG Item For Item Group Cost Center", + {"is_stock_item": 1, "item_group": "_Test Item Group", "include_item_in_manufacturing": 1}, + ) + + if not frappe.db.exists("BOM", {"item": fg_item.name, "is_active": 1, "is_default": 1}): + make_bom(item=fg_item.name, raw_materials=["_Test Item"]) + + wo_order = make_wo_order_test_record( + production_item=fg_item.name, skip_transfer=1, source_warehouse="_Test Warehouse - _TC" + ) + ste = frappe.get_doc(make_stock_entry(wo_order.name, "Manufacture", wo_order.qty)) + ste.insert() + + fg_row = next(d for d in ste.items if d.is_finished_item) + self.assertEqual(fg_row.cost_center, "_Test Cost Center 2 - _TC") + def test_operation_time_with_batch_size(self): fg_item = "Test Batch Size Item For BOM" rm1 = "Test Batch Size Item RM 1 For BOM" @@ -1523,6 +1545,38 @@ class TestWorkOrder(FrappeTestCase): work_order.reload() self.assertEqual(work_order.material_transferred_for_manufacturing, 2.0) + def test_status_in_process_when_only_one_required_item_transferred(self): + """Stock Entry created from a Pick List that picked only one of the required items: + min-fraction keeps material_transferred_for_manufacturing at 0, but the work order must + still move to In Process because material is already in WIP.""" + from erpnext.manufacturing.doctype.work_order.work_order import create_pick_list + from erpnext.stock.doctype.pick_list.pick_list import create_stock_entry + + work_order = make_wo_order_test_record( + planned_start_date=now(), qty=2, source_warehouse="Stores - _TC" + ) + test_stock_entry.make_stock_entry( + item_code="_Test Item", target="Stores - _TC", qty=10, basic_rate=5000.0 + ) + test_stock_entry.make_stock_entry( + item_code="_Test Item Home Desktop 100", target="Stores - _TC", qty=10, basic_rate=1000.0 + ) + + pick_list = create_pick_list(work_order.name, for_qty=work_order.qty) + # pick only _Test Item; the other required item is left out of this pick list + pick_list.pick_manually = 1 + pick_list.locations = [loc for loc in pick_list.locations if loc.item_code == "_Test Item"] + pick_list.save() + pick_list.submit() + + stock_entry = frappe.get_doc(create_stock_entry(frappe.as_json(pick_list.as_dict()))) + self.assertEqual(stock_entry.fg_completed_qty, 0.0) + stock_entry.submit() + + work_order.reload() + self.assertEqual(work_order.material_transferred_for_manufacturing, 0.0) + self.assertEqual(work_order.status, "In Process") + def test_backflushed_batch_raw_materials_based_on_transferred(self): frappe.db.set_single_value( "Manufacturing Settings", diff --git a/erpnext/manufacturing/doctype/work_order/work_order.py b/erpnext/manufacturing/doctype/work_order/work_order.py index d6764005a80..dd5c07c2945 100644 --- a/erpnext/manufacturing/doctype/work_order/work_order.py +++ b/erpnext/manufacturing/doctype/work_order/work_order.py @@ -157,6 +157,7 @@ class WorkOrder(Document): self.check_wip_warehouse_skip() self.calculate_operating_cost() self.validate_qty() + self.validate_dates() self.validate_transfer_against() self.validate_operations() self.status = self.get_status() @@ -175,6 +176,11 @@ class WorkOrder(Document): self.validate_operations_sequence() + def validate_dates(self): + if self.planned_start_date and self.planned_end_date: + if get_datetime(self.planned_end_date) < get_datetime(self.planned_start_date): + frappe.throw(_("Planned End Date cannot be before Planned Start Date")) + def validate_operations_sequence(self): if all([not op.sequence_id for op in self.operations]): for op in self.operations: @@ -406,7 +412,11 @@ class WorkOrder(Document): elif self.docstatus == 1: if status not in ["Closed", "Stopped"]: status = "Not Started" - if flt(self.material_transferred_for_manufacturing) > 0 or self.skip_transfer: + if ( + flt(self.material_transferred_for_manufacturing) > 0 + or self.skip_transfer + or self.has_transferred_material() + ): status = "In Process" precision = frappe.get_precision("Work Order", "produced_qty") @@ -425,6 +435,26 @@ class WorkOrder(Document): return status + def has_transferred_material(self): + """True if any raw material was transferred against this work order via a pick list + (these leave material_transferred_for_manufacturing at 0 via the min-fraction rule).""" + ste = frappe.qb.DocType("Stock Entry") + ste_child = frappe.qb.DocType("Stock Entry Detail") + qty = ( + frappe.qb.from_(ste) + .inner_join(ste_child) + .on(ste_child.parent == ste.name) + .select(Sum(ste_child.transfer_qty)) + .where( + (ste.work_order == self.name) + & (ste.docstatus == 1) + & (ste.purpose == "Material Transfer for Manufacture") + & (ste.is_return == 0) + & (ste.pick_list.isnotnull()) + ) + ).run()[0][0] + return flt(qty) > 0 + def update_work_order_qty(self): """Update **Manufactured Qty** and **Material Transferred for Qty** in Work Order based on Stock Entry""" diff --git a/erpnext/patches.txt b/erpnext/patches.txt index b48f16a7550..f3f216ac64c 100644 --- a/erpnext/patches.txt +++ b/erpnext/patches.txt @@ -262,7 +262,6 @@ execute:frappe.rename_doc("Report", "TDS Payable Monthly", "Tax Withholding Deta erpnext.patches.v14_0.update_proprietorship_to_individual erpnext.patches.v15_0.rename_subcontracting_fields erpnext.patches.v15_0.unset_incorrect_additional_discount_percentage -erpnext.patches.v16_0.create_company_custom_fields [post_model_sync] erpnext.patches.v15_0.create_asset_depreciation_schedules_from_assets @@ -422,6 +421,7 @@ execute:frappe.db.set_single_value("Accounts Settings", "fetch_valuation_rate_fo erpnext.patches.v15_0.add_company_payment_gateway_account erpnext.patches.v15_0.update_uae_zero_rated_fetch erpnext.patches.v15_0.update_fieldname_in_accounting_dimension_filter +erpnext.patches.v16_0.create_company_custom_fields erpnext.patches.v15_0.set_asset_status_if_not_already_set erpnext.patches.v15_0.toggle_legacy_controller_for_period_closing execute:frappe.db.set_single_value("Accounts Settings", "show_party_balance", 1) @@ -438,3 +438,7 @@ erpnext.patches.v16_0.migrate_address_contact_custom_fields erpnext.patches.v15_0.set_main_item_code_in_material_request_plan_item erpnext.patches.v16_0.set_posting_datetime_for_sabb_and_drop_indexes execute:frappe.db.set_single_value("Accounts Settings", "pcv_job_timeout", 3600) +erpnext.patches.v15_0.backfill_sla_link_filters_on_custom_field +erpnext.patches.v15_0.backfill_sla_link_filters_on_docfield +erpnext.patches.v16_0.crm_settings_handle_allowed_users_for_frappe_crm +erpnext.patches.v16_0.backfill_pick_list_transferred_qty diff --git a/erpnext/patches/v15_0/backfill_sla_link_filters_on_custom_field.py b/erpnext/patches/v15_0/backfill_sla_link_filters_on_custom_field.py new file mode 100644 index 00000000000..65996f258d8 --- /dev/null +++ b/erpnext/patches/v15_0/backfill_sla_link_filters_on_custom_field.py @@ -0,0 +1,21 @@ +import frappe + + +def execute(): + for custom_field in frappe.get_all( + "Custom Field", + filters={ + "fieldname": "service_level_agreement", + "fieldtype": "Link", + "options": "Service Level Agreement", + "link_filters": ("is", "not set"), + }, + fields=["name", "dt"], + ): + link_filters = frappe.as_json( + [["Service Level Agreement", "document_type", "=", custom_field.dt]], indent=None + ) + frappe.db.set_value( + "Custom Field", custom_field.name, "link_filters", link_filters, update_modified=False + ) + frappe.clear_cache(doctype=custom_field.dt) diff --git a/erpnext/patches/v15_0/backfill_sla_link_filters_on_docfield.py b/erpnext/patches/v15_0/backfill_sla_link_filters_on_docfield.py new file mode 100644 index 00000000000..22110afc9ff --- /dev/null +++ b/erpnext/patches/v15_0/backfill_sla_link_filters_on_docfield.py @@ -0,0 +1,20 @@ +import frappe + + +def execute(): + for docfield in frappe.get_all( + "DocField", + filters={ + "parenttype": "DocType", + "fieldname": "service_level_agreement", + "fieldtype": "Link", + "options": "Service Level Agreement", + "link_filters": ("is", "not set"), + }, + fields=["name", "parent"], + ): + link_filters = frappe.as_json( + [["Service Level Agreement", "document_type", "=", docfield.parent]], indent=None + ) + frappe.db.set_value("DocField", docfield.name, "link_filters", link_filters, update_modified=False) + frappe.clear_cache(doctype=docfield.parent) diff --git a/erpnext/patches/v16_0/backfill_pick_list_transferred_qty.py b/erpnext/patches/v16_0/backfill_pick_list_transferred_qty.py new file mode 100644 index 00000000000..6d3155c4c1a --- /dev/null +++ b/erpnext/patches/v16_0/backfill_pick_list_transferred_qty.py @@ -0,0 +1,58 @@ +import frappe +from frappe.query_builder.functions import Sum +from frappe.utils import flt + + +def execute(): + StockEntry = frappe.qb.DocType("Stock Entry") + StockEntryDetail = frappe.qb.DocType("Stock Entry Detail") + + pick_lists = ( + frappe.qb.from_(StockEntry) + .select(StockEntry.pick_list) + .distinct() + .where((StockEntry.pick_list.isnotnull()) & (StockEntry.docstatus == 1)) + ).run(pluck=True) + + if not pick_lists: + return + + rows = ( + frappe.qb.from_(StockEntryDetail) + .join(StockEntry) + .on(StockEntryDetail.parent == StockEntry.name) + .select( + StockEntry.pick_list, + StockEntryDetail.item_code, + StockEntryDetail.s_warehouse, + Sum(StockEntryDetail.transfer_qty).as_("qty"), + ) + .where((StockEntry.pick_list.isin(pick_lists)) & (StockEntry.docstatus == 1)) + .groupby(StockEntry.pick_list, StockEntryDetail.item_code, StockEntryDetail.s_warehouse) + ).run(as_dict=True) + + transferred = {(r.pick_list, r.item_code, r.s_warehouse): flt(r.qty) for r in rows} + + items = frappe.get_all( + "Pick List Item", + filters={"parent": ("in", pick_lists), "picked_qty": (">", 0)}, + fields=["name", "parent", "item_code", "warehouse", "picked_qty"], + order_by="idx", + ) + + updates = {} + for row in items: + key = (row.parent, row.item_code, row.warehouse) + available = transferred.get(key, 0) + if available <= 0: + continue + qty = min(flt(row.picked_qty), available) + transferred[key] = available - qty + updates[row.name] = {"transferred_qty": qty} + + if not updates: + return + + frappe.db.auto_commit_on_many_writes = True + frappe.db.bulk_update("Pick List Item", updates) + frappe.db.auto_commit_on_many_writes = False diff --git a/erpnext/patches/v16_0/crm_settings_handle_allowed_users_for_frappe_crm.py b/erpnext/patches/v16_0/crm_settings_handle_allowed_users_for_frappe_crm.py new file mode 100644 index 00000000000..166cd5c66f8 --- /dev/null +++ b/erpnext/patches/v16_0/crm_settings_handle_allowed_users_for_frappe_crm.py @@ -0,0 +1,10 @@ +import frappe + + +def execute(): + from erpnext.crm.frappe_crm_api import is_crm_installed, remove_allowed_users_on_crm_install + + if not is_crm_installed(): + return + + remove_allowed_users_on_crm_install() diff --git a/erpnext/selling/report/inactive_customers/inactive_customers.py b/erpnext/selling/report/inactive_customers/inactive_customers.py index d21d11b2447..ea0831391d3 100644 --- a/erpnext/selling/report/inactive_customers/inactive_customers.py +++ b/erpnext/selling/report/inactive_customers/inactive_customers.py @@ -4,6 +4,8 @@ import frappe from frappe import _ +from frappe.query_builder import Case, CustomFunction +from frappe.query_builder.functions import Count, Max, Sum from frappe.utils import cint @@ -24,50 +26,69 @@ def execute(filters=None): customers = get_sales_details(doctype) data = [] - for cust in customers: - if cint(cust[8]) >= cint(days_since_last_order): - cust.insert(7, get_last_sales_amt(cust[0], doctype)) - data.append(cust) + for row in customers: + if cint(row[8]) >= cint(days_since_last_order): + row.insert(7, get_last_sales_amt(row[0], doctype)) + data.append(row) return columns, data def get_sales_details(doctype): - cond = """sum(so.base_net_total) as 'total_order_considered', - max(so.posting_date) as 'last_order_date', - DATEDIFF(CURRENT_DATE, max(so.posting_date)) as 'days_since_last_order' """ - if doctype == "Sales Order": - cond = """sum(if(so.status = "Stopped", - so.base_net_total * so.per_delivered/100, - so.base_net_total)) as 'total_order_considered', - max(so.transaction_date) as 'last_order_date', - DATEDIFF(CURRENT_DATE, max(so.transaction_date)) as 'days_since_last_order'""" + customer = frappe.qb.DocType("Customer") + sales_doctype = frappe.qb.DocType(doctype) - return frappe.db.sql( - f"""select - cust.name, - cust.customer_name, - cust.territory, - cust.customer_group, - count(distinct(so.name)) as 'num_of_order', - sum(base_net_total) as 'total_order_value', {cond} - from `tabCustomer` cust, `tab{doctype}` so - where cust.name = so.customer and so.docstatus = 1 - group by cust.name - order by 'days_since_last_order' desc """, - as_list=1, - ) + date_diff = CustomFunction("DATEDIFF", ["d1", "d2"]) + current_date = CustomFunction("CURRENT_DATE", []) + + if doctype == "Sales Order": + total_considered = Sum( + Case() + .when( + sales_doctype.status == "Stopped", + sales_doctype.base_net_total * sales_doctype.per_delivered / 100, + ) + .else_(sales_doctype.base_net_total) + ) + date_col = sales_doctype.transaction_date + else: + total_considered = Sum(sales_doctype.base_net_total) + date_col = sales_doctype.posting_date + + last_order_date = Max(date_col) + days_since_last_order = date_diff(current_date(), last_order_date) + + return ( + frappe.qb.from_(customer) + .inner_join(sales_doctype) + .on(customer.name == sales_doctype.customer) + .select( + customer.name, + customer.customer_name, + customer.territory, + customer.customer_group, + Count(sales_doctype.name).distinct().as_("num_of_order"), + Sum(sales_doctype.base_net_total).as_("total_order_value"), + total_considered.as_("total_order_considered"), + last_order_date.as_("last_order_date"), + days_since_last_order.as_("days_since_last_order"), + ) + .where(sales_doctype.docstatus == 1) + .groupby(customer.name) + .orderby(days_since_last_order, order=frappe.qb.desc) + ).run(as_list=True) def get_last_sales_amt(customer, doctype): - cond = "posting_date" - if doctype == "Sales Order": - cond = "transaction_date" - res = frappe.db.sql( - f"""select base_net_total from `tab{doctype}` - where customer = %s and docstatus = 1 order by {cond} desc - limit 1""", - customer, - ) + sales_doctype = frappe.qb.DocType(doctype) + date_col = sales_doctype.transaction_date if doctype == "Sales Order" else sales_doctype.posting_date + + res = ( + frappe.qb.from_(sales_doctype) + .select(sales_doctype.base_net_total) + .where((sales_doctype.customer == customer) & (sales_doctype.docstatus == 1)) + .orderby(date_col, order=frappe.qb.desc) + .limit(1) + ).run() return res and res[0][0] or 0 diff --git a/erpnext/selling/report/inactive_customers/test_inactive_customers.py b/erpnext/selling/report/inactive_customers/test_inactive_customers.py new file mode 100644 index 00000000000..575b7a8fe41 --- /dev/null +++ b/erpnext/selling/report/inactive_customers/test_inactive_customers.py @@ -0,0 +1,55 @@ +# Copyright (c) 2024, Frappe Technologies Pvt. Ltd. and contributors +# For license information, please see license.txt + +import frappe +from frappe.tests.utils import FrappeTestCase +from frappe.utils import add_days, getdate, today + +from erpnext.selling.doctype.sales_order.test_sales_order import make_sales_order +from erpnext.selling.report.inactive_customers.inactive_customers import execute + + +class TestInactiveCustomers(FrappeTestCase): + def setUp(self): + self.customer = frappe.new_doc("Customer") + self.customer.customer_name = "_Test Inactive Customer" + self.customer.customer_group = "_Test Customer Group" + self.customer.insert() + self.last_order_date = add_days(today(), -120) + so = make_sales_order( + customer=self.customer.name, + transaction_date=self.last_order_date, + qty=5, + rate=200, + ) + so.submit() + self.sales_order = so + + def test_invalid_doctype_is_rejected(self): + self.assertRaises( + frappe.ValidationError, + execute, + {"doctype": "Purchase Order", "days_since_last_order": 30}, + ) + + def test_inactive_customer_is_listed_with_expected_columns(self): + columns, data = execute({"doctype": "Sales Order", "days_since_last_order": 30}) + + row = self.get_customer_row(data) + self.assertIsNotNone(row, "Inactive customer should be present in the report") + + # Column contract: the report relies on positional access. + self.assertEqual(row[0], self.customer.name) + self.assertEqual(row[7], 1000) # Last Order Amount inserted at index 7 (5 * 200) + self.assertEqual(getdate(row[8]), getdate(self.last_order_date)) # Last Order Date + self.assertGreaterEqual(row[9], 30) # Days Since Last Order + + def test_recent_customer_is_excluded(self): + _columns, data = execute({"doctype": "Sales Order", "days_since_last_order": 200}) + self.assertIsNone( + self.get_customer_row(data), + "Customer ordering within the threshold must be excluded", + ) + + def get_customer_row(self, data): + return next((row for row in data if row[0] == self.customer.name), None) diff --git a/erpnext/selling/report/sales_person_wise_transaction_summary/sales_person_wise_transaction_summary.py b/erpnext/selling/report/sales_person_wise_transaction_summary/sales_person_wise_transaction_summary.py index 23ed83cca84..405159215cd 100644 --- a/erpnext/selling/report/sales_person_wise_transaction_summary/sales_person_wise_transaction_summary.py +++ b/erpnext/selling/report/sales_person_wise_transaction_summary/sales_person_wise_transaction_summary.py @@ -4,7 +4,7 @@ import frappe from frappe import _, msgprint, qb -from frappe.query_builder import Case, Criterion +from frappe.query_builder import Criterion from erpnext import get_company_currency @@ -155,60 +155,50 @@ def get_columns(filters): def get_entries(filters): - doc_type = filters["doc_type"] + date_field = filters["doc_type"] == "Sales Order" and "transaction_date" or "posting_date" + if filters["doc_type"] == "Sales Order": + qty_field = "delivered_qty" + else: + qty_field = "qty" + conditions, values = get_conditions(filters, date_field) - date_field = "transaction_date" if doc_type == "Sales Order" else "posting_date" - qty_field = "delivered_qty" if doc_type == "Sales Order" else "qty" - - dt = frappe.qb.DocType(doc_type) - dt_item = frappe.qb.DocType(f"{doc_type} Item") - st = frappe.qb.DocType("Sales Team") - - calc_qty = dt_item[qty_field] * dt_item.conversion_factor - calc_net_amount = dt_item.base_net_rate * calc_qty - - stock_qty_case = Case().when(dt.status == "Closed", calc_qty).else_(dt_item.stock_qty).as_("stock_qty") - - base_net_amount_case = ( - Case() - .when(dt.status == "Closed", calc_net_amount) - .else_(dt_item.base_net_amount) - .as_("base_net_amount") + entries = frappe.db.sql( + """ + SELECT + dt.name, dt.customer, dt.territory, dt.{} as posting_date, dt_item.item_code, + st.sales_person, st.allocated_percentage, dt_item.warehouse, + CASE + WHEN dt.status = "Closed" THEN dt_item.{} * dt_item.conversion_factor + ELSE dt_item.stock_qty + END as stock_qty, + CASE + WHEN dt.status = "Closed" THEN (dt_item.base_net_rate * dt_item.{} * dt_item.conversion_factor) + ELSE dt_item.base_net_amount + END as base_net_amount, + CASE + WHEN dt.status = "Closed" THEN ((dt_item.base_net_rate * dt_item.{} * dt_item.conversion_factor) * st.allocated_percentage/100) + ELSE dt_item.base_net_amount * st.allocated_percentage/100 + END as contribution_amt + FROM + `tab{}` dt, `tab{} Item` dt_item, `tabSales Team` st + WHERE + st.parent = dt.name and dt.name = dt_item.parent and st.parenttype = {} + and dt.docstatus = 1 {} order by st.sales_person, dt.name desc + """.format( + date_field, + qty_field, + qty_field, + qty_field, + filters["doc_type"], + filters["doc_type"], + "%s", + conditions, + ), + tuple([filters["doc_type"], *values]), + as_dict=1, ) - contribution_amt_case = ( - Case() - .when(dt.status == "Closed", (calc_net_amount * st.allocated_percentage / 100)) - .else_(dt_item.base_net_amount * st.allocated_percentage / 100) - .as_("contribution_amt") - ) - - query = ( - frappe.get_query(dt, filters=filters, ignore_permissions=False) - .join(dt_item) - .on(dt.name == dt_item.parent) - .join(st) - .on(dt.name == st.parent) - .select( - dt.name, - dt.customer, - dt.territory, - dt[date_field].as_("posting_date"), - dt_item.item_code, - st.sales_person, - st.allocated_percentage, - dt_item.warehouse, - stock_qty_case, - base_net_amount_case, - contribution_amt_case, - ) - .where(st.parenttype == doc_type) - .where(dt.docstatus == 1) - ) - - query = query.orderby(st.sales_person).orderby(dt.name, order=frappe.qb.desc) - - return query.run(as_dict=True) + return entries def get_conditions(filters, date_field): diff --git a/erpnext/setup/doctype/company/company.json b/erpnext/setup/doctype/company/company.json index fc6533a1e89..380320f0399 100644 --- a/erpnext/setup/doctype/company/company.json +++ b/erpnext/setup/doctype/company/company.json @@ -330,33 +330,48 @@ "options": "Account" }, { + "depends_on": "eval:!doc.__islocal", "fieldname": "round_off_account", "fieldtype": "Link", + "ignore_user_permissions": 1, "label": "Round Off Account", + "no_copy": 1, "options": "Account" }, { + "depends_on": "eval:!doc.__islocal", "fieldname": "round_off_cost_center", "fieldtype": "Link", + "ignore_user_permissions": 1, "label": "Round Off Cost Center", + "no_copy": 1, "options": "Cost Center" }, { + "depends_on": "eval:!doc.__islocal", "fieldname": "write_off_account", "fieldtype": "Link", + "ignore_user_permissions": 1, "label": "Write Off Account", + "no_copy": 1, "options": "Account" }, { + "depends_on": "eval:!doc.__islocal", "fieldname": "exchange_gain_loss_account", "fieldtype": "Link", + "ignore_user_permissions": 1, "label": "Exchange Gain / Loss Account", + "no_copy": 1, "options": "Account" }, { + "depends_on": "eval:!doc.__islocal", "fieldname": "unrealized_exchange_gain_loss_account", "fieldtype": "Link", + "ignore_user_permissions": 1, "label": "Unrealized Exchange Gain/Loss Account", + "no_copy": 1, "options": "Account" }, { @@ -482,6 +497,7 @@ "options": "Account" }, { + "depends_on": "eval:!doc.__islocal", "fieldname": "expenses_included_in_valuation", "fieldtype": "Link", "ignore_user_permissions": 1, @@ -490,15 +506,19 @@ "options": "Account" }, { + "depends_on": "eval:!doc.__islocal", "fieldname": "accumulated_depreciation_account", "fieldtype": "Link", + "ignore_user_permissions": 1, "label": "Accumulated Depreciation Account", "no_copy": 1, "options": "Account" }, { + "depends_on": "eval:!doc.__islocal", "fieldname": "depreciation_expense_account", "fieldtype": "Link", + "ignore_user_permissions": 1, "label": "Depreciation Expense Account", "no_copy": 1, "options": "Account" @@ -519,29 +539,39 @@ "fieldtype": "Column Break" }, { + "depends_on": "eval:!doc.__islocal", "fieldname": "disposal_account", "fieldtype": "Link", + "ignore_user_permissions": 1, "label": "Gain/Loss Account on Asset Disposal", "no_copy": 1, "options": "Account" }, { + "depends_on": "eval:!doc.__islocal", "fieldname": "depreciation_cost_center", "fieldtype": "Link", + "ignore_user_permissions": 1, "label": "Asset Depreciation Cost Center", "no_copy": 1, "options": "Cost Center" }, { + "depends_on": "eval:!doc.__islocal", "fieldname": "capital_work_in_progress_account", "fieldtype": "Link", + "ignore_user_permissions": 1, "label": "Capital Work In Progress Account", + "no_copy": 1, "options": "Account" }, { + "depends_on": "eval:!doc.__islocal", "fieldname": "asset_received_but_not_billed", "fieldtype": "Link", + "ignore_user_permissions": 1, "label": "Asset Received But Not Billed", + "no_copy": 1, "options": "Account" }, { @@ -673,15 +703,21 @@ "options": "Warehouse" }, { + "depends_on": "eval:!doc.__islocal", "fieldname": "unrealized_profit_loss_account", "fieldtype": "Link", + "ignore_user_permissions": 1, "label": "Unrealized Profit / Loss Account", + "no_copy": 1, "options": "Account" }, { + "depends_on": "eval:!doc.__islocal", "fieldname": "default_discount_account", "fieldtype": "Link", + "ignore_user_permissions": 1, "label": "Default Payment Discount Account", + "no_copy": 1, "options": "Account" }, { @@ -723,8 +759,10 @@ "documentation_url": "https://docs.erpnext.com/docs/user/manual/en/advance-in-separate-party-account", "fieldname": "default_advance_received_account", "fieldtype": "Link", + "ignore_user_permissions": 1, "label": "Default Advance Received Account", "mandatory_depends_on": "book_advance_payments_as_liability", + "no_copy": 1, "options": "Account" }, { @@ -733,8 +771,10 @@ "documentation_url": "https://docs.erpnext.com/docs/user/manual/en/advance-in-separate-party-account", "fieldname": "default_advance_paid_account", "fieldtype": "Link", + "ignore_user_permissions": 1, "label": "Default Advance Paid Account", "mandatory_depends_on": "book_advance_payments_as_liability", + "no_copy": 1, "options": "Account" }, { @@ -814,9 +854,12 @@ "options": "Account" }, { + "depends_on": "eval:!doc.__islocal", "fieldname": "round_off_for_opening", "fieldtype": "Link", + "ignore_user_permissions": 1, "label": "Round Off for Opening", + "no_copy": 1, "options": "Account" }, { @@ -865,7 +908,7 @@ "image_field": "company_logo", "is_tree": 1, "links": [], - "modified": "2025-11-16 16:51:27.624096", + "modified": "2026-07-02 07:21:21.794533", "modified_by": "Administrator", "module": "Setup", "name": "Company", diff --git a/erpnext/setup/doctype/company/company.py b/erpnext/setup/doctype/company/company.py index 299ae82cb69..c53dfa5fb40 100644 --- a/erpnext/setup/doctype/company/company.py +++ b/erpnext/setup/doctype/company/company.py @@ -74,6 +74,7 @@ class Company(NestedSet): default_operating_cost_account: DF.Link | None default_payable_account: DF.Link | None default_provisional_account: DF.Link | None + default_purchase_price_variance_account: DF.Link | None default_receivable_account: DF.Link | None default_sales_contact: DF.Link | None default_selling_terms: DF.Link | None diff --git a/erpnext/setup/install.py b/erpnext/setup/install.py index 03fc31b253e..89e2a4c89ee 100644 --- a/erpnext/setup/install.py +++ b/erpnext/setup/install.py @@ -367,3 +367,19 @@ DEFAULT_ROLE_PROFILES = { "Purchase Manager", ], } + + +def after_app_install(app_name=None): + if app_name == "crm": + from erpnext.crm.frappe_crm_api import remove_allowed_users_on_crm_install + + remove_allowed_users_on_crm_install() + + +def after_app_uninstall(app_name=None): + if app_name == "crm": + from erpnext.crm.frappe_crm_api import disable_frappe_crm_data_synchronization_on_crm_uninstall + + disable_frappe_crm_data_synchronization_on_crm_uninstall() + + frappe.db.commit() # nosemgrep diff --git a/erpnext/stock/doctype/delivery_note/delivery_note.py b/erpnext/stock/doctype/delivery_note/delivery_note.py index 0ad8bc781a5..49aa06ddd79 100644 --- a/erpnext/stock/doctype/delivery_note/delivery_note.py +++ b/erpnext/stock/doctype/delivery_note/delivery_note.py @@ -9,6 +9,7 @@ from frappe import _ from frappe.contacts.doctype.address.address import get_company_address from frappe.contacts.doctype.contact.contact import get_default_contact from frappe.desk.notifications import clear_doctype_notifications +from frappe.model.document import Document from frappe.model.mapper import get_mapped_doc from frappe.model.utils import get_fetch_values from frappe.query_builder import DocType @@ -441,22 +442,34 @@ class DeliveryNote(SellingController): frappe.throw(_("Warehouse required for stock Item {0}").format(d["item_code"])) def update_current_stock(self): - if self.get("_action") and self._action != "update_after_submit": - for d in self.get("items"): - d.actual_qty = frappe.db.get_value( - "Bin", {"item_code": d.item_code, "warehouse": d.warehouse}, "actual_qty" - ) + if not (self.get("_action") and self._action != "update_after_submit"): + return - for d in self.get("packed_items"): - bin_qty = frappe.db.get_value( - "Bin", - {"item_code": d.item_code, "warehouse": d.warehouse}, - ["actual_qty", "projected_qty"], - as_dict=True, - ) - if bin_qty: - d.actual_qty = flt(bin_qty.actual_qty) - d.projected_qty = flt(bin_qty.projected_qty) + warehouse_item_codes = {} + for d in self.get("items") + self.get("packed_items"): + warehouse_item_codes.setdefault(d.warehouse, set()).add(d.item_code) + + if not warehouse_item_codes: + return + + bin_map = {} + for warehouse, item_codes in warehouse_item_codes.items(): + for b in frappe.get_all( + "Bin", + filters={"item_code": ["in", item_codes], "warehouse": warehouse}, + fields=["item_code", "actual_qty", "projected_qty"], + ): + bin_map[(b.item_code, warehouse)] = b + + for d in self.get("items"): + bin_data = bin_map.get((d.item_code, d.warehouse)) + d.actual_qty = bin_data.actual_qty if bin_data else None + + for d in self.get("packed_items"): + bin_data = bin_map.get((d.item_code, d.warehouse)) + if bin_data: + d.actual_qty = flt(bin_data.actual_qty) + d.projected_qty = flt(bin_data.projected_qty) def on_submit(self): self.validate_packed_qty() @@ -910,7 +923,9 @@ def get_returned_qty_map(delivery_note): @frappe.whitelist() -def make_sales_invoice(source_name, target_doc=None, args=None): +def make_sales_invoice( + source_name: str, target_doc: Document | str | None = None, args: dict | str | None = None +): if args is None: args = {} if isinstance(args, str): @@ -1015,7 +1030,12 @@ def make_sales_invoice(source_name, target_doc=None, args=None): frappe.db.get_single_value("Accounts Settings", "automatically_fetch_payment_terms") ) - if not doc.is_return: + if doc.is_return: + # A credit note made from a return Delivery Note should roll back the billed + # amount on the linked Sales Order too, so that per_billed stays consistent with + # per_delivered (which the return already reset). + doc.update_billed_amount_in_sales_order = True + else: so, doctype, fieldname = doc.get_order_details() if ( doc.linked_order_has_payment_terms(so, fieldname, doctype) diff --git a/erpnext/stock/doctype/delivery_note/test_delivery_note.py b/erpnext/stock/doctype/delivery_note/test_delivery_note.py index 90bae7f68f5..e77940b1661 100644 --- a/erpnext/stock/doctype/delivery_note/test_delivery_note.py +++ b/erpnext/stock/doctype/delivery_note/test_delivery_note.py @@ -2599,6 +2599,92 @@ class TestDeliveryNote(FrappeTestCase): self.assertEqual(dn.per_returned, 100) self.assertEqual(returned.status, "Return") + def _assert_credit_note_from_return_dn_resets_per_billed(self, so, dn): + """Given a fully billed Sales Order and a submitted Delivery Note that delivers it, + a credit note made from the return of that Delivery Note must reset per_billed to 0 + while leaving the delivery quantities exactly as the return already set them.""" + from erpnext.stock.doctype.delivery_note.delivery_note import make_sales_return + + so.load_from_db() + self.assertEqual(so.per_delivered, 100) + self.assertEqual(so.per_billed, 100) + + return_dn = make_sales_return(dn.name) + return_dn.insert() + return_dn.submit() + + # the return reverses the delivery quantities + so.load_from_db() + self.assertEqual(so.per_delivered, 0) + self.assertEqual(so.items[0].delivered_qty, 0) + + credit_note = make_sales_invoice(return_dn.name) + self.assertTrue(credit_note.is_return) + self.assertTrue(credit_note.update_billed_amount_in_sales_order) + # A Delivery Note-linked invoice can't update stock (validate_delivery_note), so the + # credit note only rolls back billing and never re-reverses the delivery quantities. + self.assertFalse(credit_note.update_stock) + credit_note.insert() + credit_note.submit() + + # per_billed is reset, and the delivery state stays exactly as the return left it + so.load_from_db() + self.assertEqual(so.per_billed, 0) + self.assertEqual(so.per_delivered, 0) + self.assertEqual(so.items[0].delivered_qty, 0) + self.assertEqual(so.items[0].returned_qty, 0) + + # Cancelling the credit note should restore the billed amount on the Sales Order. + credit_note.cancel() + so.load_from_db() + self.assertEqual(so.per_billed, 100) + + def test_sales_order_per_billed_after_credit_note_from_return_dn(self): + # Reported flow: SO -> SI (from SO) -> DN (from SI) -> return DN -> credit note. + # The DN carries si_detail in this path. + from erpnext.accounts.doctype.sales_invoice.sales_invoice import make_delivery_note + from erpnext.selling.doctype.sales_order.sales_order import make_sales_invoice as make_si_from_so + + make_stock_entry(item_code="_Test Item", target="_Test Warehouse - _TC", qty=10, basic_rate=100) + + so = make_sales_order(qty=2) + + si = make_si_from_so(so.name) + si.insert() + si.submit() + + dn = make_delivery_note(si.name) + dn.insert() + dn.submit() + + self._assert_credit_note_from_return_dn_resets_per_billed(so, dn) + + def test_sales_order_per_billed_after_credit_note_from_so_derived_dn(self): + # SO billed and delivered separately (SO -> SI, SO -> DN), then return DN -> credit note. + # SO per_billed rolls back via the status_updater in update_prevdoc_status. + from erpnext.selling.doctype.sales_order.sales_order import ( + make_delivery_note as make_dn_from_so, + ) + from erpnext.selling.doctype.sales_order.sales_order import ( + make_sales_invoice as make_si_from_so, + ) + + make_stock_entry(item_code="_Test Item", target="_Test Warehouse - _TC", qty=10, basic_rate=100) + + so = make_sales_order(qty=2) + + si = make_si_from_so(so.name) + si.insert() + si.submit() + + dn = make_dn_from_so(so.name) + dn.insert() + dn.submit() + + self.assertIsNone(dn.items[0].si_detail) + + self._assert_credit_note_from_return_dn_resets_per_billed(so, dn) + def test_sales_return_for_product_bundle(self): from erpnext.selling.doctype.product_bundle.test_product_bundle import make_product_bundle from erpnext.stock.doctype.delivery_note.delivery_note import make_sales_return diff --git a/erpnext/stock/doctype/item/item.json b/erpnext/stock/doctype/item/item.json index 10c771d1415..e8280bacf4a 100644 --- a/erpnext/stock/doctype/item/item.json +++ b/erpnext/stock/doctype/item/item.json @@ -145,6 +145,7 @@ "ignore_user_permissions": 1, "in_standard_filter": 1, "label": "Variant Of", + "link_filters": "[[\"Item\",\"has_variants\",\"=\",1]]", "options": "Item", "read_only": 1, "search_index": 1, @@ -897,7 +898,7 @@ "image_field": "image", "links": [], "make_attachments_public": 1, - "modified": "2026-03-17 20:39:05.218344", + "modified": "2026-07-05 23:24:45.734144", "modified_by": "Administrator", "module": "Stock", "name": "Item", diff --git a/erpnext/stock/doctype/item/item.py b/erpnext/stock/doctype/item/item.py index 05e8a2f3779..7f077cfd4dd 100644 --- a/erpnext/stock/doctype/item/item.py +++ b/erpnext/stock/doctype/item/item.py @@ -217,6 +217,7 @@ class Item(Document): self.validate_item_defaults() self.validate_auto_reorder_enabled_in_stock_settings() self.cant_change() + self.validate_serialized_change_with_bundle() self.validate_item_tax_net_rate_range() if not self.is_new(): @@ -1074,6 +1075,25 @@ class Item(Document): frappe.throw(msg, title=_("Linked with submitted documents")) + def validate_serialized_change_with_bundle(self): + """Block turning a serialized item non-serialized while any Serial and Batch Bundle still exists + for it. Such bundles carry the item's serial numbers; the user must delete or cancel them first.""" + if self.is_new() or self.has_serial_no or not self._doc_before_save: + return + + # Only relevant when the item was serialized before and is now being unset. + if not self._doc_before_save.has_serial_no: + return + + # Draft (docstatus 0) or submitted (docstatus 1) bundles block the change; cancelled ones don't. + if frappe.db.count("Serial and Batch Bundle", {"item_code": self.name, "docstatus": ("<", 2)}): + frappe.throw( + _( + "Cannot change Item {0} from serialized to non-serialized because a Serial and Batch Bundle exists for it. Please delete or cancel the Serial and Batch Bundle first." + ).format(frappe.bold(self.name)), + title=_("Serial and Batch Bundle Exists"), + ) + def _get_linked_submitted_documents(self, changed_fields: list[str]) -> dict[str, str] | None: linked_doctypes = [ "Delivery Note Item", diff --git a/erpnext/stock/doctype/item/test_item.py b/erpnext/stock/doctype/item/test_item.py index 8072437a173..073c8c8be93 100644 --- a/erpnext/stock/doctype/item/test_item.py +++ b/erpnext/stock/doctype/item/test_item.py @@ -443,6 +443,100 @@ class TestItem(FrappeTestCase): "Large", ) + def test_rename_attribute_abbr_updates_variant_item_code(self): + frappe.delete_doc_if_exists("Item", "_Test Variant Item-L", force=1) + frappe.delete_doc_if_exists("Item", "_Test Variant Item-LRG", force=1) + + variant = create_variant("_Test Variant Item", {"Test Size": "Large"}) + variant.save() + + attribute = frappe.get_doc("Item Attribute", "Test Size") + for row in attribute.item_attribute_values: + if row.attribute_value == "Large": + row.abbr = "LRG" + break + + def restore_test_size_abbr(): + doc = frappe.get_doc("Item Attribute", "Test Size") + for row in doc.item_attribute_values: + if row.attribute_value == "Large": + row.abbr = "L" + break + frappe.flags.attribute_values = None + doc.save() + + self.addCleanup(restore_test_size_abbr) + self.addCleanup(lambda: frappe.delete_doc_if_exists("Item", "_Test Variant Item-LRG", force=1)) + + frappe.flags.attribute_values = None + attribute.save() + + self.assertFalse(frappe.db.exists("Item", "_Test Variant Item-L")) + self.assertTrue(frappe.db.exists("Item", "_Test Variant Item-LRG")) + self.assertEqual( + frappe.db.get_value("Item", "_Test Variant Item-LRG", "item_name"), + "_Test Variant Item-LRG", + ) + + def test_rename_attribute_abbr_updates_variant_item_name_from_template_name(self): + # item_name can be derived from the template's item_name, which may differ from its + # item_code (e.g. a friendly display name vs. a SKU-style code). The variant's item_name + # must follow the abbreviation rename the same way item_code does. + frappe.delete_doc_if_exists("Item", "_Test Variant Item Diff-L", force=1) + frappe.delete_doc_if_exists("Item", "_Test Variant Item Diff-LRG", force=1) + frappe.delete_doc_if_exists("Item", "_Test Variant Item Diff", force=1) + + template = frappe.get_doc("Item", "_Test Variant Item").as_dict() + template = frappe.get_doc( + { + "doctype": "Item", + "item_code": "_Test Variant Item Diff", + "item_name": "Test Variant Friendly Name", + "item_group": template.item_group, + "stock_uom": template.stock_uom, + "has_variants": 1, + "attributes": [{"attribute": "Test Size"}], + } + ) + template.insert() + self.addCleanup(lambda: frappe.delete_doc_if_exists("Item", "_Test Variant Item Diff", force=1)) + + variant = create_variant("_Test Variant Item Diff", {"Test Size": "Large"}) + variant.save() + self.assertEqual(variant.item_code, "_Test Variant Item Diff-L") + self.assertEqual(variant.item_name, "Test Variant Friendly Name-L") + + # even a manually customized item_name (unrelated to the auto-generated pattern) must be + # rebuilt on abbreviation rename, since item_code and item_name are meant to stay in lockstep. + frappe.db.set_value("Item", variant.name, "item_name", "Custom Friendly Large Shirt Name") + + attribute = frappe.get_doc("Item Attribute", "Test Size") + for row in attribute.item_attribute_values: + if row.attribute_value == "Large": + row.abbr = "LRG" + break + + def restore_test_size_abbr(): + doc = frappe.get_doc("Item Attribute", "Test Size") + for row in doc.item_attribute_values: + if row.attribute_value == "Large": + row.abbr = "L" + break + frappe.flags.attribute_values = None + doc.save() + + self.addCleanup(restore_test_size_abbr) + self.addCleanup(lambda: frappe.delete_doc_if_exists("Item", "_Test Variant Item Diff-LRG", force=1)) + + frappe.flags.attribute_values = None + attribute.save() + + self.assertFalse(frappe.db.exists("Item", "_Test Variant Item Diff-L")) + self.assertEqual( + frappe.db.get_value("Item", "_Test Variant Item Diff-LRG", "item_name"), + "Test Variant Friendly Name-LRG", + ) + def test_make_item_variant(self): frappe.delete_doc_if_exists("Item", "_Test Variant Item-L", force=1) @@ -966,6 +1060,47 @@ class TestItem(FrappeTestCase): self.assertRaises(frappe.ValidationError, item_doc.save) + def test_cannot_unset_serialized_while_bundle_exists(self): + from erpnext.stock.doctype.serial_and_batch_bundle.test_serial_and_batch_bundle import ( + make_serial_batch_bundle, + ) + + item = make_item( + properties={"has_serial_no": 1, "is_stock_item": 1, "serial_no_series": "TSN-UNSET-.####"} + ).name + + serial_no = f"{item}-SN-01" + frappe.get_doc( + {"doctype": "Serial No", "serial_no": serial_no, "item_code": item, "company": "_Test Company"} + ).insert() + + # A draft (unsubmitted) Serial and Batch Bundle for the item must block the change. + bundle = make_serial_batch_bundle( + { + "item_code": item, + "warehouse": "_Test Warehouse - _TC", + "company": "_Test Company", + "qty": 1, + "rate": 100, + "voucher_type": "Stock Entry", + "serial_nos": [serial_no], + "type_of_transaction": "Inward", + "do_not_submit": True, + "ignore_sabb_validation": True, + } + ) + + doc = frappe.get_doc("Item", item) + doc.has_serial_no = 0 + self.assertRaises(frappe.ValidationError, doc.save) + + # Once the bundle is removed, the item can be made non-serialized. + frappe.delete_doc("Serial and Batch Bundle", bundle.name, force=True) + doc = frappe.get_doc("Item", item) + doc.has_serial_no = 0 + doc.save() + self.assertEqual(frappe.db.get_value("Item", item, "has_serial_no"), 0) + def set_item_variant_settings(fields): doc = frappe.get_doc("Item Variant Settings") diff --git a/erpnext/stock/doctype/item_attribute/item_attribute.py b/erpnext/stock/doctype/item_attribute/item_attribute.py index 14d2c6a4f12..09e1d56ffdd 100644 --- a/erpnext/stock/doctype/item_attribute/item_attribute.py +++ b/erpnext/stock/doctype/item_attribute/item_attribute.py @@ -10,6 +10,7 @@ from frappe.utils import flt from erpnext.controllers.item_variant import ( InvalidItemAttributeValueError, update_variant_attribute_values, + update_variant_item_codes_for_abbr_renames, validate_is_incremental, validate_item_attribute_value, ) @@ -49,6 +50,7 @@ class ItemAttribute(Document): def on_update(self): update_variant_attribute_values(self) + update_variant_item_codes_for_abbr_renames(self) self.validate_exising_items() self.set_enabled_disabled_in_items() diff --git a/erpnext/stock/doctype/pick_list/pick_list.json b/erpnext/stock/doctype/pick_list/pick_list.json index 4b46f4ecd82..5232d697ba0 100644 --- a/erpnext/stock/doctype/pick_list/pick_list.json +++ b/erpnext/stock/doctype/pick_list/pick_list.json @@ -184,7 +184,7 @@ "in_standard_filter": 1, "label": "Status", "no_copy": 1, - "options": "Draft\nOpen\nPartly Delivered\nCompleted\nCancelled", + "options": "Draft\nOpen\nPartly Delivered\nPartially Transferred\nCompleted\nCancelled", "print_hide": 1, "read_only": 1, "report_hide": 1, @@ -246,7 +246,7 @@ ], "is_submittable": 1, "links": [], - "modified": "2025-10-03 18:36:52.282355", + "modified": "2026-07-10 11:39:13.000000", "modified_by": "Administrator", "module": "Stock", "name": "Pick List", diff --git a/erpnext/stock/doctype/pick_list/pick_list.py b/erpnext/stock/doctype/pick_list/pick_list.py index 6d42f51a8d6..06e190c30f7 100644 --- a/erpnext/stock/doctype/pick_list/pick_list.py +++ b/erpnext/stock/doctype/pick_list/pick_list.py @@ -71,7 +71,9 @@ class PickList(TransactionBase): purpose: DF.Literal["Material Transfer for Manufacture", "Material Transfer", "Delivery"] scan_barcode: DF.Data | None scan_mode: DF.Check - status: DF.Literal["Draft", "Open", "Partly Delivered", "Completed", "Cancelled"] + status: DF.Literal[ + "Draft", "Open", "Partly Delivered", "Partially Transferred", "Completed", "Cancelled" + ] work_order: DF.Link | None # end: auto-generated types @@ -398,6 +400,34 @@ class PickList(TransactionBase): return stock_entry_exists(self.name) + def get_transfer_status(self): + """Return the pick list's transfer progress based on how much of the picked qty has been + moved into submitted Stock Entries (tracked on Pick List Item.transferred_qty). + + Only applies to purposes that move stock via Stock Entry; the Delivery purpose is tracked + via delivery_status instead. Returns "Completed", "Partially Transferred" or None.""" + if self.purpose == "Delivery": + return None + + total_picked = sum(flt(row.picked_qty) for row in self.locations) + if not total_picked: + return None + + total_transferred = sum(flt(row.transferred_qty) for row in self.locations) + if total_transferred <= 0: + return None + + if total_transferred >= total_picked: + return "Completed" + + return "Partially Transferred" + + def is_fully_transferred(self): + return self.get_transfer_status() == "Completed" + + def is_partially_transferred(self): + return self.get_transfer_status() == "Partially Transferred" + def update_reference_qty(self): packed_items = [] so_items = [] @@ -1415,6 +1445,9 @@ def map_pl_locations(pick_list, item_mapper, delivery_note, sales_order=None): if location.sales_order != sales_order or location.product_bundle_item: continue + if flt(location.picked_qty) - flt(location.delivered_qty) <= 0: + continue + if location.sales_order_item: sales_order_item = frappe.get_doc("Sales Order Item", location.sales_order_item) else: @@ -1470,13 +1503,10 @@ def add_product_bundles_to_delivery_note( @frappe.whitelist() -def create_stock_entry(pick_list): - pick_list = frappe.get_doc(json.loads(pick_list)) +def create_stock_entry(pick_list: str | dict): + pick_list = frappe.get_doc(frappe.parse_json(pick_list)) validate_item_locations(pick_list) - if stock_entry_exists(pick_list.get("name")): - return frappe.msgprint(_("Stock Entry has been already created against this Pick List")) - stock_entry = frappe.new_doc("Stock Entry") stock_entry.pick_list = pick_list.get("name") stock_entry.purpose = pick_list.get("purpose") @@ -1490,6 +1520,9 @@ def create_stock_entry(pick_list): else: stock_entry = update_stock_entry_items_with_no_reference(pick_list, stock_entry) + if not stock_entry.get("items"): + return frappe.msgprint(_("All picked items have already been transferred against this Pick List")) + stock_entry.set_missing_values() return stock_entry.as_dict() @@ -1591,6 +1624,8 @@ def update_stock_entry_based_on_work_order(pick_list, stock_entry): stock_entry.project = work_order.project for location in pick_list.locations: + if get_pending_transfer_stock_qty(location) <= 0: + continue item = frappe._dict() update_common_item_properties(item, location) item.t_warehouse = wip_warehouse @@ -1602,6 +1637,8 @@ def update_stock_entry_based_on_work_order(pick_list, stock_entry): def update_stock_entry_based_on_material_request(pick_list, stock_entry): for location in pick_list.locations: + if get_pending_transfer_stock_qty(location) <= 0: + continue target_warehouse = None if location.material_request_item: target_warehouse = frappe.get_value( @@ -1617,6 +1654,8 @@ def update_stock_entry_based_on_material_request(pick_list, stock_entry): def update_stock_entry_items_with_no_reference(pick_list, stock_entry): for location in pick_list.locations: + if get_pending_transfer_stock_qty(location) <= 0: + continue item = frappe._dict() update_common_item_properties(item, location) @@ -1625,11 +1664,18 @@ def update_stock_entry_items_with_no_reference(pick_list, stock_entry): return stock_entry +def get_pending_transfer_stock_qty(location): + """Stock qty of this pick list row still to be moved into a Stock Entry.""" + return flt(location.picked_qty) - flt(location.transferred_qty) + + def update_common_item_properties(item, location): + pending_stock_qty = get_pending_transfer_stock_qty(location) item.item_code = location.item_code + item.item_name = location.item_name item.s_warehouse = location.warehouse - item.transfer_qty = location.picked_qty - item.qty = flt(location.picked_qty / (location.conversion_factor or 1), location.precision("qty")) + item.transfer_qty = pending_stock_qty + item.qty = flt(pending_stock_qty / (location.conversion_factor or 1), location.precision("qty")) item.uom = location.uom item.conversion_factor = location.conversion_factor item.stock_uom = location.stock_uom @@ -1637,6 +1683,7 @@ def update_common_item_properties(item, location): item.serial_no = location.serial_no item.batch_no = location.batch_no item.material_request_item = location.material_request_item + item.pick_list_item = location.name def get_rejected_warehouses(): diff --git a/erpnext/stock/doctype/pick_list/pick_list_list.js b/erpnext/stock/doctype/pick_list/pick_list_list.js index a675c95f973..5bc4f2f3eef 100644 --- a/erpnext/stock/doctype/pick_list/pick_list_list.js +++ b/erpnext/stock/doctype/pick_list/pick_list_list.js @@ -7,6 +7,7 @@ frappe.listview_settings["Pick List"] = { Draft: "red", Open: "orange", "Partly Delivered": "orange", + "Partially Transferred": "yellow", Completed: "green", Cancelled: "red", }; diff --git a/erpnext/stock/doctype/pick_list/test_pick_list.py b/erpnext/stock/doctype/pick_list/test_pick_list.py index 83d1827794c..8ed07e61381 100644 --- a/erpnext/stock/doctype/pick_list/test_pick_list.py +++ b/erpnext/stock/doctype/pick_list/test_pick_list.py @@ -10,7 +10,11 @@ from erpnext.selling.doctype.sales_order.sales_order import create_pick_list from erpnext.selling.doctype.sales_order.test_sales_order import make_sales_order from erpnext.stock.doctype.item.test_item import create_item, make_item from erpnext.stock.doctype.packed_item.test_packed_item import create_product_bundle -from erpnext.stock.doctype.pick_list.pick_list import create_delivery_note, create_dn_for_pick_lists +from erpnext.stock.doctype.pick_list.pick_list import ( + create_delivery_note, + create_dn_for_pick_lists, + create_stock_entry, +) from erpnext.stock.doctype.purchase_receipt.test_purchase_receipt import make_purchase_receipt from erpnext.stock.doctype.serial_and_batch_bundle.test_serial_and_batch_bundle import ( get_batch_from_bundle, @@ -1016,6 +1020,103 @@ class TestPickList(FrappeTestCase): pl.reload() self.assertEqual(pl.status, "Cancelled") + def test_pick_list_partial_transfer_status(self): + """Partial Stock Entries from a Pick List should track transferred_qty and drive the + Partially Transferred / Completed status, and allow further transfers for the remainder.""" + from erpnext.stock.doctype.warehouse.test_warehouse import create_warehouse + + item = make_item(properties={"is_stock_item": 1}).name + source_warehouse = "_Test Warehouse - _TC" + target_warehouse = create_warehouse("_Test Transfer Target Warehouse") + make_stock_entry(item=item, to_warehouse=source_warehouse, qty=10) + + pick_list = frappe.get_doc( + { + "doctype": "Pick List", + "company": "_Test Company", + "purpose": "Material Transfer", + "pick_manually": 1, + "locations": [ + { + "item_code": item, + "qty": 10, + "stock_qty": 10, + "conversion_factor": 1, + "warehouse": source_warehouse, + "picked_qty": 10, + } + ], + } + ) + pick_list.submit() + self.assertEqual(pick_list.status, "Open") + + # Transfer 4 of the 10 picked units. + se1 = frappe.get_doc(create_stock_entry(pick_list.as_dict())) + self.assertEqual(se1.items[0].qty, 10) + se1.items[0].qty = 4 + se1.items[0].t_warehouse = target_warehouse + se1.submit() + + pick_list.reload() + self.assertEqual(pick_list.locations[0].transferred_qty, 4) + self.assertEqual(pick_list.status, "Partially Transferred") + + # The next Stock Entry should only offer the remaining 6 units. + se2 = frappe.get_doc(create_stock_entry(pick_list.as_dict())) + self.assertEqual(se2.items[0].qty, 6) + se2.items[0].t_warehouse = target_warehouse + se2.submit() + + pick_list.reload() + self.assertEqual(pick_list.locations[0].transferred_qty, 10) + self.assertEqual(pick_list.status, "Completed") + + # Cancelling the last entry rolls transferred_qty and status back. + se2.cancel() + pick_list.reload() + self.assertEqual(pick_list.locations[0].transferred_qty, 4) + self.assertEqual(pick_list.status, "Partially Transferred") + + def test_create_second_delivery_note_with_fully_delivered_location(self): + # When one pick list item is fully delivered by the first Delivery Note + # and another item is still pending, creating a second Delivery Note from + # the Pick List must not create a zero-qty row for the delivered item. + warehouse = "_Test Warehouse - _TC" + item_a = make_item(properties={"is_stock_item": 1}).name + item_b = make_item(properties={"is_stock_item": 1}).name + make_stock_entry(item=item_a, to_warehouse=warehouse, qty=20) + make_stock_entry(item=item_b, to_warehouse=warehouse, qty=20) + + so = make_sales_order( + item_list=[ + {"item_code": item_a, "warehouse": warehouse, "qty": 10, "rate": 100}, + {"item_code": item_b, "warehouse": warehouse, "qty": 5, "rate": 100}, + ] + ) + + pl = create_pick_list(so.name) + pl.save().submit() + + # First Delivery Note: fully deliver item_a, drop item_b. + dn1 = create_delivery_note(pl.name) + for row in list(dn1.items): + if row.item_code == item_b: + dn1.remove(row) + dn1.save().submit() + + pl.reload() + delivered = {loc.item_code: loc.delivered_qty for loc in pl.locations} + self.assertEqual(delivered[item_a], 10) + self.assertEqual(delivered[item_b], 0) + + # Second Delivery Note for the remaining item must succeed and must not + # include a zero-qty row for the already delivered item_a. + dn2 = create_delivery_note(pl.name) + self.assertEqual(len(dn2.items), 1) + self.assertEqual(dn2.items[0].item_code, item_b) + self.assertEqual(dn2.items[0].qty, 5) + def test_pick_list_validation(self): warehouse = "_Test Warehouse - _TC" item = make_item("Test Non Serialized Pick List Item", properties={"is_stock_item": 1}).name diff --git a/erpnext/stock/doctype/pick_list_item/pick_list_item.json b/erpnext/stock/doctype/pick_list_item/pick_list_item.json index adac858acac..1a0b69744c9 100644 --- a/erpnext/stock/doctype/pick_list_item/pick_list_item.json +++ b/erpnext/stock/doctype/pick_list_item/pick_list_item.json @@ -22,6 +22,7 @@ "conversion_factor", "stock_uom", "delivered_qty", + "transferred_qty", "available_quantity_section", "actual_qty", "column_break_kyek", @@ -254,6 +255,16 @@ "read_only": 1, "report_hide": 1 }, + { + "default": "0", + "fieldname": "transferred_qty", + "fieldtype": "Float", + "label": "Transferred Qty (in Stock UOM)", + "no_copy": 1, + "print_hide": 1, + "read_only": 1, + "report_hide": 1 + }, { "fieldname": "available_quantity_section", "fieldtype": "Section Break", @@ -284,7 +295,7 @@ ], "istable": 1, "links": [], - "modified": "2026-03-17 16:25:10.358013", + "modified": "2026-07-06 18:17:18.000000", "modified_by": "Administrator", "module": "Stock", "name": "Pick List Item", diff --git a/erpnext/stock/doctype/pick_list_item/pick_list_item.py b/erpnext/stock/doctype/pick_list_item/pick_list_item.py index bdba97f4056..97e6525c97b 100644 --- a/erpnext/stock/doctype/pick_list_item/pick_list_item.py +++ b/erpnext/stock/doctype/pick_list_item/pick_list_item.py @@ -39,6 +39,7 @@ class PickListItem(Document): stock_qty: DF.Float stock_reserved_qty: DF.Float stock_uom: DF.Link | None + transferred_qty: DF.Float uom: DF.Link | None use_serial_batch_fields: DF.Check warehouse: DF.Link | None diff --git a/erpnext/stock/doctype/purchase_receipt/test_purchase_receipt.py b/erpnext/stock/doctype/purchase_receipt/test_purchase_receipt.py index 3799e773a7d..edde28a04e6 100644 --- a/erpnext/stock/doctype/purchase_receipt/test_purchase_receipt.py +++ b/erpnext/stock/doctype/purchase_receipt/test_purchase_receipt.py @@ -1662,6 +1662,93 @@ class TestPurchaseReceipt(FrappeTestCase): self.assertEqual(query[0].value, 0) + def test_internal_transfer_pr_incoming_sle_anchored_to_dn_rate(self): + """Internal-transfer PR's inward SLE must use DN.incoming_rate even when + PR.item.valuation_rate was wrong at submit, so divisional_loss does not + leak to COGS.""" + from erpnext.stock.doctype.delivery_note.delivery_note import make_inter_company_purchase_receipt + from erpnext.stock.doctype.delivery_note.test_delivery_note import create_delivery_note + from erpnext.stock.stock_ledger import update_entries_after + + prepare_data_for_internal_transfer() + customer = "_Test Internal Customer 2" + company = "_Test Company with perpetual inventory" + + from_warehouse = create_warehouse("_Test Drift From", company=company) + transit_warehouse = create_warehouse("_Test Drift Transit", company=company) + to_warehouse = create_warehouse("_Test Drift Receiver", company=company) + item_doc = create_item("Test Internal Drift Item") + + make_purchase_receipt( + item_code=item_doc.name, + company=company, + posting_date=add_days(today(), -1), + warehouse=from_warehouse, + qty=10, + rate=100, + ) + + dn = create_delivery_note( + item_code=item_doc.name, + company=company, + customer=customer, + cost_center="Main - TCP1", + expense_account="Cost of Goods Sold - TCP1", + qty=1, + rate=100, + warehouse=from_warehouse, + target_warehouse=transit_warehouse, + ) + self.assertEqual(flt(dn.items[0].incoming_rate), 100.0) + + pr = make_inter_company_purchase_receipt(dn.name) + pr.items[0].warehouse = to_warehouse + pr.submit() + + inward_sle = frappe.db.get_value( + "Stock Ledger Entry", + { + "voucher_type": "Purchase Receipt", + "voucher_no": pr.name, + "warehouse": to_warehouse, + "is_cancelled": 0, + }, + ["name", "item_code", "warehouse", "posting_date", "posting_time", "creation", "incoming_rate"], + as_dict=True, + ) + self.assertEqual(flt(inward_sle.incoming_rate), 100.0) + + frappe.db.set_value( + "Purchase Receipt Item", + pr.items[0].name, + {"sales_incoming_rate": 0, "valuation_rate": 80}, + ) + frappe.db.set_value( + "Stock Ledger Entry", + inward_sle.name, + {"incoming_rate": 80, "stock_value_difference": 80}, + ) + + update_entries_after( + { + "item_code": inward_sle.item_code, + "warehouse": inward_sle.warehouse, + "posting_date": inward_sle.posting_date, + "posting_time": inward_sle.posting_time, + "sle_id": inward_sle.name, + "creation": inward_sle.creation, + } + ) + + refreshed = frappe.db.get_value( + "Stock Ledger Entry", + inward_sle.name, + ["incoming_rate", "stock_value_difference"], + as_dict=True, + ) + self.assertEqual(flt(refreshed.incoming_rate), 100.0) + self.assertEqual(flt(refreshed.stock_value_difference), 100.0) + def test_backdated_transaction_for_internal_transfer_in_trasit_warehouse_for_purchase_invoice( self, ): diff --git a/erpnext/stock/doctype/serial_and_batch_bundle/serial_and_batch_bundle.py b/erpnext/stock/doctype/serial_and_batch_bundle/serial_and_batch_bundle.py index 4fa630fb8a8..e3428c98add 100644 --- a/erpnext/stock/doctype/serial_and_batch_bundle/serial_and_batch_bundle.py +++ b/erpnext/stock/doctype/serial_and_batch_bundle/serial_and_batch_bundle.py @@ -26,6 +26,7 @@ from frappe.utils import ( ) from frappe.utils.csvutils import build_csv_response +from erpnext.stock.doctype.purchase_receipt_item.purchase_receipt_item import PurchaseReceiptItem from erpnext.stock.serial_batch_bundle import ( BatchNoValuation, SerialNoValuation, @@ -2092,9 +2093,14 @@ def get_reference_serial_and_batch_bundle(child_row): @frappe.whitelist() -def add_serial_batch_ledgers(entries, child_row, doc, warehouse, do_not_save=False) -> object: - if isinstance(child_row, str): - child_row = frappe._dict(parse_json(child_row)) +def add_serial_batch_ledgers( + entries: list | str, + child_row: PurchaseReceiptItem | dict | str, + doc: Document | dict | str, + warehouse: str | None = None, + do_not_save: bool = False, +): + child_row = parse_json(child_row) if isinstance(entries, str): entries = parse_json(entries) @@ -2126,7 +2132,9 @@ def create_serial_batch_no_ledgers( if parent_doc.get("doctype") == "Stock Entry": warehouse = warehouse or child_row.s_warehouse or child_row.t_warehouse - posting_datetime = combine_datetime(parent_doc.get("posting_date"), parent_doc.get("posting_time")) + posting_datetime = combine_datetime( + parent_doc.get("posting_date") or today(), parent_doc.get("posting_time") or nowtime() + ) doc = frappe.get_doc( { @@ -2243,7 +2251,9 @@ def update_serial_batch_no_ledgers(bundle, entries, child_row, parent_doc, wareh ) doc.voucher_detail_no = child_row.name - doc.posting_datetime = combine_datetime(parent_doc.get("posting_date"), parent_doc.get("posting_time")) + doc.posting_datetime = combine_datetime( + parent_doc.get("posting_date") or today(), parent_doc.get("posting_time") or nowtime() + ) doc.warehouse = warehouse or doc.warehouse doc.set("entries", []) diff --git a/erpnext/stock/doctype/stock_entry/stock_entry.py b/erpnext/stock/doctype/stock_entry/stock_entry.py index 3aea5271d3d..ebfa7269912 100644 --- a/erpnext/stock/doctype/stock_entry/stock_entry.py +++ b/erpnext/stock/doctype/stock_entry/stock_entry.py @@ -163,6 +163,15 @@ class StockEntry(StockController): def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) + self.status_updater = [ + { + "source_dt": "Stock Entry Detail", + "target_dt": "Pick List Item", + "join_field": "pick_list_item", + "target_field": "transferred_qty", + "source_field": "transfer_qty", + } + ] if self.purchase_order: self.subcontract_data = frappe._dict( { @@ -519,6 +528,7 @@ class StockEntry(StockController): self.validate_closed_subcontracting_order() self.update_subcontract_order_supplied_items() self.update_subcontracting_order_status() + self.update_pick_list_status() if self.work_order and self.purpose == "Material Consumption for Manufacture": self.validate_work_order_status() @@ -3544,6 +3554,9 @@ class StockEntry(StockController): def update_pick_list_status(self): from erpnext.stock.doctype.pick_list.pick_list import update_pick_list_status + if self.pick_list: + self.update_qty() + update_pick_list_status(self.pick_list) def set_missing_values(self): diff --git a/erpnext/stock/doctype/stock_entry_detail/stock_entry_detail.json b/erpnext/stock/doctype/stock_entry_detail/stock_entry_detail.json index 4e422e320b9..0153c8ef217 100644 --- a/erpnext/stock/doctype/stock_entry_detail/stock_entry_detail.json +++ b/erpnext/stock/doctype/stock_entry_detail/stock_entry_detail.json @@ -68,6 +68,7 @@ "col_break6", "material_request", "material_request_item", + "pick_list_item", "original_item", "reference_section", "against_stock_entry", @@ -337,7 +338,6 @@ "print_hide": 1 }, { - "default": ":Company", "depends_on": "eval:cint(erpnext.is_perpetual_inventory_enabled(parent.company))", "fieldname": "cost_center", "fieldtype": "Link", @@ -416,6 +416,16 @@ "print_hide": 1, "read_only": 1 }, + { + "fieldname": "pick_list_item", + "fieldtype": "Link", + "hidden": 1, + "label": "Pick List Item", + "no_copy": 1, + "options": "Pick List Item", + "print_hide": 1, + "read_only": 1 + }, { "fieldname": "original_item", "fieldtype": "Link", @@ -616,7 +626,7 @@ "index_web_pages_for_search": 1, "istable": 1, "links": [], - "modified": "2026-04-27 11:40:38.294196", + "modified": "2026-07-06 18:17:18.000000", "modified_by": "Administrator", "module": "Stock", "name": "Stock Entry Detail", diff --git a/erpnext/stock/doctype/stock_entry_detail/stock_entry_detail.py b/erpnext/stock/doctype/stock_entry_detail/stock_entry_detail.py index bd3dda1b98f..f5b61eed0db 100644 --- a/erpnext/stock/doctype/stock_entry_detail/stock_entry_detail.py +++ b/erpnext/stock/doctype/stock_entry_detail/stock_entry_detail.py @@ -43,6 +43,7 @@ class StockEntryDetail(Document): parent: DF.Data parentfield: DF.Data parenttype: DF.Data + pick_list_item: DF.Link | None po_detail: DF.Data | None project: DF.Link | None putaway_rule: DF.Link | None diff --git a/erpnext/stock/get_item_details.py b/erpnext/stock/get_item_details.py index 8c205a10874..fe41802fdf0 100644 --- a/erpnext/stock/get_item_details.py +++ b/erpnext/stock/get_item_details.py @@ -12,6 +12,7 @@ from frappe.model.utils import get_fetch_values from frappe.query_builder.functions import IfNull, Sum from frappe.utils import add_days, add_months, cint, cstr, flt, get_link_to_form, getdate, parse_json +import erpnext from erpnext import get_company_currency from erpnext.accounts.doctype.pricing_rule.pricing_rule import ( get_pricing_rule_for_item, @@ -410,12 +411,26 @@ def get_basic_details(args, item, overwrite_warehouse=True): expense_account = None - if args.get("doctype") == "Purchase Invoice" and item.is_fixed_asset: - from erpnext.assets.doctype.asset_category.asset_category import get_asset_category_account + if item.is_fixed_asset: + from erpnext.assets.doctype.asset.asset import get_asset_account, is_cwip_accounting_enabled - expense_account = get_asset_category_account( - fieldname="fixed_asset_account", item=args.item_code, company=args.company - ) + if is_cwip_accounting_enabled(item.asset_category): + expense_account = get_asset_account( + "capital_work_in_progress_account", + asset_category=item.asset_category, + company=args.company, + ) + elif args.get("doctype") in ( + "Purchase Invoice", + "Purchase Receipt", + "Purchase Order", + "Material Request", + ): + from erpnext.assets.doctype.asset_category.asset_category import get_asset_category_account + + expense_account = get_asset_category_account( + fieldname="fixed_asset_account", item=args.item_code, company=args.company + ) # Set the UOM to the Default Sales UOM or Default Purchase UOM if configured in the Item Master if not args.get("uom"): @@ -518,10 +533,21 @@ def get_basic_details(args, item, overwrite_warehouse=True): args.name, args.conversion_rate, item.name, out.conversion_factor ) + expense_account_field = "default_expense_account" + if ( + item.is_stock_item + and erpnext.is_perpetual_inventory_enabled(args.company) + and ( + args.doctype == "Purchase Receipt" + or (args.doctype == "Purchase Invoice" and args.get("update_stock")) + ) + ): + expense_account_field = "stock_received_but_not_billed" + # if default specified in item is for another company, fetch from company for d in [ ["Account", "income_account", "default_income_account"], - ["Account", "expense_account", "default_expense_account"], + ["Account", "expense_account", expense_account_field], ["Cost Center", "cost_center", "cost_center"], ["Warehouse", "warehouse", ""], ]: @@ -1492,6 +1518,11 @@ def apply_price_list(args, as_doc=False, doc=None): def apply_price_list_on_item(args, doc=None): item_doc = frappe.db.get_value("Item", args.item_code, ["name", "variant_of"], as_dict=1) item_details = get_price_list_rate(args, item_doc) + + args.conversion_factor = flt(args.conversion_factor) or get_conversion_factor( + args.item_code, args.uom + ).get("conversion_factor", 1) + args.stock_qty = flt(args.qty) * flt(args.conversion_factor) item_details.update(get_pricing_rule_for_item(args, doc=doc)) return item_details diff --git a/erpnext/stock/reorder_item.py b/erpnext/stock/reorder_item.py index 1f527e7071a..e3d60f0d0d3 100644 --- a/erpnext/stock/reorder_item.py +++ b/erpnext/stock/reorder_item.py @@ -186,6 +186,10 @@ def get_item_warehouse_projected_qty(items_to_consider): item_warehouse_projected_qty = {} items_to_consider = list(items_to_consider.keys()) + warehouse_parent_map = frappe._dict( + frappe.get_all("Warehouse", fields=["name", "parent_warehouse"], as_list=True) + ) + for item_code, warehouse, projected_qty in frappe.db.sql( """select item_code, warehouse, projected_qty from tabBin where item_code in ({}) @@ -200,16 +204,14 @@ def get_item_warehouse_projected_qty(items_to_consider): if warehouse not in item_warehouse_projected_qty.get(item_code): item_warehouse_projected_qty[item_code][warehouse] = flt(projected_qty) - warehouse_doc = frappe.get_doc("Warehouse", warehouse) + parent_warehouse = warehouse_parent_map.get(warehouse) - while warehouse_doc.parent_warehouse: - if not item_warehouse_projected_qty.get(item_code, {}).get(warehouse_doc.parent_warehouse): - item_warehouse_projected_qty.setdefault(item_code, {})[warehouse_doc.parent_warehouse] = flt( - projected_qty - ) + while parent_warehouse: + if not item_warehouse_projected_qty.get(item_code, {}).get(parent_warehouse): + item_warehouse_projected_qty.setdefault(item_code, {})[parent_warehouse] = flt(projected_qty) else: - item_warehouse_projected_qty[item_code][warehouse_doc.parent_warehouse] += flt(projected_qty) - warehouse_doc = frappe.get_doc("Warehouse", warehouse_doc.parent_warehouse) + item_warehouse_projected_qty[item_code][parent_warehouse] += flt(projected_qty) + parent_warehouse = warehouse_parent_map.get(parent_warehouse) return item_warehouse_projected_qty diff --git a/erpnext/stock/report/stock_ageing/stock_ageing.py b/erpnext/stock/report/stock_ageing/stock_ageing.py index 2e11fa1664b..e0106f7ddb0 100644 --- a/erpnext/stock/report/stock_ageing/stock_ageing.py +++ b/erpnext/stock/report/stock_ageing/stock_ageing.py @@ -287,6 +287,7 @@ class FIFOSlots: self.serial_no_details = {} self.batch_no_details = {} self.batchwise_valuation_by_batch = {} + self.valuation_method_by_item = {} self.filters = filters self.sle = sle @@ -307,9 +308,10 @@ class FIFOSlots: self.prepare_stock_reco_voucher_wise_count() if stock_ledger_entries is None: - # nested queries invalidate the streaming cursor below, - # so batchwise valuation flags must be resolved beforehand + # streaming path: nested queries invalidate the streaming cursor below, + # so batchwise valuation flags and item valuation methods must be resolved beforehand self._prefetch_batchwise_valuations() + self._prefetch_valuation_methods() with frappe.db.unbuffered_cursor(): if stock_ledger_entries is None: @@ -321,12 +323,28 @@ class FIFOSlots: # Note that stock_ledger_entries is an iterator, you can not reuse it like a list del stock_ledger_entries + self._recompute_moving_average_slots() + if not self.filters.get("show_warehouse_wise_stock"): # (Item 1, WH 1), (Item 1, WH 2) => (Item 1) self.item_details = self._aggregate_details_by_item(self.item_details) return self.item_details + def _recompute_moving_average_slots(self) -> None: + for item_dict in self.item_details.values(): + if item_dict.get("has_serial_no") or item_dict.get("has_batch_no"): + continue + + details = item_dict["details"] + if self._get_item_valuation_method(details.name) != "Moving Average": + continue + + rate = flt(details.valuation_rate) + for slot in item_dict["fifo_queue"]: + if is_qty_slot(slot): + slot[FIFO_VALUE_INDEX] = flt(slot[FIFO_QTY_INDEX] * rate) + def _get_bundle_wise_details(self, stock_ledger_entries: list | None) -> tuple[dict, dict]: if stock_ledger_entries is not None: return frappe._dict({}), frappe._dict({}) @@ -347,7 +365,10 @@ class FIFOSlots: if row.actual_qty > 0: self._compute_incoming_stock(row, fifo_queue, transferred_item_key, serial_nos, batch_nos) else: - self._compute_outgoing_stock(row, fifo_queue, transferred_item_key, serial_nos, batch_nos) + from_end = self._get_item_valuation_method(row.name) == "LIFO" + self._compute_outgoing_stock( + row, fifo_queue, transferred_item_key, serial_nos, batch_nos, from_end + ) self._update_balances(row, key) self._trim_serial_fifo_queue(row, key, fifo_queue) @@ -460,6 +481,43 @@ class FIFOSlots: for batch_no, use_batchwise_valuation in query.run(): self.batchwise_valuation_by_batch[batch_no] = use_batchwise_valuation + def _get_item_valuation_method(self, item_code: str) -> str: + from erpnext.stock.utils import get_valuation_method + + if item_code not in self.valuation_method_by_item: + # only reachable when stock ledger entries are passed in directly; + # the streaming path prefetches all methods before iteration + self.valuation_method_by_item[item_code] = get_valuation_method(item_code) + + return self.valuation_method_by_item[item_code] + + def _prefetch_valuation_methods(self) -> None: + from erpnext.stock.utils import get_valuation_method + + company = self.filters.get("company") + sle = frappe.qb.DocType("Stock Ledger Entry") + item = frappe.qb.DocType("Item") + to_date = get_datetime(self.filters.get("to_date") + " 23:59:59") + + query = ( + frappe.qb.from_(sle) + .inner_join(item) + .on(sle.item_code == item.name) + .select(item.name, item.valuation_method) + .distinct() + .where((sle.company == company) & (sle.posting_datetime <= to_date) & (sle.is_cancelled != 1)) + ) + query = self._apply_filter(query, sle, "item_code") + + # items with no item-level method share the company/settings default; resolve it once + default_method = None + for item_code, valuation_method in query.run(): + if not valuation_method: + if default_method is None: + default_method = get_valuation_method(item_code) + valuation_method = default_method + self.valuation_method_by_item[item_code] = valuation_method + def _init_key_stores(self, row: dict) -> tuple: "Initialise keys and FIFO Queue." @@ -492,7 +550,7 @@ class FIFOSlots: self._add_serial_fifo_slots(row, fifo_queue, serial_nos) elif batch_nos and row.get("has_batch_no"): self._add_batch_fifo_slots(row, fifo_queue, batch_nos) - elif fifo_queue and flt(fifo_queue[0][FIFO_QTY_INDEX]) <= 0: + elif fifo_queue and is_qty_slot(fifo_queue[0]) and flt(fifo_queue[0][FIFO_QTY_INDEX]) <= 0: self._add_to_negative_fifo_head(row, fifo_queue) else: fifo_queue.append([flt(row.actual_qty), row.posting_date, flt(row.stock_value_difference)]) @@ -576,7 +634,13 @@ class FIFOSlots: fifo_queue[0][FIFO_VALUE_INDEX] += flt(row.stock_value_difference) def _compute_outgoing_stock( - self, row: dict, fifo_queue: list, transfer_key: tuple, serial_nos: list, batch_nos: list + self, + row: dict, + fifo_queue: list, + transfer_key: tuple, + serial_nos: list, + batch_nos: list, + from_end: bool = False, ): "Update FIFO Queue on outward stock." if serial_nos: @@ -584,7 +648,7 @@ class FIFOSlots: elif batch_nos: self._consume_batch_fifo_slots(row, fifo_queue, transfer_key, batch_nos) else: - self._consume_fifo_slots(row, fifo_queue, transfer_key) + self._consume_fifo_slots(row, fifo_queue, transfer_key, from_end) def _consume_serial_fifo_slots(self, fifo_queue: list, serial_nos: list) -> None: fifo_queue[:] = [slot for slot in fifo_queue if slot[FIFO_QTY_INDEX] not in serial_nos] @@ -661,19 +725,23 @@ class FIFOSlots: ) self.transferred_item_details[transfer_key].append([qty, row.posting_date, stock_value_difference]) - def _consume_fifo_slots(self, row: dict, fifo_queue: list, transfer_key: tuple) -> None: + def _consume_fifo_slots( + self, row: dict, fifo_queue: list, transfer_key: tuple, from_end: bool = False + ) -> None: + # LIFO consumes the most recent inward first, so pop from the tail instead of the head. + index = -1 if from_end else 0 qty_to_pop = abs(row.actual_qty) stock_value = abs(row.stock_value_difference) while qty_to_pop: - slot = fifo_queue[0] if fifo_queue else [0, None, 0] + slot = fifo_queue[index] if fifo_queue else [0, None, 0] slot_qty = flt(slot[FIFO_QTY_INDEX]) slot_value = flt(slot[FIFO_VALUE_INDEX]) if 0 < slot_qty <= qty_to_pop: qty_to_pop -= slot_qty stock_value -= slot_value - self.transferred_item_details[transfer_key].append(fifo_queue.pop(0)) + self.transferred_item_details[transfer_key].append(fifo_queue.pop(index)) elif not fifo_queue: fifo_queue.append([-(qty_to_pop), row.posting_date, -(stock_value)]) self.transferred_item_details[transfer_key].append( diff --git a/erpnext/stock/report/stock_ageing/test_stock_ageing.py b/erpnext/stock/report/stock_ageing/test_stock_ageing.py index 003d1a51d93..2f74e1e3327 100644 --- a/erpnext/stock/report/stock_ageing/test_stock_ageing.py +++ b/erpnext/stock/report/stock_ageing/test_stock_ageing.py @@ -1,6 +1,8 @@ # Copyright (c) 2022, Frappe Technologies Pvt. Ltd. and Contributors # See license.txt +from unittest.mock import patch + import frappe from frappe.tests.utils import FrappeTestCase @@ -67,6 +69,131 @@ class TestStockAgeing(FrappeTestCase): data = format_report_data(self.filters, slots, self.filters["to_date"]) self.assertEqual(data[0][8], 40.0) # valuating for stock value between age 0-30 + def test_moving_average_value_ties_to_stock_balance(self): + """For Moving Average items the queue value is re-derived as qty * rate so the + report's stock value ties to Stock Balance, instead of stranding a residual + from FIFO-by-qty consumption vs blended outgoing value.""" + sle = [ + frappe._dict( + name="MA Item", + actual_qty=10, + qty_after_transaction=10, + stock_value_difference=1000, + valuation_rate=100, + warehouse="WH 1", + posting_date="2021-12-01", + voucher_type="Stock Entry", + voucher_no="001", + has_serial_no=False, + serial_no=None, + ), + frappe._dict( + name="MA Item", + actual_qty=10, + qty_after_transaction=20, + stock_value_difference=2000, + valuation_rate=150, + warehouse="WH 1", + posting_date="2021-12-02", + voucher_type="Stock Entry", + voucher_no="002", + has_serial_no=False, + serial_no=None, + ), + frappe._dict( + name="MA Item", + actual_qty=(-10), + qty_after_transaction=10, + stock_value_difference=(-1500), + valuation_rate=150, + warehouse="WH 1", + posting_date="2021-12-03", + voucher_type="Stock Entry", + voucher_no="003", + has_serial_no=False, + serial_no=None, + ), + frappe._dict( + name="MA Item", + actual_qty=(-5), + qty_after_transaction=5, + stock_value_difference=(-750), + valuation_rate=150, + warehouse="WH 1", + posting_date="2021-12-04", + voucher_type="Stock Entry", + voucher_no="004", + has_serial_no=False, + serial_no=None, + ), + ] + + with patch("erpnext.stock.utils.get_valuation_method", return_value="Moving Average"): + slots = FIFOSlots(self.filters, sle).generate() + + queue = slots["MA Item"]["fifo_queue"] + total_value = sum(slot[2] for slot in queue) + + # Stock Balance bal_val = qty_after_transaction * valuation_rate = 5 * 150 + self.assertEqual(total_value, 750.0) + + def test_lifo_consumes_newest_first(self): + """LIFO items consume the most recent inward first, so the oldest lot stays on + hand. The remaining queue, stock value and average age must reflect the older + stock, unlike the default FIFO which retains the newest lots.""" + sle = [ + frappe._dict( + name="LIFO Item", + actual_qty=30, + qty_after_transaction=30, + stock_value_difference=30, + warehouse="WH 1", + posting_date="2021-12-01", + voucher_type="Stock Entry", + voucher_no="001", + has_serial_no=False, + serial_no=None, + ), + frappe._dict( + name="LIFO Item", + actual_qty=20, + qty_after_transaction=50, + stock_value_difference=20, + warehouse="WH 1", + posting_date="2021-12-02", + voucher_type="Stock Entry", + voucher_no="002", + has_serial_no=False, + serial_no=None, + ), + frappe._dict( + name="LIFO Item", + actual_qty=(-10), + qty_after_transaction=40, + stock_value_difference=(-10), + warehouse="WH 1", + posting_date="2021-12-03", + voucher_type="Stock Entry", + voucher_no="003", + has_serial_no=False, + serial_no=None, + ), + ] + + with patch("erpnext.stock.utils.get_valuation_method", return_value="LIFO"): + slots = FIFOSlots(self.filters, sle).generate() + + queue = slots["LIFO Item"]["fifo_queue"] + + # newest lot (day 2) is consumed first: oldest 30 stays, newest drops 20 -> 10 + self.assertEqual(queue[0][0], 30.0) + self.assertEqual(queue[-1][0], 10.0) + self.assertEqual(sum(slot[0] for slot in queue), 40.0) + self.assertEqual(sum(slot[2] for slot in queue), 40.0) + + # average age skews older than the FIFO result (8.5) because the old lot is retained + self.assertEqual(get_average_age(queue, self.filters["to_date"]), 8.75) + def test_insufficient_balance(self): "Reference: Case 3 in stock_ageing_fifo_logic.md (same wh)" sle = [ @@ -1438,6 +1565,47 @@ class TestStockAgeing(FrappeTestCase): self.assertEqual(item_result["total_qty"], -4.0) self.assertEqual(item_result["fifo_queue"], [[batch_no, 1, -4.0, "2021-11-10", -40.0]]) + def test_untagged_receipt_with_negative_batch_head(self): + """An incoming SLE without batch details must not treat a negative + batch slot at the queue head as a qty slot (TypeError: str += float).""" + sle = [ + frappe._dict( + name="Enclosure Item", + actual_qty=-10, + qty_after_transaction=-10, + stock_value_difference=-100, + warehouse="WH 1", + posting_date="2021-12-01", + voucher_type="Stock Entry", + voucher_no="001", + has_serial_no=False, + has_batch_no=True, + serial_no=None, + batch_no="QI-06448", + ), + frappe._dict( + name="Enclosure Item", + actual_qty=45, + qty_after_transaction=35, + stock_value_difference=1051.65, + warehouse="WH 1", + posting_date="2021-12-05", + voucher_type="Purchase Receipt", + voucher_no="002", + has_serial_no=False, + serial_no=None, + batch_no=None, + serial_and_batch_bundle="SABB-00001294", + ), + ] + + slots = FIFOSlots(self.filters, sle).generate() + queue = slots["Enclosure Item"]["fifo_queue"] + + self.assertEqual(slots["Enclosure Item"]["total_qty"], 35.0) + self.assertEqual(queue[0], ["QI-06448", None, -10.0, "2021-12-01", -100.0]) + self.assertEqual(queue[1], [45.0, "2021-12-05", 1051.65]) + def test_batchwise_valuation_stock_reconciliation_with_bundle(self): from frappe.utils import add_days, getdate, nowdate diff --git a/erpnext/stock/report/stock_and_account_value_comparison/stock_and_account_value_comparison.py b/erpnext/stock/report/stock_and_account_value_comparison/stock_and_account_value_comparison.py index b83e46012cc..ec02318ce71 100644 --- a/erpnext/stock/report/stock_and_account_value_comparison/stock_and_account_value_comparison.py +++ b/erpnext/stock/report/stock_and_account_value_comparison/stock_and_account_value_comparison.py @@ -171,14 +171,20 @@ def get_columns(filters): @frappe.whitelist() -def create_reposting_entries(rows, company): +def create_reposting_entries(rows: str | list, company: str): if isinstance(rows, str): rows = parse_json(rows) entries = [] item_wh = frappe._dict() - vouchers = [row.get("voucher_no") for row in rows] + vouchers = [ + row.get("voucher_no") + for row in rows + if row.get("voucher_type") not in ["Purchase Receipt", "Purchase Invoice"] + ] + repost_based_on_transaction(rows, company, entries) + sles = get_stock_ledgers(vouchers) for sle in sles: key = (sle.item_code, sle.warehouse) @@ -211,3 +217,39 @@ def create_reposting_entries(rows, company): if entries: entries = ", ".join(entries) frappe.msgprint(_("Reposting entries created: {0}").format(entries)) + + +def repost_based_on_transaction(rows, company=None, entries=None): + if entries is None: + entries = [] + + duplicate_vouchers = set() + for row in rows: + if ( + row.get("voucher_type") == "Purchase Invoice" + and frappe.get_cached_value("Purchase Invoice", row.get("voucher_no"), "update_stock") == 0 + ): + continue + + if row.get("voucher_type") in ["Purchase Receipt", "Purchase Invoice"]: + voucher_key = (row.get("voucher_type"), row.get("voucher_no")) + if voucher_key in duplicate_vouchers: + continue + + duplicate_vouchers.add(voucher_key) + doc = frappe.get_doc( + { + "doctype": "Repost Item Valuation", + "based_on": "Transaction", + "status": "Queued", + "voucher_type": row.get("voucher_type"), + "voucher_no": row.get("voucher_no"), + "posting_date": row.get("posting_date"), + "posting_time": row.get("posting_time"), + "company": company, + "allow_nagative_stock": 1, + "recalculate_valuation_rate": 1, + } + ).submit() + + entries.append(get_link_to_form("Repost Item Valuation", doc.name)) diff --git a/erpnext/stock/report/stock_and_account_value_comparison/test_stock_and_account_value_comparison.py b/erpnext/stock/report/stock_and_account_value_comparison/test_stock_and_account_value_comparison.py new file mode 100644 index 00000000000..66120a56b79 --- /dev/null +++ b/erpnext/stock/report/stock_and_account_value_comparison/test_stock_and_account_value_comparison.py @@ -0,0 +1,57 @@ +# Copyright (c) 2026, Frappe Technologies Pvt. Ltd. and contributors +# For license information, please see license.txt + +import frappe +from frappe.tests.utils import FrappeTestCase +from frappe.utils import today + +from erpnext.stock.doctype.item.test_item import make_item +from erpnext.stock.doctype.purchase_receipt.test_purchase_receipt import make_purchase_receipt +from erpnext.stock.report.stock_and_account_value_comparison.stock_and_account_value_comparison import ( + create_reposting_entries, + execute, +) + +PI_COMPANY = "_Test Company with perpetual inventory" +PI_STORES = "Stores - TCP1" + + +class TestStockAndAccountValueComparison(FrappeTestCase): + def test_purchase_voucher_reposted_transaction_based(self): + # A Purchase Receipt whose GL entries are missing must surface in the report and, when reposted + # from it, be reposted Transaction-based (so its own GL is regenerated) rather than the slower + # Item-and-Warehouse based reposting. + item = make_item(properties={"is_stock_item": 1, "valuation_method": "FIFO"}).name + + pr = make_purchase_receipt(item_code=item, company=PI_COMPANY, warehouse=PI_STORES, qty=5, rate=100) + + # Simulate the out-of-sync state: stock ledger exists but the accounting ledger does not. + frappe.db.delete("GL Entry", {"voucher_type": "Purchase Receipt", "voucher_no": pr.name}) + + # The receipt now shows up in the comparison report (stock value 500 vs account value 0). + filters = frappe._dict(company=PI_COMPANY, as_on_date=today()) + _columns, data = execute(filters) + + row = next((d for d in data if d.get("voucher_no") == pr.name), None) + self.assertIsNotNone(row, "Out-of-sync Purchase Receipt should appear in the report") + self.assertEqual(row.get("voucher_type"), "Purchase Receipt") + + # Repost from the report. + create_reposting_entries([row], PI_COMPANY) + + # A Transaction-based Repost Item Valuation must have been created for this voucher... + transaction_rivs = frappe.get_all( + "Repost Item Valuation", + filters={"voucher_no": pr.name, "voucher_type": "Purchase Receipt"}, + fields=["name", "based_on"], + ) + + self.assertTrue(transaction_rivs, "Expected a Repost Item Valuation for the Purchase Receipt") + self.assertTrue(all(riv.based_on == "Transaction" for riv in transaction_rivs)) + + # ...and no Item-and-Warehouse based reposting should have been created for this item. + item_wh_rivs = frappe.get_all( + "Repost Item Valuation", + filters={"based_on": "Item and Warehouse", "item_code": item}, + ) + self.assertFalse(item_wh_rivs, "Purchase vouchers must not be reposted Item-and-Warehouse based") diff --git a/erpnext/stock/report/stock_ledger_invariant_check/stock_ledger_invariant_check.py b/erpnext/stock/report/stock_ledger_invariant_check/stock_ledger_invariant_check.py index 954acf998d8..aef9fec6414 100644 --- a/erpnext/stock/report/stock_ledger_invariant_check/stock_ledger_invariant_check.py +++ b/erpnext/stock/report/stock_ledger_invariant_check/stock_ledger_invariant_check.py @@ -20,6 +20,7 @@ SLE_FIELDS = ( "outgoing_rate", "stock_queue", "batch_no", + "serial_no", "stock_value", "stock_value_difference", "valuation_rate", @@ -52,16 +53,16 @@ def add_invariant_check_fields(sles, filters): balance_qty = 0.0 balance_stock_value = 0.0 - incorrect_idx = 0 - precision = frappe.get_precision("Stock Ledger Entry", "actual_qty") + incorrect_idx = None + float_precision = cint(frappe.db.get_single_value("System Settings", "float_precision")) or 3 + currency_precision = ( + cint(frappe.db.get_single_value("System Settings", "currency_precision")) or float_precision + ) for idx, sle in enumerate(sles): - queue = json.loads(sle.stock_queue) if sle.stock_queue else [] - - fifo_qty = 0.0 - fifo_value = 0.0 - for qty, rate in queue: - fifo_qty += qty - fifo_value += qty * rate + if sle.batch_no: + sle.use_batchwise_valuation = frappe.db.get_value( + "Batch", sle.batch_no, "use_batchwise_valuation", cache=True + ) if sle.actual_qty < 0: sle.consumption_rate = sle.stock_value_difference / sle.actual_qty @@ -77,57 +78,67 @@ def add_invariant_check_fields(sles, filters): if balance_qty is None: balance_qty = sle.qty_after_transaction - sle.fifo_queue_qty = fifo_qty - sle.fifo_stock_value = fifo_value - sle.fifo_valuation_rate = fifo_value / fifo_qty if fifo_qty else None sle.balance_value_by_qty = ( sle.stock_value / sle.qty_after_transaction if sle.qty_after_transaction else None ) sle.expected_qty_after_transaction = balance_qty sle.stock_value_from_diff = balance_stock_value - # set difference fields sle.difference_in_qty = sle.qty_after_transaction - sle.expected_qty_after_transaction - sle.fifo_qty_diff = sle.qty_after_transaction - fifo_qty - sle.fifo_value_diff = sle.stock_value - fifo_value - sle.fifo_valuation_diff = ( - sle.valuation_rate - sle.fifo_valuation_rate if sle.fifo_valuation_rate else None - ) sle.valuation_diff = ( sle.valuation_rate - sle.balance_value_by_qty if sle.balance_value_by_qty else None ) sle.diff_value_diff = sle.stock_value_from_diff - sle.stock_value - if not incorrect_idx and filters.get("show_incorrect_entries"): - if is_sle_has_correct_data(sle, precision): - continue - else: - incorrect_idx = idx + if maintains_fifo_queue(sle): + add_fifo_fields(sle, sles[idx - 1] if idx else None) - if idx > 0: - sle.fifo_stock_diff = sle.fifo_stock_value - sles[idx - 1].fifo_stock_value - sle.fifo_difference_diff = sle.fifo_stock_diff - sle.stock_value_difference - - if sle.batch_no: - sle.use_batchwise_valuation = frappe.db.get_value( - "Batch", sle.batch_no, "use_batchwise_valuation", cache=True - ) + if incorrect_idx is None and not is_sle_has_correct_data(sle, float_precision, currency_precision): + incorrect_idx = idx if filters.get("show_incorrect_entries"): - if incorrect_idx > 0: - sles = sles[cint(incorrect_idx) - 1 :] - - return [] + if incorrect_idx is None: + return [] + return sles[max(incorrect_idx - 1, 0) :] return sles -def is_sle_has_correct_data(sle, precision): - if flt(sle.difference_in_qty, precision) != 0.0 or flt(sle.diff_value_diff, precision) != 0: - print(flt(sle.difference_in_qty, precision), flt(sle.diff_value_diff, precision)) - return False +def maintains_fifo_queue(sle): + # no queue is maintained for serialized/batchwise-valued stock + return not ( + sle.serial_and_batch_bundle or sle.serial_no or (sle.batch_no and sle.use_batchwise_valuation) + ) - return True + +def add_fifo_fields(sle, prev_sle): + queue = json.loads(sle.stock_queue) if sle.stock_queue else [] + + fifo_qty = 0.0 + fifo_value = 0.0 + for qty, rate in queue: + fifo_qty += qty + fifo_value += qty * rate + + sle.fifo_queue_qty = fifo_qty + sle.fifo_stock_value = fifo_value + sle.fifo_valuation_rate = fifo_value / fifo_qty if fifo_qty else None + sle.fifo_qty_diff = sle.qty_after_transaction - fifo_qty + sle.fifo_value_diff = sle.stock_value - fifo_value + sle.fifo_valuation_diff = ( + sle.valuation_rate - sle.fifo_valuation_rate if sle.fifo_valuation_rate else None + ) + # prev row may not maintain a queue; H and H - F stay blank across the gap + if prev_sle and prev_sle.fifo_stock_value is not None: + sle.fifo_stock_diff = sle.fifo_stock_value - prev_sle.fifo_stock_value + sle.fifo_difference_diff = sle.fifo_stock_diff - sle.stock_value_difference + + +def is_sle_has_correct_data(sle, float_precision, currency_precision): + return ( + flt(sle.difference_in_qty, float_precision) == 0.0 + and flt(sle.diff_value_diff, currency_precision) == 0.0 + ) def get_columns(): diff --git a/erpnext/stock/report/stock_ledger_invariant_check/test_stock_ledger_invariant_check.py b/erpnext/stock/report/stock_ledger_invariant_check/test_stock_ledger_invariant_check.py new file mode 100644 index 00000000000..b82e341c84a --- /dev/null +++ b/erpnext/stock/report/stock_ledger_invariant_check/test_stock_ledger_invariant_check.py @@ -0,0 +1,75 @@ +# Copyright (c) 2026, Frappe Technologies Pvt. Ltd. and Contributors +# See license.txt + +import frappe +from frappe.tests.utils import FrappeTestCase + +from erpnext.stock.doctype.item.test_item import make_item +from erpnext.stock.doctype.stock_entry.stock_entry_utils import make_stock_entry +from erpnext.stock.report.stock_ledger_invariant_check.stock_ledger_invariant_check import execute + +WAREHOUSE = "Stores - _TC" +COMPANY = "_Test Company" + + +class TestStockLedgerInvariantCheck(FrappeTestCase): + def run_report(self, **extra): + filters = frappe._dict({"company": COMPANY, "warehouse": WAREHOUSE}) + filters.update(extra) + return execute(filters)[1] + + def make_movements(self) -> str: + # fresh item per test: db is only rolled back at class teardown on v15 + item = make_item(properties={"valuation_method": "FIFO"}).name + make_stock_entry(item_code=item, to_warehouse=WAREHOUSE, qty=10, rate=100, posting_date="2026-06-01") + make_stock_entry(item_code=item, to_warehouse=WAREHOUSE, qty=5, rate=120, posting_date="2026-06-02") + make_stock_entry(item_code=item, from_warehouse=WAREHOUSE, qty=4, rate=0, posting_date="2026-06-03") + return item + + def test_diagnostic_rows_have_no_discrepancy(self): + item = self.make_movements() + + data = self.run_report(item_code=item) + + self.assertEqual(len(data), 3) + for row in data: + self.assertLess(abs(row.difference_in_qty), 0.01) + self.assertLess(abs(row.fifo_qty_diff), 0.01) + self.assertLess(abs(row.diff_value_diff), 0.01) + + def test_running_balance_matches(self): + item = self.make_movements() + + data = self.run_report(item_code=item) + + self.assertEqual(data[-1].qty_after_transaction, 11) + + def test_show_incorrect_entries(self): + item = self.make_movements() + + self.assertEqual(self.run_report(item_code=item, show_incorrect_entries=1), []) + + sle = frappe.get_last_doc( + "Stock Ledger Entry", {"item_code": item, "warehouse": WAREHOUSE, "is_cancelled": 0} + ) + frappe.db.set_value( + "Stock Ledger Entry", sle.name, "qty_after_transaction", sle.qty_after_transaction + 5 + ) + + data = self.run_report(item_code=item, show_incorrect_entries=1) + self.assertEqual(len(data), 2) # incorrect entry + one before it for context + self.assertEqual(data[-1].name, sle.name) + + def test_batch_item_skips_fifo_queue_checks(self): + item = make_item( + properties={"has_batch_no": 1, "create_new_batch": 1, "batch_number_series": "SLIC-BAT-.####"} + ).name + make_stock_entry(item_code=item, to_warehouse=WAREHOUSE, qty=10, rate=100) + + data = self.run_report(item_code=item) + self.assertTrue(data) + for row in data: + self.assertIsNone(row.fifo_qty_diff) + self.assertIsNone(row.fifo_value_diff) + + self.assertEqual(self.run_report(item_code=item, show_incorrect_entries=1), []) diff --git a/erpnext/stock/report/stock_ledger_variance/stock_ledger_variance.py b/erpnext/stock/report/stock_ledger_variance/stock_ledger_variance.py index 327f158e3f6..04b888d85fe 100644 --- a/erpnext/stock/report/stock_ledger_variance/stock_ledger_variance.py +++ b/erpnext/stock/report/stock_ledger_variance/stock_ledger_variance.py @@ -205,7 +205,10 @@ def get_data(filters=None): data = [] if item_warehouse_map: - precision = cint(frappe.db.get_single_value("System Settings", "float_precision")) + float_precision = cint(frappe.db.get_single_value("System Settings", "float_precision")) or 3 + currency_precision = ( + cint(frappe.db.get_single_value("System Settings", "currency_precision")) or float_precision + ) for item_warehouse in item_warehouse_map: report_data = stock_ledger_invariant_check(item_warehouse) @@ -215,7 +218,11 @@ def get_data(filters=None): for row in report_data: if has_difference( - row, precision, filters.difference_in, item_warehouse.valuation_method or valuation_method + row, + float_precision, + currency_precision, + filters.difference_in, + item_warehouse.valuation_method or valuation_method, ): row.update( { @@ -261,23 +268,26 @@ def get_item_warehouse_combinations(filters: dict | None = None) -> dict: return query.run(as_dict=1) -def has_difference(row, precision, difference_in, valuation_method): +def has_difference(row, float_precision, currency_precision, difference_in, valuation_method): if valuation_method == "Moving Average": - qty_diff = flt(row.difference_in_qty, precision) - value_diff = flt(row.diff_value_diff, precision) - valuation_diff = flt(row.valuation_diff, precision) + qty_diff = flt(row.difference_in_qty, float_precision) + value_diff = flt(row.diff_value_diff, currency_precision) + valuation_diff = flt(row.valuation_diff, currency_precision) else: - qty_diff = flt(row.difference_in_qty, precision) - value_diff = flt(row.diff_value_diff, precision) + qty_diff = flt(row.difference_in_qty, float_precision) + value_diff = flt(row.diff_value_diff, currency_precision) if row.stock_queue and json.loads(row.stock_queue): value_diff = value_diff or ( - flt(row.fifo_value_diff, precision) or flt(row.fifo_difference_diff, precision) + flt(row.fifo_value_diff, currency_precision) + or flt(row.fifo_difference_diff, currency_precision) ) - qty_diff = qty_diff or flt(row.fifo_qty_diff, precision) + qty_diff = qty_diff or flt(row.fifo_qty_diff, float_precision) - valuation_diff = flt(row.valuation_diff, precision) or flt(row.fifo_valuation_diff, precision) + valuation_diff = flt(row.valuation_diff, currency_precision) or flt( + row.fifo_valuation_diff, currency_precision + ) if difference_in == "Qty" and qty_diff: return True @@ -287,3 +297,5 @@ def has_difference(row, precision, difference_in, valuation_method): return True elif difference_in not in ["Qty", "Value", "Valuation"] and (qty_diff or value_diff or valuation_diff): return True + + return False diff --git a/erpnext/stock/stock_ledger.py b/erpnext/stock/stock_ledger.py index 93ee2eaa651..ba578f69814 100644 --- a/erpnext/stock/stock_ledger.py +++ b/erpnext/stock/stock_ledger.py @@ -870,10 +870,16 @@ class update_entries_after: if ( sle.voucher_type in ["Purchase Receipt", "Purchase Invoice"] and sle.voucher_detail_no - and sle.actual_qty < 0 and is_internal_transfer(sle) ): - sle.outgoing_rate = get_incoming_rate_for_inter_company_transfer(sle) + # Anchor both legs of an internal-transfer PR/PI to the DN/SI incoming_rate; + # otherwise an inward SLE that inherits a stale PR.valuation_rate leaks the + # gap to COGS via divisional_loss. + rate = get_incoming_rate_for_inter_company_transfer(sle) + if sle.actual_qty < 0: + sle.outgoing_rate = rate + elif rate: + sle.incoming_rate = rate dimensions = get_inventory_dimensions() has_dimensions = False @@ -1080,7 +1086,11 @@ class update_entries_after: self.wh_data.stock_queue = json.loads(stock_queue[0]) if stock_queue else [] self.wh_data.stock_value = round_off_if_near_zero(self.wh_data.stock_value + doc.total_amount) - self.wh_data.qty_after_transaction += flt(doc.total_qty, self.flt_precision) + # Replay the immutable qty recorded on the SLE at submission, not the bundle's recomputed + # total_qty. A valuation repost must never rewrite physical quantities; if the bundle's child + # rows were edited after submission, doc.total_qty would silently corrupt qty_after_transaction + # (and every downstream balance). sle.actual_qty is the frozen movement for this entry. + self.wh_data.qty_after_transaction += flt(sle.actual_qty, self.flt_precision) if flt(self.wh_data.qty_after_transaction, self.flt_precision): self.wh_data.valuation_rate = flt(self.wh_data.stock_value, self.flt_precision) / flt( self.wh_data.qty_after_transaction, self.flt_precision @@ -2426,7 +2436,16 @@ def get_incoming_rate_for_inter_company_transfer(sle) -> float: if lcv_amount: lcv_rate = flt(lcv_amount / abs(sle.actual_qty)) - return rate + lcv_rate + charges_rate = 0.0 + if flt(sle.actual_qty) > 0: + charge_fields = ["item_tax_amount", "rm_supp_cost"] + charges = frappe.db.get_value( + f"{sle.voucher_type} Item", sle.voucher_detail_no, charge_fields, as_dict=True + ) + if charges: + charges_rate = flt(sum(flt(charges.get(f)) for f in charge_fields)) / abs(sle.actual_qty) + + return rate + lcv_rate + charges_rate def is_internal_transfer(sle): diff --git a/erpnext/stock/tests/test_get_item_details.py b/erpnext/stock/tests/test_get_item_details.py index fc19bac0a44..af98aa43980 100644 --- a/erpnext/stock/tests/test_get_item_details.py +++ b/erpnext/stock/tests/test_get_item_details.py @@ -35,6 +35,52 @@ class TestGetItemDetail(FrappeTestCase): details = get_item_details(args) self.assertEqual(details.get("price_list_rate"), 100) + def test_fetch_asset_category_expense_account_on_purchase_receipt(self): + from erpnext.stock.doctype.item.test_item import make_item + + asset_category = "Test Expense Account Asset Category" + if not frappe.db.exists("Asset Category", asset_category): + frappe.get_doc( + { + "doctype": "Asset Category", + "asset_category_name": asset_category, + "enable_cwip_accounting": 0, + "depreciation_method": "Straight Line", + "total_number_of_depreciations": 12, + "frequency_of_depreciation": 1, + "accounts": [ + { + "company_name": "_Test Company", + "fixed_asset_account": "_Test Fixed Asset - _TC", + "accumulated_depreciation_account": "_Test Accumulated Depreciations - _TC", + "depreciation_expense_account": "_Test Depreciations - _TC", + } + ], + } + ).insert() + + asset_item = make_item( + "Test Expense Account Asset Item", + {"is_stock_item": 0, "is_fixed_asset": 1, "asset_category": asset_category}, + ).item_code + + args = frappe._dict( + { + "item_code": asset_item, + "company": "_Test Company", + "conversion_rate": 1.0, + "price_list_currency": "USD", + "plc_conversion_rate": 1.0, + "doctype": "Purchase Receipt", + "supplier": "_Test Supplier", + "price_list": "_Test Buying Price List", + "ignore_pricing_rule": 1, + "qty": 1, + } + ) + details = get_item_details(args) + self.assertEqual(details.get("expense_account"), "_Test Fixed Asset - _TC") + # making this test in get_item_details test file as feat/fix is present in that method def test_fetch_price_from_list_rate_on_doc_save(self): # create item diff --git a/erpnext/support/doctype/service_level_agreement/service_level_agreement.py b/erpnext/support/doctype/service_level_agreement/service_level_agreement.py index 6f7c943ddad..531c6371591 100644 --- a/erpnext/support/doctype/service_level_agreement/service_level_agreement.py +++ b/erpnext/support/doctype/service_level_agreement/service_level_agreement.py @@ -232,7 +232,7 @@ class ServiceLevelAgreement(Document): if self.document_type == "Issue": return - service_level_agreement_fields = get_service_level_agreement_fields() + service_level_agreement_fields = get_service_level_agreement_fields(self.document_type) meta = frappe.get_meta(self.document_type, cached=False) if meta.custom: @@ -276,6 +276,7 @@ class ServiceLevelAgreement(Document): "hidden": field.get("hidden"), "description": field.get("description"), "default": field.get("default"), + "link_filters": field.get("link_filters"), } ).insert(ignore_permissions=True) else: @@ -302,6 +303,7 @@ class ServiceLevelAgreement(Document): "hidden": field.get("hidden"), "description": field.get("description"), "default": field.get("default"), + "link_filters": field.get("link_filters"), } ).insert(ignore_permissions=True) else: @@ -309,7 +311,7 @@ class ServiceLevelAgreement(Document): self.reset_field_properties(existing_field, "Custom Field", field) def reset_field_properties(self, field, field_dt, sla_field): - field = frappe.get_doc(field_dt, {"fieldname": field.fieldname}) + field = frappe.get_doc(field_dt, field.name) field.label = sla_field.get("label") field.fieldname = sla_field.get("fieldname") field.fieldtype = sla_field.get("fieldtype") @@ -320,6 +322,7 @@ class ServiceLevelAgreement(Document): field.hidden = sla_field.get("hidden") field.description = sla_field.get("description") field.default = sla_field.get("default") + field.link_filters = sla_field.get("link_filters") field.save(ignore_permissions=True) @@ -909,7 +912,7 @@ def record_assigned_users_on_failure(doc): doc.add_comment(comment_type="Assigned", text=message) -def get_service_level_agreement_fields(): +def get_service_level_agreement_fields(doctype: str): return [ { "collapsible": 1, @@ -922,6 +925,9 @@ def get_service_level_agreement_fields(): "fieldtype": "Link", "label": "Service Level Agreement", "options": "Service Level Agreement", + "link_filters": frappe.as_json( + [["Service Level Agreement", "document_type", "=", doctype]], indent=None + ), }, {"fieldname": "priority", "fieldtype": "Link", "label": "Priority", "options": "Issue Priority"}, {"fieldname": "response_by", "fieldtype": "Datetime", "label": "Response By", "read_only": 1}, diff --git a/erpnext/support/doctype/service_level_agreement/test_service_level_agreement.py b/erpnext/support/doctype/service_level_agreement/test_service_level_agreement.py index cabd38f6427..7d579786f51 100644 --- a/erpnext/support/doctype/service_level_agreement/test_service_level_agreement.py +++ b/erpnext/support/doctype/service_level_agreement/test_service_level_agreement.py @@ -2,6 +2,7 @@ # See license.txt import datetime +import json import unittest import frappe @@ -176,11 +177,14 @@ class TestServiceLevelAgreement(unittest.TestCase): self.assertEqual(lead_sla.name, default_sla.name) # check SLA custom fields created for leads - sla_fields = get_service_level_agreement_fields() + sla_fields = get_service_level_agreement_fields(doctype) for field in sla_fields: - self.assertTrue( - frappe.db.exists("Custom Field", {"dt": doctype, "fieldname": field.get("fieldname")}) + filters = {"dt": doctype, "fieldname": field.get("fieldname")} + self.assertTrue(frappe.db.exists("Custom Field", filters)) + self.assertEqual( + get_link_filters("Custom Field", filters), + json.loads(field["link_filters"]) if field.get("link_filters") else None, ) def test_docfield_creation_for_sla_on_custom_dt(self): @@ -200,13 +204,66 @@ class TestServiceLevelAgreement(unittest.TestCase): self.assertEqual(sla.name, default_sla.name) # check SLA docfields created - sla_fields = get_service_level_agreement_fields() + sla_fields = get_service_level_agreement_fields(doctype.name) for field in sla_fields: - self.assertTrue( - frappe.db.exists("DocField", {"fieldname": field.get("fieldname"), "parent": doctype.name}) + filters = {"fieldname": field.get("fieldname"), "parent": doctype.name} + self.assertTrue(frappe.db.exists("DocField", filters)) + self.assertEqual( + get_link_filters("DocField", filters), + json.loads(field["link_filters"]) if field.get("link_filters") else None, ) + def test_reset_field_properties_does_not_clobber_other_doctypes_field(self): + """Two doctypes each get their own "service_level_agreement" custom field + (same fieldname, different owning doctype). Updating the field on one of + them must not clobber the other's, even though both share the fieldname + (regression test for the fix in reset_field_properties, see PR #56954).""" + doctype_a = create_custom_doctype("Test SLA Dt A") + doctype_b = create_custom_doctype("Test SLA Dt B") + + for doctype in (doctype_a.name, doctype_b.name): + create_service_level_agreement( + default_service_level_agreement=1, + holiday_list="__Test Holiday List", + entity_type=None, + entity=None, + response_time=14400, + resolution_time=21600, + doctype=doctype, + ) + + def get_sla_field_link_filters(doctype): + return get_link_filters("DocField", {"parent": doctype, "fieldname": "service_level_agreement"}) + + self.assertEqual( + get_sla_field_link_filters(doctype_a.name), + [["Service Level Agreement", "document_type", "=", doctype_a.name]], + ) + + # The field on doctype_b already exists, so creating another, entity-specific + # SLA for doctype_b takes the "update existing field" branch (reset_field_properties) + # instead of creating a new field. + customer = create_customer() + create_service_level_agreement( + default_service_level_agreement=0, + holiday_list="__Test Holiday List", + entity_type="Customer", + entity=customer, + response_time=7200, + resolution_time=10800, + doctype=doctype_b.name, + ) + + self.assertEqual( + get_sla_field_link_filters(doctype_a.name), + [["Service Level Agreement", "document_type", "=", doctype_a.name]], + ) + self.assertEqual( + get_sla_field_link_filters(doctype_b.name), + [["Service Level Agreement", "document_type", "=", doctype_b.name]], + ) + def test_sla_application(self): # Default Service Level Agreement doctype = "Lead" @@ -362,6 +419,11 @@ class TestServiceLevelAgreement(unittest.TestCase): frappe.delete_doc("Service Level Agreement", d.name, force=1) +def get_link_filters(field_doctype, filters): + value = frappe.db.get_value(field_doctype, filters, "link_filters") + return json.loads(value) if value else None + + def get_service_level_agreement( default_service_level_agreement=None, entity_type=None, entity=None, doctype="Issue" ): @@ -602,8 +664,8 @@ def make_holiday_list(): ).insert() -def create_custom_doctype(): - if not frappe.db.exists("DocType", "Test SLA on Custom Dt"): +def create_custom_doctype(name="Test SLA on Custom Dt"): + if not frappe.db.exists("DocType", name): doc = frappe.get_doc( { "doctype": "DocType", @@ -626,13 +688,13 @@ def create_custom_doctype(): }, ], "permissions": [{"role": "System Manager", "read": 1, "write": 1}], - "name": "Test SLA on Custom Dt", + "name": name, } ) doc.insert() return doc else: - return frappe.get_doc("DocType", "Test SLA on Custom Dt") + return frappe.get_doc("DocType", name) def make_lead(creation=None, index=0, company=None):