9fa1f73316
All checks were successful
CI / checks (pull_request) Successful in 36s
Problem
-------
1. Re-clicking an already-confirmed confirmation link (e.g. /confirm?token=…)
returned from confirmSubscription because the SQL WHERE
clause required . The caller then threw an
ActionError('invalid or expired') which rendered the 'Email not confirmed'
error page — misleading for someone who had already confirmed.
2. A second PUT /subscription/email with the same email+frequency could
silently bypass the upsert path when getSubscriptionByPubkey found the
active row but updateSubscription returned it unchanged (email and
frequency matched). While the unique index prevented a true duplicate
INSERT, the code path was fragile and the regression test was missing.
Changes
-------
database.ts:
- confirmSubscription now returns { sub, alreadyConfirmed } | undefined.
First it tries the existing UPDATE (unconfirmed tokens only). If that
returns no rows, it looks up the key directly: if the row exists and is
already confirmed, returns { sub, alreadyConfirmed: true }. If the row
doesn't exist or is unsubscribed, returns undefined (invalid/expired).
- Exported new ConfirmResult type for callers.
actions.ts:
- confirmSubscriptionAction destructures the new return type.
- Only registers the cron job on fresh confirmation (not re-confirms).
- Returns the ConfirmResult so the route can distinguish the two cases.
server.ts:
- /confirm route checks result.alreadyConfirmed and renders
confirm-already.html instead of confirm-success.html.
pages/confirm-already.html:
- New page with title 'Email already confirmed' and an info message
explaining the address was already confirmed.
Tests:
- test/confirm-already-confirmed.test.ts — NEW (3 tests): first confirm
succeeds with alreadyConfirmed=false; second confirm returns
alreadyConfirmed=true; nonexistent token returns undefined.
- test/duplicate-subscription.test.ts — NEW (4 tests): full cycle of
register → confirm → re-register → assert one active row with
unchanged key, verifying the upsert is idempotent.
- Adapted 3 existing test files to destructure the new ConfirmResult.
78 lines
No EOL
2.5 KiB
TypeScript
78 lines
No EOL
2.5 KiB
TypeScript
// POST /notify/:id response shape test
|
|
//
|
|
// Verifies that the endpoint always includes a `stored` boolean
|
|
// in its response, matching the documented contract in README.md:
|
|
// Response: { ok: true, stored: boolean }
|
|
//
|
|
// Bug: when the event is not found at the relay, the handler returns
|
|
// { ok: true, skipped: true }
|
|
// missing the documented `stored` field.
|
|
|
|
import { describe, it, expect, beforeAll, afterAll } from 'vitest'
|
|
import * as db from '../src/database.js'
|
|
import { server } from '../src/server.js'
|
|
import { createServer, type Server } from 'http'
|
|
|
|
// Partially mock @welshman/net so that `load()` returns an empty array,
|
|
// simulating the case where the relay does not have the requested event,
|
|
// while preserving all other exports that other modules depend on.
|
|
vi.mock('@welshman/net', async (importOriginal) => {
|
|
const actual = await importOriginal()
|
|
return {
|
|
...(actual as Record<string, unknown>),
|
|
load: vi.fn().mockResolvedValue([]),
|
|
}
|
|
})
|
|
|
|
describe('notify_response_shape', () => {
|
|
let httpServer: Server
|
|
let baseUrl: string
|
|
let subId: string
|
|
|
|
beforeAll(async () => {
|
|
await db.migrate()
|
|
|
|
// Create and confirm a subscription we can use for the notify call
|
|
const pubkey = 'shape-test-pk-' + Date.now()
|
|
const email = 'shape-test-' + Date.now() + '@example.com'
|
|
const sub = await db.insertSubscription(pubkey, email, 'daily')
|
|
const confirmed = await db.confirmSubscription(sub.key)
|
|
subId = confirmed!.sub.id
|
|
|
|
// Start the express server on a random available port
|
|
await new Promise<void>((resolve) => {
|
|
httpServer = createServer(server)
|
|
httpServer.listen(0, () => {
|
|
const addr = httpServer.address()
|
|
if (addr && typeof addr === 'object') {
|
|
baseUrl = `http://localhost:${addr.port}`
|
|
}
|
|
resolve()
|
|
})
|
|
})
|
|
})
|
|
|
|
afterAll(async () => {
|
|
httpServer?.close()
|
|
})
|
|
|
|
it('returns stored=false instead of skipped=true when event not found', async () => {
|
|
const res = await fetch(`${baseUrl}/notify/${subId}`, {
|
|
method: 'POST',
|
|
headers: { 'Content-Type': 'application/json' },
|
|
body: JSON.stringify({
|
|
id: 'nonexistent-' + Date.now(),
|
|
relay: 'wss://relay.damus.io',
|
|
}),
|
|
})
|
|
|
|
const body = await res.json()
|
|
|
|
// The documented contract says: { ok: true, stored: boolean }
|
|
// The current buggy code returns: { ok: true, skipped: true }
|
|
expect(body).not.toHaveProperty('skipped')
|
|
expect(body).toHaveProperty('stored')
|
|
expect(body.stored).toBe(false)
|
|
expect(body.ok).toBe(true)
|
|
})
|
|
}) |