feat(sla): validate schedule/holiday JSON on write (reject malformed, 422) #420

Merged
longjacksonle merged 4 commits from feat/PMS-604-sla-json-validation into main 2026-07-01 22:51:07 +02:00

What

Follow-up to PMS-585. The SLA business-hours and holiday-calendar upsert endpoints validated only the name length; schedule and holidays were serde_json::Value with #[serde(default)] and no #[validate(...)], so any JSON shape was accepted and stored. The clock reader then silently skips unknown weekday keys, malformed windows, and unparseable holiday entries, so a typo'd payload is persisted and quietly ignored, distorting SLA due-time math (a day the admin believes is closed, or a holiday that never takes effect).

This adds strict write-time validation for both fields. It is a validation gap independent of the JSONB-vs-normalized decision (PMS-585 kept JSONB; see dev-docs/sla-jsonb-vs-normalized.md).

How

  • Widen the clock reader's parse_weekday_key / parse_hhmm from private to pub(crate) so the strict validator accepts exactly the key set and time formats the engine parses. The strict writer and the tolerant reader single-source the same helpers and cannot drift.
  • validate_business_schedule (custom validator on UpsertBusinessHoursRequest::schedule): accepts null / empty object (24/7), or a weekday-keyed object whose day values are null, a {start, end} window, or an array of windows with parseable HH:MM(:SS) times and end strictly after start. Everything else is a 422.
  • validate_holiday_list (custom validator on UpsertHolidayCalendarRequest::holidays): accepts null / empty array, or an array of bare YYYY-MM-DD strings or {date, name} objects with a parseable date. Everything else is a 422.
  • Both errors are keyed onto their own field (schedule / holidays) so the frontend binds the message inline rather than as a generic banner.

Tests

cargo test -p mokosh-server --lib modules::sla::models (10 tests): valid/empty shapes accepted; malformed payloads (non-object schedule, unknown weekday key, non-window day value, missing/unparseable times, end <= start, non-array holidays, non-date entries, impossible dates) rejected; plus one end-to-end .validate() per request confirming the error keys onto the right field.

The frontend inline-error wiring (AC3) ships as a separate mokosh-apps PR; the server already surfaces these as field-keyed 422s that the existing form banner shows in the meantime.

Closes PMS-604.

## What Follow-up to PMS-585. The SLA business-hours and holiday-calendar upsert endpoints validated only the `name` length; `schedule` and `holidays` were `serde_json::Value` with `#[serde(default)]` and no `#[validate(...)]`, so any JSON shape was accepted and stored. The clock reader then silently skips unknown weekday keys, malformed windows, and unparseable holiday entries, so a typo'd payload is persisted and quietly ignored, distorting SLA due-time math (a day the admin believes is closed, or a holiday that never takes effect). This adds strict write-time validation for both fields. It is a validation gap independent of the JSONB-vs-normalized decision (PMS-585 kept JSONB; see `dev-docs/sla-jsonb-vs-normalized.md`). ## How - Widen the clock reader's `parse_weekday_key` / `parse_hhmm` from private to `pub(crate)` so the strict validator accepts exactly the key set and time formats the engine parses. The strict writer and the tolerant reader single-source the same helpers and cannot drift. - `validate_business_schedule` (custom validator on `UpsertBusinessHoursRequest::schedule`): accepts null / empty object (24/7), or a weekday-keyed object whose day values are null, a `{start, end}` window, or an array of windows with parseable `HH:MM`(`:SS`) times and `end` strictly after `start`. Everything else is a 422. - `validate_holiday_list` (custom validator on `UpsertHolidayCalendarRequest::holidays`): accepts null / empty array, or an array of bare `YYYY-MM-DD` strings or `{date, name}` objects with a parseable date. Everything else is a 422. - Both errors are keyed onto their own field (`schedule` / `holidays`) so the frontend binds the message inline rather than as a generic banner. ## Tests `cargo test -p mokosh-server --lib modules::sla::models` (10 tests): valid/empty shapes accepted; malformed payloads (non-object schedule, unknown weekday key, non-window day value, missing/unparseable times, `end <= start`, non-array holidays, non-date entries, impossible dates) rejected; plus one end-to-end `.validate()` per request confirming the error keys onto the right field. The frontend inline-error wiring (AC3) ships as a separate mokosh-apps PR; the server already surfaces these as field-keyed 422s that the existing form banner shows in the meantime. Closes PMS-604.
PMS-604 groundwork. `parse_weekday_key` and `parse_hhmm` are the single source of truth for which weekday keys and time formats the SLA clock understands. Widening them from private to `pub(crate)` lets the upcoming write-time schedule validator accept exactly the shapes the engine parses, so the strict validator and the tolerant reader stay in lock-step instead of duplicating the mapping.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
PMS-604. The `schedule` field on UpsertBusinessHoursRequest was `serde_json::Value` with `#[serde(default)]` and no validation, so any JSON shape was accepted and stored. The clock reader then silently skips unknown weekday keys and malformed windows, so a typo'd payload is persisted and quietly ignored, distorting SLA due-time math (a day the admin believes is closed, or a window that never takes effect).

Add a strict `validate_business_schedule` custom validator run on create/update. It accepts exactly the shapes the reader understands (null / empty object for 24/7, weekday-keyed windows) by reusing the reader's own `parse_weekday_key` / `parse_hhmm`, so the strict writer and tolerant reader cannot drift. Anything else (non-object payload, unknown weekday key, non-window day value, missing/unparseable times, end <= start) is rejected with a 422 keyed onto the `schedule` field so the frontend can bind it inline.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
PMS-604. Same gap as the schedule field: `holidays` was an unvalidated `serde_json::Value`, and the clock reader (`parse_holidays`) silently skips unparseable entries. A holiday the admin meant to exclude but mistyped would be stored and quietly ignored, so the SLA clock keeps running on what should be a day off.

Add `validate_holiday_list`, mirroring the reader's accepted shapes: null / empty array for "no holidays", otherwise an array whose entries are each a bare "YYYY-MM-DD" string or a {date, name} object with a parseable date. Any other shape or a bad date is rejected with a 422 keyed onto the `holidays` field.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
test(sla): cover accept/reject cases for schedule and holiday JSON
All checks were successful
E2E / Playwright against staging (pull_request) Successful in 38s
Check / fmt + clippy + build + tests (pull_request) Successful in 1m35s
Create release / Gate (release-branch merges only) (pull_request) Successful in 1s
Create release / Create release from merged PR (pull_request) Has been skipped
Integration / integration tests (pull_request) Successful in 7m40s
774cc389bb
PMS-604. Unit-test both write-time validators directly (valid/empty shapes accepted; non-object payloads, unknown weekday keys, non-window day values, missing/unparseable times, end <= start, non-array holidays, non-date entries and impossible dates rejected), plus one end-to-end `.validate()` per request confirming the error keys onto the `schedule` / `holidays` field so the frontend binds it inline. Also picks up rustfmt's re-wrap of the window-tuple destructure.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
longjacksonle deleted branch feat/PMS-604-sla-json-validation 2026-07-01 22:51:07 +02:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
psa-systems/mokosh-server!420
No description provided.