Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 39 additions & 24 deletions src/api/conversations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* §3a) and `GET /api/v1/conversations/{id}` (one conversation with its
* threads, §3b) — plus the two HT-18 write paths — `POST
* /api/v1/conversations/{id}/replies` (the Agent replies, §4a) and `PATCH
* /api/v1/conversations/{id}` (close/reopen, §4b).
* /api/v1/conversations/{id}` (set status, §4b).
*
* Each handler is a pure function of an already-authenticated, already-
* routed `Request` plus its dependencies — `src/api/index.ts` is what
Expand All @@ -18,7 +18,12 @@
import type { Keyring } from '../mail/reply-token.js'
import { sendReply } from '../mail/send.js'
import type { EmailSender } from '../providers/index.js'
import type { ConversationStore, StoredThread } from '../store/conversations.js'
import type {
ConversationFolder,
ConversationStatus,
ConversationStore,
StoredThread,
} from '../store/conversations.js'
import { decodeCursor, encodeCursor } from './cursor.js'
import { apiError, json } from './responses.js'
import { isUuid } from './uuid.js'
Expand Down Expand Up @@ -60,7 +65,7 @@ interface ConversationSummaryJson {
id: string
subject: string
customerEmail: string
status: 'open' | 'closed'
status: ConversationStatus
threadCount: number
createdAt: string
updatedAt: string
Expand Down Expand Up @@ -94,14 +99,17 @@ export async function handleListConversations(
): Promise<Response> {
const url = new URL(request.url)

// `status` here is a FOLDER, not a raw status (spec §3a, v1.1): `open` =
// active + pending; `closed`/`spam` = exactly that status. `active` and
// `pending` are deliberately NOT accepted — folders are the reading grain.
const statusParam = url.searchParams.get('status')
let status: 'open' | 'closed' | undefined
let folder: ConversationFolder
if (statusParam === null) {
status = 'open'
} else if (statusParam === 'open' || statusParam === 'closed') {
status = statusParam
folder = 'open'
} else if (statusParam === 'open' || statusParam === 'closed' || statusParam === 'spam') {
folder = statusParam
} else {
return apiError(400, 'validation_failed', "status must be 'open' or 'closed'.")
return apiError(400, 'validation_failed', "status must be 'open', 'closed' or 'spam'.")
}

const limitParam = url.searchParams.get('limit')
Expand All @@ -128,7 +136,7 @@ export async function handleListConversations(
// Fetch one extra row: if it comes back, there IS a next page, and its
// presence is detected by count alone — its own data is discarded (spec
// §3a's over-fetch-by-one pagination-detection trick).
const rows = await deps.store.listConversations({ status, limit: limit + 1, cursor })
const rows = await deps.store.listConversations({ folder, limit: limit + 1, cursor })
const hasNextPage = rows.length > limit
const page = rows.slice(0, limit)

Expand Down Expand Up @@ -176,7 +184,7 @@ export async function handleGetConversation(
// conversation. The `=== 'deleted'` arm is defense-in-depth — it can't fire
// at runtime today, but it guarantees the API never serves a deleted
// conversation even if the store's contract later regressed, and it narrows
// `status` to the `'open' | 'closed'` the response body commits to.
// `status` to the `ConversationStatus` the response body commits to.
if (conversation === null || conversation.status === 'deleted') {
return apiError(404, 'not_found', 'No conversation with that id.')
}
Expand Down Expand Up @@ -220,8 +228,8 @@ export async function handleGetConversation(
* touching the sender again.
*
* Outcomes (spec §4a): `201` with the created `ThreadView` on success (a
* reply to a `closed` conversation reopens it, via `sendReply` →
* `ConversationStore.appendThread`'s existing policy); `404 not_found` if
* reply to a `closed` or `spam` conversation reopens it to `active`, via
* `sendReply` → `ConversationStore.appendThread`'s existing policy); `404 not_found` if
* the conversation is missing or `deleted` (checked BEFORE minting/sending,
* and again as a race check on `sendReply`'s own result — see below);
* `400 validation_failed` on a missing `Idempotency-Key` header or a body
Expand Down Expand Up @@ -355,14 +363,15 @@ export async function handleReply(
}

/**
* Handle `PATCH /api/v1/conversations/{id}` — close or reopen a conversation
* (spec §4b). Body: `{ status: 'open' | 'closed' }` — `'deleted'` is
* deliberately not a settable value here (`400`, not `404`, since the body
* itself is malformed regardless of whether `{id}` exists).
* Handle `PATCH /api/v1/conversations/{id}` — set a conversation's status
* (spec §4b, v1.1). Body: `{ status: 'active' | 'pending' | 'closed' |
* 'spam' }` — `'deleted'` is deliberately not a settable value here (`400`,
* not `404`, since the body itself is malformed regardless of whether
* `{id}` exists).
*
* Outcomes: `200` with the updated `ConversationSummary` on success; `404
* not_found` if `{id}` is missing or names a `deleted` conversation (a
* deleted conversation is not reopenable through this endpoint — spec §4b);
* deleted conversation is not reachable through this endpoint — spec §4b);
* `400 validation_failed` on any other `status` value.
*/
export async function handlePatchConversation(
Expand All @@ -381,7 +390,11 @@ export async function handlePatchConversation(

const status = parsePatchStatusBody(parsedBody.value)
if (status === null) {
return apiError(400, 'validation_failed', "status must be 'open' or 'closed'.")
return apiError(
400,
'validation_failed',
"status must be 'active', 'pending', 'closed' or 'spam'.",
)
}

const updated = await deps.store.setConversationStatus(id, status)
Expand Down Expand Up @@ -437,14 +450,16 @@ function parseReplyBody(raw: unknown): ReplyRequestBody | null {
}

/**
* Validate a parsed PATCH body against spec §4b: `status` must be exactly
* `'open'` or `'closed'` — notably `'deleted'` is NOT settable here. Returns
* `null` on any violation — never throws.
* Validate a parsed PATCH body against spec §4b (v1.1): `status` must be one
* of the four {@link ConversationStatus} values — notably `'deleted'` is NOT
* settable here. Returns `null` on any violation — never throws.
*/
function parsePatchStatusBody(raw: unknown): 'open' | 'closed' | null {
function parsePatchStatusBody(raw: unknown): ConversationStatus | null {
if (typeof raw !== 'object' || raw === null) return null
const { status } = raw as Record<string, unknown>
return status === 'open' || status === 'closed' ? status : null
return status === 'active' || status === 'pending' || status === 'closed' || status === 'spam'
? status
: null
}

/**
Expand Down Expand Up @@ -492,7 +507,7 @@ function toConversationSummaryJson(row: {
id: string
subject: string
customerEmail: string
status: 'open' | 'closed'
status: ConversationStatus
threadCount: number
createdAt: Date
updatedAt: Date
Expand Down
84 changes: 68 additions & 16 deletions src/api/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,11 @@ function newConversation(overrides: Partial<NewConversation> = {}): NewConversat
}
}

async function setStatus(db: Db, conversationId: string, status: 'open' | 'closed' | 'deleted') {
async function setStatus(
db: Db,
conversationId: string,
status: 'active' | 'pending' | 'closed' | 'spam' | 'deleted',
) {
await db.query('UPDATE conversations SET status = $1 WHERE id = $2', [status, conversationId])
}

Expand Down Expand Up @@ -251,19 +255,39 @@ describe('createInboxApi', () => {
expect(body.conversations.map((c) => c.id)).toEqual([a, b])
})

it('filters by status: open vs closed', async () => {
it('filters by folder: open is active + pending; closed and spam are exact (spec §3a, v1.1)', async () => {
const { db, store, api } = await freshApi()
const { conversationId: openId } = await store.createConversation(newConversation())
const { conversationId: activeId } = await store.createConversation(newConversation())
const { conversationId: pendingId } = await store.createConversation(newConversation())
const { conversationId: closedId } = await store.createConversation(newConversation())
const { conversationId: spamId } = await store.createConversation(newConversation())
await setStatus(db, pendingId, 'pending')
await setStatus(db, closedId, 'closed')
await setStatus(db, spamId, 'spam')

const openRes = await api(get('/api/v1/conversations?status=open'))
const openBody = (await openRes.json()) as { conversations: Array<{ id: string }> }
expect(openBody.conversations.map((c) => c.id)).toEqual([openId])
const openBody = (await openRes.json()) as {
conversations: Array<{ id: string; status: string }>
}
expect(openBody.conversations.map((c) => c.id).sort()).toEqual([activeId, pendingId].sort())
// The wire summary carries the REAL status — the query param is the folder.
expect(openBody.conversations.find((c) => c.id === pendingId)?.status).toBe('pending')

const closedRes = await api(get('/api/v1/conversations?status=closed'))
const closedBody = (await closedRes.json()) as { conversations: Array<{ id: string }> }
expect(closedBody.conversations.map((c) => c.id)).toEqual([closedId])

const spamRes = await api(get('/api/v1/conversations?status=spam'))
const spamBody = (await spamRes.json()) as { conversations: Array<{ id: string }> }
expect(spamBody.conversations.map((c) => c.id)).toEqual([spamId])
})

it("rejects raw statuses as filter values — 'active' and 'pending' are not folders", async () => {
const { api } = await freshApi()
for (const value of ['active', 'pending']) {
const res = await api(get(`/api/v1/conversations?status=${value}`))
expect(res.status).toBe(400)
}
})

it('rejects an invalid status value with 400', async () => {
Expand Down Expand Up @@ -394,7 +418,7 @@ describe('createInboxApi', () => {
expect(body.id).toBe(conversationId)
expect(body.subject).toBe('Help with my order')
expect(body.customerEmail).toBe('customer@example.test')
expect(body.status).toBe('open')
expect(body.status).toBe('active')
expect(body.threadCount).toBe(2)
expect(body.threads).toHaveLength(2)
expect(body.threads[0]).toMatchObject({
Expand Down Expand Up @@ -519,7 +543,7 @@ describe('createInboxApi', () => {
expect(sent[0].subject).toBe('Re: Already replied')
})

it('a reply reopens a closed conversation', async () => {
it('a reply reopens a closed conversation to active', async () => {
const { db, store, api } = await freshApi()
const { conversationId } = await store.createConversation(newConversation())
await setStatus(db, conversationId, 'closed')
Expand All @@ -530,7 +554,23 @@ describe('createInboxApi', () => {
expect(res.status).toBe(201)

const updated = await store.getConversation(conversationId, { includeDeleted: false })
expect(updated?.status).toBe('open')
expect(updated?.status).toBe('active')
})

it('a reply reopens a spam conversation to active (spec §4a, v1.1)', async () => {
const { db, store, api } = await freshApi()
const { conversationId } = await store.createConversation(newConversation())
await setStatus(db, conversationId, 'spam')

const res = await api(
replyPost(`/api/v1/conversations/${conversationId}/replies`, {
text: 'Not spam after all.',
}),
)
expect(res.status).toBe(201)

const updated = await store.getConversation(conversationId, { includeDeleted: false })
expect(updated?.status).toBe('active')
})

it('404s for a missing conversation id; the sender is never called', async () => {
Expand Down Expand Up @@ -839,7 +879,7 @@ describe('createInboxApi', () => {
// --- patch (status) -----------------------------------------------------------

describe('patch status', () => {
it('closes an open conversation: 200 with the updated summary', async () => {
it('closes an active conversation: 200 with the updated summary', async () => {
const { store, api } = await freshApi()
const { conversationId } = await store.createConversation(newConversation())

Expand All @@ -851,20 +891,32 @@ describe('createInboxApi', () => {
expect(body.status).toBe('closed')
})

it('reopens a closed conversation: 200 with the updated summary', async () => {
it('reopens a closed conversation to active: 200 with the updated summary', async () => {
const { db, store, api } = await freshApi()
const { conversationId } = await store.createConversation(newConversation())
await setStatus(db, conversationId, 'closed')

const res = await api(patch(`/api/v1/conversations/${conversationId}`, { status: 'open' }))
const res = await api(patch(`/api/v1/conversations/${conversationId}`, { status: 'active' }))
expect(res.status).toBe(200)
const body = (await res.json()) as { status: string }
expect(body.status).toBe('open')
expect(body.status).toBe('active')
})

it('sets pending and spam: every surfaceable status is settable (spec §4b, v1.1)', async () => {
const { store, api } = await freshApi()
const { conversationId } = await store.createConversation(newConversation())

for (const status of ['pending', 'spam'] as const) {
const res = await api(patch(`/api/v1/conversations/${conversationId}`, { status }))
expect(res.status).toBe(200)
const body = (await res.json()) as { status: string }
expect(body.status).toBe(status)
}
})

it('404s for a missing conversation id', async () => {
const { api } = await freshApi()
const res = await api(patch(`/api/v1/conversations/${RANDOM_UUID}`, { status: 'open' }))
const res = await api(patch(`/api/v1/conversations/${RANDOM_UUID}`, { status: 'active' }))
expect(res.status).toBe(404)
expect(await res.json()).toEqual({
error: { code: 'not_found', message: expect.any(String) },
Expand All @@ -873,16 +925,16 @@ describe('createInboxApi', () => {

it('404s for a non-UUID-shaped id — never reaches the uuid column', async () => {
const { api } = await freshApi()
const res = await api(patch('/api/v1/conversations/not-a-uuid', { status: 'open' }))
const res = await api(patch('/api/v1/conversations/not-a-uuid', { status: 'active' }))
expect(res.status).toBe(404)
})

it('404s for a deleted conversation — not reopenable through this endpoint', async () => {
it('404s for a deleted conversation — not reachable through this endpoint', async () => {
const { db, store, api } = await freshApi()
const { conversationId } = await store.createConversation(newConversation())
await setStatus(db, conversationId, 'deleted')

const res = await api(patch(`/api/v1/conversations/${conversationId}`, { status: 'open' }))
const res = await api(patch(`/api/v1/conversations/${conversationId}`, { status: 'active' }))
expect(res.status).toBe(404)
})

Expand Down
Loading
Loading