Finish the audience surface: wire dead controls + session auth#660
Finish the audience surface: wire dead controls + session auth#660jaeyunha wants to merge 8 commits into
Conversation
contacts/[id], segments/[id], topics/[id], and properties/[id] all authenticated with validateApiKey only, so their GET/PATCH/DELETE handlers returned 401 for a logged-in dashboard user (session cookie, no Bearer key). This made every Edit/Delete action in the Audience UI non-functional regardless of whether the buttons were wired. Switch all four route families to authorizeDashboardOrApiKey + a resolveUserId helper (session cookie OR full-access API key), matching the pattern already used by /api/contacts POST and the CSV import route. Update the route tests that previously asserted API-key-only behavior to expect session-or-key auth, and re-establish the new auth mock after the mid-test vi.resetAllMocks() in the contact tenant-isolation suite. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The contact detail page rendered an actions dropdown but passed
onEdit={() => {}} and onDelete={() => {}} — both no-ops. Clicking
Edit/Delete did nothing.
Add an EditContactModal (first/last name + unsubscribed toggle) that
PATCHes /api/contacts/[id], and a delete confirmation dialog that
DELETEs the contact and routes back to /audience. Both rely on the
dashboard session cookie now that the detail route accepts it.
Tests: behavioral cases that open the edit modal with seeded values and
assert the delete API is called on confirm — both fail on the prior
dead handlers.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The segments list rendered seg.contactsCount / seg.unsubscribedCount and
seg.createdAt, but GET /api/segments returned only {id, name, created_at}
— so the Contacts/Unsubscribed columns were blank and the created date
was mis-mapped (snake_case created_at vs camelCase createdAt).
- segmentRepo.listForApi now computes contactsCount and unsubscribedCount
per segment via subqueries on the contacts_to_segments join table.
- toSegmentListItem exposes contacts_count and unsubscribed_count.
- segments-list maps the snake_case API response to its camelCase shape,
fixing the counts and the created date together.
Docs + tests updated to the new response shape.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Every audience list table (contacts, segments, topics, properties) rendered a row "⋮ More actions" button with no onClick — a dead control. Add a reusable RowActionsMenu (dropdown + confirm dialog) and wire it into all four lists with a working Delete action against the now session-aware detail routes, refreshing the list on success. Contacts also get a View / edit shortcut to the detail page. Tests: behavioral coverage for the menu opening, running a non-destructive action, and requiring confirmation before delete. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The topics page stacked the unsubscribe-page preview far below the topics list, so the two were never visible together. Match the Resend-style split layout: topics list on the left, the live unsubscribe-page preview on the right (sticky on wide screens), collapsing to a single column on narrow viewports. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5d29245432
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const res = await fetch(`/api/contacts/${contact.id}`, { | ||
| method: "PATCH", |
There was a problem hiding this comment.
When saving a normal persisted contact whose contact.id is a UUID, this PATCH goes through contactService().updateContact() → findByIdOrEmailForUser(). That helper currently only recognizes a non-standard four-part UUID regex, so a real DB UUID is treated as an email address and the route returns Contact not found; the newly wired edit modal therefore cannot save most contacts until the ID lookup is fixed or this path uses an identifier the service can resolve.
Useful? React with 👍 / 👎.
| contactsCount: sql<number>`( | ||
| select count(*) from ${contactsToSegments} | ||
| where ${contactsToSegments.segmentId} = ${segments.id} |
There was a problem hiding this comment.
For contacts assigned to segments through the existing POST /api/contacts, bulk-add, or import flows, membership is persisted in contacts.segments and those flows do not insert rows into contacts_to_segments. Reading counts only from the join table makes the new contacts_count/dashboard segment totals show zero or stale values for those common assignment paths, even though the same contacts still filter/list as segment members elsewhere.
Useful? React with 👍 / 👎.
| const auth = await authorizeDashboardOrApiKey( | ||
| _request.headers.get("authorization"), | ||
| ); |
There was a problem hiding this comment.
src/app/segments/[id]/route.ts re-exports this handler for the public /segments/:id alias, so switching the shared detail handler to authorizeDashboardOrApiKey makes GET/DELETE /segments/:id succeed with only a Better Auth dashboard cookie. That violates the root segment alias contract that public API paths require an API key; split the dashboard-session adapter from the root alias or keep the root export on API-key-only auth.
Useful? React with 👍 / 👎.
Hermes maintainer reviewVerdict: changes requested. CI is green, but the diff has a few real product/accessibility defects that should be fixed before this main-target PR moves. Blocking findings
Evidence
I spawned a follow-up Clawhip lane in |
|
Addressed the Hermes review feedback on this existing PR branch. Summary:
Head SHA: Validation:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 68ccbf16bc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const auth = await authorizeDashboardOrApiKey( | ||
| request.headers.get("authorization"), | ||
| ); |
There was a problem hiding this comment.
When this handler is reached through src/app/contacts/[contact_id]/route.ts, a logged-in dashboard browser can now call GET/PATCH/DELETE /contacts/:contact_id with only the Better Auth cookie because this path resolves dashboard sessions instead of requiring validateApiKey. Those root contact routes are the public SDK/API aliases, so this changes them from API-key-only to cookie-authenticated; split the dashboard /api/contacts/:id handler from the root alias or keep API-key validation in the alias path.
Useful? React with 👍 / 👎.
| const auth = await authorizeDashboardOrApiKey( | ||
| request.headers.get("authorization"), | ||
| ); |
There was a problem hiding this comment.
For public compatibility requests such as PATCH /topics/:id or DELETE /topics/:id, src/middleware.ts rewrites the root alias to this /api/topics/:id handler and tags it as a root alias. After this auth switch, a dashboard session cookie is enough to mutate topic records without any Authorization header, so the public topic alias no longer enforces API-key auth; route root aliases through an API-key-only adapter or reject dashboard-only auth when the root-alias header is present.
Useful? React with 👍 / 👎.
| const auth = await authorizeDashboardOrApiKey( | ||
| request.headers.get("authorization"), | ||
| ); |
There was a problem hiding this comment.
For public compatibility requests such as PATCH /contact-properties/:id or DELETE /contact-properties/:id, src/middleware.ts rewrites the root alias to this /api/properties/:id handler and marks it as a root alias. With this new authorizeDashboardOrApiKey path, a dashboard cookie can now update or delete a contact property through the public alias without a Bearer key, so keep the root alias on API-key-only auth or split it from the dashboard handler.
Useful? React with 👍 / 👎.
|
Correction to the previous comment: shell interpolation stripped the inline code spans. Literal summary below. Addressed the current Codex P2 blockers on PR #660. Fixed:
Head SHA: f3cf23d Validation:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f3cf23d788
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| }; | ||
|
|
||
| function normalizeProperty(property: PropertyApiPayload): Property { | ||
| const type = property.type === "number" ? "number" : "string"; |
There was a problem hiding this comment.
When /api/properties returns properties created through the public/root API with supported type values like boolean or date (the audience metadata service explicitly accepts those enum values and returns row.type), this normalizer coerces everything except number to string. The dashboard will therefore display existing boolean/date contact properties as string properties, which can mislead operators reviewing their schema; keep the full set of supported types instead of collapsing them here.
Useful? React with 👍 / 👎.
1 participant
What
UltraQA of the Audience surface found it was broken at two layers: dead UI controls and mutation APIs that rejected dashboard logins. This finishes it.
The root problem
The Audience dashboard was scaffolded UI-first and API-key-first, so per-row actions were dead stubs and the detail routes only accepted Bearer API keys — a logged-in dashboard user (session cookie, no key) got 401 on every Edit/Delete. The unit tests were green throughout because they asserted rendering, not behavior.
Changes (5 commits)
contacts/[id],segments/[id],topics/[id],properties/[id]now accept a dashboard session OR an API key (authorizeDashboardOrApiKey), matching/api/contactsPOST.onEdit={() => {}}no-ops; now a real edit modal (PATCH) + delete confirm (DELETE).contactsCount/unsubscribedCountbut the API never returned them (and the created date was mis-mapped); now computed from the join table and mapped correctly.⋮button; now a reusableRowActionsMenuwith working Delete + confirm.Tests
Behavioral coverage added for the new wiring (each fails on the prior dead version): contact edit/delete, the row menu, segment counts. Full suite: 1731 passed, 0 failed;
make checkclean. Public segments API doc updated +docs:generate.Note
Discovered during QA prompted by the CSV-import work (#659, merged). These are pre-existing gaps, not regressions.
🤖 Generated with Claude Code