feat(immich): multi-time-point scheduling for scheduled/periodic/memory

Replace the single comma-separated text box with an add/remove list of
native HH:MM pickers for the Immich scheduled-assets, periodic-summary,
and memory slots. The backend already stored comma-separated *_times and
scheduled one cron job per time; this makes entering several times per
day discoverable and hardens the read/write path.

Backend:
- services/time_list.py: normalize_time_list (validate / dedup / sort /
  cap at 24, raising TimeListError) + lenient parse_hhmm_list; the
  scheduler now uses the shared parser (drops its private copy).
- tracking_configs API normalizes *_times on every write (422 on bad
  input) and rejects enabling a slot whose times list is empty.
- scheduler warns when an enabled slot has zero or dropped fire times,
  restoring the observability lost with the old per-call warning.

Frontend:
- TimeListEditor.svelte: add/remove native time rows, dedupe + sort on
  emit, per-day cap, collapses on-screen duplicates, aria-labelled rows;
  syncs from the value prop only on external changes (untrack guard) so
  keyboard entry isn't clobbered mid-edit.
- Descriptor-driven save guard: an enabled feature section must have at
  least one time.
- i18n (en/ru) keys; refreshed help text; removed dead invalidTimeList.

Tests: time_list normalization/parsing (incl. non-ASCII/odd shapes) and
the enabled-implies-times validation.
This commit is contained in:
2026-05-29 14:57:41 +03:00
parent e2c17dd343
commit 8065e6effa
11 changed files with 718 additions and 89 deletions
@@ -11,11 +11,66 @@ from ..auth.dependencies import get_current_user
from ..database.engine import get_session
from ..database.models import TrackingConfig, User
from ..services.scheduler import reschedule_immich_dispatch_jobs
from ..services.time_list import TimeListError, normalize_time_list
_LOGGER = logging.getLogger(__name__)
router = APIRouter(prefix="/api/tracking-configs", tags=["tracking-configs"])
# Immich dispatch slots that fire on a wall-clock schedule. Each has a
# ``{kind}_enabled`` flag and a ``{kind}_times`` comma-separated HH:MM list.
_DISPATCH_KINDS = ("periodic", "scheduled", "memory")
# TrackingConfig fields holding comma-separated ``HH:MM`` dispatch schedules.
# Normalized (validated, de-duplicated, sorted, capped) on every write so the
# scheduler only ever reads clean values and cron jobs stay deterministic.
_TIME_LIST_FIELDS = tuple(f"{k}_times" for k in _DISPATCH_KINDS)
def _normalize_time_fields(values: dict) -> None:
"""Canonicalize any ``*_times`` keys present in ``values`` (in place).
Raises HTTP 422 with a field-scoped message when an entry is malformed or
the list exceeds the per-day cap, so the client surfaces exactly which slot
was rejected instead of the input being silently dropped at schedule time.
"""
for field in _TIME_LIST_FIELDS:
if values.get(field) is None:
continue
try:
values[field] = normalize_time_list(values[field])
except TimeListError as err:
raise HTTPException(
status_code=status.HTTP_422_UNPROCESSABLE_ENTITY,
detail=f"{field}: {err}",
) from err
def _validate_enabled_have_times(values: dict, existing: TrackingConfig | None) -> None:
"""Reject enabling a dispatch slot with no fire times.
An enabled slot whose ``{kind}_times`` normalizes to empty would save fine
but the scheduler creates zero cron jobs for it — a silently dead slot that
shows as "enabled" in the UI yet never fires. We fail the write with a 422
instead. Only kinds the request actually touches (enabled flag or times) are
checked, so unrelated edits to a pre-existing config aren't blocked; the
effective state is the request value merged over ``existing``.
"""
for kind in _DISPATCH_KINDS:
enabled_key, times_key = f"{kind}_enabled", f"{kind}_times"
if enabled_key not in values and times_key not in values:
continue
enabled = values.get(
enabled_key, getattr(existing, enabled_key, False) if existing else False
)
times = values.get(
times_key, getattr(existing, times_key, "") if existing else ""
)
if enabled and not (times or "").strip():
raise HTTPException(
status_code=status.HTTP_422_UNPROCESSABLE_ENTITY,
detail=f"{times_key}: add at least one time when {kind} is enabled",
)
class TrackingConfigCreate(BaseModel):
provider_type: str
@@ -124,7 +179,10 @@ async def create_config(
user: User = Depends(get_current_user),
session: AsyncSession = Depends(get_session),
):
config = TrackingConfig(user_id=user.id, **body.model_dump())
data = body.model_dump()
_normalize_time_fields(data)
_validate_enabled_have_times(data, None)
config = TrackingConfig(user_id=user.id, **data)
session.add(config)
await session.commit()
await session.refresh(config)
@@ -150,7 +208,10 @@ async def update_config(
session: AsyncSession = Depends(get_session),
):
config = await _get(session, config_id, user.id)
for field, value in body.model_dump(exclude_unset=True).items():
updates = body.model_dump(exclude_unset=True)
_normalize_time_fields(updates)
_validate_enabled_have_times(updates, config)
for field, value in updates.items():
setattr(config, field, value)
session.add(config)
await session.commit()
@@ -7,6 +7,8 @@ from zoneinfo import ZoneInfo, ZoneInfoNotFoundError
from apscheduler.schedulers.asyncio import AsyncIOScheduler
from .time_list import parse_hhmm_list
_LOGGER = logging.getLogger(__name__)
@@ -896,30 +898,6 @@ _IMMICH_DISPATCH_KINDS = ("scheduled", "periodic", "memory")
_IMMICH_DISPATCH_PREFIX = "immich_dispatch_"
def _parse_hhmm_list(raw: str) -> list[tuple[int, int]]:
"""Parse ``"09:00,18:30"`` → ``[(9, 0), (18, 30)]``, skipping bad entries.
A typo in one slot must not prevent the others from scheduling — we log
and move on rather than raising.
"""
out: list[tuple[int, int]] = []
for part in (raw or "").split(","):
part = part.strip()
if not part:
continue
try:
h_str, m_str = part.split(":", 1)
hour, minute = int(h_str), int(m_str)
except ValueError:
_LOGGER.warning("Skipping invalid time literal %r", part)
continue
if not (0 <= hour <= 23 and 0 <= minute <= 59):
_LOGGER.warning("Skipping out-of-range time %r", part)
continue
out.append((hour, minute))
return out
async def _run_immich_dispatch(tracker_id: int, kind: str) -> None:
"""APScheduler entry point — wraps the dispatch helper to swallow errors."""
from .scheduled_dispatch import dispatch_scheduled_for_tracker
@@ -994,7 +972,24 @@ async def _load_immich_dispatch_jobs() -> None:
if not getattr(tc, f"{kind}_enabled", False):
continue
times_raw = getattr(tc, f"{kind}_times", "") or ""
for hour, minute in _parse_hhmm_list(times_raw):
parsed = parse_hhmm_list(times_raw)
# Observability for misconfigured/legacy data: warn when some tokens
# were unparseable (the lenient parser drops them silently) and when
# an enabled slot resolves to zero fire times (it will never fire).
raw_tokens = [p for p in times_raw.split(",") if p.strip()]
if len(parsed) < len(raw_tokens):
_LOGGER.warning(
"Tracker %d %s: dropped %d unparseable time(s) from %r",
tracker.id, kind, len(raw_tokens) - len(parsed), times_raw,
)
if not parsed:
_LOGGER.warning(
"Tracker %d has %s enabled but no valid fire times (%r); "
"slot will not fire",
tracker.id, kind, times_raw,
)
continue
for hour, minute in parsed:
job_id = f"{_IMMICH_DISPATCH_PREFIX}{kind}_{tracker.id}_{hour:02d}{minute:02d}"
scheduler.add_job(
_run_immich_dispatch,
@@ -0,0 +1,99 @@
"""Parsing + normalization for comma-separated ``HH:MM`` dispatch time lists.
The Immich scheduled / periodic / memory slots fire at one or more wall-clock
times per day, stored on ``TrackingConfig`` as a comma-separated ``HH:MM``
string (e.g. ``"09:00,18:30"``). Two consumers share this module:
* the API layer (``api.tracking_configs``) calls :func:`normalize_time_list`
to validate + canonicalize user input before persisting — rejecting malformed
entries, de-duplicating, sorting ascending, and capping the count.
* the scheduler (``services.scheduler``) calls :func:`parse_hhmm_list` to turn a
stored (already-normalized) string into ``(hour, minute)`` tuples, tolerating
legacy or hand-edited values by skipping anything unparseable rather than
letting one bad slot stop the others from firing.
"""
from __future__ import annotations
# A generous ceiling: 24 distinct fire times per day is already an unusual
# config. The cap mainly exists so a pathological paste can't spawn hundreds of
# cron jobs for a single tracker.
MAX_DISPATCH_TIMES = 24
class TimeListError(ValueError):
"""Raised by :func:`normalize_time_list` on malformed input or over-cap.
Subclasses ``ValueError`` so callers that only care about "bad input" can
catch the broad type; the API layer catches this specifically to map it to
an HTTP 422 with the human-readable message.
"""
def _parse_one(token: str) -> tuple[int, int] | None:
"""Parse a single ``HH:MM`` token; return ``None`` if malformed/out-of-range.
Strict on shape (exactly one ``:``, both parts 1-2 plain ASCII digits) but
lenient on width so ``"9:0"`` parses to ``(9, 0)`` — :func:`normalize_time_list`
re-emits it zero-padded. ``int()`` alone is too permissive here: it would
accept signs (``"+9"``), PEP 515 underscores (``"1_0"``), and non-ASCII
decimal digits (Arabic-Indic ``"٠٩"``), none of which are valid wall-clock
literals — so we gate on ``str.isdigit()``/ASCII before converting.
"""
h_str, sep, m_str = token.partition(":")
if not sep:
return None
for part in (h_str, m_str):
if not (1 <= len(part) <= 2 and part.isascii() and part.isdigit()):
return None
hour, minute = int(h_str), int(m_str)
if not (0 <= hour <= 23 and 0 <= minute <= 59):
return None
return hour, minute
def parse_hhmm_list(raw: str) -> list[tuple[int, int]]:
"""Lenient parse: ``"09:00,18:30"`` → ``[(9, 0), (18, 30)]``.
Skips blank/invalid entries rather than raising — the scheduler must keep
firing the valid times even if one slot was hand-edited to garbage. Order is
preserved and duplicates are *not* collapsed (the scheduler keys jobs by
time, so a duplicate simply replaces its own job id).
"""
out: list[tuple[int, int]] = []
for part in (raw or "").split(","):
part = part.strip()
if not part:
continue
hm = _parse_one(part)
if hm is not None:
out.append(hm)
return out
def normalize_time_list(raw: str, *, max_count: int = MAX_DISPATCH_TIMES) -> str:
"""Validate + canonicalize a comma-separated ``HH:MM`` list.
Returns the canonical form: zero-padded, de-duplicated, sorted ascending,
and comma-joined with no spaces — e.g. ``" 9:0, 18:30 ,09:00"`` →
``"09:00,18:30"``. An empty/whitespace input returns ``""`` (the valid
"no scheduled fires" state).
Raises :class:`TimeListError` when any entry is not a valid ``HH:MM`` time,
or when the de-duplicated count exceeds ``max_count``. The caller maps this
to an HTTP 422 so the user gets a clear message instead of silent dropping.
"""
seen: set[tuple[int, int]] = set()
for part in (raw or "").split(","):
part = part.strip()
if not part:
continue
hm = _parse_one(part)
if hm is None:
raise TimeListError(f"Invalid time {part!r}; use HH:MM (00:00-23:59)")
seen.add(hm)
if len(seen) > max_count:
raise TimeListError(
f"Too many times ({len(seen)}); at most {max_count} are allowed"
)
return ",".join(f"{h:02d}:{m:02d}" for h, m in sorted(seen))
+116
View File
@@ -0,0 +1,116 @@
"""Unit tests for the shared HH:MM dispatch time-list parser/normalizer."""
from __future__ import annotations
import pytest
from notify_bridge_server.services.time_list import (
MAX_DISPATCH_TIMES,
TimeListError,
normalize_time_list,
parse_hhmm_list,
)
class TestNormalizeTimeList:
def test_single_time_passthrough(self):
assert normalize_time_list("09:00") == "09:00"
def test_zero_pads_short_parts(self):
assert normalize_time_list("9:0") == "09:00"
def test_trims_surrounding_and_inner_whitespace(self):
assert normalize_time_list(" 09:00 , 18:30 ") == "09:00,18:30"
def test_sorts_ascending(self):
assert normalize_time_list("18:30,09:00,12:15") == "09:00,12:15,18:30"
def test_deduplicates(self):
# Duplicates collapse even when written with different padding.
assert normalize_time_list("09:00,9:00,09:00") == "09:00"
def test_empty_string_returns_empty(self):
assert normalize_time_list("") == ""
def test_whitespace_only_returns_empty(self):
assert normalize_time_list(" ") == ""
def test_trailing_comma_ignored(self):
assert normalize_time_list("09:00,") == "09:00"
def test_midnight_and_last_minute_are_valid(self):
assert normalize_time_list("00:00,23:59") == "00:00,23:59"
@pytest.mark.parametrize(
"bad",
["24:00", "09:60", "noon", "9", "09-00", "-1:00", "09:5a", "1:2:3"],
)
def test_invalid_entry_raises(self, bad):
with pytest.raises(TimeListError):
normalize_time_list(bad)
@pytest.mark.parametrize(
"bad",
[
"+9:00", # sign not allowed
"1_0:00", # PEP 515 underscore not allowed
"09:0_0", # underscore in minutes
"٩:00", # Arabic-Indic digit (non-ASCII)
"٠٩:٠٠", # Arabic-Indic "09:00"
"09: 00", # inner whitespace in part
"9 :00", # inner whitespace in part
"009:00", # 3-digit hour part
],
)
def test_rejects_non_ascii_or_oddly_shaped_digits(self, bad):
# int() alone would accept these; the parser must not.
with pytest.raises(TimeListError):
normalize_time_list(bad)
def test_one_bad_entry_rejects_whole_list(self):
# Strict on writes: a single bad slot fails the request rather than
# being silently dropped.
with pytest.raises(TimeListError):
normalize_time_list("09:00,bad,18:30")
def test_at_cap_is_allowed(self):
times = ",".join(f"{h:02d}:00" for h in range(MAX_DISPATCH_TIMES))
assert normalize_time_list(times) == times
def test_over_cap_raises(self):
# 25 distinct times (minute-granular) exceeds the default cap of 24.
times = ",".join(f"00:{m:02d}" for m in range(MAX_DISPATCH_TIMES + 1))
with pytest.raises(TimeListError):
normalize_time_list(times)
def test_custom_max_count(self):
with pytest.raises(TimeListError):
normalize_time_list("09:00,10:00,11:00", max_count=2)
def test_duplicates_do_not_count_against_cap(self):
# Three entries but one distinct → fits under a cap of 1.
assert normalize_time_list("09:00,09:00,9:00", max_count=1) == "09:00"
class TestParseHhmmList:
def test_basic(self):
assert parse_hhmm_list("09:00,18:30") == [(9, 0), (18, 30)]
def test_preserves_order_and_duplicates(self):
# Lenient parser keeps input order and does not collapse duplicates —
# the scheduler keys jobs by time so a dup just replaces its own id.
assert parse_hhmm_list("18:30,09:00,18:30") == [(18, 30), (9, 0), (18, 30)]
def test_skips_invalid_entries(self):
assert parse_hhmm_list("09:00,bad,18:30") == [(9, 0), (18, 30)]
def test_empty_returns_empty_list(self):
assert parse_hhmm_list("") == []
assert parse_hhmm_list(" ") == []
def test_skips_out_of_range(self):
assert parse_hhmm_list("24:00,09:00") == [(9, 0)]
def test_skips_non_ascii_and_oddly_shaped(self):
# Lenient parser drops the odd shapes but keeps the valid neighbour.
assert parse_hhmm_list("+9:00,1_0:00,٠٩:٠٠,18:30") == [(18, 30)]
@@ -0,0 +1,114 @@
"""Tests for the tracking-config write-time guards.
Covers the cross-field rule that an *enabled* Immich dispatch slot
(periodic / scheduled / memory) must carry at least one fire time — otherwise
the config saves but the scheduler creates zero cron jobs and the slot is
silently dead.
"""
from __future__ import annotations
import pytest
from fastapi import HTTPException
from notify_bridge_server.api.tracking_configs import (
_normalize_time_fields,
_validate_enabled_have_times,
)
from notify_bridge_server.database.models import TrackingConfig
class TestNormalizeTimeFields:
def test_normalizes_each_field_in_place(self):
values = {
"periodic_times": "18:30, 09:00",
"scheduled_times": "9:0",
"memory_times": "",
}
_normalize_time_fields(values)
assert values["periodic_times"] == "09:00,18:30"
assert values["scheduled_times"] == "09:00"
assert values["memory_times"] == ""
def test_ignores_absent_fields(self):
values = {"name": "x"}
_normalize_time_fields(values) # no KeyError, no-op
assert values == {"name": "x"}
def test_malformed_raises_422(self):
with pytest.raises(HTTPException) as exc:
_normalize_time_fields({"scheduled_times": "25:00"})
assert exc.value.status_code == 422
assert "scheduled_times" in str(exc.value.detail)
class TestValidateEnabledHaveTimes:
# --- create path (existing=None) ---
def test_create_enabled_with_times_ok(self):
_validate_enabled_have_times(
{"scheduled_enabled": True, "scheduled_times": "09:00"}, None
)
def test_create_enabled_without_times_raises(self):
with pytest.raises(HTTPException) as exc:
_validate_enabled_have_times(
{"scheduled_enabled": True, "scheduled_times": ""}, None
)
assert exc.value.status_code == 422
assert "scheduled_times" in str(exc.value.detail)
def test_create_disabled_without_times_ok(self):
_validate_enabled_have_times(
{"memory_enabled": False, "memory_times": ""}, None
)
def test_all_three_kinds_checked(self):
for kind in ("periodic", "scheduled", "memory"):
with pytest.raises(HTTPException):
_validate_enabled_have_times(
{f"{kind}_enabled": True, f"{kind}_times": ""}, None
)
# --- update path (effective state = values merged over existing) ---
def test_update_enable_without_providing_times_uses_existing_empty(self):
existing = TrackingConfig(provider_type="immich", name="t", scheduled_times="")
with pytest.raises(HTTPException):
_validate_enabled_have_times({"scheduled_enabled": True}, existing)
def test_update_enable_with_existing_times_ok(self):
existing = TrackingConfig(
provider_type="immich", name="t", scheduled_times="09:00"
)
_validate_enabled_have_times({"scheduled_enabled": True}, existing)
def test_update_clear_times_while_enabled_in_existing_raises(self):
existing = TrackingConfig(
provider_type="immich",
name="t",
scheduled_enabled=True,
scheduled_times="09:00",
)
with pytest.raises(HTTPException):
_validate_enabled_have_times({"scheduled_times": ""}, existing)
def test_update_unrelated_field_does_not_trigger_check(self):
# A pre-existing enabled-but-empty config must not block edits that don't
# touch the slot's enabled flag or times (only touched kinds are checked).
existing = TrackingConfig(
provider_type="immich",
name="t",
scheduled_enabled=True,
scheduled_times="",
)
_validate_enabled_have_times({"name": "renamed"}, existing)
def test_update_disabling_slot_clears_requirement(self):
existing = TrackingConfig(
provider_type="immich",
name="t",
scheduled_enabled=True,
scheduled_times="09:00",
)
_validate_enabled_have_times(
{"scheduled_enabled": False, "scheduled_times": ""}, existing
)