From e34f1cd8213cb9662f765948edc5f2c85346a31e Mon Sep 17 00:00:00 2001 From: Agent Date: Wed, 23 Sep 2026 10:50:04 -0400 Subject: [PATCH] feat: allow users to choose digest schedule (hour, minute, dayOfWeek, timezone) Add DB migration (hour, minute, day_of_week, timezone columns) with idempotent ALTER TABLE upgrade path for existing databases. Extend getCronExpression with optional dayOfWeek parameter; weekly defaults to Monday (1) when not specified. Worker createJob now passes stored schedule fields to cron expression and uses the subscription's IANA timezone instead of hardcoded 'UTC'. PUT /subscription/email accepts optional hour (0-23), minute (0-59), dayOfWeek (1-7, only for weekly), timezone (IANA). GET response includes the new fields. Omitted fields fall back to defaults (17:00 UTC). Closes mailship-200 --- README.md | 23 +++++++-- src/actions.ts | 8 ++- src/alert.ts | 39 +++++++++++++-- src/database.ts | 57 ++++++++++++++++----- src/server.ts | 38 +++++++++++++- src/worker/email.ts | 4 +- test/schedule-fields.test.ts | 97 ++++++++++++++++++++++++++++++++++++ 7 files changed, 239 insertions(+), 27 deletions(-) create mode 100644 test/schedule-fields.test.ts diff --git a/README.md b/README.md index 53634a8..d07fedc 100644 --- a/README.md +++ b/README.md @@ -51,15 +51,30 @@ Flotilla ──HTTP──▶ Mailship (PUT /subscription/email) ### PUT /subscription/email Idempotently register or update an email subscription. Re-sends the confirmation email only when the subscription is new or the email address changed; a frequency -change keeps the existing confirmation. The pubkey is extracted from the NIP-98 -Authorization header — the body does not include a `pubkey` field. +or schedule change keeps the existing confirmation. The pubkey is extracted from +the NIP-98 Authorization header — the body does not include a `pubkey` field. ``` -Body: { email, frequency } +Body: { email, frequency, hour?, minute?, dayOfWeek?, timezone? } Auth: NIP-98 (Nostr Authorization header) Response: { key, callback } ``` +**Optional schedule fields (defaults: `hour=17`, `minute=0`, `timezone="UTC"`):** + +| Field | Type | Constraints | Default | +|---|---|---|---| +| `hour` | integer | 0–23 | `17` | +| `minute` | integer | 0–59 | `0` | +| `dayOfWeek` | integer | 1=Mon … 7=Sun (0 also accepted as Sun); only valid when `frequency="weekly"` | `1` (Monday) for weekly, omitted for daily | +| `timezone` | string | IANA timezone name, e.g. `"America/New_York"` | `"UTC"` | + +When omitted, each field falls back to its default. `dayOfWeek` is silently +ignored for daily frequency (the stored value is `NULL`). + +**DOW convention:** `dayOfWeek` follows cron: 1=Monday, 2=Tuesday, …, 7=Sunday. +`0` is also accepted as Sunday (standard cron alias). + ### GET /subscription/email Look up an existing subscription for the authenticated pubkey, so clients can avoid re-registering (and re-confirming) when settings haven't changed. @@ -67,7 +82,7 @@ Returns 404 if none exists. ``` Auth: NIP-98 (Nostr Authorization header) -Response: { key, callback, email, frequency, confirmed } +Response: { key, callback, email, frequency, hour, minute, dayOfWeek, timezone, confirmed } ``` ### DELETE /subscription/:key diff --git a/src/actions.ts b/src/actions.ts index 3235afd..dec2608 100644 --- a/src/actions.ts +++ b/src/actions.ts @@ -13,12 +13,16 @@ export type RegisterSubscriptionParams = { pubkey: string email: string frequency: string + hour?: number + minute?: number + dayOfWeek?: number + timezone?: string } export const registerSubscription = instrument( 'actions.registerSubscription', - async ({ pubkey, email, frequency }: RegisterSubscriptionParams) => { - const sub = await db.insertSubscription(pubkey, email, frequency) + async ({ pubkey, email, frequency, hour, minute, dayOfWeek, timezone }: RegisterSubscriptionParams) => { + const sub = await db.insertSubscription(pubkey, email, frequency, hour, minute, dayOfWeek, timezone) const callback = `${process.env.BASE_URL}/notify/${sub.id}` if (!sub.confirmed_at) { diff --git a/src/alert.ts b/src/alert.ts index c61586f..542086b 100644 --- a/src/alert.ts +++ b/src/alert.ts @@ -4,6 +4,10 @@ export type Subscription = { pubkey: string email: string frequency: string + hour: number + minute: number + day_of_week?: number + timezone: string created_at: number confirmed_at?: number unsubscribed_at?: number @@ -19,14 +23,39 @@ export const getSubscriptionError = (sub: Subscription) => { return 'Frequency must be "daily" or "weekly"' } - // Daily: fire at 17:00 UTC. Weekly: fire Monday at 17:00 UTC. - // Validation: just ensure frequency is valid, cron is generated internally. + if (sub.hour < 0 || sub.hour > 23 || !Number.isInteger(sub.hour)) { + return 'Hour must be an integer between 0 and 23' + } + + if (sub.minute < 0 || sub.minute > 59 || !Number.isInteger(sub.minute)) { + return 'Minute must be an integer between 0 and 59' + } + + if (sub.frequency === 'weekly') { + if (sub.day_of_week === undefined || sub.day_of_week === null) { + return 'dayOfWeek is required for weekly frequency' + } + const dow = sub.day_of_week + if (dow < 0 || dow > 7 || !Number.isInteger(dow)) { + return 'dayOfWeek must be an integer between 0 and 7 (1=Mon, 7=Sun, 0=Sun)' + } + } + + if (!sub.timezone || typeof sub.timezone !== 'string') { + return 'A valid IANA timezone is required' + } + + try { + Intl.DateTimeFormat(undefined, { timeZone: sub.timezone }) + } catch { + return `"${sub.timezone}" is not a valid IANA timezone` + } } -export const getCronExpression = (frequency: string, hour = 17, minute = 0) => { +export const getCronExpression = (frequency: string, hour = 17, minute = 0, dayOfWeek?: number) => { if (frequency === 'daily') { return `0 ${minute} ${hour} * * *` } - // Weekly on Monday - return `0 ${minute} ${hour} * * 1` + // Weekly: default to Monday (1) when not specified + return `0 ${minute} ${hour} * * ${dayOfWeek ?? 1}` } \ No newline at end of file diff --git a/src/database.ts b/src/database.ts index 25d70d1..ce52cc3 100644 --- a/src/database.ts +++ b/src/database.ts @@ -9,7 +9,7 @@ import type { Subscription } from './alert.js' const DATA_DIR = process.env.DATA_DIR || '.' const db = new sqlite3.Database(DATA_DIR + '/mailship.db') -type Param = number | string | boolean +type Param = number | string | boolean | null | undefined type Row = Record @@ -58,6 +58,10 @@ export const migrate = () => pubkey TEXT NOT NULL, email TEXT NOT NULL, frequency TEXT NOT NULL DEFAULT 'daily', + hour INTEGER NOT NULL DEFAULT 17, + minute INTEGER NOT NULL DEFAULT 0, + day_of_week INTEGER, + timezone TEXT NOT NULL DEFAULT 'UTC', created_at INTEGER NOT NULL, confirmed_at INTEGER, unsubscribed_at INTEGER, @@ -108,6 +112,20 @@ export const migrate = () => WHERE unsubscribed_at IS NULL ` ) + + // Idempotent migration: add schedule columns to existing databases. + // ALTER TABLE ADD COLUMN fails if the column already exists, so we + // run each one and ignore "duplicate column" errors. + const addCol = (colDef: string) => + run(`ALTER TABLE subscriptions ADD COLUMN ${colDef}`).catch((err: any) => { + if (!err?.message?.includes('duplicate column')) throw err + }) + + await addCol('hour INTEGER NOT NULL DEFAULT 17') + await addCol('minute INTEGER NOT NULL DEFAULT 0') + await addCol('day_of_week INTEGER') + await addCol("timezone TEXT NOT NULL DEFAULT 'UTC'") + resolve() }) } catch (err) { @@ -125,28 +143,39 @@ const parseSubscription = (row: any): Subscription | undefined => { export const updateSubscription = instrument( 'database.updateSubscription', - async (existing: Subscription, email: string, frequency: string) => { - if (existing.email === email && existing.frequency === frequency) { + async (existing: Subscription, email: string, frequency: string, hour?: number, minute?: number, dayOfWeek?: number, timezone?: string) => { + const scheduleChanged = + (hour !== undefined && hour !== existing.hour) || + (minute !== undefined && minute !== existing.minute) || + (dayOfWeek !== undefined && dayOfWeek !== existing.day_of_week) || + (timezone !== undefined && timezone !== existing.timezone) + + if (existing.email === email && existing.frequency === frequency && !scheduleChanged) { return existing } + const hr = hour ?? existing.hour + const mn = minute ?? existing.minute + const dow = dayOfWeek ?? existing.day_of_week + const tz = timezone ?? existing.timezone + // Update by id, not pubkey, so tombstoned rows for the same account are never // re-activated alongside this one (the unique index would reject that anyway). if (existing.email === email) { return parseSubscription( await get( - `UPDATE subscriptions SET frequency = ?, unsubscribed_at = NULL + `UPDATE subscriptions SET frequency = ?, hour = ?, minute = ?, day_of_week = ?, timezone = ?, unsubscribed_at = NULL WHERE id = ? RETURNING *`, - [frequency, existing.id] + [frequency, hr, mn, dow, tz, existing.id] ) ) } return parseSubscription( await get( - `UPDATE subscriptions SET email = ?, frequency = ?, confirmed_at = NULL, unsubscribed_at = NULL + `UPDATE subscriptions SET email = ?, frequency = ?, hour = ?, minute = ?, day_of_week = ?, timezone = ?, confirmed_at = NULL, unsubscribed_at = NULL WHERE id = ? RETURNING *`, - [email, frequency, existing.id] + [email, frequency, hr, mn, dow, tz, existing.id] ) ) } @@ -154,25 +183,29 @@ export const updateSubscription = instrument( export const insertSubscription = instrument( 'database.insertSubscription', - async (pubkey: string, email: string, frequency: string) => { + async (pubkey: string, email: string, frequency: string, hour?: number, minute?: number, dayOfWeek?: number, timezone?: string) => { const existing = await getSubscriptionByPubkey(pubkey) if (existing) { - return assertResult(await updateSubscription(existing, email, frequency)) + return assertResult(await updateSubscription(existing, email, frequency, hour, minute, dayOfWeek, timezone)) } try { return assertResult( parseSubscription( await get( - `INSERT INTO subscriptions (id, key, pubkey, email, frequency, created_at) - VALUES (?, ?, ?, ?, ?, ?) RETURNING *`, + `INSERT INTO subscriptions (id, key, pubkey, email, frequency, hour, minute, day_of_week, timezone, created_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?) RETURNING *`, [ crypto.randomUUID(), crypto.randomBytes(32).toString('hex'), pubkey, email, frequency, + hour ?? 17, + minute ?? 0, + dayOfWeek ?? null, + timezone ?? 'UTC', now(), ] ) @@ -186,7 +219,7 @@ export const insertSubscription = instrument( const concurrent = await getSubscriptionByPubkey(pubkey) if (concurrent) { - return assertResult(await updateSubscription(concurrent, email, frequency)) + return assertResult(await updateSubscription(concurrent, email, frequency, hour, minute, dayOfWeek, timezone)) } } diff --git a/src/server.ts b/src/server.ts index 94749b3..674d305 100644 --- a/src/server.ts +++ b/src/server.ts @@ -186,6 +186,10 @@ addRoute('get', '/subscription/email', async (req: Request, res: Response) => { callback, email: sub.email, frequency: sub.frequency, + hour: sub.hour, + minute: sub.minute, + dayOfWeek: sub.day_of_week, + timezone: sub.timezone, confirmed: Boolean(sub.confirmed_at), }) }) @@ -194,7 +198,7 @@ addRoute('get', '/subscription/email', async (req: Request, res: Response) => { // auth proving the caller controls the pubkey — the pubkey is extracted from // the auth event, not from the request body. addRoute('put', '/subscription/email', async (req: Request, res: Response) => { - const { email, frequency } = req.body + const { email, frequency, hour, minute, dayOfWeek, timezone } = req.body const pubkey = await verifyNip98Auth(req) @@ -210,8 +214,38 @@ addRoute('put', '/subscription/email', async (req: Request, res: Response) => { return res.status(400).json({ error: 'Frequency must be "daily" or "weekly"' }) } + // Validate optional schedule fields + if (hour !== undefined) { + if (!Number.isInteger(hour) || hour < 0 || hour > 23) { + return res.status(400).json({ error: 'Hour must be an integer between 0 and 23' }) + } + } + + if (minute !== undefined) { + if (!Number.isInteger(minute) || minute < 0 || minute > 59) { + return res.status(400).json({ error: 'Minute must be an integer between 0 and 59' }) + } + } + + if (dayOfWeek !== undefined) { + if (frequency !== 'weekly') { + return res.status(400).json({ error: 'dayOfWeek is only valid for weekly frequency' }) + } + if (!Number.isInteger(dayOfWeek) || dayOfWeek < 0 || dayOfWeek > 7) { + return res.status(400).json({ error: 'dayOfWeek must be an integer between 0 and 7 (1=Mon, 7=Sun, 0=Sun)' }) + } + } + + if (timezone !== undefined) { + try { + Intl.DateTimeFormat(undefined, { timeZone: timezone }) + } catch { + return res.status(400).json({ error: `"${timezone}" is not a valid IANA timezone` }) + } + } + try { - const result = await registerSubscription({ pubkey, email, frequency }) + const result = await registerSubscription({ pubkey, email, frequency, hour, minute, dayOfWeek, timezone }) res.json(result) } catch (error: any) { // The subscription was still created, but sending the confirmation email diff --git a/src/worker/email.ts b/src/worker/email.ts index 06a546e..a970fd6 100644 --- a/src/worker/email.ts +++ b/src/worker/email.ts @@ -49,7 +49,7 @@ export const runJob = async (sub: Subscription) => { } const createJob = (sub: Subscription) => { - const cron = getCronExpression(sub.frequency) + const cron = getCronExpression(sub.frequency, sub.hour, sub.minute, sub.day_of_week) const run = async () => { // Re-fetch the subscription to pick up the latest last_digest_at. @@ -64,7 +64,7 @@ const createJob = (sub: Subscription) => { cronTime: cron, onTick: run, start: true, - timeZone: 'UTC', + timeZone: sub.timezone ?? 'UTC', }) } diff --git a/test/schedule-fields.test.ts b/test/schedule-fields.test.ts new file mode 100644 index 0000000..9609707 --- /dev/null +++ b/test/schedule-fields.test.ts @@ -0,0 +1,97 @@ +import { describe, it, expect, beforeAll, afterAll } from 'vitest' +import * as db from '../src/database.js' +import { registerSubscription } from '../src/actions.js' +import { getCronExpression } from '../src/alert.js' +import { getJobCronSource, removeJob } from '../src/worker/email.js' +import { registerSubscription as regSub } from '../src/worker/index.js' + +const pubkey = 'schedule-test-' + Date.now() +const email = 'schedule-test-' + Date.now() + '@example.com' +let sub: any = null + +describe('Schedule fields', () => { + beforeAll(async () => { + await db.migrate() + }) + + it('inserts subscription with custom hour/minute/timezone', async () => { + const s = await db.insertSubscription(pubkey, email, 'daily', 7, 30, undefined, 'America/New_York') + expect(s).toBeTruthy() + expect(s!.hour).toBe(7) + expect(s!.minute).toBe(30) + expect(s!.timezone).toBe('America/New_York') + expect(s!.day_of_week).toBeNull() + sub = s + }) + + it('inserts subscription with custom weekly dayOfWeek', async () => { + const pk2 = 'schedule-test-weekly-' + Date.now() + const em2 = pk2 + '@example.com' + const s = await db.insertSubscription(pk2, em2, 'weekly', 9, 0, 6, 'UTC') + expect(s).toBeTruthy() + expect(s!.hour).toBe(9) + expect(s!.minute).toBe(0) + expect(s!.day_of_week).toBe(6) + expect(s!.timezone).toBe('UTC') + }) + + it('inserts subscription with defaults when schedule omitted', async () => { + const pk3 = 'schedule-test-defaults-' + Date.now() + const em3 = pk3 + '@example.com' + const s = await db.insertSubscription(pk3, em3, 'daily') + expect(s).toBeTruthy() + expect(s!.hour).toBe(17) + expect(s!.minute).toBe(0) + expect(s!.timezone).toBe('UTC') + expect(s!.day_of_week).toBeNull() + }) + + it('getCronExpression returns daily with custom hour/minute', () => { + expect(getCronExpression('daily', 7, 30)).toBe('0 30 7 * * *') + }) + + it('getCronExpression returns weekly with custom dayOfWeek', () => { + expect(getCronExpression('weekly', 9, 0, 6)).toBe('0 0 9 * * 6') + }) + + it('getCronExpression defaults to Monday for weekly', () => { + expect(getCronExpression('weekly', 17, 0)).toBe('0 0 17 * * 1') + }) + + it('confirms and registers job with custom schedule', async () => { + // Confirm the daily subscription created above + const confirmed = await db.confirmSubscription(sub.key) + expect(confirmed).toBeTruthy() + sub = confirmed!.sub + + regSub(sub) + const dailySource = getJobCronSource(sub.id) + // Expect 0 30 7 * * * (from custom hour=7, minute=30) + expect(dailySource).toBe('0 30 7 * * *') + }) + + it('updateSubscription preserves schedule fields through frequency change', async () => { + const updated = await db.updateSubscription(sub, email, 'weekly', undefined, undefined, 2, undefined) + expect(updated).toBeTruthy() + expect(updated!.frequency).toBe('weekly') + // hour/minute should keep the previously set values (7/30) since we passed undefined + expect(updated!.hour).toBe(7) + expect(updated!.minute).toBe(30) + expect(updated!.day_of_week).toBe(2) + expect(updated!.timezone).toBe('America/New_York') + }) + + it('registerSubscription preserves schedule fields', async () => { + await registerSubscription({ pubkey, email, frequency: 'daily', hour: 10, minute: 15, timezone: 'Europe/London' }) + const reloaded = await db.getSubscriptionByPubkey(pubkey) + expect(reloaded).toBeTruthy() + expect(reloaded!.hour).toBe(10) + expect(reloaded!.minute).toBe(15) + expect(reloaded!.timezone).toBe('Europe/London') + }) +}) + +afterAll(async () => { + const updated = await db.getSubscriptionByPubkey(pubkey) + if (updated) removeJob(updated) +}) \ No newline at end of file -- 2.45.2