feat: allow users to choose digest schedule (hour, minute, dayOfWeek, timezone)
All checks were successful
CI / checks (pull_request) Successful in 44s
All checks were successful
CI / checks (pull_request) Successful in 44s
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
This commit is contained in:
parent
27bb4a5342
commit
e34f1cd821
7 changed files with 239 additions and 27 deletions
23
README.md
23
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 <base64> 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 <base64> Authorization header)
|
||||
Response: { key, callback, email, frequency, confirmed }
|
||||
Response: { key, callback, email, frequency, hour, minute, dayOfWeek, timezone, confirmed }
|
||||
```
|
||||
|
||||
### DELETE /subscription/:key
|
||||
|
|
|
|||
|
|
@ -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) {
|
||||
|
|
|
|||
39
src/alert.ts
39
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}`
|
||||
}
|
||||
|
|
@ -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<string, any>
|
||||
|
||||
|
|
@ -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))
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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',
|
||||
})
|
||||
}
|
||||
|
||||
|
|
|
|||
97
test/schedule-fields.test.ts
Normal file
97
test/schedule-fields.test.ts
Normal file
|
|
@ -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)
|
||||
})
|
||||
Loading…
Reference in a new issue