From 2bc38b895fcbd15f2a9cc878946181d7f6347651 Mon Sep 17 00:00:00 2001 From: Maharshi Patel Date: Thu, 11 Jan 2024 09:20:19 +0530 Subject: [PATCH 1/6] feat(minor): add more event repeat options added option of quarterly and half yearly repeats. --- frappe/desk/doctype/event/event.json | 6 +-- frappe/desk/doctype/event/event.py | 59 +++++++++++++++++++++++++++- 2 files changed, 61 insertions(+), 4 deletions(-) diff --git a/frappe/desk/doctype/event/event.json b/frappe/desk/doctype/event/event.json index 3a4f192656..9a4f63d618 100644 --- a/frappe/desk/doctype/event/event.json +++ b/frappe/desk/doctype/event/event.json @@ -123,7 +123,7 @@ "fieldtype": "Select", "in_global_search": 1, "label": "Repeat On", - "options": "\nDaily\nWeekly\nMonthly\nYearly" + "options": "\nDaily\nWeekly\nMonthly\nQuarterly\nHalf Yearly\nYearly" }, { "depends_on": "repeat_this_event", @@ -295,7 +295,7 @@ "icon": "fa fa-calendar", "idx": 1, "links": [], - "modified": "2023-06-23 10:33:15.685368", + "modified": "2024-01-11 07:11:17.467503", "modified_by": "Administrator", "module": "Desk", "name": "Event", @@ -336,4 +336,4 @@ "track_changes": 1, "track_seen": 1, "track_views": 1 -} +} \ No newline at end of file diff --git a/frappe/desk/doctype/event/event.py b/frappe/desk/doctype/event/event.py index bf56498780..0442c73d23 100644 --- a/frappe/desk/doctype/event/event.py +++ b/frappe/desk/doctype/event/event.py @@ -21,6 +21,7 @@ from frappe.utils import ( format_datetime, get_datetime_str, getdate, + month_diff, now_datetime, nowdate, ) @@ -62,7 +63,7 @@ class Event(Document): google_meet_link: DF.Data | None monday: DF.Check pulled_from_google_calendar: DF.Check - repeat_on: DF.Literal["", "Daily", "Weekly", "Monthly", "Yearly"] + repeat_on: DF.Literal["", "Daily", "Weekly", "Monthly", "Quarterly", "Half Yearly", "Yearly"] repeat_this_event: DF.Check repeat_till: DF.Date | None saturday: DF.Check @@ -392,6 +393,62 @@ def get_events(start, end, user=None, for_reminder=False, filters=None) -> list[ remove_events.append(e) + if e.repeat_on == "Half Yearly": + # creates a string with date (27) and month (07) and year (2019) eg: 2019-07-27 + year, month = start.split("-", maxsplit=2)[:2] + date = f"{year}-{month}-" + event_start.split("-", maxsplit=3)[2] + + # last day of month issue, start from prev month! + try: + getdate(date) + except Exception: + date = date.split("-") + date = date[0] + "-" + str(cint(date[1]) - 1) + "-" + date[2] + + start_from = date + for i in range(int(date_diff(end, start) / 30) + 3): + diff = month_diff(date, event_start) - 1 + if diff % 6 != 0: + continue + if ( + getdate(date) >= getdate(start) + and getdate(date) <= getdate(end) + and getdate(date) <= getdate(repeat) + and getdate(date) >= getdate(event_start) + ): + add_event(e, date) + + date = add_months(start_from, i + 1) + remove_events.append(e) + + if e.repeat_on == "Quarterly": + # creates a string with date (27) and month (07) and year (2019) eg: 2019-07-27 + year, month = start.split("-", maxsplit=2)[:2] + date = f"{year}-{month}-" + event_start.split("-", maxsplit=3)[2] + + # last day of month issue, start from prev month! + try: + getdate(date) + except Exception: + date = date.split("-") + date = date[0] + "-" + str(cint(date[1]) - 1) + "-" + date[2] + + start_from = date + for i in range(int(date_diff(end, start) / 30) + 3): + diff = month_diff(date, event_start) - 1 + if diff % 3 != 0: + continue + if ( + getdate(date) >= getdate(start) + and getdate(date) <= getdate(end) + and getdate(date) <= getdate(repeat) + and getdate(date) >= getdate(event_start) + ): + add_event(e, date) + + date = add_months(start_from, i + 1) + remove_events.append(e) + if e.repeat_on == "Monthly": # creates a string with date (27) and month (07) and year (2019) eg: 2019-07-27 year, month = start.split("-", maxsplit=2)[:2] From 399ccfcdaf7924d3d67a700c6f718134ee100ca0 Mon Sep 17 00:00:00 2001 From: Maharshi Patel Date: Tue, 16 Jan 2024 10:39:50 +0530 Subject: [PATCH 2/6] test: added test for new event repeat options added test for quarterly and half yearly repeat option. --- frappe/desk/doctype/event/test_event.py | 74 +++++++++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/frappe/desk/doctype/event/test_event.py b/frappe/desk/doctype/event/test_event.py index 72eab8f416..0680167329 100644 --- a/frappe/desk/doctype/event/test_event.py +++ b/frappe/desk/doctype/event/test_event.py @@ -136,3 +136,77 @@ class TestEvent(FrappeTestCase): ev_list3 = get_events("2015-02-01", "2015-02-01", "Administrator", for_reminder=True) self.assertTrue(bool(list(filter(lambda e: e.name == ev.name, ev_list3)))) + + def test_quaterly_repeat(self): + ev = frappe.get_doc( + { + "doctype": "Event", + "subject": "_Test Event", + "starts_on": "2023-02-17", + "repeat_till": "2024-02-17", + "event_type": "Public", + "repeat_this_event": 1, + "repeat_on": "Quarterly", + } + ) + ev.insert() + # Test Quaterly months + ev_list = get_events("2023-02-17", "2023-02-17", "Administrator", for_reminder=True) + self.assertTrue(bool(list(filter(lambda e: e.name == ev.name, ev_list)))) + + ev_list1 = get_events("2023-05-17", "2023-05-17", "Administrator", for_reminder=True) + self.assertTrue(bool(list(filter(lambda e: e.name == ev.name, ev_list1)))) + + ev_list2 = get_events("2023-08-17", "2023-08-17", "Administrator", for_reminder=True) + self.assertTrue(bool(list(filter(lambda e: e.name == ev.name, ev_list2)))) + + ev_list3 = get_events("2023-11-17", "2023-11-17", "Administrator", for_reminder=True) + self.assertTrue(bool(list(filter(lambda e: e.name == ev.name, ev_list3)))) + + # Test before event start date and after event end date + ev_list4 = get_events("2022-11-17", "2022-11-17", "Administrator", for_reminder=True) + self.assertFalse(bool(list(filter(lambda e: e.name == ev.name, ev_list4)))) + + ev_list4 = get_events("2024-02-17", "2024-02-17", "Administrator", for_reminder=True) + self.assertFalse(bool(list(filter(lambda e: e.name == ev.name, ev_list4)))) + + # Test months that aren't part of the quarterly cycle + ev_list4 = get_events("2023-12-17", "2023-12-17", "Administrator", for_reminder=True) + self.assertFalse(bool(list(filter(lambda e: e.name == ev.name, ev_list4)))) + + ev_list4 = get_events("2023-03-17", "2023-03-17", "Administrator", for_reminder=True) + self.assertFalse(bool(list(filter(lambda e: e.name == ev.name, ev_list4)))) + + def test_half_yearly_repeat(self): + ev = frappe.get_doc( + { + "doctype": "Event", + "subject": "_Test Event", + "starts_on": "2023-02-17", + "repeat_till": "2024-02-17", + "event_type": "Public", + "repeat_this_event": 1, + "repeat_on": "Half Yearly", + } + ) + ev.insert() + # Test Half Yearly months + ev_list = get_events("2023-02-17", "2023-02-17", "Administrator", for_reminder=True) + self.assertTrue(bool(list(filter(lambda e: e.name == ev.name, ev_list)))) + + ev_list1 = get_events("2023-08-17", "2023-08-17", "Administrator", for_reminder=True) + self.assertTrue(bool(list(filter(lambda e: e.name == ev.name, ev_list1)))) + + # Test before event start date and after event end date + ev_list4 = get_events("2022-08-17", "2022-08-17", "Administrator", for_reminder=True) + self.assertFalse(bool(list(filter(lambda e: e.name == ev.name, ev_list4)))) + + ev_list4 = get_events("2024-02-17", "2024-02-17", "Administrator", for_reminder=True) + self.assertFalse(bool(list(filter(lambda e: e.name == ev.name, ev_list4)))) + + # Test months that aren't part of the half yearly cycle + ev_list4 = get_events("2023-12-17", "2023-12-17", "Administrator", for_reminder=True) + self.assertFalse(bool(list(filter(lambda e: e.name == ev.name, ev_list4)))) + + ev_list4 = get_events("2023-05-17", "2023-05-17", "Administrator", for_reminder=True) + self.assertFalse(bool(list(filter(lambda e: e.name == ev.name, ev_list4)))) From 608f2ed20b55fe0a6d6cd078b32ed21a153ad35a Mon Sep 17 00:00:00 2001 From: David Arnold Date: Tue, 16 Jan 2024 06:26:47 +0100 Subject: [PATCH 3/6] test: make phone number unique bis (#24367) --- frappe/contacts/doctype/contact/test_contact.py | 12 ++++++------ .../test_addresses_and_contacts.py | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/frappe/contacts/doctype/contact/test_contact.py b/frappe/contacts/doctype/contact/test_contact.py index b5f1c4bdf8..f203983309 100644 --- a/frappe/contacts/doctype/contact/test_contact.py +++ b/frappe/contacts/doctype/contact/test_contact.py @@ -23,15 +23,15 @@ class TestContact(FrappeTestCase): def test_check_default_phone_and_mobile(self): phones = [ - {"phone": "+91 0000000000", "is_primary_phone": 0, "is_primary_mobile_no": 0}, - {"phone": "+91 0000000001", "is_primary_phone": 0, "is_primary_mobile_no": 0}, - {"phone": "+91 0000000002", "is_primary_phone": 1, "is_primary_mobile_no": 0}, - {"phone": "+91 0000000003", "is_primary_phone": 0, "is_primary_mobile_no": 1}, + {"phone": "+91 0000000010", "is_primary_phone": 0, "is_primary_mobile_no": 0}, + {"phone": "+91 0000000011", "is_primary_phone": 0, "is_primary_mobile_no": 0}, + {"phone": "+91 0000000012", "is_primary_phone": 1, "is_primary_mobile_no": 0}, + {"phone": "+91 0000000013", "is_primary_phone": 0, "is_primary_mobile_no": 1}, ] contact = create_contact("Phone", "Mr", phones=phones) - self.assertEqual(contact.phone, "+91 0000000002") - self.assertEqual(contact.mobile_no, "+91 0000000003") + self.assertEqual(contact.phone, "+91 0000000012") + self.assertEqual(contact.mobile_no, "+91 0000000013") def test_get_full_name(self): self.assertEqual(get_full_name(first="John"), "John") diff --git a/frappe/contacts/report/addresses_and_contacts/test_addresses_and_contacts.py b/frappe/contacts/report/addresses_and_contacts/test_addresses_and_contacts.py index bbb1b03e79..fe76d28c06 100644 --- a/frappe/contacts/report/addresses_and_contacts/test_addresses_and_contacts.py +++ b/frappe/contacts/report/addresses_and_contacts/test_addresses_and_contacts.py @@ -76,7 +76,7 @@ def create_linked_contact(link_list, address): } ) contact.add_email("test_contact@example.com", is_primary=True) - contact.add_phone("+91 0000000000", is_primary_phone=True) + contact.add_phone("+91 0000000020", is_primary_phone=True) for name in link_list: contact.append("links", {"link_doctype": "Test Custom Doctype", "link_name": name}) @@ -105,7 +105,7 @@ class TestAddressesAndContacts(FrappeTestCase): "_Test First Name", "_Test Last Name", "_Test Address-Billing", - "+91 0000000000", + "+91 0000000020", "", "test_contact@example.com", 1, From a2525e545a46e51b44c4da6bad9197ab0c8d6d50 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 16 Jan 2024 11:00:12 +0530 Subject: [PATCH 4/6] perf: Unbuffered cursors for large result sets (#24365) If you're reading 1000s of rows from MySQL, the default behaviour is to read all of them in memory at once. One of the use case for reading large rows is reporting where a lot of data is read and then processed in Python. The read row is hoever not used again but still consumes memory until entire function exits. SSCursor (Server Side Cursor) allows fetching one row at a time. Note: This is slower than fetching everything at once AND has risk of connection loss. So, don't use this as a crutch. If possible rewrite code so processing is done in SQL. --- frappe/database/database.py | 59 +++++++++++++++++++---------- frappe/database/mariadb/database.py | 13 +++++++ frappe/tests/test_db.py | 7 +++- 3 files changed, 58 insertions(+), 21 deletions(-) diff --git a/frappe/database/database.py b/frappe/database/database.py index 97d48db693..b117c2c6dc 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -46,6 +46,8 @@ INDEX_PATTERN = re.compile(r"\s*\([^)]+\)\s*") SINGLE_WORD_PATTERN = re.compile(r'([`"]?)(tab([A-Z]\w+))\1') MULTI_WORD_PATTERN = re.compile(r'([`"])(tab([A-Z]\w+)( [A-Z]\w+)+)\1') +SQL_ITERATOR_BATCH_SIZE = 100 + class Database: """ @@ -175,6 +177,8 @@ class Database: :param pluck: Get the plucked field only. :param explain: Print `EXPLAIN` in error log. :param as_iterator: Returns iterator over results instead of fetching all results at once. + This should be used with unbuffered cursor as default cursors used by pymysql and postgres + buffer the results internally. See `Database.unbuffered_cursor`. Examples: # return customer names as dicts @@ -276,12 +280,10 @@ class Database: if not self._cursor.description: return () - last_result = self._transform_result(self._cursor.fetchall()) if as_iterator: - return self._return_as_iterator( - last_result, pluck=pluck, as_dict=as_dict, as_list=as_list, update=update - ) + return self._return_as_iterator(pluck=pluck, as_dict=as_dict, as_list=as_list, update=update) + last_result = self._transform_result(self._cursor.fetchall()) if pluck: last_result = [r[0] for r in last_result] self._clean_up() @@ -300,24 +302,25 @@ class Database: self._clean_up() return last_result - def _return_as_iterator(self, result, *, pluck, as_dict, as_list, update): - if pluck: - for row in result: - yield row[0] + def _return_as_iterator(self, *, pluck, as_dict, as_list, update): + while result := self._transform_result(self._cursor.fetchmany(SQL_ITERATOR_BATCH_SIZE)): + if pluck: + for row in result: + yield row[0] - elif as_dict: - keys = [column[0] for column in self._cursor.description] - for row in result: - row = frappe._dict(zip(keys, row)) - if update: - row.update(update) - yield row + elif as_dict: + keys = [column[0] for column in self._cursor.description] + for row in result: + row = frappe._dict(zip(keys, row)) + if update: + row.update(update) + yield row - elif as_list: - for row in result: - yield list(row) - else: - frappe.throw(_("`as_iterator` only works with `as_list=True` or `as_dict=True`")) + elif as_list: + for row in result: + yield list(row) + else: + frappe.throw(_("`as_iterator` only works with `as_list=True` or `as_dict=True`")) self._clean_up() @@ -1344,6 +1347,22 @@ class Database: def rename_column(self, doctype: str, old_column_name: str, new_column_name: str): raise NotImplementedError + @contextmanager + def unbuffered_cursor(self): + """Context manager to temporarily use unbuffered cursor. + + Using this with `as_iterator=True` provides O(1) memory usage while reading large result sets. + + NOTE: You MUST do entire result set processing in the context, otherwise underlying cursor + will be switched and you'll not get complete results. + + Usage: + with frappe.db.unbuffered_cursor(): + for row in frappe.db.sql("query with huge result", as_iterator=True): + continue # Do some processing. + """ + raise NotImplementedError + @contextmanager def savepoint(catch: type | tuple[type, ...] = Exception): diff --git a/frappe/database/mariadb/database.py b/frappe/database/mariadb/database.py index d6d3258b5e..4c4c468fe4 100644 --- a/frappe/database/mariadb/database.py +++ b/frappe/database/mariadb/database.py @@ -1,4 +1,5 @@ import re +from contextlib import contextmanager import pymysql from pymysql.constants import ER, FIELD_TYPE @@ -525,3 +526,15 @@ class MariaDBDatabase(MariaDBConnectionUtil, MariaDBExceptionUtil, Database): if est_row_size: return int(est_row_size[0][0]) + + @contextmanager + def unbuffered_cursor(self): + from pymysql.cursors import SSCursor + + try: + original_cursor = self._cursor + new_cursor = self._cursor = self._conn.cursor(SSCursor) + yield + finally: + self._cursor = original_cursor + new_cursor.close() diff --git a/frappe/tests/test_db.py b/frappe/tests/test_db.py index f8c11649cf..42d4347aa0 100644 --- a/frappe/tests/test_db.py +++ b/frappe/tests/test_db.py @@ -11,7 +11,7 @@ import frappe from frappe.core.utils import find from frappe.custom.doctype.custom_field.custom_field import create_custom_field from frappe.database import savepoint -from frappe.database.database import Database, get_query_execution_timeout +from frappe.database.database import get_query_execution_timeout from frappe.database.utils import FallBackDateTimeStr from frappe.query_builder import Field from frappe.query_builder.functions import Concat_ws @@ -1007,3 +1007,8 @@ class TestSqlIterator(FrappeTestCase): list(frappe.db.sql(query, as_list=True, as_iterator=True)), msg=f"{query=} results not same as iterator", ) + + @run_only_if(db_type_is.MARIADB) + def test_unbuffered_cursor(self): + with frappe.db.unbuffered_cursor(): + self.test_db_sql_iterator() From cbbd22a7ebf86cf4b67ad9420589178c36048043 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Tue, 16 Jan 2024 11:36:34 +0530 Subject: [PATCH 5/6] fix: Handle edge case while searching in current context --- frappe/public/js/frappe/ui/toolbar/awesome_bar.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/frappe/public/js/frappe/ui/toolbar/awesome_bar.js b/frappe/public/js/frappe/ui/toolbar/awesome_bar.js index a37979a7cb..7ef9e19383 100644 --- a/frappe/public/js/frappe/ui/toolbar/awesome_bar.js +++ b/frappe/public/js/frappe/ui/toolbar/awesome_bar.js @@ -317,7 +317,9 @@ frappe.search.AwesomeBar = class AwesomeBar { var route = frappe.get_route(); if (route[0] === "List" && txt.indexOf(" in") === -1) { // search in title field - var meta = frappe.get_meta(frappe.container.page.list_view.doctype); + const doctype = frappe.container.page?.list_view?.doctype; + if (!doctype) return; + var meta = frappe.get_meta(doctype); var search_field = meta.title_field || "name"; var options = {}; options[search_field] = ["like", "%" + txt + "%"]; From 92326d143d61ac8b74d9b2304c8a8b27ebda6c8c Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Tue, 16 Jan 2024 16:26:48 +0530 Subject: [PATCH 6/6] fix(sentry): set scope for background jobs Signed-off-by: Akhil Narang --- frappe/utils/sentry.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/frappe/utils/sentry.py b/frappe/utils/sentry.py index 61ba48eecc..c0e9dd806f 100644 --- a/frappe/utils/sentry.py +++ b/frappe/utils/sentry.py @@ -107,13 +107,13 @@ def capture_exception(message: str | None = None) -> None: return try: hub = Hub.current - if frappe.request: - with hub.configure_scope() as scope: - if ( - os.getenv("ENABLE_SENTRY_DB_MONITORING") is None - or os.getenv("SENTRY_TRACING_SAMPLE_RATE") is None - ): - set_scope(scope) + with hub.configure_scope() as scope: + if ( + os.getenv("ENABLE_SENTRY_DB_MONITORING") is None + or os.getenv("SENTRY_TRACING_SAMPLE_RATE") is None + ): + set_scope(scope) + if frappe.request: evt_processor = _make_wsgi_event_processor(frappe.request.environ, False) scope.add_event_processor(evt_processor) if frappe.request.is_json: