-
Notifications
You must be signed in to change notification settings - Fork 1.1k
feat: video privacy settings enhancement #827
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThis change introduces public/private sharing controls for videos ("caps"), allowing users to toggle the public visibility of videos in addition to sharing with organizations and spaces. It updates backend data models, API actions, UI components, and environment variable handling to support and reflect the new public sharing state throughout the application. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI (CapCard/SharingDialog)
participant Backend (shareCap)
participant DB
User->>UI (CapCard/SharingDialog): Opens sharing dialog for a video
UI->>User: Displays sharing options (spaces, orgs, public toggle)
User->>UI: Updates sharing (selects spaces/orgs, toggles public)
UI->>Backend (shareCap): Sends sharing update (spaces/orgs/public)
Backend (shareCap)->>DB: Updates sharing associations and public field
Backend (shareCap)-->>UI: Confirms sharing update
UI->>User: Shows updated sharing status and notifications
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/database/schema.ts (1)
231-231: Consider implementing the configurable default as previously discussed.Based on the past review comments, there was discussion about making the default privacy setting configurable via environment variables rather than hardcoding it to
false. This would provide flexibility for different deployment scenarios while maintaining the privacy-first approach.Consider implementing this as:
- public: boolean("public").notNull().default(false), + public: boolean("public").notNull().default(process.env.DEFAULT_VIDEO_PUBLIC === 'true'),This would allow teams to configure the default behavior while maintaining privacy-first as the safe default.
🧹 Nitpick comments (2)
apps/web/app/(org)/dashboard/caps/components/SharingDialog.tsx (1)
170-186: Review the space filtering logic for potential improvements.The current filtering logic that separates organization entries from spaces based on
space.id === space.organizationId && space.primary === trueseems fragile and could be improved:
- Consider using a more explicit property to identify organization entries
- The nested conditions with extra parentheses on lines 172-173 and 176-177 could be simplified
Consider refactoring to use a more explicit approach:
- const organizationEntries = spacesData?.filter((space) => - space.id === space.organizationId && space.primary === true - ) || []; - - const realSpaces = spacesData?.filter((space) => - !(space.id === space.organizationId && space.primary === true) - ) || []; + const organizationEntries = spacesData?.filter((space) => + space.primary === true + ) || []; + + const realSpaces = spacesData?.filter((space) => + space.primary !== true + ) || [];Or better yet, add a discriminator property to the Spaces type to make this distinction clearer.
apps/web/app/s/[videoId]/page.tsx (1)
38-92: Consider refactoring the helper function for better maintainability.The
getSharedSpacesForVideofunction works correctly but could be simplified and potentially moved to a shared utility file for reuse.Simplify the array building logic:
- const sharedSpaces: Array<{ - id: string; - name: string; - organizationId: string; - iconUrl?: string; - }> = []; - - // Add space-level sharing - spaceSharing.forEach(space => { - sharedSpaces.push({ - id: space.id, - name: space.name, - organizationId: space.organizationId, - iconUrl: space.iconUrl || undefined, - }); - }); - - // Add organization-level sharing - orgSharing.forEach(org => { - sharedSpaces.push({ - id: org.id, - name: org.name, - organizationId: org.organizationId, - iconUrl: org.iconUrl || undefined, - }); - }); - - return sharedSpaces; + return [ + ...spaceSharing.map(space => ({ + id: space.id, + name: space.name, + organizationId: space.organizationId, + iconUrl: space.iconUrl || undefined, + })), + ...orgSharing.map(org => ({ + id: org.id, + name: org.name, + organizationId: org.organizationId, + iconUrl: org.iconUrl || undefined, + })) + ];Also consider extracting this to a shared utilities file since it might be useful in other components.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
apps/web/actions/caps/share.ts(2 hunks)apps/web/app/(org)/dashboard/caps/Caps.tsx(1 hunks)apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx(2 hunks)apps/web/app/(org)/dashboard/caps/components/CapCard/CapCardContent.tsx(1 hunks)apps/web/app/(org)/dashboard/caps/components/SharingDialog.tsx(12 hunks)apps/web/app/(org)/dashboard/caps/page.tsx(1 hunks)apps/web/app/embed/[videoId]/page.tsx(3 hunks)apps/web/app/s/[videoId]/_components/ShareHeader.tsx(5 hunks)apps/web/app/s/[videoId]/page.tsx(8 hunks)packages/database/schema.ts(1 hunks)
⏰ 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: Clippy
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
🔇 Additional comments (14)
apps/web/app/embed/[videoId]/page.tsx (2)
45-62: LGTM! Excellent access control improvement.The addition of user access verification in metadata generation is a crucial security enhancement. This prevents private and password-protected videos from exposing metadata to unauthorized users, search engines, and social media crawlers.
166-171: Enhanced private video messaging improves user experience.The updated private video message provides clearer guidance with a proper heading and sign-in link, making it more actionable for video owners compared to the previous simple paragraph.
apps/web/app/(org)/dashboard/caps/Caps.tsx (1)
29-29: LGTM! Type extension aligns with public sharing feature.The addition of the
publicboolean property to theVideoDatatype properly supports the new public sharing functionality introduced across the application.apps/web/app/(org)/dashboard/caps/page.tsx (1)
148-148: LGTM! Database query correctly includes public field.The addition of
public: videos.publicto the select statement properly fetches the public sharing state from the database, enabling the UI components to display and manage the public visibility of videos.apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx (2)
37-37: LGTM! Interface extension supports public sharing.The addition of the optional
publicboolean property toCapCardPropsproperly extends the interface to support the new public sharing feature while maintaining backward compatibility.
260-260: LGTM! Proper integration with SharingDialog.Passing
cap.publicas theisPublicprop to theSharingDialogcomponent enables the sharing dialog to correctly display and manage the public sharing state of the video.apps/web/app/s/[videoId]/_components/ShareHeader.tsx (1)
21-180: LGTM! Well-implemented public sharing and space-level sharing features.The changes properly integrate the new public visibility state and space-level sharing:
- Correctly combines organization and space sharing checks into
hasSpaceSharing- Appropriately displays "Not shared" only when both space sharing and public visibility are false
- Props are properly typed with sensible defaults
apps/web/app/(org)/dashboard/caps/components/CapCard/CapCardContent.tsx (1)
147-150: LGTM! Consistent implementation with ShareHeader.The shared status logic correctly:
- Uses nullish coalescing operator (
?? 0) for safe array length checks- Maintains consistency with the ShareHeader component's logic
- Properly reflects both space-level and public sharing states
apps/web/actions/caps/share.ts (1)
7-133: LGTM! Clean implementation of public sharing backend logic.The changes properly handle the public visibility toggle:
- Correctly uses type guard (
typeof isPublic === 'boolean') to only update when explicitly provided- Maintains backward compatibility by making the parameter optional
- Properly structured database update using drizzle ORM
apps/web/app/(org)/dashboard/caps/components/SharingDialog.tsx (2)
48-77: LGTM! Well-structured state management for public sharing.The component properly initializes and tracks public sharing state alongside space sharing, with appropriate defaults and reset logic when the dialog opens.
237-251: LGTM! Clean UI implementation for public sharing toggle.The public sharing toggle UI is well-designed with:
- Clear visual hierarchy using icons and descriptive text
- Dynamic description based on toggle state
- Proper integration with the Cap UI Switch component
apps/web/app/s/[videoId]/page.tsx (3)
132-217: LGTM! Proper access control in metadata generation.The metadata generation correctly restricts information exposure for private and password-protected videos, only showing full metadata when the user has access.
315-324: Good UX improvement for private video message.The enhanced UI with styled message and sign-in link provides a better user experience compared to plain text.
352-366: LGTM! Robust error handling for spaces data fetching.The code properly handles potential errors when fetching dashboard data and provides a sensible fallback.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.env.example(1 hunks)apps/web/actions/video/upload.ts(1 hunks)apps/web/app/(org)/dashboard/caps/Caps.tsx(1 hunks)apps/web/app/api/desktop/[...route]/video.ts(1 hunks)packages/env/server.ts(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- apps/web/app/api/desktop/[...route]/video.ts
- packages/env/server.ts
- .env.example
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/app/(org)/dashboard/caps/Caps.tsx
⏰ 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: Clippy
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
Changes:
Test build:
ghcr.io/mogita/cap-web:video-privacy.2Summary by CodeRabbit
New Features
Improvements
Documentation