Repository navigation
Add detailed feedback notifications and admin deep links - #785
Conversation
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
📝 WalkthroughWalkthroughThe PR adds an audited admin platform-feedback review route, persists submitter identity snapshots, expands the feedback subscription event with approved untrusted content and metadata, and changes queued delivery to reload by feedback ID with permanent cancellation after deletion. Documentation and MCP consent guidance are updated accordingly. ChangesPlatform feedback administration and delivery
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Admin
participant AdminPlatformFeedbackRoute
participant AdminFeedbackAPI
participant FeedbackLoader
participant FeedbackDatabase
Admin->>AdminPlatformFeedbackRoute: open queue or detail URL
AdminPlatformFeedbackRoute->>AdminFeedbackAPI: authenticated JSON request
AdminFeedbackAPI->>FeedbackLoader: enforce role and load filters
FeedbackLoader->>FeedbackDatabase: query list and selected feedback
FeedbackDatabase-->>AdminPlatformFeedbackRoute: return feedback data
sequenceDiagram
participant FeedbackQueue
participant SubscriptionDispatcher
participant FeedbackRepository
participant NotificationSubscription
FeedbackQueue->>SubscriptionDispatcher: deliver feedbackId
SubscriptionDispatcher->>FeedbackRepository: reload feedback before invocation
FeedbackRepository-->>SubscriptionDispatcher: feedback row or deleted result
SubscriptionDispatcher->>NotificationSubscription: invoke expanded event
SubscriptionDispatcher-->>FeedbackQueue: acknowledge permanent cancellation
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
|
🔎 Preview deployed: https://kody-pr-785.kody-a99.workers.dev Worker: Mocks:
|
There was a problem hiding this comment.
🧹 Nitpick comments (3)
packages/worker/client/routes/account-management-components.tsx (1)
101-121: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNav item sourcing is inconsistent with its siblings.
The new "Platform feedback" entry derives
href/pathsfromroutes.adminPlatformFeedback.href(), but every other entry inadminNavItemsstill hardcodes its path as a literal string. Both approaches work today, but this creates two ways to express the same information, and only the new entry benefits from route-constant safety ifroutes.tschanges.Consider migrating the whole array to reference
routes.*.href()for consistency (can be deferred, not blocking this PR).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/worker/client/routes/account-management-components.tsx` around lines 101 - 121, Standardize adminNavItems route sourcing by replacing the hardcoded href and paths values with the corresponding routes.*.href() references for every entry, matching the existing adminPlatformFeedback entry while preserving labels and navigation behavior.packages/worker/src/platform-feedback/repo.ts (1)
179-241: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated filter/where-building logic between the two admin listing functions.
listPlatformFeedbackRowsForAdminandlistPlatformFeedbackPageRowsForAdmineach independently rebuild the samefilters/bindings/whereclause fromstatus/category. Any future filter added to one function but not the other would silently desync thetotalcount from the returned page items.♻️ Extract shared filter-building helper
+function buildPlatformFeedbackAdminFilters(input: { + status?: PlatformFeedbackStatus + category?: PlatformFeedbackCategory +}) { + const filters: Array<string> = [] + const bindings: Array<unknown> = [] + if (input.status !== undefined) { + filters.push('status = ?') + bindings.push(input.status) + } + if (input.category !== undefined) { + filters.push('category = ?') + bindings.push(input.category) + } + return { + where: filters.length > 0 ? `WHERE ${filters.join(' AND ')}` : '', + bindings, + } +} + export async function listPlatformFeedbackRowsForAdmin( db: D1Database, input: { page: number; pageSize: number; status?: PlatformFeedbackStatus; category?: PlatformFeedbackCategory }, ): Promise<{ total: number; items: Array<PlatformFeedbackListItem> }> { - const filters: Array<string> = [] - const bindings: Array<unknown> = [] - if (input.status !== undefined) { - filters.push('status = ?') - bindings.push(input.status) - } - if (input.category !== undefined) { - filters.push('category = ?') - bindings.push(input.category) - } - const where = filters.length > 0 ? `WHERE ${filters.join(' AND ')}` : '' + const { where, bindings } = buildPlatformFeedbackAdminFilters(input) const countRow = await db .prepare(`SELECT COUNT(*) AS total FROM platform_feedback ${where}`) .bind(...bindings) .first<{ total: number }>() const items = await listPlatformFeedbackPageRowsForAdmin(db, input) return { total: Number(countRow?.total ?? 0), items } } export async function listPlatformFeedbackPageRowsForAdmin( db: D1Database, input: { page: number; pageSize: number; status?: PlatformFeedbackStatus; category?: PlatformFeedbackCategory }, ): Promise<Array<PlatformFeedbackListItem>> { - const filters: Array<string> = [] - const bindings: Array<unknown> = [] - if (input.status !== undefined) { - filters.push('status = ?') - bindings.push(input.status) - } - if (input.category !== undefined) { - filters.push('category = ?') - bindings.push(input.category) - } - const where = filters.length > 0 ? `WHERE ${filters.join(' AND ')}` : '' + const { where, bindings } = buildPlatformFeedbackAdminFilters(input) const rows = await db .prepare(`SELECT ${platformFeedbackListColumns} FROM platform_feedback ${where} ORDER BY created_at DESC, id DESC LIMIT ? OFFSET ?`) .bind(...bindings, input.pageSize, (input.page - 1) * input.pageSize) .all<Record<string, unknown>>() return (rows.results ?? []).map(mapPlatformFeedbackListRow) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/worker/src/platform-feedback/repo.ts` around lines 179 - 241, Extract the shared status/category filter, bindings, and WHERE-clause construction from listPlatformFeedbackRowsForAdmin and listPlatformFeedbackPageRowsForAdmin into a helper, then reuse it in both queries so count and page filtering remain synchronized. Preserve the existing filter order and binding behavior.packages/worker/src/platform-feedback/dispatch-queue.ts (1)
23-55: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider logging permanent cancellations for observability.
The cancellation branch (Lines 41-44) acks silently, unlike the failure branch which logs via
console.error. A lightweight log here would help track how often feedback rows are deleted before dispatch completes.🔎 Optional: add a log line on cancellation
if (error instanceof PlatformFeedbackDispatchCancelledError) { + console.info('platform-feedback-dispatch-cancelled', { + queueMessageId: queueMessage.id, + feedbackId: parsed.feedbackId, + }) queueMessage.ack() continue }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/worker/src/platform-feedback/dispatch-queue.ts` around lines 23 - 55, Log a lightweight cancellation event in the PlatformFeedbackDispatchCancelledError branch of handlePlatformFeedbackDispatchQueue before acknowledging the message, including the queue message ID and feedback ID; preserve the existing ack-and-continue behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/worker/client/routes/account-management-components.tsx`:
- Around line 101-121: Standardize adminNavItems route sourcing by replacing the
hardcoded href and paths values with the corresponding routes.*.href()
references for every entry, matching the existing adminPlatformFeedback entry
while preserving labels and navigation behavior.
In `@packages/worker/src/platform-feedback/dispatch-queue.ts`:
- Around line 23-55: Log a lightweight cancellation event in the
PlatformFeedbackDispatchCancelledError branch of
handlePlatformFeedbackDispatchQueue before acknowledging the message, including
the queue message ID and feedback ID; preserve the existing ack-and-continue
behavior.
In `@packages/worker/src/platform-feedback/repo.ts`:
- Around line 179-241: Extract the shared status/category filter, bindings, and
WHERE-clause construction from listPlatformFeedbackRowsForAdmin and
listPlatformFeedbackPageRowsForAdmin into a helper, then reuse it in both
queries so count and page filtering remain synchronized. Preserve the existing
filter order and binding behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 15b98b6c-aa66-4c2a-9cf8-f7fdc92a5116
📒 Files selected for processing (38)
docs/contributing/architecture/authorization.mddocs/contributing/packages-and-manifests.mddocs/guides/package-subscriptions.mddocs/guides/platform-friction.mddocs/use/privacy.mdpackages/worker/client/routes/account-management-components.tsxpackages/worker/client/routes/admin-platform-feedback.tsxpackages/worker/client/routes/index.tsxpackages/worker/client/routes/privacy.tsxpackages/worker/migrations/0063-platform-feedback-submitter-snapshot.sqlpackages/worker/src/app/account-deletion.node.test.tspackages/worker/src/app/account-export.node.test.tspackages/worker/src/app/admin-platform-feedback-data.node.test.tspackages/worker/src/app/admin-platform-feedback-data.tspackages/worker/src/app/handlers/admin-platform-feedback.node.test.tspackages/worker/src/app/handlers/admin-platform-feedback.tspackages/worker/src/app/loader-data.tspackages/worker/src/app/router.tspackages/worker/src/app/routes.tspackages/worker/src/mcp/capabilities/coding/kody-official-guide.tspackages/worker/src/mcp/capabilities/meta/meta-platform-feedback-submit.tspackages/worker/src/mcp/capabilities/packages/list-package-subscriptions.tspackages/worker/src/mcp/capabilities/platform-feedback-capabilities.node.test.tspackages/worker/src/mcp/server-instructions.tspackages/worker/src/platform-feedback/dispatch-queue.node.test.tspackages/worker/src/platform-feedback/dispatch-queue.tspackages/worker/src/platform-feedback/errors.tspackages/worker/src/platform-feedback/package-subscriptions.node.test.tspackages/worker/src/platform-feedback/package-subscriptions.tspackages/worker/src/platform-feedback/platform-feedback-service.node.test.tspackages/worker/src/platform-feedback/platform-feedback-submitter-snapshot-migration.node.test.tspackages/worker/src/platform-feedback/platform-feedback-subscriptions.workers.test.tspackages/worker/src/platform-feedback/repo.tspackages/worker/src/platform-feedback/service.tspackages/worker/src/platform-feedback/submitter-identity.node.test.tspackages/worker/src/platform-feedback/submitter-identity.tspackages/worker/src/platform-feedback/subscription-event.tspackages/worker/src/platform-feedback/types.ts
Summary
/admin/platform-feedback?feedbackId=…review page and JSON endpointTesting
npm run validatepasses: formatting, lint, typecheck, primitive map, 905 unit tests, 15 Playwright E2E tests, and 2 MCP E2E tests.window.__feedback_xssis undefined.CLEAN.Walkthrough
System recap — extends existing primitives (medium risk)
Mode: recap · Base:
main@f161b9b3· Head:aa15f275Classification: extends — broadens the explicitly consented feedback event contract, snapshots attribution, and adds a role-gated review surface without adding a new primitive.
Primitives touched
platform-feedbackapp-uirbacd1-app-dbplatform-feedback-dispatch-queuesaved-packagesmcp-serverSystem map
An approved submission snapshots identity in D1 but keeps the Queue body opaque; immediately before fan-out, Kody rechecks the row and builds a stable full-content event plus guarded admin deep link.
Legend: green = composes (wiring only) · amber = extended by this PR · red = new primitive · gray = context (unchanged, included only when an edge crosses it).
Invariants
Summary by CodeRabbit
New Features
Documentation
Bug Fixes