Move video to folder #1132
Move video to folder #1132
Conversation
- Add validation to ensure all requested videos exist in the space before updating - Prevent silent partial updates when some videos are missing from space_videos - Throw descriptive error when videos are not found in the specified space - Improves data integrity and prevents misleading success responses
- Remove inline comments per coding guidelines - Add scope isolation with isNull(folders.spaceId) filters - Implement permission checks with SpacesPolicy validation - Replace broad cache invalidation with targeted setQueryData updates - Return detailed result data for efficient cache mutations - Ensure space context validation to prevent cross-scope moves
- Rename FolderSelectionDialog.tsx to folder-selection-dialog.tsx per coding guidelines - Update import path in SelectedCapsBar.tsx to use new kebab-case filename - Preserve git history using git mv command
- Replace over-deduction logic with precise per-folder video count deltas - Add videoCountDeltas to moveVideosToFolder return value with exact decrements/increments - Implement recursive applyFolderDeltas helper to update entire folder tree - Fix multi-source folder moves by calculating actual videos moved per folder - Ensure nested folders at all depths get updated correctly - Prevent cache staleness in grandchildren and deeper folder descendants
WalkthroughAdds server actions to list and move folders with root-aware (user/space/org) validation, a FolderSelectionDialog UI and SelectedCapsBar integration for moving items, and new Effect-based folder library functions implementing hierarchical queries and move logic. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant SB as SelectedCapsBar
participant FD as FolderSelectionDialog
participant SA as moveVideosToFolderAction
participant LIB as folder.lib (moveVideosToFolder)
participant POL as CurrentUser/Policy
participant DB as Database
participant RV as Revalidation
U->>SB: Click "Move to folder"
SB->>FD: Open dialog
FD->>SA: Submit { videoIds, targetFolderId, spaceId? }
SA->>POL: Authenticate & validate active org (and space membership if needed)
SA->>LIB: moveVideosToFolder(videoIds, targetFolderId, root)
LIB->>DB: Validate target scope, update video.folderId rows, compute per-folder deltas
DB-->>LIB: Updated rows / deltas
LIB-->>SA: Return movedCounts, deltas
SA->>RV: Revalidate org/space/folder routes (affected)
SA-->>FD: Respond success/error
FD-->>SB: Notify, optimistic cache updated, close
sequenceDiagram
autonumber
actor U as User
participant FD as FolderSelectionDialog
participant GA as getAllFoldersAction
participant LIB as folder.lib (getAllFolders)
participant POL as CurrentUser/Policy
participant DB as Database
U->>FD: Open dialog
FD->>GA: Request folders for root
GA->>POL: Validate user + active org
GA->>LIB: getAllFolders(root)
LIB->>DB: Query folders, child relationships, and video counts
DB-->>LIB: Rows
LIB-->>GA: Hierarchical folder tree
GA-->>FD: { success, folders }
FD-->>U: Render folder tree
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/web/actions/folders/getAllFolders.ts(1 hunks)apps/web/actions/folders/move-videos-to-folder.ts(1 hunks)apps/web/app/(org)/dashboard/_components/folder-selection-dialog.tsx(1 hunks)apps/web/app/(org)/dashboard/caps/components/SelectedCapsBar.tsx(1 hunks)apps/web/lib/folder.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
apps/web/actions/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
All Groq/OpenAI calls must be implemented in Next.js Server Actions under apps/web/actions; do not place AI calls elsewhere
Files:
apps/web/actions/folders/move-videos-to-folder.tsapps/web/actions/folders/getAllFolders.ts
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/**/*.{ts,tsx}: Use TanStack Query v5 for all client-side server state and data fetching in the web app
Web mutations should call Server Actions directly and perform targeted cache updates with setQueryData/setQueriesData rather than broad invalidations
Client code should use useEffectQuery/useEffectMutation and useRpcClient from apps/web/lib/EffectRuntime.ts; do not create ManagedRuntime inside components
Files:
apps/web/actions/folders/move-videos-to-folder.tsapps/web/actions/folders/getAllFolders.tsapps/web/app/(org)/dashboard/_components/folder-selection-dialog.tsxapps/web/app/(org)/dashboard/caps/components/SelectedCapsBar.tsxapps/web/lib/folder.ts
**/*.{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:
apps/web/actions/folders/move-videos-to-folder.tsapps/web/actions/folders/getAllFolders.tsapps/web/app/(org)/dashboard/_components/folder-selection-dialog.tsxapps/web/app/(org)/dashboard/caps/components/SelectedCapsBar.tsxapps/web/lib/folder.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:
apps/web/actions/folders/move-videos-to-folder.tsapps/web/actions/folders/getAllFolders.tsapps/web/app/(org)/dashboard/_components/folder-selection-dialog.tsxapps/web/app/(org)/dashboard/caps/components/SelectedCapsBar.tsxapps/web/lib/folder.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:
apps/web/actions/folders/move-videos-to-folder.tsapps/web/actions/folders/getAllFolders.tsapps/web/app/(org)/dashboard/_components/folder-selection-dialog.tsxapps/web/app/(org)/dashboard/caps/components/SelectedCapsBar.tsxapps/web/lib/folder.ts
apps/web/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
On the client, always use
useEffectQueryoruseEffectMutationfrom@/lib/EffectRuntime; never callEffectRuntime.run*directly in components.
Files:
apps/web/actions/folders/move-videos-to-folder.tsapps/web/actions/folders/getAllFolders.tsapps/web/app/(org)/dashboard/_components/folder-selection-dialog.tsxapps/web/app/(org)/dashboard/caps/components/SelectedCapsBar.tsxapps/web/lib/folder.ts
apps/web/app/**/*.{tsx,ts}
📄 CodeRabbit inference engine (CLAUDE.md)
Prefer Server Components for initial data in the Next.js App Router and pass initialData to client components
Files:
apps/web/app/(org)/dashboard/_components/folder-selection-dialog.tsxapps/web/app/(org)/dashboard/caps/components/SelectedCapsBar.tsx
🧬 Code graph analysis (5)
apps/web/actions/folders/move-videos-to-folder.ts (3)
apps/web/lib/folder.ts (1)
moveVideosToFolder(383-534)packages/web-domain/src/Authentication.ts (1)
CurrentUser(7-10)apps/web/lib/server.ts (1)
runPromise(59-71)
apps/web/actions/folders/getAllFolders.ts (3)
apps/web/lib/server.ts (1)
runPromise(59-71)apps/web/lib/folder.ts (1)
getAllFolders(309-381)packages/web-domain/src/Authentication.ts (1)
CurrentUser(7-10)
apps/web/app/(org)/dashboard/_components/folder-selection-dialog.tsx (5)
apps/web/app/(org)/dashboard/Contexts.tsx (1)
useDashboardContext(44-44)apps/web/lib/EffectRuntime.ts (2)
useEffectQuery(23-23)useEffectMutation(24-24)apps/web/actions/folders/getAllFolders.ts (1)
getAllFoldersAction(9-35)packages/database/schema.ts (1)
folders(219-243)apps/web/actions/folders/move-videos-to-folder.ts (1)
moveVideosToFolderAction(17-98)
apps/web/app/(org)/dashboard/caps/components/SelectedCapsBar.tsx (3)
packages/web-domain/src/Video.ts (3)
Video(14-59)VideoId(10-10)VideoId(11-11)apps/web/app/(org)/dashboard/_components/ConfirmationDialog.tsx (1)
ConfirmationDialog(25-72)apps/web/app/(org)/dashboard/_components/folder-selection-dialog.tsx (1)
FolderSelectionDialog(44-332)
apps/web/lib/folder.ts (6)
packages/database/index.ts (1)
db(29-34)packages/web-backend/src/Database.ts (1)
Database(9-19)packages/database/schema.ts (9)
folders(219-243)spaceVideos(577-597)spaces(532-552)organizations(149-169)sharedVideos(295-315)videos(245-293)comments(317-337)users(47-96)videoUploads(656-662)packages/web-domain/src/Folder.ts (3)
Folder(19-27)FolderId(8-8)FolderId(9-9)packages/web-domain/src/Video.ts (3)
Video(14-59)VideoId(10-10)VideoId(11-11)packages/web-domain/src/Authentication.ts (1)
CurrentUser(7-10)
🪛 GitHub Actions: CI
apps/web/actions/folders/move-videos-to-folder.ts
[error] 3-39: The imports and exports are not sorted.
apps/web/actions/folders/getAllFolders.ts
[error] 8-9: File content differs from formatting output.
apps/web/app/(org)/dashboard/_components/folder-selection-dialog.tsx
[error] 1-3: The imports and exports are not sorted.
apps/web/app/(org)/dashboard/caps/components/SelectedCapsBar.tsx
[error] 3-3: File content differs from formatting output.
[error] 3-3: The imports and exports are not sorted.
apps/web/lib/folder.ts
[error] 4-4: File content differs from formatting output.
[error] 4-4: File content differs from formatting output.
⏰ 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). (2)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
🔇 Additional comments (5)
apps/web/actions/folders/move-videos-to-folder.ts (1)
3-8: Run the formatter to sort importsCI is failing on this file because the import block is out of the order expected by Biome. Please run
pnpm format(or the project’s standard formatter) so the imports line up with the linting rules.apps/web/app/(org)/dashboard/caps/components/SelectedCapsBar.tsx (1)
1-25: Format this module to satisfy CIGitHub Actions is failing on this file due to formatting/import-order expectations. Please run the project formatter (e.g.,
pnpm format) so the file matches Biome’s output.apps/web/app/(org)/dashboard/_components/folder-selection-dialog.tsx (1)
1-25: Run Biome formatting hereThis file is also flagged by CI for unsorted imports/formatting drift. Run the formatter (
pnpm format) to bring it back in line with the project rules.apps/web/actions/folders/getAllFolders.ts (1)
3-8: Fix formatting to clear CIBiome expects the imports in this action to be sorted consistently. Please run the project formatter so CI stops failing on this file.
apps/web/lib/folder.ts (1)
3-23: Formatting cleanup requiredCI reports this file is out of sync with the formatter (import order/spacing). Please run
pnpm formatso the generated output matches Biome’s expectations.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/app/(org)/dashboard/_components/folder-selection-dialog.tsx(1 hunks)apps/web/lib/folder.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/**/*.{ts,tsx}: Use TanStack Query v5 for all client-side server state and data fetching in the web app
Web mutations should call Server Actions directly and perform targeted cache updates with setQueryData/setQueriesData rather than broad invalidations
Client code should use useEffectQuery/useEffectMutation and useRpcClient from apps/web/lib/EffectRuntime.ts; do not create ManagedRuntime inside components
Files:
apps/web/lib/folder.tsapps/web/app/(org)/dashboard/_components/folder-selection-dialog.tsx
**/*.{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:
apps/web/lib/folder.tsapps/web/app/(org)/dashboard/_components/folder-selection-dialog.tsx
**/*.{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:
apps/web/lib/folder.tsapps/web/app/(org)/dashboard/_components/folder-selection-dialog.tsx
**/*.{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:
apps/web/lib/folder.tsapps/web/app/(org)/dashboard/_components/folder-selection-dialog.tsx
apps/web/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
On the client, always use
useEffectQueryoruseEffectMutationfrom@/lib/EffectRuntime; never callEffectRuntime.run*directly in components.
Files:
apps/web/lib/folder.tsapps/web/app/(org)/dashboard/_components/folder-selection-dialog.tsx
apps/web/app/**/*.{tsx,ts}
📄 CodeRabbit inference engine (CLAUDE.md)
Prefer Server Components for initial data in the Next.js App Router and pass initialData to client components
Files:
apps/web/app/(org)/dashboard/_components/folder-selection-dialog.tsx
🧬 Code graph analysis (2)
apps/web/lib/folder.ts (6)
packages/database/index.ts (1)
db(29-34)packages/web-backend/src/Database.ts (1)
Database(9-19)packages/database/schema.ts (9)
folders(219-243)spaceVideos(577-597)spaces(532-552)organizations(149-169)sharedVideos(295-315)videos(245-293)comments(317-337)users(47-96)videoUploads(656-662)packages/web-domain/src/Folder.ts (3)
Folder(19-27)FolderId(8-8)FolderId(9-9)packages/web-domain/src/Video.ts (3)
Video(14-59)VideoId(10-10)VideoId(11-11)packages/web-domain/src/Authentication.ts (1)
CurrentUser(7-10)
apps/web/app/(org)/dashboard/_components/folder-selection-dialog.tsx (4)
apps/web/app/(org)/dashboard/Contexts.tsx (1)
useDashboardContext(44-44)apps/web/lib/EffectRuntime.ts (2)
useEffectQuery(23-23)useEffectMutation(24-24)apps/web/actions/folders/getAllFolders.ts (1)
getAllFoldersAction(9-35)apps/web/actions/folders/move-videos-to-folder.ts (1)
moveVideosToFolderAction(17-98)
⏰ 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). (2)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/lib/folder.ts (1)
155-261: Enforce org/space context in getVideosByFolderId
getVideosByFolderId only takes a folderId and doesn’t verify that the folder (and its videos) belong to the caller’s organization or space, risking data leakage across org boundaries.
• Refactor getVideosByFolderId to accept a context argument (organizationId or spaceId) and include it in the SQL WHERE clause.
• Update all call sites (e.g. apps/web/app/(org)/dashboard/**/page.tsx) to pass the appropriate context from getSpaceOrOrg.
🧹 Nitpick comments (2)
apps/web/lib/folder.ts (2)
40-65: Consider safeguarding against circular parent references.The while loop traverses the folder hierarchy but could theoretically loop infinitely if the database contains circular parent relationships (e.g., folder A → folder B → folder A). While this shouldn't happen with proper constraints, adding a depth limit or visited-set check would make the code more resilient.
Consider adding a maximum depth check:
export const getFolderBreadcrumb = Effect.fn(function* ( folderId: Folder.FolderId ) { const breadcrumb: Array<{ id: Folder.FolderId; name: string; color: "normal" | "blue" | "red" | "yellow"; }> = []; let currentFolderId = folderId; + let depth = 0; + const MAX_DEPTH = 100; - while (currentFolderId) { + while (currentFolderId && depth < MAX_DEPTH) { const folder = yield* getFolderById(currentFolderId); if (!folder) break; breadcrumb.unshift({ id: folder.id, name: folder.name, color: folder.color, }); if (!folder.parentId) break; currentFolderId = folder.parentId; + depth++; } return breadcrumb; });
86-90: Consider usinginArrayfor cleaner SQL.The manual IN clause construction with
sql.joinworks but is less idiomatic than using drizzle-orm'sinArrayhelper, which you're already using elsewhere (e.g., line 411).Apply this refactor:
.where( - sql`${spaceVideos.videoId} IN (${sql.join( - videoIds.map((id) => sql`${id}`), - sql`, ` - )})` + inArray(spaceVideos.videoId, videoIds) )Make the same change for the
orgSharingquery at lines 107-112.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/lib/folder.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/**/*.{ts,tsx}: Use TanStack Query v5 for all client-side server state and data fetching in the web app
Web mutations should call Server Actions directly and perform targeted cache updates with setQueryData/setQueriesData rather than broad invalidations
Client code should use useEffectQuery/useEffectMutation and useRpcClient from apps/web/lib/EffectRuntime.ts; do not create ManagedRuntime inside components
Files:
apps/web/lib/folder.ts
**/*.{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:
apps/web/lib/folder.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:
apps/web/lib/folder.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:
apps/web/lib/folder.ts
apps/web/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
On the client, always use
useEffectQueryoruseEffectMutationfrom@/lib/EffectRuntime; never callEffectRuntime.run*directly in components.
Files:
apps/web/lib/folder.ts
🧬 Code graph analysis (1)
apps/web/lib/folder.ts (6)
packages/database/index.ts (1)
db(29-34)packages/web-backend/src/Database.ts (1)
Database(9-19)packages/database/schema.ts (9)
folders(219-243)spaceVideos(577-597)spaces(532-552)organizations(149-169)sharedVideos(295-315)videos(245-293)comments(317-337)users(47-96)videoUploads(656-662)packages/web-domain/src/Folder.ts (3)
Folder(19-27)FolderId(8-8)FolderId(9-9)packages/web-domain/src/Video.ts (3)
Video(14-59)VideoId(10-10)VideoId(11-11)packages/web-domain/src/Authentication.ts (1)
CurrentUser(7-10)
⏰ 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). (2)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
🔇 Additional comments (3)
apps/web/lib/folder.ts (3)
283-294: Space-scoped folder counts correctly implemented.The conditional video count logic properly uses
space_videostable whenroot.variant === "space"and falls back to the globalvideostable otherwise. This resolves the past review concern about incorrect counts in space-scoped contexts.Based on past review comments, this fix addresses the reported issue.
330-341: Space-scoped folder counts correctly implemented.Like
getChildFolders, this function properly implements conditional video counting based onroot.variant, usingspace_videosfor space contexts. The fix is consistent across both functions.Based on past review comments, this addresses the same concern.
464-522: Video count delta accounting looks correct.The logic properly tracks video count changes for both source and target folders:
- Lines 464-476 (space context): counts videos moving from each source folder
- Lines 492-504 (non-space context): same logic for non-space moves
- Lines 519-522: adds count to target folder
The deltas are returned for optimistic UI updates, which is appropriate since folder counts are computed dynamically via subqueries rather than cached.
| export const getFolderById = Effect.fn(function* (folderId: string) { | ||
| if (!folderId) throw new Error("Folder ID is required"); | ||
| const db = yield* Database; | ||
| if (!folderId) throw new Error("Folder ID is required"); | ||
| const db = yield* Database; | ||
|
|
||
| const [folder] = yield* db.execute((db) => | ||
| db | ||
| .select() | ||
| .from(folders) | ||
| .where(eq(folders.id, Folder.FolderId.make(folderId))), | ||
| ); | ||
| const [folder] = yield* db.execute((db) => | ||
| db | ||
| .select() | ||
| .from(folders) | ||
| .where(eq(folders.id, Folder.FolderId.make(folderId))) | ||
| ); | ||
|
|
||
| if (!folder) throw new Error("Folder not found"); | ||
| if (!folder) throw new Error("Folder not found"); | ||
|
|
||
| return folder; | ||
| return folder; | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
Verify access control for folder queries.
getFolderById fetches any folder by ID without validating that the folder belongs to the user's active organization or checking access permissions. This could potentially expose folder data across organization boundaries if called without upstream permission checks.
Run the following script to verify where this function is called and whether permission checks exist at those call sites:
🏁 Script executed:
#!/bin/bash
# Description: Find all call sites of getFolderById and check for permission validation
# Find all imports and usages of getFolderById
rg -n -A5 -B5 'getFolderById' --type ts --type tsxLength of output: 27
🏁 Script executed:
#!/bin/bash
# Search for all usages of getFolderById across the repository
rg -n -C3 '\bgetFolderById\b'Length of output: 1211
Scope getFolderById to the user’s organization.
It currently retrieves any folder by ID without filtering on organizationId, allowing cross-org data exposure (e.g. in getFolderBreadcrumb at line 51). Add a .where(eq(folders.organizationId, user.activeOrganizationId)) filter or enforce this check inside getFolderById so all callers are correctly scoped.
| const existingVideos = yield* db.execute((db) => | ||
| db | ||
| .select({ | ||
| id: videos.id, | ||
| folderId: videos.folderId, | ||
| ownerId: videos.ownerId, | ||
| }) | ||
| .from(videos) | ||
| .where(and(inArray(videos.id, videoIds), eq(videos.ownerId, user.id))) | ||
| ); | ||
|
|
||
| if (existingVideos.length !== videoIds.length) { | ||
| throw new Error( | ||
| "Some videos not found or you don't have permission to move them" | ||
| ); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Verify organization validation for video moves.
The function validates video ownership (line 411: eq(videos.ownerId, user.id)) but doesn't check whether videos belong to the user's active organization. This creates an asymmetry with the space context path (lines 449-462), which validates videos exist in the specified space.
In the non-space context, a user could potentially move videos they own from one organization into folders of their currently active organization. This may be intentional if videos are truly user-owned rather than organization-owned, but it differs from the stricter space-scoped validation.
Run the following script to check how videos.orgId relates to folder organization membership in existing code:
If cross-organization video moves should be prevented, consider adding:
const existingVideos = yield* db.execute((db) =>
db
.select({
id: videos.id,
folderId: videos.folderId,
ownerId: videos.ownerId,
+ orgId: videos.orgId,
})
.from(videos)
- .where(and(inArray(videos.id, videoIds), eq(videos.ownerId, user.id)))
+ .where(
+ and(
+ inArray(videos.id, videoIds),
+ eq(videos.ownerId, user.id),
+ eq(videos.orgId, user.activeOrganizationId)
+ )
+ )
);🏁 Script executed:
#!/bin/bash
# Description: Check if there are existing patterns of video organization validation
# Search for other places where video org membership is validated
rg -n -C5 'videos\.orgId' --type ts --type tsx
# Check if there are policies or patterns for video organization membership
ast-grep --pattern 'eq(videos.orgId, $_)'Length of output: 27
🏁 Script executed:
#!/bin/bash
# Locate the videos table schema to confirm its columns
rg -n "export const videos" -C5 --type tsLength of output: 1253
🏁 Script executed:
#!/bin/bash
# Print the full videos table definition to verify its columns
sed -n '245,340p' packages/database/schema.tsLength of output: 3391
Enforce video organization membership in non-space moves
The initial existingVideos query (apps/web/lib/folder.ts lines 403–412) must also check videos.orgId = user.activeOrganizationId to prevent cross‐org moves:
.where(
and(
inArray(videos.id, videoIds),
eq(videos.ownerId, user.id),
+ eq(videos.orgId, user.activeOrganizationId),
)
)📝 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 existingVideos = yield* db.execute((db) => | |
| db | |
| .select({ | |
| id: videos.id, | |
| folderId: videos.folderId, | |
| ownerId: videos.ownerId, | |
| }) | |
| .from(videos) | |
| .where(and(inArray(videos.id, videoIds), eq(videos.ownerId, user.id))) | |
| ); | |
| if (existingVideos.length !== videoIds.length) { | |
| throw new Error( | |
| "Some videos not found or you don't have permission to move them" | |
| ); | |
| } | |
| const existingVideos = yield* db.execute((db) => | |
| db | |
| .select({ | |
| id: videos.id, | |
| folderId: videos.folderId, | |
| ownerId: videos.ownerId, | |
| }) | |
| .from(videos) | |
| .where( | |
| and( | |
| inArray(videos.id, videoIds), | |
| eq(videos.ownerId, user.id), | |
| eq(videos.orgId, user.activeOrganizationId), | |
| ) | |
| ) | |
| ); |
🤖 Prompt for AI Agents
In apps/web/lib/folder.ts around lines 403 to 418, the existingVideos query does
not enforce organization membership which allows cross-org moves; update the
WHERE clause to also require videos.orgId equals user.activeOrganizationId
(i.e., add an eq(videos.orgId, user.activeOrganizationId) condition combined
with the existing predicates) so only videos belonging to the user's active
organization are selected before the permission check.
Summary
Implements comprehensive video folder management functionality with proper validation and space-aware operations.
Changes
moveVideosToFolderfunction with proper validationFeatures
r.mp4
Summary by CodeRabbit
New Features
Bug Fixes