implement video folder movement functionality #1126
Conversation
WalkthroughAdds server actions to list folders and move videos, a FolderSelectionDialog UI integrated into the selected-items toolbar, and library functions to build folder trees and perform video-folder moves with validation and cache revalidation. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant SelectedCapsBar as SelectedCapsBar
participant Dialog as FolderSelectionDialog
participant Actions as Server Actions
participant Lib as Folder Lib
participant Cache as Next.js Revalidate
User->>SelectedCapsBar: Click "Move to folder"
SelectedCapsBar->>Dialog: open()
Dialog->>Actions: getAllFoldersAction(root)
Actions->>Lib: getAllFolders(root) [with CurrentUser]
Lib-->>Actions: folders
Actions-->>Dialog: { success, folders }
User->>Dialog: Choose target & confirm
Dialog->>Actions: moveVideosToFolderAction({videoIds, targetFolderId, spaceId?})
Actions->>Lib: moveVideosToFolder(..) [with CurrentUser]
Lib-->>Actions: move result
Actions->>Cache: revalidatePath(...) (caps, space/org folders)
Actions-->>Dialog: { success, message }
Dialog-->>SelectedCapsBar: onConfirm(folderId)
sequenceDiagram
actor Client
participant getAll as getAllFoldersAction
participant lib as getAllFolders
Client->>getAll: request(root)
getAll->>getAll: validate user / org
getAll->>lib: fetch folders
lib-->>getAll: folders
getAll-->>Client: { success, folders } / { error }
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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 |
move_video.mp4This is the result |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (9)
apps/web/actions/folders/getAllFolders.ts (1)
10-14: Trim unused variant or handle it.The
uservariant isn’t used ingetAllFolders(onlyorg/spacepaths are honored). Remove it here or implement semantics to avoid confusion.apps/web/lib/folder.ts (2)
19-21: Remove unused import.
revalidatePathisn’t used in this module. Drop the import.-import { revalidatePath } from "next/cache";
88-93: PreferinArrayover manual SQL IN building.Use Drizzle’s
inArrayto avoid manual SQL construction and to leverage parameter binding.- .where( - sql`${spaceVideos.videoId} IN (${sql.join( - videoIds.map((id) => sql`${id}`), - sql`, ` - )})` - ) + .where(inArray(spaceVideos.videoId, videoIds)) @@ - .where( - sql`${sharedVideos.videoId} IN (${sql.join( - videoIds.map((id) => sql`${id}`), - sql`, ` - )})` - ) + .where(inArray(sharedVideos.videoId, videoIds))Also applies to: 111-116
apps/web/actions/folders/moveVideosToFolder.ts (1)
45-69: Revalidation paths are fine; consider minimizing broad invalidations.
/dashboard/capsis always revalidated. Prefer targeted cache updates on the client (setQueryData/setQueriesData) per coding guidelines; keep server revalidation minimal.As per coding guidelines
apps/web/app/(org)/dashboard/_components/FolderSelectionDialog.tsx (3)
58-76: AvoiduseQueryhere; adoptuseEffectQueryand update client cache on move.Replace this with the project’s standard query hook and wire the move mutation to update the relevant query cache rather than relying solely on route revalidation.
As per coding guidelines
168-187: Make the dialog UI-only to avoid duplicating business logic.
handleConfirmperforms the server mutation. WithonConfirmalready provided, keep the mutation in the parent to maintain separation of concerns and enable cache updates there.Apply this minimal change to make the dialog UI-only:
- const handleConfirm = async () => { - try { - const result = await moveVideosToFolderAction({ - videoIds, - targetFolderId: selectedFolderId, - spaceId: activeSpace?.id, - }); - - if (result.success) { - toast.success(result.message); - onConfirm(selectedFolderId); - setSelectedFolderId(null); - } else { - toast.error(result.error); - } - } catch (error) { - toast.error("Failed to move videos"); - console.error("Error moving videos:", error); - } - }; + const handleConfirm = () => { + onConfirm(selectedFolderId); + setSelectedFolderId(null); + };Then implement the mutation in the parent with
useEffectMutation.
115-147: Remove inline comments per repo policy.JSX/inline comments are disallowed; the code is self-explanatory.
As per coding guidelines
Also applies to: 150-156, 158-166, 207-239, 240-246, 247-250, 252-263
apps/web/app/(org)/dashboard/caps/components/SelectedCapsBar.tsx (2)
15-21: Type consistency and prop usage.
selectedCapsisstring[]whilesetSelectedCapsexpectsVideo.VideoId[]. Align types toVideo.VideoId[]for both to prevent widening.-interface SelectedCapsBarProps { - selectedCaps: string[]; - setSelectedCaps: (caps: Video.VideoId[]) => void; +interface SelectedCapsBarProps { + selectedCaps: Video.VideoId[]; + setSelectedCaps: (caps: Video.VideoId[]) => void;
118-125: Own the mutation in the parent using EffectRuntime.With the dialog made UI-only, handle the move via
useEffectMutationhere (and update local/query caches withsetQueryData).As per coding guidelines
📜 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/moveVideosToFolder.ts(1 hunks)apps/web/app/(org)/dashboard/_components/FolderSelectionDialog.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/getAllFolders.tsapps/web/actions/folders/moveVideosToFolder.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/getAllFolders.tsapps/web/actions/folders/moveVideosToFolder.tsapps/web/app/(org)/dashboard/_components/FolderSelectionDialog.tsxapps/web/lib/folder.tsapps/web/app/(org)/dashboard/caps/components/SelectedCapsBar.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/actions/folders/getAllFolders.tsapps/web/actions/folders/moveVideosToFolder.tsapps/web/app/(org)/dashboard/_components/FolderSelectionDialog.tsxapps/web/lib/folder.tsapps/web/app/(org)/dashboard/caps/components/SelectedCapsBar.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/actions/folders/getAllFolders.tsapps/web/actions/folders/moveVideosToFolder.tsapps/web/app/(org)/dashboard/_components/FolderSelectionDialog.tsxapps/web/lib/folder.tsapps/web/app/(org)/dashboard/caps/components/SelectedCapsBar.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/actions/folders/getAllFolders.tsapps/web/actions/folders/moveVideosToFolder.tsapps/web/app/(org)/dashboard/_components/FolderSelectionDialog.tsxapps/web/lib/folder.tsapps/web/app/(org)/dashboard/caps/components/SelectedCapsBar.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/actions/folders/getAllFolders.tsapps/web/actions/folders/moveVideosToFolder.tsapps/web/app/(org)/dashboard/_components/FolderSelectionDialog.tsxapps/web/lib/folder.tsapps/web/app/(org)/dashboard/caps/components/SelectedCapsBar.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/FolderSelectionDialog.tsxapps/web/app/(org)/dashboard/caps/components/SelectedCapsBar.tsx
🧬 Code graph analysis (5)
apps/web/actions/folders/getAllFolders.ts (3)
apps/web/lib/server.ts (1)
runPromise(59-71)apps/web/lib/folder.ts (1)
getAllFolders(308-380)packages/web-domain/src/Authentication.ts (1)
CurrentUser(7-10)
apps/web/actions/folders/moveVideosToFolder.ts (4)
packages/web-domain/src/Folder.ts (1)
Folder(19-27)apps/web/lib/server.ts (1)
runPromise(59-71)apps/web/lib/folder.ts (1)
moveVideosToFolder(382-449)packages/web-domain/src/Authentication.ts (1)
CurrentUser(7-10)
apps/web/app/(org)/dashboard/_components/FolderSelectionDialog.tsx (3)
apps/web/app/(org)/dashboard/Contexts.tsx (1)
useDashboardContext(44-44)apps/web/actions/folders/getAllFolders.ts (1)
getAllFoldersAction(9-35)apps/web/actions/folders/moveVideosToFolder.ts (1)
moveVideosToFolderAction(16-84)
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/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/FolderSelectionDialog.tsx (1)
FolderSelectionDialog(43-287)
⏰ 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 (1)
apps/web/actions/folders/getAllFolders.ts (1)
24-26: LGTM: Effect context + runtime wiringProviding CurrentUser via Effect and running with runPromise is consistent with the existing pattern.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 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 (7)
folders(219-243)spaceVideos(577-597)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)
| 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))) | ||
| ); |
There was a problem hiding this comment.
Enforce org scoping when selecting videos to move.
The validation query only checks ownerId, so a user active in org A can move their videos from another org into A’s folders. That breaks tenant isolation and poisons folder listings. Add the org constraint to the WHERE clause before proceeding.
Apply this diff:
.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)
+ )
+ )📝 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))) | |
| ); | |
| 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 398 to 407, the query that selects
existingVideos only filters by ownerId which allows cross-org moves; add the org
constraint to the WHERE clause (e.g., include eq(videos.orgId, user.orgId)) so
the .where(...) becomes an and(...) that combines inArray(videos.id, videoIds),
eq(videos.ownerId, user.id), and eq(videos.orgId, user.orgId); ensure the new
condition uses the correct videos org column name and is applied before
proceeding.
| if (root?.variant === "space") { | ||
| // Collect originals from space_videos | ||
| const spaceRows = yield* db.execute((db) => | ||
| db | ||
| .select({ | ||
| folderId: spaceVideos.folderId, | ||
| videoId: spaceVideos.videoId, | ||
| }) | ||
| .from(spaceVideos) | ||
| .where( | ||
| and( | ||
| eq(spaceVideos.spaceId, root.spaceId), | ||
| inArray(spaceVideos.videoId, videoIds) | ||
| ) | ||
| ) | ||
| ); | ||
| originalFolderIds = [ | ||
| ...new Set(spaceRows.map((r) => r.folderId).filter(Boolean)), | ||
| ]; | ||
|
|
||
| // Update per-space folder placement | ||
| yield* db.execute((db) => | ||
| db | ||
| .update(spaceVideos) | ||
| .set({ folderId: targetFolderId }) | ||
| .where( | ||
| and( | ||
| eq(spaceVideos.spaceId, root.spaceId), | ||
| inArray(spaceVideos.videoId, videoIds) | ||
| ) | ||
| ) | ||
| ); | ||
| } else { |
There was a problem hiding this comment.
Abort when a video isn’t linked to the target space.
If any videoId isn’t present in space_videos for the given space, spaceRows misses it and the update silently skips that video while movedCount still reports success. Fail fast so callers get a consistent result.
Apply this diff:
const spaceRows = yield* db.execute((db) =>
db
.select({
folderId: spaceVideos.folderId,
videoId: spaceVideos.videoId,
})
.from(spaceVideos)
.where(
and(
eq(spaceVideos.spaceId, root.spaceId),
inArray(spaceVideos.videoId, videoIds)
)
)
);
+ if (spaceRows.length !== videoIds.length) {
+ throw new Error(
+ "Some videos are not part of the specified space or you lack access"
+ );
+ }📝 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.
| if (root?.variant === "space") { | |
| // Collect originals from space_videos | |
| const spaceRows = yield* db.execute((db) => | |
| db | |
| .select({ | |
| folderId: spaceVideos.folderId, | |
| videoId: spaceVideos.videoId, | |
| }) | |
| .from(spaceVideos) | |
| .where( | |
| and( | |
| eq(spaceVideos.spaceId, root.spaceId), | |
| inArray(spaceVideos.videoId, videoIds) | |
| ) | |
| ) | |
| ); | |
| originalFolderIds = [ | |
| ...new Set(spaceRows.map((r) => r.folderId).filter(Boolean)), | |
| ]; | |
| // Update per-space folder placement | |
| yield* db.execute((db) => | |
| db | |
| .update(spaceVideos) | |
| .set({ folderId: targetFolderId }) | |
| .where( | |
| and( | |
| eq(spaceVideos.spaceId, root.spaceId), | |
| inArray(spaceVideos.videoId, videoIds) | |
| ) | |
| ) | |
| ); | |
| } else { | |
| if (root?.variant === "space") { | |
| // Collect originals from space_videos | |
| const spaceRows = yield* db.execute((db) => | |
| db | |
| .select({ | |
| folderId: spaceVideos.folderId, | |
| videoId: spaceVideos.videoId, | |
| }) | |
| .from(spaceVideos) | |
| .where( | |
| and( | |
| eq(spaceVideos.spaceId, root.spaceId), | |
| inArray(spaceVideos.videoId, videoIds) | |
| ) | |
| ) | |
| ); | |
| if (spaceRows.length !== videoIds.length) { | |
| throw new Error( | |
| "Some videos are not part of the specified space or you lack access" | |
| ); | |
| } | |
| originalFolderIds = [ | |
| ...new Set(spaceRows.map((r) => r.folderId).filter(Boolean)), | |
| ]; | |
| // Update per-space folder placement | |
| yield* db.execute((db) => | |
| db | |
| .update(spaceVideos) | |
| .set({ folderId: targetFolderId }) | |
| .where( | |
| and( | |
| eq(spaceVideos.spaceId, root.spaceId), | |
| inArray(spaceVideos.videoId, videoIds) | |
| ) | |
| ) | |
| ); | |
| } else { |
🤖 Prompt for AI Agents
In apps/web/lib/folder.ts around lines 432 to 464, the code queries space_videos
for the given space and videoIds then performs an update but doesn’t detect if
some videoIds were missing from space_videos; this causes the update to silently
skip those videos while movedCount later reports success. After fetching
spaceRows, compute which requested videoIds are absent (compare input videoIds
to spaceRows.videoId); if any are missing, abort immediately by throwing or
returning an error that lists the missing videoIds (do not perform the update).
Only proceed to set originalFolderIds and run the update when all requested
videoIds are present to ensure movedCount remains consistent.
Video Folder Movement Functionality
Overview
This PR adds the ability for users to move multiple videos between folders using a simple dialog interface.
Changes
New file
moveVideosToFolder.ts– Server action for handling video moves with authentication, validation, and revalidation.Updated files
folder.ts– AddedmoveVideosToFolderfunction using Effect.js for validation and batch database updates.FolderSelectionDialog.tsx– Upgraded from UI-only to functional; now actually moves videos and shows user feedback.SelectedCapsBar.tsx– Added missingvideoIdsprop to enable moving selected videos.Benefits
Next Steps
Summary by CodeRabbit
New Features
Improvements