Cleanup after the orgId on videos backfill
#1136
Conversation
WalkthroughMade orgId required across the stack: added a nanoIdRequired helper and changed the videos table, domain Video type, repo storage, and import flow to use a non-optional orgId (removed Option wrapping). Changes
Sequence Diagram(s)sequenceDiagram
participant Import as ImportVideo
participant Repo as VideosRepo
participant DB as Database
rect rgba(135,206,250,0.12)
Note over Import,Repo: New flow passes required orgId (string)
Import->>Repo: createVideo({ ..., orgId: payload.cap.orgId })
Repo->>DB: insert videos (orgId: data.orgId)
DB-->>Repo: inserted row
Repo-->>Import: created video
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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: 1
🧹 Nitpick comments (1)
packages/database/schema.ts (1)
503-514: Consider adding a relation fororgId.The
videosRelationsdefine relations forowner(ownerId) andfolder(folderId), but not for the newly requiredorgIdfield. If videos belong to organizations, adding this relation would improve type safety and enable easier joins.Add the following relation:
export const videosRelations = relations(videos, ({ one, many }) => ({ owner: one(users, { fields: [videos.ownerId], references: [users.id], }), + organization: one(organizations, { + fields: [videos.orgId], + references: [organizations.id], + }), sharedVideos: many(sharedVideos), spaceVideos: many(spaceVideos), folder: one(folders, { fields: [videos.folderId], references: [folders.id], }), }));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/database/schema.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx,rs}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not add inline, block, or docstring comments in any language; code must be self-explanatory
Files:
packages/database/schema.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript and avoid any; leverage shared types from packages
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.
Files:
packages/database/schema.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
packages/database/schema.ts
🧬 Code graph analysis (1)
packages/database/schema.ts (1)
packages/database/helpers.ts (1)
nanoId(6-9)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Analyze (rust)
🔇 Additional comments (1)
packages/database/schema.ts (1)
250-250: Ensure orgId backfill completed before merging
The schema now enforcesvideos.orgIdas non-null. Before deploying, run:SELECT COUNT(*) FROM videos WHERE orgId IS NULL;and confirm it returns 0 to avoid runtime errors.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/database/schema.ts (1)
670-670: Replace nanoIdNullable("orgId").notNull() with nanoIdRequired("orgId") in imported_videos schema for consistency- orgId: nanoIdNullable("orgId").notNull(), + orgId: nanoIdRequired("orgId"),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/database/schema.ts(2 hunks)packages/web-backend/src/Loom/ImportVideo.ts(1 hunks)packages/web-backend/src/Videos/VideosRepo.ts(2 hunks)packages/web-domain/src/Video.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx,rs}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not add inline, block, or docstring comments in any language; code must be self-explanatory
Files:
packages/web-domain/src/Video.tspackages/web-backend/src/Videos/VideosRepo.tspackages/web-backend/src/Loom/ImportVideo.tspackages/database/schema.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript and avoid any; leverage shared types from packages
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.
Files:
packages/web-domain/src/Video.tspackages/web-backend/src/Videos/VideosRepo.tspackages/web-backend/src/Loom/ImportVideo.tspackages/database/schema.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
packages/web-domain/src/Video.tspackages/web-backend/src/Videos/VideosRepo.tspackages/web-backend/src/Loom/ImportVideo.tspackages/database/schema.ts
🧬 Code graph analysis (1)
packages/database/schema.ts (1)
packages/database/helpers.ts (1)
nanoIdLength(3-3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Analyze (rust)
🔇 Additional comments (5)
packages/web-backend/src/Videos/VideosRepo.ts (1)
61-61: LGTM! Clean removal of Option wrapping.The changes correctly reflect the now-required orgId field by using direct value access instead of Option unwrapping. The simplified import condition at line 75 (removing the
Option.isSome(data.orgId)check) is appropriate since orgId is now guaranteed to be present.Also applies to: 75-80
packages/web-domain/src/Video.ts (1)
17-17: LGTM! Domain model correctly reflects required orgId.Changing from optional to required string aligns with the database schema change and ensures type safety across the stack.
packages/web-backend/src/Loom/ImportVideo.ts (1)
73-73: LGTM! Correctly passes direct orgId value.The removal of the Option wrapper is consistent with the required orgId field in the domain model and repository layer.
packages/database/schema.ts (2)
27-29: Well-designed helper for required nanoId fields.The
nanoIdRequiredhelper explicitly chains.notNull(), ensuring the database constraint is properly enforced. This is clearer and more maintainable than relying on the type-levelnotNull: truein thenanoIddefinition.
253-253: LGTM! Consistent use of nanoIdRequired.The change from nullable to required aligns with the backfill and domain model updates. Using the new
nanoIdRequiredhelper ensures the NOT NULL constraint is properly set.
This will be merged once the backfill has been run to make
orgIdrequired. This should be merged after the backfill and Planetscale PR have been merged.https://app.planetscale.com/cap/cap-production/deploy-requests/39
Summary by CodeRabbit
Bug Fixes
Chores