Skip to content

Conversation

@ucswift
Copy link
Member

@ucswift ucswift commented Jul 20, 2025

No description provided.

@ucswift
Copy link
Member Author

ucswift commented Jul 20, 2025

Task linked: CU-868cu9311 Migrate to React Native

@ucswift ucswift requested a review from Copilot July 20, 2025 01:25
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds audio stream player functionality and implements multi-select deletion for inbox messages. The implementation introduces comprehensive audio streaming capabilities using expo-av and enhances the notification inbox with bulk operations.

  • Audio stream player with real-time playback controls and status indicators
  • Multi-select functionality for notification inbox with bulk delete operations
  • Translation support for audio streams in English, Spanish, and Arabic

Reviewed Changes

Copilot reviewed 16 out of 17 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
src/stores/app/audio-stream-store.ts New Zustand store managing audio stream state and playback operations
src/components/audio-stream/audio-stream-bottom-sheet.tsx Bottom sheet UI component for audio stream selection and control
src/components/sidebar/unit-sidebar.tsx Adds audio stream button integration to the unit sidebar
src/components/notifications/NotificationInbox.tsx Implements multi-select mode with bulk delete functionality
src/api/voice/index.ts Disables caching for audio streams API endpoint
src/api/novu/inbox.ts Fixes parameter name in delete message API call
package.json Adds expo-av and expo-navigation-bar dependencies
src/translations/*.json Adds audio stream translations for 3 languages


// For live streams, try to reconnect
const { currentStream } = get();
if (currentStream && currentStream.Id === stream.Id) {
Copy link

Copilot AI Jul 20, 2025

Choose a reason for hiding this comment

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

There's a potential race condition where currentStream could be null between the check and accessing currentStream.Id. Consider using optional chaining: if (currentStream?.Id === stream.Id)

Suggested change
if (currentStream && currentStream.Id === stream.Id) {
if (currentStream?.Id === stream.Id) {

Copilot uses AI. Check for mistakes.
</HStack>
<HStack space="sm" className="items-center">
<Text className="flex-1 text-sm text-gray-500">{t('audio_streams.type')}:</Text>
<Text className="text-sm font-medium">{currentStream.Type || t('common.unknown')}</Text>
Copy link

Copilot AI Jul 20, 2025

Choose a reason for hiding this comment

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

The translation key 'common.unknown' is used but not defined in the provided translation files. This will likely result in a missing translation error.

Copilot uses AI. Check for mistakes.
) : null}

<Button onPress={() => setIsBottomSheetVisible(false)} variant="outline">
<ButtonText>{t('common.close')}</ButtonText>
Copy link

Copilot AI Jul 20, 2025

Choose a reason for hiding this comment

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

The translation key 'common.close' is used but not defined in the provided translation files. This should use 'audio_streams.close' instead which is properly defined.

Suggested change
<ButtonText>{t('common.close')}</ButtonText>
<ButtonText>{t('audio_streams.close')}</ButtonText>

Copilot uses AI. Check for mistakes.

const getDisplayText = () => {
if (isLoading) {
return t('audio_streams.loading_stream');
Copy link

Copilot AI Jul 20, 2025

Choose a reason for hiding this comment

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

The translation key 'audio_streams.loading_stream' is not defined in any of the translation files. This will cause a missing translation error.

Suggested change
return t('audio_streams.loading_stream');
return t('audio_streams.loading_stream'); // Ensure this key is defined in the translation files

Copilot uses AI. Check for mistakes.
return t('audio_streams.loading_stream');
}
if (isBuffering) {
return t('audio_streams.buffering_stream');
Copy link

Copilot AI Jul 20, 2025

Choose a reason for hiding this comment

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

The translation key 'audio_streams.buffering_stream' is not defined in any of the translation files. This will cause a missing translation error.

Copilot uses AI. Check for mistakes.
{isLoading || isBuffering ? (
<>
<Loader size={14} color="#3b82f6" />
<Text className="text-sm font-medium text-blue-600">{isLoading ? t('audio_streams.loading') : t('audio_streams.buffering')}</Text>
Copy link

Copilot AI Jul 20, 2025

Choose a reason for hiding this comment

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

The translation key 'audio_streams.loading' and 'audio_streams.buffering' are not defined in the translation files. These will cause missing translation errors.

Copilot uses AI. Check for mistakes.
export const deleteMessage = async (messageId: string) => {
const response = await getGroupsApi.delete({
groupId: encodeURIComponent(messageId),
messageId: encodeURIComponent(messageId),
Copy link

Copilot AI Jul 20, 2025

Choose a reason for hiding this comment

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

The parameter name was changed from 'groupId' to 'messageId', but the endpoint path still references '/Inbox/DeleteMessage'. Verify that this parameter name matches the expected API contract for the delete message endpoint.

Copilot uses AI. Check for mistakes.
@ucswift
Copy link
Member Author

ucswift commented Jul 20, 2025

Approve

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This PR is approved.

@ucswift ucswift merged commit 8dc3263 into master Jul 20, 2025
5 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants