diff --git a/frappe/core/doctype/scheduled_job_type/test_scheduled_job_type.py b/frappe/core/doctype/scheduled_job_type/test_scheduled_job_type.py index 0db573c5b9..6d77c876dc 100644 --- a/frappe/core/doctype/scheduled_job_type/test_scheduled_job_type.py +++ b/frappe/core/doctype/scheduled_job_type/test_scheduled_job_type.py @@ -68,15 +68,6 @@ class TestScheduledJobType(FrappeTestCase): self.assertFalse(job.is_event_due(get_datetime("2019-01-31 23:59:59"))) def test_cron_job(self): - # Daily but offset by 45 minutes - job = frappe.get_doc( - "Scheduled Job Type", - dict(method="frappe.core.doctype.log_settings.log_settings.run_log_clean_up"), - ) - self.assertEqual( - job.next_execution, - add_to_date(None, days=1).replace(hour=0, minute=45, second=0, microsecond=0), - ) # runs every 15 mins job = frappe.get_doc("Scheduled Job Type", dict(method="frappe.oauth.delete_oauth2_data")) job.db_set("last_execution", "2019-01-01 00:00:00") diff --git a/frappe/model/db_query.py b/frappe/model/db_query.py index f54886a4ef..62c0538298 100644 --- a/frappe/model/db_query.py +++ b/frappe/model/db_query.py @@ -3,10 +3,10 @@ """build query for doclistview and return results""" import copy +import datetime import json import re from collections import Counter -from datetime import datetime import frappe import frappe.defaults @@ -21,7 +21,6 @@ from frappe.model.utils import is_virtual_doctype from frappe.model.utils.user_settings import get_user_settings, update_user_settings from frappe.query_builder.utils import Column from frappe.utils import ( - add_to_date, cint, cstr, flt, @@ -30,7 +29,7 @@ from frappe.utils import ( get_timespan_date_range, make_filter_tuple, ) -from frappe.utils.data import sbool +from frappe.utils.data import DateTimeLikeObject, get_datetime, getdate, sbool LOCATE_PATTERN = re.compile(r"locate\([^,]+,\s*[`\"]?name[`\"]?\s*\)", flags=re.IGNORECASE) LOCATE_CAST_PATTERN = re.compile( @@ -853,7 +852,7 @@ class DatabaseQuery: value = frappe.db.format_date(f.value) fallback = "'0001-01-01'" - elif (df and df.fieldtype == "Datetime") or isinstance(f.value, datetime): + elif (df and df.fieldtype == "Datetime") or isinstance(f.value, datetime.datetime): value = frappe.db.format_datetime(f.value) fallback = f"'{FallBackDateTimeStr}'" @@ -1247,10 +1246,18 @@ def has_any_user_permission_for_doctype(doctype, user, applicable_for): def get_between_date_filter(value, df=None): + """Handle datetime filter bounds for between filter values. + + If date is passed but fieldtype is datetime then + from part is converted to start of day and to part is converted to end of day. + If any of filter part (to or from) are missing then: + start or end of current day is assumed as fallback. + If fieldtypes match with filter values then: + no change is applied. """ - return the formattted date as per the given example - [u'2017-11-01', u'2017-11-03'] => '2017-11-01 00:00:00.000000' AND '2017-11-04 00:00:00.000000' - """ + + fieldtype = df and df.fieldtype or "Datetime" + from_date = frappe.utils.nowdate() to_date = frappe.utils.nowdate() @@ -1260,18 +1267,35 @@ def get_between_date_filter(value, df=None): if len(value) >= 2: to_date = value[1] - if not df or (df and df.fieldtype == "Datetime"): - to_date = add_to_date(to_date, days=1) + # if filter value is date but fieldtype is datetime: + if fieldtype == "Datetime": + from_date = _convert_type_for_between_filters(from_date, set_time=datetime.time()) + to_date = _convert_type_for_between_filters(to_date, set_time=datetime.time(23, 59, 59, 999999)) - if df and df.fieldtype == "Datetime": - data = "'{}' AND '{}'".format( - frappe.db.format_datetime(from_date), - frappe.db.format_datetime(to_date), - ) + # If filter value is already datetime, do nothing. + if fieldtype == "Datetime": + cond = f"'{frappe.db.format_datetime(from_date)}' AND '{frappe.db.format_datetime(to_date)}'" else: - data = f"'{frappe.db.format_date(from_date)}' AND '{frappe.db.format_date(to_date)}'" + cond = f"'{frappe.db.format_date(from_date)}' AND '{frappe.db.format_date(to_date)}'" - return data + return cond + + +def _convert_type_for_between_filters( + value: DateTimeLikeObject, set_time: datetime.time +) -> datetime.datetime: + if isinstance(value, str): + if " " in value.strip(): + value = get_datetime(value) + else: + value = getdate(value) + + if isinstance(value, datetime.datetime): + return value + elif isinstance(value, datetime.date): + return datetime.datetime.combine(value, set_time) + + return value def get_additional_filter_field(additional_filters_config, f, value): diff --git a/frappe/tests/test_db_query.py b/frappe/tests/test_db_query.py index cfd7a7e464..5227d7ab4e 100644 --- a/frappe/tests/test_db_query.py +++ b/frappe/tests/test_db_query.py @@ -11,7 +11,7 @@ from frappe.custom.doctype.property_setter.property_setter import make_property_ from frappe.database.utils import DefaultOrderBy from frappe.desk.reportview import get_filters_cond from frappe.handler import execute_cmd -from frappe.model.db_query import DatabaseQuery +from frappe.model.db_query import DatabaseQuery, get_between_date_filter from frappe.permissions import add_user_permission, clear_user_permissions_for_doctype from frappe.query_builder import Column from frappe.tests.utils import FrappeTestCase @@ -286,7 +286,7 @@ class TestDBQuery(FrappeTestCase): event1 = create_event(starts_on="2016-07-05 23:59:59") event2 = create_event(starts_on="2016-07-06 00:00:00") event3 = create_event(starts_on="2016-07-07 23:59:59") - event4 = create_event(starts_on="2016-07-08 00:00:01") + event4 = create_event(starts_on="2016-07-08 00:00:00") # if the values are not passed in filters then event should be filter as current datetime data = DatabaseQuery("Event").execute(filters={"starts_on": ["between", None]}, fields=["name"]) @@ -298,27 +298,58 @@ class TestDBQuery(FrappeTestCase): filters={"starts_on": ["between", ["2016-07-06", "2016-07-07"]]}, fields=["name"] ) - self.assertTrue({"name": event2.name} in data) - self.assertTrue({"name": event3.name} in data) - self.assertTrue({"name": event1.name} not in data) - self.assertTrue({"name": event4.name} not in data) + self.assertIn({"name": event2.name}, data) + self.assertIn({"name": event3.name}, data) + self.assertNotIn({"name": event1.name}, data) + self.assertNotIn({"name": event4.name}, data) # if only one value is passed in the filter data = DatabaseQuery("Event").execute( filters={"starts_on": ["between", ["2016-07-07"]]}, fields=["name"] ) - self.assertTrue({"name": event3.name} in data) - self.assertTrue({"name": event4.name} in data) - self.assertTrue({"name": todays_event.name} in data) - self.assertTrue({"name": event1.name} not in data) - self.assertTrue({"name": event2.name} not in data) + self.assertIn({"name": event3.name}, data) + self.assertIn({"name": event4.name}, data) + self.assertIn({"name": todays_event.name}, data) + self.assertNotIn({"name": event1.name}, data) + self.assertNotIn({"name": event2.name}, data) # test between is formatted for creation column data = DatabaseQuery("Event").execute( filters={"creation": ["between", ["2016-07-06", "2016-07-07"]]}, fields=["name"] ) + def test_between_filters_date_bounds(self): + date_df = frappe._dict(fieldtype="Date") + datetime_df = frappe._dict(fieldtype="Datetime") + today = frappe.utils.nowdate() + + # No filters -> assumes today + cond = get_between_date_filter("", date_df) + self.assertQueryEqual(cond, f"'{today}' AND '{today}'") + + # One filter assumes "from" bound and to is today + start = "2021-01-01" + cond = get_between_date_filter([start], date_df) + self.assertQueryEqual(cond, f"'{start}' AND '{today}'") + + # both date filters are applied + start = "2021-01-01" + end = "2022-01-02" + cond = get_between_date_filter([start, end], date_df) + self.assertQueryEqual(cond, f"'{start}' AND '{end}'") + + # single date should include entire day + start = "2021-01-01" + cond = get_between_date_filter([start, start], datetime_df) + self.assertQueryEqual(cond, f"'{start} 00:00:00.000000' AND '{start} 23:59:59.999999'") + + # datetime field on datetime type should remain same + start = "2021-01-01 01:01:00" + end = "2022-01-02 12:23:43" + cond = get_between_date_filter([start, end], datetime_df) + self.assertQueryEqual(cond, f"'{start}.000000' AND '{end}.000000'") + def test_ignore_permissions_for_get_filters_cond(self): frappe.set_user("test2@example.com") self.assertRaises(frappe.PermissionError, get_filters_cond, "DocType", dict(istable=1), []) diff --git a/frappe/tests/utils.py b/frappe/tests/utils.py index fa7981340e..34e729eb0f 100644 --- a/frappe/tests/utils.py +++ b/frappe/tests/utils.py @@ -86,6 +86,17 @@ class FrappeTestCase(unittest.TestCase): return BeautifulSoup(code, "html.parser").prettify(formatter=None) + def normalize_sql(self, query: str) -> str: + """Formats SQL consistently so simple string comparisons can work on them.""" + import sqlparse + + return ( + sqlparse.format(query.strip(), keyword_case="upper", reindent=True, strip_comments=True), + ) + + def assertQueryEqual(self, first: str, second: str): + self.assertEqual(self.normalize_sql(first), self.normalize_sql(second)) + @contextmanager def assertQueryCount(self, count): queries = []