docs(t-paliad-064): reminder system redesign — design doc
Design for zero-overdue SLO, per-user bundled digests (one email per slot per local-day), DRINGEND evening escalation, and global-admin escalation on overdues. Includes the actual TZ root cause (alpine container has no tzdata; LoadLocation silently falls back to UTC) and the embed-tzdata fix. Awaiting m's go/no-go before implementation.
This commit is contained in:
615
docs/design-reminder-redesign-2026-04-28.md
Normal file
615
docs/design-reminder-redesign-2026-04-28.md
Normal file
@@ -0,0 +1,615 @@
|
||||
# Reminder system redesign — zero-overdue SLO, escalation, per-user bundling
|
||||
|
||||
**Task:** t-paliad-064 (cronus, inventor)
|
||||
**Date:** 2026-04-28
|
||||
**Status:** design — awaiting m's go/no-go before implementation
|
||||
|
||||
## Problem statement (from m)
|
||||
|
||||
> "Our main purpose is to avoid ANY DEADLINE EVER becoming past due. So we
|
||||
> remind one week before and on the same day. And if it is not done by the end
|
||||
> of / late in the day, we need to send another urgent reminder and also
|
||||
> escalate."
|
||||
|
||||
Three things going wrong today:
|
||||
|
||||
1. **Timezone bug.** m set morning=09:00 Berlin and got 4 reminder emails this
|
||||
morning at 11:16 Berlin (= 09:16 UTC). Root cause is below — it is **not**
|
||||
the bug m suspected.
|
||||
2. **"Überfällig" wording is wrong.** A deadline due *today* triggered the
|
||||
`overdue` template, which says *"war heute oder früher fällig"*. "Überfällig"
|
||||
should mean past today, not today.
|
||||
3. **Schedule doesn't match the SLO.** Today's design treats overdue as a
|
||||
normal recurring nudge. m wants overdue to be a system-failure exception:
|
||||
the day-of escalation must be aggressive enough that we engineer overdues
|
||||
away.
|
||||
|
||||
---
|
||||
|
||||
## 1. The actual timezone bug
|
||||
|
||||
### What the spec hints
|
||||
|
||||
> "11:16 Berlin = 09:00 UTC + ~16min ticker phase. Fix: compute
|
||||
> `now.In(user.tz).Hour()` and compare against `user.reminder_morning_time.Hour()`."
|
||||
|
||||
### What the code already does
|
||||
|
||||
[`reminder_service.go:177-198`](../internal/services/reminder_service.go#L177-L198):
|
||||
|
||||
```go
|
||||
func inSlot(now time.Time, tz, morning, evening, slot string) bool {
|
||||
loc, err := time.LoadLocation(tz)
|
||||
if err != nil { loc = time.UTC } // <-- silent fallback
|
||||
local := now.In(loc)
|
||||
...
|
||||
return local.Hour() == hour
|
||||
}
|
||||
```
|
||||
|
||||
The conversion to `local` is already there. So the in-process logic is right.
|
||||
|
||||
### The actual root cause
|
||||
|
||||
[`Dockerfile:13-14`](../Dockerfile#L13-L14) is `alpine:3.21` with only
|
||||
`ca-certificates` installed. **The runtime image has no `tzdata` package**
|
||||
(`/usr/share/zoneinfo` doesn't exist on minimal alpine). Therefore
|
||||
`time.LoadLocation("Europe/Berlin")` returns an error in production, and
|
||||
`inSlot` silently falls back to UTC. With `local := now.In(UTC)`, the gate
|
||||
fires when `now.UTC().Hour() == 9` — which on 2026-04-28 (CEST, UTC+2) is
|
||||
**11:00 Berlin** plus the per-tick ~16min phase. Exactly what m saw.
|
||||
|
||||
This bug is invisible in `go test` on a dev box because Linux/macOS dev
|
||||
machines have system tzdata. It only manifests in the alpine container.
|
||||
|
||||
### Fix
|
||||
|
||||
Two small changes; the first is the actual fix, the second is defense-in-depth:
|
||||
|
||||
1. **Embed Go's tzdata into the binary.** Add one line to `cmd/server/main.go`:
|
||||
|
||||
```go
|
||||
import _ "time/tzdata"
|
||||
```
|
||||
|
||||
This adds ~450 KB to the binary and makes tz lookups work without OS
|
||||
`/usr/share/zoneinfo`. No Dockerfile change needed; the binary becomes
|
||||
self-contained. (Equivalent alternative: `apk add --no-cache tzdata` in
|
||||
the runtime stage — but the embedded approach also covers any future
|
||||
stripped-down container.)
|
||||
|
||||
2. **Stop falling back to UTC silently.** When `time.LoadLocation(tz)` fails,
|
||||
log a `slog.Error` and **skip the user this tick** instead of pretending
|
||||
they live in UTC. Combined with the embedded tzdata, the only way to hit
|
||||
this branch is a corrupt or empty `reminder_timezone` value — which we
|
||||
should fix at write time, not paper over at read time.
|
||||
|
||||
Add validation at the user-update boundary (`UserService.UpdateReminderTimes`
|
||||
/ settings handler / admin-team handler): reject empty or unparseable IANA
|
||||
names with HTTP 400 instead of silently storing them. Existing rows are
|
||||
safe (NOT NULL DEFAULT 'Europe/Berlin' from migration 022).
|
||||
|
||||
### Regression test
|
||||
|
||||
`TestInSlot_TZDataAvailable` — explicit check that
|
||||
`time.LoadLocation("Europe/Berlin")` succeeds in the test binary (with the
|
||||
new `_ "time/tzdata"` import in `main.go`, this is automatic in any test
|
||||
that imports the services package transitively). Plus the existing
|
||||
`TestInSlot` cases against `Europe/Berlin` already cover the conversion
|
||||
path — they pass today only because dev machines have tzdata; with the
|
||||
embed, they pass everywhere.
|
||||
|
||||
Also: a new test that asserts `inSlot` returns **false** (skip) when `tz`
|
||||
is empty or invalid — i.e. we no longer silently fall back to UTC.
|
||||
|
||||
---
|
||||
|
||||
## 2. New deadline categorization
|
||||
|
||||
Replace the current per-kind framing (`overdue`/`tomorrow`/`due_today_evening`/
|
||||
`weekly`) with four mutually exclusive categories, computed in the user's
|
||||
local tz on each tick:
|
||||
|
||||
| Category | Predicate (local date) | Wording (DE) | Wording (EN) | Severity |
|
||||
|----------------|-------------------------------------|----------------------|-----------------|----------|
|
||||
| `overdue` | `due_date < today` | "Überfällig" | "Overdue" | red, system-failure framing |
|
||||
| `due_today` | `due_date == today` | "Heute fällig" | "Due today" | amber |
|
||||
| `due_this_week`| `due_date in [today+1, today+offset]` (default offset=7) | "Diese Woche" | "This week" | informational |
|
||||
| `upcoming` | `due_date > today + offset` | "Kommend" | "Upcoming" | not in reminder emails |
|
||||
|
||||
Key correction: `due_date == today` is **not** overdue. The string *"war heute
|
||||
oder früher fällig"* is retired. Today-due deadlines render under "Heute
|
||||
fällig" in normal slots; under "DRINGEND — heute noch zu erledigen" in the
|
||||
evening escalation slot.
|
||||
|
||||
`reminder_warning_offset_days` (new column, default 7) controls the boundary
|
||||
between `due_this_week` and `upcoming`. Per-user customisation lives on the
|
||||
Settings → Notifications page.
|
||||
|
||||
---
|
||||
|
||||
## 3. New reminder schedule
|
||||
|
||||
Replace today's four send paths (`overdue` / `tomorrow` / `due_today_evening`
|
||||
/ `weekly`) with **two slots × bundled emails**, plus an exception path for
|
||||
overdues:
|
||||
|
||||
| Trigger | When (per user, in user's tz) | Audience | Email subject (DE) | Purpose |
|
||||
|----------------------|----------------------------------------------------------------|--------------------------------------------------------|----------------------------------------------------------|--------------------------|
|
||||
| **Morning digest** | morning slot, ANY of: `due_date == today+offset`, `due_date == today`, `due_date < today` (status=pending) | Created_by ∪ project lead set ∪ (global_admins, only for the overdue section) | `[Paliad] Frist-Erinnerung: N offen` (or `… ÜBERFÄLLIG: N` if any overdue) | Day-of awareness + 1-week heads-up + system-failure flag |
|
||||
| **Evening escalation** | evening slot, ANY of: `due_date == today` (status=pending), `due_date < today` (status=pending) | Created_by ∪ project lead set ∪ global_admins | `[Paliad] DRINGEND — heute noch offen: N` (or `… SYSTEMAUSFALL` if overdue) | Last call before tomorrow's escalation |
|
||||
| **Overdue carry** | morning + evening slots, while `due_date < today` and status=pending | (same as escalation; flagged as system failure) | `[Paliad] ÜBERFÄLLIG (System-Eskalation): N` | Until completed |
|
||||
|
||||
Carry rule: an overdue deadline appears in *every* slot until completed (both
|
||||
morning and evening, because it has already breached the SLO). Today-due
|
||||
deadlines appear in the morning, then in the evening escalation if still
|
||||
pending.
|
||||
|
||||
The current per-kind system is fully replaced. The Monday weekly digest
|
||||
(`weekly`) is dropped — its job (heads-up of upcoming deadlines) is now done
|
||||
per-deadline by the +`offset_days` warning, which fires exactly N days before
|
||||
each deadline rather than lumping them on a Monday.
|
||||
|
||||
### Per-trigger SQL predicate (deadline-side, in the user's tz)
|
||||
|
||||
For a candidate user U with timezone `tz`, on tick `now`:
|
||||
|
||||
```sql
|
||||
WITH local_today AS (
|
||||
SELECT (now AT TIME ZONE :tz)::date AS today
|
||||
)
|
||||
SELECT f.id, f.title, f.due_date,
|
||||
CASE
|
||||
WHEN f.due_date < (SELECT today FROM local_today) THEN 'overdue'
|
||||
WHEN f.due_date = (SELECT today FROM local_today) THEN 'due_today'
|
||||
WHEN f.due_date = (SELECT today FROM local_today) + :offset_days * INTERVAL '1 day' THEN 'due_warning'
|
||||
ELSE NULL
|
||||
END AS category
|
||||
FROM paliad.deadlines f
|
||||
WHERE f.status = 'pending'
|
||||
AND (
|
||||
f.due_date < (SELECT today FROM local_today)
|
||||
OR f.due_date = (SELECT today FROM local_today)
|
||||
OR f.due_date = (SELECT today FROM local_today) + :offset_days
|
||||
)
|
||||
AND <visibility predicate for U>
|
||||
```
|
||||
|
||||
In the evening slot, drop the `due_warning` branch (the +N-days heads-up is a
|
||||
morning-only signal):
|
||||
|
||||
```sql
|
||||
WHERE f.status = 'pending'
|
||||
AND (f.due_date < today_local OR f.due_date = today_local)
|
||||
```
|
||||
|
||||
### Audience computation
|
||||
|
||||
Three audience predicates compose the recipient set:
|
||||
|
||||
```sql
|
||||
-- 1) The deadline's creator
|
||||
created_by = U.id
|
||||
|
||||
-- 2) Project leadership along the project's hierarchy path
|
||||
EXISTS (
|
||||
SELECT 1
|
||||
FROM paliad.project_teams pt
|
||||
JOIN paliad.projects pp ON pp.id = ANY(string_to_array(p.path, '.')::uuid[])
|
||||
WHERE pt.user_id = U.id
|
||||
AND pt.project_id = pp.id
|
||||
AND pt.role = 'lead'
|
||||
)
|
||||
|
||||
-- 3) Global admin (system escalation channel)
|
||||
U.global_role = 'global_admin'
|
||||
```
|
||||
|
||||
For a given (slot, deadline, candidate user U), U is a recipient iff:
|
||||
|
||||
- **`due_warning` category:** `created_by(U)` OR `project_lead(U)` — early heads-up for the team that owns it, not for global escalation.
|
||||
- **`due_today` morning:** `created_by(U)` OR `project_lead(U)` — same.
|
||||
- **`due_today` evening (DRINGEND):** `created_by(U)` OR `project_lead(U)` OR `global_admin(U)` — the day is closing, time to escalate.
|
||||
- **`overdue` (any slot, system-failure):** `created_by(U)` OR `global_admin(U)` (+ future per-user `escalation_contact_id`) — owner and the escalation channel; project leads no longer help here, the system failed.
|
||||
|
||||
The wider audience for the urgent and overdue tiers is intentional:
|
||||
*one* person forgetting is the failure mode we want to engineer away, so by
|
||||
the evening of the due day, multiple eyes are on it.
|
||||
|
||||
---
|
||||
|
||||
## 4. Bundling: one email per user per slot
|
||||
|
||||
**Today:** N pending deadlines → N reminder emails (m got 4 this morning).
|
||||
|
||||
**New:** 1 email per (user, slot, local date). The email body is grouped by
|
||||
category, in fixed order:
|
||||
|
||||
1. ÜBERFÄLLIG (red banner, system-failure framing) — only if any
|
||||
2. DRINGEND — heute noch offen (amber, evening only) / Heute fällig (amber, morning) — only if any
|
||||
3. In einer Woche fällig (informational, morning only) — only if any
|
||||
|
||||
Each section is a table of (Frist title, Akte reference, due-date,
|
||||
"Open in Paliad" link) — the same row shape as today's `deadline_weekly.html`.
|
||||
|
||||
If all three sections are empty for a user in a given slot, no email is sent.
|
||||
|
||||
### Dedup
|
||||
|
||||
Per `(user_id, slot, local_date)`, not per deadline. A new column on
|
||||
`paliad.reminder_log`:
|
||||
|
||||
```sql
|
||||
ALTER TABLE paliad.reminder_log
|
||||
ADD COLUMN IF NOT EXISTS slot text, -- 'morning' | 'evening'
|
||||
ADD COLUMN IF NOT EXISTS slot_date date; -- user-local date
|
||||
|
||||
CREATE UNIQUE INDEX reminder_log_slot_dedup_idx
|
||||
ON paliad.reminder_log (user_id, slot, slot_date)
|
||||
WHERE slot IS NOT NULL;
|
||||
```
|
||||
|
||||
The unique index — partial on `slot IS NOT NULL` — coexists with the legacy
|
||||
`(user_id, reminder_type, deadline_id)` rows still on disk. The CHECK
|
||||
constraint on `reminder_type` widens to allow the new `'morning_digest'` /
|
||||
`'evening_digest'` values:
|
||||
|
||||
```sql
|
||||
ALTER TABLE paliad.reminder_log
|
||||
DROP CONSTRAINT IF EXISTS reminder_log_reminder_type_check;
|
||||
ALTER TABLE paliad.reminder_log
|
||||
ADD CONSTRAINT reminder_log_reminder_type_check
|
||||
CHECK (reminder_type IN ('overdue','tomorrow','weekly','morning_digest','evening_digest'));
|
||||
```
|
||||
|
||||
Ordering in the row when inserted by the new code: `slot` is the canonical
|
||||
field; `reminder_type = slot || '_digest'` is set for backward-compatibility
|
||||
with anything querying by type. `deadline_id` is NULL on digest rows.
|
||||
|
||||
### Local-date math for dedup
|
||||
|
||||
The dedup key uses the user's local date, not server-UTC. So a user in
|
||||
`Pacific/Auckland` whose morning slot fires at 18:00 UTC the previous day
|
||||
gets dedup'd against the local "tomorrow" — a second tick at 19:00 UTC that
|
||||
same evening (= local 09:00 next day) is a new local_date and would fire
|
||||
again only if their morning_time is 09:00 (which it won't be at 19:00 UTC).
|
||||
The (slot, slot_date) tuple resolves the boundary cleanly.
|
||||
|
||||
---
|
||||
|
||||
## 5. Email layout (bundled)
|
||||
|
||||
Single new template `deadline_digest.html` replaces the three current ones
|
||||
(`deadline_reminder.html`, `deadline_due_today.html`, `deadline_weekly.html`).
|
||||
|
||||
Skeleton:
|
||||
|
||||
```
|
||||
{{define "content"}}
|
||||
{{if .HasOverdue}}
|
||||
<h1 style="color:#b91c1c">{{t "ÜBERFÄLLIG" "Overdue"}} ({{.OverdueCount}})</h1>
|
||||
<p>{{t "Folgende Fristen sind nicht rechtzeitig erledigt worden. Diese E-Mail geht an die Eskalationskontakte."
|
||||
"These deadlines were not completed on time. This email goes to the escalation contacts."}}</p>
|
||||
{{template "deadline-table" .OverdueItems}}
|
||||
{{end}}
|
||||
|
||||
{{if .HasDueToday}}
|
||||
<h1 style="color:#b45309">
|
||||
{{if .Slot "evening"}}{{t "DRINGEND — heute noch offen" "URGENT — still open today"}}
|
||||
{{else}} {{t "Heute fällig" "Due today"}}{{end}}
|
||||
({{.DueTodayCount}})
|
||||
</h1>
|
||||
{{template "deadline-table" .DueTodayItems}}
|
||||
{{end}}
|
||||
|
||||
{{if .HasWarning}}
|
||||
<h2>{{t "In einer Woche fällig" "Due in one week"}} ({{.WarningCount}})</h2>
|
||||
{{template "deadline-table" .WarningItems}}
|
||||
{{end}}
|
||||
|
||||
<p style="margin-top:24px;">
|
||||
<a href="{{.DeadlinesURL}}">{{t "Alle Fristen" "All deadlines"}}</a>
|
||||
</p>
|
||||
{{end}}
|
||||
```
|
||||
|
||||
The shared `deadline-table` partial renders one row per deadline, similar to
|
||||
today's `deadline_weekly.html` table, plus an "owner" column when the
|
||||
recipient isn't the deadline's `created_by` (so a project lead seeing a
|
||||
team-mate's deadline can immediately tell whose plate it's on).
|
||||
|
||||
### Subject line
|
||||
|
||||
```
|
||||
DE morning, no overdue: [Paliad] Frist-Erinnerung: 3 offen
|
||||
DE morning, with overdue: [Paliad] ÜBERFÄLLIG: 1 — plus 3 weitere
|
||||
DE evening, no overdue: [Paliad] DRINGEND — 2 heute noch offen
|
||||
DE evening, with overdue: [Paliad] SYSTEMAUSFALL: 1 überfällig — plus 2 heute offen
|
||||
```
|
||||
|
||||
Subjects are deliberately scary when overdue is in the bundle — the SLO
|
||||
*is* "no overdues, ever".
|
||||
|
||||
---
|
||||
|
||||
## 6. Schema changes — migration **025**
|
||||
|
||||
(Note: the task brief says "migration 024", but `024_rename_department_columns.up.sql`
|
||||
already exists. The new migration is 025.)
|
||||
|
||||
```sql
|
||||
-- 025_reminder_redesign.up.sql
|
||||
|
||||
-- 1) Per-user warning offset (default 7).
|
||||
ALTER TABLE paliad.users
|
||||
ADD COLUMN IF NOT EXISTS reminder_warning_offset_days
|
||||
INT NOT NULL DEFAULT 7
|
||||
CHECK (reminder_warning_offset_days BETWEEN 1 AND 30);
|
||||
|
||||
-- 2) Optional escalation contact (deferred wiring; column ships now to
|
||||
-- avoid a follow-up migration if m says yes within a sprint).
|
||||
ALTER TABLE paliad.users
|
||||
ADD COLUMN IF NOT EXISTS escalation_contact_id UUID
|
||||
REFERENCES paliad.users(id) ON DELETE SET NULL;
|
||||
|
||||
-- 3) Slot-based dedup on reminder_log.
|
||||
ALTER TABLE paliad.reminder_log
|
||||
ADD COLUMN IF NOT EXISTS slot TEXT,
|
||||
ADD COLUMN IF NOT EXISTS slot_date DATE;
|
||||
|
||||
ALTER TABLE paliad.reminder_log
|
||||
DROP CONSTRAINT IF EXISTS reminder_log_slot_check;
|
||||
ALTER TABLE paliad.reminder_log
|
||||
ADD CONSTRAINT reminder_log_slot_check
|
||||
CHECK (slot IS NULL OR slot IN ('morning','evening'));
|
||||
|
||||
ALTER TABLE paliad.reminder_log
|
||||
DROP CONSTRAINT IF EXISTS reminder_log_reminder_type_check;
|
||||
ALTER TABLE paliad.reminder_log
|
||||
ADD CONSTRAINT reminder_log_reminder_type_check
|
||||
CHECK (reminder_type IN ('overdue','tomorrow','weekly','morning_digest','evening_digest'));
|
||||
|
||||
CREATE UNIQUE INDEX IF NOT EXISTS reminder_log_slot_dedup_idx
|
||||
ON paliad.reminder_log (user_id, slot, slot_date)
|
||||
WHERE slot IS NOT NULL;
|
||||
```
|
||||
|
||||
### Backfill
|
||||
|
||||
None needed for `reminder_warning_offset_days` (default 7 picks up existing
|
||||
rows). `escalation_contact_id` is NULL by default → behaves as "use
|
||||
global_admins" in code.
|
||||
|
||||
For `reminder_log`: legacy rows have `slot=NULL` and are ignored by the new
|
||||
dedup index (partial). The new code only queries via the partial-index
|
||||
predicate. Old rows are kept for audit; a follow-up housekeeping migration
|
||||
can prune them after the new path runs for a week.
|
||||
|
||||
### Down migration
|
||||
|
||||
`025_reminder_redesign.down.sql` drops the index, both columns, the new
|
||||
constraint variant, and restores the previous CHECK with only the original
|
||||
three values. Reversible.
|
||||
|
||||
---
|
||||
|
||||
## 7. Settings UI changes
|
||||
|
||||
Settings → Notifications gains one new control. The existing morning/evening
|
||||
times and DE/EN toggles stay.
|
||||
|
||||
```
|
||||
[ ] Master toggle: Erinnerungen aktiv
|
||||
|
||||
Morgen-Slot [09:00]
|
||||
Abend-Slot [16:00]
|
||||
Zeitzone [Europe/Berlin ▼]
|
||||
|
||||
Vorwarnung [7] Tage ← NEW (1–30)
|
||||
"Wir erinnern Sie diese viele Tage vor jeder Frist."
|
||||
```
|
||||
|
||||
Backend: `PATCH /api/me/preferences` already accepts a JSON body for
|
||||
`reminder_morning_time` etc. ([settings.ts:338-340](../frontend/src/client/settings.ts#L338-L340));
|
||||
add `reminder_warning_offset_days: number` to the same payload.
|
||||
|
||||
Validation: integer in [1, 30]; reject anything else with HTTP 400. The
|
||||
`reminder_timezone` field also gains stricter validation (reject empty
|
||||
string, reject anything `time.LoadLocation` can't parse) — the same
|
||||
validator used by the new tz-fix.
|
||||
|
||||
### Escalation contact (deferred)
|
||||
|
||||
A `<select>` populated from team users would expose
|
||||
`escalation_contact_id`. Defer the UI to a follow-up task; the column ships
|
||||
now so wiring it later doesn't need a second migration.
|
||||
|
||||
---
|
||||
|
||||
## 8. ReminderService rewrite shape
|
||||
|
||||
The existing `sendPerFrist` (per-deadline kind scan) and `sendWeekly`
|
||||
(Monday digest) are both retired. Replaced by a single
|
||||
`runSlotForUser(ctx, now, user, slot)` per (user, slot) pair the tick
|
||||
matches.
|
||||
|
||||
Loop shape:
|
||||
|
||||
```go
|
||||
func (s *ReminderService) RunOnce(ctx context.Context) {
|
||||
now := s.clock()
|
||||
users, _ := s.users.ListAll(ctx) // small table, untouched today
|
||||
for _, u := range users {
|
||||
for _, slot := range []string{"morning", "evening"} {
|
||||
if !inSlot(now, u, slot) { continue }
|
||||
if alreadySentToday(ctx, u, slot){ continue }
|
||||
if !s.preferenceAllows(u, slot) { continue }
|
||||
s.runSlotForUser(ctx, now, u, slot)
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
`runSlotForUser`:
|
||||
|
||||
1. Compute `today_local` from `now` and `u.reminder_timezone` (errors → log + skip; no UTC fallback).
|
||||
2. Pull `overdue`, `due_today`, `due_warning` deadlines for `u`'s recipient set (one query joining `paliad.deadlines`, `paliad.projects`, audience predicates from §3).
|
||||
3. If the result is empty → skip.
|
||||
4. Render `deadline_digest` with the categorized buckets.
|
||||
5. Send. Insert dedup row (user_id, slot, today_local).
|
||||
|
||||
The recipient query unifies all three audience predicates — the user is a
|
||||
recipient iff *any* of the three matches:
|
||||
|
||||
```sql
|
||||
WHERE
|
||||
-- created_by
|
||||
f.created_by = $1
|
||||
OR
|
||||
-- project lead on path
|
||||
EXISTS (SELECT 1 FROM paliad.project_teams pt
|
||||
WHERE pt.user_id = $1
|
||||
AND pt.role = 'lead'
|
||||
AND pt.project_id = ANY(string_to_array(p.path, '.')::uuid[]))
|
||||
OR
|
||||
-- global admin, but only when category is overdue / urgent
|
||||
( $2 = TRUE -- :is_global_admin
|
||||
AND ( f.due_date < $3 -- overdue
|
||||
OR ($4 = 'evening' AND f.due_date = $3) ) -- urgent today, evening only
|
||||
)
|
||||
```
|
||||
|
||||
Per-category eligibility is computed in Go after the SELECT, so the
|
||||
"global_admin only sees overdues / urgent" rule isn't smuggled into SQL.
|
||||
|
||||
---
|
||||
|
||||
## 9. Test plan
|
||||
|
||||
### Existing tests to preserve
|
||||
|
||||
- `TestReminderEnabled` — JSON preference parsing, unchanged.
|
||||
- `TestSlotForKind` — drop, kinds collapse to two slots; replace with `TestSlotMapping` over `('morning','evening')`.
|
||||
- `TestMatchesLocalDueDate` — replaced by `TestCategorize` over the four categories.
|
||||
|
||||
### New unit tests
|
||||
|
||||
| Test | What it locks down |
|
||||
|------|--------------------|
|
||||
| `TestTZDataEmbedded` | `time.LoadLocation("Europe/Berlin")` succeeds in the test binary — guards against losing the `_ "time/tzdata"` import |
|
||||
| `TestInSlot_InvalidTzSkips` | `inSlot(_, "", _, _, _) == false` and `inSlot(_, "Mars/Olympus", _, _, _) == false` (no UTC fallback) |
|
||||
| `TestCategorize_Boundaries` | due_date exactly today → `due_today` (not `overdue`); exactly today+offset → `due_warning`; today-1 → `overdue` |
|
||||
| `TestBundleEmpty_NoSend` | a user with zero matching deadlines in their slot gets no email and no log row |
|
||||
| `TestBundleMultiCategory` | one user with overdue + due_today + warning → exactly one email, three sections, one log row |
|
||||
| `TestDedupBySlotDate` | a second tick in the same slot+local-date → no second send |
|
||||
| `TestDedupRollsOverAtMidnight` | freezing the clock to advance the user's local date past midnight → next slot fires again |
|
||||
| `TestRecipientSet_OwnerOnly` | non-admin, non-lead user gets only their own deadlines |
|
||||
| `TestRecipientSet_ProjectLead` | a project lead sees a team-mate's deadline alongside their own in the same email |
|
||||
| `TestRecipientSet_GlobalAdmin` | a global_admin sees the overdue section but not the warning section |
|
||||
| `TestEscalationContactFallback` | when `escalation_contact_id` is NULL, global_admins fill the role; when set, the chosen user receives instead |
|
||||
| `TestSubjectLine_OverdueFraming` | overdue presence flips the subject from "Frist-Erinnerung" to "ÜBERFÄLLIG"/"SYSTEMAUSFALL" |
|
||||
|
||||
### TZ-fix regression test (the headline acceptance)
|
||||
|
||||
`TestInSlot_BerlinAt0900_NotAt1100` — set `now = 2026-04-28 07:00:00 UTC`,
|
||||
`tz = "Europe/Berlin"`, `morning = "09:00"`. Asserts `inSlot(...) == true`
|
||||
(09:00 Berlin). Same now with `morning = "11:00"` → false. Same now without
|
||||
the `_ "time/tzdata"` import would (today) fail; with the import it passes.
|
||||
|
||||
Plus an *integration* test against the email-send path: with `mailSvc`
|
||||
disabled (Enabled()=false), `RunOnce` at 07:00 UTC writes a dedup row for
|
||||
user m at slot=morning, slot_date=2026-04-28 — but does **not** write one
|
||||
when `now = 09:16 UTC` (= 11:16 Berlin), the bug's signature.
|
||||
|
||||
### Smoke (manual)
|
||||
|
||||
1. Set `tester@hlc.de`'s morning_time to 09:00, tz=Europe/Berlin.
|
||||
2. Create three deadlines: due 2026-04-21 (overdue), 2026-04-28 (today), 2026-05-05 (today+7).
|
||||
3. Trigger `RunOnce` at simulated `now = 2026-04-28 07:05 UTC` (= 09:05 Berlin).
|
||||
4. Verify: one email to tester@hlc.de with three sections; subject contains
|
||||
"ÜBERFÄLLIG"; one row in `paliad.reminder_log` with slot=morning,
|
||||
slot_date=2026-04-28.
|
||||
5. Trigger again at `08:05 UTC` (= 10:05 Berlin) → no second email
|
||||
(out-of-slot).
|
||||
6. Trigger at `14:05 UTC` (= 16:05 Berlin) → evening email arrives,
|
||||
"DRINGEND" wording on the today-due section, overdue section repeated.
|
||||
7. Mark the today-due deadline as completed; trigger at next morning's
|
||||
slot → only the overdue remains (system-failure framing); deadlines
|
||||
completed in the meantime are gone.
|
||||
|
||||
---
|
||||
|
||||
## 10. Migration plan
|
||||
|
||||
1. **PR 1 (this design + tz fix only):** add `_ "time/tzdata"` to `cmd/server/main.go`; tighten `inSlot` to skip on bad tz; add tz validation on user save. **Ships fast** — fixes m's prod 11:16 surprise without waiting for the full redesign. Existing schedule remains functional.
|
||||
2. **PR 2 (schema):** migration 025 (warning_offset_days, escalation_contact_id, reminder_log slot/slot_date). Idempotent, additive only. Deployed via Dokploy auto-deploy on merge to main.
|
||||
3. **PR 3 (service rewrite):** new `runSlotForUser`, new `deadline_digest` template, retire `sendPerFrist`/`sendWeekly` and the three legacy templates. Backward-compatible during deploy: the new code only writes `slot/slot_date` rows; the old code wrote `(reminder_type, deadline_id)` rows. There's no overlap window (old code is replaced, not run in parallel).
|
||||
4. **PR 4 (settings UI):** expose `warning_offset_days` on Settings → Notifications. Optional: escalation_contact dropdown if scope holds.
|
||||
5. **Cleanup follow-up (separate task):** prune legacy reminder_log rows older than 30 days; remove old templates; remove `sendWeekly` test scaffolding.
|
||||
|
||||
A single PR for #2-4 is also reasonable if the diff stays under ~600 lines.
|
||||
The tz fix in PR 1 should ship first, isolated.
|
||||
|
||||
---
|
||||
|
||||
## 11. Open questions for m
|
||||
|
||||
1. **What does "project_team admins" mean?** The `paliad.project_teams.role`
|
||||
enum is `lead | associate | pa | of_counsel | local_counsel | expert |
|
||||
observer` — there is no `admin` role. My proposal: notify `role = 'lead'`
|
||||
only. Alternative: notify `role IN ('lead','associate')`. Or: notify
|
||||
everyone on the team's path (closest to existing visibility semantics,
|
||||
but spammy for observers).
|
||||
|
||||
2. **Drop the Monday weekly digest?** The new per-deadline +7-day warning
|
||||
covers the same job (heads-up of upcoming deadlines), more precisely
|
||||
(each deadline gets its warning on its own +7 day, not lumped on Mondays).
|
||||
Proposal: drop the Monday digest. If you'd like to keep it as an
|
||||
additional weekly summary (different content from per-deadline warnings,
|
||||
e.g. "everything in your next 30 days"), say so.
|
||||
|
||||
3. **Escalation contact UI in this scope or deferred?** Column ships in
|
||||
migration 025 either way. UI dropdown pulls in user-search — could fit
|
||||
in PR 4 or be its own follow-up.
|
||||
|
||||
4. **`due_warning` recipients — owner only, or owner + leads?** Spec says
|
||||
"created_by + project_team admins". I've matched that (owner ∪ leads).
|
||||
Confirm that's wider than you intended, or accept.
|
||||
|
||||
5. **Calendar-aware skipping (weekends, holidays).** The spec asks me to
|
||||
note this as a known gap — it's not in scope. The `paliad.holidays`
|
||||
table already exists (used by Fristenrechner). A future enhancement
|
||||
could shift the +7 warning earlier when day 7 falls on a weekend, and
|
||||
skip the morning slot on weekends/holidays for low-severity categories.
|
||||
Overdue and DRINGEND should still fire — those are the SLO-critical
|
||||
ones.
|
||||
|
||||
6. **Per-deadline custom reminder offset (defer).** Today the offset is
|
||||
per-user. m might eventually want per-deadline override (e.g., "this
|
||||
filing deadline needs a +14 warning"). Out of scope for this round;
|
||||
noting for the backlog.
|
||||
|
||||
7. **One canonical worry:** the morning email when there's *no* overdue
|
||||
and *no* due-today and *no* +7 warning — i.e. nothing — does **not**
|
||||
send. Confirm that's what you want (no "everything's quiet" ack
|
||||
email). I'm proposing yes-skip; an empty-state daily ack email is
|
||||
noise.
|
||||
|
||||
---
|
||||
|
||||
## 12. Acceptance criteria (mirrored from task brief)
|
||||
|
||||
- `tester@hlc.de` morning=09:00 Berlin → ticker fires at 09:xx Berlin (= 07:xx UTC) and never at 11:xx
|
||||
- A deadline due today + still pending → "Heute fällig" bundled email at 09:00 (one email even with 4 such deadlines), then "DRINGEND" at 16:00 if still pending
|
||||
- A deadline that escapes to tomorrow uncompleted → "ÜBERFÄLLIG (System-Eskalation)" framing, sent to created_by + global_admins
|
||||
- Settings page exposes `morning_time` + `evening_time` + `warning_offset_days`
|
||||
- `go build/vet/test` clean, `bun run build` clean, regression tests for tz + bundle dedup
|
||||
- Self-merge to main authorised on PR-by-PR basis
|
||||
|
||||
---
|
||||
|
||||
## 13. Out of scope (per task brief)
|
||||
|
||||
- WhatsApp / SMS / push escalation channels — defer
|
||||
- Per-deadline custom reminder offset — defer
|
||||
- Calendar-aware skipping (weekends, holidays) — noted as known gap (§11.5)
|
||||
Reference in New Issue
Block a user