From a1cb19c820f0881b47b1e11503b39044cc6b4162 Mon Sep 17 00:00:00 2001 From: scdanieli <23150094+scdanieli@users.noreply.github.com> Date: Sun, 18 Feb 2024 16:05:31 +0100 Subject: [PATCH] fix: ensure has_value_changed works for datetime, date and timedelta fields --- frappe/model/document.py | 19 +++++++++++++++++-- frappe/tests/test_utils.py | 1 + frappe/utils/data.py | 7 +++++-- 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/frappe/model/document.py b/frappe/model/document.py index 090008492e..664684b529 100644 --- a/frappe/model/document.py +++ b/frappe/model/document.py @@ -4,6 +4,7 @@ import hashlib import json import time from collections.abc import Generator, Iterable +from datetime import date, datetime, timedelta from typing import TYPE_CHECKING, Any, Optional from werkzeug.exceptions import NotFound @@ -22,7 +23,7 @@ from frappe.model.utils import is_virtual_doctype from frappe.model.workflow import set_workflow_state_on_action, validate_workflow from frappe.types import DF from frappe.utils import compare, cstr, date_diff, file_lock, flt, now -from frappe.utils.data import get_absolute_url +from frappe.utils.data import get_absolute_url, get_datetime, get_timedelta, getdate from frappe.utils.global_search import update_global_search if TYPE_CHECKING: @@ -456,7 +457,21 @@ class Document(BaseDocument): def has_value_changed(self, fieldname): """Return True if value has changed before and after saving.""" previous = self.get_doc_before_save() - return previous.get(fieldname) != self.get(fieldname) if previous else True + + if not previous: + return True + + previous_value = previous.get(fieldname) + current_value = self.get(fieldname) + + if isinstance(previous_value, datetime): + current_value = get_datetime(current_value) + elif isinstance(previous_value, date): + current_value = getdate(current_value) + elif isinstance(previous_value, timedelta): + current_value = get_timedelta(current_value) + + return previous_value != current_value def set_new_name(self, force=False, set_name=None, set_child_names=True): """Calls `frappe.naming.set_new_name` for parent and child docs.""" diff --git a/frappe/tests/test_utils.py b/frappe/tests/test_utils.py index 312a9f3228..b8db9421fc 100644 --- a/frappe/tests/test_utils.py +++ b/frappe/tests/test_utils.py @@ -605,6 +605,7 @@ class TestDateUtils(FrappeTestCase): self.assertIsInstance(get_timedelta(str(datetime_input)), timedelta) self.assertIsInstance(get_timedelta(str(timedelta_input)), timedelta) self.assertIsInstance(get_timedelta(str(time_input)), timedelta) + self.assertIsInstance(get_timedelta(get_timedelta("100:2:12")), timedelta) def test_to_timedelta(self): self.assertEqual(to_timedelta("00:00:01"), timedelta(seconds=1)) diff --git a/frappe/utils/data.py b/frappe/utils/data.py index fc07b4ad8f..803a555a16 100644 --- a/frappe/utils/data.py +++ b/frappe/utils/data.py @@ -161,13 +161,13 @@ def get_datetime( return parser.parse(datetime_str) -def get_timedelta(time: str | None = None) -> datetime.timedelta | None: +def get_timedelta(time: str | datetime.timedelta | None = None) -> datetime.timedelta | None: """Return `datetime.timedelta` object from string value of a valid time format. Return None if `time` is not a valid format. Args: - time (str): A valid time representation. This string is parsed + time (str | datetime.timedelta): A valid time representation. This string is parsed using `dateutil.parser.parse`. Examples of valid inputs are: '0:0:0', '17:21:00', '2012-01-19 17:21:00'. Checkout https://dateutil.readthedocs.io/en/stable/parser.html#dateutil.parser.parse @@ -175,6 +175,9 @@ def get_timedelta(time: str | None = None) -> datetime.timedelta | None: Return: datetime.timedelta: Timedelta object equivalent of the passed `time` string """ + if isinstance(time, datetime.timedelta): + return time + time = time or "0:0:0" try: