From 1e929e2c6cb4c8041cc13a30911516122f6c2cdf Mon Sep 17 00:00:00 2001 From: Diptanil Saha Date: Thu, 26 Jun 2025 16:26:53 +0530 Subject: [PATCH] fix: pos opening and closing validation (#48059) * fix: pos opening and closing validation * test: pos opening entry tests * test: added test for pos opening entry * fix: patch to set status cancelled on cancelled POS Opening Entry and POS Closing Entry * fix: error messages --- .../pos_closing_entry/pos_closing_entry.json | 4 +- .../pos_closing_entry/pos_closing_entry.py | 20 +++- .../doctype/pos_invoice/pos_invoice.py | 12 --- .../pos_opening_entry/pos_opening_entry.py | 38 +++++++ .../test_pos_opening_entry.py | 100 +++++++++++++++++- .../doctype/sales_invoice/sales_invoice.py | 32 ++++++ erpnext/patches.txt | 1 + ...pos_opening_entry_and_pos_closing_entry.py | 14 +++ .../page/point_of_sale/pos_controller.js | 29 ++++- 9 files changed, 226 insertions(+), 24 deletions(-) create mode 100644 erpnext/patches/v15_0/set_status_cancelled_on_cancelled_pos_opening_entry_and_pos_closing_entry.py diff --git a/erpnext/accounts/doctype/pos_closing_entry/pos_closing_entry.json b/erpnext/accounts/doctype/pos_closing_entry/pos_closing_entry.json index 09d2dc0c758..feb58d705a6 100644 --- a/erpnext/accounts/doctype/pos_closing_entry/pos_closing_entry.json +++ b/erpnext/accounts/doctype/pos_closing_entry/pos_closing_entry.json @@ -74,6 +74,7 @@ "label": "User Details" }, { + "fetch_from": "pos_opening_entry.company", "fetch_if_empty": 1, "fieldname": "company", "fieldtype": "Link", @@ -103,6 +104,7 @@ "fieldtype": "Link", "label": "Cashier", "options": "User", + "read_only": 1, "reqd": 1 }, { @@ -259,7 +261,7 @@ "link_fieldname": "pos_closing_entry" } ], - "modified": "2025-06-06 12:00:31.955176", + "modified": "2025-06-14 02:38:14.962291", "modified_by": "Administrator", "module": "Accounts", "name": "POS Closing Entry", diff --git a/erpnext/accounts/doctype/pos_closing_entry/pos_closing_entry.py b/erpnext/accounts/doctype/pos_closing_entry/pos_closing_entry.py index e7ef1c15dcf..ea93b23aa11 100644 --- a/erpnext/accounts/doctype/pos_closing_entry/pos_closing_entry.py +++ b/erpnext/accounts/doctype/pos_closing_entry/pos_closing_entry.py @@ -209,13 +209,16 @@ class POSClosingEntry(StatusUpdater): def on_submit(self): consolidate_pos_invoices(closing_entry=self) frappe.publish_realtime( - f"poe_{self.pos_opening_entry}_closed", - self, + f"poe_{self.pos_opening_entry}", + message={"operation": "Closed", "doc": self}, docname=f"POS Opening Entry/{self.pos_opening_entry}", ) self.update_sales_invoices_closing_entry() + def before_cancel(self): + self.check_pce_is_cancellable() + def on_cancel(self): unconsolidate_pos_invoices(closing_entry=self) @@ -237,6 +240,15 @@ class POSClosingEntry(StatusUpdater): "Sales Invoice", d.sales_invoice, "pos_closing_entry", self.name if not cancel else None ) + def check_pce_is_cancellable(self): + if frappe.db.exists("POS Opening Entry", {"pos_profile": self.pos_profile, "status": "Open"}): + frappe.throw( + title=_("Cannot cancel POS Closing Entry"), + msg=_( + "POS Profile - {0} is currently open. Please close the POS or cancel the existing POS Opening Entry before cancelling this POS Closing Entry." + ).format(frappe.bold(self.pos_profile)), + ) + @frappe.whitelist() @frappe.validate_and_sanitize_search_inputs @@ -267,7 +279,7 @@ def get_invoices(start, end, pos_profile, user): def get_payments(invoices): if not len(invoices): - return + return [] invoices_name = [d.name for d in invoices] @@ -301,7 +313,7 @@ def get_payments(invoices): def get_taxes(invoices): if not len(invoices): - return + return [] invoices_name = [d.name for d in invoices] diff --git a/erpnext/accounts/doctype/pos_invoice/pos_invoice.py b/erpnext/accounts/doctype/pos_invoice/pos_invoice.py index ce9e04c2869..195bda08151 100644 --- a/erpnext/accounts/doctype/pos_invoice/pos_invoice.py +++ b/erpnext/accounts/doctype/pos_invoice/pos_invoice.py @@ -373,18 +373,6 @@ class POSInvoice(SalesInvoice): _("Payment related to {0} is not completed").format(pay.mode_of_payment) ) - def validate_pos_opening_entry(self): - opening_entries = frappe.get_list( - "POS Opening Entry", filters={"pos_profile": self.pos_profile, "status": "Open", "docstatus": 1} - ) - if len(opening_entries) == 0: - frappe.throw( - title=_("POS Opening Entry Missing"), - msg=_("No open POS Opening Entry found for POS Profile {0}.").format( - frappe.bold(self.pos_profile) - ), - ) - def validate_stock_availablility(self): if self.is_return: return diff --git a/erpnext/accounts/doctype/pos_opening_entry/pos_opening_entry.py b/erpnext/accounts/doctype/pos_opening_entry/pos_opening_entry.py index 7f1890ceabf..d8768055437 100644 --- a/erpnext/accounts/doctype/pos_opening_entry/pos_opening_entry.py +++ b/erpnext/accounts/doctype/pos_opening_entry/pos_opening_entry.py @@ -37,6 +37,8 @@ class POSOpeningEntry(StatusUpdater): def validate(self): self.validate_pos_profile_and_cashier() + self.check_open_pos_exists() + self.check_user_already_assigned() self.validate_payment_method_account() self.set_status() @@ -49,6 +51,22 @@ class POSOpeningEntry(StatusUpdater): if not cint(frappe.db.get_value("User", self.user, "enabled")): frappe.throw(_("User {} is disabled. Please select valid user/cashier").format(self.user)) + def check_open_pos_exists(self): + if frappe.db.exists("POS Opening Entry", {"pos_profile": self.pos_profile, "status": "Open"}): + frappe.throw( + title=_("POS Opening Entry Exists"), + msg=_( + "{0} is open. Close the POS or cancel the existing POS Opening Entry to create a new POS Opening Entry." + ).format(frappe.bold(self.pos_profile)), + ) + + def check_user_already_assigned(self): + if frappe.db.exists("POS Opening Entry", {"user": self.user, "status": "Open"}): + frappe.throw( + title=_("Cannot Assign Cashier"), + msg=_("Cashier is currently assigned to another POS."), + ) + def validate_payment_method_account(self): invalid_modes = [] for d in self.balance_details: @@ -71,5 +89,25 @@ class POSOpeningEntry(StatusUpdater): def on_submit(self): self.set_status(update=True) + def before_cancel(self): + self.check_poe_is_cancellable() + def on_cancel(self): self.set_status(update=True) + frappe.publish_realtime( + f"poe_{self.name}", + message={"operation": "Cancelled"}, + docname=f"POS Opening Entry/{self.name}", + ) + + def check_poe_is_cancellable(self): + from erpnext.accounts.doctype.pos_closing_entry.pos_closing_entry import get_invoices + + invoices = get_invoices( + self.period_start_date, frappe.utils.get_datetime(), self.pos_profile, self.user + ) + if invoices.get("invoices"): + frappe.throw( + title=_("POS Opening Entry Cancellation Error"), + msg=_("POS Opening Entry cannot be cancelled as unconsolidated Invoices exists."), + ) diff --git a/erpnext/accounts/doctype/pos_opening_entry/test_pos_opening_entry.py b/erpnext/accounts/doctype/pos_opening_entry/test_pos_opening_entry.py index 03bfc03bde2..e371e5408fc 100644 --- a/erpnext/accounts/doctype/pos_opening_entry/test_pos_opening_entry.py +++ b/erpnext/accounts/doctype/pos_opening_entry/test_pos_opening_entry.py @@ -3,14 +3,107 @@ import unittest import frappe +from frappe.core.doctype.user_permission.test_user_permission import create_user from frappe.tests import IntegrationTestCase +from erpnext.accounts.doctype.pos_invoice.test_pos_invoice import create_pos_invoice +from erpnext.accounts.doctype.pos_profile.test_pos_profile import make_pos_profile +from erpnext.stock.doctype.stock_entry.test_stock_entry import make_stock_entry + class TestPOSOpeningEntry(IntegrationTestCase): - pass + @classmethod + def setUpClass(cls): + frappe.db.sql("delete from `tabPOS Opening Entry`") + cls.enterClassContext(cls.change_settings("POS Settings", {"invoice_type": "POS Invoice"})) + + @classmethod + def tearDownClass(cls): + frappe.db.sql("delete from `tabPOS Opening Entry`") + + def setUp(self): + # Make stock available for POS Sales + frappe.db.sql("delete from `tabPOS Opening Entry`") + make_stock_entry(target="_Test Warehouse - _TC", qty=2, basic_rate=100) + from erpnext.accounts.doctype.pos_closing_entry.test_pos_closing_entry import init_user_and_profile + + self.init_user_and_profile = init_user_and_profile + + def tearDown(self): + frappe.set_user("Administrator") + frappe.db.sql("delete from `tabPOS Profile`") + + def test_pos_opening_entry(self): + test_user, pos_profile = self.init_user_and_profile() + opening_entry = create_opening_entry(pos_profile, test_user.name) + + self.assertEqual(opening_entry.status, "Open") + self.assertNotEqual(opening_entry.docstatus, 0) + + def test_multiple_pos_opening_entries_for_same_pos_profile(self): + test_user, pos_profile = self.init_user_and_profile() + opening_entry = create_opening_entry(pos_profile, test_user.name) + + self.assertEqual(opening_entry.status, "Open") + with self.assertRaises(frappe.ValidationError): + create_opening_entry(pos_profile, test_user.name) + + def test_multiple_pos_opening_entry_for_multiple_pos_profiles(self): + test_user, pos_profile = self.init_user_and_profile() + opening_entry_1 = create_opening_entry(pos_profile, test_user.name) + + self.assertEqual(opening_entry_1.status, "Open") + self.assertEqual(opening_entry_1.user, test_user.name) + + cashier_user = create_user("test_cashier@example.com", "Accounts Manager", "Sales Manager") + frappe.set_user(cashier_user.name) + + pos_profile2 = make_pos_profile(name="_Test POS Profile 2") + opening_entry_2 = create_opening_entry(pos_profile2, cashier_user.name) + + self.assertEqual(opening_entry_2.status, "Open") + self.assertEqual(opening_entry_2.user, cashier_user.name) + + def test_multiple_pos_opening_entry_for_same_pos_profile_by_multiple_user(self): + test_user, pos_profile = self.init_user_and_profile() + cashier_user = create_user("test_cashier@example.com", "Accounts Manager", "Sales Manager") + + opening_entry = create_opening_entry(pos_profile, test_user.name) + self.assertEqual(opening_entry.status, "Open") + + with self.assertRaises(frappe.ValidationError): + create_opening_entry(pos_profile, cashier_user.name) + + def test_user_assignment_to_multiple_pos_profile(self): + test_user, pos_profile = self.init_user_and_profile() + opening_entry_1 = create_opening_entry(pos_profile, test_user.name) + self.assertEqual(opening_entry_1.user, test_user.name) + + pos_profile2 = make_pos_profile(name="_Test POS Profile 2") + with self.assertRaises(frappe.ValidationError): + create_opening_entry(pos_profile2, test_user.name) + + def test_cancel_pos_opening_entry_without_invoices(self): + test_user, pos_profile = self.init_user_and_profile() + opening_entry = create_opening_entry(pos_profile, test_user.name, get_obj=True) + + opening_entry.cancel() + self.assertEqual(opening_entry.status, "Cancelled") + self.assertNotEqual(opening_entry.docstatus, 1) + + def test_cancel_pos_opening_entry_with_invoice(self): + test_user, pos_profile = self.init_user_and_profile() + opening_entry = create_opening_entry(pos_profile, test_user.name, get_obj=True) + + pos_inv1 = create_pos_invoice(pos_profile=pos_profile.name, rate=100, do_not_save=1) + pos_inv1.append("payments", {"mode_of_payment": "Cash", "account": "Cash - _TC", "amount": 100}) + pos_inv1.save() + pos_inv1.submit() + + self.assertRaises(frappe.ValidationError, opening_entry.cancel) -def create_opening_entry(pos_profile, user): +def create_opening_entry(pos_profile, user, get_obj=False): entry = frappe.new_doc("POS Opening Entry") entry.pos_profile = pos_profile.name entry.user = user @@ -24,4 +117,7 @@ def create_opening_entry(pos_profile, user): entry.set("balance_details", balance_details) entry.submit() + if get_obj: + return entry + return entry.as_dict() diff --git a/erpnext/accounts/doctype/sales_invoice/sales_invoice.py b/erpnext/accounts/doctype/sales_invoice/sales_invoice.py index bee314e0b82..9e3c1c58aec 100644 --- a/erpnext/accounts/doctype/sales_invoice/sales_invoice.py +++ b/erpnext/accounts/doctype/sales_invoice/sales_invoice.py @@ -3,6 +3,7 @@ import frappe +import frappe.utils from frappe import _, msgprint, throw from frappe.contacts.doctype.address.address import get_address_display from frappe.model.mapper import get_mapped_doc @@ -1100,6 +1101,8 @@ class SalesInvoice(SellingController): if self.invoice_type_in_pos == "POS Invoice" and not self.is_return: frappe.throw(_("Transactions using Sales Invoice in POS are disabled.")) + self.validate_pos_opening_entry() + def validate_full_payment(self): invoice_total = flt(self.rounded_total) or flt(self.grand_total) @@ -1116,6 +1119,35 @@ class SalesInvoice(SellingController): exc=PartialPaymentValidationError, ) + def validate_pos_opening_entry(self): + opening_entries = frappe.get_all( + "POS Opening Entry", + fields=["name", "period_start_date"], + filters={"pos_profile": self.pos_profile, "status": "Open"}, + order_by="period_start_date desc", + ) + if not opening_entries: + frappe.throw( + title=_("POS Opening Entry Missing"), + msg=_("No open POS Opening Entry found for POS Profile {0}.").format( + frappe.bold(self.pos_profile) + ), + ) + if len(opening_entries) > 1: + frappe.throw( + title=_("Multiple POS Opening Entry"), + msg=_( + "POS Profile - {0} has multiple open POS Opening Entries. Please close or cancel the existing entries before proceeding." + ).format(self.pos_profile), + ) + if frappe.utils.get_date_str(opening_entries[0].get("period_start_date")) != frappe.utils.today(): + frappe.throw( + title=_("Outdated POS Opening Entry"), + msg=_( + "POS Opening Entry - {0} is outdated. Please close the POS and create a new POS Opening Entry." + ).format(opening_entries[0].get("name")), + ) + def validate_warehouse(self): super().validate_warehouse() diff --git a/erpnext/patches.txt b/erpnext/patches.txt index 69f5ec159fa..3561e147d59 100644 --- a/erpnext/patches.txt +++ b/erpnext/patches.txt @@ -423,3 +423,4 @@ erpnext.patches.v15_0.update_pick_list_fields execute:frappe.db.set_single_value("Accounts Settings", "confirm_before_resetting_posting_date", 1) erpnext.patches.v15_0.rename_pos_closing_entry_fields #2025-06-13 erpnext.patches.v15_0.update_pegged_currencies +erpnext.patches.v15_0.set_status_cancelled_on_cancelled_pos_opening_entry_and_pos_closing_entry diff --git a/erpnext/patches/v15_0/set_status_cancelled_on_cancelled_pos_opening_entry_and_pos_closing_entry.py b/erpnext/patches/v15_0/set_status_cancelled_on_cancelled_pos_opening_entry_and_pos_closing_entry.py new file mode 100644 index 00000000000..3a5ee01ac0b --- /dev/null +++ b/erpnext/patches/v15_0/set_status_cancelled_on_cancelled_pos_opening_entry_and_pos_closing_entry.py @@ -0,0 +1,14 @@ +import frappe +from frappe.query_builder import DocType + + +def execute(): + POSOpeningEntry = DocType("POS Opening Entry") + POSClosingEntry = DocType("POS Closing Entry") + + frappe.qb.update(POSOpeningEntry).set(POSOpeningEntry.status, "Cancelled").where( + POSOpeningEntry.docstatus == 2 + ).run() + frappe.qb.update(POSClosingEntry).set(POSClosingEntry.status, "Cancelled").where( + POSClosingEntry.docstatus == 2 + ).run() diff --git a/erpnext/selling/page/point_of_sale/pos_controller.js b/erpnext/selling/page/point_of_sale/pos_controller.js index f05a2471eca..5166c895367 100644 --- a/erpnext/selling/page/point_of_sale/pos_controller.js +++ b/erpnext/selling/page/point_of_sale/pos_controller.js @@ -155,6 +155,7 @@ erpnext.PointOfSale.Controller = class { this.fetch_invoice_fields(); this.setup_listener_for_pos_closing(); + this.check_outdated_pos_opening_entry(); } async fetch_invoice_fields() { @@ -174,16 +175,22 @@ erpnext.PointOfSale.Controller = class { } setup_listener_for_pos_closing() { - frappe.realtime.on(`poe_${this.pos_opening}_closed`, (data) => { + frappe.realtime.on(`poe_${this.pos_opening}`, (data) => { const route = frappe.get_route_str(); if (data && route == "point-of-sale") { frappe.dom.freeze(); + const title = + data.operation === "Closed" ? __("POS Closed") : __("POS Opening Entry Cancelled"); + const msg = + data.operation === "Closed" + ? __("POS has been closed at {0}. Please refresh the page.", [ + frappe.datetime.str_to_user(data.doc?.creation).bold(), + ]) + : __("POS Opening Entry has been cancelled. Please refresh the page."); frappe.msgprint({ - title: __("POS Closed"), + title: title, indicator: "orange", - message: __("POS has been closed at {0}. Please refresh the page.", [ - frappe.datetime.str_to_user(data.creation).bold(), - ]), + message: msg, primary_action_label: __("Refresh"), primary_action: { action() { @@ -195,6 +202,18 @@ erpnext.PointOfSale.Controller = class { }); } + check_outdated_pos_opening_entry() { + if (frappe.datetime.get_day_diff(frappe.datetime.get_today(), this.pos_opening_time.slice(0, 10))) { + frappe.msgprint({ + title: __("Outdated POS Opening Entry"), + message: __( + "The current POS opening entry is outdated. Please close it and create a new one." + ), + indicator: "yellow", + }); + } + } + set_opening_entry_status() { this.page.set_title_sub( `