diff --git a/erpnext/accounts/custom/address.py b/erpnext/accounts/custom/address.py index 246aee3b0ec..53b304b6916 100644 --- a/erpnext/accounts/custom/address.py +++ b/erpnext/accounts/custom/address.py @@ -65,4 +65,6 @@ def get_shipping_address(company, address=None): if address: address_as_dict = address[0] name, address_template = get_address_templates(address_as_dict) - return address_as_dict.get("name"), frappe.render_template(address_template, address_as_dict) + return address_as_dict.get("name"), frappe.render_template( + address_template, address_as_dict, restrict_globals=True + ) diff --git a/erpnext/accounts/doctype/chart_of_accounts_importer/chart_of_accounts_importer.js b/erpnext/accounts/doctype/chart_of_accounts_importer/chart_of_accounts_importer.js index 1d8bb853083..b7200883124 100644 --- a/erpnext/accounts/doctype/chart_of_accounts_importer/chart_of_accounts_importer.js +++ b/erpnext/accounts/doctype/chart_of_accounts_importer/chart_of_accounts_importer.js @@ -110,18 +110,6 @@ frappe.ui.form.on("Chart of Accounts Importer", { args: { company: frm.doc.company, }, - callback: function (r) { - if (r.message === false) { - frm.set_value("company", ""); - frappe.throw( - __( - "Transactions against the Company already exist! Chart of Accounts can only be imported for a Company with no transactions." - ) - ); - } else { - frm.trigger("refresh"); - } - }, }); } }, diff --git a/erpnext/accounts/doctype/chart_of_accounts_importer/chart_of_accounts_importer.py b/erpnext/accounts/doctype/chart_of_accounts_importer/chart_of_accounts_importer.py index eeaa5abfc6d..4949d407cf4 100644 --- a/erpnext/accounts/doctype/chart_of_accounts_importer/chart_of_accounts_importer.py +++ b/erpnext/accounts/doctype/chart_of_accounts_importer/chart_of_accounts_importer.py @@ -70,22 +70,37 @@ def validate_company(company): frappe.throw(msg, title=_("Wrong Company")) if frappe.db.get_all("GL Entry", {"company": company}, "name", limit=1): - return False + frappe.throw( + _( + "Transactions against the Company already exist! Chart of Accounts can only be imported for a Company with no transactions." + ) + ) + + validate_user_perms(company) @frappe.whitelist() def import_coa(file_name, company): + frappe.only_for("Accounts Manager") + # delete existing data for accounts - unset_existing_data(company) + frappe.has_permission("Company", "write", company, throw=True) # create accounts file_doc, extension = get_file(file_name) + validate_accounts(file_doc, extension) if extension == "csv": data = generate_data_from_csv(file_doc) else: data = generate_data_from_excel(file_doc, extension) + validate_columns(data) + + validate_company(company) + + unset_existing_data(company) + frappe.local.flags.ignore_root_company_validation = True forest = build_forest(data) create_charts(company, custom_chart=forest, from_coa_importer=True) @@ -452,6 +467,7 @@ def unset_existing_data(company): fieldnames = get_linked_fields("Account").get("Company", {}).get("fieldname", []) linked = [{"fieldname": name} for name in fieldnames] update_values = {d.get("fieldname"): "" for d in linked} + frappe.db.set_value("Company", company, update_values, update_values) # remove accounts data from various doctypes @@ -467,6 +483,19 @@ def unset_existing_data(company): frappe.qb.from_(dt).where(dt.company == company).delete().run() +def validate_user_perms(company): + # User Permission Check for Account Deletion + company_accounts = frappe.get_query("Account", filters={"company": company}).run(as_dict=1) + + for d in company_accounts: + if not frappe.get_cached_doc("Account", d.name).has_permission(): + frappe.throw( + _( + "Accounts cannot be removed, as user doesn't have access to all the accounts of {0}." + ).format(frappe.bold(company)) + ) + + def set_default_accounts(company): from erpnext.setup.doctype.company.company import install_country_fixtures diff --git a/erpnext/accounts/doctype/journal_entry/journal_entry.py b/erpnext/accounts/doctype/journal_entry/journal_entry.py index 762585601e5..8acd5cf8587 100644 --- a/erpnext/accounts/doctype/journal_entry/journal_entry.py +++ b/erpnext/accounts/doctype/journal_entry/journal_entry.py @@ -7,7 +7,7 @@ import json import frappe from frappe import _, msgprint, scrub from frappe.core.doctype.submission_queue.submission_queue import queue_submission -from frappe.utils import comma_and, cstr, flt, fmt_money, formatdate, get_link_to_form, nowdate +from frappe.utils import comma_and, cstr, flt, fmt_money, formatdate, get_link_to_form, getdate, nowdate import erpnext from erpnext.accounts.deferred_revenue import get_deferred_booking_accounts @@ -154,7 +154,8 @@ class JournalEntry(AccountsController): if self.docstatus == 0: self.apply_tax_withholding() - if self.is_new() or not self.title: + + if not self.title or (self.is_new() and self.amended_from): self.title = self.get_title() def validate_advance_accounts(self): @@ -798,6 +799,23 @@ class JournalEntry(AccountsController): ) ) + if reference_type == "Purchase Invoice": + on_hold, release_date = frappe.db.get_value( + reference_type, reference_name, ["on_hold", "release_date"] + ) + + if not on_hold or (release_date and getdate(release_date) <= getdate(nowdate())): + continue + + msg = ( + _("{0} {1} is blocked and on hold until {2}.").format( + reference_type, reference_name, release_date + ) + if release_date + else _("{0} {1} is blocked.").format(reference_type, reference_name) + ) + frappe.throw(msg) + def set_against_account(self): accounts_debited, accounts_credited = [], [] if self.voucher_type in ("Deferred Revenue", "Deferred Expense"): diff --git a/erpnext/accounts/doctype/journal_entry/test_journal_entry.py b/erpnext/accounts/doctype/journal_entry/test_journal_entry.py index f8fde0b54bc..62e74d02033 100644 --- a/erpnext/accounts/doctype/journal_entry/test_journal_entry.py +++ b/erpnext/accounts/doctype/journal_entry/test_journal_entry.py @@ -6,7 +6,7 @@ import unittest import frappe from frappe.tests.utils import change_settings -from frappe.utils import flt, nowdate +from frappe.utils import add_days, flt, nowdate from erpnext.accounts.doctype.account.test_account import get_inventory_account from erpnext.accounts.doctype.journal_entry.journal_entry import StockAccountInvalidTransaction @@ -602,6 +602,69 @@ class TestJournalEntry(unittest.TestCase): jv.save() self.assertRaises(frappe.ValidationError, jv.submit) + def make_jv_against_purchase_invoice(self, invoice, amount=100): + jv = make_journal_entry("Creditors - _TC", "_Test Cash - _TC", amount, save=False) + jv.accounts[0].party_type = "Supplier" + jv.accounts[0].party = invoice.supplier + jv.accounts[0].reference_type = "Purchase Invoice" + jv.accounts[0].reference_name = invoice.name + return jv + + def test_jv_against_purchase_invoice_respects_hold_state(self): + """Payment can be booked against a Purchase Invoice only while it is not on hold.""" + from erpnext.accounts.doctype.purchase_invoice.test_purchase_invoice import make_purchase_invoice + + release_date = add_days(nowdate(), 10) + + def never_held(): + return make_purchase_invoice() + + def held_until_a_future_date(): + invoice = make_purchase_invoice() + invoice.block_invoice(hold_comment="Waiting for the goods", release_date=release_date) + return invoice + + def held_without_a_release_date(): + invoice = make_purchase_invoice() + invoice.block_invoice(hold_comment="Under dispute") + return invoice + + def held_until_a_date_that_has_passed(): + invoice = held_until_a_future_date() + frappe.db.set_value("Purchase Invoice", invoice.name, "release_date", add_days(nowdate(), -1)) + return invoice + + def unblocked_again(): + invoice = held_until_a_future_date() + invoice.unblock_invoice() + return invoice + + for build_invoice in (held_until_a_future_date, held_without_a_release_date): + with self.subTest(build_invoice.__name__): + jv = self.make_jv_against_purchase_invoice(build_invoice()) + self.assertRaisesRegex(frappe.ValidationError, "is blocked", jv.insert) + + for build_invoice in (never_held, held_until_a_date_that_has_passed, unblocked_again): + with self.subTest(build_invoice.__name__): + invoice = build_invoice() + jv = self.make_jv_against_purchase_invoice(invoice) + jv.insert() + self.assertEqual(jv.reference_types[invoice.name], "Purchase Invoice") + + def test_jv_against_blocked_sales_invoice_reference_is_not_checked(self): + """A Sales Invoice has no hold state, so the check must skip it rather than fail.""" + from erpnext.accounts.doctype.sales_invoice.test_sales_invoice import create_sales_invoice + + invoice = create_sales_invoice(rate=500) + jv = make_journal_entry("_Test Cash - _TC", "Debtors - _TC", 100, save=False) + jv.accounts[1].party_type = "Customer" + jv.accounts[1].party = "_Test Customer" + jv.accounts[1].reference_type = "Sales Invoice" + jv.accounts[1].reference_name = invoice.name + jv.insert() + + self.assertEqual(jv.reference_types[invoice.name], "Sales Invoice") + def make_journal_entry( account1, diff --git a/erpnext/accounts/doctype/payment_request/payment_request.py b/erpnext/accounts/doctype/payment_request/payment_request.py index f5dc2fb479e..218ab9196f3 100644 --- a/erpnext/accounts/doctype/payment_request/payment_request.py +++ b/erpnext/accounts/doctype/payment_request/payment_request.py @@ -461,7 +461,7 @@ class PaymentRequest(Document): } if self.message: - return frappe.render_template(self.message, context) + return frappe.render_template(self.message, context, restrict_globals=True) def set_failed(self): pass diff --git a/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.js b/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.js index 195b29e38c6..cb7f9d6af76 100644 --- a/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.js +++ b/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.js @@ -237,10 +237,8 @@ erpnext.accounts.PurchaseInvoice = class PurchaseInvoice extends erpnext.buying. unblock_invoice() { const me = this; - frappe.call({ - method: "erpnext.accounts.doctype.purchase_invoice.purchase_invoice.unblock_invoice", - args: { name: me.frm.doc.name }, - callback: (r) => me.frm.reload_doc(), + me.frm.call("unblock_invoice", null, () => { + me.frm.reload_doc(); }); } @@ -291,15 +289,16 @@ erpnext.accounts.PurchaseInvoice = class PurchaseInvoice extends erpnext.buying. this.dialog.set_primary_action(__("Save"), function () { const dialog_data = me.dialog.get_values(); - frappe.call({ - method: "erpnext.accounts.doctype.purchase_invoice.purchase_invoice.block_invoice", - args: { - name: me.frm.doc.name, + me.frm.call( + "block_invoice", + { hold_comment: dialog_data.hold_comment, release_date: dialog_data.release_date, }, - callback: (r) => me.frm.reload_doc(), - }); + () => { + me.frm.reload_doc(); + } + ); me.dialog.hide(); }); @@ -338,10 +337,9 @@ erpnext.accounts.PurchaseInvoice = class PurchaseInvoice extends erpnext.buying. } set_release_date(data) { - return frappe.call({ - method: "erpnext.accounts.doctype.purchase_invoice.purchase_invoice.change_release_date", - args: data, - callback: (r) => this.frm.reload_doc(), + const me = this; + return me.frm.call("change_release_date", { release_date: data.release_date }, () => { + me.frm.reload_doc(); }); } diff --git a/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.json b/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.json index 8e9da9baa9d..058b8c8613b 100644 --- a/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.json +++ b/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.json @@ -352,6 +352,7 @@ { "collapsible": 1, "collapsible_depends_on": "eval:doc.on_hold", + "depends_on": "eval:doc.on_hold", "fieldname": "sb_14", "fieldtype": "Section Break", "label": "Hold Invoice" @@ -1662,7 +1663,7 @@ "idx": 204, "is_submittable": 1, "links": [], - "modified": "2026-07-12 23:54:21.263951", + "modified": "2026-08-05 15:40:16.519774", "modified_by": "Administrator", "module": "Accounts", "name": "Purchase Invoice", diff --git a/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py b/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py index f92252df2a7..afc08c0e4d6 100644 --- a/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py +++ b/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py @@ -8,7 +8,7 @@ import frappe from frappe import _, qb, throw from frappe.model.mapper import get_mapped_doc from frappe.query_builder.functions import Sum -from frappe.utils import cint, cstr, flt, formatdate, get_link_to_form, getdate, nowdate +from frappe.utils import DateTimeLikeObject, cint, cstr, flt, formatdate, get_link_to_form, getdate, nowdate import erpnext from erpnext.accounts.deferred_revenue import validate_service_stop_date @@ -299,6 +299,9 @@ class PurchaseInvoice(BuyingController): self.reset_default_field_value("set_from_warehouse", "items", "from_warehouse") self.set_percentage_received() + if self.on_hold: + self.validate_invoice_hold() + def set_percentage_received(self): total_billed_qty = 0.0 total_received_qty = 0.0 @@ -310,6 +313,13 @@ class PurchaseInvoice(BuyingController): if total_billed_qty and total_received_qty: self.per_received = total_received_qty / total_billed_qty * 100 + def validate_invoice_hold(self): + if self.is_return: + frappe.throw(_("Return Purchase Invoice cannot be held.")) + + if self.docstatus < 1: + frappe.throw(_("Purchase Invoice can be held after submitting.")) + def validate_release_date(self): if self.release_date and getdate(nowdate()) >= getdate(self.release_date): frappe.throw(_("Release date must be in the future")) @@ -1855,14 +1865,38 @@ class PurchaseInvoice(BuyingController): def on_recurring(self, reference_doc, auto_repeat_doc): self.due_date = None - def block_invoice(self, hold_comment=None, release_date=None): - self.db_set("on_hold", 1) - self.db_set("hold_comment", cstr(hold_comment)) + @frappe.whitelist(methods=["POST"]) + def block_invoice(self, hold_comment: str | None = None, release_date: DateTimeLikeObject | None = None): + self.check_permission("write") + self.on_hold = 1 + self.release_date = release_date + self.validate_block_invoice() + + self.db_set({"on_hold": 1, "hold_comment": cstr(hold_comment), "release_date": release_date}) + + @frappe.whitelist(methods=["POST"]) + def unblock_invoice(self): + self.check_permission("write") + self.db_set({"on_hold": 0, "release_date": None}) + + @frappe.whitelist(methods=["POST"]) + def change_release_date(self, release_date: DateTimeLikeObject | None = None): + self.check_permission("write") + + if not self.on_hold: + frappe.throw(_("Invoice is not blocked. Block the invoice to change the release date.")) + + self.release_date = release_date + self.validate_block_invoice() + self.db_set("release_date", release_date) - def unblock_invoice(self): - self.db_set("on_hold", 0) - self.db_set("release_date", None) + def validate_block_invoice(self): + self.validate_invoice_hold() + if self.outstanding_amount <= 0: + frappe.throw(_("Purchase Invoice without any outstanding amount cannot be held.")) + + self.validate_release_date() def set_tax_withholding(self): self.set("advance_tax", []) @@ -2082,28 +2116,6 @@ def make_stock_entry(source_name, target_doc=None): return doc -@frappe.whitelist() -def change_release_date(name, release_date=None): - if frappe.db.exists("Purchase Invoice", name): - pi = frappe.get_doc("Purchase Invoice", name) - pi.check_permission() - pi.db_set("release_date", release_date) - - -@frappe.whitelist() -def unblock_invoice(name): - if frappe.db.exists("Purchase Invoice", name): - pi = frappe.get_doc("Purchase Invoice", name) - pi.unblock_invoice() - - -@frappe.whitelist() -def block_invoice(name, release_date, hold_comment=None): - if frappe.db.exists("Purchase Invoice", name): - pi = frappe.get_doc("Purchase Invoice", name) - pi.block_invoice(hold_comment, release_date) - - @frappe.whitelist() def make_inter_company_sales_invoice(source_name, target_doc=None): from erpnext.accounts.doctype.sales_invoice.sales_invoice import make_inter_company_transaction diff --git a/erpnext/accounts/doctype/purchase_invoice/test_purchase_invoice.py b/erpnext/accounts/doctype/purchase_invoice/test_purchase_invoice.py index 5aa2faed1a1..ae9b8442c34 100644 --- a/erpnext/accounts/doctype/purchase_invoice/test_purchase_invoice.py +++ b/erpnext/accounts/doctype/purchase_invoice/test_purchase_invoice.py @@ -287,14 +287,166 @@ class TestPurchaseInvoice(FrappeTestCase, StockTestMixin): def test_purchase_invoice_explicit_block(self): pi = make_purchase_invoice() - pi.block_invoice() + release_date = add_days(nowdate(), 10) + + pi.block_invoice(hold_comment="Waiting for the goods", release_date=release_date) self.assertEqual(pi.on_hold, 1) + on_hold, hold_comment, saved_release_date = frappe.db.get_value( + "Purchase Invoice", pi.name, ["on_hold", "hold_comment", "release_date"] + ) + self.assertEqual(on_hold, 1) + self.assertEqual(hold_comment, "Waiting for the goods") + self.assertEqual(getdate(saved_release_date), getdate(release_date)) + pi.unblock_invoice() self.assertEqual(pi.on_hold, 0) + on_hold, saved_release_date = frappe.db.get_value( + "Purchase Invoice", pi.name, ["on_hold", "release_date"] + ) + self.assertEqual(on_hold, 0) + self.assertIsNone(saved_release_date) + + def test_purchase_invoice_cannot_be_held_before_submission(self): + pi = make_purchase_invoice(do_not_save=True) + pi.on_hold = 1 + + self.assertRaises(frappe.ValidationError, pi.save) + + pi.on_hold = 0 + pi.save() + pi.submit() + + pi.block_invoice() + self.assertEqual(frappe.db.get_value("Purchase Invoice", pi.name, "on_hold"), 1) + + def test_return_purchase_invoice_cannot_be_held(self): + from erpnext.controllers.sales_and_purchase_return import make_return_doc + + pi = make_purchase_invoice() + + return_pi = make_return_doc(pi.doctype, pi.name) + return_pi.on_hold = 1 + self.assertRaisesRegex(frappe.ValidationError, "cannot be held", return_pi.save) + + return_pi.on_hold = 0 + return_pi.save() + return_pi.submit() + + self.assertRaisesRegex(frappe.ValidationError, "cannot be held", return_pi.block_invoice) + + def test_return_purchase_invoice_is_not_affected_by_hold_validations(self): + from erpnext.controllers.sales_and_purchase_return import make_return_doc + + pi = make_purchase_invoice() + + # a return has a negative outstanding amount, which must not be mistaken + # for an invalid hold on a document that was never held + return_pi = make_return_doc(pi.doctype, pi.name) + return_pi.save() + return_pi.submit() + + self.assertEqual(return_pi.docstatus, 1) + self.assertEqual(return_pi.on_hold, 0) + self.assertLess(return_pi.outstanding_amount, 0) + + def test_settled_purchase_invoice_cannot_be_held(self): + pi = make_purchase_invoice() + + pe = get_payment_entry("Purchase Invoice", dn=pi.name, bank_account="_Test Bank - _TC") + pe.reference_no = "1" + pe.reference_date = nowdate() + pe.save() + pe.submit() + + pi.reload() + self.assertEqual(pi.outstanding_amount, 0) + + self.assertRaises(frappe.ValidationError, pi.block_invoice) + self.assertEqual(frappe.db.get_value("Purchase Invoice", pi.name, "on_hold"), 0) + + def test_release_date_of_held_invoice_must_be_in_future(self): + pi = make_purchase_invoice() + + self.assertRaises(frappe.ValidationError, pi.block_invoice, "Hold", add_days(nowdate(), -1)) + self.assertRaises(frappe.ValidationError, pi.block_invoice, "Hold", nowdate()) + + def test_rejected_hold_does_not_partially_update_invoice(self): + pi = make_purchase_invoice() + + self.assertRaises(frappe.ValidationError, pi.block_invoice, "Hold", add_days(nowdate(), -1)) + + pi.reload() + self.assertEqual(pi.on_hold, 0) + self.assertIsNone(pi.release_date) + + def test_change_release_date_of_held_invoice(self): + pi = make_purchase_invoice() + pi.block_invoice(hold_comment="Hold", release_date=add_days(nowdate(), 10)) + + new_release_date = add_days(nowdate(), 20) + pi.change_release_date(new_release_date) + + self.assertEqual( + getdate(frappe.db.get_value("Purchase Invoice", pi.name, "release_date")), + getdate(new_release_date), + ) + + self.assertRaises(frappe.ValidationError, pi.change_release_date, add_days(nowdate(), -1)) + + def test_release_date_cannot_be_changed_on_an_invoice_that_is_not_held(self): + pi = make_purchase_invoice() + + self.assertRaisesRegex( + frappe.ValidationError, + "Invoice is not blocked", + pi.change_release_date, + add_days(nowdate(), 10), + ) + + self.assertIsNone(frappe.db.get_value("Purchase Invoice", pi.name, "release_date")) + + def test_hold_methods_are_whitelisted_document_methods(self): + import erpnext.accounts.doctype.purchase_invoice.purchase_invoice as purchase_invoice_module + + pi = frappe.new_doc("Purchase Invoice") + + for method in ("block_invoice", "unblock_invoice", "change_release_date"): + # raises if the method is not whitelisted for client side calls + pi.is_whitelisted(method) + + self.assertFalse( + hasattr(purchase_invoice_module, method), + f"{method} should only be exposed as a document method", + ) + + def test_hold_methods_require_write_permission(self): + pi = make_purchase_invoice() + user = "test_pi_hold_permission@example.com" + + if not frappe.db.exists("User", user): + frappe.get_doc( + { + "doctype": "User", + "email": user, + "first_name": "Test PI Hold", + "roles": [{"role": "Employee"}], + } + ).insert(ignore_permissions=True) + + frappe.set_user(user) + try: + self.assertRaises(frappe.PermissionError, pi.block_invoice) + self.assertRaises(frappe.PermissionError, pi.unblock_invoice) + self.assertRaises(frappe.PermissionError, pi.change_release_date, add_days(nowdate(), 10)) + finally: + frappe.set_user("Administrator") + + self.assertEqual(frappe.db.get_value("Purchase Invoice", pi.name, "on_hold"), 0) + def test_gl_entries_with_perpetual_inventory_against_pr(self): pr = make_purchase_receipt( company="_Test Company with perpetual inventory", diff --git a/erpnext/accounts/doctype/subscription/subscription.py b/erpnext/accounts/doctype/subscription/subscription.py index 3665bf34bf2..e7dc9b078fc 100644 --- a/erpnext/accounts/doctype/subscription/subscription.py +++ b/erpnext/accounts/doctype/subscription/subscription.py @@ -222,6 +222,9 @@ class Subscription(Document): """ Sets the status of the `Subscription` """ + if self.status == "Cancelled": + return + if self.is_trialling(): self.status = "Trialling" elif self.status == "Active" and self.end_date and getdate(posting_date) > getdate(self.end_date): @@ -558,6 +561,11 @@ class Subscription(Document): 1. `process_for_active` 2. `process_for_past_due` """ + # Snapshot before update_subscription_period() below can roll this forward, + # so the cancel_at_period_end check further down still targets the period + # that just ended, not the next one. + current_period_end = self.current_invoice_end + if not self.is_current_invoice_generated( self.current_invoice_start, self.current_invoice_end ) and self.can_generate_new_invoice(posting_date): @@ -567,8 +575,8 @@ class Subscription(Document): self.update_subscription_period() if self.cancel_at_period_end and ( - getdate(posting_date) >= getdate(self.current_invoice_end) - or getdate(posting_date) >= getdate(self.end_date) + getdate(posting_date) >= getdate(current_period_end) + or (self.end_date and getdate(posting_date) >= getdate(self.end_date)) ): self.cancel_subscription() diff --git a/erpnext/accounts/doctype/subscription/test_subscription.py b/erpnext/accounts/doctype/subscription/test_subscription.py index 41ada4c804f..5cf52fa1eb4 100644 --- a/erpnext/accounts/doctype/subscription/test_subscription.py +++ b/erpnext/accounts/doctype/subscription/test_subscription.py @@ -280,6 +280,59 @@ class TestSubscription(FrappeTestCase): settings.cancel_after_grace = default_grace_period_action settings.save() + def test_cancelled_subscription_stays_cancelled_after_payment_and_reprocess(self): + # https://github.com/frappe/erpnext/issues/57761 + subscription = create_subscription( + start_date=nowdate(), generate_invoice_at="Beginning of the current subscription period" + ) + subscription.process(posting_date=nowdate()) # generate first invoice + invoice = subscription.get_current_invoice() + self.assertIsNotNone(invoice) + + invoice.db_set("outstanding_amount", 0) + invoice.db_set("status", "Paid") + + subscription.cancel_subscription() + self.assertEqual(subscription.status, "Cancelled") + cancelation_date = getdate(subscription.cancelation_date) + + subscription.set_subscription_status() + self.assertEqual(subscription.status, "Cancelled") + self.assertEqual(getdate(subscription.cancelation_date), cancelation_date) + + subscription.cancel_at_period_end = 1 + subscription.end_date = None + invoice_count = len(subscription.invoices) + subscription.process() + self.assertEqual(subscription.status, "Cancelled") + self.assertEqual(len(subscription.invoices), invoice_count) + + def test_subscription_cancels_at_period_end_without_end_date(self): + # https://github.com/frappe/erpnext/issues/57761 -- generate_invoice() rolls + # current_invoice_end forward to the next period before this check runs, so + # with no end_date to fall back on, cancel_at_period_end must compare + # against the period that just ended, not the (already advanced) next one. + create_plan( + plan_name="_Test plan name 11", + cost=80, + currency="INR", + billing_interval="Day", + billing_interval_count=3, + ) + subscription = create_subscription( + start_date=nowdate(), + generate_invoice_at="End of the current subscription period", + plans=[{"plan": "_Test plan name 11", "qty": 1}], + ) + subscription.cancel_at_period_end = 1 + self.assertEqual(len(subscription.invoices), 0) + period_end = subscription.current_invoice_end + + subscription.process(posting_date=period_end) + + self.assertEqual(subscription.status, "Cancelled") + self.assertEqual(len(subscription.invoices), 1) + def test_subscription_restart_and_process(self): settings = frappe.get_single("Subscription Settings") default_grace_period_action = settings.cancel_after_grace diff --git a/erpnext/accounts/report/accounts_payable/accounts_payable.js b/erpnext/accounts/report/accounts_payable/accounts_payable.js index 4da827f1a81..8de9d60a8fd 100644 --- a/erpnext/accounts/report/accounts_payable/accounts_payable.js +++ b/erpnext/accounts/report/accounts_payable/accounts_payable.js @@ -117,8 +117,11 @@ frappe.query_reports["Accounts Payable"] = { { fieldname: "supplier_group", label: __("Supplier Group"), - fieldtype: "Link", + fieldtype: "MultiSelectList", options: "Supplier Group", + get_data: function (txt) { + return frappe.db.get_link_options("Supplier Group", txt); + }, hidden: 1, }, { diff --git a/erpnext/accounts/report/accounts_payable/test_accounts_payable.py b/erpnext/accounts/report/accounts_payable/test_accounts_payable.py index 5a4e11b5291..03bc55684d2 100644 --- a/erpnext/accounts/report/accounts_payable/test_accounts_payable.py +++ b/erpnext/accounts/report/accounts_payable/test_accounts_payable.py @@ -121,6 +121,36 @@ class TestAccountsPayable(AccountsTestMixin, FrappeTestCase): self.assertEqual(len(report[1]), 2) self.assertEqual([pi.name, payment_term1.payment_term_name], [row.voucher_no, row.payment_term]) + def test_supplier_group_filter(self): + pi = self.create_purchase_invoice() + supplier_group = frappe.db.get_value("Supplier", self.supplier, "supplier_group") + other_group = frappe.get_doc( + doctype="Supplier Group", + supplier_group_name="_Test Supplier Group AP", + parent_supplier_group="All Supplier Groups", + ).insert() + + filters = { + "company": self.company, + "party_type": "Supplier", + "report_date": today(), + "range": "30, 60, 90, 120", + "supplier_group": supplier_group, + } + self.assertIn(pi.name, [row.voucher_no for row in execute(filters)[1]]) + + filters.update({"supplier_group": [other_group.name]}) + self.assertEqual(len(execute(filters)[1]), 0) + + filters.update({"supplier_group": [supplier_group, other_group.name]}) + self.assertIn(pi.name, [row.voucher_no for row in execute(filters)[1]]) + + filters.update({"supplier_group": ["All Supplier Groups"]}) + self.assertIn(pi.name, [row.voucher_no for row in execute(filters)[1]]) + + filters.update({"supplier_group": ["_Test Supplier Group Mars"]}) + self.assertRaises(frappe.ValidationError, execute, filters) + def test_project_filter(self): project = frappe.get_doc( {"doctype": "Project", "project_name": "_Test AP Project", "company": self.company} diff --git a/erpnext/accounts/report/accounts_payable_summary/accounts_payable_summary.js b/erpnext/accounts/report/accounts_payable_summary/accounts_payable_summary.js index 0b3bc077698..a5a42cb963b 100644 --- a/erpnext/accounts/report/accounts_payable_summary/accounts_payable_summary.js +++ b/erpnext/accounts/report/accounts_payable_summary/accounts_payable_summary.js @@ -100,8 +100,11 @@ frappe.query_reports["Accounts Payable Summary"] = { { fieldname: "supplier_group", label: __("Supplier Group"), - fieldtype: "Link", + fieldtype: "MultiSelectList", options: "Supplier Group", + get_data: function (txt) { + return frappe.db.get_link_options("Supplier Group", txt); + }, }, { fieldname: "based_on_payment_terms", diff --git a/erpnext/accounts/report/accounts_receivable/accounts_receivable.js b/erpnext/accounts/report/accounts_receivable/accounts_receivable.js index e0444a0af1e..20170d7eddc 100644 --- a/erpnext/accounts/report/accounts_receivable/accounts_receivable.js +++ b/erpnext/accounts/report/accounts_receivable/accounts_receivable.js @@ -140,8 +140,11 @@ frappe.query_reports["Accounts Receivable"] = { { fieldname: "territory", label: __("Territory"), - fieldtype: "Link", + fieldtype: "MultiSelectList", options: "Territory", + get_data: function (txt) { + return frappe.db.get_link_options("Territory", txt); + }, }, { fieldname: "group_by_party", diff --git a/erpnext/accounts/report/accounts_receivable/accounts_receivable.py b/erpnext/accounts/report/accounts_receivable/accounts_receivable.py index 5405bafab07..ef8c9a193c4 100644 --- a/erpnext/accounts/report/accounts_receivable/accounts_receivable.py +++ b/erpnext/accounts/report/accounts_receivable/accounts_receivable.py @@ -1013,7 +1013,13 @@ class ReceivablePayableReport: self.qb_selection_filter.append(self.ple.party.isin(customers)) if self.filters.get("territory"): - self.get_hierarchical_filters("Territory", "territory") + territories = get_nested_set_children("Territory", self.filters.territory) + customers = ( + qb.from_(self.customer) + .select(self.customer.name) + .where(self.customer["territory"].isin(territories)) + ) + self.qb_selection_filter.append(self.ple.party.isin(customers)) if self.filters.get("payment_terms_template"): customer_ptt = self.ple.party.isin( @@ -1034,11 +1040,10 @@ class ReceivablePayableReport: def add_supplier_filters(self): supplier = qb.DocType("Supplier") if self.filters.get("supplier_group"): + groups = get_party_group_with_children("Supplier", self.filters.supplier_group) self.qb_selection_filter.append( self.ple.party.isin( - qb.from_(supplier) - .select(supplier.name) - .where(supplier.supplier_group == self.filters.get("supplier_group")) + qb.from_(supplier).select(supplier.name).where(supplier.supplier_group.isin(groups)) ) ) @@ -1090,16 +1095,6 @@ class ReceivablePayableReport: return ptt - def get_hierarchical_filters(self, doctype, key): - lft, rgt = frappe.db.get_value(doctype, self.filters.get(key), ["lft", "rgt"]) - - doc = qb.DocType(doctype) - ple = self.ple - customer = self.customer - groups = qb.from_(doc).select(doc.name).where((doc.lft >= lft) & (doc.rgt <= rgt)) - customers = qb.from_(customer).select(customer.name).where(customer[key].isin(groups)) - self.qb_selection_filter.append(ple.party.isin(customers)) - def add_accounting_dimensions_filters(self): accounting_dimensions = get_accounting_dimensions(as_list=False) @@ -1329,19 +1324,23 @@ def get_party_group_with_children(party, party_groups): if party not in ("Customer", "Supplier"): return [] - group_dtype = f"{party} Group" - if not isinstance(party_groups, list): - party_groups = [d.strip() for d in party_groups.strip().split(",") if d] + return get_nested_set_children(f"{party} Group", party_groups) - all_party_groups = [] - for d in party_groups: - if frappe.db.exists(group_dtype, d): - lft, rgt = frappe.db.get_value(group_dtype, d, ["lft", "rgt"]) - children = frappe.get_all( - group_dtype, filters={"lft": [">=", lft], "rgt": ["<=", rgt]}, pluck="name" - ) - all_party_groups += children + +def get_nested_set_children(doctype, values): + if not isinstance(values, list): + values = [d.strip() for d in values.split(",") if d.strip()] + + if not values: + frappe.throw(_("Please select a valid {0}").format(_(doctype))) + + all_values = [] + for d in values: + if frappe.db.exists(doctype, d): + lft, rgt = frappe.db.get_value(doctype, d, ["lft", "rgt"]) + children = frappe.get_all(doctype, filters={"lft": [">=", lft], "rgt": ["<=", rgt]}, pluck="name") + all_values += children else: - frappe.throw(_("{0}: {1} does not exist").format(group_dtype, d)) + frappe.throw(_("{0}: {1} does not exist").format(doctype, d)) - return list(set(all_party_groups)) + return list(set(all_values)) diff --git a/erpnext/accounts/report/accounts_receivable/test_accounts_receivable.py b/erpnext/accounts/report/accounts_receivable/test_accounts_receivable.py index 7354b48e4a2..3395ad3a34a 100644 --- a/erpnext/accounts/report/accounts_receivable/test_accounts_receivable.py +++ b/erpnext/accounts/report/accounts_receivable/test_accounts_receivable.py @@ -771,6 +771,38 @@ class TestAccountsReceivable(AccountsTestMixin, FrappeTestCase): # Assert that the customer group of each row is in the list of customer groups self.assertIn(row.customer_group, cus_groups_list) + def test_territory_filter(self): + self.create_sales_invoice() + territory = frappe.db.get_value("Customer", self.customer, "territory") + + filters = { + "company": self.company, + "report_date": today(), + "range": "30, 60, 90, 120", + "territory": territory, + } + report = execute(filters)[1] + self.assertEqual(len(report), 1) + self.assertEqual( + [100.0, 100.0, territory], [report[0].invoiced, report[0].outstanding, report[0].territory] + ) + + filters.update({"territory": ["_Test Territory United States"]}) + self.assertEqual(len(execute(filters)[1]), 0) + + filters.update({"territory": [territory, "_Test Territory United States"]}) + self.assertEqual(len(execute(filters)[1]), 1) + + frappe.db.set_value("Customer", self.customer, "territory", "_Test Territory Maharashtra") + filters.update({"territory": ["_Test Territory India"]}) + self.assertEqual(len(execute(filters)[1]), 1) + + filters.update({"territory": ["_Test Territory Mars"]}) + self.assertRaises(frappe.ValidationError, execute, filters) + + filters.update({"territory": " "}) + self.assertRaises(frappe.ValidationError, execute, filters) + def test_party_account_filter(self): si1 = self.create_sales_invoice() self.customer2 = ( diff --git a/erpnext/accounts/report/accounts_receivable_summary/accounts_receivable_summary.js b/erpnext/accounts/report/accounts_receivable_summary/accounts_receivable_summary.js index c15ec8b0124..3d7121f3836 100644 --- a/erpnext/accounts/report/accounts_receivable_summary/accounts_receivable_summary.js +++ b/erpnext/accounts/report/accounts_receivable_summary/accounts_receivable_summary.js @@ -106,8 +106,11 @@ frappe.query_reports["Accounts Receivable Summary"] = { { fieldname: "territory", label: __("Territory"), - fieldtype: "Link", + fieldtype: "MultiSelectList", options: "Territory", + get_data: function (txt) { + return frappe.db.get_link_options("Territory", txt); + }, }, { fieldname: "sales_partner", diff --git a/erpnext/accounts/report/sales_register/sales_register.py b/erpnext/accounts/report/sales_register/sales_register.py index e55f217682d..bbffaa7d015 100644 --- a/erpnext/accounts/report/sales_register/sales_register.py +++ b/erpnext/accounts/report/sales_register/sales_register.py @@ -151,7 +151,13 @@ def _execute(filters, additional_table_columns=None): ) if inv.doctype == "Sales Invoice": - row.update({"debit": inv.base_grand_total, "credit": 0.0}) + # credit only settlements the invoice itself posts to the receivable (mirrors its GL) + row.update( + { + "debit": inv.base_grand_total, + "credit": get_in_invoice_receivable_credit(inv), + } + ) else: row.update({"debit": 0.0, "credit": inv.base_grand_total}) data.append(row) @@ -167,6 +173,14 @@ def _execute(filters, additional_table_columns=None): return columns, res, None, None, None, include_payments +def get_in_invoice_receivable_credit(inv): + # amount the invoice settles against its own receivable, matching the invoice's GL entries + credit = flt(inv.loyalty_amount) # loyalty redemption, POS or not + if inv.is_pos: # POS payments and write-off credit the receivable only on POS invoices + credit += flt(inv.base_paid_amount) - flt(inv.base_change_amount) + flt(inv.base_write_off_amount) + return credit + + def get_columns(invoice_list, additional_table_columns, include_payments=False): """return columns based on filters""" columns = [ @@ -433,6 +447,11 @@ def get_invoices(filters, additional_query_columns): si.base_net_total, si.base_grand_total, si.base_rounded_total, + si.is_pos, + si.base_paid_amount, + si.base_change_amount, + si.base_write_off_amount, + si.loyalty_amount, si.outstanding_amount, si.is_internal_customer, si.represents_company, diff --git a/erpnext/accounts/report/sales_register/test_sales_register.py b/erpnext/accounts/report/sales_register/test_sales_register.py index 9e72f81f6e5..3b8b14a763d 100644 --- a/erpnext/accounts/report/sales_register/test_sales_register.py +++ b/erpnext/accounts/report/sales_register/test_sales_register.py @@ -1,7 +1,8 @@ import frappe from frappe.tests.utils import FrappeTestCase -from frappe.utils import getdate, today +from frappe.utils import flt, getdate, today +from erpnext.accounts.doctype.pos_profile.test_pos_profile import make_pos_profile from erpnext.accounts.doctype.sales_invoice.test_sales_invoice import create_sales_invoice from erpnext.accounts.report.sales_register.sales_register import execute from erpnext.accounts.test.accounts_mixin import AccountsTestMixin @@ -54,6 +55,46 @@ class TestItemWiseSalesRegister(AccountsTestMixin, FrappeTestCase): si = si.submit() return si + def test_ledger_view_nets_pos_paid_invoice(self): + # A POS payment settles the receivable inside the invoice, so the ledger view must credit it + # and net to zero instead of showing a phantom outstanding. + make_pos_profile() + si = create_sales_invoice( + item=self.item, + company=self.company, + customer=self.customer, + debit_to=self.debit_to, + posting_date=today(), + parent_cost_center=self.cost_center, + cost_center=self.cost_center, + rate=100, + price_list_rate=100, + do_not_save=1, + ) + si.is_pos = 1 + si.append("payments", {"mode_of_payment": "Cash", "amount": 100}) + si = si.save().submit() + self.assertEqual(flt(si.outstanding_amount), 0.0) + + filters = frappe._dict( + { + "from_date": today(), + "to_date": today(), + "company": self.company, + "include_payments": True, + "customer": self.customer, + } + ) + rows = execute(filters)[1] + inv_row = next(x for x in rows if x.get("voucher_no") == si.name) + + self.assertEqual(flt(inv_row.get("debit")), 100.0) + self.assertEqual(flt(inv_row.get("credit")), 100.0) + + # running balance is unchanged by a fully-paid POS invoice + idx = rows.index(inv_row) + self.assertEqual(flt(inv_row.get("balance")), flt(rows[idx - 1].get("balance"))) + def test_basic_report_output(self): si = self.create_sales_invoice(rate=98) diff --git a/erpnext/assets/doctype/asset_repair/asset_repair.js b/erpnext/assets/doctype/asset_repair/asset_repair.js index a477c0bfcbc..8daff02b780 100644 --- a/erpnext/assets/doctype/asset_repair/asset_repair.js +++ b/erpnext/assets/doctype/asset_repair/asset_repair.js @@ -86,24 +86,39 @@ frappe.ui.form.on("Asset Repair", { }, repair_status: (frm) => { - if (frm.doc.completion_date && frm.doc.repair_status == "Completed") { - frappe.call({ - method: "erpnext.assets.doctype.asset_repair.asset_repair.get_downtime", - args: { - failure_date: frm.doc.failure_date, - completion_date: frm.doc.completion_date, - }, - callback: function (r) { - if (r.message) { - frm.set_value("downtime", r.message + " Hrs"); - } - }, - }); - } - if (frm.doc.repair_status == "Completed" && !frm.doc.completion_date) { frm.set_value("completion_date", frappe.datetime.now_datetime()); } + + frm.events.set_downtime(frm); + }, + + failure_date: (frm) => { + frm.events.set_downtime(frm); + }, + + completion_date: (frm) => { + frm.events.set_downtime(frm); + }, + + set_downtime: (frm) => { + if (frm.doc.repair_status != "Completed" || !frm.doc.failure_date || !frm.doc.completion_date) { + frm.set_value("downtime", null); + return; + } + + frappe.call({ + method: "erpnext.assets.doctype.asset_repair.asset_repair.get_downtime", + args: { + failure_date: frm.doc.failure_date, + completion_date: frm.doc.completion_date, + }, + callback: function (r) { + if (r.message) { + frm.set_value("downtime", r.message + " Hrs"); + } + }, + }); }, stock_items_on_form_rendered() { diff --git a/erpnext/assets/doctype/asset_repair/asset_repair.py b/erpnext/assets/doctype/asset_repair/asset_repair.py index 793101f2072..47a55b3b3c6 100644 --- a/erpnext/assets/doctype/asset_repair/asset_repair.py +++ b/erpnext/assets/doctype/asset_repair/asset_repair.py @@ -65,6 +65,7 @@ class AssetRepair(AccountsController): self.set_stock_items_cost() self.calculate_total_repair_cost() self.validate_purchase_invoice_status() + self.set_downtime() def validate_purchase_invoice_status(self): if self.purchase_invoice: @@ -215,6 +216,13 @@ class AssetRepair(AccountsController): if self.repair_status == "Pending": frappe.throw(_("Please update Repair Status.")) + def set_downtime(self): + # keep downtime in sync with the entered dates, regardless of edit order + if self.repair_status == "Completed" and self.failure_date and self.completion_date: + self.downtime = f"{get_downtime(self.failure_date, self.completion_date)} Hrs" + else: + self.downtime = None + def check_for_stock_items_and_warehouse(self): if not self.get("stock_items"): frappe.throw(_("Please enter Stock Items consumed during the Repair."), title=_("Missing Items")) diff --git a/erpnext/assets/doctype/asset_repair/test_asset_repair.py b/erpnext/assets/doctype/asset_repair/test_asset_repair.py index 3a92f0ec71a..89e75de34bf 100644 --- a/erpnext/assets/doctype/asset_repair/test_asset_repair.py +++ b/erpnext/assets/doctype/asset_repair/test_asset_repair.py @@ -100,6 +100,21 @@ class TestAssetRepair(unittest.TestCase): asset_repair = create_asset_repair(submit=1) self.assertNotEqual(asset_repair.repair_status, "Pending") + def test_downtime_stays_in_sync_with_dates(self): + asset = create_asset(submit=1) + asset_repair = create_asset_repair(asset=asset) + + asset_repair.failure_date = "2026-07-31 09:00:00" + asset_repair.completion_date = "2026-07-31 11:00:00" + asset_repair.repair_status = "Completed" + asset_repair.save() + self.assertEqual(asset_repair.downtime, "2.0 Hrs") + + # editing a date must refresh downtime, not leave a stale value + asset_repair.completion_date = "2026-07-31 14:30:00" + asset_repair.save() + self.assertEqual(asset_repair.downtime, "5.5 Hrs") + def test_stock_items(self): asset_repair = create_asset_repair(stock_consumption=1) self.assertTrue(asset_repair.stock_consumption) diff --git a/erpnext/buying/doctype/purchase_order/test_purchase_order.py b/erpnext/buying/doctype/purchase_order/test_purchase_order.py index c4394c066e0..81c992214d2 100644 --- a/erpnext/buying/doctype/purchase_order/test_purchase_order.py +++ b/erpnext/buying/doctype/purchase_order/test_purchase_order.py @@ -211,6 +211,7 @@ class TestPurchaseOrder(FrappeTestCase): po.load_from_db() existing_ordered_qty = get_ordered_qty() + existing_ordered_qty_in_new_warehouse = get_ordered_qty(warehouse="_Test Warehouse 2 - _TC") first_item_of_po = po.get("items")[0] trans_item = json.dumps( @@ -221,16 +222,64 @@ class TestPurchaseOrder(FrappeTestCase): "qty": first_item_of_po.qty, "docname": first_item_of_po.name, }, - {"item_code": "_Test Item", "rate": 200, "qty": 7}, + {"item_code": "_Test Item", "rate": 200, "qty": 7, "warehouse": "_Test Warehouse 2 - _TC"}, ] ) update_child_qty_rate("Purchase Order", trans_item, po.name) po.reload() self.assertEqual(len(po.get("items")), 2) + self.assertEqual(po.get("items")[-1].warehouse, "_Test Warehouse 2 - _TC") self.assertEqual(po.status, "To Receive and Bill") - # ordered qty should increase on row addition - self.assertEqual(get_ordered_qty(), existing_ordered_qty + 7) + # ordered qty should increase on row addition, in the warehouse passed for the new row + self.assertEqual(get_ordered_qty(), existing_ordered_qty) + self.assertEqual( + get_ordered_qty(warehouse="_Test Warehouse 2 - _TC"), + existing_ordered_qty_in_new_warehouse + 7, + ) + + def test_update_child_adding_new_item_without_any_default_warehouse(self): + stock_item = make_item("_Test PO Item Without Default Warehouse", {"is_stock_item": 1}).name + service_item = make_item("_Test PO Item Non Stock", {"is_stock_item": 0}).name + + po = create_purchase_order(do_not_save=1) + po.save() + po.submit() + first_item_of_po = po.get("items")[0] + + stock_settings_default = frappe.db.get_single_value("Stock Settings", "default_warehouse") + frappe.db.set_single_value("Stock Settings", "default_warehouse", None) + self.addCleanup( + frappe.db.set_single_value, "Stock Settings", "default_warehouse", stock_settings_default + ) + + def get_trans_items(item_code): + return json.dumps( + [ + { + "item_code": first_item_of_po.item_code, + "rate": first_item_of_po.rate, + "qty": first_item_of_po.qty, + "docname": first_item_of_po.name, + }, + {"item_code": item_code, "rate": 200, "qty": 7}, + ] + ) + + self.assertRaisesRegex( + frappe.ValidationError, + "Cannot find a default warehouse", + update_child_qty_rate, + "Purchase Order", + get_trans_items(stock_item), + po.name, + ) + + update_child_qty_rate("Purchase Order", get_trans_items(service_item), po.name) + + po.reload() + self.assertEqual(po.get("items")[-1].item_code, service_item) + self.assertFalse(po.get("items")[-1].warehouse) def test_update_child_removing_item(self): po = create_purchase_order(do_not_save=1) @@ -416,11 +465,13 @@ class TestPurchaseOrder(FrappeTestCase): "item_code": item, "rate": 100, "qty": 1, + "warehouse": po.items[0].warehouse, }, # added item whose tax account head already exists in PO { "item_code": new_item_with_tax.name, "rate": 100, "qty": 1, + "warehouse": po.items[0].warehouse, }, # added item whose tax account head is missing in PO ] ) @@ -948,6 +999,8 @@ class TestPurchaseOrder(FrappeTestCase): # self.assertEqual(po.payment_terms_template, pi.payment_terms_template) compare_payment_schedules(self, po, pi) + @change_settings("Selling Settings", {"maintain_same_sales_rate": 1}) + @change_settings("Buying Settings", {"maintain_same_rate": 1}) def test_internal_transfer_flow(self): from erpnext.accounts.doctype.cost_center.test_cost_center import create_cost_center from erpnext.accounts.doctype.sales_invoice.sales_invoice import ( @@ -959,9 +1012,6 @@ class TestPurchaseOrder(FrappeTestCase): ) from erpnext.stock.doctype.delivery_note.delivery_note import make_inter_company_purchase_receipt - frappe.db.set_single_value("Selling Settings", "maintain_same_sales_rate", 1) - frappe.db.set_single_value("Buying Settings", "maintain_same_rate", 1) - prepare_data_for_internal_transfer() supplier = "_Test Internal Supplier 2" diff --git a/erpnext/buying/doctype/request_for_quotation/request_for_quotation.py b/erpnext/buying/doctype/request_for_quotation/request_for_quotation.py index 95b0ec8b389..c4ada801cd2 100644 --- a/erpnext/buying/doctype/request_for_quotation/request_for_quotation.py +++ b/erpnext/buying/doctype/request_for_quotation/request_for_quotation.py @@ -325,14 +325,14 @@ class RequestforQuotation(BuyingController): message_template = self.mfs_html if self.use_html else self.message_for_supplier # nosemgrep: frappe-semgrep-rules.rules.security.frappe-ssti - rendered_message = frappe.render_template(message_template, doc_args) + rendered_message = frappe.render_template(message_template, doc_args, restrict_globals=True) subject_source = ( self.subject or frappe.get_value("Email Template", self.email_template, "subject") or _("Request for Quotation") ) - rendered_subject = frappe.render_template(subject_source, doc_args) + rendered_subject = frappe.render_template(subject_source, doc_args, restrict_globals=True) if preview: return { "message": rendered_message, diff --git a/erpnext/controllers/accounts_controller.py b/erpnext/controllers/accounts_controller.py index 7f2afecaf9f..c19d254b56c 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -75,6 +75,11 @@ from erpnext.stock.get_item_details import ( get_item_tax_map, get_item_warehouse, ) +from erpnext.stock.utils import ( + is_group_warehouse, + validate_disabled_warehouse, + validate_warehouse_company, +) from erpnext.utilities.regional import temporary_flag from erpnext.utilities.transaction_base import TransactionBase @@ -258,6 +263,7 @@ class AccountsController(TransactionBase): if self.is_return: self.validate_qty() else: + self.clear_stale_deferred_fields() self.validate_deferred_start_and_end_date() self.validate_inter_company_reference() @@ -643,6 +649,23 @@ class AccountsController(TransactionBase): if self.get("from_date") and self.get("to_date") and getdate(self.from_date) > getdate(self.to_date): frappe.throw(_("To Date cannot be before From Date"), title=_("Invalid Auto Repeat Date")) + def clear_stale_deferred_fields(self): + field_map = { + "Sales Invoice": "deferred_revenue_account", + "Purchase Invoice": "deferred_expense_account", + } + account_field = field_map.get(self.doctype) + + for item in self.get("items"): + if item.get("enable_deferred_revenue") or item.get("enable_deferred_expense"): + continue + + item.service_start_date = None + item.service_end_date = None + item.service_stop_date = None + if account_field: + item.set(account_field, None) + def validate_deferred_start_and_end_date(self): for d in self.items: if d.get("enable_deferred_revenue") or d.get("enable_deferred_expense"): @@ -3722,7 +3745,7 @@ def set_order_defaults(parent_doctype, parent_doctype_name, child_doctype, child child_item.update({date_fieldname: trans_item.get(date_fieldname) or p_doc.get(date_fieldname)}) child_item.stock_uom = item.stock_uom child_item.uom = trans_item.get("uom") or item.stock_uom - child_item.warehouse = get_item_warehouse(item, p_doc, overwrite_warehouse=True) + child_item.warehouse = get_new_child_item_warehouse(p_doc, item, trans_item, child_doctype) conversion_factor = flt(get_conversion_factor(item.item_code, child_item.uom).get("conversion_factor")) child_item.conversion_factor = flt(trans_item.get("conversion_factor")) or conversion_factor child_item.update(get_bin_details(child_item.item_code, child_item.warehouse, p_doc.get("company"))) @@ -3731,20 +3754,45 @@ def set_order_defaults(parent_doctype, parent_doctype_name, child_doctype, child # Initialized value will update in parent validation child_item.base_rate = 1 child_item.base_amount = 1 - if child_doctype == "Sales Order Item": - child_item.warehouse = get_item_warehouse(item, p_doc, overwrite_warehouse=True) - if not child_item.warehouse: - frappe.throw( - _( - "Cannot find a default warehouse for item {0}. Please set one in the Item Master or in Stock Settings." - ).format(frappe.bold(item.item_code)) - ) set_child_tax_template_and_map(item, child_item, p_doc) add_taxes_from_tax_template(child_item, p_doc) return child_item +def get_new_child_item_warehouse(p_doc, item, trans_item: dict, child_doctype: str) -> str | None: + """Return the warehouse picked in the Update Items dialog, else the configured default. + + Validates whichever warehouse was resolved, since a submitted parent skips validate(). + """ + warehouse = trans_item.get("warehouse") or get_item_warehouse(item, p_doc, overwrite_warehouse=True) + + if not warehouse: + if is_warehouse_required_for_new_child_item(child_doctype, item, trans_item): + frappe.throw( + _( + "Cannot find a default warehouse for item {0}. Please select one in the Update Items dialog, or set a default in the Item Master or in Stock Settings." + ).format(frappe.bold(item.item_code)) + ) + return None + + validate_warehouse_company(warehouse, p_doc.company) + validate_disabled_warehouse(warehouse) + is_group_warehouse(warehouse) + return warehouse + + +def is_warehouse_required_for_new_child_item(child_doctype: str, item, trans_item: dict) -> bool: + """Sales Order always needs one; buying documents only for stock rows, as in validate_stock_item_warehouse.""" + if child_doctype == "Sales Order Item": + return True + + if child_doctype in ("Purchase Order Item", "Supplier Quotation Item"): + return bool(item.is_stock_item and flt(trans_item.get("qty")) and not item.delivered_by_supplier) + + return False + + def validate_child_on_delete(row, parent, ordered_item=None): """Check if partially transacted item (row) is being deleted.""" if parent.doctype == "Sales Order": diff --git a/erpnext/controllers/queries.py b/erpnext/controllers/queries.py index 88a40eb72b1..3b0602fa209 100644 --- a/erpnext/controllers/queries.py +++ b/erpnext/controllers/queries.py @@ -967,9 +967,8 @@ def get_payment_terms_for_references(doctype, txt, searchfield, start, page_len, def get_filtered_child_rows(doctype, txt, searchfield, start, page_len, filters) -> list: table = frappe.qb.DocType(doctype) query = ( - frappe.qb.from_(table) + frappe.get_query(table, filters=filters) .select( - table.name, Concat("#", table.idx, ", ", table.item_code), ) .orderby(table.idx) @@ -977,10 +976,6 @@ def get_filtered_child_rows(doctype, txt, searchfield, start, page_len, filters) .limit(page_len) ) - if filters: - for field, value in filters.items(): - query = query.where(table[field] == value) - if txt: txt += "%" query = query.where( diff --git a/erpnext/controllers/selling_controller.py b/erpnext/controllers/selling_controller.py index 5655fea9915..f1e3baebc51 100644 --- a/erpnext/controllers/selling_controller.py +++ b/erpnext/controllers/selling_controller.py @@ -236,7 +236,7 @@ class SellingController(StockController): total += sales_person.allocated_percentage - if sales_team and total != 100.0: + if sales_team and flt(total, self.precision("allocated_percentage", "sales_team")) != 100.0: throw(_("Total allocated percentage for sales team should be 100")) def validate_sales_team(self, sales_team): diff --git a/erpnext/controllers/status_updater.py b/erpnext/controllers/status_updater.py index 05011a6b9d9..34ff799a83c 100644 --- a/erpnext/controllers/status_updater.py +++ b/erpnext/controllers/status_updater.py @@ -312,13 +312,12 @@ class StatusUpdater(Document): qty_or_amount, ) - role_allowed_to_over_deliver_receive = frappe.db.get_single_value( - "Stock Settings", "role_allowed_to_over_deliver_receive" - ) - role_allowed_to_over_bill = frappe.db.get_single_value( - "Accounts Settings", "role_allowed_to_over_bill" - ) - role = role_allowed_to_over_deliver_receive if qty_or_amount == "qty" else role_allowed_to_over_bill + role = None + if qty_or_amount == "qty": + if args.get("overflow_type") in ("delivery", "receipt"): + role = frappe.get_single_value("Stock Settings", "role_allowed_to_over_deliver_receive") + else: + role = frappe.get_single_value("Accounts Settings", "role_allowed_to_over_bill") overflow_percent = ( (item[args["target_field"]] - item[args["target_ref_field"]]) / item[args["target_ref_field"]] diff --git a/erpnext/controllers/stock_controller.py b/erpnext/controllers/stock_controller.py index 269f85ffcbb..f5e344a4045 100644 --- a/erpnext/controllers/stock_controller.py +++ b/erpnext/controllers/stock_controller.py @@ -1778,6 +1778,11 @@ def is_reposting_pending(): ) +def invalidate_future_sle_cache(voucher_type, voucher_no): + if hasattr(frappe.local, "future_sle"): + frappe.local.future_sle.pop((voucher_type, voucher_no), None) + + def future_sle_exists(args, sl_entries=None): from erpnext.stock.utils import get_combine_datetime diff --git a/erpnext/controllers/tests/test_stock_controller.py b/erpnext/controllers/tests/test_stock_controller.py new file mode 100644 index 00000000000..39b8a884b9d --- /dev/null +++ b/erpnext/controllers/tests/test_stock_controller.py @@ -0,0 +1,184 @@ +# Copyright (c) 2025, Frappe Technologies Pvt. Ltd. and Contributors +# See license.txt + +import frappe +from frappe.tests.utils import FrappeTestCase, change_settings +from frappe.utils import add_days, today + + +class TestStockControllerConversions(FrappeTestCase): + def tearDown(self): + # FrappeTestCase only rolls back once per class, so undo this test's writes here: a + # submitted Repost Item Valuation left behind cannot be deleted by the cleanups below. + frappe.db.rollback() + if hasattr(frappe.local, "future_sle"): + frappe.local.future_sle.clear() + + @staticmethod + def _cancel_and_delete(doctype, name): + if not frappe.db.exists(doctype, name): + return + doc = frappe.get_doc(doctype, name) + if doc.docstatus == 1: + doc.cancel() + frappe.delete_doc(doctype, name, force=1) + + def test_future_sle_exists_detects_later_entries(self): + # A later SLE for the same item+warehouse must be reported as a future entry, which + # exercises the GROUP BY query in future_sle_exists on both engines. + from erpnext.controllers.stock_controller import future_sle_exists + from erpnext.stock.doctype.item.test_item import make_item + from erpnext.stock.doctype.stock_entry.stock_entry_utils import make_stock_entry + + item = make_item("_Test Future SLE Item", {"is_stock_item": 1}).name + se = make_stock_entry(item_code=item, target="_Test Warehouse - _TC", qty=10, basic_rate=100) + self.addCleanup(self._cancel_and_delete, "Stock Entry", se.name) + + # Pretend a different voucher posts a day earlier for the same item/warehouse: the existing + # (later) SLE must be reported as a future entry. + args = frappe._dict( + voucher_type="Stock Entry", + voucher_no="_TEST-NONEXISTENT-SE", + posting_date=add_days(today(), -1), + posting_time="00:00:00", + ) + sl_entries = [frappe._dict(item_code=item, warehouse="_Test Warehouse - _TC")] + + self.assertTrue(future_sle_exists(args, sl_entries)) + + def _make_opening_entry(self, item, warehouse): + from erpnext.stock.doctype.stock_entry.stock_entry_utils import make_stock_entry + + opening = make_stock_entry( + item_code=item, + target=warehouse, + qty=100, + basic_rate=100, + posting_date=add_days(today(), -5), + posting_time="01:00:00", + ) + self.addCleanup(self._cancel_and_delete, "Stock Entry", opening.name) + + return opening + + def _later_sle(self, item, warehouse, opening): + sle = frappe.get_doc( + { + "doctype": "Stock Ledger Entry", + "item_code": item, + "warehouse": warehouse, + "posting_date": today(), + "posting_time": "12:00:00", + "voucher_type": "Stock Entry", + "voucher_no": opening.name, + "actual_qty": 7, + "incoming_rate": 100, + "qty_after_transaction": 107, + "valuation_rate": 100, + "stock_value": 10700, + "company": opening.company, + "stock_uom": "Nos", + } + ) + sle.flags.ignore_permissions = True + sle.flags.ignore_links = True + + return sle + + def _submit_entry(self, item, warehouse, inject=None): + from erpnext.stock import stock_ledger + from erpnext.stock.doctype.stock_entry.stock_entry_utils import make_stock_entry + + original_make_entry = stock_ledger.make_entry + injected = [] + + def make_entry_with_injection(*args, **kwargs): + if inject is not None and not injected: + injected.append(True) + inject.submit() + return original_make_entry(*args, **kwargs) + + stock_ledger.make_entry = make_entry_with_injection + try: + entry = make_stock_entry( + item_code=item, + target=warehouse, + qty=5, + basic_rate=500, + posting_date=today(), + posting_time="06:00:00", + ) + finally: + stock_ledger.make_entry = original_make_entry + + self.addCleanup(self._cancel_and_delete, "Stock Entry", entry.name) + if inject is not None: + self.assertTrue(injected, "the later SL Entry was not written during the submit") + + return entry + + def _reposts_queued_for(self, item, warehouse, voucher_no): + names = set( + frappe.get_all( + "Repost Item Valuation", + filters={"docstatus": 1, "item_code": item, "warehouse": warehouse}, + pluck="name", + ) + ) | set( + frappe.get_all( + "Repost Item Valuation", + filters={"docstatus": 1, "voucher_no": voucher_no}, + pluck="name", + ) + ) + for name in names: + self.addCleanup(frappe.delete_doc, "Repost Item Valuation", name, force=1) + + return names + + def test_repost_queued_for_entry_backdated_while_its_sl_entries_were_written(self): + from erpnext.stock.doctype.item.test_item import make_item + + item = make_item("_Test Concurrent Backdated Item", {"is_stock_item": 1}).name + warehouse = "_Test Warehouse - _TC" + + opening = self._make_opening_entry(item, warehouse) + backdated = self._submit_entry(item, warehouse, inject=self._later_sle(item, warehouse, opening)) + + self.assertTrue( + self._reposts_queued_for(item, warehouse, backdated.name), + "No Repost Item Valuation was queued for an entry that a later SL Entry made backdated", + ) + + def test_repost_queued_against_voucher_when_item_based_reposting_is_off(self): + from erpnext.stock.doctype.item.test_item import make_item + + item = make_item("_Test Voucher Based Repost Item", {"is_stock_item": 1}).name + warehouse = "_Test Warehouse - _TC" + + with change_settings("Stock Reposting Settings", item_based_reposting=0): + opening = self._make_opening_entry(item, warehouse) + backdated = self._submit_entry(item, warehouse, inject=self._later_sle(item, warehouse, opening)) + + self.assertTrue( + frappe.get_all( + "Repost Item Valuation", + filters={"docstatus": 1, "voucher_no": backdated.name}, + pluck="name", + ), + "No voucher based Repost Item Valuation was queued", + ) + + def test_no_repost_queued_when_nothing_was_written_after_the_entry(self): + from erpnext.stock.doctype.item.test_item import make_item + + item = make_item("_Test Unconcurrent Item", {"is_stock_item": 1}).name + warehouse = "_Test Warehouse - _TC" + + self._make_opening_entry(item, warehouse) + entry = self._submit_entry(item, warehouse) + + self.assertFalse( + self._reposts_queued_for(item, warehouse, entry.name), + "A Repost Item Valuation was queued for an entry with nothing posted after it", + ) diff --git a/erpnext/crm/doctype/appointment/appointment.py b/erpnext/crm/doctype/appointment/appointment.py index da91a73f105..8beed20befa 100644 --- a/erpnext/crm/doctype/appointment/appointment.py +++ b/erpnext/crm/doctype/appointment/appointment.py @@ -13,6 +13,7 @@ from frappe.model.document import Document from frappe.share import add_docshare from frappe.utils import add_to_date, cint, date_diff, get_datetime, get_url, getdate, now, now_datetime from frappe.utils.data import sha256_hash +from frappe.utils.html_utils import escape_html from erpnext.setup.doctype.holiday_list.holiday_list import is_holiday @@ -269,7 +270,11 @@ class Appointment(Document): if self.customer_details: lead.append( "notes", - {"note": self.customer_details, "added_by": frappe.session.user, "added_on": now()}, + { + "note": escape_html(self.customer_details), + "added_by": frappe.session.user, + "added_on": now(), + }, ) self.party = lead.insert(ignore_permissions=True).name diff --git a/erpnext/crm/doctype/contract_template/contract_template.py b/erpnext/crm/doctype/contract_template/contract_template.py index 700197500fb..d0bf738d79d 100644 --- a/erpnext/crm/doctype/contract_template/contract_template.py +++ b/erpnext/crm/doctype/contract_template/contract_template.py @@ -30,7 +30,7 @@ class ContractTemplate(Document): def validate(self): if self.contract_terms: - validate_template(self.contract_terms) + validate_template(self.contract_terms, restrict_globals=True) @frappe.whitelist() @@ -42,6 +42,6 @@ def get_contract_template(template_name, doc): contract_terms = None if contract_template.contract_terms: - contract_terms = frappe.render_template(contract_template.contract_terms, doc) + contract_terms = frappe.render_template(contract_template.contract_terms, doc, restrict_globals=True) return {"contract_template": contract_template, "contract_terms": contract_terms} diff --git a/erpnext/crm/doctype/email_campaign/email_campaign.py b/erpnext/crm/doctype/email_campaign/email_campaign.py index 4454ede5310..dbc4382a041 100644 --- a/erpnext/crm/doctype/email_campaign/email_campaign.py +++ b/erpnext/crm/doctype/email_campaign/email_campaign.py @@ -171,8 +171,8 @@ def send_mail(entry, email_campaign): context = {"doc": frappe.get_doc("Email Group", recipient)} # Render template - subject = frappe.render_template(email_template.get("subject"), context) - content = frappe.render_template(email_template.response_, context) + subject = frappe.render_template(email_template.get("subject"), context, restrict_globals=True) + content = frappe.render_template(email_template.response_, context, restrict_globals=True) try: comm = make( diff --git a/erpnext/manufacturing/doctype/blanket_order/test_blanket_order.py b/erpnext/manufacturing/doctype/blanket_order/test_blanket_order.py index a6eb18f47bc..b28d5a1b41b 100644 --- a/erpnext/manufacturing/doctype/blanket_order/test_blanket_order.py +++ b/erpnext/manufacturing/doctype/blanket_order/test_blanket_order.py @@ -1,7 +1,7 @@ # Copyright (c) 2018, Frappe Technologies Pvt. Ltd. and Contributors # See license.txt import frappe -from frappe.tests.utils import FrappeTestCase +from frappe.tests.utils import FrappeTestCase, change_settings from frappe.utils import add_months, today from erpnext import get_company_currency @@ -91,6 +91,32 @@ class TestBlanketOrder(FrappeTestCase): frappe.db.set_single_value("Buying Settings", "blanket_order_allowance", 10) po.submit() + @change_settings("Selling Settings", {"blanket_order_allowance": 0}) + @change_settings("Buying Settings", {"blanket_order_allowance": 0}) + @change_settings( + "Stock Settings", + {"over_delivery_receipt_allowance": 10, "role_allowed_to_over_deliver_receive": "Stock Manager"}, + ) + def test_stock_over_delivery_role_does_not_bypass_blanket_order_allowance(self): + test_user = frappe.get_doc("User", "test@example.com") + test_user.add_roles("Accounts User", "Stock Manager") + + frappe.clear_cache() + for blanket_order_type, doctype, date_field in ( + ("Selling", "Sales Order", "delivery_date"), + ("Purchasing", "Purchase Order", "schedule_date"), + ): + bo = make_blanket_order(blanket_order_type=blanket_order_type, quantity=100) + frappe.flags.args.doctype = doctype + order = make_order(bo.name) + order.currency = get_company_currency(order.company) + setattr(order, date_field, today()) + order.items[0].qty = 110 + + with self.set_user("test@example.com"): + order.flags.ignore_permissions = True + self.assertRaises(frappe.ValidationError, order.submit) + def test_party_item_code(self): item_doc = make_item("_Test Item 1 for Blanket Order") item_code = item_doc.name diff --git a/erpnext/manufacturing/doctype/bom/bom.py b/erpnext/manufacturing/doctype/bom/bom.py index 5976cb4fa94..3a799a508dd 100644 --- a/erpnext/manufacturing/doctype/bom/bom.py +++ b/erpnext/manufacturing/doctype/bom/bom.py @@ -1577,10 +1577,10 @@ def item_query(doctype, txt, searchfield, start, page_len, filters): query_filters = {"disabled": 0, "ifnull(end_of_life, '3099-12-31')": (">", today())} - or_cond_filters = {} + or_cond_filters = [] if txt: for s_field in searchfields: - or_cond_filters[s_field] = ("like", f"%{txt}%") + or_cond_filters.append([s_field, "like", f"%{txt}%"]) barcodes = frappe.get_all( "Item Barcode", @@ -1590,7 +1590,7 @@ def item_query(doctype, txt, searchfield, start, page_len, filters): barcodes = [d.item_code for d in barcodes] if barcodes: - or_cond_filters["name"] = ("in", barcodes) + or_cond_filters.append(["name", "in", barcodes]) if filters and filters.get("item_code"): has_variants = frappe.get_cached_value("Item", filters.get("item_code"), "has_variants") diff --git a/erpnext/manufacturing/doctype/bom/test_bom.py b/erpnext/manufacturing/doctype/bom/test_bom.py index cc942d59c74..60cb68d23cc 100644 --- a/erpnext/manufacturing/doctype/bom/test_bom.py +++ b/erpnext/manufacturing/doctype/bom/test_bom.py @@ -461,6 +461,29 @@ class TestBOM(FrappeTestCase): self.assertNotEqual(len(test_items), len(filtered), msg="Item filtering showing excessive results") self.assertTrue(0 < len(filtered) <= 3, msg="Item filtering showing excessive results") + @timeout + def test_bom_item_query_matches_item_code_colliding_with_another_barcode(self): + item = make_item( + "_Test BOM Query 2.5MM", + {"is_stock_item": 1, "item_name": "_Test BOM Query Sheet", "description": "sheet"}, + ) + make_item( + "_Test BOM Query Barcode Holder", + {"is_stock_item": 1}, + barcode=f"90{item.name}90", + ) + + results = item_query( + doctype="Item", + txt=item.name, + searchfield="name", + start=0, + page_len=20, + filters={"is_stock_item": 1}, + ) + + self.assertIn(item.name, [d[0] for d in results]) + @timeout def test_exclude_exploded_items_from_bom(self): bom_no = get_default_bom() diff --git a/erpnext/manufacturing/doctype/job_card/job_card.js b/erpnext/manufacturing/doctype/job_card/job_card.js index cc8bdf04176..52423c600d9 100644 --- a/erpnext/manufacturing/doctype/job_card/job_card.js +++ b/erpnext/manufacturing/doctype/job_card/job_card.js @@ -296,7 +296,16 @@ frappe.ui.form.on("Job Card", { prepare_timer_buttons: function (frm) { frm.trigger("make_dashboard"); + const transfer_pending = + !frm.doc.is_corrective_job_card && + (frm.doc.items || []).length && + flt(frm.doc.transferred_qty) < flt(frm.doc.for_quantity); + if (!frm.doc.started_time && !frm.doc.current_time) { + if (transfer_pending) { + return; + } + frm.add_custom_button(__("Start Job"), () => { if ((frm.doc.employee && !frm.doc.employee.length) || !frm.doc.employee) { frappe.prompt( diff --git a/erpnext/manufacturing/doctype/job_card/job_card.py b/erpnext/manufacturing/doctype/job_card/job_card.py index 9fddeda3e96..b1854088975 100644 --- a/erpnext/manufacturing/doctype/job_card/job_card.py +++ b/erpnext/manufacturing/doctype/job_card/job_card.py @@ -513,6 +513,8 @@ class JobCard(Document): ) def add_time_log(self, args): + self.validate_transfer_qty() + last_row = [] employees = args.employees if isinstance(employees, str): diff --git a/erpnext/manufacturing/doctype/job_card/test_job_card.py b/erpnext/manufacturing/doctype/job_card/test_job_card.py index a50d0cba62b..ceb5ce299eb 100644 --- a/erpnext/manufacturing/doctype/job_card/test_job_card.py +++ b/erpnext/manufacturing/doctype/job_card/test_job_card.py @@ -249,6 +249,26 @@ class TestJobCard(FrappeTestCase): # JC is Completed with excess transfer self.assertEqual(job_card.status, "Completed") + def test_job_card_time_log_blocked_until_material_transfer(self): + "Time logs must wait for the transfer when RMs move against Job Card." + self.transfer_material_against = "Job Card" + self.source_warehouse = "Stores - _TC" + + self.generate_required_stock(self.work_order) + job_card = frappe.get_last_doc("Job Card", {"work_order": self.work_order.name}) + + self.assertRaises( + frappe.ValidationError, job_card.add_time_log, frappe._dict(start_time=now(), employees=[]) + ) + + transfer_entry = make_stock_entry_from_jc(job_card.name) + transfer_entry.insert() + transfer_entry.submit() + + job_card.reload() + job_card.add_time_log(frappe._dict(start_time=now(), employees=[])) + self.assertTrue(job_card.time_logs) + @change_settings("Manufacturing Settings", {"job_card_excess_transfer": 0}) def test_job_card_excess_material_transfer_block(self): self.transfer_material_against = "Job Card" diff --git a/erpnext/manufacturing/doctype/workstation/workstation.py b/erpnext/manufacturing/doctype/workstation/workstation.py index d996d8a98ea..ac7b5f043e2 100644 --- a/erpnext/manufacturing/doctype/workstation/workstation.py +++ b/erpnext/manufacturing/doctype/workstation/workstation.py @@ -189,7 +189,6 @@ class Workstation(Document): for row in doc.time_logs: if not row.to_time: row.to_time = to_time - row.time_in_mins = time_diff_in_hours(row.to_time, row.from_time) / 60 row.completed_qty = qty doc.save() diff --git a/erpnext/manufacturing/report/bom_explorer/bom_explorer.py b/erpnext/manufacturing/report/bom_explorer/bom_explorer.py index de6dec9ebb8..7fffed14866 100644 --- a/erpnext/manufacturing/report/bom_explorer/bom_explorer.py +++ b/erpnext/manufacturing/report/bom_explorer/bom_explorer.py @@ -21,7 +21,17 @@ def get_exploded_items(bom, data, indent=0, qty=1): exploded_items = frappe.get_all( "BOM Item", filters={"parent": bom}, - fields=["qty", "bom_no", "qty", "item_code", "item_name", "description", "uom", "idx"], + fields=[ + "qty", + "bom_no", + "bom_no.quantity as child_bom_qty", + "stock_qty", + "item_code", + "item_name", + "description", + "uom", + "idx", + ], order_by="idx ASC", ) @@ -40,7 +50,12 @@ def get_exploded_items(bom, data, indent=0, qty=1): } ) if item.bom_no: - get_exploded_items(item.bom_no, data, indent=indent + 1, qty=item.qty) + get_exploded_items( + item.bom_no, + data, + indent=indent + 1, + qty=qty * item.stock_qty / item.child_bom_qty, + ) def get_columns(): diff --git a/erpnext/manufacturing/report/bom_explorer/test_bom_explorer.py b/erpnext/manufacturing/report/bom_explorer/test_bom_explorer.py new file mode 100644 index 00000000000..b2cac10a434 --- /dev/null +++ b/erpnext/manufacturing/report/bom_explorer/test_bom_explorer.py @@ -0,0 +1,41 @@ +# Copyright (c) 2026, Frappe Technologies Pvt. Ltd. and Contributors +# See license.txt + +import unittest +from unittest.mock import patch + +import frappe + +from erpnext.manufacturing.report.bom_explorer.bom_explorer import get_exploded_items + + +class TestBOMExplorer(unittest.TestCase): + def test_nested_bom_normalizes_and_accumulates_qty(self): + def item(item_code, qty, stock_qty, bom_no="", uom="Nos", child_bom_qty=None): + return frappe._dict( + item_code=item_code, + item_name=item_code, + description="", + qty=qty, + stock_qty=stock_qty, + bom_no=bom_no, + child_bom_qty=child_bom_qty, + uom=uom, + idx=1, + ) + + children = { + "root": [item("parent", 2, 20, "parent-bom", "Box", 5)], + "parent-bom": [item("child", 3, 12, "child-bom", "Pack", 4)], + "child-bom": [item("raw-material", 2, 2, uom="Kg")], + } + + def get_items(_doctype, filters, **kwargs): + self.assertIn("bom_no.quantity as child_bom_qty", kwargs["fields"]) + return children[filters["parent"]] + + data = [] + with patch.object(frappe, "get_all", side_effect=get_items): + get_exploded_items("root", data) + + self.assertEqual([row["qty"] for row in data], [2, 12, 24]) diff --git a/erpnext/patches/v14_0/clear_reconciliation_values_from_singles.py b/erpnext/patches/v14_0/clear_reconciliation_values_from_singles.py index c1f5b60a406..21b31e3bda9 100644 --- a/erpnext/patches/v14_0/clear_reconciliation_values_from_singles.py +++ b/erpnext/patches/v14_0/clear_reconciliation_values_from_singles.py @@ -1,3 +1,4 @@ +import frappe from frappe import qb @@ -13,5 +14,8 @@ def execute(): "Payment Reconciliation Allocation", ] for x in doctypes: + # child tables may not exist yet on sites where this pre-model-sync patch runs first + if not frappe.db.table_exists(x): + continue dt = qb.DocType(x) qb.from_(dt).delete().run() diff --git a/erpnext/projects/doctype/task/task.py b/erpnext/projects/doctype/task/task.py index 7eb55272eb8..5028c60f1ad 100755 --- a/erpnext/projects/doctype/task/task.py +++ b/erpnext/projects/doctype/task/task.py @@ -88,6 +88,7 @@ class Task(NestedSet): self.validate_dependencies_for_template_task() self.validate_completed_on() self.validate_parent_is_group() + self.validate_web_form_project_permission() def validate_dates(self): self.validate_from_to_dates("exp_start_date", "exp_end_date") @@ -287,6 +288,23 @@ class Task(NestedSet): if project_user: return True + def validate_web_form_project_permission(self): + project_unchanged = not self.is_new() and self.project == self.get_db_value("project") + + if ( + not frappe.flags.in_web_form + or not self.project + or project_unchanged + or frappe.has_permission("Project", "write", doc=self.project) + or self.has_webform_permission() + ): + return + + frappe.throw( + _("You are not permitted to create a Task for Project {0}").format(self.project), + frappe.PermissionError, + ) + def populate_depends_on(self): if self.parent_task: parent = frappe.get_doc("Task", self.parent_task) diff --git a/erpnext/public/js/controllers/transaction.js b/erpnext/public/js/controllers/transaction.js index 88dc01d5845..e7f4cdec979 100644 --- a/erpnext/public/js/controllers/transaction.js +++ b/erpnext/public/js/controllers/transaction.js @@ -561,9 +561,7 @@ erpnext.TransactionController = class TransactionController extends erpnext.taxe var update_stock = 0, show_batch_dialog = 0; item.weight_per_unit = 0; item.weight_uom = ''; - if(!item.barcode){ - item.uom = null // make UOM blank to update the existing UOM when item changes - } + item.uom = null // make UOM blank to update the existing UOM when item changes item.conversion_factor = 0; if(['Sales Invoice', 'Purchase Invoice'].includes(this.frm.doc.doctype)) { diff --git a/erpnext/public/js/utils.js b/erpnext/public/js/utils.js index 8746aa822aa..2f66c0720d8 100755 --- a/erpnext/public/js/utils.js +++ b/erpnext/public/js/utils.js @@ -648,6 +648,7 @@ erpnext.utils.update_child_items = function (opts) { qty: d.qty, rate: d.rate, uom: d.uom, + warehouse: d.warehouse, fg_item: d.fg_item, fg_item_qty: d.fg_item_qty, }; @@ -727,8 +728,14 @@ erpnext.utils.update_child_items = function (opts) { }, callback: function (r) { if (r.message) { - const { qty, price_list_rate: rate, uom, conversion_factor, bom_no } = r.message; - + const { + qty, + price_list_rate: rate, + uom, + conversion_factor, + bom_no, + warehouse, + } = r.message; const row = dialog.fields_dict.trans_items.df.data.find( (doc) => doc.idx == me.doc.idx ); @@ -739,6 +746,7 @@ erpnext.utils.update_child_items = function (opts) { qty: me.doc.qty || qty, rate: me.doc.rate || rate, bom_no: bom_no, + warehouse: me.doc.docname ? me.doc.warehouse : warehouse, }); dialog.fields_dict.trans_items.grid.refresh(); } @@ -812,6 +820,29 @@ erpnext.utils.update_child_items = function (opts) { }); } + const warehouse_df = child_meta.fields.find((f) => f.fieldname == "warehouse"); + if (warehouse_df) { + fields.splice(3, 0, { + fieldtype: "Link", + fieldname: "warehouse", + options: "Warehouse", + in_list_view: 1, + label: __(warehouse_df.label), + // only new rows may set it, existing rows would leave their + // reserved qty stranded in the previous warehouse's bin + read_only_depends_on: "eval:doc.docname", + get_query: () => { + return { + filters: { + company: frm.doc.company, + is_group: 0, + disabled: 0, + }, + }; + }, + }); + } + if ( frm.doc.doctype == "Purchase Order" && frm.doc.is_subcontracted && diff --git a/erpnext/selling/doctype/customer/customer.py b/erpnext/selling/doctype/customer/customer.py index 4d21ff94d3e..4df15cac65e 100644 --- a/erpnext/selling/doctype/customer/customer.py +++ b/erpnext/selling/doctype/customer/customer.py @@ -165,7 +165,8 @@ class Customer(TransactionBase): self.loyalty_program_tier = customer.loyalty_program_tier if self.sales_team: - if sum(member.allocated_percentage or 0 for member in self.sales_team) != 100: + total = sum(flt(member.allocated_percentage) for member in self.sales_team) + if flt(total, self.precision("allocated_percentage", "sales_team")) != 100: frappe.throw(_("Total contribution percentage should be equal to 100")) @frappe.whitelist() diff --git a/erpnext/selling/doctype/product_bundle/product_bundle.js b/erpnext/selling/doctype/product_bundle/product_bundle.js index 67b9ae5ba31..d65cddba5f0 100644 --- a/erpnext/selling/doctype/product_bundle/product_bundle.js +++ b/erpnext/selling/doctype/product_bundle/product_bundle.js @@ -9,6 +9,11 @@ frappe.ui.form.on("Product Bundle", { query: "erpnext.selling.doctype.product_bundle.product_bundle.get_new_item_code", }; }); + frm.set_query("item_code", "items", () => { + return { + query: "erpnext.controllers.queries.item_query", + }; + }); frm.set_query("item_code", "items", () => { return { diff --git a/erpnext/selling/doctype/sales_order/sales_order.py b/erpnext/selling/doctype/sales_order/sales_order.py index 1dd62a65239..a825d08cc15 100755 --- a/erpnext/selling/doctype/sales_order/sales_order.py +++ b/erpnext/selling/doctype/sales_order/sales_order.py @@ -1114,8 +1114,20 @@ def make_delivery_note(source_name, target_doc=None, kwargs=None): return target_doc +def get_qty_net_of_returns(so_item) -> float: + """Return the ordered quantity billable after returns and re-deliveries.""" + qty = flt(so_item.qty) + + return min(qty, max(qty - flt(so_item.returned_qty), flt(so_item.delivered_qty))) + + @frappe.whitelist() -def make_sales_invoice(source_name, target_doc=None, ignore_permissions=False, args=None): +def make_sales_invoice( + source_name: str, + target_doc: str | dict | Document | None = None, + ignore_permissions: bool = False, + args: str | dict | None = None, +): if args is None: args = {} if isinstance(args, str): @@ -1123,10 +1135,40 @@ def make_sales_invoice(source_name, target_doc=None, ignore_permissions=False, a # 0 qty is accepted, as the qty is uncertain for some items has_unit_price_items = frappe.db.get_value("Sales Order", source_name, "has_unit_price_items") + billed_qty_by_item = None + pending_qty_by_item = {} def is_unit_price_row(source): return has_unit_price_items and source.qty == 0 + def get_billed_qty_by_item(): + nonlocal billed_qty_by_item + + if billed_qty_by_item is None: + invoice_item = frappe.qb.DocType("Sales Invoice Item") + sales_order_item = frappe.qb.DocType("Sales Order Item") + rows = ( + frappe.qb.from_(invoice_item) + .inner_join(sales_order_item) + .on(invoice_item.so_detail == sales_order_item.name) + .select(invoice_item.so_detail, Sum(invoice_item.qty).as_("qty")) + .where((invoice_item.docstatus == 1) & (sales_order_item.parent == source_name)) + .groupby(invoice_item.so_detail) + ).run(as_dict=True) + billed_qty_by_item = {row.so_detail: flt(row.qty) for row in rows} + + return billed_qty_by_item + + def get_pending_qty(source): + if source.name not in pending_qty_by_item: + billable_qty = get_qty_net_of_returns(source) + if source.qty and source.billed_amt: + billable_qty -= get_billed_qty_by_item().get(source.name, 0) + + pending_qty_by_item[source.name] = max(flt(billable_qty, source.precision("qty")), 0) + + return pending_qty_by_item[source.name] + def postprocess(source, target): set_missing_values(source, target) # Get the advance paid Journal Entries in Sales Invoice Advance @@ -1156,17 +1198,6 @@ def make_sales_invoice(source_name, target_doc=None, ignore_permissions=False, a target.debit_to = get_party_account("Customer", source.customer, source.company) def update_item(source, target, source_parent): - def get_billed_qty(so_item_name): - from frappe.query_builder.functions import Sum - - table = frappe.qb.DocType("Sales Invoice Item") - query = ( - frappe.qb.from_(table) - .select(Sum(table.qty).as_("qty")) - .where((table.docstatus == 1) & (table.so_detail == so_item_name)) - ) - return query.run(pluck="qty")[0] or 0 - if source_parent.has_unit_price_items: # 0 Amount rows (as seen in Unit Price Items) should be mapped as it is pending_amount = flt(source.amount) - flt(source.billed_amt) @@ -1175,11 +1206,7 @@ def make_sales_invoice(source_name, target_doc=None, ignore_permissions=False, a target.amount = flt(source.amount) - flt(source.billed_amt) target.base_amount = target.amount * flt(source_parent.conversion_rate) - target.qty = ( - source.qty - get_billed_qty(source.name) - if (source.qty and source.billed_amt) - else (source.qty if is_unit_price_row(source) else source.qty - source.returned_qty) - ) + target.qty = source.qty if is_unit_price_row(source) else get_pending_qty(source) if source_parent.project: target.cost_center = frappe.db.get_value("Project", source_parent.project, "cost_center") @@ -1215,12 +1242,16 @@ def make_sales_invoice(source_name, target_doc=None, ignore_permissions=False, a "parent": "sales_order", }, "postprocess": update_item, - "condition": lambda doc: ( + "condition": lambda doc: select_item(doc) + and ( True if is_unit_price_row(doc) - else (doc.qty and (doc.base_amount == 0 or abs(doc.billed_amt) < abs(doc.amount))) - ) - and select_item(doc), + else ( + doc.qty + and (doc.base_amount == 0 or abs(doc.billed_amt) < abs(doc.amount)) + and get_pending_qty(doc) > 0 + ) + ), }, "Sales Taxes and Charges": { "doctype": "Sales Taxes and Charges", diff --git a/erpnext/selling/doctype/sales_order/test_sales_order.py b/erpnext/selling/doctype/sales_order/test_sales_order.py index 7a796d090ff..b71e2d6878a 100644 --- a/erpnext/selling/doctype/sales_order/test_sales_order.py +++ b/erpnext/selling/doctype/sales_order/test_sales_order.py @@ -33,6 +33,7 @@ from erpnext.selling.doctype.sales_order.sales_order import ( 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.get_item_details import get_bin_details +from erpnext.stock.utils import InvalidWarehouseCompany class TestSalesOrder(AccountsTestMixin, FrappeTestCase): @@ -225,6 +226,95 @@ class TestSalesOrder(AccountsTestMixin, FrappeTestCase): si1 = make_sales_invoice(so.name) self.assertEqual(len(si1.get("items")), 0) + def test_make_sales_invoice_after_return_and_redelivery(self): + from erpnext.stock.doctype.delivery_note.delivery_note import make_sales_return + + so = make_sales_order(qty=10, rate=100) + dn = create_dn_against_so(so.name, 10) + + dn_return = frappe.get_doc(make_sales_return(dn.name).as_dict()) + dn_return.insert() + dn_return.submit() + + self.assertEqual(len(make_sales_invoice(so.name).get("items")), 0) + + create_dn_against_so(so.name, 10) + + so.load_from_db() + item = so.get("items")[0] + self.assertEqual(item.delivered_qty, 10) + self.assertEqual(item.returned_qty, 10) + + si = make_sales_invoice(so.name) + self.assertEqual(si.get("items")[0].qty, 10) + + def test_make_sales_invoice_bills_ordered_qty_for_partial_delivery(self): + so = make_sales_order(qty=10, rate=100) + create_dn_against_so(so.name, 4) + + si = make_sales_invoice(so.name) + self.assertEqual(si.get("items")[0].qty, 10) + + def test_make_sales_invoice_after_partial_billing_return_and_redelivery(self): + from erpnext.stock.doctype.delivery_note.delivery_note import make_sales_return + + so = make_sales_order(qty=10, rate=100) + dn = create_dn_against_so(so.name, 10) + + si = make_sales_invoice(so.name) + si.get("items")[0].qty = 4 + si.insert() + si.submit() + + dn_return = frappe.get_doc(make_sales_return(dn.name).as_dict()) + dn_return.insert() + dn_return.submit() + create_dn_against_so(so.name, 5) + + so.load_from_db() + item = so.get("items")[0] + self.assertEqual(item.delivered_qty, 5) + self.assertEqual(item.returned_qty, 10) + self.assertEqual(item.billed_amt, 400) + + pending_invoice = make_sales_invoice(so.name) + self.assertEqual(pending_invoice.get("items")[0].qty, 1) + pending_invoice.insert() + pending_invoice.submit() + + so.load_from_db() + self.assertEqual(so.get("items")[0].billed_amt, 500) + + def test_make_sales_invoice_after_partial_billing_multiple_items(self): + so = make_sales_order( + item_list=[ + { + "item_code": "_Test Item", + "warehouse": "_Test Warehouse - _TC", + "qty": 10, + "rate": 100, + }, + { + "item_code": "_Test FG Item", + "warehouse": "_Test Warehouse - _TC", + "qty": 10, + "rate": 100, + }, + ] + ) + + si = make_sales_invoice(so.name) + si.get("items")[0].qty = 4 + si.get("items")[1].qty = 6 + si.insert() + si.submit() + + pending_invoice = make_sales_invoice(so.name) + self.assertEqual( + {item.so_detail: item.qty for item in pending_invoice.get("items")}, + {so.get("items")[0].name: 6, so.get("items")[1].name: 4}, + ) + def test_so_billed_amount_against_return_entry(self): from erpnext.accounts.doctype.sales_invoice.sales_invoice import make_sales_return @@ -558,6 +648,117 @@ class TestSalesOrder(AccountsTestMixin, FrappeTestCase): self.assertEqual(updated_total, prev_total + 1400) self.assertNotEqual(updated_total_in_words, prev_total_in_words) + def test_update_child_adding_new_item_with_warehouse(self): + so = make_sales_order(item_code="_Test Item", qty=4) + + first_item_of_so = so.get("items")[0] + self.assertNotEqual(first_item_of_so.warehouse, "_Test Warehouse 2 - _TC") + + def get_trans_item(warehouse): + return json.dumps( + [ + { + "item_code": first_item_of_so.item_code, + "rate": first_item_of_so.rate, + "qty": first_item_of_so.qty, + "docname": first_item_of_so.name, + "warehouse": warehouse, + }, + {"item_code": "_Test Item 2", "rate": 200, "qty": 7, "warehouse": warehouse}, + ] + ) + + self.assertRaises( + InvalidWarehouseCompany, + update_child_qty_rate, + "Sales Order", + get_trans_item("_Test Warehouse 2 - _TC1"), + so.name, + ) + + self.assertRaisesRegex( + frappe.ValidationError, + "Group node warehouse", + update_child_qty_rate, + "Sales Order", + get_trans_item("_Test Warehouse Group - _TC"), + so.name, + ) + + if not frappe.db.exists("Warehouse", "_Test Disabled Warehouse - _TC"): + frappe.get_doc( + { + "doctype": "Warehouse", + "warehouse_name": "_Test Disabled Warehouse", + "company": "_Test Company", + "disabled": 1, + } + ).insert() + + self.assertRaisesRegex( + frappe.ValidationError, + "Disabled Warehouse", + update_child_qty_rate, + "Sales Order", + get_trans_item("_Test Disabled Warehouse - _TC"), + so.name, + ) + + update_child_qty_rate("Sales Order", get_trans_item("_Test Warehouse 2 - _TC"), so.name) + + so.reload() + # the new row picks up the warehouse selected in the dialog + self.assertEqual(so.get("items")[-1].item_code, "_Test Item 2") + self.assertEqual(so.get("items")[-1].warehouse, "_Test Warehouse 2 - _TC") + # existing rows keep theirs, so their reserved qty stays in the same bin + self.assertEqual(so.get("items")[0].warehouse, first_item_of_so.warehouse) + + def test_update_child_adding_new_item_without_any_default_warehouse(self): + item_code = make_item("_Test Item Without Default Warehouse", {"is_stock_item": 1}).name + so = make_sales_order(item_code="_Test Item", qty=4) + existing_item = so.get("items")[0] + + stock_settings_default = frappe.db.get_single_value("Stock Settings", "default_warehouse") + frappe.db.set_single_value("Stock Settings", "default_warehouse", None) + self.addCleanup( + frappe.db.set_single_value, "Stock Settings", "default_warehouse", stock_settings_default + ) + + def get_trans_items(warehouse=None): + new_row = {"item_code": item_code, "rate": 200, "qty": 7} + if warehouse: + new_row["warehouse"] = warehouse + + return json.dumps( + [ + { + "item_code": existing_item.item_code, + "rate": existing_item.rate, + "qty": existing_item.qty, + "docname": existing_item.name, + }, + new_row, + ] + ) + + # no default in the Item Master, Item Group, Brand or Stock Settings + self.assertRaisesRegex( + frappe.ValidationError, + "Cannot find a default warehouse", + update_child_qty_rate, + "Sales Order", + get_trans_items(), + so.name, + ) + + update_child_qty_rate("Sales Order", get_trans_items("_Test Warehouse - _TC"), so.name) + + so.reload() + self.assertEqual(len(so.get("items")), 2) + self.assertEqual(so.get("items")[0].warehouse, existing_item.warehouse) + self.assertEqual(so.get("items")[-1].item_code, item_code) + self.assertEqual(so.get("items")[-1].warehouse, "_Test Warehouse - _TC") + def test_update_child_removing_item(self): so = make_sales_order(**{"item_list": [{"item_code": "_Test Item", "qty": 5, "rate": 1000}]}) create_dn_against_so(so.name, 2) @@ -2646,6 +2847,17 @@ class TestSalesOrder(AccountsTestMixin, FrappeTestCase): so = make_sales_order(item_code=fg_item, qty=10, rate=50, warehouse=fg_warehouse, do_not_save=1) self.assertRaises(frappe.ValidationError, so.save) + def test_sales_team_allocated_percentage_tolerates_floating_point_drift(self): + # 10.0 + 58.02 + 31.98 accumulates to 100.00000000000001 in binary floating point + so = make_sales_order(do_not_save=True) + for sales_person, percentage in ( + ("_Test Sales Person", 10.0), + ("_Test Sales Person 1", 58.02), + ("_Test Sales Person 2", 31.98), + ): + so.append("sales_team", {"sales_person": sales_person, "allocated_percentage": percentage}) + so.save() + def compare_payment_schedules(doc, doc1, doc2): for index, schedule in enumerate(doc1.get("payment_schedule")): diff --git a/erpnext/setup/doctype/company/company.py b/erpnext/setup/doctype/company/company.py index c53dfa5fb40..05b3ce7aa7b 100644 --- a/erpnext/setup/doctype/company/company.py +++ b/erpnext/setup/doctype/company/company.py @@ -346,6 +346,7 @@ class Company(NestedSet): ) warehouse.flags.ignore_permissions = True warehouse.flags.ignore_mandatory = True + warehouse.flags.ignore_inventory_account_validation = True warehouse.insert() if wh_detail["is_group"]: diff --git a/erpnext/setup/doctype/driver/driver.js b/erpnext/setup/doctype/driver/driver.js index 35f8bff5874..3372b75bc35 100644 --- a/erpnext/setup/doctype/driver/driver.js +++ b/erpnext/setup/doctype/driver/driver.js @@ -23,15 +23,20 @@ frappe.ui.form.on("Driver", { }, transporter: function (frm, cdt, cdn) { - // this assumes that supplier's address has same title as supplier's name if (!frm.doc.transporter) return; - frappe.db - .get_doc("Address", null, { address_title: frm.doc.transporter }) - .then((r) => { - frappe.model.set_value(cdt, cdn, "address", r.name); - }) - .catch((err) => { - console.log(err); - }); + + const transporter = frm.doc.transporter; + frappe.call({ + method: "frappe.contacts.doctype.address.address.get_default_address", + args: { + doctype: "Supplier", + name: transporter, + }, + callback: function (r) { + if (frm.doc.transporter === transporter) { + frappe.model.set_value(cdt, cdn, "address", r.message); + } + }, + }); }, }); diff --git a/erpnext/stock/__init__.py b/erpnext/stock/__init__.py index aa556c62434..19d8bcd21e3 100644 --- a/erpnext/stock/__init__.py +++ b/erpnext/stock/__init__.py @@ -16,6 +16,17 @@ install_docs = [ ] +class WarehouseAccountMap(frappe._dict): + def __missing__(self, warehouse): + account = get_warehouse_account(frappe.get_cached_doc("Warehouse", warehouse)) + account_details = frappe._dict( + account=account, + account_currency=frappe.get_cached_value("Account", account, "account_currency"), + ) + self[warehouse] = account_details + return account_details + + def get_warehouse_account_map(company=None): company_warehouse_account_map = company and frappe.flags.setdefault("warehouse_account_map", {}).get( company @@ -23,7 +34,7 @@ def get_warehouse_account_map(company=None): warehouse_account_map = frappe.flags.warehouse_account_map if not warehouse_account_map or not company_warehouse_account_map or frappe.flags.in_test: - warehouse_account = frappe._dict() + warehouse_account = WarehouseAccountMap() filters = {} if company: @@ -37,7 +48,7 @@ def get_warehouse_account_map(company=None): order_by="lft, rgt", ): if not d.account: - d.account = get_warehouse_account(d, warehouse_account) + d.account = get_warehouse_account(d, warehouse_account, raise_error=False) if d.account: d.account_currency = frappe.db.get_value("Account", d.account, "account_currency", cache=True) @@ -47,10 +58,13 @@ def get_warehouse_account_map(company=None): else: frappe.flags.warehouse_account_map = warehouse_account - return frappe.flags.warehouse_account_map.get(company) or frappe.flags.warehouse_account_map + if company: + return frappe.flags.warehouse_account_map.get(company, WarehouseAccountMap()) + + return frappe.flags.warehouse_account_map -def get_warehouse_account(warehouse, warehouse_account=None): +def get_warehouse_account(warehouse, warehouse_account=None, *, raise_error=True): account = warehouse.account if not account and warehouse.parent_warehouse: if warehouse_account: @@ -86,7 +100,7 @@ def get_warehouse_account(warehouse, warehouse_account=None): if len(inventory_accounts) == 1: account = inventory_accounts[0] - if not account and warehouse.company and not warehouse.is_group: + if raise_error and not account and warehouse.company and not warehouse.is_group: frappe.throw( _("Please set Account in Warehouse {0} or Default Inventory Account in Company {1}").format( warehouse.name, warehouse.company diff --git a/erpnext/stock/doctype/delivery_trip/delivery_trip.py b/erpnext/stock/doctype/delivery_trip/delivery_trip.py index e1e308c735e..fbaafbe73fd 100644 --- a/erpnext/stock/doctype/delivery_trip/delivery_trip.py +++ b/erpnext/stock/doctype/delivery_trip/delivery_trip.py @@ -406,7 +406,7 @@ def notify_customers(delivery_trip): frappe.sendmail( recipients=contact_info.email_id, subject=dispatch_template.subject, - message=frappe.render_template(dispatch_template.response, context), + message=frappe.render_template(dispatch_template.response, context, restrict_globals=True), attachments=get_attachments(stop), ) diff --git a/erpnext/stock/doctype/purchase_receipt/purchase_receipt.py b/erpnext/stock/doctype/purchase_receipt/purchase_receipt.py index 08fe7feff56..25c6fd987f5 100644 --- a/erpnext/stock/doctype/purchase_receipt/purchase_receipt.py +++ b/erpnext/stock/doctype/purchase_receipt/purchase_receipt.py @@ -609,7 +609,7 @@ class PurchaseReceipt(BuyingController): def make_sub_contracting_gl_entries(item): # sub-contracting warehouse - if flt(item.rm_supp_cost) and warehouse_account.get(self.supplier_warehouse): + if flt(item.rm_supp_cost): self.add_gl_entry( gl_entries=gl_entries, account=supplier_warehouse_account, @@ -718,22 +718,22 @@ class PurchaseReceipt(BuyingController): stock_value_diff = ( flt(d.base_net_amount) + flt(d.item_tax_amount) + flt(d.landed_cost_voucher_amount) ) - elif warehouse_account.get(d.warehouse): + elif d.warehouse: stock_value_diff = get_stock_value_difference(self.name, d.name, d.warehouse) stock_asset_account_name = warehouse_account[d.warehouse]["account"] - supplier_warehouse_account = warehouse_account.get(self.supplier_warehouse, {}).get( - "account" - ) - supplier_warehouse_account_currency = warehouse_account.get( - self.supplier_warehouse, {} - ).get("account_currency") + supplier_warehouse_details = warehouse_account.get(self.supplier_warehouse, {}) + if flt(d.rm_supp_cost): + supplier_warehouse_details = warehouse_account[self.supplier_warehouse] + + supplier_warehouse_account = supplier_warehouse_details.get("account") + supplier_warehouse_account_currency = supplier_warehouse_details.get("account_currency") # If PR is sub-contracted and fg item rate is zero # in that case if account for source and target warehouse are same, # then GL entries should not be posted if ( flt(stock_value_diff) == flt(d.rm_supp_cost) - and warehouse_account.get(self.supplier_warehouse) + and supplier_warehouse_account and stock_asset_account_name == supplier_warehouse_account ): continue diff --git a/erpnext/stock/doctype/purchase_receipt/test_purchase_receipt.py b/erpnext/stock/doctype/purchase_receipt/test_purchase_receipt.py index 471de86f0a7..40cf324010d 100644 --- a/erpnext/stock/doctype/purchase_receipt/test_purchase_receipt.py +++ b/erpnext/stock/doctype/purchase_receipt/test_purchase_receipt.py @@ -4996,6 +4996,66 @@ class TestPurchaseReceipt(FrappeTestCase): self.assertEqual(frappe.parse_json(stock_queue), [[20, 0.0]]) + def test_purchase_return_valuation_for_batchwise_valuation_batch(self): + from erpnext.controllers.sales_and_purchase_return import make_return_doc + from erpnext.stock.doctype.delivery_note.test_delivery_note import create_delivery_note + + item_code = make_item( + "Test Purchase Return Batchwise Valn Item", + { + "is_stock_item": 1, + "has_batch_no": 1, + "batch_number_series": "BN-TPRBWV-.#####", + }, + ).name + + batch_no = "BN-TPRBWV-00001" + batch = frappe.new_doc("Batch").update({"batch_id": batch_no, "item": item_code}).insert() + self.assertEqual(batch.use_batchwise_valuation, 1) + + warehouse = "_Test Warehouse - _TC" + pr = make_purchase_receipt( + item_code=item_code, + qty=100, + rate=1000, + warehouse=warehouse, + batch_no=batch_no, + use_serial_batch_fields=1, + ) + make_purchase_receipt( + item_code=item_code, + qty=100, + rate=400, + warehouse=warehouse, + batch_no=batch_no, + use_serial_batch_fields=1, + ) + create_delivery_note( + item_code=item_code, + qty=100, + warehouse=warehouse, + batch_no=batch_no, + use_serial_batch_fields=1, + ) + + return_pr = make_return_doc("Purchase Receipt", pr.name) + return_pr.submit() + + sle = frappe.db.get_value( + "Stock Ledger Entry", + {"voucher_no": return_pr.name, "is_cancelled": 0}, + ["stock_value_difference", "qty_after_transaction", "stock_value", "serial_and_batch_bundle"], + as_dict=True, + ) + self.assertEqual(flt(sle.qty_after_transaction), 0.0) + self.assertEqual(flt(sle.stock_value_difference, 2), -70000.0) + self.assertEqual(flt(sle.stock_value, 2), 0.0) + + rate = frappe.db.get_value( + "Serial and Batch Entry", {"parent": sle.serial_and_batch_bundle}, "incoming_rate" + ) + self.assertEqual(flt(rate, 2), 700.0) + def test_negative_stock_error_for_purchase_return(self): from erpnext.controllers.sales_and_purchase_return import make_return_doc from erpnext.stock.doctype.stock_entry.test_stock_entry import make_stock_entry diff --git a/erpnext/stock/doctype/quality_inspection/quality_inspection.py b/erpnext/stock/doctype/quality_inspection/quality_inspection.py index a7733b0bf8b..1fbf323f0dd 100644 --- a/erpnext/stock/doctype/quality_inspection/quality_inspection.py +++ b/erpnext/stock/doctype/quality_inspection/quality_inspection.py @@ -253,6 +253,9 @@ class QualityInspection(Document): self.modified, ) + if self.reference_type and self.reference_name: + frappe.get_lazy_doc(self.reference_type, self.reference_name).notify_update() + def inspect_and_set_status(self): for reading in self.readings: if not reading.manual_inspection: # dont auto set status if manual diff --git a/erpnext/stock/doctype/quality_inspection/test_quality_inspection.py b/erpnext/stock/doctype/quality_inspection/test_quality_inspection.py index e49a0b3b678..17b9a82c492 100644 --- a/erpnext/stock/doctype/quality_inspection/test_quality_inspection.py +++ b/erpnext/stock/doctype/quality_inspection/test_quality_inspection.py @@ -1,6 +1,8 @@ # Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors and Contributors # See license.txt +from unittest.mock import patch + import frappe from frappe.tests.utils import FrappeTestCase, change_settings from frappe.utils import nowdate @@ -58,6 +60,27 @@ class TestQualityInspection(FrappeTestCase): qa.delete() dn.delete() + def test_doc_update_published_for_reference_on_submit(self): + """Submitting a QI publishes doc_update so open reference forms resync their timestamp.""" + dn = create_delivery_note(item_code="_Test Item with QA", do_not_submit=True) + qa = create_quality_inspection( + reference_type="Delivery Note", reference_name=dn.name, do_not_submit=True + ) + + with patch.object(frappe, "publish_realtime") as publish_realtime: + qa.submit() + + reference_updates = [ + call + for call in publish_realtime.call_args_list + if call.args and call.args[0] == "doc_update" and call.kwargs.get("docname") == dn.name + ] + self.assertEqual(len(reference_updates), 1) + + message = reference_updates[0].args[1] + self.assertEqual(message["doctype"], "Delivery Note") + self.assertEqual(message["modified"], frappe.db.get_value("Delivery Note", dn.name, "modified")) + def test_value_based_qi_readings(self): # Test QI based on acceptance values (Non formula) dn = create_delivery_note(item_code="_Test Item with QA", do_not_submit=True) diff --git a/erpnext/stock/doctype/repost_item_valuation/test_repost_item_valuation.py b/erpnext/stock/doctype/repost_item_valuation/test_repost_item_valuation.py index 5291b2e4381..85c7372be59 100644 --- a/erpnext/stock/doctype/repost_item_valuation/test_repost_item_valuation.py +++ b/erpnext/stock/doctype/repost_item_valuation/test_repost_item_valuation.py @@ -475,6 +475,55 @@ class TestRepostItemValuation(FrappeTestCase, StockTestMixin): # incoming rate after reposting should be 150 self.assertSLEs(se, [{"incoming_rate": 150}]) + def test_repost_multi_line_moving_average_return(self): + from erpnext.controllers.sales_and_purchase_return import make_return_doc + + item = self.make_item(properties={"valuation_method": "Moving Average"}).name + warehouse = "_Test Warehouse - _TC" + + make_purchase_receipt(item_code=item, qty=100, rate=100, warehouse=warehouse) + + pr = make_purchase_receipt(item_code=item, qty=400, rate=200, warehouse=warehouse, do_not_submit=1) + for qty in (100, 300, 100): + pr.append( + "items", + { + "item_code": item, + "warehouse": warehouse, + "qty": qty, + "received_qty": qty, + "rate": 200, + "uom": pr.items[0].uom, + "conversion_factor": 1.0, + }, + ) + pr.save() + pr.submit() + + return_pr = make_return_doc(pr.doctype, pr.name) + return_pr.save() + return_pr.submit() + + expected_sles = [ + {"outgoing_rate": 190.0, "valuation_rate": 190.0, "qty_after_transaction": 600.0}, + {"outgoing_rate": 190.0, "valuation_rate": 190.0, "qty_after_transaction": 500.0}, + {"outgoing_rate": 190.0, "valuation_rate": 190.0, "qty_after_transaction": 200.0}, + {"outgoing_rate": 190.0, "valuation_rate": 190.0, "qty_after_transaction": 100.0}, + ] + + for _ in range(2): + riv = frappe.get_doc( + doctype="Repost Item Valuation", + based_on="Transaction", + voucher_type=pr.doctype, + voucher_no=pr.name, + posting_date=pr.posting_date, + posting_time=pr.posting_time, + ) + riv.submit() + + self.assertSLEs(return_pr, expected_sles) + def test_remove_attached_file(self): item_code = make_item("_Test Remove Attached File Item", properties={"is_stock_item": 1}) 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 ca18baac969..0b614be1596 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 @@ -402,6 +402,13 @@ class SerialandBatchBundle(Document): valuation_method = get_valuation_method(self.item_code) + # An outward return must go out at the batch's current average rate for a + # batchwise valuation batch. The original receipt rate is only correct while + # the batch still holds stock at that rate; once other receipts have changed + # the average, removing at the original rate strands a residue in the batch + # value (negative when returning the costlier receipt). + batchwise_avg_rates = self.get_batchwise_return_avg_rates() + stock_queue = [] non_batchwise_batches = [] if not self.has_serial_no and valuation_method == "FIFO": @@ -435,6 +442,12 @@ class SerialandBatchBundle(Document): batches = sorted(list(valuation_details["batches"].keys())) valuation_rate = valuation_details["batches"].get(batches[cint(row.idx) - 1]) + # a batch with an available balance goes out at its current average rate (a + # valid 0.0 included); the original receipt rate applies only when there is + # no balance to average + if not row.serial_no and row.batch_no in batchwise_avg_rates: + valuation_rate = batchwise_avg_rates[row.batch_no] + row.incoming_rate = flt(valuation_rate) row.stock_value_difference = flt(row.qty) * flt(row.incoming_rate) @@ -463,6 +476,41 @@ class SerialandBatchBundle(Document): elif self.type_of_transaction == "Inward": self.set_incoming_rate_for_inward_transaction(row, save, prev_sle=prev_sle) + def get_batchwise_return_avg_rates(self): + from erpnext.stock.utils import get_valuation_method + + if self.type_of_transaction != "Outward" or self.has_serial_no: + return {} + + batch_nos = [d.batch_no for d in self.entries if d.batch_no] + if not batch_nos: + return {} + + if get_valuation_method(self.item_code) == "Moving Average" and frappe.db.get_single_value( + "Stock Settings", "do_not_use_batchwise_valuation" + ): + return {} + + batchwise_batches = frappe.get_all( + "Batch", + filters={"name": ("in", batch_nos), "use_batchwise_valuation": 1}, + pluck="name", + ) + if not batchwise_batches: + return {} + + # scoped to batchwise batches only, so BatchNoValuation's non-batchwise + # machinery never runs for them + sle = self.get_sle_for_outward_transaction() + sle.batch_nos = {batch_no: sle.batch_nos[batch_no] for batch_no in batchwise_batches} + sle.batchwise_valuation_batches = batchwise_batches + sn_obj = BatchNoValuation(sle=sle, item_code=self.item_code, warehouse=self.warehouse) + return { + batch_no: abs(flt(sn_obj.batch_avg_rate.get(batch_no))) + for batch_no in batchwise_batches + if flt(sn_obj.available_qty.get(batch_no)) + } + def validate_returned_serial_batch_no(self, return_against, row, original_inv_details): if frappe.flags.through_repost_item_valuation: return diff --git a/erpnext/stock/doctype/stock_reservation_entry/stock_reservation_entry.py b/erpnext/stock/doctype/stock_reservation_entry/stock_reservation_entry.py index ca31f33bf76..32b42d85cdc 100644 --- a/erpnext/stock/doctype/stock_reservation_entry/stock_reservation_entry.py +++ b/erpnext/stock/doctype/stock_reservation_entry/stock_reservation_entry.py @@ -882,7 +882,7 @@ def get_ssb_bundle_for_voucher(sre: dict) -> object: def has_reserved_stock(voucher_type: str, voucher_no: str, voucher_detail_no: str | None = None) -> bool: """Returns True if there is any Stock Reservation Entry for the given voucher.""" - if get_stock_reservation_entries_for_voucher( + if _get_stock_reservation_entries_for_voucher( voucher_type, voucher_no, voucher_detail_no, fields=["name"], ignore_status=True ): return True @@ -1113,7 +1113,7 @@ def cancel_stock_reservation_entries( sre_list = {} if voucher_type and voucher_no: - sre_list = get_stock_reservation_entries_for_voucher( + sre_list = _get_stock_reservation_entries_for_voucher( voucher_type, voucher_no, voucher_detail_no, fields=["name"] ) elif from_voucher_type and from_voucher_no: @@ -1156,6 +1156,24 @@ def get_stock_reservation_entries_for_voucher( ) -> list[dict]: """Returns list of Stock Reservation Entries against a Voucher.""" + return _get_stock_reservation_entries_for_voucher( + voucher_type, voucher_no, voucher_detail_no, fields, ignore_status, ignore_permissions=False + ) + + +def _get_stock_reservation_entries_for_voucher( + voucher_type: str, + voucher_no: str, + voucher_detail_no: str | None = None, + fields: list[str] | None = None, + ignore_status: bool = False, + ignore_permissions: bool = True, +) -> list[dict]: + """Returns list of Stock Reservation Entries against a Voucher.""" + + if not ignore_permissions: + frappe.has_permission(voucher_type, doc=voucher_no, throw=True) + if not fields or not isinstance(fields, list): fields = [ "name", @@ -1169,14 +1187,11 @@ def get_stock_reservation_entries_for_voucher( sre = frappe.qb.DocType("Stock Reservation Entry") query = ( - frappe.qb.from_(sre) + frappe.get_query(sre, fields=fields) .where((sre.docstatus == 1) & (sre.voucher_type == voucher_type) & (sre.voucher_no == voucher_no)) .orderby(sre.creation) ) - for field in fields: - query = query.select(sre[field]) - if voucher_detail_no: query = query.where(sre.voucher_detail_no == voucher_detail_no) diff --git a/erpnext/stock/doctype/stock_reservation_entry/test_stock_reservation_entry.py b/erpnext/stock/doctype/stock_reservation_entry/test_stock_reservation_entry.py index 942c7f482ae..deba0d967c8 100644 --- a/erpnext/stock/doctype/stock_reservation_entry/test_stock_reservation_entry.py +++ b/erpnext/stock/doctype/stock_reservation_entry/test_stock_reservation_entry.py @@ -13,9 +13,9 @@ from erpnext.stock.doctype.item.test_item import make_item from erpnext.stock.doctype.stock_entry.stock_entry import StockEntry from erpnext.stock.doctype.stock_entry.test_stock_entry import make_stock_entry from erpnext.stock.doctype.stock_reservation_entry.stock_reservation_entry import ( + _get_stock_reservation_entries_for_voucher, cancel_stock_reservation_entries, get_sre_reserved_qty_details_for_voucher, - get_stock_reservation_entries_for_voucher, has_reserved_stock, ) from erpnext.stock.utils import get_stock_balance @@ -278,7 +278,7 @@ class TestStockReservationEntry(FrappeTestCase): self.assertTrue(has_reserved_stock("Sales Order", so.name)) for item in so.items: - sre_details = get_stock_reservation_entries_for_voucher( + sre_details = _get_stock_reservation_entries_for_voucher( "Sales Order", so.name, item.name, fields=["reserved_qty", "status"] )[0] self.assertEqual(item.stock_reserved_qty, sre_details.reserved_qty) @@ -335,7 +335,7 @@ class TestStockReservationEntry(FrappeTestCase): dn1.submit() for item in so.items: - sre_details = get_stock_reservation_entries_for_voucher( + sre_details = _get_stock_reservation_entries_for_voucher( "Sales Order", so.name, item.name, fields=["delivered_qty", "status"] )[0] self.assertGreater(sre_details.delivered_qty, 0) @@ -352,7 +352,7 @@ class TestStockReservationEntry(FrappeTestCase): dn2.submit() for item in so.items: - sre_details = get_stock_reservation_entries_for_voucher( + sre_details = _get_stock_reservation_entries_for_voucher( "Sales Order", so.name, item.name, @@ -396,7 +396,7 @@ class TestStockReservationEntry(FrappeTestCase): so.load_from_db() for item in so.items: - sre_details = get_stock_reservation_entries_for_voucher( + sre_details = _get_stock_reservation_entries_for_voucher( "Sales Order", so.name, item.name, fields=["status", "reserved_qty"] )[0] @@ -411,7 +411,7 @@ class TestStockReservationEntry(FrappeTestCase): dn.submit() for item in so.items: - sre_details = get_stock_reservation_entries_for_voucher( + sre_details = _get_stock_reservation_entries_for_voucher( "Sales Order", so.name, item.name, fields=["status", "delivered_qty", "reserved_qty"] )[0] @@ -459,7 +459,7 @@ class TestStockReservationEntry(FrappeTestCase): so.load_from_db() for item in so.items: - sre_details = get_stock_reservation_entries_for_voucher( + sre_details = _get_stock_reservation_entries_for_voucher( "Sales Order", so.name, item.name, @@ -520,7 +520,7 @@ class TestStockReservationEntry(FrappeTestCase): so.load_from_db() for item in so.items: - sre_details = get_stock_reservation_entries_for_voucher( + sre_details = _get_stock_reservation_entries_for_voucher( "Sales Order", so.name, item.name, fields=["reserved_qty"] )[0] diff --git a/erpnext/stock/doctype/stock_settings/stock_settings.json b/erpnext/stock/doctype/stock_settings/stock_settings.json index 58ea8083087..488eefeefc7 100644 --- a/erpnext/stock/doctype/stock_settings/stock_settings.json +++ b/erpnext/stock/doctype/stock_settings/stock_settings.json @@ -131,7 +131,8 @@ "description": "The percentage you are allowed to receive or deliver more against the quantity ordered. For example, if you have ordered 100 units, and your Allowance is 10%, then you are allowed to receive 110 units.", "fieldname": "over_delivery_receipt_allowance", "fieldtype": "Float", - "label": "Over Delivery/Receipt Allowance (%)" + "label": "Over Delivery/Receipt Allowance (%)", + "non_negative": 1 }, { "default": "Stop", @@ -282,7 +283,8 @@ "description": "The percentage you are allowed to transfer more against the quantity ordered. For example, if you have ordered 100 units, and your Allowance is 10%, then you are allowed transfer 110 units.", "fieldname": "mr_qty_allowance", "fieldtype": "Float", - "label": "Over Transfer Allowance" + "label": "Over Transfer Allowance", + "non_negative": 1 }, { "default": "0", @@ -446,7 +448,8 @@ "description": "The percentage you are allowed to pick more items in the pick list than the ordered quantity.", "fieldname": "over_picking_allowance", "fieldtype": "Percent", - "label": "Over Picking Allowance" + "label": "Over Picking Allowance", + "non_negative": 1 }, { "default": "1", @@ -528,7 +531,7 @@ "index_web_pages_for_search": 1, "issingle": 1, "links": [], - "modified": "2026-03-27 22:39:16.812184", + "modified": "2026-08-01 23:35:02.896836", "modified_by": "Administrator", "module": "Stock", "name": "Stock Settings", diff --git a/erpnext/stock/doctype/stock_settings/stock_settings.py b/erpnext/stock/doctype/stock_settings/stock_settings.py index f573c35925b..2924e8c70f9 100644 --- a/erpnext/stock/doctype/stock_settings/stock_settings.py +++ b/erpnext/stock/doctype/stock_settings/stock_settings.py @@ -101,6 +101,7 @@ class StockSettings(Document): ) self.validate_warehouses() + self.validate_over_delivery_receipt_allowance() self.cant_change_valuation_method() self.validate_clean_description_html() self.validate_pending_reposts() @@ -110,6 +111,10 @@ class StockSettings(Document): self.change_precision_for_purchase() self.validate_do_not_use_batchwise_valuation() + def validate_over_delivery_receipt_allowance(self): + if not self.over_delivery_receipt_allowance: + self.role_allowed_to_over_deliver_receive = None + def validate_do_not_use_batchwise_valuation(self): doc_before_save = self.get_doc_before_save() if not doc_before_save: diff --git a/erpnext/stock/doctype/warehouse/test_warehouse.py b/erpnext/stock/doctype/warehouse/test_warehouse.py index 5b6f8f727fb..4417e89a882 100644 --- a/erpnext/stock/doctype/warehouse/test_warehouse.py +++ b/erpnext/stock/doctype/warehouse/test_warehouse.py @@ -112,6 +112,7 @@ class TestWarehouse(FrappeTestCase): frappe.delete_doc("Account", "Extra Inventory Account - _TCIF") warehouse = frappe.get_doc("Warehouse", {"company": company, "is_group": 0}) + warehouse.db_set("account", None) single_account = frappe.db.get_value( "Account", {"account_type": "Stock", "is_group": 0, "company": company}, "name" ) @@ -125,6 +126,101 @@ class TestWarehouse(FrappeTestCase): ) self.assertRaises(frappe.ValidationError, get_warehouse_account, warehouse) + def test_unrelated_warehouse_without_inventory_account_is_ignored(self): + from erpnext.stock import get_warehouse_account_map + + company, warehouse = create_ambiguous_inventory_account_warehouse() + warehouse_account_map = get_warehouse_account_map(company) + resolved_warehouse = next(iter(warehouse_account_map)) + + self.assertNotIn(warehouse.name, warehouse_account_map) + self.assertTrue(warehouse_account_map[resolved_warehouse].account) + + def test_direct_warehouse_account_map_lookup_remains_strict(self): + from erpnext.stock import get_warehouse_account_map + + company, warehouse = create_ambiguous_inventory_account_warehouse() + + with self.assertRaises(frappe.ValidationError): + get_warehouse_account_map(company)[warehouse.name] + + def test_new_warehouse_requires_inventory_account(self): + company, _warehouse = create_ambiguous_inventory_account_warehouse() + frappe.db.set_value("Company", company, "enable_perpetual_inventory", 1) + parent_warehouse = frappe.db.get_value("Warehouse", {"company": company, "is_group": 1}, "name") + frappe.db.set_value("Warehouse", parent_warehouse, "account", None) + warehouse = frappe.get_doc( + { + "doctype": "Warehouse", + "warehouse_name": "Missing Inventory Account", + "parent_warehouse": parent_warehouse, + "company": company, + } + ) + + self.assertRaisesRegex(frappe.ValidationError, "Missing Inventory Account - _TCIF", warehouse.insert) + + def test_new_warehouse_can_inherit_inventory_account(self): + from erpnext.stock import get_warehouse_account + + company, _warehouse = create_ambiguous_inventory_account_warehouse() + frappe.db.set_value("Company", company, "enable_perpetual_inventory", 1) + parent_warehouse = frappe.db.get_value("Warehouse", {"company": company, "is_group": 1}, "name") + inventory_account = frappe.db.get_value( + "Account", {"company": company, "account_type": "Stock", "is_group": 0}, "name" + ) + frappe.db.set_value("Warehouse", parent_warehouse, "account", inventory_account) + + warehouse = frappe.get_doc( + { + "doctype": "Warehouse", + "warehouse_name": "Inherited Inventory Account", + "parent_warehouse": parent_warehouse, + "company": company, + } + ).insert() + + self.assertEqual(get_warehouse_account(warehouse), inventory_account) + + def test_new_warehouse_inherits_from_parent_created_in_same_transaction(self): + from erpnext.stock import get_warehouse_account + + company, _warehouse = create_ambiguous_inventory_account_warehouse() + frappe.db.set_value("Company", company, "enable_perpetual_inventory", 1) + root_warehouse = frappe.db.get_value("Warehouse", {"company": company, "is_group": 1}, "name") + inventory_account = frappe.db.get_value( + "Account", {"company": company, "account_type": "Stock", "is_group": 0}, "name" + ) + + parent_warehouse = frappe.get_doc( + { + "doctype": "Warehouse", + "warehouse_name": "New Parent Warehouse", + "parent_warehouse": root_warehouse, + "company": company, + "is_group": 1, + "account": inventory_account, + } + ).insert() + child_warehouse = frappe.get_doc( + { + "doctype": "Warehouse", + "warehouse_name": "New Child Warehouse", + "parent_warehouse": parent_warehouse.name, + "company": company, + } + ).insert() + + self.assertEqual(get_warehouse_account(child_warehouse), inventory_account) + + def test_warehouse_onload_allows_missing_inventory_account(self): + company, warehouse = create_ambiguous_inventory_account_warehouse() + frappe.db.set_value("Company", company, "enable_perpetual_inventory", 1) + + warehouse.run_method("onload") + + self.assertNotIn("account", warehouse.get_onload()) + def create_inventory_fallback_company(): company = "_Test Company Inventory Fallback" @@ -142,6 +238,38 @@ def create_inventory_fallback_company(): return company +def create_ambiguous_inventory_account_warehouse(): + company = create_inventory_fallback_company() + frappe.db.set_value("Company", company, "default_inventory_account", None) + + single_account = frappe.db.get_value( + "Account", {"account_type": "Stock", "is_group": 0, "company": company}, "name" + ) + warehouses = frappe.get_all( + "Warehouse", filters={"company": company, "is_group": 0}, pluck="name", order_by="name" + ) + for warehouse_name in warehouses: + frappe.db.set_value("Warehouse", warehouse_name, "account", single_account) + + for group_warehouse in frappe.get_all( + "Warehouse", filters={"company": company, "is_group": 1}, pluck="name" + ): + frappe.db.set_value("Warehouse", group_warehouse, "account", None) + + warehouse = frappe.get_doc("Warehouse", warehouses[0]) + warehouse.db_set({"account": None, "disabled": 0}) + + if not frappe.db.exists("Account", "Extra Inventory Account - _TCIF"): + create_account( + account_name="Extra Inventory Account", + parent_account=frappe.db.get_value("Account", single_account, "parent_account"), + account_type="Stock", + company=company, + ) + + return company, warehouse + + def create_warehouse(warehouse_name, properties=None, company=None): if not company: company = "_Test Company" diff --git a/erpnext/stock/doctype/warehouse/warehouse.py b/erpnext/stock/doctype/warehouse/warehouse.py index b9600fddc9b..e92b617600e 100644 --- a/erpnext/stock/doctype/warehouse/warehouse.py +++ b/erpnext/stock/doctype/warehouse/warehouse.py @@ -55,15 +55,35 @@ class Warehouse(NestedSet): def onload(self): """load account name for General Ledger Report""" if self.company and cint(frappe.db.get_value("Company", self.company, "enable_perpetual_inventory")): - account = self.account or get_warehouse_account(self) + account = self.account or get_warehouse_account(self, raise_error=False) if account: self.set_onload("account", account) load_address_and_contact(self) def validate(self): + self.validate_inventory_account() self.warn_about_multiple_warehouse_account() + def validate_inventory_account(self): + if ( + not self.is_new() + or not self.company + or self.flags.ignore_inventory_account_validation + or not frappe.get_cached_value("Company", self.company, "enable_perpetual_inventory") + ): + return + + warehouse = frappe._dict(self.as_dict()) + if not self.account and self.parent_warehouse: + parent_bounds = frappe.db.get_value( + "Warehouse", self.parent_warehouse, ["lft", "rgt"], as_dict=True + ) + if parent_bounds: + warehouse.update(parent_bounds) + + get_warehouse_account(warehouse) + def on_update(self): self.update_nsm_model() diff --git a/erpnext/stock/report/stock_ledger/stock_ledger.py b/erpnext/stock/report/stock_ledger/stock_ledger.py index 2152fce31c0..6c4ce0844cf 100644 --- a/erpnext/stock/report/stock_ledger/stock_ledger.py +++ b/erpnext/stock/report/stock_ledger/stock_ledger.py @@ -7,8 +7,10 @@ from collections import defaultdict import frappe from frappe import _ -from frappe.query_builder.functions import CombineDatetime, Sum +from frappe.query_builder.functions import CombineDatetime, IfNull, Sum from frappe.utils import cint, flt, get_datetime +from pypika import Order +from pypika.analytics import RowNumber from erpnext.stock.doctype.inventory_dimension.inventory_dimension import get_inventory_dimensions from erpnext.stock.doctype.serial_no.serial_no import get_serial_nos @@ -53,14 +55,15 @@ def execute(filters=None): data = [] conversion_factors = [] - if opening_row: - data.append(opening_row) + opening_rows = opening_row if isinstance(opening_row, list) else ([opening_row] if opening_row else []) + for row in opening_rows: + data.append(row) conversion_factors.append(0) actual_qty = stock_value = 0 - if opening_row: - actual_qty = opening_row.get("qty_after_transaction") - stock_value = opening_row.get("stock_value") + if opening_rows: + actual_qty = opening_rows[0].get("qty_after_transaction", 0) + stock_value = opening_rows[0].get("stock_value", 0) available_serial_nos = {} @@ -687,43 +690,120 @@ def get_opening_balance(filters, columns, sl_entries, inv_dimension_wise_value=N if not (filters.item_code and filters.warehouse and filters.from_date): return - from erpnext.stock.stock_ledger import get_previous_sle + item_codes = filters.item_code + if isinstance(item_codes, str): + item_codes = [item_codes] - project = None - if filters.get("project") and not frappe.get_all( - "Inventory Dimension", filters={"reference_document": "Project"} - ): - project = filters.get("project") + warehouses = get_matching_warehouses(filters.warehouse) + if not warehouses: + return - last_entry = get_previous_sle( - { - "item_code": filters.item_code, - "warehouse_condition": get_warehouse_condition(filters.warehouse), - "posting_date": filters.from_date, - "posting_time": "00:00:00", - "project": project, - }, - for_report=True, + sle_doctype = frappe.qb.DocType("Stock Ledger Entry") + sr_doctype = frappe.qb.DocType("Stock Reconciliation") + + opening_reco_query = ( + frappe.qb.from_(sle_doctype) + .inner_join(sr_doctype) + .on(sle_doctype.voucher_no == sr_doctype.name) + .select(sle_doctype.voucher_no) + .where(sle_doctype.docstatus < 2) + .where(sle_doctype.is_cancelled == 0) + .where(sle_doctype.item_code.isin(item_codes)) + .where(sle_doctype.warehouse.isin(warehouses)) + .where(sle_doctype.voucher_type == "Stock Reconciliation") + .where(sle_doctype.posting_date == filters.from_date) + .where(sr_doctype.purpose == "Opening Stock") ) - # check if any SLEs are actually Opening Stock Reconciliation - for sle in list(sl_entries): - if ( - sle.get("voucher_type") == "Stock Reconciliation" - and sle.posting_date == filters.from_date - and frappe.db.get_value("Stock Reconciliation", sle.voucher_no, "purpose") == "Opening Stock" - ): - last_entry = sle - sl_entries.remove(sle) + opening_reco_vouchers = set(opening_reco_query.run(pluck=True)) - row = { + if opening_reco_vouchers: + sl_entries[:] = [sle for sle in sl_entries if sle.get("voucher_no") not in opening_reco_vouchers] + + sle_cond = (sle_doctype.posting_date < filters.from_date) | ( + (sle_doctype.posting_date == filters.from_date) & (sle_doctype.posting_time == "00:00:00") + ) + if opening_reco_vouchers: + sle_cond = sle_cond | ( + (sle_doctype.posting_date == filters.from_date) + & (sle_doctype.voucher_no.isin(list(opening_reco_vouchers))) + ) + + subq = ( + frappe.qb.from_(sle_doctype) + .select( + sle_doctype.qty_after_transaction, + sle_doctype.stock_value, + RowNumber() + .over(sle_doctype.item_code, sle_doctype.warehouse) + .orderby(sle_doctype.posting_datetime, sle_doctype.creation, sle_doctype.name, order=Order.desc) + .as_("rn"), + ) + .where(sle_doctype.docstatus < 2) + .where(sle_doctype.is_cancelled == 0) + .where(sle_doctype.item_code.isin(item_codes)) + .where(sle_doctype.warehouse.isin(warehouses)) + .where(sle_cond) + ) + + for field in ["voucher_no", "project", "company"]: + if filters.get(field): + subq = subq.where(sle_doctype[field] == filters.get(field)) + + inventory_dimension_fields = get_inventory_dimension_fields() + if inventory_dimension_fields: + for fieldname in inventory_dimension_fields: + if filters.get(fieldname): + subq = subq.where(sle_doctype[fieldname].isin(filters.get(fieldname))) + + query = ( + frappe.qb.from_(subq) + .select( + IfNull(Sum(subq.qty_after_transaction), 0.0).as_("total_qty"), + IfNull(Sum(subq.stock_value), 0.0).as_("total_stock_value"), + ) + .where(subq.rn == 1) + ) + + res = query.run(as_dict=True) + + total_qty = flt(res[0].total_qty) if res else 0.0 + total_stock_value = flt(res[0].total_stock_value) if res else 0.0 + valuation_rate = flt(total_stock_value / total_qty) if total_qty else 0.0 + + return { "item_code": _("'Opening'"), - "qty_after_transaction": last_entry.get("qty_after_transaction", 0), - "valuation_rate": last_entry.get("valuation_rate", 0), - "stock_value": last_entry.get("stock_value", 0), + "qty_after_transaction": total_qty, + "valuation_rate": valuation_rate, + "stock_value": total_stock_value, } - return row + +def get_matching_warehouses(warehouses): + if not warehouses: + return [] + + if isinstance(warehouses, str): + warehouses = [warehouses] + + warehouse_details = frappe.get_all( + "Warehouse", + filters={"name": ("in", warehouses)}, + fields=["lft", "rgt"], + ) + + if not warehouse_details: + return warehouses + + wh = frappe.qb.DocType("Warehouse") + cond = None + for d in warehouse_details: + c = (wh.lft >= d.lft) & (wh.rgt <= d.rgt) + cond = c if cond is None else (cond | c) + + matching = (frappe.qb.from_(wh).select(wh.name).where(cond)).run(pluck=True) + + return matching if matching else warehouses def get_warehouse_condition(warehouses): @@ -779,7 +859,15 @@ def get_opening_balance_for_inv_dimension(filters, inv_dimension_wise_value): if not filters.item_code or not filters.warehouse or not filters.from_date: return - if len(filters.get("item_code")) > 1 or len(filters.get("warehouse")) > 1: + item_codes = filters.get("item_code") + if isinstance(item_codes, str): + item_codes = [item_codes] + + warehouses = filters.get("warehouse") + if isinstance(warehouses, str): + warehouses = [warehouses] + + if len(item_codes) > 1 or len(warehouses) > 1: return sl_doctype = frappe.qb.DocType("Stock Ledger Entry") @@ -799,17 +887,11 @@ def get_opening_balance_for_inv_dimension(filters, inv_dimension_wise_value): ) ) - if filters.get("item_code"): - if isinstance(filters.item_code, list | tuple): - query = query.where(sl_doctype.item_code.isin(filters.item_code)) - else: - query = query.where(sl_doctype.item_code == filters.item_code) + if item_codes: + query = query.where(sl_doctype.item_code.isin(item_codes)) - if filters.get("warehouse"): - if isinstance(filters.warehouse, list | tuple): - query = query.where(sl_doctype.warehouse.isin(filters.warehouse)) - else: - query = query.where(sl_doctype.warehouse == filters.warehouse) + if warehouses: + query = query.where(sl_doctype.warehouse.isin(warehouses)) for key, value in inv_dimension_wise_value.items(): if isinstance(value, list | tuple): diff --git a/erpnext/stock/report/stock_ledger/test_stock_ledger_report.py b/erpnext/stock/report/stock_ledger/test_stock_ledger_report.py index d57052e905f..a41658a45c8 100644 --- a/erpnext/stock/report/stock_ledger/test_stock_ledger_report.py +++ b/erpnext/stock/report/stock_ledger/test_stock_ledger_report.py @@ -5,20 +5,335 @@ import frappe from frappe.tests.utils import FrappeTestCase from frappe.utils import add_days, today -from erpnext.maintenance.doctype.maintenance_schedule.test_maintenance_schedule import ( - make_serial_item_with_serial, -) +from erpnext.stock.doctype.stock_entry.stock_entry_utils import make_stock_entry +from erpnext.stock.report.stock_ledger.stock_ledger import execute + +WAREHOUSE = "Stores - _TC" -class TestStockLedgerReeport(FrappeTestCase): - def setUp(self) -> None: - make_serial_item_with_serial("_Test Stock Report Serial Item") - self.filters = frappe._dict( - company="_Test Company", - from_date=today(), - to_date=add_days(today(), 30), - item_code=["_Test Stock Report Serial Item"], - ) +class TestStockLedgerReport(FrappeTestCase): + """Correctness tests for the Stock Ledger report. + + A shared `make_movements`/`run` pair keeps each test small without persisting + any data: movements are created per test and rolled back, while the report runs + read-only. Tests reuse bootstrap items and transact in `Stores - _TC`, which + starts clean (zero balance) for these items. + """ def tearDown(self) -> None: frappe.db.rollback() + + def make_movements(self, item_code, movements): + for movement in movements: + make_stock_entry(item_code=item_code, **movement) + + def run_report(self, item_code, from_date=None, to_date=None): + filters = frappe._dict( + company="_Test Company", + from_date=from_date or add_days(today(), -1), + to_date=to_date or today(), + item_code=[item_code], + warehouse=WAREHOUSE, + ) + return list(execute(filters)[1]) + + def test_in_out_quantities_and_running_balance(self): + item = "_Test Item" + self.make_movements( + item, + [ + {"qty": 10, "to_warehouse": WAREHOUSE, "basic_rate": 100}, + {"qty": 4, "from_warehouse": WAREHOUSE}, + ], + ) + + rows = self.run_report(item) + receipt = next(row for row in rows if row.get("in_qty")) + issue = next(row for row in rows if row.get("out_qty")) + + self.assertEqual(receipt["in_qty"], 10) + self.assertEqual(receipt["qty_after_transaction"], 10) + self.assertEqual(issue["out_qty"], -4) + self.assertEqual(issue["qty_after_transaction"], 6) + + def test_opening_balance_reflects_movements_before_from_date(self): + item = "_Test Item" + self.make_movements( + item, + [ + { + "qty": 10, + "to_warehouse": WAREHOUSE, + "basic_rate": 100, + "posting_date": add_days(today(), -10), + }, + {"qty": 4, "from_warehouse": WAREHOUSE, "posting_date": today()}, + ], + ) + + rows = self.run_report(item, from_date=add_days(today(), -5), to_date=today()) + + # the receipt predates the range, so it surfaces as the opening balance + self.assertEqual(rows[0]["item_code"], "'Opening'") + self.assertEqual(rows[0]["qty_after_transaction"], 10) + + # the in-range issue draws down from the opening balance + issue = next(row for row in rows if row.get("out_qty")) + self.assertEqual(issue["qty_after_transaction"], 6) + + def test_filters_to_requested_item_only(self): + item_a = "_Test Item" + item_b = "_Test Item 2" + self.make_movements(item_a, [{"qty": 5, "to_warehouse": WAREHOUSE, "basic_rate": 100}]) + self.make_movements(item_b, [{"qty": 7, "to_warehouse": WAREHOUSE, "basic_rate": 100}]) + + rows = self.run_report(item_a) + item_codes = {row["item_code"] for row in rows if row.get("voucher_no")} + self.assertEqual(item_codes, {item_a}) + + def test_multi_item_opening_balance_with_and_without_transactions(self): + item_a = "_Test Item" + item_b = "_Test Item 2" + self.make_movements( + item_a, + [ + { + "qty": 10, + "to_warehouse": WAREHOUSE, + "basic_rate": 100, + "posting_date": add_days(today(), -10), + } + ], + ) + self.make_movements( + item_b, + [{"qty": 5, "to_warehouse": WAREHOUSE, "basic_rate": 50, "posting_date": add_days(today(), -10)}], + ) + self.make_movements( + item_a, + [{"qty": 2, "from_warehouse": WAREHOUSE, "posting_date": today()}], + ) + + filters = frappe._dict( + company="_Test Company", + from_date=add_days(today(), -5), + to_date=today(), + item_code=[item_a, item_b], + warehouse=WAREHOUSE, + ) + columns, rows = execute(filters) + + opening_rows = [row for row in rows if row.get("item_code") == "'Opening'"] + self.assertEqual(len(opening_rows), 1) + self.assertEqual(opening_rows[0]["qty_after_transaction"], 15) + + def test_multi_warehouse_opening_balance_aggregation(self): + item = "_Test Item" + warehouse_1 = "Stores - _TC" + warehouse_2 = "Finished Goods - _TC" + + self.make_movements( + item, + [ + { + "qty": 10, + "to_warehouse": warehouse_1, + "basic_rate": 100, + "posting_date": add_days(today(), -10), + }, + { + "qty": 20, + "to_warehouse": warehouse_2, + "basic_rate": 100, + "posting_date": add_days(today(), -10), + }, + ], + ) + + filters = frappe._dict( + company="_Test Company", + from_date=add_days(today(), -5), + to_date=today(), + item_code=[item], + warehouse=[warehouse_1, warehouse_2], + ) + columns, rows = execute(filters) + + opening_rows = [row for row in rows if row.get("item_code") == "'Opening'"] + self.assertEqual(len(opening_rows), 1) + self.assertEqual(opening_rows[0]["qty_after_transaction"], 30) + + def test_opening_stock_reconciliation_on_from_date_non_midnight_time(self): + from erpnext.stock.doctype.stock_reconciliation.test_stock_reconciliation import ( + create_stock_reconciliation, + ) + + item = "_Test Item" + from_date = today() + + sr = create_stock_reconciliation( + item_code=item, + warehouse=WAREHOUSE, + qty=25, + rate=100, + posting_date=from_date, + posting_time="10:30:00", + purpose="Opening Stock", + do_not_submit=False, + ) + + filters = frappe._dict( + company="_Test Company", + from_date=from_date, + to_date=from_date, + item_code=[item], + warehouse=WAREHOUSE, + ) + columns, rows = execute(filters) + + opening_rows = [row for row in rows if row.get("item_code") == "'Opening'"] + self.assertEqual(len(opening_rows), 1) + self.assertEqual(opening_rows[0]["qty_after_transaction"], 25) + + # Ensure the Opening Stock Reconciliation is not duplicated in detail transaction rows + reco_rows = [row for row in rows if row.get("voucher_no") == sr.name] + self.assertEqual(len(reco_rows), 0) + + def test_backdated_sle_independent_maxima_handling(self): + item = "_Test Item" + # Entry 1: Later posting date (2026-07-20), created first + self.make_movements( + item, + [ + { + "qty": 10, + "to_warehouse": WAREHOUSE, + "basic_rate": 100, + "posting_date": add_days(today(), -10), + } + ], + ) + # Entry 2: Backdated posting date (2026-07-15), created LATER + self.make_movements( + item, + [ + { + "qty": 5, + "to_warehouse": WAREHOUSE, + "basic_rate": 100, + "posting_date": add_days(today(), -15), + } + ], + ) + + filters = frappe._dict( + company="_Test Company", + from_date=add_days(today(), -5), + to_date=today(), + item_code=[item], + warehouse=WAREHOUSE, + ) + columns, rows = execute(filters) + + opening_rows = [row for row in rows if row.get("item_code") == "'Opening'"] + self.assertEqual(len(opening_rows), 1) + # Should correctly pick the latest posting date entry (15 Qty) despite backdated creation order + self.assertEqual(opening_rows[0]["qty_after_transaction"], 15) + + def test_filtered_opening_balance_does_not_pick_excluded_creation_entry(self): + item = "_Test Item" + posting_date = add_days(today(), -10) + posting_time = "09:00:00" + + included_entry = make_stock_entry( + item_code=item, + qty=10, + to_warehouse=WAREHOUSE, + basic_rate=100, + posting_date=posting_date, + posting_time=posting_time, + ) + make_stock_entry( + item_code=item, + qty=50, + to_warehouse=WAREHOUSE, + basic_rate=100, + posting_date=posting_date, + posting_time=posting_time, + ) + + filters = frappe._dict( + company="_Test Company", + from_date=add_days(today(), -5), + to_date=today(), + item_code=[item], + warehouse=WAREHOUSE, + voucher_no=included_entry.name, + ) + columns, rows = execute(filters) + + opening_rows = [row for row in rows if row.get("item_code") == "'Opening'"] + self.assertEqual(len(opening_rows), 1) + self.assertEqual(opening_rows[0]["qty_after_transaction"], 10) + + def test_tied_creation_terminal_sle_is_not_summed_twice(self): + item = "_Test Item" + posting_date = add_days(today(), -10) + posting_time = "09:00:00" + + stock_entry_1 = make_stock_entry( + item_code=item, + qty=10, + to_warehouse=WAREHOUSE, + basic_rate=100, + posting_date=posting_date, + posting_time=posting_time, + ) + stock_entry_2 = make_stock_entry( + item_code=item, + qty=5, + to_warehouse=WAREHOUSE, + basic_rate=100, + posting_date=posting_date, + posting_time=posting_time, + ) + + sle_rows = frappe.get_all( + "Stock Ledger Entry", + filters={ + "voucher_type": "Stock Entry", + "voucher_no": ("in", [stock_entry_1.name, stock_entry_2.name]), + "item_code": item, + "warehouse": WAREHOUSE, + "is_cancelled": 0, + }, + fields=["name", "qty_after_transaction"], + order_by="name desc", + ) + self.assertEqual(len(sle_rows), 2) + + for sle in sle_rows: + frappe.db.set_value( + "Stock Ledger Entry", + sle.name, + "creation", + "2026-01-01 00:00:00.000000", + update_modified=False, + ) + + filters = frappe._dict( + company="_Test Company", + from_date=add_days(today(), -5), + to_date=today(), + item_code=[item], + warehouse=WAREHOUSE, + ) + columns, rows = execute(filters) + + opening_rows = [row for row in rows if row.get("item_code") == "'Opening'"] + self.assertEqual(len(opening_rows), 1) + self.assertEqual(opening_rows[0]["qty_after_transaction"], sle_rows[0].qty_after_transaction) + self.assertNotEqual( + opening_rows[0]["qty_after_transaction"], + sum(sle.qty_after_transaction for sle in sle_rows), + ) 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 ffb024acfb1..ca5e3eec3c8 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 @@ -7,6 +7,8 @@ import frappe from frappe import _ from frappe.utils import cint, flt, get_link_to_form, parse_json +from erpnext.stock.utils import get_valuation_method + SLE_FIELDS = ( "name", "posting_date", @@ -53,6 +55,8 @@ def add_invariant_check_fields(sles, filters): balance_qty = 0.0 balance_stock_value = 0.0 + valuation_method = get_valuation_method(filters.item_code) + incorrect_idx = None float_precision = cint(frappe.db.get_single_value("System Settings", "float_precision")) or 3 currency_precision = ( @@ -90,7 +94,7 @@ def add_invariant_check_fields(sles, filters): ) sle.diff_value_diff = sle.stock_value_from_diff - sle.stock_value - if maintains_fifo_queue(sle): + if maintains_fifo_queue(sle, valuation_method): add_fifo_fields(sle, sles[idx - 1] if idx else None) if incorrect_idx is None and not is_sle_has_correct_data(sle, float_precision, currency_precision): @@ -104,8 +108,10 @@ def add_invariant_check_fields(sles, filters): return sles -def maintains_fifo_queue(sle): - # no queue is maintained for serialized/batchwise-valued stock +def maintains_fifo_queue(sle, valuation_method): + if valuation_method == "Moving Average": + return False + return not ( sle.serial_and_batch_bundle or sle.serial_no or (sle.batch_no and sle.use_batchwise_valuation) ) @@ -138,6 +144,8 @@ 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 + and flt(sle.fifo_qty_diff, float_precision) == 0.0 + and flt(sle.fifo_value_diff, currency_precision) == 0.0 ) 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 index b82e341c84a..67382375862 100644 --- 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 @@ -1,6 +1,8 @@ # Copyright (c) 2026, Frappe Technologies Pvt. Ltd. and Contributors # See license.txt +import json + import frappe from frappe.tests.utils import FrappeTestCase @@ -60,6 +62,34 @@ class TestStockLedgerInvariantCheck(FrappeTestCase): self.assertEqual(len(data), 2) # incorrect entry + one before it for context self.assertEqual(data[-1].name, sle.name) + def test_show_incorrect_entries_catches_queue_mismatch(self): + item = self.make_movements() + + sle = frappe.get_last_doc( + "Stock Ledger Entry", {"item_code": item, "warehouse": WAREHOUSE, "is_cancelled": 0} + ) + tampered_queue = json.dumps([[sle.qty_after_transaction + 5, 100]]) + frappe.db.set_value("Stock Ledger Entry", sle.name, "stock_queue", tampered_queue) + + data = self.run_report(item_code=item, show_incorrect_entries=1) + self.assertEqual(len(data), 2) + self.assertEqual(data[-1].name, sle.name) + + def test_moving_average_item_skips_fifo_queue_checks(self): + from erpnext.stock.doctype.item.test_item import make_item + + item = make_item(properties={"valuation_method": "Moving Average"}).name + make_stock_entry(item_code=item, to_warehouse=WAREHOUSE, qty=10, rate=100) + make_stock_entry(item_code=item, from_warehouse=WAREHOUSE, qty=4) + + 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), []) + 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-.####"} diff --git a/erpnext/stock/serial_batch_bundle.py b/erpnext/stock/serial_batch_bundle.py index 4e0d69134bb..967fc8bea3e 100644 --- a/erpnext/stock/serial_batch_bundle.py +++ b/erpnext/stock/serial_batch_bundle.py @@ -774,6 +774,9 @@ class SerialNoValuation(DeprecatedSerialNoValuation): return is_rejected(self.sle.voucher_type, self.sle.voucher_detail_no, self.sle.warehouse) def get_incoming_rate(self): + if not self.sle.actual_qty and self.sle.voucher_type == "Stock Reconciliation": + return 0.0 + return abs(flt(self.stock_value_change) / flt(self.sle.actual_qty)) def get_incoming_rate_of_serial_no(self, serial_no): @@ -910,6 +913,11 @@ class BatchNoValuation(DeprecatedBatchNoValuation): self.batchwise_valuation_batches = [] self.non_batchwise_valuation_batches = [] + if batchwise_batches := self.sle.get("batchwise_valuation_batches"): + self.batchwise_valuation_batches = list(batchwise_batches) + self.non_batchwise_valuation_batches = list(set(self.batches) - set(batchwise_batches)) + return + if get_valuation_method(self.sle.item_code) == "Moving Average" and frappe.db.get_single_value( "Stock Settings", "do_not_use_batchwise_valuation" ): diff --git a/erpnext/stock/stock_ledger.py b/erpnext/stock/stock_ledger.py index a2cce420e3a..7a9bc900307 100644 --- a/erpnext/stock/stock_ledger.py +++ b/erpnext/stock/stock_ledger.py @@ -38,7 +38,6 @@ from erpnext.stock.doctype.stock_reservation_entry.stock_reservation_entry impor from erpnext.stock.utils import ( get_combine_datetime, get_incoming_outgoing_rate_for_cancel, - get_incoming_rate, get_or_make_bin, get_serial_nos_data, get_stock_balance, @@ -66,7 +65,7 @@ def make_sl_entries(sl_entries, allow_negative_stock=False, via_landed_cost_vouc such cases certain validations need to be ignored (like negative stock) """ - from erpnext.controllers.stock_controller import future_sle_exists + from erpnext.controllers.stock_controller import future_sle_exists, invalidate_future_sle_cache if sl_entries: cancelled = sl_entries[0].get("is_cancelled") @@ -114,6 +113,8 @@ def make_sl_entries(sl_entries, allow_negative_stock=False, via_landed_cost_vouc _("Item {0} ignored since it is not a stock item").format(args.get("item_code")) ) + invalidate_future_sle_cache(sl_entries[0].get("voucher_type"), sl_entries[0].get("voucher_no")) + def repost_current_voucher(args, allow_negative_stock=False, via_landed_cost_voucher=False): if args.get("actual_qty") or args.get("voucher_type") == "Stock Reconciliation": @@ -1260,23 +1261,7 @@ class update_entries_after: and not sle.get("batch_no") and not sle.get("serial_and_batch_bundle") ): - rate = get_incoming_rate( - { - "item_code": sle.item_code, - "warehouse": sle.warehouse, - "posting_date": sle.posting_date, - "posting_time": sle.posting_time, - "qty": sle.actual_qty, - "serial_no": sle.get("serial_no"), - "batch_no": sle.get("batch_no"), - "serial_and_batch_bundle": sle.get("serial_and_batch_bundle"), - "company": sle.company, - "voucher_type": sle.voucher_type, - "voucher_no": sle.voucher_no, - "allow_zero_valuation": self.allow_zero_rate, - "sle": sle.name, - } - ) + rate = self.get_moving_average_rate_for_return(sle) if not rate and sle.voucher_type in ["Delivery Note", "Sales Invoice"]: rate = get_rate_for_return( @@ -1344,6 +1329,38 @@ class update_entries_after: return rate + def get_moving_average_rate_for_return(self, sle): + """Rate just before this entry, taken from the in-memory running state so a + multi-line return never reads a sibling row of its own voucher.""" + rate = flt(self.wh_data.valuation_rate) + if rate: + return rate + + previous_sle = get_previous_sle_of_current_voucher( + frappe._dict( + item_code=sle.item_code, + warehouse=sle.warehouse, + posting_date=sle.posting_date, + posting_time=sle.posting_time, + voucher_no=sle.voucher_no, + ), + exclude_current_voucher=True, + ) + + rate = previous_sle.get("valuation_rate") + if rate is None: + rate = get_valuation_rate( + sle.item_code, + sle.warehouse, + sle.voucher_type, + sle.voucher_no, + self.allow_zero_rate, + currency=erpnext.get_company_currency(sle.company), + company=sle.company, + ) + + return flt(rate) + def update_outgoing_rate_on_transaction(self, sle): """ Update outgoing rate in Stock Entry, Delivery Note, Sales Invoice and Sales Return diff --git a/erpnext/subcontracting/doctype/subcontracting_receipt/subcontracting_receipt.py b/erpnext/subcontracting/doctype/subcontracting_receipt/subcontracting_receipt.py index 8be5f453632..501fde5b3dd 100644 --- a/erpnext/subcontracting/doctype/subcontracting_receipt/subcontracting_receipt.py +++ b/erpnext/subcontracting/doctype/subcontracting_receipt/subcontracting_receipt.py @@ -633,7 +633,7 @@ class SubcontractingReceipt(SubcontractingController): for item in self.items: if flt(item.rate) and flt(item.qty): - if warehouse_account and warehouse_account.get(item.warehouse): + if warehouse_account is not None: stock_value_diff = frappe.db.get_value( "Stock Ledger Entry", { @@ -647,9 +647,11 @@ class SubcontractingReceipt(SubcontractingController): ) accepted_warehouse_account = warehouse_account[item.warehouse]["account"] - supplier_warehouse_account = warehouse_account.get(self.supplier_warehouse, {}).get( - "account" - ) + supplier_warehouse_details = warehouse_account.get(self.supplier_warehouse, {}) + if flt(item.rm_supp_cost): + supplier_warehouse_details = warehouse_account[self.supplier_warehouse] + + supplier_warehouse_account = supplier_warehouse_details.get("account") remarks = self.get("remarks") or _("Accounting Entry for Stock") # Accepted Warehouse Account (Debit)