From a7d4ed6a60dd408b6abbc72693945e5ff6b0b6d8 Mon Sep 17 00:00:00 2001 From: Moshfegh Hamedani Date: Thu, 12 Mar 2026 09:56:03 -0700 Subject: [PATCH] Fix broken tests --- CLAUDE.md | 9 +++ client/src/pages/TicketsPage.test.tsx | 6 +- e2e/fixtures/auth.ts | 4 +- e2e/tests/auth.spec.ts | 25 ++++--- e2e/tests/ticket-detail.spec.ts | 25 +++++-- e2e/tests/tickets.spec.ts | 19 ++++- e2e/tests/webhook-inbound-email.spec.ts | 92 ++++++++++++++----------- server/src/routes/webhooks.ts | 2 +- 8 files changed, 119 insertions(+), 63 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 083ddd0..f0e2866 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -65,6 +65,15 @@ The client proxies `/api/*` requests to the server via Vite config (target is co - To add a new background job: create a queue with `boss.createQueue()`, register a worker with `boss.work()` in `startQueue()`, and export a `send*Job()` function - **Existing queues**: - `classify-ticket` — classifies inbound tickets via GPT (retryLimit: 3, retryDelay: 30s, exponential backoff) + - `auto-resolve-ticket` — attempts to auto-resolve tickets via GPT; if unsuccessful, transitions status to `open` + +## Ticket Lifecycle + +- Inbound emails arrive via the `/api/webhooks/inbound-email` endpoint (SendGrid multipart format) and are created with status `new` +- The system enqueues `classify-ticket` and `auto-resolve-ticket` background jobs automatically +- Status flow: `new` → `processing` (AI working) → `open` (if not auto-resolved) or `resolved` (if auto-resolved) +- `new` and `processing` tickets are system-managed and never shown in the agent UI — agents only see `open`, `resolved`, and `closed` tickets +- The `/api/tickets` endpoint excludes `new` and `processing` tickets by default (no `status` filter param) ## Authentication diff --git a/client/src/pages/TicketsPage.test.tsx b/client/src/pages/TicketsPage.test.tsx index cc3b6d0..dc06aeb 100644 --- a/client/src/pages/TicketsPage.test.tsx +++ b/client/src/pages/TicketsPage.test.tsx @@ -107,15 +107,15 @@ describe("TicketsPage", () => { expect(screen.getByText("Closed")).toBeInTheDocument(); }); - it("should display category with underscores replaced by spaces", async () => { + it("should display category using category labels", async () => { mockedAxios.get.mockResolvedValue(mockResponse()); renderWithQuery(); await waitFor(() => { - expect(screen.getByText("technical question")).toBeInTheDocument(); + expect(screen.getByText("Technical")).toBeInTheDocument(); }); - expect(screen.getByText("refund request")).toBeInTheDocument(); + expect(screen.getByText("Refund")).toBeInTheDocument(); }); it("should show dash for null category", async () => { diff --git a/e2e/fixtures/auth.ts b/e2e/fixtures/auth.ts index 48b042e..2d00874 100644 --- a/e2e/fixtures/auth.ts +++ b/e2e/fixtures/auth.ts @@ -50,8 +50,8 @@ export async function logout(page: Page) { */ export async function expectLoginPage(page: Page) { await expect(page).toHaveURL("/login"); - await expect(page.getByText("Helpdesk")).toBeVisible(); - await expect(page.getByText(/sign in to your account/i)).toBeVisible(); + await expect(page.getByText("Welcome back")).toBeVisible(); + await expect(page.getByText(/sign in to your helpdesk account/i)).toBeVisible(); } /** diff --git a/e2e/tests/auth.spec.ts b/e2e/tests/auth.spec.ts index 3f4f376..3df9f43 100644 --- a/e2e/tests/auth.spec.ts +++ b/e2e/tests/auth.spec.ts @@ -16,8 +16,8 @@ test.describe("Authentication", () => { test("should display login form with all elements", async ({ page }) => { // Check page title and description - await expect(page.getByText("Helpdesk")).toBeVisible(); - await expect(page.getByText(/sign in to your account/i)).toBeVisible(); + await expect(page.getByText("Welcome back")).toBeVisible(); + await expect(page.getByText(/sign in to your helpdesk account/i)).toBeVisible(); // Check form fields await expect(page.getByLabel("Email")).toBeVisible(); @@ -129,18 +129,27 @@ test.describe("Authentication", () => { }); test("should show loading state during login", async ({ page }) => { + // Delay the auth response so we can observe the loading state + let resolveDelay!: () => void; + const delay = new Promise((resolve) => { + resolveDelay = resolve; + }); + + await page.route("**/api/auth/sign-in/email", async (route) => { + await delay; + await route.continue(); + }); + await page.getByLabel("Email").fill(TEST_USERS.admin.email); await page.getByLabel("Password").fill(TEST_USERS.admin.password); - - // Start login - const loginPromise = page - .getByRole("button", { name: /sign in/i }) - .click(); + await page.getByRole("button", { name: /sign in/i }).click(); // Check loading state appears (button disabled with loading text) await expect(page.getByText(/signing in.../i)).toBeVisible(); - await loginPromise; + // Allow the request to complete + resolveDelay(); + await page.unrouteAll({ behavior: "ignoreErrors" }); }); test("should redirect to home if already authenticated", async ({ diff --git a/e2e/tests/ticket-detail.spec.ts b/e2e/tests/ticket-detail.spec.ts index 70de20c..e859783 100644 --- a/e2e/tests/ticket-detail.spec.ts +++ b/e2e/tests/ticket-detail.spec.ts @@ -1,4 +1,4 @@ -import { test, expect } from "@playwright/test"; +import { test, expect, type APIRequestContext } from "@playwright/test"; import { loginAsAdmin } from "../fixtures/auth"; import type { InboundEmailInput } from "core/schemas/tickets.ts"; @@ -9,14 +9,22 @@ const API_BASE_URL = process.env.BETTER_AUTH_URL!; * Creates a ticket via the inbound email webhook and returns the ticket object. */ async function createTicketViaWebhook( - request: Parameters[1]>[0]["request"], + request: APIRequestContext, payload: InboundEmailInput ) { + const from = payload.fromName?.trim() + ? `${payload.fromName} <${payload.from}>` + : payload.from; const response = await request.post( `${API_BASE_URL}/api/webhooks/inbound-email`, { headers: { "x-webhook-secret": WEBHOOK_SECRET }, - data: payload, + multipart: { + from, + subject: payload.subject, + text: payload.body, + ...(payload.bodyHtml ? { html: payload.bodyHtml } : {}), + }, } ); expect(response.status()).toBe(201); @@ -76,6 +84,9 @@ test.describe("Ticket Detail Page", () => { ); await loginAsAdmin(page); + await page.request.patch(`${API_BASE_URL}/api/tickets/${ticket.id}`, { + data: { status: "open" }, + }); await page.goto(`/tickets/${ticket.id}`); // Update status @@ -107,10 +118,7 @@ test.describe("Ticket Detail Page", () => { resp.request().method() === "PATCH" && resp.status() === 200 ); - await page - .getByRole("combobox") - .filter({ hasText: "Unassigned" }) - .click(); + await page.getByRole("combobox").nth(2).click(); await page.getByRole("option", { name: /^Admin$/ }).click(); await assignPatch; @@ -170,6 +178,9 @@ test.describe("Ticket Detail Page", () => { const ticket = await createTicketViaWebhook(request, payload); await loginAsAdmin(page); + await page.request.patch(`${API_BASE_URL}/api/tickets/${ticket.id}`, { + data: { status: "open" }, + }); // Navigate from list to detail await page.goto("/tickets"); diff --git a/e2e/tests/tickets.spec.ts b/e2e/tests/tickets.spec.ts index adfffa2..eb403a3 100644 --- a/e2e/tests/tickets.spec.ts +++ b/e2e/tests/tickets.spec.ts @@ -12,13 +12,21 @@ async function createTicketViaWebhook( request: any, payload: Partial & { from: string; fromName: string; subject: string; body: string } ) { + const from = payload.fromName?.trim() + ? `${payload.fromName} <${payload.from}>` + : payload.from; const response = await request.post( `${API_BASE_URL}/api/webhooks/inbound-email`, { headers: { "x-webhook-secret": WEBHOOK_SECRET, }, - data: payload, + multipart: { + from, + subject: payload.subject, + text: payload.body, + ...(payload.bodyHtml ? { html: payload.bodyHtml } : {}), + }, } ); @@ -73,13 +81,17 @@ test.describe("Tickets Page", () => { const ticket = await createTicketViaWebhook(request, payload); await loginAsAdmin(page); + await page.request.patch(`${API_BASE_URL}/api/tickets/${ticket.id}`, { + data: { status: "open" }, + }); + await page.goto("/tickets"); const row = page.getByRole("row").filter({ hasText: ticket.subject }); await expect(row).toBeVisible(); await expect(row.getByText(payload.fromName)).toBeVisible(); await expect(row.getByText(payload.from)).toBeVisible(); - await expect(row.locator("text=open").first()).toBeVisible(); + await expect(row.locator("text=Open").first()).toBeVisible(); }); test("should show newly created ticket after page reload", async ({ page, request }) => { @@ -89,6 +101,9 @@ test.describe("Tickets Page", () => { const uniqueId = `refresh-${Date.now()}`; const payload = createTestPayload(uniqueId); const ticket = await createTicketViaWebhook(request, payload); + await page.request.patch(`${API_BASE_URL}/api/tickets/${ticket.id}`, { + data: { status: "open" }, + }); await page.reload(); diff --git a/e2e/tests/webhook-inbound-email.spec.ts b/e2e/tests/webhook-inbound-email.spec.ts index a175532..1e87724 100644 --- a/e2e/tests/webhook-inbound-email.spec.ts +++ b/e2e/tests/webhook-inbound-email.spec.ts @@ -4,6 +4,22 @@ import type { InboundEmailInput } from "core/schemas/tickets.ts"; const WEBHOOK_SECRET = process.env.WEBHOOK_SECRET!; const API_BASE_URL = process.env.BETTER_AUTH_URL!; +/** + * Converts an InboundEmailInput to multipart form fields matching SendGrid's format + */ +function toMultipart(payload: InboundEmailInput): Record { + const from = payload.fromName?.trim() + ? `${payload.fromName} <${payload.from}>` + : (payload.from ?? ""); + const result: Record = { + from, + subject: payload.subject, + text: payload.body, + }; + if (payload.bodyHtml) result.html = payload.bodyHtml; + return result; +} + /** * Helper to create a valid inbound email payload */ @@ -30,7 +46,7 @@ test.describe("Webhook: Inbound Email", () => { const response = await request.post( `${API_BASE_URL}/api/webhooks/inbound-email`, { - data: payload, + multipart: toMultipart(payload), failOnStatusCode: false, } ); @@ -52,7 +68,7 @@ test.describe("Webhook: Inbound Email", () => { headers: { "x-webhook-secret": "wrong-secret", }, - data: payload, + multipart: toMultipart(payload), failOnStatusCode: false, } ); @@ -71,7 +87,7 @@ test.describe("Webhook: Inbound Email", () => { const response = await request.post( `${API_BASE_URL}/api/webhooks/inbound-email?secret=wrong-secret`, { - data: payload, + multipart: toMultipart(payload), failOnStatusCode: false, } ); @@ -93,7 +109,7 @@ test.describe("Webhook: Inbound Email", () => { headers: { "x-webhook-secret": WEBHOOK_SECRET, }, - data: payload, + multipart: toMultipart(payload), } ); @@ -110,7 +126,7 @@ test.describe("Webhook: Inbound Email", () => { const response = await request.post( `${API_BASE_URL}/api/webhooks/inbound-email?secret=${WEBHOOK_SECRET}`, { - data: payload, + multipart: toMultipart(payload), } ); @@ -132,7 +148,7 @@ test.describe("Webhook: Inbound Email", () => { headers: { "x-webhook-secret": WEBHOOK_SECRET, }, - data: payload, + multipart: toMultipart(payload), failOnStatusCode: false, } ); @@ -156,7 +172,7 @@ test.describe("Webhook: Inbound Email", () => { headers: { "x-webhook-secret": WEBHOOK_SECRET, }, - data: payload, + multipart: toMultipart(payload), failOnStatusCode: false, } ); @@ -164,7 +180,7 @@ test.describe("Webhook: Inbound Email", () => { expect(response.status()).toBe(400); }); - test("should reject request with empty fromName", async ({ request }) => { + test("should use email as sender name when from field has no name", async ({ request }) => { const payload = createValidPayload({ fromName: "" }); const response = await request.post( @@ -173,18 +189,16 @@ test.describe("Webhook: Inbound Email", () => { headers: { "x-webhook-secret": WEBHOOK_SECRET, }, - data: payload, - failOnStatusCode: false, + multipart: toMultipart(payload), } ); - expect(response.status()).toBe(400); + expect(response.status()).toBe(201); const body = await response.json(); - expect(body).toHaveProperty("error"); - expect(body.error).toMatch(/sender name is required/i); + expect(body.ticket.senderName).toBe(payload.from); }); - test("should reject request with whitespace-only fromName", async ({ + test("should use email as sender name when from field has whitespace-only name", async ({ request, }) => { const payload = createValidPayload({ fromName: " " }); @@ -195,15 +209,13 @@ test.describe("Webhook: Inbound Email", () => { headers: { "x-webhook-secret": WEBHOOK_SECRET, }, - data: payload, - failOnStatusCode: false, + multipart: toMultipart(payload), } ); - expect(response.status()).toBe(400); + expect(response.status()).toBe(201); const body = await response.json(); - expect(body).toHaveProperty("error"); - expect(body.error).toMatch(/sender name is required/i); + expect(body.ticket.senderName).toBe(payload.from); }); test("should reject request with empty subject", async ({ request }) => { @@ -215,7 +227,7 @@ test.describe("Webhook: Inbound Email", () => { headers: { "x-webhook-secret": WEBHOOK_SECRET, }, - data: payload, + multipart: toMultipart(payload), failOnStatusCode: false, } ); @@ -237,7 +249,7 @@ test.describe("Webhook: Inbound Email", () => { headers: { "x-webhook-secret": WEBHOOK_SECRET, }, - data: payload, + multipart: toMultipart(payload), failOnStatusCode: false, } ); @@ -257,7 +269,7 @@ test.describe("Webhook: Inbound Email", () => { headers: { "x-webhook-secret": WEBHOOK_SECRET, }, - data: payload, + multipart: toMultipart(payload), failOnStatusCode: false, } ); @@ -280,7 +292,7 @@ test.describe("Webhook: Inbound Email", () => { headers: { "x-webhook-secret": WEBHOOK_SECRET, }, - data: payload, + multipart: toMultipart(payload), } ); @@ -309,7 +321,7 @@ test.describe("Webhook: Inbound Email", () => { headers: { "x-webhook-secret": WEBHOOK_SECRET, }, - data: payload, + multipart: toMultipart(payload), } ); @@ -324,7 +336,7 @@ test.describe("Webhook: Inbound Email", () => { expect(ticket.body).toBe(payload.body); expect(ticket.senderName).toBe(payload.fromName); expect(ticket.senderEmail).toBe(payload.from); - expect(ticket.status).toBe("open"); + expect(ticket.status).toBe("new"); expect(ticket.category).toBeNull(); expect(ticket.bodyHtml).toBeNull(); // Not provided in payload @@ -353,7 +365,7 @@ test.describe("Webhook: Inbound Email", () => { headers: { "x-webhook-secret": WEBHOOK_SECRET, }, - data: payload, + multipart: toMultipart(payload), } ); @@ -387,7 +399,7 @@ test.describe("Webhook: Inbound Email", () => { headers: { "x-webhook-secret": WEBHOOK_SECRET, }, - data: firstPayload, + multipart: toMultipart(firstPayload), } ); @@ -409,7 +421,7 @@ test.describe("Webhook: Inbound Email", () => { headers: { "x-webhook-secret": WEBHOOK_SECRET, }, - data: secondPayload, + multipart: toMultipart(secondPayload), } ); @@ -444,7 +456,7 @@ test.describe("Webhook: Inbound Email", () => { headers: { "x-webhook-secret": WEBHOOK_SECRET, }, - data: firstPayload, + multipart: toMultipart(firstPayload), } ); @@ -466,7 +478,7 @@ test.describe("Webhook: Inbound Email", () => { headers: { "x-webhook-secret": WEBHOOK_SECRET, }, - data: replyPayload, + multipart: toMultipart(replyPayload), } ); @@ -497,7 +509,7 @@ test.describe("Webhook: Inbound Email", () => { headers: { "x-webhook-secret": WEBHOOK_SECRET, }, - data: firstPayload, + multipart: toMultipart(firstPayload), } ); @@ -519,7 +531,7 @@ test.describe("Webhook: Inbound Email", () => { headers: { "x-webhook-secret": WEBHOOK_SECRET, }, - data: forwardPayload, + multipart: toMultipart(forwardPayload), } ); @@ -549,7 +561,7 @@ test.describe("Webhook: Inbound Email", () => { headers: { "x-webhook-secret": WEBHOOK_SECRET, }, - data: firstPayload, + multipart: toMultipart(firstPayload), } ); @@ -569,7 +581,7 @@ test.describe("Webhook: Inbound Email", () => { headers: { "x-webhook-secret": WEBHOOK_SECRET, }, - data: replyPayload, + multipart: toMultipart(replyPayload), } ); @@ -598,7 +610,7 @@ test.describe("Webhook: Inbound Email", () => { headers: { "x-webhook-secret": WEBHOOK_SECRET, }, - data: firstPayload, + multipart: toMultipart(firstPayload), } ); @@ -618,7 +630,7 @@ test.describe("Webhook: Inbound Email", () => { headers: { "x-webhook-secret": WEBHOOK_SECRET, }, - data: lowercasePayload, + multipart: toMultipart(lowercasePayload), } ); @@ -647,7 +659,7 @@ test.describe("Webhook: Inbound Email", () => { headers: { "x-webhook-secret": WEBHOOK_SECRET, }, - data: firstPayload, + multipart: toMultipart(firstPayload), } ); @@ -668,7 +680,7 @@ test.describe("Webhook: Inbound Email", () => { headers: { "x-webhook-secret": WEBHOOK_SECRET, }, - data: secondPayload, + multipart: toMultipart(secondPayload), } ); @@ -699,7 +711,7 @@ test.describe("Webhook: Inbound Email", () => { headers: { "x-webhook-secret": WEBHOOK_SECRET, }, - data: firstPayload, + multipart: toMultipart(firstPayload), } ); @@ -720,7 +732,7 @@ test.describe("Webhook: Inbound Email", () => { headers: { "x-webhook-secret": WEBHOOK_SECRET, }, - data: secondPayload, + multipart: toMultipart(secondPayload), } ); diff --git a/server/src/routes/webhooks.ts b/server/src/routes/webhooks.ts index 89f0a95..b182e2d 100644 --- a/server/src/routes/webhooks.ts +++ b/server/src/routes/webhooks.ts @@ -48,7 +48,7 @@ router.post("/inbound-email", requireWebhookSecret, upload.any(), async (req, re const existingTicket = await prisma.ticket.findFirst({ where: { senderEmail: data.from, - status: "open", + status: { notIn: ["resolved", "closed"] }, subject: { equals: normalizedSubject, mode: "insensitive" }, }, });