fix: reschedule cron job when confirmed subscriber changes frequency

When a confirmed subscriber calls PUT /subscription/email with a changed
frequency, db.updateSubscription updates the row but the running CronJob
captured the original frequency in createJob and was never rescheduled.
A subscriber switching daily→weekly kept the daily cadence until restart.

Fix: in actions.ts:registerSubscription, call worker.registerSubscription(sub)
when the subscription is already confirmed, so addJob stops the old job and
creates a new one with the updated frequency.

Changes:
- src/actions.ts: add else branch calling worker.registerSubscription when
  sub.confirmed_at is set
- test/reschedule-on-frequency-change.test.js: new failing-before/passing-after
  test verifying the cron expression is updated after frequency change

Closes mailship-e04
This commit is contained in:
Agent 2026-09-10 11:28:53 -04:00
parent d5f54845b6
commit 3e667fe3c6
3 changed files with 94 additions and 3 deletions

View file

@ -23,11 +23,13 @@ export const registerSubscription = instrument(
const sub = await db.insertSubscription(pubkey, email, frequency) const sub = await db.insertSubscription(pubkey, email, frequency)
const callback = `${process.env.BASE_URL}/notify/${sub.id}` const callback = `${process.env.BASE_URL}/notify/${sub.id}`
// Only send a confirmation when the subscription is new, unconfirmed, or
// its email address changed. An already-confirmed, unchanged subscription
// (or one where only the frequency changed) skips it.
if (!sub.confirmed_at) { if (!sub.confirmed_at) {
// New or email-changed subscription — send a confirmation email.
await mailer.sendConfirm(sub) await mailer.sendConfirm(sub)
} else {
// Already confirmed (e.g. frequency-only change) — reschedule the
// cron job so it uses the new cadence immediately.
worker.registerSubscription(sub)
} }
return { key: sub.key, callback } return { key: sub.key, callback }

View file

@ -6,6 +6,12 @@ import * as db from '../database.js'
const jobsById = new Map<string, CronJob>() const jobsById = new Map<string, CronJob>()
// Test-only accessor to inspect stored jobs
export const getJobCronSource = (id: string): string | undefined => {
const source = jobsById.get(id)?.cronTime.source
return typeof source === 'string' ? source : undefined
}
export const runJob = async (sub: Subscription) => { export const runJob = async (sub: Subscription) => {
try { try {
if (!sub.confirmed_at || sub.unsubscribed_at) { if (!sub.confirmed_at || sub.unsubscribed_at) {

View file

@ -0,0 +1,83 @@
#!/usr/bin/env node
// FAILING test: frequency change does not reschedule the running cron job.
//
// The fix: actions.ts:registerSubscription calls worker.registerSubscription()
// after a DB update on an already-confirmed subscription, so addJob creates
// a new CronJob with the updated frequency on the fly.
//
// Step 1-2: Create subscription via raw DB (bypass mailer for simplicity)
// Step 3: Register cron job with daily (simulating confirmSubscriptionAction)
// Step 4: Call registerSubscription with new frequency — the bug path
// BEFORE FIX: cron stays daily; AFTER FIX: cron becomes weekly
import * as db from '../dist/database.js'
import { registerSubscription } from '../dist/actions.js'
import { getJobCronSource, removeJob } from '../dist/worker/email.js'
import { registerSubscription as regSub } from '../dist/worker/index.js'
let passed = 0
let failed = 0
function assert(label, ok, detail) {
if (ok) {
console.log(` ? ${label}`)
passed++
} else {
console.log(` ? ${label} -- ${detail || ''}`)
failed++
}
}
async function main() {
await db.migrate()
const pubkey = 'freq-test-' + Date.now()
const email = 'freq-test-' + Date.now() + '@example.com'
// Step 1: Insert subscription directly (bypass mailer) and confirm
console.log('1. Create confirmed subscription with daily frequency')
const sub = await db.insertSubscription(pubkey, email, 'daily')
assert('subscription created', !!sub, 'insert returned null')
if (!sub) { process.exit(1) }
const confirmed = await db.confirmSubscription(sub.key)
assert('subscription confirmed', !!confirmed, 'confirm returned null')
if (!confirmed) { process.exit(1) }
// Step 2: Register cron job with daily (simulating confirmSubscriptionAction)
console.log('\n2. Register cron job with daily frequency')
regSub(confirmed)
const dailySource = getJobCronSource(confirmed.id)
assert(
'cron source is daily',
dailySource === '0 0 17 * * *',
`expected 0 0 17 * * *, got ${dailySource}`
)
// Step 3: Register subscription again with weekly — the bug path.
// BEFORE FIX: registerSubscription skips worker call because
// sub.confirmed_at is set → cron stays daily
// AFTER FIX: registerSubscription calls worker.registerSubscription
// → addJob reschedules → cron becomes weekly
console.log('\n3. Change frequency to weekly via registerSubscription')
await registerSubscription({ pubkey, email, frequency: 'weekly' })
const weeklySource = getJobCronSource(confirmed.id)
assert(
'cron source is weekly after frequency change',
weeklySource === '0 0 17 * * 1',
`expected 0 0 17 * * 1, got ${weeklySource}`
)
// Cleanup
const updated = await db.getSubscriptionByPubkey(pubkey)
if (updated) removeJob(updated)
console.log('')
console.log(`Results: ${passed} passed, ${failed} failed`)
process.exit(failed > 0 ? 1 : 0)
}
main().catch(err => {
console.error('Unhandled error in test:', err)
process.exit(1)
})