-
-
Notifications
You must be signed in to change notification settings - Fork 3
share feature #10
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
base: main
Are you sure you want to change the base?
share feature #10
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -8,7 +8,12 @@ | |||||||||
|
|
||||||||||
| from app.services.manim import generate_manim_script, generate_improved_code | ||||||||||
| from app.services.video_renderer import render_animation | ||||||||||
| from app.services.database_service import upload_video, get_user_videos, get_current_user, get_supabase, create_chat_in_db, get_user_chats_from_db, delete_chat_from_db, get_chat_tasks_from_db, ensure_user_exists, get_user_credits, deduct_credit | ||||||||||
| from app.services.database_service import ( | ||||||||||
| upload_video, get_user_videos, get_current_user, get_supabase, | ||||||||||
| create_chat_in_db, get_user_chats_from_db, delete_chat_from_db, | ||||||||||
| get_chat_tasks_from_db, ensure_user_exists, get_user_credits, deduct_credit, | ||||||||||
| toggle_video_sharing, toggle_chat_sharing | ||||||||||
| ) | ||||||||||
|
|
||||||||||
| import logging | ||||||||||
| logger = logging.getLogger(__name__) | ||||||||||
|
|
@@ -339,3 +344,56 @@ async def list_videos(user_identity: tuple[str, str] = Depends(get_current_user) | |||||||||
| return await get_user_videos(user_id) | ||||||||||
| except Exception as e: | ||||||||||
| return [] | ||||||||||
|
|
||||||||||
|
|
||||||||||
| # ============================================================================ | ||||||||||
| # SHARING ENDPOINTS | ||||||||||
| # ============================================================================ | ||||||||||
|
|
||||||||||
| class ShareRequest(BaseModel): | ||||||||||
| isPublic: bool | ||||||||||
|
|
||||||||||
|
|
||||||||||
| @router.post("/videos/{video_id}/share") | ||||||||||
| async def toggle_video_share( | ||||||||||
| video_id: str, | ||||||||||
| request: ShareRequest, | ||||||||||
| user_identity: tuple[str, str] = Depends(get_current_user) | ||||||||||
| ): | ||||||||||
| """Toggle video sharing on/off.""" | ||||||||||
| user_id, _ = user_identity | ||||||||||
| try: | ||||||||||
| result = await toggle_video_sharing(video_id, user_id, request.isPublic) | ||||||||||
| return { | ||||||||||
| "success": True, | ||||||||||
| "video": result, | ||||||||||
| "message": f"Video {'shared' if request.isPublic else 'unshared'} successfully" | ||||||||||
| } | ||||||||||
| except HTTPException: | ||||||||||
| raise | ||||||||||
| except Exception as e: | ||||||||||
| print(f"[Share] Error toggling video sharing: {e}") | ||||||||||
| raise HTTPException(status_code=500, detail="Failed to update sharing status") | ||||||||||
|
|
||||||||||
|
|
||||||||||
| @router.post("/chats/{chat_id}/share") | ||||||||||
| async def toggle_chat_share( | ||||||||||
| chat_id: str, | ||||||||||
| request: ShareRequest, | ||||||||||
| user_identity: tuple[str, str] = Depends(get_current_user) | ||||||||||
| ): | ||||||||||
| """Toggle chat sharing on/off.""" | ||||||||||
| user_id, _ = user_identity | ||||||||||
| try: | ||||||||||
| result = await toggle_chat_sharing(chat_id, user_id, request.isPublic) | ||||||||||
| return { | ||||||||||
| "success": True, | ||||||||||
| "chat": result, | ||||||||||
| "message": f"Chat {'shared' if request.isPublic else 'unshared'} successfully" | ||||||||||
| } | ||||||||||
| except HTTPException: | ||||||||||
| raise | ||||||||||
| except Exception as e: | ||||||||||
| print(f"[Share] Error toggling chat sharing: {e}") | ||||||||||
| raise HTTPException(status_code=500, detail="Failed to update sharing status") | ||||||||||
|
Comment on lines
+397
to
+398
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar to the other new endpoint, it's better to use the
Suggested change
|
||||||||||
|
|
||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,51 @@ | ||||||||||
| from fastapi import APIRouter, HTTPException | ||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||||||||||
| from app.services.database_service import ( | ||||||||||
| get_public_video, | ||||||||||
| get_public_chat, | ||||||||||
| get_public_chat_tasks, | ||||||||||
| increment_video_views, | ||||||||||
| increment_chat_views | ||||||||||
| ) | ||||||||||
|
|
||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||||||||||
| router = APIRouter() | ||||||||||
|
|
||||||||||
|
|
||||||||||
| @router.get("/video/{video_id}") | ||||||||||
| async def get_shared_video(video_id: str): | ||||||||||
|
Comment on lines
+13
to
+14
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To use
Suggested change
|
||||||||||
| """Get public video details (no authentication required).""" | ||||||||||
| try: | ||||||||||
| video = await get_public_video(video_id) | ||||||||||
|
|
||||||||||
| # Increment view count in background | ||||||||||
| await increment_video_views(video_id) | ||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Incrementing the view count is a side effect that doesn't need to block the user's request. Awaiting it here adds latency to the response. Consider using
Suggested change
|
||||||||||
|
|
||||||||||
| return video | ||||||||||
| except HTTPException: | ||||||||||
| raise | ||||||||||
| except Exception as e: | ||||||||||
| print(f"[Share] Error fetching video: {e}") | ||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using
Suggested change
|
||||||||||
| raise HTTPException(status_code=500, detail="Failed to fetch video") | ||||||||||
|
|
||||||||||
|
|
||||||||||
| @router.get("/chat/{chat_id}") | ||||||||||
| async def get_shared_chat(chat_id: str): | ||||||||||
|
Comment on lines
+30
to
+31
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To run the view count increment as a background task, you need to inject
Suggested change
|
||||||||||
| """Get public chat details with all tasks (no authentication required).""" | ||||||||||
| try: | ||||||||||
| # Get chat metadata | ||||||||||
| chat = await get_public_chat(chat_id) | ||||||||||
|
|
||||||||||
| # Get all tasks/videos in the chat | ||||||||||
| tasks = await get_public_chat_tasks(chat_id) | ||||||||||
|
|
||||||||||
| # Increment view count in background | ||||||||||
| await increment_chat_views(chat_id) | ||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Awaiting the view count increment here can delay the response to the user. Since this operation is not critical to the immediate response, it's best to run it as a background task to improve performance.
Suggested change
|
||||||||||
|
|
||||||||||
| return { | ||||||||||
| "chat": chat, | ||||||||||
| "tasks": tasks | ||||||||||
| } | ||||||||||
| except HTTPException: | ||||||||||
| raise | ||||||||||
| except Exception as e: | ||||||||||
| print(f"[Share] Error fetching chat: {e}") | ||||||||||
| raise HTTPException(status_code=500, detail="Failed to fetch chat") | ||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -286,3 +286,147 @@ async def delete_video(video_id: str, user_id: str) -> bool: | |
| client.table("videos").delete().eq("id", video_id).execute() | ||
|
|
||
| return True | ||
|
|
||
|
|
||
| # ============================================================================ | ||
| # SHARING FUNCTIONALITY | ||
| # ============================================================================ | ||
|
|
||
| async def toggle_video_sharing(video_id: str, user_id: str, is_public: bool) -> dict: | ||
| """Toggle video sharing on/off. Returns updated video data.""" | ||
| client = get_supabase() | ||
|
|
||
| # Verify ownership | ||
| video = client.table("videos").select("*").eq("id", video_id).eq("user_id", user_id).single().execute() | ||
| if not video.data: | ||
| raise HTTPException(status_code=404, detail="Video not found") | ||
|
|
||
| # Prepare update | ||
| updates = { | ||
| "is_public": is_public, | ||
| "updated_at": datetime.now(timezone.utc).isoformat() | ||
| } | ||
|
|
||
| # Set sharedAt timestamp when first enabled | ||
| if is_public and not video.data.get("shared_at"): | ||
| updates["shared_at"] = datetime.now(timezone.utc).isoformat() | ||
|
|
||
| # Update database | ||
| result = client.table("videos").update(updates).eq("id", video_id).execute() | ||
|
|
||
| return result.data[0] if result.data else {} | ||
|
|
||
|
|
||
| async def get_public_video(video_id: str) -> dict: | ||
| """Get public video details (no authentication required).""" | ||
| client = get_supabase() | ||
|
|
||
| # Only return if video is public | ||
| result = client.table("videos").select("*").eq("id", video_id).eq("is_public", True).single().execute() | ||
|
|
||
| if not result.data: | ||
| raise HTTPException(status_code=404, detail="Video not found or not public") | ||
|
|
||
| return result.data | ||
|
|
||
|
|
||
| async def increment_video_views(video_id: str) -> bool: | ||
| """Increment view count for a video.""" | ||
| client = get_supabase() | ||
|
|
||
| try: | ||
| # Get current view count | ||
| video = client.table("videos").select("view_count").eq("id", video_id).single().execute() | ||
| if not video.data: | ||
| return False | ||
|
|
||
| current_views = video.data.get("view_count", 0) | ||
|
|
||
| # Increment | ||
| client.table("videos").update({ | ||
| "view_count": current_views + 1, | ||
| "updated_at": datetime.now(timezone.utc).isoformat() | ||
| }).eq("id", video_id).execute() | ||
|
|
||
| return True | ||
| except Exception as e: | ||
| print(f"[Views] Failed to increment video views: {e}") | ||
| return False | ||
|
Comment on lines
+333
to
+354
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This function implements a 'read-then-write' pattern to increment the view count, which can lead to a race condition under concurrent requests. Two requests could read the same value, increment it, and write back, resulting in a lost view count. This should be an atomic operation. I recommend creating a PostgreSQL RPC function that performs an atomic update, like |
||
|
|
||
|
|
||
| async def toggle_chat_sharing(chat_id: str, user_id: str, is_public: bool) -> dict: | ||
| """Toggle chat sharing on/off. Returns updated chat data.""" | ||
| client = get_supabase() | ||
|
|
||
| # Verify ownership | ||
| chat = client.table("chats").select("*").eq("id", chat_id).eq("user_id", user_id).single().execute() | ||
| if not chat.data: | ||
| raise HTTPException(status_code=404, detail="Chat not found") | ||
|
|
||
| # Prepare update | ||
| updates = { | ||
| "is_public": is_public, | ||
| "updated_at": datetime.now(timezone.utc).isoformat() | ||
| } | ||
|
|
||
| # Set sharedAt timestamp when first enabled | ||
| if is_public and not chat.data.get("shared_at"): | ||
| updates["shared_at"] = datetime.now(timezone.utc).isoformat() | ||
|
|
||
| # Update database | ||
| result = client.table("chats").update(updates).eq("id", chat_id).execute() | ||
|
|
||
| return result.data[0] if result.data else {} | ||
|
|
||
|
|
||
| async def get_public_chat(chat_id: str) -> dict: | ||
| """Get public chat details (no authentication required).""" | ||
| client = get_supabase() | ||
|
|
||
| # Only return if chat is public | ||
| result = client.table("chats").select("*").eq("id", chat_id).eq("is_public", True).single().execute() | ||
|
|
||
| if not result.data: | ||
| raise HTTPException(status_code=404, detail="Chat not found or not public") | ||
|
|
||
| return result.data | ||
|
|
||
|
|
||
| async def get_public_chat_tasks(chat_id: str) -> list: | ||
| """Get all tasks for a public chat (no authentication required).""" | ||
| client = get_supabase() | ||
|
|
||
| # Verify chat is public first | ||
| chat = client.table("chats").select("is_public").eq("id", chat_id).single().execute() | ||
| if not chat.data or not chat.data.get("is_public"): | ||
| raise HTTPException(status_code=404, detail="Chat not found or not public") | ||
|
|
||
| # Fetch all tasks for this chat | ||
| result = client.table("tasks").select("*").eq("chat_id", chat_id).order("created_at", desc=False).execute() | ||
|
|
||
| return result.data or [] | ||
|
|
||
|
|
||
| async def increment_chat_views(chat_id: str) -> bool: | ||
| """Increment view count for a chat.""" | ||
| client = get_supabase() | ||
|
|
||
| try: | ||
| # Get current view count | ||
| chat = client.table("chats").select("view_count").eq("id", chat_id).single().execute() | ||
| if not chat.data: | ||
| return False | ||
|
|
||
| current_views = chat.data.get("view_count", 0) | ||
|
|
||
| # Increment | ||
| client.table("chats").update({ | ||
| "view_count": current_views + 1, | ||
| "updated_at": datetime.now(timezone.utc).isoformat() | ||
| }).eq("id", chat_id).execute() | ||
|
|
||
| return True | ||
| except Exception as e: | ||
| print(f"[Views] Failed to increment chat views: {e}") | ||
| return False | ||
|
Comment on lines
+410
to
+431
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This function has the same race condition issue as Please create a dedicated RPC function in PostgreSQL to handle the increment, for example: |
||
|
|
||
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.
For consistency with existing logging practices in this file and for better production monitoring, it's recommended to use the
loggingmodule instead ofprint()for error messages. This allows for configurable log levels, formatting, and output streams.