Signed-off-by: mbrucedogs <mbrucedogs@gmail.com>

src/firebase/services.ts

@@ -327,12 +327,24 @@ export const singerService = {
       throw new Error('Singer already exists');
     }
     
-    // Find the next available numeric key
+    // Find the next available sequential key (0, 1, 2, etc.)
-    const numericKeys = Object.keys(currentSingers)
+    const existingKeys = Object.keys(currentSingers)
-      .map((key) => parseInt(key, 10))
+      .filter(key => /^\d+$/.test(key)) // Only consider numerical keys
-      .filter((num) => !isNaN(num));
+      .map(key => parseInt(key, 10))
-    const nextKey = numericKeys.length > 0 ? Math.max(...numericKeys) + 1 : 0;
+      .sort((a, b) => a - b);
-    const nextKeyStr = String(nextKey);
+    
+    // Find the first gap in the sequence, or use the next number after the highest
+    let nextKey = 0;
+    for (let i = 0; i < existingKeys.length; i++) {
+      if (existingKeys[i] !== i) {
+        nextKey = i;
+        break;
+      }
+      nextKey = i + 1;
+    }
+    
+    debugLog('addSinger - existing keys:', existingKeys);
+    debugLog('addSinger - next key:', nextKey);
     
     // Create new singer with current timestamp
     const newSinger: Omit<Singer, 'key'> = {
@@ -340,11 +352,11 @@ export const singerService = {
       lastLogin: new Date().toISOString()
     };
     
-    // Add to singers list with numeric key
+    // Add to singers list with sequential key
-    const newSingerRef = ref(database, `controllers/${controllerName}/player/singers/${nextKeyStr}`);
+    const newSingerRef = ref(database, `controllers/${controllerName}/player/singers/${nextKey}`);
     await set(newSingerRef, newSinger);
     
-    return { key: nextKeyStr };
+    return { key: nextKey.toString() };
   },
 
   // Remove singer and all their queue items
@@ -370,22 +382,40 @@ export const singerService = {
       }
     }
     
-    // Then, remove the singer from the singers list
+    // Then, remove the singer from the singers list and reindex
     const singersRef = ref(database, `controllers/${controllerName}/player/singers`);
     const singersSnapshot = await get(singersRef);
     
     if (singersSnapshot.exists()) {
       const singers = singersSnapshot.val();
+      debugLog('removeSinger - original singers:', singers);
+      
+      // Get all remaining singers (excluding the one to be removed)
+      const remainingSingers = Object.entries(singers)
+        .filter(([, singer]) => singer && (singer as Singer).name !== singerName)
+        .map(([key, singer]) => ({ key, singer: singer as Singer }))
+        .sort((a, b) => a.singer.name.localeCompare(b.singer.name)); // Keep alphabetical order
+      
+      debugLog('removeSinger - remaining singers:', remainingSingers);
+      
+      // Create a completely new singers list with sequential keys
       const updates: Record<string, Singer | null> = {};
       
-      // Find the singer by name and mark for removal
+      // First, remove all existing singers
-      Object.entries(singers).forEach(([key, singer]) => {
+      Object.keys(singers).forEach(key => {
-        if (singer && (singer as Singer).name === singerName) {
+        updates[key] = null;
-          updates[key] = null; // Mark for removal
-        }
       });
       
-      // Remove the singer
+      // Then, add back the remaining singers with sequential keys
+      remainingSingers.forEach(({ singer }, index) => {
+        const newKey = index.toString();
+        debugLog(`removeSinger - reindexing: old key ${singer.key} -> new key ${newKey}, singer: ${singer.name}`);
+        updates[newKey] = singer;
+      });
+      
+      debugLog('removeSinger - updates to apply:', updates);
+      
+      // Apply all updates atomically
       if (Object.keys(updates).length > 0) {
         await update(singersRef, updates);
       }

src/hooks/useSingers.ts

@@ -1,13 +1,15 @@
 import { useCallback } from 'react';
-import { useAppSelector, selectSingersArray, selectIsAdmin, selectControllerName } from '../redux';
+import { useAppSelector, useAppDispatch } from '../redux';
+import { selectSingersArray, selectIsAdmin, selectControllerName } from '../redux';
+import { addSinger, removeSinger } from '../redux/controllerSlice';
 import { useToast } from './useToast';
-import { singerService } from '../firebase/services';
 import type { Singer } from '../types';
 
 export const useSingers = () => {
   const singers = useAppSelector(selectSingersArray);
   const isAdmin = useAppSelector(selectIsAdmin);
   const controllerName = useAppSelector(selectControllerName);
+  const dispatch = useAppDispatch();
   const toast = useToast();
   const showSuccess = toast?.showSuccess;
   const showError = toast?.showError;
@@ -24,13 +26,13 @@ export const useSingers = () => {
     }
 
     try {
-      await singerService.removeSinger(controllerName, singer.name);
+      await dispatch(removeSinger({ controllerName, singerName: singer.name })).unwrap();
       showSuccess && showSuccess(`${singer.name} removed from singers list and queue`);
     } catch (error) {
       console.error('Failed to remove singer:', error);
       showError && showError('Failed to remove singer');
     }
-  }, [isAdmin, controllerName, showSuccess, showError]);
+  }, [isAdmin, controllerName, dispatch, showSuccess, showError]);
 
   const handleAddSinger = useCallback(async (singerName: string) => {
     if (!isAdmin) {
@@ -49,7 +51,7 @@ export const useSingers = () => {
     }
 
     try {
-      await singerService.addSinger(controllerName, singerName.trim());
+      await dispatch(addSinger({ controllerName, singerName: singerName.trim() })).unwrap();
       showSuccess && showSuccess(`${singerName} added to singers list`);
     } catch (error) {
       console.error('Failed to add singer:', error);
@@ -59,7 +61,7 @@ export const useSingers = () => {
         showError && showError('Failed to add singer');
       }
     }
-  }, [isAdmin, controllerName, showSuccess, showError]);
+  }, [isAdmin, controllerName, dispatch, showSuccess, showError]);
 
   return {
     singers,

src/redux/controllerSlice.ts

@@ -1,7 +1,7 @@
 import { createSlice, createAsyncThunk } from '@reduxjs/toolkit';
 import type { PayloadAction } from '@reduxjs/toolkit';
-import type { Controller, Song, QueueItem, TopPlayed } from '../types';
+import type { Controller, Song, QueueItem, TopPlayed, Singer } from '../types';
-import { controllerService } from '../firebase/services';
+import { controllerService, singerService } from '../firebase/services';
 
 // Async thunks for Firebase operations
 export const fetchController = createAsyncThunk(
@@ -23,6 +23,22 @@ export const updateController = createAsyncThunk(
   }
 );
 
+export const addSinger = createAsyncThunk(
+  'controller/addSinger',
+  async ({ controllerName, singerName }: { controllerName: string; singerName: string }) => {
+    const result = await singerService.addSinger(controllerName, singerName);
+    return { key: result.key, singerName };
+  }
+);
+
+export const removeSinger = createAsyncThunk(
+  'controller/removeSinger',
+  async ({ controllerName, singerName }: { controllerName: string; singerName: string }) => {
+    await singerService.removeSinger(controllerName, singerName);
+    return { singerName };
+  }
+);
+
 // Initial state
 interface ControllerState {
   data: Controller | null;
@@ -85,6 +101,13 @@ const controllerSlice = createSlice({
       }
     },
     
+    updateSingers: (state, action: PayloadAction<Record<string, Singer>>) => {
+      if (state.data) {
+        state.data.player.singers = action.payload;
+        state.lastUpdated = Date.now();
+      }
+    },
+    
     clearError: (state) => {
       state.error = null;
     },
@@ -129,6 +152,36 @@ const controllerSlice = createSlice({
       .addCase(updateController.rejected, (state, action) => {
         state.loading = false;
         state.error = action.error.message || 'Failed to update controller';
+      })
+      // addSinger
+      .addCase(addSinger.pending, (state) => {
+        state.loading = true;
+        state.error = null;
+      })
+      .addCase(addSinger.fulfilled, (state) => {
+        state.loading = false;
+        // The real-time sync will handle the update
+        state.lastUpdated = Date.now();
+        state.error = null;
+      })
+      .addCase(addSinger.rejected, (state, action) => {
+        state.loading = false;
+        state.error = action.error.message || 'Failed to add singer';
+      })
+      // removeSinger
+      .addCase(removeSinger.pending, (state) => {
+        state.loading = true;
+        state.error = null;
+      })
+      .addCase(removeSinger.fulfilled, (state) => {
+        state.loading = false;
+        // The real-time sync will handle the update
+        state.lastUpdated = Date.now();
+        state.error = null;
+      })
+      .addCase(removeSinger.rejected, (state, action) => {
+        state.loading = false;
+        state.error = action.error.message || 'Failed to remove singer';
       });
   },
 });
@@ -141,6 +194,7 @@ export const {
   updateFavorites,
   updateHistory,
   updateTopPlayed,
+  updateSingers,
   clearError,
   resetController,
 } = controllerSlice.actions;