From 02a1f5e73558d5fcef330d6f570cfac3de6d2d06 Mon Sep 17 00:00:00 2001 From: isshaddad Date: Tue, 11 Aug 2026 18:12:18 -0400 Subject: [PATCH 1/5] fix(webapp): accept Plain customers without an external id on customer cards MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Plain sends customer.externalId as an explicit null rather than omitting the key, and the schema validated it with z.string().optional(), which accepts undefined but rejects null. Every customer we don't set an externalId for got a 400 instead of a card, while the rest worked — so it looked intermittent. email, externalId and thread are now nullish; one of email/externalId is still required, and the existing email fallback resolves these customers. Two related fixes in the same path: - The route returned { cards: [] } when no user matched. Plain records an integration error for any requested key it doesn't get back, so that rendered as a broken card rather than a hidden one. Every requested key is now answered, with components: null where there's no data. - The impersonation link is offered only when the customer matched on externalId, a value we set ourselves. An email match is a weaker claim — the address on a Plain customer isn't verified and, for customers created outside our own writes, comes from whoever sent the message — so email-matched customers now get the account rows without a one-click impersonation link. - The not-found log recorded raw customer identifiers; it keeps presence flags only. The schema and the response helper live in app/utils so they can be unit-tested without pulling in the db and env modules. --- .../app/routes/api.v1.plain.customer-cards.ts | 72 ++++++++------- .../app/utils/plainCustomerCards.test.ts | 87 +++++++++++++++++++ apps/webapp/app/utils/plainCustomerCards.ts | 59 +++++++++++++ 3 files changed, 180 insertions(+), 38 deletions(-) create mode 100644 apps/webapp/app/utils/plainCustomerCards.test.ts create mode 100644 apps/webapp/app/utils/plainCustomerCards.ts diff --git a/apps/webapp/app/routes/api.v1.plain.customer-cards.ts b/apps/webapp/app/routes/api.v1.plain.customer-cards.ts index 5e09fb0854..cb9e60b218 100644 --- a/apps/webapp/app/routes/api.v1.plain.customer-cards.ts +++ b/apps/webapp/app/routes/api.v1.plain.customer-cards.ts @@ -1,31 +1,11 @@ import { json, type ActionFunctionArgs } from "@remix-run/server-runtime"; import { timingSafeEqual } from "crypto"; import { uiComponent } from "@team-plain/ui-components"; -import { z } from "zod"; import { prisma } from "~/db.server"; import { env } from "~/env.server"; import { logger } from "~/services/logger.server"; import { generateImpersonationToken } from "~/services/impersonation.server"; - -// Schema for the request body from Plain -const PlainCustomerCardRequestSchema = z.object({ - cardKeys: z.array(z.string()), - customer: z - .object({ - id: z.string(), - email: z.string().optional(), - externalId: z.string().optional(), - }) - .refine((data) => data.email || data.externalId, { - message: "Either customer.email or customer.externalId must be provided", - path: ["customer"], - }), - thread: z - .object({ - id: z.string(), - }) - .optional(), -}); +import { answerAllCardKeys, PlainCustomerCardRequestSchema } from "~/utils/plainCustomerCards"; function sanitizeHeaders( request: Request, @@ -141,14 +121,25 @@ export async function action({ request }: ActionFunctionArgs) { const user = where ? await prisma.user.findFirst({ where, include: userInclude }) : null; - // If user not found, return empty cards + /** + * Impersonation is offered only when the customer was matched on `externalId` — a value we set + * ourselves from `User.id`. + * + * Matching on email is a weaker claim: the address on a Plain customer isn't verified, and for + * customers created outside our own writes it comes from whoever sent the message. Offering a + * one-click impersonation link off the back of that would let an unverified address stand in + * for an account, so email-matched customers get the account rows without it. + */ + const canImpersonate = !!customer.externalId; + + // No matching user: still answer every requested key, with no data so Plain hides the cards. if (!user) { + // Presence flags only — the identifiers themselves don't need to persist in log storage. logger.info("User not found for Plain customer card request", { - customerId: customer.id, - externalId: customer.externalId, + hasExternalId: !!customer.externalId, hasEmail: !!customer.email, }); - return json({ cards: [] }); + return json({ cards: answerAllCardKeys(cardKeys, []) }); } // Build cards based on requested cardKeys @@ -158,10 +149,21 @@ export async function action({ request }: ActionFunctionArgs) { for (const cardKey of cardKeys) { switch (cardKey) { case accountDetailsKey: { - // Generate a signed one-time token for impersonation - const impersonationToken = await generateImpersonationToken(user.id); - // Build the impersonate URL with token for CSRF protection - const impersonateUrl = `${env.APP_ORIGIN}/admin/impersonate?impersonate=${user.id}&impersonationToken=${encodeURIComponent(impersonationToken)}`; + // Only mint a token when the button will actually be rendered — see `canImpersonate`. + const impersonationComponents = canImpersonate + ? [ + uiComponent.spacer({ size: "M" }), + uiComponent.divider({ spacingSize: "M" }), + uiComponent.spacer({ size: "M" }), + uiComponent.linkButton({ + label: "Impersonate User", + // The one-time token is what protects this link against CSRF. + url: `${env.APP_ORIGIN}/admin/impersonate?impersonate=${user.id}&impersonationToken=${encodeURIComponent( + await generateImpersonationToken(user.id) + )}`, + }), + ] + : []; cards.push({ key: accountDetailsKey, @@ -241,13 +243,7 @@ export async function action({ request }: ActionFunctionArgs) { }), ], }), - uiComponent.spacer({ size: "M" }), - uiComponent.divider({ spacingSize: "M" }), - uiComponent.spacer({ size: "M" }), - uiComponent.linkButton({ - label: "Impersonate User", - url: impersonateUrl, - }), + ...impersonationComponents, ], }), ], @@ -420,13 +416,13 @@ export async function action({ request }: ActionFunctionArgs) { } default: - // Unknown card key - skip it + // Unknown card key - answered with no data by answerAllCardKeys below. logger.info("Unknown card key requested", { cardKey }); break; } } - return json({ cards }); + return json({ cards: answerAllCardKeys(cardKeys, cards) }); } catch (error) { logger.error("Error processing Plain customer card request", { error: error instanceof Error ? error.message : String(error), diff --git a/apps/webapp/app/utils/plainCustomerCards.test.ts b/apps/webapp/app/utils/plainCustomerCards.test.ts new file mode 100644 index 0000000000..53f45e62e3 --- /dev/null +++ b/apps/webapp/app/utils/plainCustomerCards.test.ts @@ -0,0 +1,87 @@ +import { describe, expect, it } from "vitest"; +import { answerAllCardKeys, PlainCustomerCardRequestSchema } from "./plainCustomerCards"; + +const request = (overrides: Record = {}) => ({ + cardKeys: ["account-details"], + customer: { id: "c_1", email: "dev@example.com", externalId: "user_1" }, + ...overrides, +}); + +describe("PlainCustomerCardRequestSchema", () => { + it("accepts a fully populated request", () => { + expect( + PlainCustomerCardRequestSchema.safeParse(request({ thread: { id: "th_1" } })).success + ).toBe(true); + }); + + // Plain sends explicit nulls rather than omitting these keys. Rejecting them meant every + // customer created outside our own writes got a 400 instead of a card. + it("accepts a null externalId when there is an email", () => { + const result = PlainCustomerCardRequestSchema.safeParse( + request({ customer: { id: "c_1", email: "dev@example.com", externalId: null } }) + ); + + expect(result.success).toBe(true); + }); + + it("accepts a null email when there is an externalId", () => { + const result = PlainCustomerCardRequestSchema.safeParse( + request({ customer: { id: "c_1", email: null, externalId: "user_1" } }) + ); + + expect(result.success).toBe(true); + }); + + it("accepts a null thread", () => { + expect(PlainCustomerCardRequestSchema.safeParse(request({ thread: null })).success).toBe(true); + }); + + it("accepts an omitted thread", () => { + expect(PlainCustomerCardRequestSchema.safeParse(request()).success).toBe(true); + }); + + it("still requires one of email or externalId", () => { + const result = PlainCustomerCardRequestSchema.safeParse( + request({ customer: { id: "c_1", email: null, externalId: null } }) + ); + + expect(result.success).toBe(false); + }); + + it("rejects a body with no card keys field", () => { + expect(PlainCustomerCardRequestSchema.safeParse({ customer: { id: "c_1" } }).success).toBe( + false + ); + }); +}); + +describe("answerAllCardKeys", () => { + it("adds a no-data card for every unanswered key", () => { + expect(answerAllCardKeys(["a", "b"], [])).toEqual([ + { key: "a", components: null }, + { key: "b", components: null }, + ]); + }); + + it("leaves answered cards untouched", () => { + const answered = { key: "a", components: [{ componentText: { text: "hi" } }] }; + + expect(answerAllCardKeys(["a"], [answered])).toEqual([answered]); + }); + + it("fills only the gaps, keeping answered cards first", () => { + const answered = { key: "b", components: [] }; + + expect(answerAllCardKeys(["a", "b", "c"], [answered])).toEqual([ + answered, + { key: "a", components: null }, + { key: "c", components: null }, + ]); + }); + + it("ignores extra cards that were not requested", () => { + const extra = { key: "unrequested", components: [] }; + + expect(answerAllCardKeys([], [extra])).toEqual([extra]); + }); +}); diff --git a/apps/webapp/app/utils/plainCustomerCards.ts b/apps/webapp/app/utils/plainCustomerCards.ts new file mode 100644 index 0000000000..e9ed1f8c4d --- /dev/null +++ b/apps/webapp/app/utils/plainCustomerCards.ts @@ -0,0 +1,59 @@ +import { z } from "zod"; + +/** + * The request Plain sends to a customer card endpoint. + * + * `email`, `externalId` and `thread` are nullish rather than optional because Plain sends these + * keys as explicit nulls rather than omitting them — `externalId` whenever the customer was + * created outside our own writes (its Slack integration, for one), `thread` when the card is + * loaded on the customer page rather than in a thread. `.optional()` accepts `undefined` but + * rejects `null`, which failed the whole request before any lookup could run. + */ +export const PlainCustomerCardRequestSchema = z.object({ + cardKeys: z.array(z.string()), + customer: z + .object({ + id: z.string(), + email: z.string().nullish(), + externalId: z.string().nullish(), + }) + .refine((data) => data.email || data.externalId, { + message: "Either customer.email or customer.externalId must be provided", + path: ["customer"], + }), + thread: z + .object({ + id: z.string(), + }) + .nullish(), +}); + +export type PlainCustomerCardRequest = z.infer; + +type NoDataCard = { key: string; components: null }; + +/** + * Fills in a `components: null` card for every requested key that wasn't answered. + * + * Plain records an integration error against any key it asked for and didn't get back, so a + * partial response surfaces in the support app as a broken card. `components: null` is how you + * say "this card has no data" and have Plain hide it instead. + */ +export function answerAllCardKeys( + cardKeys: string[], + cards: TCard[] +): (TCard | NoDataCard)[] { + const answered = new Set(cards.map((card) => card.key)); + + return [ + ...cards, + ...cardKeys + .filter((key) => !answered.has(key)) + .map( + (key): NoDataCard => ({ + key, + components: null, + }) + ), + ]; +} From b41af4929524d49dc124b47ca4a6e9921f1afba3 Mon Sep 17 00:00:00 2001 From: isshaddad Date: Tue, 11 Aug 2026 18:24:06 -0400 Subject: [PATCH 2/5] fix(webapp): normalize the email before looking up a card's customer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Users are stored with a lowercased, trimmed email and User.email is unique, so an exact lookup on whatever Plain sends missed a real account whenever the address arrived with different casing or padding — which it can, since for customers created outside our own writes it comes from a sender address. Reachable only now that a null externalId no longer 400s. The external id lookup also falls back to email when it doesn't resolve, so a stale id naming a deleted user no longer leaves the card blank for a customer we could still identify. canImpersonate is derived from which lookup matched rather than from whether an external id was sent, so falling through to email cannot unlock the impersonation link. --- .../app/routes/api.v1.plain.customer-cards.ts | 29 +++++++++++++------ .../app/utils/plainCustomerCards.test.ts | 25 +++++++++++++++- apps/webapp/app/utils/plainCustomerCards.ts | 14 +++++++++ 3 files changed, 58 insertions(+), 10 deletions(-) diff --git a/apps/webapp/app/routes/api.v1.plain.customer-cards.ts b/apps/webapp/app/routes/api.v1.plain.customer-cards.ts index cb9e60b218..1cda84bcc9 100644 --- a/apps/webapp/app/routes/api.v1.plain.customer-cards.ts +++ b/apps/webapp/app/routes/api.v1.plain.customer-cards.ts @@ -5,7 +5,11 @@ import { prisma } from "~/db.server"; import { env } from "~/env.server"; import { logger } from "~/services/logger.server"; import { generateImpersonationToken } from "~/services/impersonation.server"; -import { answerAllCardKeys, PlainCustomerCardRequestSchema } from "~/utils/plainCustomerCards"; +import { + answerAllCardKeys, + normalizeEmail, + PlainCustomerCardRequestSchema, +} from "~/utils/plainCustomerCards"; function sanitizeHeaders( request: Request, @@ -113,24 +117,31 @@ export async function action({ request }: ActionFunctionArgs) { }, }; - const where = customer.externalId - ? { id: customer.externalId } - : customer.email - ? { email: customer.email } - : null; + // The external id is ours (`User.id`), so it's tried first. Falling back to email when it + // doesn't resolve covers a stale id — one naming a user row that no longer exists — instead of + // leaving the card blank for a customer we could still identify. + const byExternalId = customer.externalId + ? await prisma.user.findFirst({ where: { id: customer.externalId }, include: userInclude }) + : null; - const user = where ? await prisma.user.findFirst({ where, include: userInclude }) : null; + const email = normalizeEmail(customer.email); + const user = + byExternalId ?? + (email ? await prisma.user.findFirst({ where: { email }, include: userInclude }) : null); /** - * Impersonation is offered only when the customer was matched on `externalId` — a value we set + * Impersonation is offered only when the customer matched on `externalId` — a value we set * ourselves from `User.id`. * * Matching on email is a weaker claim: the address on a Plain customer isn't verified, and for * customers created outside our own writes it comes from whoever sent the message. Offering a * one-click impersonation link off the back of that would let an unverified address stand in * for an account, so email-matched customers get the account rows without it. + * + * Derived from which lookup actually matched, not from whether an external id was *sent* — an + * id that misses and falls through to email must not unlock impersonation. */ - const canImpersonate = !!customer.externalId; + const canImpersonate = Boolean(byExternalId); // No matching user: still answer every requested key, with no data so Plain hides the cards. if (!user) { diff --git a/apps/webapp/app/utils/plainCustomerCards.test.ts b/apps/webapp/app/utils/plainCustomerCards.test.ts index 53f45e62e3..ac2a3dc300 100644 --- a/apps/webapp/app/utils/plainCustomerCards.test.ts +++ b/apps/webapp/app/utils/plainCustomerCards.test.ts @@ -1,5 +1,9 @@ import { describe, expect, it } from "vitest"; -import { answerAllCardKeys, PlainCustomerCardRequestSchema } from "./plainCustomerCards"; +import { + answerAllCardKeys, + normalizeEmail, + PlainCustomerCardRequestSchema, +} from "./plainCustomerCards"; const request = (overrides: Record = {}) => ({ cardKeys: ["account-details"], @@ -55,6 +59,25 @@ describe("PlainCustomerCardRequestSchema", () => { }); }); +// Users are stored with a lowercased, trimmed email, so a lookup on the raw value Plain sends +// would miss a real account whose address differs only in casing or padding. +describe("normalizeEmail", () => { + it("lowercases and trims", () => { + expect(normalizeEmail(" DEV@Example.COM ")).toBe("dev@example.com"); + }); + + it("leaves an already-normalized address alone", () => { + expect(normalizeEmail("dev@example.com")).toBe("dev@example.com"); + }); + + it("is null for absent or empty addresses, so the lookup can be skipped", () => { + expect(normalizeEmail(null)).toBeNull(); + expect(normalizeEmail(undefined)).toBeNull(); + expect(normalizeEmail("")).toBeNull(); + expect(normalizeEmail(" ")).toBeNull(); + }); +}); + describe("answerAllCardKeys", () => { it("adds a no-data card for every unanswered key", () => { expect(answerAllCardKeys(["a", "b"], [])).toEqual([ diff --git a/apps/webapp/app/utils/plainCustomerCards.ts b/apps/webapp/app/utils/plainCustomerCards.ts index e9ed1f8c4d..a465a62278 100644 --- a/apps/webapp/app/utils/plainCustomerCards.ts +++ b/apps/webapp/app/utils/plainCustomerCards.ts @@ -30,6 +30,20 @@ export const PlainCustomerCardRequestSchema = z.object({ export type PlainCustomerCardRequest = z.infer; +/** + * An email in the form `User.email` is stored in. + * + * Users are written with `email.toLowerCase().trim()` (see `createUser` / SSO upsert in + * `models/user.server.ts`), and `User.email` is unique, so an exact lookup on whatever Plain sends + * would miss a real account whenever the address arrives with different casing or padding — which + * it can, because for customers created outside our own writes it comes from a sender address. + * + * Returns null for an address with nothing left after trimming, so callers can skip the lookup. + */ +export function normalizeEmail(email: string | null | undefined): string | null { + return email?.toLowerCase().trim() || null; +} + type NoDataCard = { key: string; components: null }; /** From 7367118e2fb8cffa27247247b5af9b210cedfec8 Mon Sep 17 00:00:00 2001 From: isshaddad Date: Wed, 12 Aug 2026 08:38:30 -0400 Subject: [PATCH 3/5] fix(webapp): cap how long an empty customer card is cached, add release note MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Omitting timeToLiveSeconds on a no-data card falls back to the TTL configured for that card in Plain, so a customer who becomes resolvable — an external id gets set, or someone signs up with that address — would keep showing an empty card for however long that default is. Set explicitly to 60s. An external id that no longer resolves is now logged. The email match keeps the card useful, but a stale link we wrote ourselves should not stay invisible. --- .../plain-customer-card-external-id.md | 6 ++++++ .../app/routes/api.v1.plain.customer-cards.ts | 8 ++++++++ apps/webapp/app/utils/plainCustomerCards.test.ts | 16 ++++++++++++---- apps/webapp/app/utils/plainCustomerCards.ts | 13 ++++++++++++- 4 files changed, 38 insertions(+), 5 deletions(-) create mode 100644 .server-changes/plain-customer-card-external-id.md diff --git a/.server-changes/plain-customer-card-external-id.md b/.server-changes/plain-customer-card-external-id.md new file mode 100644 index 0000000000..250ee1c06e --- /dev/null +++ b/.server-changes/plain-customer-card-external-id.md @@ -0,0 +1,6 @@ +--- +area: webapp +type: fix +--- + +Fixed support threads showing no account details for some customers, so the team can see your plan, organizations and projects when you get in touch. diff --git a/apps/webapp/app/routes/api.v1.plain.customer-cards.ts b/apps/webapp/app/routes/api.v1.plain.customer-cards.ts index 1cda84bcc9..6f1cb711eb 100644 --- a/apps/webapp/app/routes/api.v1.plain.customer-cards.ts +++ b/apps/webapp/app/routes/api.v1.plain.customer-cards.ts @@ -129,6 +129,14 @@ export async function action({ request }: ActionFunctionArgs) { byExternalId ?? (email ? await prisma.user.findFirst({ where: { email }, include: userInclude }) : null); + // An external id we set ourselves that no longer resolves is an anomaly worth seeing, even + // though the email match keeps the card useful — otherwise the stale link stays invisible. + if (customer.externalId && !byExternalId) { + logger.warn("Plain customer card external id did not resolve", { + resolvedByEmail: !!user, + }); + } + /** * Impersonation is offered only when the customer matched on `externalId` — a value we set * ourselves from `User.id`. diff --git a/apps/webapp/app/utils/plainCustomerCards.test.ts b/apps/webapp/app/utils/plainCustomerCards.test.ts index ac2a3dc300..9061568f0a 100644 --- a/apps/webapp/app/utils/plainCustomerCards.test.ts +++ b/apps/webapp/app/utils/plainCustomerCards.test.ts @@ -81,8 +81,8 @@ describe("normalizeEmail", () => { describe("answerAllCardKeys", () => { it("adds a no-data card for every unanswered key", () => { expect(answerAllCardKeys(["a", "b"], [])).toEqual([ - { key: "a", components: null }, - { key: "b", components: null }, + { key: "a", components: null, timeToLiveSeconds: 60 }, + { key: "b", components: null, timeToLiveSeconds: 60 }, ]); }); @@ -97,11 +97,19 @@ describe("answerAllCardKeys", () => { expect(answerAllCardKeys(["a", "b", "c"], [answered])).toEqual([ answered, - { key: "a", components: null }, - { key: "c", components: null }, + { key: "a", components: null, timeToLiveSeconds: 60 }, + { key: "c", components: null, timeToLiveSeconds: 60 }, ]); }); + // Omitting the TTL would fall back to the card's configured default, keeping an empty card in + // Plain's cache after the customer becomes resolvable. + it("caps how long an empty card is cached", () => { + const [filler] = answerAllCardKeys(["a"], []); + + expect(filler).toMatchObject({ timeToLiveSeconds: 60 }); + }); + it("ignores extra cards that were not requested", () => { const extra = { key: "unrequested", components: [] }; diff --git a/apps/webapp/app/utils/plainCustomerCards.ts b/apps/webapp/app/utils/plainCustomerCards.ts index a465a62278..1ed9aa9817 100644 --- a/apps/webapp/app/utils/plainCustomerCards.ts +++ b/apps/webapp/app/utils/plainCustomerCards.ts @@ -44,7 +44,17 @@ export function normalizeEmail(email: string | null | undefined): string | null return email?.toLowerCase().trim() || null; } -type NoDataCard = { key: string; components: null }; +type NoDataCard = { key: string; components: null; timeToLiveSeconds: number }; + +/** + * How long Plain may cache a card we had no data for. + * + * Explicit rather than omitted: omitting the field falls back to the TTL configured for that card + * in Plain's settings, so a customer who becomes resolvable — an external id gets set, or someone + * signs up with that address — would keep showing an empty card for however long that default is. + * Short enough to recover promptly, long enough not to re-ask on every glance at a thread. + */ +const NO_DATA_TTL_SECONDS = 60; /** * Fills in a `components: null` card for every requested key that wasn't answered. @@ -67,6 +77,7 @@ export function answerAllCardKeys( (key): NoDataCard => ({ key, components: null, + timeToLiveSeconds: NO_DATA_TTL_SECONDS, }) ), ]; From d1cb09885f501359677e95d18a6c9a65d531b8cb Mon Sep 17 00:00:00 2001 From: isshaddad Date: Wed, 12 Aug 2026 08:48:39 -0400 Subject: [PATCH 4/5] fix(webapp): look a card's customer up by both sent and lowercased email MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit User.email isn't stored consistently cased. The SSO upsert writes email.toLowerCase().trim(), but findOrCreateMagicLinkUser and the OAuth paths store whatever the provider gave, so neither an exact match nor a lowercased one finds everybody: exact misses an SSO user whose address arrives capitalised, and lowercasing — as this branch previously did — misses a magic-link user stored with capitals, blanking a card that used to resolve. The lookup now tries the address as sent, then its lowercased form. Both are exact matches so each uses the unique index on email, which a case-insensitive query would not; the common case still hits on the first. --- .../app/routes/api.v1.plain.customer-cards.ts | 17 +++++++--- .../app/utils/plainCustomerCards.test.ts | 34 ++++++++++++------- apps/webapp/app/utils/plainCustomerCards.ts | 25 +++++++++----- 3 files changed, 50 insertions(+), 26 deletions(-) diff --git a/apps/webapp/app/routes/api.v1.plain.customer-cards.ts b/apps/webapp/app/routes/api.v1.plain.customer-cards.ts index 6f1cb711eb..bfb9988bee 100644 --- a/apps/webapp/app/routes/api.v1.plain.customer-cards.ts +++ b/apps/webapp/app/routes/api.v1.plain.customer-cards.ts @@ -7,7 +7,7 @@ import { logger } from "~/services/logger.server"; import { generateImpersonationToken } from "~/services/impersonation.server"; import { answerAllCardKeys, - normalizeEmail, + emailLookupCandidates, PlainCustomerCardRequestSchema, } from "~/utils/plainCustomerCards"; @@ -124,10 +124,17 @@ export async function action({ request }: ActionFunctionArgs) { ? await prisma.user.findFirst({ where: { id: customer.externalId }, include: userInclude }) : null; - const email = normalizeEmail(customer.email); - const user = - byExternalId ?? - (email ? await prisma.user.findFirst({ where: { email }, include: userInclude }) : null); + // Emails aren't stored consistently cased, so try the address as sent and then its lowercased + // form — see `emailLookupCandidates`. Both are exact matches on the unique index. + const findByEmail = async () => { + for (const email of emailLookupCandidates(customer.email)) { + const match = await prisma.user.findFirst({ where: { email }, include: userInclude }); + if (match) return match; + } + return null; + }; + + const user = byExternalId ?? (await findByEmail()); // An external id we set ourselves that no longer resolves is an anomaly worth seeing, even // though the email match keeps the card useful — otherwise the stale link stays invisible. diff --git a/apps/webapp/app/utils/plainCustomerCards.test.ts b/apps/webapp/app/utils/plainCustomerCards.test.ts index 9061568f0a..18048feb72 100644 --- a/apps/webapp/app/utils/plainCustomerCards.test.ts +++ b/apps/webapp/app/utils/plainCustomerCards.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it } from "vitest"; import { answerAllCardKeys, - normalizeEmail, + emailLookupCandidates, PlainCustomerCardRequestSchema, } from "./plainCustomerCards"; @@ -59,22 +59,30 @@ describe("PlainCustomerCardRequestSchema", () => { }); }); -// Users are stored with a lowercased, trimmed email, so a lookup on the raw value Plain sends -// would miss a real account whose address differs only in casing or padding. -describe("normalizeEmail", () => { - it("lowercases and trims", () => { - expect(normalizeEmail(" DEV@Example.COM ")).toBe("dev@example.com"); +// `User.email` casing depends on the signup path: the SSO upsert lowercases, magic-link and OAuth +// store what the provider gave. Either candidate alone misses one of those populations. +describe("emailLookupCandidates", () => { + it("tries the address as sent before its lowercased form", () => { + // Finds a magic-link user stored with capitals, then an SSO user stored lowercased. + expect(emailLookupCandidates("Dev@Example.com")).toEqual([ + "Dev@Example.com", + "dev@example.com", + ]); + }); + + it("yields a single candidate when the address is already lowercase", () => { + expect(emailLookupCandidates("dev@example.com")).toEqual(["dev@example.com"]); }); - it("leaves an already-normalized address alone", () => { - expect(normalizeEmail("dev@example.com")).toBe("dev@example.com"); + it("trims before comparing, so padding doesn't produce a duplicate candidate", () => { + expect(emailLookupCandidates(" dev@example.com ")).toEqual(["dev@example.com"]); }); - it("is null for absent or empty addresses, so the lookup can be skipped", () => { - expect(normalizeEmail(null)).toBeNull(); - expect(normalizeEmail(undefined)).toBeNull(); - expect(normalizeEmail("")).toBeNull(); - expect(normalizeEmail(" ")).toBeNull(); + it("is empty for absent or blank addresses, so the lookup can be skipped", () => { + expect(emailLookupCandidates(null)).toEqual([]); + expect(emailLookupCandidates(undefined)).toEqual([]); + expect(emailLookupCandidates("")).toEqual([]); + expect(emailLookupCandidates(" ")).toEqual([]); }); }); diff --git a/apps/webapp/app/utils/plainCustomerCards.ts b/apps/webapp/app/utils/plainCustomerCards.ts index 1ed9aa9817..192b504060 100644 --- a/apps/webapp/app/utils/plainCustomerCards.ts +++ b/apps/webapp/app/utils/plainCustomerCards.ts @@ -31,17 +31,26 @@ export const PlainCustomerCardRequestSchema = z.object({ export type PlainCustomerCardRequest = z.infer; /** - * An email in the form `User.email` is stored in. + * The values to try, in order, when looking a user up by email. * - * Users are written with `email.toLowerCase().trim()` (see `createUser` / SSO upsert in - * `models/user.server.ts`), and `User.email` is unique, so an exact lookup on whatever Plain sends - * would miss a real account whenever the address arrives with different casing or padding — which - * it can, because for customers created outside our own writes it comes from a sender address. + * `User.email` is not stored consistently cased: the SSO upsert writes + * `email.toLowerCase().trim()`, while `findOrCreateMagicLinkUser` and the OAuth paths store + * whatever the provider gave us. So neither an exact match nor a lowercased one finds everybody — + * exact misses an SSO user whose address arrives capitalised, lowercased misses a magic-link user + * stored with capitals. * - * Returns null for an address with nothing left after trimming, so callers can skip the lookup. + * Hence two candidates: the address as sent (trimmed), then its lowercased form. Both are exact + * matches, so each uses the unique index on `email` — a case-insensitive query would not, and this + * table is far too big to scan. The common case hits on the first. + * + * Empty when there's no usable address, so callers can skip the lookup entirely. */ -export function normalizeEmail(email: string | null | undefined): string | null { - return email?.toLowerCase().trim() || null; +export function emailLookupCandidates(email: string | null | undefined): string[] { + const asSent = email?.trim(); + if (!asSent) return []; + + const lowercased = asSent.toLowerCase(); + return asSent === lowercased ? [asSent] : [asSent, lowercased]; } type NoDataCard = { key: string; components: null; timeToLiveSeconds: number }; From 98d6b40c3899277c961279726520a4da4888d6da Mon Sep 17 00:00:00 2001 From: isshaddad Date: Wed, 12 Aug 2026 09:00:28 -0400 Subject: [PATCH 5/5] fix(webapp): stop rejecting card requests for customers with no identifier MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A contact created by an integration can legitimately have neither an email nor an external id. The schema still rejected that shape, so Plain recorded an integration error for the whole request — the same failure this PR removes for a null external id. There is nothing to look up, so the route now answers every requested key with no data and Plain hides the cards. The route already handled it; only the refine stood in the way. --- .../app/utils/plainCustomerCards.test.ts | 6 ++++-- apps/webapp/app/utils/plainCustomerCards.ts | 19 +++++++++---------- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/apps/webapp/app/utils/plainCustomerCards.test.ts b/apps/webapp/app/utils/plainCustomerCards.test.ts index 18048feb72..4a9fcb4301 100644 --- a/apps/webapp/app/utils/plainCustomerCards.test.ts +++ b/apps/webapp/app/utils/plainCustomerCards.test.ts @@ -44,12 +44,14 @@ describe("PlainCustomerCardRequestSchema", () => { expect(PlainCustomerCardRequestSchema.safeParse(request()).success).toBe(true); }); - it("still requires one of email or externalId", () => { + // A contact created by an integration can have neither identifier. There's nothing to look up, + // but rejecting it would make Plain record an integration error rather than hide the card. + it("accepts a customer with neither email nor externalId", () => { const result = PlainCustomerCardRequestSchema.safeParse( request({ customer: { id: "c_1", email: null, externalId: null } }) ); - expect(result.success).toBe(false); + expect(result.success).toBe(true); }); it("rejects a body with no card keys field", () => { diff --git a/apps/webapp/app/utils/plainCustomerCards.ts b/apps/webapp/app/utils/plainCustomerCards.ts index 192b504060..96303b0dec 100644 --- a/apps/webapp/app/utils/plainCustomerCards.ts +++ b/apps/webapp/app/utils/plainCustomerCards.ts @@ -11,16 +11,15 @@ import { z } from "zod"; */ export const PlainCustomerCardRequestSchema = z.object({ cardKeys: z.array(z.string()), - customer: z - .object({ - id: z.string(), - email: z.string().nullish(), - externalId: z.string().nullish(), - }) - .refine((data) => data.email || data.externalId, { - message: "Either customer.email or customer.externalId must be provided", - path: ["customer"], - }), + // A customer with neither an email nor an external id is valid input, not a malformed request: + // a contact created by an integration can legitimately have neither. There's nothing to look up, + // so the route answers every key with no data — rejecting it would make Plain record an + // integration error, which is the failure this schema change exists to remove. + customer: z.object({ + id: z.string(), + email: z.string().nullish(), + externalId: z.string().nullish(), + }), thread: z .object({ id: z.string(),