Refactor UserRoleManager mutations

src/components/moderation/UserRoleManager.tsx

@@ -9,9 +9,11 @@ import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from '@
 import { supabase } from '@/integrations/supabase/client';
 import { useAuth } from '@/hooks/useAuth';
 import { useUserRole } from '@/hooks/useUserRole';
-import { handleError, handleSuccess, getErrorMessage } from '@/lib/errorHandler';
+import { handleError, getErrorMessage } from '@/lib/errorHandler';
 import { logger } from '@/lib/logger';
-import { useQueryInvalidation } from '@/lib/queryInvalidation';
+import { useUserRoles } from '@/hooks/users/useUserRoles';
+import { useUserSearch } from '@/hooks/users/useUserSearch';
+import { useRoleMutations } from '@/hooks/users/useRoleMutations';
 
 // Type-safe role definitions
 const VALID_ROLES = ['admin', 'moderator', 'user'] as const;
@@ -53,192 +55,36 @@ interface UserRole {
   };
 }
 export function UserRoleManager() {
-  const [userRoles, setUserRoles] = useState<UserRole[]>([]);
-  const [loading, setLoading] = useState(true);
   const [searchTerm, setSearchTerm] = useState('');
   const [newUserSearch, setNewUserSearch] = useState('');
   const [newRole, setNewRole] = useState('');
-  const [searchResults, setSearchResults] = useState<ProfileSearchResult[]>([]);
-  const [actionLoading, setActionLoading] = useState<string | null>(null);
-  const {
-    user
-  } = useAuth();
-  const {
-    isAdmin,
-    isSuperuser,
-    permissions
-  } = useUserRole();
+  const [selectedUsers, setSelectedUsers] = useState<ProfileSearchResult[]>([]);
   
-  // Cache invalidation for role changes
-  const { invalidateUserAuth, invalidateModerationStats } = useQueryInvalidation();
-  const fetchUserRoles = async () => {
-    try {
-      const {
-        data,
-        error
-      } = await supabase.from('user_roles').select(`
-          id,
-          user_id,
-          role,
-          granted_at
-        `).order('granted_at', {
-        ascending: false
-      });
-      if (error) throw error;
+  const { user } = useAuth();
+  const { isAdmin, isSuperuser } = useUserRole();
+  const { data: userRoles = [], isLoading: loading } = useUserRoles();
+  const { data: searchResults = [] } = useUserSearch(newUserSearch);
+  const { grantRole, revokeRole } = useRoleMutations();
+  const handleGrantRole = () => {
+    const selectedUser = selectedUsers[0];
+    if (!selectedUser || !newRole || !isValidRole(newRole)) return;
 
-      // Get unique user IDs
-      const userIds = [...new Set((data || []).map(r => r.user_id))];
-
-      // Fetch user profiles with emails (for admins)
-      let profiles: Array<{ user_id: string; username: string; display_name?: string }> | null = null;
-      const { data: allProfiles, error: rpcError } = await supabase
-        .rpc('get_users_with_emails');
-      
-      if (rpcError) {
-        logger.warn('Failed to fetch users with emails, using basic profiles', { error: getErrorMessage(rpcError) });
-        const { data: basicProfiles } = await supabase
-          .from('profiles')
-          .select('user_id, username, display_name')
-          .in('user_id', userIds);
-        profiles = basicProfiles as typeof profiles;
-      } else {
-        profiles = allProfiles?.filter(p => userIds.includes(p.user_id)) || null;
+    grantRole.mutate(
+      { userId: selectedUser.user_id, role: newRole },
+      {
+        onSuccess: () => {
+          setNewUserSearch('');
+          setNewRole('');
+          setSelectedUsers([]);
+        },
       }
-      const profileMap = new Map(profiles?.map(p => [p.user_id, p]) || []);
+    );
+  };
 
-      // Combine data with profiles
-      const userRolesWithProfiles = (data || []).map(role => ({
-        ...role,
-        profiles: profileMap.get(role.user_id)
-      }));
-      setUserRoles(userRolesWithProfiles);
-    } catch (error: unknown) {
-      handleError(error, {
-        action: 'Load User Roles',
-        userId: user?.id
-      });
-    } finally {
-      setLoading(false);
-    }
+  const handleRevokeRole = (roleId: string) => {
+    revokeRole.mutate({ roleId });
   };
-  const searchUsers = async (search: string) => {
-    if (!search.trim()) {
-      setSearchResults([]);
-      return;
-    }
-    try {
-      let data;
-      const { data: allUsers, error: rpcError } = await supabase
-        .rpc('get_users_with_emails');
-      
-      if (rpcError) {
-        logger.warn('Failed to fetch users with emails, using basic profiles', { error: getErrorMessage(rpcError) });
-        const { data: basicProfiles, error: profilesError } = await supabase
-          .from('profiles')
-          .select('user_id, username, display_name')
-          .ilike('username', `%${search}%`);
-        
-        if (profilesError) throw profilesError;
-        data = basicProfiles?.slice(0, 10);
-      } else {
-        // Filter by search term
-        data = allUsers?.filter(user => 
-          user.username.toLowerCase().includes(search.toLowerCase()) ||
-          user.display_name?.toLowerCase().includes(search.toLowerCase())
-        ).slice(0, 10);
-      }
 
-      // Filter out users who already have roles
-      const existingUserIds = userRoles.map(ur => ur.user_id);
-      const filteredResults = (data || []).filter(profile => !existingUserIds.includes(profile.user_id));
-      setSearchResults(filteredResults);
-    } catch (error: unknown) {
-      logger.error('User search failed', { error: getErrorMessage(error) });
-    }
-  };
-  useEffect(() => {
-    fetchUserRoles();
-  }, []);
-  useEffect(() => {
-    const debounceTimer = setTimeout(() => {
-      searchUsers(newUserSearch);
-    }, 300);
-    return () => clearTimeout(debounceTimer);
-  }, [newUserSearch, userRoles]);
-  const grantRole = async (userId: string, role: ValidRole) => {
-    if (!isAdmin()) return;
-    
-    // Double-check role validity before database operation
-    if (!isValidRole(role)) {
-      handleError(new Error('Invalid role'), {
-        action: 'Grant Role',
-        userId: user?.id,
-        metadata: { targetUserId: userId, attemptedRole: role }
-      });
-      return;
-    }
-    
-    setActionLoading('grant');
-    try {
-      const {
-        error
-      } = await supabase.from('user_roles').insert([{
-        user_id: userId,
-        role,
-        granted_by: user?.id
-      }]);
-      
-      if (error) throw error;
-      
-      handleSuccess('Role Granted', `User has been granted ${getRoleLabel(role)} role`);
-      
-      // Invalidate caches instead of manual refetch
-      invalidateUserAuth(userId);
-      invalidateModerationStats(); // Role changes affect who can moderate
-      
-      setNewUserSearch('');
-      setNewRole('');
-      setSearchResults([]);
-      fetchUserRoles();
-    } catch (error: unknown) {
-      handleError(error, {
-        action: 'Grant Role',
-        userId: user?.id,
-        metadata: { targetUserId: userId, role }
-      });
-    } finally {
-      setActionLoading(null);
-    }
-  };
-  const revokeRole = async (roleId: string) => {
-    if (!isAdmin()) return;
-    setActionLoading(roleId);
-    try {
-      const {
-        error
-      } = await supabase.from('user_roles').delete().eq('id', roleId);
-      if (error) throw error;
-      
-      handleSuccess('Role Revoked', 'User role has been revoked');
-      
-      // Invalidate caches instead of manual refetch
-      const revokedRole = userRoles.find(r => r.id === roleId);
-      if (revokedRole) {
-        invalidateUserAuth(revokedRole.user_id);
-        invalidateModerationStats();
-      }
-      
-      fetchUserRoles();
-    } catch (error: unknown) {
-      handleError(error, {
-        action: 'Revoke Role',
-        userId: user?.id,
-        metadata: { roleId }
-      });
-    } finally {
-      setActionLoading(null);
-    }
-  };
   if (!isAdmin()) {
     return <div className="text-center py-8">
         <Shield className="w-12 h-12 text-muted-foreground mx-auto mb-4" />
@@ -253,7 +99,17 @@ export function UserRoleManager() {
         <div className="animate-spin rounded-full h-6 w-6 border-b-2 border-primary"></div>
       </div>;
   }
-  const filteredRoles = userRoles.filter(role => role.profiles?.username?.toLowerCase().includes(searchTerm.toLowerCase()) || role.profiles?.display_name?.toLowerCase().includes(searchTerm.toLowerCase()) || role.role.toLowerCase().includes(searchTerm.toLowerCase()));
+  // Filter existing user IDs for search results
+  const existingUserIds = userRoles.map(ur => ur.user_id);
+  const availableSearchResults = searchResults.filter(
+    profile => !existingUserIds.includes(profile.user_id)
+  );
+
+  const filteredRoles = userRoles.filter(
+    role =>
+      role.username?.toLowerCase().includes(searchTerm.toLowerCase()) ||
+      role.display_name?.toLowerCase().includes(searchTerm.toLowerCase())
+  );
   return <div className="space-y-6">
       {/* Add new role */}
       <Card>
@@ -267,10 +123,10 @@ export function UserRoleManager() {
                 <Input id="user-search" placeholder="Search by username or display name..." value={newUserSearch} onChange={e => setNewUserSearch(e.target.value)} className="pl-10" />
               </div>
               
-              {searchResults.length > 0 && <div className="mt-2 border rounded-lg bg-background">
-                  {searchResults.map(profile => <div key={profile.user_id} className="p-3 hover:bg-muted/50 cursor-pointer border-b last:border-b-0" onClick={() => {
+              {availableSearchResults.length > 0 && <div className="mt-2 border rounded-lg bg-background">
+                  {availableSearchResults.map(profile => <div key={profile.user_id} className="p-3 hover:bg-muted/50 cursor-pointer border-b last:border-b-0" onClick={() => {
                 setNewUserSearch(profile.display_name || profile.username);
-                setSearchResults([profile]);
+                setSelectedUsers([profile]);
               }}>
                       <div className="font-medium">
                         {profile.display_name || profile.username}
@@ -296,23 +152,13 @@ export function UserRoleManager() {
             </div>
           </div>
           
-          <Button onClick={() => {
-          const selectedUser = searchResults.find(p => (p.display_name || p.username) === newUserSearch);
-          
-          // Type-safe validation before calling grantRole
-          if (selectedUser && newRole && isValidRole(newRole)) {
-            grantRole(selectedUser.user_id, newRole);
-          } else if (selectedUser && newRole) {
-            // This should never happen due to Select component constraints,
-            // but provides safety in case of UI bugs
-            handleError(new Error('Invalid role selected'), {
-              action: 'Grant Role',
-              userId: user?.id,
-              metadata: { selectedUser: selectedUser?.user_id, newRole }
-            });
-          }
-        }} disabled={!newRole || !isValidRole(newRole) || !searchResults.find(p => (p.display_name || p.username) === newUserSearch) || actionLoading === 'grant'} className="w-full md:w-auto">
-            {actionLoading === 'grant' ? 'Granting...' : 'Grant Role'}
+          <Button 
+            onClick={handleGrantRole}
+            disabled={!newRole || !isValidRole(newRole) || selectedUsers.length === 0 || grantRole.isPending} 
+            className="w-full md:w-auto"
+          >
+            <UserPlus className="w-4 h-4 mr-2" />
+            {grantRole.isPending ? 'Granting...' : 'Grant Role'}
           </Button>
         </CardContent>
       </Card>
@@ -339,21 +185,31 @@ export function UserRoleManager() {
                 <div className="flex items-center gap-3">
                   <div>
                     <div className="font-medium">
-                      {userRole.profiles?.display_name || userRole.profiles?.username}
+                      {userRole.display_name || userRole.username}
                     </div>
-                    {userRole.profiles?.display_name && <div className="text-sm text-muted-foreground">
-                        @{userRole.profiles.username}
+                    {userRole.display_name && <div className="text-sm text-muted-foreground">
+                        @{userRole.username}
+                      </div>}
+                    {userRole.email && <div className="text-xs text-muted-foreground">
+                        {userRole.email}
                       </div>}
                   </div>
-                  <Badge variant={userRole.role === 'admin' ? 'default' : 'secondary'}>
-                    {userRole.role}
+                  <Badge variant={userRole.id === 'admin' ? 'default' : 'secondary'}>
+                    {getRoleLabel(userRole.id)}
                   </Badge>
                 </div>
                 
                 {/* Only show revoke button if current user can manage this role */}
-                {(isSuperuser() || isAdmin() && !['admin', 'superuser'].includes(userRole.role)) && <Button variant="outline" size="sm" onClick={() => revokeRole(userRole.id)} disabled={actionLoading === userRole.id}>
+                {(isSuperuser() || (isAdmin() && !['admin', 'superuser'].includes(userRole.id))) && (
+                  <Button 
+                    variant="outline" 
+                    size="sm" 
+                    onClick={() => handleRevokeRole(userRole.id)} 
+                    disabled={revokeRole.isPending}
+                  >
                     <X className="w-4 h-4" />
-                  </Button>}
+                  </Button>
+                )}
               </CardContent>
             </Card>)}
       </div>

src/hooks/users/useRoleMutations.ts

@@ -0,0 +1,144 @@
+import { useMutation, useQueryClient } from '@tanstack/react-query';
+import { supabase } from '@/integrations/supabase/client';
+import { queryKeys } from '@/lib/queryKeys';
+import { toast } from 'sonner';
+
+type ValidRole = 'admin' | 'moderator' | 'user';
+
+interface GrantRoleParams {
+  userId: string;
+  role: ValidRole;
+}
+
+interface RevokeRoleParams {
+  roleId: string;
+}
+
+interface UserWithRoles {
+  id: string;
+  user_id: string;
+  username: string;
+  email: string;
+  display_name: string | null;
+  avatar_url: string | null;
+  banned: boolean;
+  created_at: string;
+}
+
+/**
+ * useRoleMutations Hook
+ * 
+ * Provides TanStack Query mutations for granting and revoking user roles
+ * with optimistic updates for instant UI feedback.
+ * 
+ * Features:
+ * - Optimistic updates for immediate UI response
+ * - Automatic cache invalidation on success
+ * - Error handling with rollback
+ * - Toast notifications
+ * 
+ * @example
+ * ```tsx
+ * const { grantRole, revokeRole } = useRoleMutations();
+ * 
+ * grantRole.mutate({ userId: 'user-id', role: 'moderator' });
+ * revokeRole.mutate({ roleId: 'role-id' });
+ * ```
+ */
+export function useRoleMutations() {
+  const queryClient = useQueryClient();
+
+  const grantRole = useMutation({
+    mutationFn: async ({ userId, role }: GrantRoleParams) => {
+      const { data, error } = await supabase
+        .from('user_roles')
+        .insert([{ user_id: userId, role }])
+        .select()
+        .single();
+
+      if (error) throw error;
+      return data;
+    },
+    onMutate: async ({ userId, role }) => {
+      // Cancel outgoing refetches
+      await queryClient.cancelQueries({ queryKey: queryKeys.users.roles() });
+
+      // Snapshot previous value
+      const previousUsers = queryClient.getQueryData<UserWithRoles[]>(queryKeys.users.roles());
+
+      // Optimistically update cache - add role to user
+      queryClient.setQueryData<UserWithRoles[]>(queryKeys.users.roles(), (old) => {
+        if (!old) return old;
+        return old.map((user) =>
+          user.user_id === userId
+            ? { ...user, role } // Optimistically assign role
+            : user
+        );
+      });
+
+      return { previousUsers };
+    },
+    onError: (error, variables, context) => {
+      // Rollback on error
+      if (context?.previousUsers) {
+        queryClient.setQueryData(queryKeys.users.roles(), context.previousUsers);
+      }
+      toast.error(`Failed to grant role: ${error.message}`);
+    },
+    onSuccess: (data, { role }) => {
+      toast.success(`Role ${role} granted successfully`);
+    },
+    onSettled: () => {
+      // Refetch to ensure consistency
+      queryClient.invalidateQueries({ queryKey: queryKeys.users.roles() });
+      queryClient.invalidateQueries({ queryKey: ['user-roles'] });
+    },
+  });
+
+  const revokeRole = useMutation({
+    mutationFn: async ({ roleId }: RevokeRoleParams) => {
+      const { error } = await supabase
+        .from('user_roles')
+        .delete()
+        .eq('id', roleId);
+
+      if (error) throw error;
+    },
+    onMutate: async ({ roleId }) => {
+      // Cancel outgoing refetches
+      await queryClient.cancelQueries({ queryKey: queryKeys.users.roles() });
+
+      // Snapshot previous value
+      const previousUsers = queryClient.getQueryData<UserWithRoles[]>(queryKeys.users.roles());
+
+      // Optimistically remove role from cache
+      queryClient.setQueryData<UserWithRoles[]>(queryKeys.users.roles(), (old) => {
+        if (!old) return old;
+        // Remove the user from the list since they no longer have a role
+        return old.filter((user) => user.id !== roleId);
+      });
+
+      return { previousUsers };
+    },
+    onError: (error, variables, context) => {
+      // Rollback on error
+      if (context?.previousUsers) {
+        queryClient.setQueryData(queryKeys.users.roles(), context.previousUsers);
+      }
+      toast.error(`Failed to revoke role: ${error.message}`);
+    },
+    onSuccess: () => {
+      toast.success('Role revoked successfully');
+    },
+    onSettled: () => {
+      // Refetch to ensure consistency
+      queryClient.invalidateQueries({ queryKey: queryKeys.users.roles() });
+      queryClient.invalidateQueries({ queryKey: ['user-roles'] });
+    },
+  });
+
+  return {
+    grantRole,
+    revokeRole,
+  };
+}