diff --git a/custom_components/nestquest/completion.py b/custom_components/nestquest/completion.py index 3a4108e..3f22089 100644 --- a/custom_components/nestquest/completion.py +++ b/custom_components/nestquest/completion.py @@ -35,6 +35,7 @@ from .dao_instances import ( EVENT_UNCOMPLETED, CompletionEventRecord, CompletionEventsDao, + QuestInstanceRecord, QuestInstancesDao, ) from .db import NestQuestDatabase @@ -64,6 +65,64 @@ def _stamp_occurred_at(now: datetime.datetime | None) -> str: ) +def _completion_moment( + now: datetime.datetime | None, + today: datetime.date | None, +) -> tuple[datetime.date, datetime.time | None]: + """Return the HA-local completion date and optional wall-clock time. + + ``today`` is the HA-local calendar date when threaded (Feature 07 + style). ``now`` is a timezone-aware HA-local datetime; its wall + clock is used as-is and is never converted to UTC. When ``today`` + is omitted the date comes from ``now``. When both are omitted the + host calendar date is the Feature 07 fallback — never the UTC + clock. Time-of-day is None unless ``now`` is supplied; comparing + against ``due_time`` requires ``now``. + """ + if now is not None: + if not isinstance(now, datetime.datetime): + raise ValueError(f"now must be a datetime, got {now!r}") + if now.tzinfo is None: + raise ValueError("now must be timezone-aware") + if today is not None: + completion_date = today + elif now is not None: + completion_date = now.date() + else: + completion_date = datetime.date.today() + if now is None: + return completion_date, None + return completion_date, now.time() + + +def _was_on_time( + instance: QuestInstanceRecord, + now: datetime.datetime | None, + today: datetime.date | None, +) -> bool: + """Return whether the completion moment is on time for ``instance``. + + On ``due_date`` before ``due_time`` is on time; on ``due_date`` + after ``due_time``, or any later date, is late. With no + ``due_time``, on time if the completion date equals ``due_date`` + (or is earlier); late if after ``due_date``. Same-day + ``due_time`` comparison requires ``now``; UTC time-of-day is never + substituted. + """ + completion_date, completion_time = _completion_moment(now, today) + due_date = datetime.date.fromisoformat(instance.due_date) + if completion_date < due_date: + return True + if completion_date > due_date: + return False + if instance.due_time is None: + return True + if completion_time is None: + raise ValueError("now is required to compare due_time") + due_time = datetime.datetime.strptime(instance.due_time, "%H:%M").time() + return completion_time <= due_time + + class _TaskReentrantLock: """Shared connection lock the owning task may re-enter.""" @@ -278,11 +337,11 @@ async def complete_instance( append run under one connection lock so a concurrent complete cannot double-append. - ``was_on_time`` is recorded as True — a placeholder until the - on-time task computes it from ``now`` / ``today`` against the - instance due_date and optional due_time. ``today`` is accepted so - callers can thread the HA-local date through without a signature - change; it is unused here. + ``was_on_time`` is computed from the HA-local completion moment + (threaded ``now`` / ``today``) against the instance ``due_date`` + and optional ``due_time``. ``now`` is the timezone-aware HA-local + datetime; its wall clock is never converted to UTC. Same-day + ``due_time`` comparison requires ``now``. """ instance_id = _validate_int_id(instance_id, "instance_id") actor_source, actor_user_id, actor_child_id = _validate_actor( @@ -312,7 +371,7 @@ async def complete_instance( actor_source=actor_source, actor_user_id=actor_user_id, actor_child_id=actor_child_id, - was_on_time=True, + was_on_time=_was_on_time(instance, now, today), now=now, ) return "done" diff --git a/tests/test_completion.py b/tests/test_completion.py index ac54dbd..da2493e 100644 --- a/tests/test_completion.py +++ b/tests/test_completion.py @@ -687,6 +687,207 @@ def test_complete_instance_concurrent_second_call_is_noop(tmp_path) -> None: _run(_main()) +_HA_TZ = datetime.timezone(datetime.timedelta(hours=-8)) + + +def _ha_local(day: datetime.date, hour: int, minute: int = 0) -> datetime.datetime: + return datetime.datetime( + day.year, day.month, day.day, hour, minute, 0, + tzinfo=_HA_TZ, + ) + + +def test_complete_instance_on_due_date_before_due_time_is_on_time( + tmp_path, +) -> None: + async def _body(database, child, instance): + due_date = datetime.date.fromisoformat(instance.due_date) + instance = await QuestInstancesDao(database).upsert( + instance.definition_id, + child.id, + instance.due_date, + _now_stamp(), + window=instance.window, + due_time="12:00", + ) + await complete_instance( + database, + instance.id, + actor_source="user", + actor_user_id="user-1", + now=_ha_local(due_date, 11, 0), + today=due_date, + ) + rows = await CompletionEventsDao(database).list_by_instance( + instance.id + ) + assert rows[0].was_on_time is True + + _with_db(tmp_path, "ontime-before-due-time.db")(_body) + + +def test_complete_instance_on_due_date_after_due_time_is_late( + tmp_path, +) -> None: + async def _body(database, child, instance): + due_date = datetime.date.fromisoformat(instance.due_date) + instance = await QuestInstancesDao(database).upsert( + instance.definition_id, + child.id, + instance.due_date, + _now_stamp(), + window=instance.window, + due_time="12:00", + ) + await complete_instance( + database, + instance.id, + actor_source="user", + actor_user_id="user-1", + now=_ha_local(due_date, 12, 1), + today=due_date, + ) + rows = await CompletionEventsDao(database).list_by_instance( + instance.id + ) + assert rows[0].was_on_time is False + + _with_db(tmp_path, "late-after-due-time.db")(_body) + + +def test_complete_instance_microsecond_after_due_time_is_late(tmp_path) -> None: + async def _body(database, child, instance): + due_date = datetime.date.fromisoformat(instance.due_date) + instance = await QuestInstancesDao(database).upsert( + instance.definition_id, + child.id, + instance.due_date, + _now_stamp(), + window=instance.window, + due_time="12:00", + ) + now = datetime.datetime( + due_date.year, due_date.month, due_date.day, 12, 0, 0, 500, + tzinfo=_HA_TZ, + ) + await complete_instance( + database, + instance.id, + actor_source="user", + actor_user_id="user-1", + now=now, + today=due_date, + ) + rows = await CompletionEventsDao(database).list_by_instance( + instance.id + ) + assert rows[0].was_on_time is False + + _with_db(tmp_path, "late-microsecond-after-due-time.db")(_body) + + +def test_complete_instance_no_due_time_on_due_date_is_on_time( + tmp_path, +) -> None: + async def _body(database, child, instance): + due_date = datetime.date.fromisoformat(instance.due_date) + assert instance.due_time is None + await complete_instance( + database, + instance.id, + actor_source="user", + actor_user_id="user-1", + now=_ha_local(due_date, 23, 59), + today=due_date, + ) + rows = await CompletionEventsDao(database).list_by_instance( + instance.id + ) + assert rows[0].was_on_time is True + + _with_db(tmp_path, "ontime-no-due-time.db")(_body) + + +def test_complete_instance_no_due_time_after_due_date_is_late( + tmp_path, +) -> None: + async def _body(database, child, instance): + due_date = datetime.date.fromisoformat(instance.due_date) + later = due_date + datetime.timedelta(days=1) + assert instance.due_time is None + await complete_instance( + database, + instance.id, + actor_source="user", + actor_user_id="user-1", + now=_ha_local(later, 8, 0), + today=later, + ) + rows = await CompletionEventsDao(database).list_by_instance( + instance.id + ) + assert rows[0].was_on_time is False + + _with_db(tmp_path, "late-after-due-date.db")(_body) + + +def test_complete_instance_ha_local_wall_clock_not_utc(tmp_path) -> None: + """HA-local 11:00 with due_time 12:00 is on time even when UTC is 19:00. + + The helper's zone is UTC-8, so 11:00 local is 19:00 UTC. Converting + to UTC before comparing would mark this late. + """ + async def _body(database, child, instance): + due_date = datetime.date.fromisoformat(instance.due_date) + instance = await QuestInstancesDao(database).upsert( + instance.definition_id, + child.id, + instance.due_date, + _now_stamp(), + window=instance.window, + due_time="12:00", + ) + now = _ha_local(due_date, 11, 0) + assert now.astimezone(datetime.timezone.utc).hour == 19 + await complete_instance( + database, + instance.id, + actor_source="user", + actor_user_id="user-1", + now=now, + today=due_date, + ) + rows = await CompletionEventsDao(database).list_by_instance( + instance.id + ) + assert rows[0].was_on_time is True + + _with_db(tmp_path, "ontime-ha-local-not-utc.db")(_body) + + +def test_complete_instance_due_time_requires_now_on_due_date(tmp_path) -> None: + async def _body(database, child, instance): + due_date = datetime.date.fromisoformat(instance.due_date) + instance = await QuestInstancesDao(database).upsert( + instance.definition_id, + child.id, + instance.due_date, + _now_stamp(), + window=instance.window, + due_time="12:00", + ) + with pytest.raises(ValueError, match="now is required to compare due_time"): + await complete_instance( + database, + instance.id, + actor_source="user", + actor_user_id="user-1", + today=due_date, + ) + + _with_db(tmp_path, "due-time-requires-now.db")(_body) + + # --------------------------------------------------------------------------- # uncomplete_instance: append uncompleted, no-op if already open # ---------------------------------------------------------------------------