Merge pull request #3670 from open-formulieren/fix/3647-invalid-time-…

…crashes-form

[#3647] Prevent invalid time from crashing form

src/openforms/formio/constants.py

@@ -9,4 +9,5 @@
     "npFamilyMembers": "object",
     "map": "array",
     "editgrid": "array",
+    "datetime": "datetime",
 }

src/openforms/submissions/models/submission_value_variable.py

@@ -5,14 +5,14 @@
 from django.core.serializers.json import DjangoJSONEncoder
 from django.db import models
 from django.utils import timezone
-from django.utils.dateparse import parse_date, parse_datetime, parse_time
+from django.utils.dateparse import parse_date
 from django.utils.functional import empty
 from django.utils.translation import gettext_lazy as _
 
 from openforms.formio.service import FormioData
 from openforms.forms.models.form_variable import FormVariable
 from openforms.typing import DataMapping, JSONEncodable, JSONSerializable
-from openforms.utils.date import format_date_value
+from openforms.utils.date import format_date_value, parse_datetime, parse_time
 from openforms.variables.constants import FormVariableDataTypes
 from openforms.variables.service import get_static_variables
 
@@ -358,28 +358,23 @@ def to_python(self) -> Any:
                 return aware_date.date()
 
             maybe_naive_datetime = parse_datetime(self.value)
-            if maybe_naive_datetime is not None:
-                if timezone.is_aware(maybe_naive_datetime):
-                    return maybe_naive_datetime.date()
-                return timezone.make_aware(maybe_naive_datetime).date()
+            if maybe_naive_datetime is None:
+                return
 
-            raise ValueError(f"Could not parse date '{self.value}'")  # pragma: nocover
+            if timezone.is_aware(maybe_naive_datetime):
+                return maybe_naive_datetime.date()
+            return timezone.make_aware(maybe_naive_datetime).date()
 
         if self.value and data_type == FormVariableDataTypes.datetime:
             maybe_naive_datetime = parse_datetime(self.value)
-            if maybe_naive_datetime is not None:
-                if timezone.is_aware(maybe_naive_datetime):
-                    return maybe_naive_datetime
-                return timezone.make_aware(maybe_naive_datetime)
+            if maybe_naive_datetime is None:
+                return
 
-            raise ValueError(
-                f"Could not parse datetime '{self.value}'"
-            )  # pragma: nocover
+            if timezone.is_aware(maybe_naive_datetime):
+                return maybe_naive_datetime
+            return timezone.make_aware(maybe_naive_datetime)
 
         if self.value and data_type == FormVariableDataTypes.time:
-            value = parse_time(self.value)
-            if value is not None:
-                return value
-            raise ValueError(f"Could not parse time '{self.value}'")  # pragma: nocover
+            return parse_time(self.value)
 
         return self.value

src/openforms/submissions/tests/form_logic/test_endpoint.py

@@ -1,6 +1,9 @@
+from django.test import tag
+
 from rest_framework import status
 from rest_framework.reverse import reverse
 from rest_framework.test import APIRequestFactory, APITestCase
+from testfixtures import LogCapture
 
 from openforms.accounts.tests.factories import SuperUserFactory
 from openforms.forms.tests.factories import (
@@ -195,3 +198,160 @@ def test_updating_data_marks_step_as_applicable_again(self):
         data = response.json()
 
         self.assertTrue(data["submission"]["steps"][2]["isApplicable"])
+
+    @tag("gh-3647")
+    def test_sending_invalid_time_values(self):
+        submission = SubmissionFactory.from_components(
+            components_list=[
+                {"type": "time", "key": "time"},
+                {"type": "date", "key": "date"},
+                {"type": "datetime", "key": "datetime"},
+            ]
+        )
+
+        endpoint = reverse(
+            "api:submission-steps-logic-check",
+            kwargs={
+                "submission_uuid": submission.uuid,
+                "step_uuid": submission.submissionstep_set.first().form_step.uuid,
+            },
+        )
+        self._add_submission_to_session(submission)
+
+        with (
+            self.subTest("Invalid time with good format"),
+            LogCapture() as log_capture,
+        ):
+            self.client.post(endpoint, data={"data": {"time": "25:00"}})
+
+            log_capture.check_present(
+                (
+                    "openforms.utils.date",
+                    "INFO",
+                    "Invalid time '25:00', falling back to 'None' instead.",
+                )
+            )
+
+        with (
+            self.subTest("Invalid time with bad format"),
+            LogCapture() as log_capture,
+        ):
+            self.client.post(endpoint, data={"data": {"time": "Invalid"}})
+
+            log_capture.check_present(
+                (
+                    "openforms.utils.date",
+                    "INFO",
+                    "Badly formatted time 'Invalid', falling back to 'None' instead.",
+                )
+            )
+
+        with (self.subTest("Invalid date"), LogCapture() as log_capture):
+            self.client.post(endpoint, data={"data": {"date": "2020-13-46"}})
+
+            log_capture.check_present(
+                (
+                    "openforms.utils.date",
+                    "INFO",
+                    "Can't format date '2020-13-46', falling back to an empty string.",
+                ),
+                (
+                    "openforms.utils.date",
+                    "INFO",
+                    "Badly formatted datetime '2020-13-46', falling back to 'None' instead.",
+                ),
+            )
+
+        with (self.subTest("Invalid datetime"), LogCapture() as log_capture):
+            self.client.post(
+                endpoint, data={"data": {"datetime": "2022-13-46T00:00:00+02:00"}}
+            )
+
+            log_capture.check_present(
+                (
+                    "openforms.utils.date",
+                    "INFO",
+                    "Can't parse datetime '2022-13-46T00:00:00+02:00', falling back to 'None' instead.",
+                )
+            )
+
+    def test_handling_none_values_in_logic(self):
+        submission = SubmissionFactory.from_components(
+            components_list=[
+                {"type": "time", "key": "time"},
+                {"type": "date", "key": "date"},
+                {"type": "datetime", "key": "datetime"},
+                {"type": "string", "key": "result"},
+                {"type": "date", "key": "resultDate"},
+                {"type": "datetime", "key": "resultDatetime"},
+            ]
+        )
+        FormLogicFactory.create(
+            form=submission.form,
+            json_logic_trigger={
+                "and": [
+                    {"==": [None, {"var": "time"}]},
+                    {"==": [None, {"var": "date"}]},
+                    {"==": [None, {"var": "datetime"}]},
+                ]
+            },
+            actions=[
+                {
+                    "variable": "result",
+                    "action": {
+                        "type": "variable",
+                        "value": "All the variables were None",
+                    },
+                }
+            ],
+        )
+        FormLogicFactory.create(
+            form=submission.form,
+            json_logic_trigger=True,
+            actions=[
+                {
+                    "variable": "resultDate",
+                    "action": {
+                        "type": "variable",
+                        "value": {"+": [{"var": "date"}, {"duration": "P1M"}]},
+                    },
+                },
+                {
+                    "variable": "resultDatetime",
+                    "action": {
+                        "type": "variable",
+                        "value": {"+": [{"var": "datetime"}, {"duration": "P1M"}]},
+                    },
+                },
+            ],
+        )
+        endpoint = reverse(
+            "api:submission-steps-logic-check",
+            kwargs={
+                "submission_uuid": submission.uuid,
+                "step_uuid": submission.submissionstep_set.first().form_step.uuid,
+            },
+        )
+        self._add_submission_to_session(submission)
+
+        response = self.client.post(
+            endpoint,
+            data={
+                "data": {
+                    "time": "Invalid",
+                    "date": "2020-13-46",
+                    "datetime": "2022-13-46T00:00:00+02:00",
+                }
+            },
+        )
+
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+
+        data = response.json()
+
+        self.assertEqual(data["step"]["data"]["result"], "All the variables were None")
+        self.assertEqual(data["step"]["data"]["time"], "Invalid")
+        self.assertEqual(data["step"]["data"]["date"], "2020-13-46")
+        self.assertEqual(data["step"]["data"]["datetime"], "2022-13-46T00:00:00+02:00")
+        self.assertNotIn("resultDate", data["step"]["data"])
+        self.assertNotIn("resultDatetime", data["step"]["data"])

src/openforms/submissions/tests/test_variables/test_model.py

@@ -210,6 +210,17 @@ def test_to_python(self):
             # we'll see if that causes any issues
             self.assertEqual(time_value, time(11, 15))
 
+        with self.subTest("Invalid time"):
+            time_var = SubmissionValueVariableFactory.create(
+                form_variable__user_defined=True,
+                form_variable__data_type=FormVariableDataTypes.time,
+                value="Invalid date",  # Issue 3647
+            )
+
+            time_value = time_var.to_python()
+
+            self.assertIsNone(time_value)
+
     def test_is_initially_prefilled_is_set(self):
         config = {
             "display": "form",

src/openforms/utils/date.py

@@ -1,7 +1,11 @@
 import logging
-from datetime import date, datetime
+from datetime import date, datetime, time
 
 from django.utils import timezone
+from django.utils.dateparse import (
+    parse_datetime as _parse_datetime,
+    parse_time as _parse_time,
+)
 
 logger = logging.getLogger(__name__)
 
@@ -13,7 +17,9 @@ def format_date_value(date_value: str) -> str:
         try:
             parsed_date = datetime.strptime(date_value, "%Y%m%d").date()
         except ValueError:
-            logger.info("Can't parse date %s, using empty value.", date_value)
+            logger.info(
+                "Can't format date '%s', falling back to an empty string.", date_value
+            )
             return ""
 
     return parsed_date.isoformat()
@@ -31,3 +37,33 @@ def parse_date(value: str) -> date:
     dt = datetime.fromisoformat(value)
     assert dt.tzinfo is not None, "Expected the input variable to be timezone aware!"
     return timezone.localtime(value=dt).date()
+
+
+def parse_datetime(value: str) -> None | datetime:
+    try:
+        datetime_value = _parse_datetime(value)
+    except ValueError:
+        logger.info("Can't parse datetime '%s', falling back to 'None' instead.", value)
+        return
+
+    if datetime_value is None:
+        logger.info(
+            "Badly formatted datetime '%s', falling back to 'None' instead.", value
+        )
+        return
+
+    return datetime_value
+
+
+def parse_time(value: str) -> None | time:
+    try:
+        time_value = _parse_time(value)
+    except ValueError:
+        logger.info("Invalid time '%s', falling back to 'None' instead.", value)
+        return
+
+    if time_value is None:
+        logger.info("Badly formatted time '%s', falling back to 'None' instead.", value)
+        return
+
+    return time_value