Implement videos page#622
Conversation
|
|
|
PR changed again? Review this PR in Change Stack to compare snapshots and stay oriented. Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds end-to-end video support: backend config, DB/table creation, folder integration, video utils and routes; frontend API clients, Plyr player, VideoCard, Videos page, and build config for Plyr. ChangesVideos feature
Sequence Diagram(s)sequenceDiagram
participant User
participant Frontend as Videos Page
participant API as /videos
participant Utils as video_util_process_folder_videos
participant DB as VideosDB
participant FS as FileSystem
User->>Frontend: open Videos page
Frontend->>API: GET /videos/
API->>DB: db_get_all_videos()
DB-->>API: videos list
API-->>Frontend: GetAllVideosResponse
alt scan requested or no videos
Frontend->>API: POST /videos/scan
API->>DB: db_get_all_folder_details()
API->>Utils: video_util_process_folder_videos(folder_data)
Utils->>FS: scan folders & read files
loop per file
Utils->>Utils: extract metadata (OpenCV)
Utils->>DB: db_insert_video(record)
end
API->>DB: db_get_all_videos()
API-->>Frontend: updated GetAllVideosResponse
end
User->>Frontend: click VideoCard
Frontend->>Frontend: init PlyrPlayer(src)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels: Suggested reviewers:
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 7
🧹 Nitpick comments (11)
frontend/src/components/ui/button.tsx (1)
8-8: Good UX improvement with cursor-pointer, but disabled:cursor-not-allowed is redundant.The
cursor-pointeraddition improves button UX by indicating clickability. However,disabled:cursor-not-allowedhas no effect becausedisabled:pointer-events-noneis already set—when pointer events are disabled, the browser won't display custom cursor styles.If you want to keep the class string cleaner, you can remove the redundant class:
- "inline-flex items-center justify-center gap-2 whitespace-nowrap rounded-md text-sm font-medium transition-all cursor-pointer disabled:pointer-events-none disabled:opacity-50 disabled:cursor-not-allowed [&_svg]:pointer-events-none [&_svg:not([class*='size-'])]:size-4 shrink-0 [&_svg]:shrink-0 outline-none focus-visible:border-ring focus-visible:ring-ring/50 focus-visible:ring-[3px] aria-invalid:ring-destructive/20 dark:aria-invalid:ring-destructive/40 aria-invalid:border-destructive", + "inline-flex items-center justify-center gap-2 whitespace-nowrap rounded-md text-sm font-medium transition-all cursor-pointer disabled:pointer-events-none disabled:opacity-50 [&_svg]:pointer-events-none [&_svg:not([class*='size-'])]:size-4 shrink-0 [&_svg]:shrink-0 outline-none focus-visible:border-ring focus-visible:ring-ring/50 focus-visible:ring-[3px] aria-invalid:ring-destructive/20 dark:aria-invalid:ring-destructive/40 aria-invalid:border-destructive",backend/app/schemas/videos.py (1)
5-13: Consider adding field validators for VideoMetadata.The VideoMetadata model lacks validation constraints. Consider adding validators to ensure data integrity (e.g., positive values for width, height, duration, and file_size).
Example validation:
from pydantic import BaseModel, Field class VideoMetadata(BaseModel): name: str date_created: Optional[str] = None width: int = Field(gt=0) height: int = Field(gt=0) duration: float = Field(gt=0) file_location: str file_size: int = Field(ge=0) item_type: strbackend/app/config/settings.py (1)
27-29: Consider using absolute paths for video storage.The relative paths
./videosand./videos/thumbnailsdepend on the current working directory. While this is consistent with the existingIMAGES_PATHpattern, consider usingpathlib.Pathwith absolute paths or environment variables for more robust path handling across different execution contexts.frontend/src/types/Media.ts (1)
40-48: Consider nullable thumbnailPath for backend consistency.The
thumbnailPathfield is typed asstring(line 43), but the backendVideoData.thumbnailPathisOptional[str]. Consider usingthumbnailPath: string | nullto handle cases where thumbnails might not be available.export interface Video { id: string; path: string; - thumbnailPath: string; + thumbnailPath: string | null; folder_id?: string; metadata?: VideoMetadata; title?: string; tags?: string[]; }backend/app/routes/folders.py (2)
71-73: Consider checking video processing return value.While
video_util_process_folder_videosreturns a boolean indicating success/failure, the return value is not checked. If video processing fails, the function continues silently. Consider logging the result or adding explicit error handling for better observability.# Process images and videos in all folders image_util_process_folder_images(folder_data) -video_util_process_folder_videos(folder_data) +video_success = video_util_process_folder_videos(folder_data) +if not video_success: + logger.warning(f"Video processing encountered errors for folder {folder_path}")
119-121: Consider checking video processing return value.Similar to the add folder sequence, the return value of
video_util_process_folder_videosis not checked here. Consider adding result validation for consistency and better error visibility.# Process images and videos in all folders image_util_process_folder_images(folder_data) -video_util_process_folder_videos(folder_data) +video_success = video_util_process_folder_videos(folder_data) +if not video_success: + logger.warning(f"Video processing encountered errors during sync for folder {folder_path}") image_util_process_untagged_images()frontend/src/components/VideoPlayer/PlyrPlayer.tsx (1)
102-102: Consider supporting multiple video formats.The video source is hardcoded to
type="video/mp4", which may not support other formats like WebM or OGV.If you need to support multiple formats, consider:
- <source src={src} type="video/mp4" /> + <source src={src} />Or detect the format from the file extension and set the appropriate MIME type.
backend/app/routes/videos.py (1)
24-33: Consider extracting duplicate VideoData mapping logic.The same VideoData construction pattern appears in all four endpoints. Extracting this into a helper function would improve maintainability.
Example:
def _map_videos_to_response(videos: List[dict]) -> List[VideoData]: """Helper to map database video dicts to VideoData schema.""" return [ VideoData( id=v["id"], path=v["path"], folder_id=v.get("folder_id"), thumbnailPath=v["thumbnailPath"], metadata=image_util_parse_metadata(v.get("metadata")), ) for v in videos ] # Then use in endpoints: data = _map_videos_to_response(videos)Also applies to: 57-66, 100-109, 133-142
backend/app/utils/videos.py (3)
55-81: Consider refactoring the import and directory check.Two minor inefficiencies:
Import inside function (line 59): The local import of
db_get_video_by_pathsuggests a circular dependency issue. Consider restructuring imports or using TYPE_CHECKING to resolve this more cleanly.Redundant directory check (line 61):
video_util_ensure_dirs()is called for every file registration. While harmless (due toexist_ok=True), it's unnecessary overhead. Consider calling it once during application startup or at the beginning of batch operations likevideo_util_process_folder_videos.For the import issue, consider moving it to the top with TYPE_CHECKING:
from typing import TYPE_CHECKING if TYPE_CHECKING: from app.database.videos import db_get_video_by_pathOr restructure the module dependencies to eliminate the circular import.
90-108: Add consistent error handling for recursive mode.The non-recursive mode catches
OSError(line 106), but the recursive mode usingos.walk(lines 94-99) doesn't. Theos.walkfunction can raiseOSErrorfor permission issues or other filesystem errors when traversing subdirectories, leading to inconsistent behavior.Apply this diff to add consistent error handling:
def video_util_get_videos_from_folder( folder_path: str, recursive: bool = True ) -> List[str]: videos: List[str] = [] if recursive: - for root, _, files in os.walk(folder_path): - for f in files: - p = os.path.join(root, f) - if video_util_is_valid_video(p): - videos.append(p) + try: + for root, _, files in os.walk(folder_path): + for f in files: + p = os.path.join(root, f) + if video_util_is_valid_video(p): + videos.append(p) + except OSError: + pass else: try: for f in os.listdir(folder_path): p = os.path.join(folder_path, f) if os.path.isfile(p) and video_util_is_valid_video(p): videos.append(p) except OSError: pass return videos
114-126: Add logging for top-level exceptions.The top-level exception handler (lines 125-126) silently returns
Falsewithout logging the error. This makes it difficult to diagnose why folder processing failed, especially since the function processes multiple folders and could fail for various reasons (filesystem issues, database errors, etc.).Apply this diff to add error logging:
def video_util_process_folder_videos(folder_data: List[essential_tuple]) -> bool: try: video_util_ensure_dirs() for path, folder_id, recursive in folder_data: file_list = video_util_get_videos_from_folder(path, recursive) for fp in file_list: try: video_util_register_file(fp, folder_id) except Exception: continue return True - except Exception: + except Exception as e: + logger.error(f"Failed to process folder videos: {e}") return False
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (19)
backend/app/config/settings.py(1 hunks)backend/app/database/videos.py(1 hunks)backend/app/routes/folders.py(3 hunks)backend/app/routes/videos.py(1 hunks)backend/app/schemas/videos.py(1 hunks)backend/app/utils/videos.py(1 hunks)backend/main.py(3 hunks)frontend/package.json(1 hunks)frontend/src/api/api-functions/index.ts(1 hunks)frontend/src/api/api-functions/videos.ts(1 hunks)frontend/src/api/apiEndpoints.ts(1 hunks)frontend/src/components/Media/VideoCard.tsx(1 hunks)frontend/src/components/VideoPlayer/NetflixStylePlayer.tsx(0 hunks)frontend/src/components/VideoPlayer/PlyrPlayer.tsx(1 hunks)frontend/src/components/ui/button.tsx(1 hunks)frontend/src/pages/VideosPage/Videos.tsx(1 hunks)frontend/src/routes/AppRoutes.tsx(1 hunks)frontend/src/types/Media.ts(2 hunks)frontend/vite.config.ts(1 hunks)
💤 Files with no reviewable changes (1)
- frontend/src/components/VideoPlayer/NetflixStylePlayer.tsx
🧰 Additional context used
🧬 Code graph analysis (10)
frontend/src/api/api-functions/videos.ts (3)
frontend/src/types/API.ts (1)
APIResponse(1-8)frontend/src/api/axiosConfig.ts (1)
apiClient(5-12)frontend/src/api/apiEndpoints.ts (1)
videosEndpoints(5-9)
frontend/src/types/Media.ts (1)
backend/app/schemas/videos.py (1)
VideoMetadata(5-13)
frontend/src/routes/AppRoutes.tsx (1)
frontend/src/constants/routes.ts (1)
ROUTES(1-11)
frontend/src/pages/VideosPage/Videos.tsx (6)
frontend/src/types/API.ts (1)
APIResponse(1-8)frontend/src/api/api-functions/videos.ts (2)
fetchAllVideos(5-10)scanVideos(25-28)frontend/src/types/Media.ts (1)
Video(40-48)frontend/src/components/ui/LoadingScreen/LoadingScreen.tsx (1)
LoadingScreen(15-64)frontend/src/components/Media/VideoCard.tsx (1)
VideoCard(13-52)frontend/src/components/VideoPlayer/PlyrPlayer.tsx (1)
PlyrPlayer(10-106)
backend/app/schemas/videos.py (1)
frontend/src/types/Media.ts (1)
VideoMetadata(13-22)
backend/app/utils/videos.py (1)
backend/app/database/videos.py (2)
db_insert_video(52-75)db_get_video_by_path(128-156)
backend/app/routes/videos.py (5)
backend/app/database/videos.py (1)
db_get_all_videos(78-125)backend/app/schemas/videos.py (3)
GetAllVideosResponse(24-27)ErrorResponse(30-33)VideoData(16-21)backend/app/utils/images.py (1)
image_util_parse_metadata(496-513)backend/app/utils/videos.py (3)
video_util_register_file(55-81)video_util_ensure_dirs(15-16)video_util_process_folder_videos(114-126)backend/app/database/folders.py (1)
db_get_all_folder_details(397-418)
frontend/src/components/Media/VideoCard.tsx (1)
frontend/src/types/Media.ts (1)
Video(40-48)
backend/app/routes/folders.py (2)
backend/app/utils/videos.py (1)
video_util_process_folder_videos(114-126)backend/app/utils/images.py (1)
image_util_process_folder_images(32-84)
backend/main.py (1)
backend/app/database/videos.py (1)
db_create_videos_table(31-49)
⏰ 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: sync-labels
- GitHub Check: Backend Tests
- GitHub Check: Tauri Build Check (windows-latest)
- GitHub Check: Tauri Build Check (macos-latest, --target aarch64-apple-darwin)
- GitHub Check: Tauri Build Check (ubuntu-22.04)
🔇 Additional comments (18)
frontend/src/routes/AppRoutes.tsx (1)
11-11: LGTM!The Videos component import and route registration follow the existing patterns and integrate cleanly with the route structure.
Also applies to: 19-19
frontend/vite.config.ts (1)
17-19: LGTM!Pre-bundling plyr in
optimizeDepsis the recommended approach for improving dev server startup time and handling potential ESM compatibility issues with the library.frontend/src/api/api-functions/index.ts (1)
4-4: LGTM!The videos module export follows the established pattern for API function organization.
frontend/src/types/Media.ts (1)
13-22: LGTM!The VideoMetadata interface correctly mirrors the backend schema fields and types, ensuring consistent data shape across frontend and backend.
backend/app/routes/folders.py (1)
45-45: LGTM!The import of video processing utilities integrates cleanly with the existing folder processing infrastructure.
frontend/package.json (1)
56-56: plyr 3.8.3 is current and secure.Version 3.8.3 is the latest release with no known security vulnerabilities or CVEs. The package was released August 27, 2025, indicating active maintenance. No action needed.
backend/app/schemas/videos.py (1)
21-21: The code is correct for the project's Python version requirement.The union syntax
Mapping[str, Any] | VideoMetadatais appropriate here. The project's GitHub Actions workflows explicitly specify Python 3.12, which fully supports PEP 604 union syntax (available since Python 3.10). No changes are needed.frontend/src/api/apiEndpoints.ts (1)
5-9: LGTM!The new
videosEndpointsobject follows the established pattern and provides clear endpoint definitions for video operations.backend/main.py (1)
22-22: LGTM!The video module integration follows the established pattern for other entities in the application. The table creation is correctly placed in the startup sequence, and the router is properly mounted with appropriate tags.
Also applies to: 28-28, 58-58, 132-132
frontend/src/api/api-functions/videos.ts (1)
5-28: LGTM!The API functions follow a clean pattern and leverage the existing
apiClientconfiguration. Error handling appears to be delegated to the caller (React Query), which is a reasonable approach for this architecture.frontend/src/pages/VideosPage/Videos.tsx (1)
68-141: LGTM!The UI implementation is well-structured with responsive grid layout, sorting controls, and a modal player. The component provides a good user experience with loading states and empty states.
frontend/src/components/Media/VideoCard.tsx (1)
13-52: Well-structured video card component.The component provides good UX with hover effects, play button overlay, and responsive design. The video metadata fallback chain is robust.
frontend/src/components/VideoPlayer/PlyrPlayer.tsx (1)
10-106: Excellent player implementation with proper cleanup.The component handles Plyr initialization, configuration, and cleanup correctly. The dynamic import approach and effect dependencies are appropriate.
backend/app/database/videos.py (4)
31-49: LGTM!The table schema is well-designed with appropriate constraints. The foreign key relationship with
ON DELETE SET NULLensures data integrity when folders are removed.
52-76: LGTM!The upsert logic using
ON CONFLICTis appropriate for handling duplicate paths, and error handling with rollback is properly implemented.
78-126: Excellent auto-cleanup implementation.The automatic cleanup of database entries for deleted files is a smart approach that keeps the database in sync with the filesystem. The implementation is safe and well-logged.
128-171: LGTM!Both query and delete functions are well-implemented with proper error handling and connection management.
backend/app/utils/videos.py (1)
15-16: Now let me search for broader "thumbnail" references in the codebase:Remove
THUMBNAIL_VIDEOS_PATHfrom settings or implement directory creation.The constant is defined in
backend/app/config/settings.py(line 29) but never used anywhere in the codebase. Since the PR optimizes by avoiding unnecessary thumbnails (line 77 setsthumbnailPathtoNone), either remove this unused constant from settings to prevent confusion, or addos.makedirs(THUMBNAIL_VIDEOS_PATH, exist_ok=True)tovideo_util_ensure_dirs()if thumbnail functionality is planned for the future.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
backend/app/routes/videos.py (4)
9-9: Consider adding tags to the router for better API documentation.Adding tags helps organize endpoints in the automatically generated OpenAPI/Swagger documentation.
Apply this diff:
-router = APIRouter() +router = APIRouter(tags=["videos"])
12-39: Consider pagination for scalability.The endpoint returns all videos without pagination, which could cause performance issues and large response sizes with extensive video libraries.
Consider adding pagination parameters:
@router.get( "/", response_model=GetAllVideosResponse, responses={500: {"model": ErrorResponse}}, ) def get_all_videos(skip: int = 0, limit: int = 100): try: videos = db_get_all_videos() # Apply pagination paginated_videos = videos[skip:skip + limit] data: List[VideoData] = [ VideoData( id=v["id"], path=v["path"], folder_id=v.get("folder_id"), thumbnailPath=v["thumbnailPath"], metadata=image_util_parse_metadata(v.get("metadata")), ) for v in paginated_videos ] return GetAllVideosResponse( success=True, message=f"Retrieved {len(data)} videos (showing {skip}-{skip+len(data)} of {len(videos)} total)", data=data ) except Exception as e: raise HTTPException( status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail=ErrorResponse( success=False, error="Internal server error", message=str(e) ).model_dump(), )
47-47: Removeasynckeyword or use asynchronous operations.The function is declared as
asyncbut doesn't use anyawaitexpressions. This creates unnecessary overhead and inconsistency withget_all_videos, which is synchronous.Apply this diff:
-async def scan_videos_from_folders(): +def scan_videos_from_folders():
80-80: Removeasynckeyword or use asynchronous operations.Similar to
scan_videos_from_folders, this function is declared asasyncbut doesn't use anyawaitexpressions, creating inconsistency withget_all_videos.Apply this diff:
-async def cleanup_deleted_videos(): +def cleanup_deleted_videos():
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/app/routes/videos.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
backend/app/routes/videos.py (5)
backend/app/database/videos.py (1)
db_get_all_videos(78-125)backend/app/schemas/videos.py (3)
GetAllVideosResponse(24-27)ErrorResponse(30-33)VideoData(16-21)backend/app/utils/images.py (1)
image_util_parse_metadata(496-513)backend/app/utils/videos.py (1)
video_util_process_folder_videos(114-126)backend/app/database/folders.py (1)
db_get_all_folder_details(397-418)
⏰ 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: Backend Tests
- GitHub Check: Tauri Build Check (ubuntu-22.04)
- GitHub Check: Tauri Build Check (macos-latest, --target aarch64-apple-darwin)
- GitHub Check: Tauri Build Check (windows-latest)
🔇 Additional comments (3)
backend/app/routes/videos.py (3)
80-81: LGTM! The cleanup mechanism works via implicit behavior.The endpoint correctly leverages the automatic cleanup behavior in
db_get_all_videos, which removes database entries for videos whose files no longer exist (as shown in the relevant code snippet frombackend/app/database/videos.py). The docstring clearly explains the purpose.
33-39: LGTM! Error handling is consistent and appropriate.The error handling pattern correctly uses
ErrorResponse.model_dump()to convert the Pydantic model to a dictionary for the HTTPException detail field. The consistent structure across all endpoints makes the API predictable.Also applies to: 66-72, 99-105
50-50: Review comment is incorrect—AI_Tagging and recursive scanning are unrelated features.The hardcoded
recursive=Falseis intentional system-wide behavior (replicated inbackend/app/routes/folders.py:68with an explicit comment). The database schema has no recursive field; it's not configurable per folder.AI_Tagging is a separate per-folder feature flag for AI-based image tagging (queried in
backend/app/database/images.py), not a control for scan recursion. It does not indicate or require recursive folder scanning.Likely an incorrect or invalid review comment.
|
@SiddharthJiyani Please resolve conflicts. |
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 (3)
frontend/src/api/apiEndpoints.ts(1 hunks)frontend/src/routes/AppRoutes.tsx(1 hunks)frontend/src/types/Media.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/src/api/apiEndpoints.ts
- frontend/src/routes/AppRoutes.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/src/types/Media.ts (1)
backend/app/schemas/videos.py (1)
VideoMetadata(5-13)
⏰ 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: Backend Tests
- GitHub Check: Tauri Build Check (macos-latest, --target aarch64-apple-darwin)
- GitHub Check: Tauri Build Check (ubuntu-22.04)
- GitHub Check: Tauri Build Check (windows-latest)
🔇 Additional comments (2)
frontend/src/types/Media.ts (2)
13-22: LGTM! VideoMetadata interface aligns with backend schema.The VideoMetadata interface correctly mirrors the backend schema with appropriate TypeScript types and includes the essential
durationfield for video playback.
24-34: The review comment is incorrect.The git diff shows that
folder_id: stringwas already required in the codebase before this PR and was not modified. The line appears as unchanged context in the diff, not as a new addition or modification. The actual changes in this PR are:
- Added
VideoMetadatainterface- Added
isFavourite?: booleanto theImageinterface- Added
Videointerface with optionalfolder_id?: string- Added
images: Image[]property toMediaViewPropsThere is no breaking change to
Image.folder_idin this PR.Likely an incorrect or invalid review comment.
Done |
|
Please resolve the merge conflicts before review. Your PR will only be reviewed by a maintainer after all conflicts have been resolved. 📺 Watch this video to understand why conflicts occur and how to resolve them: |
|
The Please resolve the merge conflicts or leave a comment if you are still actively working on it. If there is no further activity within the next 5 days, this PR may be automatically closed to help keep the PR backlog manageable. |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds first-class video support across frontend and backend: listing videos, scanning folders for videos, and playing them in-app using Plyr.
Changes:
- Frontend: adds Videos page/route, video card UI, and Plyr-based player (plus
plyrdependency). - Backend: adds videos table + CRUD helpers, video metadata extraction/registration utilities, and
/videosroutes (list/scan/cleanup). - Folder workflows updated to process videos alongside images.
Reviewed changes
Copilot reviewed 19 out of 20 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/vite.config.ts | Pre-bundles plyr to avoid dev-time optimization issues. |
| frontend/src/types/Media.ts | Introduces Video / VideoMetadata types. |
| frontend/src/routes/AppRoutes.tsx | Routes /videos to the new Videos page. |
| frontend/src/pages/VideosPage/Videos.tsx | Implements videos listing, sorting, scanning fallback, and modal playback. |
| frontend/src/components/ui/button.tsx | Adjusts cursor/disabled cursor behavior for buttons. |
| frontend/src/components/VideoPlayer/PlyrPlayer.tsx | Adds Plyr wrapper component and loads Plyr dynamically. |
| frontend/src/components/VideoPlayer/NetflixStylePlayer.tsx | Removes old custom video player implementation. |
| frontend/src/components/Media/VideoCard.tsx | Adds a clickable video preview card. |
| frontend/src/api/apiEndpoints.ts | Adds videos API endpoint constants. |
| frontend/src/api/api-functions/videos.ts | Adds fetch/upload/scan API helpers for videos. |
| frontend/src/api/api-functions/index.ts | Re-exports videos API helpers. |
| frontend/package.json | Adds plyr dependency. |
| backend/main.py | Initializes videos table and registers videos router; starts sync microservice. |
| backend/app/utils/videos.py | Implements video scanning, validation, metadata extraction, and DB registration. |
| backend/app/schemas/videos.py | Adds Pydantic schemas for video responses. |
| backend/app/routes/videos.py | Adds /videos routes: list, scan, cleanup. |
| backend/app/routes/folders.py | Processes videos when folders are added/synced. |
| backend/app/database/videos.py | Adds videos table and DB operations, plus cleanup of missing files. |
| backend/app/config/settings.py | Adds videos storage paths. |
Files not reviewed (1)
- frontend/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import { AITagging } from '@/pages/AITagging/AITagging'; | ||
| import { PersonImages } from '@/pages/PersonImages/PersonImages'; | ||
| import { ComingSoon } from '@/pages/ComingSoon/ComingSoon'; | ||
| import Videos from '@/pages/VideosPage/Videos'; |
| <Route element={<Layout />}> | ||
| <Route path={ROUTES.HOME} element={<Home />} /> | ||
| <Route path={ROUTES.VIDEOS} element={<ComingSoon />} /> | ||
| <Route path={ROUTES.VIDEOS} element={<Videos />} /> |
| ); | ||
| }; |
| useEffect(() => { | ||
| const run = async () => { | ||
| if (data?.success && Array.isArray(data.data)) { | ||
| const list = data.data as unknown as Video[]; | ||
| setVideos(list); | ||
| if (list.length === 0) { | ||
| try { | ||
| const scanned = await scanVideos(); | ||
| if (scanned?.success && Array.isArray(scanned.data)) { | ||
| setVideos(scanned.data as unknown as Video[]); | ||
| } | ||
| } catch (error) { | ||
| console.error('Failed to scan videos:', error); | ||
| // Videos will remain empty, showing "No videos found" message | ||
| } | ||
| } | ||
| } | ||
| }; | ||
| run(); | ||
| }, [data]); |
| <video | ||
| ref={ref} | ||
| playsInline | ||
| crossOrigin="anonymous" | ||
| poster={poster} | ||
| controls | ||
| className="h-full w-full" | ||
| > | ||
| <source src={src} type="video/mp4" /> | ||
| </video> |
| import os | ||
| import uuid | ||
| import json | ||
| import datetime | ||
| import cv2 | ||
| from typing import Dict, List, Tuple |
| def video_util_process_folder_videos(folder_data: List[essential_tuple]) -> bool: | ||
| try: | ||
| video_util_ensure_dirs() | ||
| for path, folder_id, recursive in folder_data: | ||
| file_list = video_util_get_videos_from_folder(path, recursive) | ||
| for fp in file_list: | ||
| try: | ||
| video_util_register_file(fp, folder_id) | ||
| except Exception: | ||
| continue | ||
| return True | ||
| except Exception: | ||
| return False |
| videos.append( | ||
| { | ||
| "id": v_id, | ||
| "path": path, | ||
| "folder_id": str(folder_id) if folder_id is not None else "", | ||
| "thumbnailPath": thumb, | ||
| "metadata": metadata, | ||
| } | ||
| ) |
| from app.schemas.videos import GetAllVideosResponse, ErrorResponse, VideoData | ||
| from app.utils.images import image_util_parse_metadata |
| def _map_videos_to_response(videos: List[dict]) -> List[VideoData]: | ||
| """Helper function to map database video dicts to VideoData objects.""" | ||
| return [ | ||
| VideoData( | ||
| id=v["id"], | ||
| path=v["path"], | ||
| folder_id=v.get("folder_id"), | ||
| thumbnailPath=v["thumbnailPath"], | ||
| metadata=image_util_parse_metadata(v.get("metadata")), | ||
| ) |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/main.py`:
- Around line 169-170: The server is currently bound to localhost
(host="localhost") which prevents external/Docker/LAN access; change the binding
to allow external access by default (for example use 0.0.0.0) or make the host
configurable via an environment variable (e.g., BIND_HOST) and default to an
externally reachable address, updating the host="localhost" occurrences in
backend/main.py (the server start call) and add a short comment documenting if
loopback-only binding is intentional.
- Around line 49-50: When creating the DB parent directory, guard against an
empty dirname: compute path = os.path.dirname(DATABASE_PATH) and only call
os.makedirs(path, exist_ok=True) when path is non-empty (e.g., if path). Update
the block that uses DATABASE_PATH and os.path.dirname to skip os.makedirs when
dirname returns "" so startup won't call os.makedirs on an empty string.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4c5c8595-5f9d-4cd9-8897-06ed11ce63ae
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
backend/app/config/settings.pybackend/app/routes/folders.pybackend/main.pyfrontend/package.jsonfrontend/src/api/api-functions/index.tsfrontend/src/api/apiEndpoints.tsfrontend/src/routes/AppRoutes.tsxfrontend/src/types/Media.ts
✅ Files skipped from review due to trivial changes (2)
- frontend/src/api/api-functions/index.ts
- frontend/package.json
🚧 Files skipped from review as they are similar to previous changes (4)
- frontend/src/routes/AppRoutes.tsx
- frontend/src/api/apiEndpoints.ts
- backend/app/routes/folders.py
- backend/app/config/settings.py
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/main.py`:
- Around line 169-170: The server is currently bound to localhost
(host="localhost") which prevents external/Docker/LAN access; change the binding
to allow external access by default (for example use 0.0.0.0) or make the host
configurable via an environment variable (e.g., BIND_HOST) and default to an
externally reachable address, updating the host="localhost" occurrences in
backend/main.py (the server start call) and add a short comment documenting if
loopback-only binding is intentional.
- Around line 49-50: When creating the DB parent directory, guard against an
empty dirname: compute path = os.path.dirname(DATABASE_PATH) and only call
os.makedirs(path, exist_ok=True) when path is non-empty (e.g., if path). Update
the block that uses DATABASE_PATH and os.path.dirname to skip os.makedirs when
dirname returns "" so startup won't call os.makedirs on an empty string.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4c5c8595-5f9d-4cd9-8897-06ed11ce63ae
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
backend/app/config/settings.pybackend/app/routes/folders.pybackend/main.pyfrontend/package.jsonfrontend/src/api/api-functions/index.tsfrontend/src/api/apiEndpoints.tsfrontend/src/routes/AppRoutes.tsxfrontend/src/types/Media.ts
✅ Files skipped from review due to trivial changes (2)
- frontend/src/api/api-functions/index.ts
- frontend/package.json
🚧 Files skipped from review as they are similar to previous changes (4)
- frontend/src/routes/AppRoutes.tsx
- frontend/src/api/apiEndpoints.ts
- backend/app/routes/folders.py
- backend/app/config/settings.py
🛑 Comments failed to post (2)
backend/main.py (2)
49-50:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard empty parent directory before
makedirsAt Line 49-50,
os.path.dirname(DATABASE_PATH)may be""(e.g., filename-only DB path), which makesos.makedirs("")fail during startup.Suggested fix
-path = os.path.dirname(DATABASE_PATH) -os.makedirs(path, exist_ok=True) +path = os.path.dirname(DATABASE_PATH) +if path: + os.makedirs(path, exist_ok=True)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/main.py` around lines 49 - 50, When creating the DB parent directory, guard against an empty dirname: compute path = os.path.dirname(DATABASE_PATH) and only call os.makedirs(path, exist_ok=True) when path is non-empty (e.g., if path). Update the block that uses DATABASE_PATH and os.path.dirname to skip os.makedirs when dirname returns "" so startup won't call os.makedirs on an empty string.
169-170:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid binding only to loopback unless intentionally local-only
At Line 169-170 (and reflected in Line 93), binding to
localhostrestricts access to the same host. This can break Docker/LAN access and cross-service integration if the backend is expected to be reachable externally.Suggested fix
- host="localhost", + host="0.0.0.0",- {"url": "http://localhost:52123", "description": "Local Development server"} + {"url": "http://0.0.0.0:52123", "description": "Local Development server"}If local-only binding is intentional, document that explicitly in this file to prevent deployment regressions.
Also applies to: 93-93
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/main.py` around lines 169 - 170, The server is currently bound to localhost (host="localhost") which prevents external/Docker/LAN access; change the binding to allow external access by default (for example use 0.0.0.0) or make the host configurable via an environment variable (e.g., BIND_HOST) and default to an externally reachable address, updating the host="localhost" occurrences in backend/main.py (the server start call) and add a short comment documenting if loopback-only binding is intentional.
|
Please resolve the merge conflicts before review. Your PR will only be reviewed by a maintainer after all conflicts have been resolved. 📺 Watch this video to understand why conflicts occur and how to resolve them: |
Feat: Implement Videos Page with Dynamic Thumbnails
Overview
Adds full video management functionality with an optimized approach that eliminates unnecessary thumbnail storage and provides automatic database cleanup.
Key Changes
Backend
Frontend
Technical Details
Testing
Demo
video_page_implementation_.1.mp4
Summary by CodeRabbit
New Features
Removed