improvement: only video owners will be able to perform additional actions#1068
improvement: only video owners will be able to perform additional actions#1068
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughReplaces the legacy CapCardButtons with a new CapCardButton and moves per-cap actions into the card header; removes the CapCardButtons file, drops FolderVideosSection's cardType prop and useUploadingContext import, removes two console.log calls from onboarding route, and adjusts SharedCapCard/Folder page invocations accordingly. (≤50 words) Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as CapCard UI
participant Btn as CapCardButton
participant Clipboard
participant Download as DownloadService
participant OwnerMenu
User->>UI: view cap card
UI->>Btn: render header buttons (copy, download, owner menu)
User->>Btn: click Copy
Btn->>UI: call copy handler
UI->>Clipboard: write URL
Clipboard-->>UI: success
User->>Btn: click Download
Btn->>UI: call download handler
UI->>UI: set downloading=true (spinner)
UI->>Download: request cap download
Download-->>UI: complete
UI->>UI: set downloading=false
User->>UI: open owner menu
UI->>OwnerMenu: show duplicate/edit/delete
OwnerMenu->>UI: trigger delete -> UI shows confirmation dialog
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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 |
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/web/app/(org)/dashboard/spaces/[spaceId]/SharedCaps.tsx (1)
8-8: Use useEffectQuery from EffectRuntime instead of useQueryClient code in apps/web should use useEffectQuery/useEffectMutation (per guidelines). Switch the import.
-import { useQuery } from "@tanstack/react-query"; +import { useEffectQuery } from "@/lib/EffectRuntime";apps/web/app/(org)/dashboard/folder/[id]/components/FolderVideosSection.tsx (1)
4-4: Use useEffectQuery instead of useQueryapps/web must use EffectRuntime hooks.
-import { useQuery } from "@tanstack/react-query"; +import { useEffectQuery } from "@/lib/EffectRuntime";apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx (1)
22-23: Avoid useMutation; use useEffectMutation for client mutationsapps/web should standardize on EffectRuntime hooks. Replace deleteMutation.
-import { useMutation } from "@tanstack/react-query"; +// remove this import; already using useEffectMutation elsewhereAnd replace the implementation:
- const deleteMutation = useMutation({ - mutationFn: async () => { - await onDelete?.(); - }, - onError: (error) => { - console.error("Error deleting cap:", error); - }, - onSettled: () => { - setConfirmOpen(false); - }, - }); + const deleteMutation = useEffectMutation({ + mutationFn: () => Effect.promise(async () => onDelete?.()), + onError: (error) => { + console.error("Error deleting cap:", error); + }, + onSettled: () => { + setConfirmOpen(false); + }, + });
🧹 Nitpick comments (2)
apps/web/app/(org)/dashboard/spaces/[spaceId]/SharedCaps.tsx (1)
102-135: Batch analytics fetch to reduce N requestsYou’re firing one request per video. Prefer a single batched endpoint (ids=[]) to cut latency and load.
I can draft the API shape and client call if helpful.
apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx (1)
176-186: Remove inline comments per repo guidelineRepo forbids inline/block comments; these new comments should be dropped.
- // Helper function to create a drag preview element const createDragPreview = (text: string): HTMLElement => { - // Create the element const element = document.createElement("div"); - - // Add text content element.textContent = text; - - // Apply Tailwind-like styles directly element.className = "px-2 py-1.5 text-sm font-medium rounded-lg shadow-md text-gray-1 bg-gray-12"; - - // Position off-screen element.style.position = "absolute"; element.style.top = "-9999px"; element.style.left = "-9999px"; return element; }; @@ - // Set the data transfer e.dataTransfer.setData( "application/cap", JSON.stringify({ id: cap.id, name: cap.name, }) ); - - // Set drag effect to 'move' to avoid showing the + icon e.dataTransfer.effectAllowed = "move"; - - // Set the drag image using the helper function try { const dragPreview = createDragPreview(cap.name); document.body.appendChild(dragPreview); e.dataTransfer.setDragImage(dragPreview, 10, 10); - - // Clean up after a short delay setTimeout(() => document.body.removeChild(dragPreview), 100); } catch (error) { console.error("Error setting drag image:", error); }Also applies to: 199-222
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx(2 hunks)apps/web/app/(org)/dashboard/caps/components/CapCard/CapCardButton.tsx(1 hunks)apps/web/app/(org)/dashboard/caps/components/CapCard/CapCardButtons.tsx(0 hunks)apps/web/app/(org)/dashboard/folder/[id]/components/FolderVideosSection.tsx(1 hunks)apps/web/app/(org)/dashboard/spaces/[spaceId]/SharedCaps.tsx(1 hunks)apps/web/app/(org)/dashboard/spaces/[spaceId]/components/SharedCapCard.tsx(1 hunks)apps/web/app/(org)/dashboard/spaces/[spaceId]/folder/[folderId]/page.tsx(1 hunks)apps/web/app/api/settings/onboarding/route.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/web/app/(org)/dashboard/caps/components/CapCard/CapCardButtons.tsx
🧰 Additional context used
📓 Path-based instructions (7)
apps/web/app/api/**/route.ts
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/app/api/**/route.ts: Place API routes only under apps/web/app/api and implement each route in a route.ts file
Construct API routes with @effect/platform HttpApi/HttpApiBuilder and export only the handler from apiToHandler(ApiLive)
Map domain errors to transport errors with HttpApiError.* and keep error translation exhaustive
Use HttpAuthMiddleware for required auth and provideOptionalAuth for guest routes; avoid duplicate session lookups
Provide dependencies via Layer.provide in API routes instead of manual provideService calls
Files:
apps/web/app/api/settings/onboarding/route.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/app/api/settings/onboarding/route.tsapps/web/app/(org)/dashboard/spaces/[spaceId]/folder/[folderId]/page.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/SharedCaps.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/components/SharedCapCard.tsxapps/web/app/(org)/dashboard/caps/components/CapCard/CapCardButton.tsxapps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsxapps/web/app/(org)/dashboard/folder/[id]/components/FolderVideosSection.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/api/settings/onboarding/route.tsapps/web/app/(org)/dashboard/spaces/[spaceId]/folder/[folderId]/page.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/SharedCaps.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/components/SharedCapCard.tsxapps/web/app/(org)/dashboard/caps/components/CapCard/CapCardButton.tsxapps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsxapps/web/app/(org)/dashboard/folder/[id]/components/FolderVideosSection.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/app/api/settings/onboarding/route.tsapps/web/app/(org)/dashboard/spaces/[spaceId]/folder/[folderId]/page.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/SharedCaps.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/components/SharedCapCard.tsxapps/web/app/(org)/dashboard/caps/components/CapCard/CapCardButton.tsxapps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsxapps/web/app/(org)/dashboard/folder/[id]/components/FolderVideosSection.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/app/api/settings/onboarding/route.tsapps/web/app/(org)/dashboard/spaces/[spaceId]/folder/[folderId]/page.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/SharedCaps.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/components/SharedCapCard.tsxapps/web/app/(org)/dashboard/caps/components/CapCard/CapCardButton.tsxapps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsxapps/web/app/(org)/dashboard/folder/[id]/components/FolderVideosSection.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/app/api/settings/onboarding/route.tsapps/web/app/(org)/dashboard/spaces/[spaceId]/folder/[folderId]/page.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/SharedCaps.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/components/SharedCapCard.tsxapps/web/app/(org)/dashboard/caps/components/CapCard/CapCardButton.tsxapps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsxapps/web/app/(org)/dashboard/folder/[id]/components/FolderVideosSection.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/app/api/settings/onboarding/route.tsapps/web/app/(org)/dashboard/spaces/[spaceId]/folder/[folderId]/page.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/SharedCaps.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/components/SharedCapCard.tsxapps/web/app/(org)/dashboard/caps/components/CapCard/CapCardButton.tsxapps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsxapps/web/app/(org)/dashboard/folder/[id]/components/FolderVideosSection.tsx
🧬 Code graph analysis (5)
apps/web/app/(org)/dashboard/spaces/[spaceId]/folder/[folderId]/page.tsx (2)
apps/web/lib/folder.ts (1)
getVideosByFolderId(145-245)apps/web/app/(org)/dashboard/folder/[id]/components/FolderVideosSection.tsx (1)
FolderVideosSection(24-214)
apps/web/app/(org)/dashboard/spaces/[spaceId]/SharedCaps.tsx (5)
packages/web-domain/src/Video.ts (3)
Video(14-59)VideoId(10-10)VideoId(11-11)apps/web/app/(org)/dashboard/spaces/[spaceId]/page.tsx (1)
SpaceMemberData(40-47)apps/web/app/(org)/dashboard/spaces/[spaceId]/components/OrganizationIndicator.tsx (2)
OrganizationMemberData(20-27)OrganizationIndicator(37-148)apps/web/app/(org)/dashboard/caps/components/Folder.tsx (1)
FolderDataType(20-27)apps/web/app/(org)/dashboard/spaces/[spaceId]/components/SharedCapCard.tsx (1)
SharedCapCard(29-80)
apps/web/app/(org)/dashboard/spaces/[spaceId]/components/SharedCapCard.tsx (2)
packages/web-domain/src/Video.ts (3)
Video(14-59)VideoId(10-10)VideoId(11-11)apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx (1)
CapCard(85-569)
apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx (3)
apps/web/app/(org)/dashboard/Contexts.tsx (1)
useDashboardContext(44-44)apps/web/app/s/[videoId]/_components/ProgressCircle.tsx (1)
useUploadProgress(26-66)apps/web/app/(org)/dashboard/caps/components/CapCard/CapCardButton.tsx (1)
CapCardButton(14-38)
apps/web/app/(org)/dashboard/folder/[id]/components/FolderVideosSection.tsx (4)
apps/web/app/(org)/dashboard/caps/Caps.tsx (1)
VideoData(29-50)apps/web/app/(org)/dashboard/Contexts.tsx (1)
useDashboardContext(44-44)apps/web/app/(org)/dashboard/caps/UploadingContext.tsx (1)
useUploadingStatus(50-62)apps/web/app/(org)/dashboard/caps/components/UploadPlaceholderCard.tsx (1)
UploadPlaceholderCard(10-90)
⏰ 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). (5)
- GitHub Check: Vercel Agent Review
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Analyze (rust)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (6)
apps/web/app/(org)/dashboard/spaces/[spaceId]/SharedCaps.tsx (1)
203-219: Non-owner drag message is unreachableonDragStart won’t fire for non-owners because CapCard sets draggable only for owners. The “Only the video owner can drag and move the video” branch never displays.
Consider showing a tooltip on pointer down for non-owners, or remove the else branch.
apps/web/app/(org)/dashboard/spaces/[spaceId]/folder/[folderId]/page.tsx (1)
73-76: LGTM: Updated FolderVideosSection API usagecardType prop removal aligns with the component’s new API.
apps/web/app/(org)/dashboard/folder/[id]/components/FolderVideosSection.tsx (1)
24-27: LGTM: Props simplifiedRemoving cardType cleans up the surface area without functional loss.
apps/web/app/(org)/dashboard/spaces/[spaceId]/components/SharedCapCard.tsx (1)
46-79: LGTM: CapCard usage matches refactorDropping sharedCapCard is safe here (no selection handlers passed). Owner label block remains correct.
apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx (2)
352-391: Confirm owner-only gating for quick actionsPR goal: only owners can perform additional actions. Copy and Download are currently visible to everyone; More options are owner-gated. Should Copy/Download also be owner-only?
If yes, wrap these buttons with {isOwner && …}.
290-304: LGTM: Drag restricted to ownersdraggable={isOwner && !anyCapSelected} and early return in handleDragStart correctly prevent drag by non-owners.
apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx
Outdated
Show resolved
Hide resolved
apps/web/app/(org)/dashboard/caps/components/CapCard/CapCardButton.tsx
Outdated
Show resolved
Hide resolved
apps/web/app/(org)/dashboard/folder/[id]/components/FolderVideosSection.tsx
Outdated
Show resolved
Hide resolved
| getFolderBreadcrumb(params.folderId), | ||
| getVideosByFolderId(params.folderId), | ||
| ]); | ||
| const userId = user?.id as string; |
There was a problem hiding this comment.
Remove unused variable
userId is declared but never used.
- const userId = user?.id as string;📝 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 userId = user?.id as string; |
🤖 Prompt for AI Agents
In apps/web/app/(org)/dashboard/spaces/[spaceId]/folder/[folderId]/page.tsx
around line 30, the constant "userId" is declared but never used; remove the
unused declaration (delete the line "const userId = user?.id as string;") or, if
the user id was intended to be used, replace the unused declaration by using
"user?.id" directly where needed or pass it into the relevant component/logic so
the variable is consumed.
apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx
Outdated
Show resolved
Hide resolved
apps/web/app/(org)/dashboard/spaces/[spaceId]/components/SharedCapCard.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
apps/web/app/(org)/dashboard/caps/components/CapCard/CapCardButton.tsx (4)
8-8: Type onClick as MouseEvent for better specificityImproves typing and JSX intellisense for the event target.
- onClick?: (e: MouseEvent) => void; + onClick?: (e: MouseEvent<HTMLButtonElement>) => void;
10-11: Make className optional and default to empty stringAvoids forcing callers to pass a className.
- className: string; + className?: string; @@ export const CapCardButton = ({ tooltipContent, onClick = () => {}, disabled, - className, + className = "", icon, }: CapCardButtonProps) => {Also applies to: 18-20
22-22: Remove redundant key prop on TooltipNot a list; key is unnecessary.
- <Tooltip key={tooltipContent} content={tooltipContent}> + <Tooltip content={tooltipContent}>
2-2: Use clsx/lite to trim bundle (string-only usage here)This component only composes strings; lite is sufficient.
-import clsx from "clsx"; +import clsx from "clsx/lite";apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx (1)
141-151: Align delete mutation with EffectRuntime (useEffectMutation)For consistency with app conventions and hooks already used in this file.
- const deleteMutation = useMutation({ - mutationFn: async () => { - await onDelete?.(); - }, - onError: (error) => { - console.error("Error deleting cap:", error); - }, - onSettled: () => { - setConfirmOpen(false); - }, - }); + const deleteMutation = useEffectMutation({ + mutationFn: () => + Effect.promise(async () => { + await onDelete?.(); + }), + onError: (error) => { + console.error("Error deleting cap:", error); + }, + onSettled: () => { + setConfirmOpen(false); + }, + });Note: Remove the now-unused
useMutationimport at the top.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx(5 hunks)apps/web/app/(org)/dashboard/caps/components/CapCard/CapCardButton.tsx(1 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/app/(org)/dashboard/caps/components/CapCard/CapCard.tsxapps/web/app/(org)/dashboard/caps/components/CapCard/CapCardButton.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/caps/components/CapCard/CapCard.tsxapps/web/app/(org)/dashboard/caps/components/CapCard/CapCardButton.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/app/(org)/dashboard/caps/components/CapCard/CapCard.tsxapps/web/app/(org)/dashboard/caps/components/CapCard/CapCardButton.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/app/(org)/dashboard/caps/components/CapCard/CapCard.tsxapps/web/app/(org)/dashboard/caps/components/CapCard/CapCardButton.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/app/(org)/dashboard/caps/components/CapCard/CapCard.tsxapps/web/app/(org)/dashboard/caps/components/CapCard/CapCardButton.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/app/(org)/dashboard/caps/components/CapCard/CapCard.tsxapps/web/app/(org)/dashboard/caps/components/CapCard/CapCardButton.tsx
🧬 Code graph analysis (1)
apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx (3)
apps/web/app/(org)/dashboard/Contexts.tsx (1)
useDashboardContext(44-44)apps/web/app/(org)/dashboard/caps/components/CapCard/CapCardButton.tsx (1)
CapCardButton(14-38)apps/web/app/(org)/dashboard/_components/ConfirmationDialog.tsx (1)
ConfirmationDialog(25-72)
⏰ 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). (4)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Vercel Agent Review
- GitHub Check: Analyze (rust)
🔇 Additional comments (1)
apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx (1)
321-326: Copy link should copy a full URL, not the raw IDBuild a shareable link (uses verified custom domain when present; falls back to current origin).
- onClick={(e) => { - e.stopPropagation(); - handleCopy(cap.id); - }} + onClick={(e) => { + e.stopPropagation(); + const url = + customDomain && domainVerified + ? `https://${customDomain}/s/${cap.id}` + : `${window.location.origin}/s/${cap.id}`; + handleCopy(url); + }}
Summary by CodeRabbit
New Features
Refactor
Chores