feat: Posts #195
feat: Posts #195
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. WalkthroughAdds a Post feature: DB Post table and types, TRPC posts router with CRUD/indexing/mentions, UI for listing/creating/editing posts (page, list, form, tabs), search indexing support for posts, navbar link, Radix tabs dependency, and migration/meta and DB connection configuration changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant Page as Posts Page
participant List as PostsList
participant Form as PostForm
participant API as TRPC postsRouter
participant DB as Database
participant Search as Search Service
participant Activity as Activity Log
U->>Page: Open Posts page
Page->>API: list / myDrafts (projectId)
API->>DB: Query posts
DB-->>API: Posts data
API-->>Page: Return posts
Page->>List: Render posts
U->>Page: Click "New"
Page->>Form: Open create panel (create=true)
U->>Form: Submit (publish/draft)
Form->>API: upsert(post payload)
API->>API: Permission checks (canEditProject)
API->>DB: Insert/Update Post (transaction)
DB-->>API: Post record
API->>Search: indexPost(post, project)
API->>Activity: logActivity("post", ...)
Search-->>API: Indexed
Activity-->>API: Logged
API-->>Form: Success
Form->>Page: Close & refresh lists
Page->>List: Re-fetch posts
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
scripts/post-upgrade-maintenance.ts (1)
6-8: Sanitize database names to safe identifier charset before embedding in raw SQLYou later embed databaseName into sql.raw. Replace non [a‑z0‑9_] to prevent identifier injection/parse issues.
-function getDatabaseName(ownerId: string): string { - return ownerId.toLowerCase().replace(/ /g, "_"); -} +function getDatabaseName(ownerId: string): string { + return ownerId.toLowerCase().replace(/[^a-z0-9_]/g, "_"); +}scripts/migrate-all-tenants.ts (2)
24-32: Use opsSchema for the ops DB clientgetOpsDatabase currently registers tenant schema, which is inconsistent and loses typed helpers for ops tables.
async function getOpsDatabase() { return drizzle({ connection: { url: `${process.env.DATABASE_URL}/manage`, ssl: process.env.DATABASE_SSL === "true", }, - schema, + schema: opsSchema, }); }Based on learnings.
8-10: Harden getDatabaseName to avoid unsafe identifier contentSame reasoning as the maintenance script.
-function getDatabaseName(ownerId: string): string { - return ownerId.toLowerCase().replace(/ /g, "_"); -} +function getDatabaseName(ownerId: string): string { + return ownerId.toLowerCase().replace(/[^a-z0-9_]/g, "_"); +}lib/search/index.ts (1)
90-100: Wrap all Upstash calls with runAndLogError (service boundary).Guideline requires runAndLogError for search code under lib/search/**. Wrap upsert/delete/deleteIndex/reset (and optionally search) to standardize logging and avoid noisy console logs.
Apply this pattern (example for indexPost and deleteItem; replicate in other methods):
+import { runAndLogError } from "@/lib/search/helpers"; @@ - await index.upsert({ - id: searchableItem.id, - content, - metadata: { - url: searchableItem.url, - createdAt: searchableItem.createdAt.toISOString(), - category: searchableItem.metadata?.category, - isDraft: searchableItem.metadata?.isDraft, - publishedAt: searchableItem.metadata?.publishedAt, - createdByUser: searchableItem.metadata?.createdByUser, - }, - }); + await runAndLogError("indexing post", async () => { + await index.upsert({ + id: searchableItem.id, + content, + metadata: { + url: searchableItem.url, + createdAt: searchableItem.createdAt.toISOString(), + category: searchableItem.metadata?.category, + isDraft: searchableItem.metadata?.isDraft, + publishedAt: searchableItem.metadata?.publishedAt, + createdByUser: searchableItem.metadata?.createdByUser, + }, + }); + }); @@ - async deleteItem(id: string) { - await this.index.delete(id); - } + async deleteItem(id: string) { + await runAndLogError(`removing ${id} from search index`, async () => { + await this.index.delete(id); + }); + } @@ - async deleteTenantIndex() { - try { - await this.index.deleteIndex(); - } catch (error) { - console.log( - `Index for tenant ${this.tenant} does not exist or already deleted`, - error, - ); - } - } + async deleteTenantIndex() { + await runAndLogError(`deleting index for tenant ${this.tenant}`, async () => { + await this.index.deleteIndex(); + }); + } @@ - async clearIndex() { - try { - await this.index.reset(); - } catch (error) { - console.log( - `Index for tenant ${this.tenant} could not be cleared:`, - error, - ); - } - } + async clearIndex() { + await runAndLogError(`clearing index for tenant ${this.tenant}`, async () => { + await this.index.reset(); + }); + }Based on learnings.
Also applies to: 137-149, 183-193, 226-238, 272-284, 375-377, 387-395, 397-405
🧹 Nitpick comments (13)
scripts/post-upgrade-maintenance.ts (1)
58-64: Avoid duplicating opsDb creation — reuse getOpsDatabase()Reduces config drift and keeps SSL/schema consistent.
- const opsDb = drizzle({ - connection: { - url: `${process.env.DATABASE_URL}/manage`, - ssl: process.env.DATABASE_SSL === "true", - }, - schema: opsSchema, - }); + const opsDb = await getOpsDatabase();scripts/migrate-all-tenants.ts (1)
60-66: Reuse getOpsDatabase() instead of inlining drizzle configPrevents drift and keeps SSL/schema logic centralized.
- const opsDb = drizzle({ - connection: { - url: `${process.env.DATABASE_URL}/manage`, - ssl: process.env.DATABASE_SSL === "true", - }, - schema: opsSchema, - }); + const opsDb = await getOpsDatabase();drizzle/schema.ts (2)
285-301: Add indexes for common Post queries (projectId, isDraft, publishedAt/updatedAt)List/listAll/myDrafts/published lists will benefit from indexes. Without them, large tables will full-scan.
Two options:
- Add a follow-up migration with indexes (preferred for deployed envs).
- Or amend the current migration if not yet applied anywhere.
Example SQL (new migration):
CREATE INDEX IF NOT EXISTS idx_post_project ON "Post"("projectId"); CREATE INDEX IF NOT EXISTS idx_post_project_isdraft ON "Post"("projectId","isDraft"); CREATE INDEX IF NOT EXISTS idx_post_project_published_at ON "Post"("projectId","publishedAt" DESC) WHERE "isDraft" = false; CREATE INDEX IF NOT EXISTS idx_post_project_updated_at ON "Post"("projectId","updatedAt" DESC);
285-301: Consider a pgEnum or CHECK constraint for categoryRouter constrains category to a small set; enforce at DB level to prevent invalid writes.
Example:
DO $$ BEGIN CREATE TYPE post_category AS ENUM ('announcement','fyi','question'); EXCEPTION WHEN duplicate_object THEN NULL END $$; ALTER TABLE "Post" ALTER COLUMN "category" TYPE post_category USING "category"::post_category;drizzle/0008_soft_serpent_society.sql (1)
1-16: Add indexes to support Post queriesImprove query plans for filters by projectId/isDraft and ordering by publishedAt/updatedAt.
If safe to amend this migration, append:
CREATE INDEX IF NOT EXISTS idx_post_project ON "Post"("projectId"); CREATE INDEX IF NOT EXISTS idx_post_project_isdraft ON "Post"("projectId","isDraft"); CREATE INDEX IF NOT EXISTS idx_post_project_published_at ON "Post"("projectId","publishedAt" DESC) WHERE "isDraft" = false; CREATE INDEX IF NOT EXISTS idx_post_project_updated_at ON "Post"("projectId","updatedAt" DESC);Otherwise, create a new 0009_* migration with the above statements.
trpc/routers/search.ts (1)
66-69: Use TRPCError with FORBIDDEN instead of generic ErrorAligns with tRPC conventions and consistent error handling.
- if (!ctx.isOrgAdmin) { - throw new Error("Only organization admins can reindex content"); - } + if (!ctx.isOrgAdmin) { + throw new TRPCError({ code: "FORBIDDEN", message: "Only organization admins can reindex content" }); + }Add import:
- import type { inferRouterOutputs } from "@trpc/server"; + import type { inferRouterOutputs } from "@trpc/server"; + import { TRPCError } from "@trpc/server";As per coding guidelines.
lib/search/helpers.ts (1)
158-169: Reduce duplication across fetch-and-index helpers.indexTaskListWithProjectFetch, indexEventWithProjectFetch, and indexPostWithProjectFetch share structure. Consider a small generic util to fetch project and invoke a provided indexer to cut repetition.
app/(dashboard)/[tenant]/projects/[projectId]/posts/page.tsx (1)
65-94: Optional: prefetch both tab queries to avoid first-switch latency.You can prefetch the inactive tab in background (keep enabled gating) using queryClient.prefetchQuery on mount to eliminate the loading hit when switching.
components/form/post.tsx (2)
112-116: Tighten category typing.Avoid any; constrain onValueChange to the union type to prevent invalid values.
- onValueChange={(value: any) => setCategory(value)} + onValueChange={(value: "announcement" | "fyi" | "question") => setCategory(value)}
152-176: Optional: disable actions while saving.Prevent double submissions and reflect pending state by disabling buttons when upsertPost.isPending is true.
- <Button + <Button type="button" variant="outline" + disabled={upsertPost.isPending} ... /> - <Button + <Button type="button" + disabled={upsertPost.isPending} ... > {item ? "Update" : "Publish"} </Button>lib/search/index.ts (1)
252-253: Deep-link post results to the specific post.Align with task/event URLs by including the post id; otherwise users land on the generic posts page.
Apply this change:
- url: `/${this.orgSlug}/projects/${projectData.id}/posts`, + url: `/${this.orgSlug}/projects/${projectData.id}/posts?post=${postData.id}`,trpc/routers/posts.ts (1)
306-316: Add a "post" case in mention notifications; link to the specific post.sendMentionNotifications lacks a "post" case; fallback message/target is generic. Add a branch and deep-link.
In lib/utils/mentionNotifications.ts:
switch (context.type) { + case "post": + message = `${fromUserName} mentioned you in post "${context.entityName}"`; + target = `/${context.orgSlug}/projects/${context.projectId}/posts?post=${context.entityId}`; + break;Based on learnings.
Also applies to: 341-351
components/core/search-panel.tsx (1)
176-189: Hide status filter for posts.Posts don’t have a status; offering status options is confusing. Treat like events (no statuses).
Apply:
switch (typeFilter) { case "project": return ["active", "archived"]; case "task": return ["todo", "done"]; case "tasklist": return ["active", "archived"]; case "event": return []; // Events don't have status + case "post": + return []; // Posts don't have status default: return ["active", "archived", "todo", "done"]; // All statuses }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (18)
app/(dashboard)/[tenant]/projects/[projectId]/posts/page.tsx(1 hunks)components/core/search-panel.tsx(12 hunks)components/form/post.tsx(1 hunks)components/layout/navbar.tsx(1 hunks)components/project/posts/posts-list.tsx(1 hunks)components/ui/tabs.tsx(1 hunks)drizzle/0008_soft_serpent_society.sql(1 hunks)drizzle/meta/0008_snapshot.json(1 hunks)drizzle/meta/_journal.json(1 hunks)drizzle/schema.ts(2 hunks)lib/search/helpers.ts(2 hunks)lib/search/index.ts(4 hunks)package.json(2 hunks)scripts/migrate-all-tenants.ts(1 hunks)scripts/post-upgrade-maintenance.ts(1 hunks)trpc/routers/_app.ts(2 hunks)trpc/routers/posts.ts(1 hunks)trpc/routers/search.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Never add comments that merely restate what the code does; avoid obvious, redundant comments
Write comments to explain WHY, not WHAT; add context such as constraints, decisions, performance, gotchas, or references
Remove or update outdated comments; keep comments synchronized with code changes
Prefer self-documenting code over comments: better naming, smaller functions, constants over magic values, and expressive types
Use TypeScript types/annotations to document data structures and contracts; JSDoc for public API documentation where needed
Files:
app/(dashboard)/[tenant]/projects/[projectId]/posts/page.tsxcomponents/ui/tabs.tsxtrpc/routers/_app.tscomponents/project/posts/posts-list.tsxcomponents/core/search-panel.tsxlib/search/helpers.tsdrizzle/schema.tstrpc/routers/posts.tscomponents/layout/navbar.tsxcomponents/form/post.tsxscripts/migrate-all-tenants.tstrpc/routers/search.tsscripts/post-upgrade-maintenance.tslib/search/index.ts
app/{(api),(dashboard),(legal)}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Organize Next.js routes within the defined App Router groups: (api), (dashboard), and (legal)
Files:
app/(dashboard)/[tenant]/projects/[projectId]/posts/page.tsx
components/ui/**
📄 CodeRabbit inference engine (CLAUDE.md)
UI components are excluded from linting as configured; do not rely on linter coverage in components/ui
Files:
components/ui/tabs.tsx
trpc/routers/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use protectedProcedure for authenticated tRPC endpoints
Files:
trpc/routers/_app.tstrpc/routers/posts.tstrpc/routers/search.ts
{lib/search/**/*.ts,trpc/routers/search.ts}
📄 CodeRabbit inference engine (CLAUDE.md)
Use runAndLogError utility for error handling in search-related code
Files:
lib/search/helpers.tstrpc/routers/search.tslib/search/index.ts
lib/search/helpers.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Keep shared search indexing and helper utilities in lib/search/helpers.ts
Files:
lib/search/helpers.ts
drizzle/schema.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Keep the main application database schema in drizzle/schema.ts (Drizzle ORM)
Files:
drizzle/schema.ts
lib/search/index.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Implement centralized search operations in lib/search/index.ts (SearchService)
Files:
lib/search/index.ts
🧠 Learnings (3)
📚 Learning: 2025-10-24T11:41:23.518Z
Learnt from: CR
PR: techulus/manage#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-24T11:41:23.518Z
Learning: Applies to lib/search/helpers.ts : Keep shared search indexing and helper utilities in lib/search/helpers.ts
Applied to files:
lib/search/helpers.tslib/search/index.ts
📚 Learning: 2025-10-24T11:41:23.518Z
Learnt from: CR
PR: techulus/manage#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-24T11:41:23.518Z
Learning: Applies to ops/drizzle/schema.ts : Keep the operations database schema in ops/drizzle/schema.ts (separate database)
Applied to files:
scripts/post-upgrade-maintenance.ts
📚 Learning: 2025-10-24T11:41:23.518Z
Learnt from: CR
PR: techulus/manage#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-24T11:41:23.518Z
Learning: Applies to lib/search/index.ts : Implement centralized search operations in lib/search/index.ts (SearchService)
Applied to files:
lib/search/index.ts
🧬 Code graph analysis (8)
app/(dashboard)/[tenant]/projects/[projectId]/posts/page.tsx (5)
components/layout/page-title.tsx (1)
PageTitle(16-63)components/core/section.tsx (1)
PageSection(3-48)components/project/posts/posts-list.tsx (1)
PostsList(56-237)components/core/empty-state.tsx (1)
EmptyState(3-38)components/core/panel.tsx (1)
Panel(8-39)
trpc/routers/_app.ts (1)
trpc/routers/posts.ts (1)
postsRouter(14-356)
components/project/posts/posts-list.tsx (8)
lib/utils/error.ts (1)
displayMutationError(5-26)drizzle/schema.ts (2)
post(285-301)user(12-23)components/core/user-avatar.tsx (1)
UserAvatar(5-25)components/core/panel.tsx (1)
Panel(8-39)components/layout/page-title.tsx (1)
PageTitle(16-63)components/form/button.tsx (1)
DeleteButton(11-69)components/core/html-view.tsx (1)
HtmlPreview(8-23)components/project/comment/comments-section.tsx (1)
CommentsSection(12-46)
lib/search/helpers.ts (4)
lib/search/index.ts (2)
indexPost(240-284)search(286-373)drizzle/schema.ts (2)
post(285-301)project(34-45)lib/error.ts (1)
runAndLogError(1-9)drizzle/types.ts (1)
Database(15-15)
trpc/routers/posts.ts (6)
trpc/init.ts (2)
createTRPCRouter(51-51)protectedProcedure(55-76)lib/permissions/index.ts (2)
canViewProject(68-86)canEditProject(48-66)drizzle/schema.ts (1)
post(285-301)lib/activity/index.ts (1)
logActivity(17-46)lib/search/helpers.ts (2)
deleteSearchItem(113-121)indexPostWithProjectFetch(158-169)lib/utils/mentionNotifications.ts (1)
sendMentionNotifications(19-82)
components/form/post.tsx (1)
lib/utils/error.ts (1)
displayMutationError(5-26)
trpc/routers/search.ts (3)
trpc/init.ts (2)
createTRPCRouter(51-51)protectedProcedure(55-76)drizzle/schema.ts (1)
post(285-301)lib/error.ts (1)
runAndLogError(1-9)
lib/search/index.ts (1)
drizzle/schema.ts (2)
post(285-301)project(34-45)
🪛 Biome (2.1.2)
components/project/posts/posts-list.tsx
[error] 103-106: Static Elements should not be interactive.
To add interactivity such as a mouse or key event listener to a static element, give the element an appropriate role value.
(lint/a11y/noStaticElementInteractions)
[error] 103-106: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.
(lint/a11y/useKeyWithClickEvents)
🔇 Additional comments (14)
package.json (2)
43-43: UI tabs dependency looks goodAdopting @radix-ui/react-tabs is fine and consistent with your UI stack. No concerns.
86-86: Canary dependency for resend — confirm intentionality and add test coverageThe canary version introduces pre-release stability risk, though the main v4→v6 breaking change (attachment field renaming) doesn't apply to current usage, which relies on
reactandtextparameters. Confirm this canary version is intentional and consider pinning to a stable release. Additionally, add test coverage for the email sending functionality in:
app/(api)/api/jobs/account/mark-for-deletion/route.tsapp/(api)/api/jobs/daily-summary/route.tsapp/(api)/api/jobs/user/mark-for-deletion/route.tsapp/(api)/api/webhook/auth/route.tsdrizzle/meta/_journal.json (1)
60-67: Journal entry addition looks consistentNew entry aligns with 0008 migration tag. No action.
trpc/routers/_app.ts (1)
5-5: Exposing posts router in appRouter — good integrationRoutes are now accessible under appRouter.posts. Matches the new CRUD router.
Also applies to: 18-19
drizzle/schema.ts (1)
303-312: Relations look correctpostRelations and projectRelations.posts wiring is coherent.
Also applies to: 55-56
trpc/routers/search.ts (2)
133-146: Post indexing path looks correctNon-draft posts fetched with project relation; errors wrapped via runAndLogError; counts included.
Also applies to: 149-156
56-60: Access filter now covers posts — goodFiltering by accessibleProjectIds for non-project types is correct and includes posts.
components/layout/navbar.tsx (1)
97-103: Posts nav item matches existing route patterns; LGTM.Active-state detection and href mirror Tasks/Events. No issues.
lib/search/helpers.ts (1)
148-156: Search helper aligns with error-handling guidance; LGTM.Uses runAndLogError and typed inputs correctly.
Based on learnings
drizzle/meta/0008_snapshot.json (1)
514-633: Post table snapshot looks consistent with schema; LGTM.PK, defaults, and FKs match drizzle/schema.ts definitions.
components/ui/tabs.tsx (1)
10-55: Radix Tabs wrappers are clean and idiomatic; LGTM.Forwarded refs + class merging look good.
lib/search/index.ts (2)
2-2: "post" type wiring looks consistent.Type unions and allowedTypes now include "post". Good parity with other entities.
Also applies to: 8-8, 289-289, 309-313, 358-360
309-313: Filter field scope is correct—no changes needed.The verification confirms that filters targeting content fields (like
type) should use unprefixed field names, exactly as implemented. Document indexing storestype,projectId, andstatusin thecontentobject (not metadata), and the filter expressionstype = '${options.type}'and similar are correctly scoped to those content fields. No migration to metadata is required.components/core/search-panel.tsx (1)
40-41: UI additions for "post" look good.Icon, labels, type unions, placeholder, grouping, and filter option are consistent with other types.
Also applies to: 60-62, 77-79, 112-114, 191-195, 211-215, 230-231, 320-323, 456-457
components/form/post.tsx
Outdated
| const handleSubmit = (isDraft: boolean) => (formData: FormData) => { | ||
| const content = formData.get("content") as string; | ||
| const metadata = formData.get("metadata"); | ||
|
|
||
| upsertPost.mutate({ | ||
| title, | ||
| content, | ||
| metadata: metadata ? JSON.parse(metadata as string) : undefined, | ||
| category, | ||
| projectId: +projectId!, | ||
| id: item?.id ?? undefined, | ||
| isDraft, | ||
| }); | ||
| }; |
There was a problem hiding this comment.
Guard JSON.parse to prevent runtime crashes on invalid metadata.
JSON parsing can throw; add a try/catch and fail fast instead of crashing the handler.
- const metadata = formData.get("metadata");
-
- upsertPost.mutate({
+ const metadataRaw = formData.get("metadata");
+ let parsedMetadata: unknown | undefined = undefined;
+ if (typeof metadataRaw === "string" && metadataRaw.trim()) {
+ try {
+ parsedMetadata = JSON.parse(metadataRaw);
+ } catch (e) {
+ console.error("Invalid metadata JSON", e);
+ return; // optionally show a toast here
+ }
+ }
+
+ upsertPost.mutate({
title,
content,
- metadata: metadata ? JSON.parse(metadata as string) : undefined,
+ metadata: parsedMetadata,
category,
projectId: +projectId!,
id: item?.id ?? undefined,
isDraft,
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleSubmit = (isDraft: boolean) => (formData: FormData) => { | |
| const content = formData.get("content") as string; | |
| const metadata = formData.get("metadata"); | |
| upsertPost.mutate({ | |
| title, | |
| content, | |
| metadata: metadata ? JSON.parse(metadata as string) : undefined, | |
| category, | |
| projectId: +projectId!, | |
| id: item?.id ?? undefined, | |
| isDraft, | |
| }); | |
| }; | |
| const handleSubmit = (isDraft: boolean) => (formData: FormData) => { | |
| const content = formData.get("content") as string; | |
| const metadataRaw = formData.get("metadata"); | |
| let parsedMetadata: unknown | undefined = undefined; | |
| if (typeof metadataRaw === "string" && metadataRaw.trim()) { | |
| try { | |
| parsedMetadata = JSON.parse(metadataRaw); | |
| } catch (e) { | |
| console.error("Invalid metadata JSON", e); | |
| return; // optionally show a toast here | |
| } | |
| } | |
| upsertPost.mutate({ | |
| title, | |
| content, | |
| metadata: parsedMetadata, | |
| category, | |
| projectId: +projectId!, | |
| id: item?.id ?? undefined, | |
| isDraft, | |
| }); | |
| }; |
🤖 Prompt for AI Agents
In components/form/post.tsx around lines 80 to 93, the handler calls JSON.parse
on metadata without guarding against invalid JSON which can throw; wrap the
metadata parse in a try/catch, validate metadata is a string before parsing, and
on parse failure fail fast by returning early and surfacing an error (e.g., set
a form error or show an alert) instead of calling upsertPost.mutate; ensure
successful parsing passes the parsed object to mutate and that the catch avoids
throwing so the handler doesn't crash the app.
| const checkDb = await opsDb.execute( | ||
| sql`SELECT 1 FROM pg_database WHERE datname = ${databaseName}`, | ||
| ); | ||
|
|
||
| if (checkDb.count === 0) { | ||
| return { success: true, skipped: true }; |
There was a problem hiding this comment.
DB existence check is incorrect (uses .count) — may mis-handle missing DBs
Same bug as in maintenance script; check array length/rowCount.
- const checkDb = await opsDb.execute(
- sql`SELECT 1 FROM pg_database WHERE datname = ${databaseName}`,
- );
-
- if (checkDb.count === 0) {
+ const checkDb = await opsDb.execute(
+ sql`SELECT 1 FROM pg_database WHERE datname = ${databaseName}`,
+ );
+ const dbExists =
+ Array.isArray(checkDb) ? checkDb.length > 0 : (checkDb as any)?.rowCount > 0;
+ if (!dbExists) {
return { success: true, skipped: true };
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const checkDb = await opsDb.execute( | |
| sql`SELECT 1 FROM pg_database WHERE datname = ${databaseName}`, | |
| ); | |
| if (checkDb.count === 0) { | |
| return { success: true, skipped: true }; | |
| const checkDb = await opsDb.execute( | |
| sql`SELECT 1 FROM pg_database WHERE datname = ${databaseName}`, | |
| ); | |
| const dbExists = | |
| Array.isArray(checkDb) ? checkDb.length > 0 : (checkDb as any)?.rowCount > 0; | |
| if (!dbExists) { | |
| return { success: true, skipped: true }; |
🤖 Prompt for AI Agents
In scripts/migrate-all-tenants.ts around lines 68 to 73, the DB existence check
incorrectly inspects checkDb.count which is not provided by the query result;
change the check to use the actual row count (e.g., checkDb.rowCount or
checkDb.rows.length depending on the DB client) and return skipped when that
count is 0. Ensure you use the correct property for the opsDb.execute result
type, and update any typing if necessary.
| const checkDb = await opsDb.execute( | ||
| sql`SELECT 1 FROM pg_database WHERE datname = ${databaseName}`, | ||
| ); | ||
|
|
||
| if (checkDb.count === 0) { | ||
| return { success: true, skipped: true }; |
There was a problem hiding this comment.
DB existence check is incorrect (uses non-existent .count) — can skip wrongly or always proceed
drizzle.execute for postgres-js returns rows (array), not an object with count. Use length/rowCount to determine existence.
Apply:
- const checkDb = await opsDb.execute(
- sql`SELECT 1 FROM pg_database WHERE datname = ${databaseName}`,
- );
-
- if (checkDb.count === 0) {
+ const checkDb = await opsDb.execute(
+ sql`SELECT 1 FROM pg_database WHERE datname = ${databaseName}`,
+ );
+ const dbExists =
+ Array.isArray(checkDb) ? checkDb.length > 0 : (checkDb as any)?.rowCount > 0;
+ if (!dbExists) {
return { success: true, skipped: true };
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const checkDb = await opsDb.execute( | |
| sql`SELECT 1 FROM pg_database WHERE datname = ${databaseName}`, | |
| ); | |
| if (checkDb.count === 0) { | |
| return { success: true, skipped: true }; | |
| const checkDb = await opsDb.execute( | |
| sql`SELECT 1 FROM pg_database WHERE datname = ${databaseName}`, | |
| ); | |
| const dbExists = | |
| Array.isArray(checkDb) ? checkDb.length > 0 : (checkDb as any)?.rowCount > 0; | |
| if (!dbExists) { | |
| return { success: true, skipped: true }; |
🤖 Prompt for AI Agents
In scripts/post-upgrade-maintenance.ts around lines 66 to 71, the code checks
database existence using checkDb.count but drizzle.execute for postgres-js
returns an array of rows (not an object with count), so the condition can be
wrong; replace the .count check with a proper rows length check (e.g., treat the
result as an array and use checkDb.length === 0 or destructure rows and check
rows.length) and return skipped when length is 0, otherwise proceed.
| publishedAt: isDraft ? null : new Date(), | ||
| }; |
There was a problem hiding this comment.
Don’t overwrite publishedAt on every edit. Only set on draft→published; clear on published→draft.
Current logic resets publish date on any non-draft update, losing original publish time.
Apply this update-branch change:
- const postData = {
+ // Build base data; publishedAt patched below for updates
+ const postData = {
title,
content,
metadata,
category,
isDraft,
projectId,
- publishedAt: isDraft ? null : new Date(),
+ publishedAt: isDraft ? null : new Date(), // used for create only
};And in the update path (inside if (id) { ... }):
- const updatedPost = await ctx.db
+ // Preserve/transition publish time
+ const nextPublishedAt =
+ existing.isDraft && !isDraft
+ ? new Date()
+ : !existing.isDraft && isDraft
+ ? null
+ : existing.publishedAt;
+
+ const updatedPost = await ctx.db
.update(post)
- .set(postData)
+ .set({ ...postData, publishedAt: nextPublishedAt })
.where(eq(post.id, id))
.returning();Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In trpc/routers/posts.ts around lines 275-276, the current assignment always
sets publishedAt to new Date() for any non-draft save which overwrites the
original publish time; change this so that when creating/updating you only set
publishedAt to now if the post is transitioning from draft→published, preserve
existing.publishedAt for edits that keep it published, and set publishedAt to
null if transitioning from published→draft. In the update branch (inside if (id)
{ ... }) fetch the existing post, compare existing.isDraft vs incoming isDraft,
then: if existing.isDraft && !isDraft set publishedAt = new Date(); else if
(!existing.isDraft && isDraft) set publishedAt = null; else keep publishedAt =
existing.publishedAt.
| if (updatedPost?.[0] && !isDraft) { | ||
| await indexPostWithProjectFetch(ctx.db, ctx.search, updatedPost[0]); | ||
|
|
||
| if (content) { | ||
| await sendMentionNotifications(content, { | ||
| type: "post", | ||
| entityName: title, | ||
| entityId: id, | ||
| projectId, | ||
| orgSlug: ctx.orgSlug, | ||
| fromUserId: ctx.userId, | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
De-index when unpublishing (published → draft).
When a published post becomes draft, remove it from the search index to avoid stale public results.
Apply this after the update/insert operations:
- if (updatedPost?.[0] && !isDraft) {
+ if (updatedPost?.[0] && !isDraft) {
await indexPostWithProjectFetch(ctx.db, ctx.search, updatedPost[0]);
@@
- }
+ } else if (updatedPost?.[0] && isDraft) {
+ await deleteSearchItem(ctx.search, `post-${updatedPost[0].id}`, "post");
+ }- if (newPost?.[0] && !isDraft) {
+ if (newPost?.[0] && !isDraft) {
await indexPostWithProjectFetch(ctx.db, ctx.search, newPost[0]);
- }
+ } // drafts are never indexedAlso applies to: 338-351
🤖 Prompt for AI Agents
In trpc/routers/posts.ts around lines 303-316 (and similarly 338-351), the code
indexes posts when updated but does not remove posts from the search index when
they transition from published to draft; update the post handling so that after
the update/insert you check the previous published state vs the new isDraft
flag: if the post was previously published and is now a draft, call the search
de-index function (e.g., removePostFromIndex or the counterpart to
indexPostWithProjectFetch) with the post id/project context to remove it from
the index; if the post is published (previously draft → published or still
published) keep the existing indexPostWithProjectFetch call; ensure this logic
is applied in both code paths at lines 303-316 and 338-351 so unpublished posts
are removed from search results.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
components/project/posts/posts-list.tsx (1)
85-88: A11y issue previously flagged.This accessibility issue (interactive div should be a button) was already identified in a previous review. Please refer to the earlier comment for the proposed fix.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
app/(dashboard)/[tenant]/projects/[projectId]/posts/page.tsx(1 hunks)components/project/posts/posts-list.tsx(1 hunks)drizzle/types.ts(3 hunks)lib/activity/index.ts(1 hunks)lib/utils/mentionNotifications.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Never add comments that merely restate what the code does; avoid obvious, redundant comments
Write comments to explain WHY, not WHAT; add context such as constraints, decisions, performance, gotchas, or references
Remove or update outdated comments; keep comments synchronized with code changes
Prefer self-documenting code over comments: better naming, smaller functions, constants over magic values, and expressive types
Use TypeScript types/annotations to document data structures and contracts; JSDoc for public API documentation where needed
Files:
drizzle/types.tsapp/(dashboard)/[tenant]/projects/[projectId]/posts/page.tsxlib/activity/index.tscomponents/project/posts/posts-list.tsxlib/utils/mentionNotifications.ts
app/{(api),(dashboard),(legal)}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Organize Next.js routes within the defined App Router groups: (api), (dashboard), and (legal)
Files:
app/(dashboard)/[tenant]/projects/[projectId]/posts/page.tsx
🧬 Code graph analysis (3)
drizzle/types.ts (1)
drizzle/schema.ts (1)
post(285-301)
app/(dashboard)/[tenant]/projects/[projectId]/posts/page.tsx (5)
components/layout/page-title.tsx (1)
PageTitle(16-63)components/core/section.tsx (1)
PageSection(3-48)components/project/posts/posts-list.tsx (1)
PostsList(40-232)components/core/empty-state.tsx (1)
EmptyState(3-38)components/core/panel.tsx (1)
Panel(8-39)
components/project/posts/posts-list.tsx (9)
drizzle/types.ts (1)
PostWithCreator(73-75)lib/utils/error.ts (1)
displayMutationError(5-26)drizzle/schema.ts (2)
post(285-301)user(12-23)components/core/user-avatar.tsx (1)
UserAvatar(5-25)components/core/panel.tsx (1)
Panel(8-39)components/layout/page-title.tsx (1)
PageTitle(16-63)components/form/button.tsx (1)
DeleteButton(11-69)components/core/html-view.tsx (1)
HtmlPreview(8-23)components/project/comment/comments-section.tsx (1)
CommentsSection(12-46)
🪛 Biome (2.1.2)
components/project/posts/posts-list.tsx
[error] 85-88: Static Elements should not be interactive.
To add interactivity such as a mouse or key event listener to a static element, give the element an appropriate role value.
(lint/a11y/noStaticElementInteractions)
[error] 85-88: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.
(lint/a11y/useKeyWithClickEvents)
🔇 Additional comments (2)
lib/activity/index.ts (1)
26-26: LGTM!The addition of "post" to the activity type union correctly extends activity logging to support the new Post entity.
drizzle/types.ts (1)
9-9: LGTM!The Post type definitions follow the established pattern used for other entities in the codebase and correctly align with the new post schema.
Also applies to: 26-26, 73-75
|
|
||
| const { data: project } = useQuery( | ||
| trpc.projects.getProjectById.queryOptions({ | ||
| id: +projectId!, |
There was a problem hiding this comment.
Validate projectId before using non-null assertions.
The pattern +projectId! is used repeatedly (lines 31, 37, 44, 74, 86) to coerce and assert the projectId parameter. This bypasses TypeScript's safety checks and could lead to runtime errors if projectId is undefined or not a valid numeric string.
Consider validating the parameter early in the component:
export default function Posts() {
const { projectId, tenant } = useParams();
+
+ if (!projectId || Array.isArray(projectId)) {
+ throw new Error("Invalid project ID");
+ }
+
+ const numericProjectId = Number(projectId);
+ if (Number.isNaN(numericProjectId)) {
+ throw new Error("Project ID must be a number");
+ }
+
const [create, setCreate] = useQueryState(
"create",
parseAsBoolean.withDefault(false),
);Then use numericProjectId throughout instead of +projectId!.
Also applies to: 37-37, 44-44, 74-74, 86-86
🤖 Prompt for AI Agents
In app/(dashboard)/[tenant]/projects/[projectId]/posts/page.tsx around lines
31-86, the code uses the non-null assertion/coercion pattern `+projectId!` in
multiple places which bypasses TypeScript safety; validate and normalize
projectId once at the top (e.g., check that projectId exists, parse it to a
number with Number() or parseInt, verify it is not NaN and positive) and assign
to a local const like numericProjectId; if validation fails, return an
appropriate fallback (throw, return notFound(), or render an error) so runtime
errors are avoided, then replace all `+projectId!` occurrences with the
validated numericProjectId.
| <div className="flex items-center gap-2"> | ||
| <Badge | ||
| className={ | ||
| categoryColors[post.category as keyof typeof categoryColors] |
There was a problem hiding this comment.
Guard against undefined category colors.
The type assertions at lines 108 and 198 assume all post categories exist in categoryColors. If a post has a category not defined in the categoryColors object (lines 28-33), the lookup will return undefined, resulting in missing styles.
Consider adding a fallback color:
+const defaultCategoryColor = "bg-gray-100 text-gray-800 dark:bg-gray-900 dark:text-gray-200";
+
const categoryColors = {
announcement: "bg-blue-100 text-blue-800 dark:bg-blue-900 dark:text-blue-200",
fyi: "bg-green-100 text-green-800 dark:bg-green-900 dark:text-green-200",
question:
"bg-purple-100 text-purple-800 dark:bg-purple-900 dark:text-purple-200",
};Then use:
<Badge
className={
- categoryColors[post.category as keyof typeof categoryColors]
+ categoryColors[post.category as keyof typeof categoryColors] ?? defaultCategoryColor
}
variant="secondary"
>Apply the same pattern at line 197-199.
Also applies to: 197-199
🤖 Prompt for AI Agents
In components/project/posts/posts-list.tsx around lines 108 and 197-199, the
code assumes every post.category exists in categoryColors which can return
undefined and drop styles; update both lookups to use a safe fallback (e.g.,
const color = categoryColors[post.category as keyof typeof categoryColors] ??
fallbackColor) and then use that color variable in place of the direct lookup so
undefined categories fall back to a default color.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (5)
lib/utils/mentionNotifications.ts (1)
68-71: LGTM! Post mention handling implemented correctly.The post case follows the established pattern for other entity types, with appropriate message formatting and target URL construction.
components/form/post.tsx (2)
87-87: LGTM! Form ref implementation is correct.The component properly uses a ref instead of
document.querySelector, which improves reliability and testability. The ref is correctly typed and null-checked before use.Also applies to: 105-105, 167-167, 179-179
89-102: Guard JSON.parse to prevent runtime crashes.JSON parsing can throw on invalid input, causing the mutation to fail without proper error handling. Add try-catch to handle malformed metadata gracefully.
Apply this diff to add error handling:
const savePost = (formData: FormData, isDraft = false) => { const content = formData.get("content") as string; - const metadata = formData.get("metadata"); + const metadataRaw = formData.get("metadata"); + let parsedMetadata: unknown | undefined = undefined; + + if (typeof metadataRaw === "string" && metadataRaw.trim()) { + try { + parsedMetadata = JSON.parse(metadataRaw); + } catch (e) { + console.error("Invalid metadata JSON", e); + displayMutationError({ + message: "Invalid metadata format", + } as any); + return; + } + } upsertPost.mutate({ title, content, - metadata: metadata ? JSON.parse(metadata as string) : undefined, + metadata: parsedMetadata, category, projectId: +projectId!, id: item?.id ?? undefined, isDraft, }); };components/project/posts/posts-list.tsx (2)
86-89: LGTM! Accessibility issue resolved.The interactive element is now a semantic
<button>with proper type, styling, and keyboard accessibility. This addresses the a11y concerns from the previous review.
108-115: Add fallback for undefined category colors.If a post has a category not defined in
categoryColors(lines 29-34), the lookup returnsundefined, resulting in missing styles. Add a fallback to handle unexpected categories.Apply this diff:
+const defaultCategoryColor = "bg-gray-100 text-gray-800 dark:bg-gray-900 dark:text-gray-200"; + const categoryColors = { announcement: "bg-blue-100 text-blue-800 dark:bg-blue-900 dark:text-blue-200", fyi: "bg-green-100 text-green-800 dark:bg-green-900 dark:text-green-200", question: "bg-purple-100 text-purple-800 dark:bg-purple-900 dark:text-purple-200", };Then update both usages:
<Badge className={ - categoryColors[post.category as keyof typeof categoryColors] + categoryColors[post.category as keyof typeof categoryColors] ?? defaultCategoryColor } variant="secondary" >Also apply the same pattern at lines 197-206.
🧹 Nitpick comments (1)
components/form/post.tsx (1)
121-134: Replaceanywith proper type for better type safety.The
onValueChangehandler usesany, which bypasses type checking. Use the defined category union type instead.Apply this diff:
<Select name="category" value={category} - onValueChange={(value: any) => setCategory(value)} + onValueChange={(value: "announcement" | "fyi" | "question") => setCategory(value)} >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
components/form/post.tsx(1 hunks)components/project/posts/posts-list.tsx(1 hunks)lib/utils/mentionNotifications.ts(2 hunks)trpc/routers/posts.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- trpc/routers/posts.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Never add comments that merely restate what the code does; avoid obvious, redundant comments
Write comments to explain WHY, not WHAT; add context such as constraints, decisions, performance, gotchas, or references
Remove or update outdated comments; keep comments synchronized with code changes
Prefer self-documenting code over comments: better naming, smaller functions, constants over magic values, and expressive types
Use TypeScript types/annotations to document data structures and contracts; JSDoc for public API documentation where needed
Files:
components/project/posts/posts-list.tsxcomponents/form/post.tsxlib/utils/mentionNotifications.ts
🧬 Code graph analysis (2)
components/project/posts/posts-list.tsx (9)
drizzle/types.ts (1)
PostWithCreator(73-75)lib/utils/error.ts (1)
displayMutationError(5-26)drizzle/schema.ts (2)
post(285-301)user(12-23)components/core/user-avatar.tsx (1)
UserAvatar(5-25)components/core/panel.tsx (1)
Panel(8-39)components/layout/page-title.tsx (1)
PageTitle(16-63)components/form/button.tsx (1)
DeleteButton(11-69)components/core/html-view.tsx (1)
HtmlPreview(8-23)components/project/comment/comments-section.tsx (1)
CommentsSection(12-46)
components/form/post.tsx (1)
lib/utils/error.ts (1)
displayMutationError(5-26)
🔇 Additional comments (1)
components/project/posts/posts-list.tsx (1)
41-234: Well-structured component with proper state management.The component demonstrates good practices:
- Proper React Query usage with conditional fetching
- Correct cache invalidation on mutations
- Authorization checks before allowing edit/delete
- Clean separation between viewing and editing modes
Summary by CodeRabbit
New Features
Chores