Skip to content

feat: Add Listen Together synchronized podcast feature#2039

Open
vivek0028 wants to merge 3 commits into
SB2318:mainfrom
vivek0028:feature/listen-together
Open

feat: Add Listen Together synchronized podcast feature#2039
vivek0028 wants to merge 3 commits into
SB2318:mainfrom
vivek0028:feature/listen-together

Conversation

@vivek0028

Copy link
Copy Markdown

📌 Description

Overview

This pull request introduces the "Listen Together" feature, enabling users to enjoy health podcasts in real-time synchronization with friends and family while chatting live during the session.

The implementation provides the frontend architecture for collaborative podcast listening using Socket.IO, including room creation, synchronized playback, participant management, and a real-time chat interface.


✨ Features Implemented

🎧 Core Sync Engine

  • Added a new useListenTogether React hook.

  • Manages Socket.IO connections and room lifecycle.

  • Synchronizes playback events:

    • Play
    • Pause
    • Resume
    • Seek
    • Playback progress
  • Handles participant join/leave events.

  • Includes latency compensation for improved synchronization.

🎵 ListenTogetherScreen

  • New premium dark-themed listening screen.
  • Shared podcast player interface.
  • Host-controlled playback actions.
  • Real-time participant synchronization.
  • Integrated live chat overlay.

🔑 JoinListenTogetherScreen

  • New screen for joining active listening sessions.
  • Enter a 6-character room code.
  • Handles validation and connection flow.

💬 ListenTogetherChat

  • Glassmorphic live chat component.
  • Real-time messaging.
  • Auto-scrolling conversation.
  • Optimized for synchronized listening sessions.

👥 ListenTogetherRoomBar

Displays:

  • Room code
  • Participant avatars
  • Live connection/sync status
  • Share room action
  • End session control (Host only)

🎙 Podcast Detail Integration

  • Added "Listen Together" action button on the Podcast Detail screen.
  • Unauthenticated users are redirected to GuestPlaceholderScreen before joining a session.

🛠 Technical Details

Frontend

  • Integrated socket.io-client using the existing SocketContext.

  • Added strongly typed socket events.

  • Introduced ListenTogetherTypes.ts for complete type safety.

  • Registered:

    • ListenTogetherScreen
    • JoinListenTogetherScreen
  • Updated:

    • StackNavigation.tsx
    • type.ts

Socket Events

Implemented support for frontend events including:

  • Room creation
  • Room join/leave
  • Playback synchronization
  • Chat messaging
  • Participant updates
  • Session termination

✅ Benefits

  • Real-time collaborative podcast listening.
  • Improved user engagement.
  • Shared learning experience for health content.
  • Scalable Socket.IO-based architecture.
  • Clean, modular, and reusable frontend components.

🧪 Testing

  • Room creation flow
  • Join room flow
  • Playback synchronization
  • Participant synchronization
  • Chat UI rendering
  • Navigation flow
  • Guest user redirection
  • End-to-end backend socket testing (requires backend implementation)

📝 Note for Backend Reviewers

To fully test this feature, the backend should implement the listen-together:* Socket.IO namespace and corresponding room management APIs as defined in the frontend implementation.


🔗 Related Issue

Closes #1829


🏷 Labels

enhancement · feature · frontend · real-time · socket.io · webrtc · react-native · GSSoC'26

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Thank you @, for creating the PR and contributing to our UltimateHealth project 💗.
Our team will review the PR and will reach out to you soon! 😇
Make sure that you have marked all the tasks that you are done with ✅.
Thank you for your patience! 😀

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Automated Review Feedback

Provide actionable comments grouped by severity:

Important

  • Missing Unit/Integration Tests for Core Logic: The useListenTogether hook and ListenTogetherScreen are central to this feature's functionality and complexity. Lack of unit tests for the hook's state management, socket event emissions/responses, and integration tests for the screen's interactions (e.g., host controls, listener sync, navigation flows) increases the risk of regressions and makes future maintenance harder. Given the real-time nature and multiple states, robust testing is crucial.

Suggestions

  • Clipboard Integration in ListenTogetherRoomBar.tsx: The current handleCopyCode uses Snackbar.show as a workaround. Consider integrating expo-clipboard (if not already present in the project) to provide a native clipboard copy functionality for a better user experience.
  • Error Handling for player.replace: In ListenTogetherScreen.tsx, the player.replace(secureSource) call is wrapped in a try-catch block, but the error is only logged to console.warn. Consider displaying a user-friendly message (e.g., via Snackbar) if the audio source fails to load or replace, especially if it's a critical part of the user experience.
  • Accessibility Labels for Playback Controls: While accessibilityLabel is present for play/pause/skip buttons, consider adding accessibilityHint for more detailed instructions, especially for the slider (onSlidingComplete).
  • SHARE_BASE_URL Source: The SHARE_BASE_URL is imported from ../helper/APIUtils. Ensure this URL is correctly configured for deep linking to the "Listen Together" feature on both development and production environments.

Maintainer Note:

Once the initial automated feedback has been addressed, maintainer @SB2318 will review the pull request for final evaluation.

@SB2318

SB2318 commented Jul 5, 2026

Copy link
Copy Markdown
Owner

/review

@github-actions

github-actions Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

🤖 Gemini AI Code Review

This Pull Request introduces the "Listen Together" feature, a significant enhancement enabling real-time synchronized podcast listening and live chat. The implementation spans several new components and a core React hook (useListenTogether) to manage Socket.IO communication, playback synchronization, and participant state.

Overall, the PR demonstrates a solid understanding of React Native component architecture, state management, and real-time communication patterns. The use of Tamagui for styling, expo-audio (or a similar audio player hook), and Redux for global state is consistent with modern React Native practices. The feature is well-structured with clear separation of concerns between the UI components and the useListenTogether hook.

However, a thorough review reveals several areas for improvement, particularly concerning security, edge case handling, and minor UI/UX refinements.

🔴 High Severity

  • Issue: Potential SSRF/Arbitrary File Access via GET_IMAGE and isAllowedUrl in ListenTogetherScreen.tsx
    The isAllowedUrl helper function attempts to whitelist hostnames for audio and cover image URLs. However, it explicitly allows urlStr that do not start with http:// or https:// to pass through if they don't contain ://. These URLs are then prefixed with GET_IMAGE/. If GET_IMAGE is a backend endpoint that fetches files based on the provided path, an attacker could potentially craft a urlStr like ../../../../etc/passwd or file:///path/to/sensitive/file (if GET_IMAGE supports local file paths or arbitrary protocols) which would then be requested by the backend. This could lead to Server-Side Request Forgery (SSRF) or arbitrary file disclosure from the server hosting GET_IMAGE.

    • Impact: Critical security vulnerability allowing attackers to read arbitrary files on the server or make requests to internal network resources.
    • Fix:
      1. Strictly enforce http(s) protocol for GET_IMAGE paths: The GET_IMAGE endpoint should only accept relative paths or specific identifiers, and never arbitrary URLs or paths that could escape its intended directory.
      2. Refine isAllowedUrl: The isAllowedUrl function should be more restrictive. If GET_IMAGE is meant for relative paths, then isAllowedUrl should ensure the input is a valid relative path (e.g., no .., no ://). If GET_IMAGE is a proxy for external URLs, then isAllowedUrl should only allow http(s) and perform strict hostname validation.
      3. Example Fix (assuming GET_IMAGE expects relative paths/identifiers):
        // In ListenTogetherScreen.tsx
        const isAllowedUrl = (urlStr?: string | null): boolean => {
          if (!urlStr) return false;
          // If it starts with http/https, it must be whitelisted explicitly.
          // Otherwise, assume it's a relative path for GET_IMAGE and ensure it's safe.
          if (urlStr.startsWith('http://') || urlStr.startsWith('https://')) {
            try {
              const match = urlStr.match(/^https?:\/\/([^/?#:]+)/i);
              if (!match) return false;
              const hostname = match[1].toLowerCase();
              return hostname === 'uhsocial.in' || hostname === 'localhost' || hostname === '10.0.2.2';
            } catch {
              return false;
            }
          }
          // For non-http/https, assume it's a path for GET_IMAGE.
          // Ensure it doesn't contain protocol separators or directory traversal attempts.
          return !urlStr.includes('://') && !urlStr.includes('..') && !urlStr.startsWith('/');
        };
        
        const getFormattedSource = (url?: string | null): string | null => {
          if (!url) return null;
          if (!isAllowedUrl(url)) {
            console.warn('Attempted to load disallowed URL:', url);
            return null; // Block disallowed URLs
          }
          return url.startsWith('http') ? url : `${GET_IMAGE}/${url}`;
        };
        Crucially, the backend GET_IMAGE endpoint must also be hardened against these attacks.
  • Issue: Inconsistent and Outdated Clipboard API Usage in ListenTogetherRoomBar.tsx
    The handleCopyCode function uses expo-linking for clipboard functionality, which is incorrect. expo-linking is for opening URLs, not for copying text to the clipboard. The comment // React Native doesn't have a direct Clipboard in Expo without the module, is also outdated; expo-clipboard is the standard and correct module for this. As a result, the room code is not actually copied to the clipboard, only displayed in a Snackbar.

    • Impact: Core functionality (copying room code) is broken, leading to a poor user experience and potential frustration.
    • Fix:
      1. Install expo-clipboard: npx expo install expo-clipboard
      2. Update the import and usage:
        // In ListenTogetherRoomBar.tsx
        import * as Clipboard from 'expo-clipboard'; // Correct module
        
        // ...
        
        const handleCopyCode = useCallback(() => {
          Clipboard.setStringAsync(roomCode); // Use the correct API
          Snackbar.show({
            text: `Room code copied: ${roomCode}`,
            duration: Snackbar.LENGTH_SHORT,
          });
        }, [roomCode]);

🟡 Medium Severity

  • Issue: Race Condition and Incomplete State Reset on useListenTogether Cleanup
    The useEffect cleanup function in useListenTogether emits a LEAVE_ROOM event on unmount. If the component unmounts very quickly after a createRoom or joinRoom call, but before the ROOM_CREATED or ROOM_JOINED event is received (meaning roomCodeRef.current might still be null or outdated), the LEAVE_ROOM event might not be sent or might be sent with incorrect data. Additionally, the optimistic cleanup in leaveRoom and endRoom might not fully align with the server's state if the server rejects the leave/end request or if the socket connection is lost.

    • Impact: Users might not properly leave rooms, leading to ghost participants on the server or unexpected state issues if they try to rejoin.
    • Fix:
      1. Ensure roomCodeRef.current is valid before emitting LEAVE_ROOM in cleanup:
        // In useListenTogether.ts
        useEffect(() => {
          return () => {
            // Only attempt to leave if we actually believe we are in a room
            if (roomCodeRef.current && socket && isInRoom) { // Add isInRoom check
              socket.emit(LISTEN_TOGETHER_EVENTS.LEAVE_ROOM, {
                roomCode: roomCodeRef.current,
                userId: user_id,
              });
              // Consider adding a server-side confirmation for leave/end,
              // or a more robust client-side state machine for room lifecycle.
            }
          };
        }, [socket, user_id, isInRoom]); // Add isInRoom to dependencies
      2. Implement server-side confirmation for LEAVE_ROOM and END_ROOM: The server should acknowledge these events, and the client should only clear its state upon receiving that acknowledgment, or a ROOM_ENDED event. For now, the optimistic update is acceptable, but for production, a more robust approach is recommended.
  • Issue: Incomplete Error Handling and User Feedback for Socket Disconnections
    The useListenTogether hook handles LISTEN_TOGETHER_EVENTS.ERROR but doesn't explicitly handle general socket disconnection events (e.g., disconnect, reconnect). If the socket disconnects while in a room, the user might remain in a stale state without clear indication or automatic re-connection/re-joining attempts.

    • Impact: Users might experience a broken real-time experience without understanding why, leading to frustration.
    • Fix:
      1. Add listeners for socket disconnect and reconnect events:
        // In useListenTogether.ts, inside useEffect for socket listeners
        const onSocketDisconnect = () => {
          setError('Disconnected from the server. Attempting to reconnect...');
          // Potentially reset some room state or mark as disconnected
        };
        
        const onSocketReconnect = () => {
          setError(null); // Clear error on successful reconnect
          // If in a room, attempt to rejoin or re-sync state with the server
          if (roomCodeRef.current && user_id) {
            // This would require a 'rejoin' or 'resync' event on the backend
            // or a full re-join flow if the server doesn't maintain state.
            // For now, just clear error.
          }
        };
        
        socket.on('disconnect', onSocketDisconnect);
        socket.on('reconnect', onSocketReconnect);
        
        return () => {
          // ... existing socket.off calls
          socket.off('disconnect', onSocketDisconnect);
          socket.off('reconnect', onSocketReconnect);
        };
      2. Consider a more sophisticated reconnection strategy: For a production-grade feature, automatic rejoining of a room after a brief disconnection (if the server supports it) would greatly improve UX.
  • Issue: JoinListenTogetherScreen Navigates with Empty trackId and audioUrl
    When a listener joins a room via JoinListenTogetherScreen, it navigates to ListenTogetherScreen with trackId: '' and audioUrl: null. The ListenTogetherScreen then relies on the useListenTogether hook to populate the room object, which contains the actual podcastId and audioUrl. This creates a temporary inconsistent state and relies on the useListenTogether hook to quickly fetch the correct podcast details.

    • Impact: Minor, but could lead to brief UI glitches or incorrect data being displayed if the socket connection is slow or fails before room data is received. It also makes the ListenTogetherScreen's initial state handling more complex.
    • Fix:
      The JoinListenTogetherScreen should ideally wait for the joinRoom socket event to succeed and receive the RoomJoinedPayload before navigating. This would allow it to pass the correct trackId and audioUrl directly to ListenTogetherScreen. However, this would require refactoring the joinRoom logic to return a promise or use a callback, and potentially introduce a loading state in JoinListenTogetherScreen.
      Alternative (less ideal but simpler for now): Ensure ListenTogetherScreen handles trackId and audioUrl being null/empty gracefully until room data is available. The current implementation does this reasonably well with the isPodcastLoading and !isInRoom checks, but the trackId: '' is still a bit of a hack.
  • Issue: useAudioPlayer is not a standard Expo module.
    The PR uses useAudioPlayer from expo-audio. This is not a standard Expo package. While it might be a custom hook or a wrapper around expo-av or react-native-track-player, it's important to clarify its origin and ensure it's a robust, well-maintained solution for production.

    • Impact: If this is a custom or unmaintained hook, it could introduce stability issues, performance problems, or be difficult to debug/update in the future.
    • Fix:
      1. Clarify useAudioPlayer origin: Add a comment or documentation explaining where expo-audio comes from and what it wraps.
      2. Consider using a standard library: If expo-audio is custom, evaluate replacing it with a widely adopted and well-supported library like expo-av or react-native-track-player for better long-term maintainability and community support.
  • Issue: KeyboardAvoidingView keyboardVerticalOffset in ListenTogetherChat.tsx
    The keyboardVerticalOffset for KeyboardAvoidingView is hardcoded to 90 for iOS. This value is highly dependent on the specific header/navigation bar height and might not be correct across different devices or future UI changes.

    • Impact: The chat input might be partially obscured by the keyboard or have excessive padding, leading to a suboptimal user experience.
    • Fix:
      1. Use useHeaderHeight from @react-navigation/elements: This hook provides the actual height of the navigation header, allowing for a more dynamic and accurate offset.
        // In ListenTogetherChat.tsx
        import { useHeaderHeight } from '@react-navigation/elements';
        
        // ... inside component
        const headerHeight = useHeaderHeight();
        // ...
        <KeyboardAvoidingView
          style={styles.container}
          behavior={Platform.OS === 'ios' ? 'padding' : undefined}
          keyboardVerticalOffset={Platform.OS === 'ios' ? headerHeight : 0}>
        (Note: This might require passing headerHeight down if ListenTogetherChat is not directly under a navigator, or making ListenTogetherChat a screen itself.)
      2. Alternatively, use insets.bottom from react-native-safe-area-context: This can help adjust for the safe area, though keyboardVerticalOffset is specifically for the keyboard.

🟢 Low Severity / Nits

  • ListenTogetherChat.tsx - formatTime Error Handling: The formatTime function uses a generic try...catch block that returns an empty string on any error. While functional, it might be more informative to log the error or return a placeholder like --:-- to indicate an invalid timestamp.
    // In ListenTogetherChat.tsx
    const formatTime = (isoString: string) => {
      try {
        const date = new Date(isoString);
        // Check if date is valid before formatting
        if (isNaN(date.getTime())) {
          console.warn('Invalid date string for chat message:', isoString);
          return '--:--';
        }
        const hours = date.getHours().toString().padStart(2, '0');
        const minutes = date.getMinutes().toString().padStart(2, '0');
        return `${hours}:${minutes}`;
      } catch (e) {
        console.error('Error formatting chat message time:', e, isoString);
        return '--:--';
      }
    };
  • ListenTogetherChat.tsx - FlatList ListEmptyComponent Styling: The ListEmptyComponent uses flex: 1 which might not always correctly center its content if the FlatList itself doesn't have a defined height or flexGrow. It's generally safer to ensure the FlatList or its container has flex: 1 and then the ListEmptyComponent will naturally fill it. The current setup seems to work due to flexGrow: 1 on messagesList.
  • ListenTogetherRoomBar.tsx - SHARE_BASE_URL Import: SHARE_BASE_URL is imported from ../helper/APIUtils. It's good practice to ensure this constant is correctly configured for production and development environments.
  • useListenTogether.ts - setTimeout for setIsSyncing(false): The setTimeout(() => setIsSyncing(false), 500) in onSyncUpdate is a simple way to show a brief "Syncing" indicator. While acceptable for a first iteration, a more robust solution might involve tracking the actual time it takes for the player to seek/play and then marking as "In Sync" once the player reports the correct state.
  • useListenTogether.ts - Redux Selector Destructuring:
    const {user_id, user_handle} = useSelector((state: any) => state.user);
    const profileImage = useSelector((state: any) => state.user.profile_image ?? null);
    This can be combined into a single useSelector call for slight optimization and cleaner code, especially since state.user is accessed multiple times.
    const {user_id, user_handle, profile_image} = useSelector((state: any) => state.user);
    const profileImage = profile_image ?? null;
  • ListenTogetherScreen.tsx - defaultFallback Audio: The defaultFallback audio file funny-cartoon-sound-397415.mp3 is used if audioUrl is null or invalid. While functional, a more neutral or silent fallback audio might be less jarring for users.
  • ListenTogetherScreen.tsx - player.currentStatus Null Check: In the position/duration polling useEffect, player.currentStatus is accessed directly. It's safer to check if player.currentStatus exists before accessing its properties, as it might be null during initialization or if the player encounters an error.
    // In ListenTogetherScreen.tsx
    useEffect(() => {
      if (!player) return;
      const interval = setInterval(() => {
        const status = player.currentStatus;
        if (status) { // Add this check
          setPosition(status.currentTime || 0);
          setDuration(player.duration || status.duration || 0);
          setIsPlaying(status.playing || false);
        } else {
          // Handle case where status is null, e.g., player not ready or error
          // console.warn('Audio player status is null');
        }
      }, 500);
      return () => clearInterval(interval);
    }, [player]);
  • ListenTogetherScreen.tsx - Alert for Leaving/Ending: The Alert message for leaving/ending a session is good. However, the onPress handler for the "Leave" button in the Alert directly calls isHost ? handleEnd : handleLeave. This is correct, but for clarity, it could be slightly refactored to explicitly show the host/listener action.
  • ListenTogetherScreen.tsx - ScrollView vs. FlatList for Main Content: The main screen uses a ScrollView. While fine for a fixed number of elements, if the content (e.g., podcast description, chat history) could become very long, a FlatList or SectionList might offer better performance due to virtualization. For this screen, ScrollView is likely acceptable.
  • ListenTogetherScreen.tsx - fontFamily: 'monospace': Using fontFamily: 'monospace' for time display is a nice touch for readability. Ensure this font is available and consistent across platforms.
  • ListenTogetherTypes.ts - profileImage Type: profileImage is typed as string | null. If profileImage is always a URL or a path to GET_IMAGE, it's consistent. If it could be a base64 string, that should be noted or handled.

What's Good ✅

  1. Clear Separation of Concerns with useListenTogether Hook: The core logic for Socket.IO communication, room management, and state synchronization is encapsulated within the useListenTogether hook. This makes the ListenTogetherScreen component much cleaner, focusing solely on UI rendering and user interaction, leading to highly maintainable and testable code.
  2. Robust UI/UX for Room Management and Playback: The UI components (ListenTogetherChat, ListenTogetherRoomBar, JoinListenTogetherScreen, ListenTogetherScreen) are well-designed with a consistent dark theme, clear visual indicators (e.g., sync status, live chat dot), and good accessibility labels. The host-controlled playback and clear messaging for listeners ("Host is controlling playback") provide a good user experience.
  3. Thoughtful Latency Compensation: The implementation of latency compensation in useListenTogether (adjustedPos = action === 'play' ? syncPos + latencyMs / 1000 : syncPos;) is a great detail that shows attention to the challenges of real-time synchronization, aiming for a smoother experience for listeners.

Verdict

Request Changes: The high-severity security vulnerability related to isAllowedUrl and the broken clipboard functionality are critical issues that need to be addressed before this PR can be merged. The medium-severity issues, particularly around socket disconnection handling and the useAudioPlayer origin, also warrant attention for a production-grade feature.

@SB2318 SB2318 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vivek0369 Thanks for your contribution! But please check the issue first where I have shared some implementation plan.

Please let me know your progress accordingly.

I will be happy to merge then. 😇

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature : Sync & Share – "Listen Together" for Health Podcasts

2 participants