Implement surrogate UUID keys on posts#190
Conversation
|
Warning Review limit reached
Next review available in: 10 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughThe change replaces slug-based post storage with UUID-based, locale-and-branch records, adds shared attachment storage, and updates API queries, worker synchronization, achievement metrics, relations, migrations, mocks, and tests. ChangesPosts schema and persistence refactor
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant GitHub
participant SyncPostProcessor
participant S3
participant Database
GitHub->>SyncPostProcessor: provide post and attachment content
SyncPostProcessor->>Database: load existing attachment keys
SyncPostProcessor->>S3: upload changed attachments
SyncPostProcessor->>Database: insert attachment metadata
SyncPostProcessor->>Database: insert posts and receive post IDs
SyncPostProcessor->>Database: insert author, tag, and attachment relations
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
packages/db/drizzle/20260711154730_confused_green_goblin/migration.sql (1)
47-53: 🚀 Performance & Scalability | 🔵 TrivialAdd indexes for FK columns not covered by composite PKs.
PostgreSQL does not automatically index FK columns. The composite PKs on
post_authors(post_id, author_slug),post_tags(post_id, tag), andpost_attachments(post_id, attachment_key)only index the leadingpost_idcolumn. Queries that filter or join on the second column (e.g.,post_authors.author_slugused in the profiles query, orpost_tags.tagfor tag-based lookups) will full-scan. Similarly,posts.collection_slugandposts.group_idhave FKs but no index.Consider adding:
CREATE INDEXonpost_authors(author_slug)— used inprofiles.tsjoineq(postAuthors.authorSlug, profiles.slug)CREATE INDEXonpost_tags(tag)— for tag-based post lookupsCREATE INDEXonposts(collection_slug)— for collection chapter queriesCREATE INDEXonposts(group_id)— for group-based lookupsCREATE INDEXonposts(published_at)— for ordering/filtering published posts🤖 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/db/drizzle/20260711154730_confused_green_goblin/migration.sql` around lines 47 - 53, Add indexes in the migration for the uncovered foreign-key and query columns: post_authors.author_slug, post_tags.tag, posts.collection_slug, posts.group_id, and posts.published_at. Use distinct, descriptive index names and preserve the existing foreign-key constraints.
🤖 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.
Inline comments:
In `@apps/api/src/routes/content/post.test.ts`:
- Around line 244-245: Rename the second test around db.query.posts.findFirst
and the "spanish-only-post" URL to clearly identify its locale-specific
not-found behavior, while leaving the first 404 test name unchanged.
In `@apps/worker/src/tasks/grant-author-achievements/processor.ts`:
- Around line 12-21: Remove the unused drizzle-orm imports max and exists from
the import list at the top of processor.ts, leaving all referenced imports
unchanged.
In `@apps/worker/src/tasks/sync-post/processor.ts`:
- Around line 359-368: Update the delete operation in the post-processing
transaction to call tx.delete instead of db.delete, preserving the existing
posts table and where conditions so the deletion remains part of the transaction
and can be rolled back atomically.
- Around line 311-316: Update the attachments insert using the
db.insert(attachments) call to ignore conflicts on the attachmentKey primary
key, using onConflictDoNothing or the repository’s equivalent. Preserve the
existing values and ensure re-syncs do not fail when a retained attachment row
already exists.
---
Nitpick comments:
In `@packages/db/drizzle/20260711154730_confused_green_goblin/migration.sql`:
- Around line 47-53: Add indexes in the migration for the uncovered foreign-key
and query columns: post_authors.author_slug, post_tags.tag,
posts.collection_slug, posts.group_id, and posts.published_at. Use distinct,
descriptive index names and preserve the existing foreign-key constraints.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b8924826-cf3b-45cf-830c-7a2b1b4ad0ff
📒 Files selected for processing (21)
apps/api/src/routes/content/post.test.tsapps/api/src/routes/content/post.tsapps/api/src/routes/content/profiles.test.tsapps/api/src/routes/content/profiles.tsapps/api/test-utils/setup.tsapps/worker/src/tasks/grant-author-achievements/processor.tsapps/worker/src/tasks/sync-post/processor.test.tsapps/worker/src/tasks/sync-post/processor.tsapps/worker/test-utils/setup.tspackages/db/drizzle/20260711154425_odd_silver_centurion/migration.sqlpackages/db/drizzle/20260711154425_odd_silver_centurion/snapshot.jsonpackages/db/drizzle/20260711154730_confused_green_goblin/migration.sqlpackages/db/drizzle/20260711154730_confused_green_goblin/snapshot.jsonpackages/db/package.jsonpackages/db/src/relations.tspackages/db/src/schema/attachments.tspackages/db/src/schema/collections.tspackages/db/src/schema/index.tspackages/db/src/schema/post-attachments.tspackages/db/src/schema/posts.tspackages/db/src/schema/tags.ts
💤 Files with no reviewable changes (2)
- packages/db/src/schema/tags.ts
- packages/db/src/schema/post-attachments.ts
fennifith
force-pushed
the
95/post-key-refactor
branch
from
633d28d to
2645eae
Compare
1 participant
Implements the schema changes described in #95
versionOrderis never set; by default, these would be ordered by creation date. A future issue should establish a frontmatter field to explicitly define the order.test-utils/setup.ts, making them much more readable. It might be good to expand this and move it totest-fixturesto share it withpackages/api.Summary by CodeRabbit
New Features
Bug Fixes