From 8439516f18c698c0cbf45a686cf4f06c59f49cf4 Mon Sep 17 00:00:00 2001 From: Corentin Forler <8860073-cforler_dokos@users.noreply.gitlab.com> Date: Fri, 12 Apr 2024 14:36:34 +0200 Subject: [PATCH] fix(ItemBookingCalendar): Prevent generic calendar matching before specific calendar When the `modified` timestamp of a generic item booking calendar (item="", uom="") was more recent than that of a specific calendar, the calendar matching system would erroneously grab the newest calendar. --- .../doctype/item_booking/item_booking.py | 22 ++-- .../doctype/item_booking/test_item_booking.py | 110 +++++++++++++++++- 2 files changed, 121 insertions(+), 11 deletions(-) diff --git a/erpnext/venue/doctype/item_booking/item_booking.py b/erpnext/venue/doctype/item_booking/item_booking.py index 5022b9bab17..1d2f36ba2e1 100644 --- a/erpnext/venue/doctype/item_booking/item_booking.py +++ b/erpnext/venue/doctype/item_booking/item_booking.py @@ -1367,19 +1367,21 @@ def get_item_calendar(item: str | None = None, uom: str | None = None) -> "item_ if item and not uom: uom = frappe.get_cached_value("Item", item, "sales_uom") - calendars = frappe.get_all("Item Booking Calendar", fields=["name", "item", "uom"]) + item = item or "" + uom = uom or "" + for filters in [ dict(item=item, uom=uom), - dict(item=item, uom=None), - dict(item=None, uom=uom), - dict(item=None, uom=None), + dict(item=item, uom=""), + dict(item="", uom=uom), + dict(item="", uom=""), ]: - filtered_calendars = [ - x - for x in calendars - if (x.get("item") == filters.get("item") or x.get("item") == "") - and (x.get("uom") == filters.get("uom") or x.get("uom") == "") - ] + filtered_calendars = frappe.get_all( + "Item Booking Calendar", + fields=["name", "item", "uom"], + filters=filters, + limit=1, + ) if filtered_calendars: return { "type": "Daily", diff --git a/erpnext/venue/doctype/item_booking/test_item_booking.py b/erpnext/venue/doctype/item_booking/test_item_booking.py index 87a8ef88a70..4b262522284 100644 --- a/erpnext/venue/doctype/item_booking/test_item_booking.py +++ b/erpnext/venue/doctype/item_booking/test_item_booking.py @@ -201,7 +201,7 @@ class BaseTestWithSubscriptionForBookableItem(BaseTestWithBookableItem): customer: str = TEST_CUSTOMER, item: dict = None, company: str = get_default_company(), # noqa: B008 - item_booking: bool = True + item_booking: bool = True, ): if isinstance(start_date, date): pass # ok @@ -913,3 +913,111 @@ class TestItemBookingWithSubscription(BaseTestWithSubscriptionForBookableItem): for avail in availabilities: self.assertEqual(avail["status"], "available") self.assertEqual(avail["number"], 0) + + +class TestItemBookingCalendar(FrappeTestCase): + def setUp(self) -> None: + for name in frappe.get_all("Item Booking Calendar", pluck="name"): + frappe.get_doc("Item Booking Calendar", name).delete() + + self.itm = frappe.get_doc( + { + "doctype": "Item", + "item_code": "TestItemBookingCalendar", + "item_group": frappe.get_value("Item Group", [("parent_item_group", "=", "")]), + "sales_uom": "Hour", + "is_stock_item": 0, + "enable_item_booking": 1, + "simultaneous_bookings_allowed": MAX_SIMULTANEOUS_BOOKINGS_1, + } + ).save() + + self.cal1 = frappe.get_doc( + { + "doctype": "Item Booking Calendar", + "calendar_title": "cal1", + "uom": self.itm.sales_uom, + "item": self.itm.name, + "booking_calendar": [{"day": "Monday", "start_time": "08:00:00", "end_time": "18:00:00"}], + } + ).save() + + self.cal2 = frappe.get_doc( + { + "doctype": "Item Booking Calendar", + "calendar_title": "cal2", + "uom": "", + "item": self.itm.name, + "booking_calendar": [{"day": "Monday", "start_time": "08:00:00", "end_time": "18:00:00"}], + } + ).save() + + self.cal3 = frappe.get_doc( + { + "doctype": "Item Booking Calendar", + "calendar_title": "cal3", + "uom": self.itm.sales_uom, + "item": "", + "booking_calendar": [{"day": "Monday", "start_time": "08:00:00", "end_time": "18:00:00"}], + } + ).save() + + self.cal4 = frappe.get_doc( + { + "doctype": "Item Booking Calendar", + "calendar_title": "cal4", + "uom": "", + "item": "", + "booking_calendar": [{"day": "Monday", "start_time": "08:00:00", "end_time": "18:00:00"}], + } + ).save() + + return super().setUp() + + def test_item_calendar_order(self): + from erpnext.venue.doctype.item_booking.item_booking import get_item_calendar + + current_run = None + + def check(expected, item, uom): + actual = get_item_calendar(item, uom)["name"] + try: + self.assertEqual(actual, expected) + except AssertionError: + print(f"({item=}, {uom=}) -> {actual} ({expected=})") + print("current_run =", list(map(lambda x: x.name, current_run))) + raise + + def check_run(run): + nonlocal current_run + current_run = run + for doc in run: + doc.save() # update modified timestamp + + check("cal1", self.itm.name, self.itm.sales_uom) # perfect match + check("cal2", self.itm.name, "Ipsum") # fallback to cal2 (item) + check("cal1", self.itm.name, "") # defaults to sales_uom, perfect match + + # Weird cases + check("cal3", "Lorem", self.itm.sales_uom) # fallback to cal3 (uom) + check("cal4", "Lorem", "Ipsum") # catch-all + check("cal4", "Lorem", "") # catch-all + + # Not relevant + check("cal3", "", self.itm.sales_uom) # weird case + check("cal4", "", "Ipsum") # catch-all + check("cal4", "", "") + + runs = [ + (self.cal1, self.cal2, self.cal3, self.cal4), + (self.cal2, self.cal3, self.cal4, self.cal1), + (self.cal3, self.cal4, self.cal1, self.cal2), + (self.cal4, self.cal1, self.cal2, self.cal3), + (self.cal4, self.cal3, self.cal2, self.cal1), + (self.cal1, self.cal4, self.cal3, self.cal2), + (self.cal2, self.cal1, self.cal4, self.cal3), + (self.cal3, self.cal2, self.cal1, self.cal4), + ] + + for run in runs: + check_run(run) -- GitLab