RU-T39 PR#176 fixes

ucswift · ucswift · commit d909c4c8890e · 2025-11-01T15:31:45.000-07:00
diff --git a/.github/workflows/react-native-cicd.yml b/.github/workflows/react-native-cicd.yml
@@ -320,34 +320,36 @@ jobs:
           if [ "${{ github.event_name }}" = "push" ]; then
             echo "Fetching PR body for merged commit..."
             
-            # Get the PR number associated with this merge commit
-            PR_NUMBER=$(gh pr list \
-              --state merged \
-              --search "${{ github.sha }}" \
-              --json number \
-              --jq '.[0].number' 2>/dev/null || echo "")
+            # First, try to find PR number from commit message (most reliable)
+            PR_FROM_COMMIT=$(git log -1 --pretty=%B | grep -oE '#[0-9]+' | head -1 | tr -d '#' || echo "")
             
-            if [ -n "$PR_NUMBER" ]; then
-              echo "Found merged PR #$PR_NUMBER"
-              
-              # Fetch the PR body
-              PR_BODY=$(gh pr view "$PR_NUMBER" --json body --jq '.body' 2>/dev/null || echo "")
+            if [ -n "$PR_FROM_COMMIT" ]; then
+              echo "Found PR #$PR_FROM_COMMIT from commit message"
+              PR_BODY=$(gh pr view "$PR_FROM_COMMIT" --json body --jq '.body' 2>/dev/null || echo "")
               
               if [ -n "$PR_BODY" ]; then
                 NOTES="$(extract_release_notes "$PR_BODY")"
               fi
             else
-              echo "No associated PR found, checking commit message..."
-              # Try to find PR number from commit message
-              PR_FROM_COMMIT=$(git log -1 --pretty=%B | grep -oE '#[0-9]+' | head -1 | tr -d '#' || echo "")
+              echo "No PR reference in commit message, searching by commit SHA..."
+              # Get PRs that contain this commit (using GitHub API to search by commit)
+              PR_NUMBERS=$(gh api \
+                "repos/${{ github.repository }}/commits/${{ github.sha }}/pulls" \
+                --jq '.[].number' 2>/dev/null || echo "")
               
-              if [ -n "$PR_FROM_COMMIT" ]; then
-                echo "Found PR #$PR_FROM_COMMIT from commit message"
-                PR_BODY=$(gh pr view "$PR_FROM_COMMIT" --json body --jq '.body' 2>/dev/null || echo "")
+              if [ -n "$PR_NUMBERS" ]; then
+                # Take the first PR found (most recently merged)
+                PR_NUMBER=$(echo "$PR_NUMBERS" | head -n 1)
+                echo "Found PR #$PR_NUMBER associated with commit"
+                
+                # Fetch the PR body
+                PR_BODY=$(gh pr view "$PR_NUMBER" --json body --jq '.body' 2>/dev/null || echo "")
                 
                 if [ -n "$PR_BODY" ]; then
                   NOTES="$(extract_release_notes "$PR_BODY")"
                 fi
+              else
+                echo "No associated PR found for this commit"
               fi
             fi
           fi
@@ -376,11 +378,17 @@ jobs:
         env:
           GH_TOKEN: ${{ github.token }}
 
-      - name: � Send Release Notes to Changerawr
-        if: ${{ matrix.platform == 'android' && env.CHANGERAWR_API_URL != '' && env.CHANGERAWR_API_KEY != '' }}
+      - name: 📝 Send Release Notes to Changerawr
+        if: ${{ matrix.platform == 'android' }}
         run: |
           set -eo pipefail
           
+          # Check if required secrets are set
+          if [ -z "$CHANGERAWR_API_URL" ] || [ -z "$CHANGERAWR_API_KEY" ]; then
+            echo "⚠️  Changerawr API credentials not configured, skipping release notes submission"
+            exit 0
+          fi
+          
           # Read release notes
           RELEASE_NOTES=$(cat RELEASE_NOTES.md)
           VERSION="7.${{ github.run_number }}"
diff --git a/src/components/calls/dispatch-selection-modal.tsx b/src/components/calls/dispatch-selection-modal.tsx
@@ -1,6 +1,6 @@
 import { CheckIcon, SearchIcon, UsersIcon, X } from 'lucide-react-native';
 import { useColorScheme } from 'nativewind';
-import React, { useEffect, useMemo } from 'react';
+import React, { useEffect } from 'react';
 import { useTranslation } from 'react-i18next';
 import { ScrollView, TouchableOpacity, View } from 'react-native';
 
@@ -27,7 +27,7 @@ export const DispatchSelectionModal: React.FC<DispatchSelectionModalProps> = ({
   const { data, selection, isLoading, error, searchQuery, fetchDispatchData, setSelection, toggleEveryone, toggleUser, toggleGroup, toggleRole, toggleUnit, setSearchQuery, clearSelection, getFilteredData } =
     useDispatchStore();
 
-  const filteredData = useMemo(() => getFilteredData(), [getFilteredData]);
+  const filteredData = getFilteredData();
 
   useEffect(() => {
     if (isVisible) {
diff --git a/src/services/__tests__/notification-sound.service.test.ts b/src/services/__tests__/notification-sound.service.test.ts
@@ -46,13 +46,21 @@ jest.mock('@/lib/logging', () => ({
 }));
 
 describe('NotificationSoundService', () => {
-  beforeEach(() => {
+  beforeEach(async () => {
     jest.clearAllMocks();
+    // Clean up any previous initialization state
+    await notificationSoundService.cleanup();
+  });
+
+  afterEach(async () => {
+    // Clean up after each test
+    await notificationSoundService.cleanup();
   });
 
   describe('initialization', () => {
-    it('should initialize audio mode', async () => {
-      // Mock is called in constructor, so need to check mocks
+    it('should initialize audio mode when initialize() is called', async () => {
+      // Initialization is lazy, so Audio.setAudioModeAsync should not be called until initialize() is called
+      await notificationSoundService.initialize();
       expect(Audio.setAudioModeAsync).toHaveBeenCalled();
     });
 
@@ -62,6 +70,18 @@ describe('NotificationSoundService', () => {
       // Verify sounds are created (called during initialization)
       expect(Audio.Sound.createAsync).toHaveBeenCalled();
     });
+
+    it('should not initialize multiple times concurrently', async () => {
+      // Call initialize multiple times concurrently
+      await Promise.all([
+        notificationSoundService.initialize(),
+        notificationSoundService.initialize(),
+        notificationSoundService.initialize(),
+      ]);
+
+      // Should only initialize once
+      expect(Audio.setAudioModeAsync).toHaveBeenCalledTimes(1);
+    });
   });
 
   describe('playNotificationSound', () => {
diff --git a/src/services/__tests__/push-notification.test.ts b/src/services/__tests__/push-notification.test.ts
@@ -26,9 +26,13 @@ jest.mock('react-native', () => ({
 }));
 
 // Mock expo-notifications
+const mockRemove = jest.fn();
+const mockAddNotificationReceivedListener = jest.fn().mockReturnValue({ remove: mockRemove });
+const mockAddNotificationResponseReceivedListener = jest.fn().mockReturnValue({ remove: mockRemove });
+
 jest.mock('expo-notifications', () => ({
-  addNotificationReceivedListener: jest.fn(),
-  addNotificationResponseReceivedListener: jest.fn(),
+  addNotificationReceivedListener: mockAddNotificationReceivedListener,
+  addNotificationResponseReceivedListener: mockAddNotificationResponseReceivedListener,
   removeNotificationSubscription: jest.fn(),
   setNotificationHandler: jest.fn(),
   setNotificationChannelAsync: jest.fn(),
@@ -80,16 +84,15 @@ jest.mock('@/stores/security/store', () => ({
   },
 }));
 
-// Mock the push notification service
-const mockNotificationHandler = jest.fn();
+// Mock Firebase messaging
+const mockFcmUnsubscribe = jest.fn();
+const mockOnMessage = jest.fn().mockReturnValue(mockFcmUnsubscribe);
 
-jest.mock('../push-notification', () => ({
-  pushNotificationService: {
-    getInstance: jest.fn(() => ({
-      handleNotificationReceived: mockNotificationHandler,
-    })),
-  },
-}));
+jest.mock('@react-native-firebase/messaging', () => {
+  return jest.fn(() => ({
+    onMessage: mockOnMessage,
+  }));
+});
 
 describe('Push Notification Service Integration', () => {
   const mockShowNotificationModal = jest.fn();
@@ -369,4 +372,65 @@ describe('Push Notification Service Integration', () => {
       expect(mockShowNotificationModal).not.toHaveBeenCalled();
     });
   });
+
+  describe('listener cleanup', () => {
+    // Dynamically require the service to avoid mocking issues
+    let PushNotificationService: any;
+    let service: any;
+
+    beforeEach(() => {
+      jest.clearAllMocks();
+      jest.resetModules();
+      
+      // Re-require the module to get fresh instance
+      jest.unmock('../push-notification');
+      const module = require('../push-notification');
+      service = module.pushNotificationService;
+    });
+
+    it('should store listener handles on initialization', async () => {
+      // Initialize should add listeners
+      await service.initialize();
+
+      // Verify listeners were registered
+      expect(mockAddNotificationReceivedListener).toHaveBeenCalled();
+      expect(mockAddNotificationResponseReceivedListener).toHaveBeenCalled();
+      expect(mockOnMessage).toHaveBeenCalled();
+    });
+
+    it('should properly cleanup all listeners', async () => {
+      // Initialize first to set up listeners
+      await service.initialize();
+
+      // Clear previous calls
+      mockRemove.mockClear();
+      mockFcmUnsubscribe.mockClear();
+
+      // Call cleanup
+      service.cleanup();
+
+      // Verify all listeners were removed
+      // Expo listeners should have .remove() called twice (once for each listener)
+      expect(mockRemove).toHaveBeenCalledTimes(2);
+
+      // FCM unsubscribe should be called once
+      expect(mockFcmUnsubscribe).toHaveBeenCalledTimes(1);
+    });
+
+    it('should not error when cleanup is called without initialization', () => {
+      // Should not throw when cleanup is called without initializing
+      expect(() => service.cleanup()).not.toThrow();
+    });
+
+    it('should not error when cleanup is called multiple times', async () => {
+      await service.initialize();
+
+      // Should not throw when cleanup is called multiple times
+      expect(() => {
+        service.cleanup();
+        service.cleanup();
+        service.cleanup();
+      }).not.toThrow();
+    });
+  });
 });
diff --git a/src/services/notification-sound.service.ts b/src/services/notification-sound.service.ts
@@ -12,9 +12,10 @@ class NotificationSoundService {
   private groupChatSound: Audio.Sound | null = null;
   private defaultSound: Audio.Sound | null = null;
   private isInitialized = false;
+  private initializationPromise: Promise<void> | null = null;
 
   private constructor() {
-    this.initializeAudio();
+    // Initialization will happen lazily on first use
   }
 
   static getInstance(): NotificationSoundService {
@@ -29,10 +30,22 @@ class NotificationSoundService {
   }
 
   private async initializeAudio(): Promise<void> {
+    // If already initialized, return immediately
     if (this.isInitialized) {
       return;
     }
 
+    // If initialization is in progress, wait for it to complete
+    if (this.initializationPromise) {
+      return this.initializationPromise;
+    }
+
+    // Start initialization and store the promise
+    this.initializationPromise = this.performInitialization();
+    await this.initializationPromise;
+  }
+
+  private async performInitialization(): Promise<void> {
     try {
       // Configure audio mode
       await Audio.setAudioModeAsync({
@@ -60,6 +73,8 @@ class NotificationSoundService {
         message: 'Failed to initialize notification sound service',
         context: { error },
       });
+      // Reset initialization promise on failure so it can be retried
+      this.initializationPromise = null;
     }
   }
 
@@ -83,40 +98,46 @@ class NotificationSoundService {
     }
   }
 
-  private async loadAudioFiles(): Promise<void> {
+  /**
+   * Helper method to load a sound asset from a require path
+   * @param assetRequire - The required asset module
+   * @param soundName - Name of the sound for logging purposes
+   * @returns The created Audio.Sound or null on failure
+   */
+  private async loadSound(assetRequire: number, soundName: string): Promise<Audio.Sound | null> {
     try {
-      // Load call sound
-      const callSoundAsset = Asset.fromModule(require('@assets/audio/newcall.wav'));
-      await callSoundAsset.downloadAsync();
+      const asset = Asset.fromModule(assetRequire);
+      await asset.downloadAsync();
 
-      const { sound: callSound } = await Audio.Sound.createAsync({ uri: callSoundAsset.localUri || callSoundAsset.uri } as AVPlaybackSource, {
+      const { sound } = await Audio.Sound.createAsync({ uri: asset.localUri || asset.uri } as AVPlaybackSource, {
         shouldPlay: false,
         isLooping: false,
         volume: 1.0,
       });
-      this.callSound = callSound;
-
-      // Load message sound
-      const messageSoundAsset = Asset.fromModule(require('@assets/audio/newmessage.wav'));
-      await messageSoundAsset.downloadAsync();
 
-      const { sound: messageSound } = await Audio.Sound.createAsync({ uri: messageSoundAsset.localUri || messageSoundAsset.uri } as AVPlaybackSource, {
-        shouldPlay: false,
-        isLooping: false,
-        volume: 1.0,
+      return sound;
+    } catch (error) {
+      logger.error({
+        message: `Failed to load sound: ${soundName}`,
+        context: { error },
       });
-      this.messageSound = messageSound;
+      return null;
+    }
+  }
+
+  private async loadAudioFiles(): Promise<void> {
+    try {
+      // Load call sound
+      this.callSound = await this.loadSound(require('@assets/audio/newcall.wav'), 'call');
+
+      // Load message sound
+      this.messageSound = await this.loadSound(require('@assets/audio/newmessage.wav'), 'message');
 
       // Load chat sound
       const chatSoundAsset = Asset.fromModule(require('@assets/audio/newchat.wav'));
       await chatSoundAsset.downloadAsync();
 
-      const { sound: chatSound } = await Audio.Sound.createAsync({ uri: chatSoundAsset.localUri || chatSoundAsset.uri } as AVPlaybackSource, {
-        shouldPlay: false,
-        isLooping: false,
-        volume: 1.0,
-      });
-      this.chatSound = chatSound;
+      this.chatSound = await this.loadSound(require('@assets/audio/newchat.wav'), 'chat');
 
       // Group chat uses the same sound as regular chat
       const { sound: groupChatSound } = await Audio.Sound.createAsync({ uri: chatSoundAsset.localUri || chatSoundAsset.uri } as AVPlaybackSource, {
@@ -127,15 +148,7 @@ class NotificationSoundService {
       this.groupChatSound = groupChatSound;
 
       // Load default notification sound
-      const defaultSoundAsset = Asset.fromModule(require('@assets/audio/notification.wav'));
-      await defaultSoundAsset.downloadAsync();
-
-      const { sound: defaultSound } = await Audio.Sound.createAsync({ uri: defaultSoundAsset.localUri || defaultSoundAsset.uri } as AVPlaybackSource, {
-        shouldPlay: false,
-        isLooping: false,
-        volume: 1.0,
-      });
-      this.defaultSound = defaultSound;
+      this.defaultSound = await this.loadSound(require('@assets/audio/notification.wav'), 'default');
 
       logger.debug({
         message: 'Notification audio files loaded successfully',
@@ -235,6 +248,7 @@ class NotificationSoundService {
       }
 
       this.isInitialized = false;
+      this.initializationPromise = null;
 
       logger.info({
         message: 'Notification sound service cleaned up',
diff --git a/src/services/push-notification.ts b/src/services/push-notification.ts
