Merge pull request 'Reschedule cron job when confirmed subscriber changes frequency' (#2) from mailship-e04-frequency-change-updates-db-but-never-re-63d into main

Reviewed-on: #2
Reviewed-by: matt <matt@lorentz.is>

src/actions.ts

@@ -21,11 +21,13 @@ export const registerSubscription = instrument(
     const sub = await db.insertSubscription(pubkey, email, frequency)
     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) {
+      // New or email-changed subscription — send a confirmation email.
       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 }

src/worker/email.ts

@@ -6,6 +6,12 @@ import * as db from '../database.js'
 
 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) => {
   try {
     if (!sub.confirmed_at || sub.unsubscribed_at) {

test/reschedule-on-frequency-change.test.js

@@ -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)
+})