Fix notification UX, admin panel error handling, and data bugs
Notification fixes: - Add NotificationToaster component with real-time toast notifications for new incoming notifications (30s polling, 15s stale time) - Connection request toasts show inline Accept/Reject buttons - Add inline Accept/Reject buttons to connection_request notifications in NotificationsPage (prevents bricked requests after navigation) - Don't mark connection_request as read or navigate away when pending - Auto-refetch notification list when unread count increases Admin panel fixes: - Add error state UI to UserDetailSection and ConfigPage (previously silently returned null/empty on API errors) - Fix get_user response missing must_change_password and locked_until - Fix create_user response missing preferred_name and date_of_birth - Add defensive limit(1) on settings query to prevent MultipleResultsFound - Guard _target_username_col JSONB cast with CASE to prevent crash on non-JSON audit detail values - Add connection audit action types to ConfigPage filter dropdown Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
03abbbf8a7
commit
03fd8dba97
@ -70,10 +70,21 @@ def _target_username_col(target_alias, audit_model):
|
||||
COALESCE: prefer the live username from the users table,
|
||||
fall back to the username stored in the audit detail JSON
|
||||
(survives user deletion since audit_log.target_user_id → SET NULL).
|
||||
Guard the JSONB cast with a CASE to avoid errors on non-JSON detail values.
|
||||
"""
|
||||
json_fallback = sa.case(
|
||||
(
|
||||
sa.and_(
|
||||
audit_model.detail.is_not(None),
|
||||
audit_model.detail.startswith("{"),
|
||||
),
|
||||
sa.cast(audit_model.detail, JSONB)["username"].as_string(),
|
||||
),
|
||||
else_=sa.null(),
|
||||
)
|
||||
return sa.func.coalesce(
|
||||
target_alias.username,
|
||||
sa.cast(audit_model.detail, JSONB)["username"].as_string(),
|
||||
json_fallback,
|
||||
).label("target_username")
|
||||
|
||||
|
||||
@ -170,9 +181,9 @@ async def get_user(
|
||||
)
|
||||
active_sessions = session_result.scalar_one()
|
||||
|
||||
# Fetch preferred_name from Settings
|
||||
# Fetch preferred_name from Settings (limit 1 defensive)
|
||||
settings_result = await db.execute(
|
||||
sa.select(Settings.preferred_name).where(Settings.user_id == user_id)
|
||||
sa.select(Settings.preferred_name).where(Settings.user_id == user_id).limit(1)
|
||||
)
|
||||
preferred_name = settings_result.scalar_one_or_none()
|
||||
|
||||
@ -181,6 +192,8 @@ async def get_user(
|
||||
active_sessions=active_sessions,
|
||||
preferred_name=preferred_name,
|
||||
date_of_birth=user.date_of_birth,
|
||||
must_change_password=user.must_change_password,
|
||||
locked_until=user.locked_until,
|
||||
)
|
||||
|
||||
|
||||
@ -242,6 +255,10 @@ async def create_user(
|
||||
return UserDetailResponse(
|
||||
**UserListItem.model_validate(new_user).model_dump(exclude={"active_sessions"}),
|
||||
active_sessions=0,
|
||||
preferred_name=data.preferred_name,
|
||||
date_of_birth=None,
|
||||
must_change_password=new_user.must_change_password,
|
||||
locked_until=new_user.locked_until,
|
||||
)
|
||||
|
||||
|
||||
|
||||
@ -30,6 +30,10 @@ const ACTION_TYPES = [
|
||||
'auth.setup_complete',
|
||||
'auth.registration',
|
||||
'auth.mfa_enforce_prompted',
|
||||
'connection.request_sent',
|
||||
'connection.accepted',
|
||||
'connection.rejected',
|
||||
'connection.removed',
|
||||
];
|
||||
|
||||
function actionLabel(action: string): string {
|
||||
@ -44,7 +48,7 @@ export default function ConfigPage() {
|
||||
const [filterAction, setFilterAction] = useState<string>('');
|
||||
const PER_PAGE = 25;
|
||||
|
||||
const { data, isLoading } = useAuditLog(page, PER_PAGE, filterAction || undefined);
|
||||
const { data, isLoading, error } = useAuditLog(page, PER_PAGE, filterAction || undefined);
|
||||
|
||||
const totalPages = data ? Math.ceil(data.total / PER_PAGE) : 1;
|
||||
|
||||
@ -111,6 +115,11 @@ export default function ConfigPage() {
|
||||
<Skeleton key={i} className="h-10 w-full" />
|
||||
))}
|
||||
</div>
|
||||
) : error ? (
|
||||
<div className="px-5 pb-5">
|
||||
<p className="text-sm text-destructive">Failed to load audit log</p>
|
||||
<p className="text-xs text-muted-foreground mt-1">{error.message}</p>
|
||||
</div>
|
||||
) : !data?.entries?.length ? (
|
||||
<p className="px-5 pb-5 text-sm text-muted-foreground">No audit entries found.</p>
|
||||
) : (
|
||||
|
||||
@ -55,7 +55,7 @@ function MfaBadge({ enabled, pending }: { enabled: boolean; pending: boolean })
|
||||
}
|
||||
|
||||
export default function UserDetailSection({ userId, onClose }: UserDetailSectionProps) {
|
||||
const { data: user, isLoading } = useAdminUserDetail(userId);
|
||||
const { data: user, isLoading, error } = useAdminUserDetail(userId);
|
||||
const updateRole = useUpdateRole();
|
||||
|
||||
const handleRoleChange = async (newRole: UserRole) => {
|
||||
@ -89,6 +89,22 @@ export default function UserDetailSection({ userId, onClose }: UserDetailSection
|
||||
);
|
||||
}
|
||||
|
||||
if (error) {
|
||||
return (
|
||||
<Card>
|
||||
<CardContent className="p-5">
|
||||
<div className="flex items-center justify-between">
|
||||
<p className="text-sm text-destructive">Failed to load user details</p>
|
||||
<Button variant="ghost" size="sm" className="h-6 w-6 p-0" onClick={onClose}>
|
||||
<X className="h-3.5 w-3.5" />
|
||||
</Button>
|
||||
</div>
|
||||
<p className="text-xs text-muted-foreground mt-1">{error.message}</p>
|
||||
</CardContent>
|
||||
</Card>
|
||||
);
|
||||
}
|
||||
|
||||
if (!user) return null;
|
||||
|
||||
return (
|
||||
|
||||
@ -7,6 +7,7 @@ import { LockProvider } from '@/hooks/useLock';
|
||||
import { Button } from '@/components/ui/button';
|
||||
import Sidebar from './Sidebar';
|
||||
import LockOverlay from './LockOverlay';
|
||||
import NotificationToaster from '@/components/notifications/NotificationToaster';
|
||||
|
||||
export default function AppLayout() {
|
||||
useTheme();
|
||||
@ -44,6 +45,7 @@ export default function AppLayout() {
|
||||
</div>
|
||||
</div>
|
||||
<LockOverlay />
|
||||
<NotificationToaster />
|
||||
</AlertsProvider>
|
||||
</LockProvider>
|
||||
);
|
||||
|
||||
106
frontend/src/components/notifications/NotificationToaster.tsx
Normal file
106
frontend/src/components/notifications/NotificationToaster.tsx
Normal file
@ -0,0 +1,106 @@
|
||||
import { useEffect, useRef, useCallback } from 'react';
|
||||
import { toast } from 'sonner';
|
||||
import { Check, X, Bell, UserPlus } from 'lucide-react';
|
||||
import { useQueryClient } from '@tanstack/react-query';
|
||||
import { useNotifications } from '@/hooks/useNotifications';
|
||||
import api from '@/lib/api';
|
||||
import type { AppNotification } from '@/types';
|
||||
|
||||
export default function NotificationToaster() {
|
||||
const { notifications } = useNotifications();
|
||||
const queryClient = useQueryClient();
|
||||
const seenIdsRef = useRef(new Set<number>());
|
||||
const initializedRef = useRef(false);
|
||||
|
||||
const handleConnectionRespond = useCallback(
|
||||
async (requestId: number, action: 'accept' | 'reject', toastId: string | number) => {
|
||||
try {
|
||||
await api.put(`/connections/requests/${requestId}/respond`, { action });
|
||||
toast.dismiss(toastId);
|
||||
toast.success(action === 'accept' ? 'Connection accepted' : 'Request declined');
|
||||
queryClient.invalidateQueries({ queryKey: ['connections'] });
|
||||
queryClient.invalidateQueries({ queryKey: ['people'] });
|
||||
queryClient.invalidateQueries({ queryKey: ['notifications'] });
|
||||
} catch {
|
||||
toast.dismiss(toastId);
|
||||
toast.error('Failed to respond to request');
|
||||
}
|
||||
},
|
||||
[queryClient],
|
||||
);
|
||||
|
||||
useEffect(() => {
|
||||
if (!notifications.length && !initializedRef.current) return;
|
||||
|
||||
// On first load, record all existing IDs without toasting
|
||||
if (!initializedRef.current) {
|
||||
notifications.forEach((n) => seenIdsRef.current.add(n.id));
|
||||
initializedRef.current = true;
|
||||
return;
|
||||
}
|
||||
|
||||
// Find new notifications we haven't seen
|
||||
const newNotifications = notifications.filter(
|
||||
(n) => !n.is_read && !seenIdsRef.current.has(n.id),
|
||||
);
|
||||
|
||||
// Record all current IDs
|
||||
notifications.forEach((n) => seenIdsRef.current.add(n.id));
|
||||
|
||||
// Show toasts for new notifications
|
||||
newNotifications.forEach((notification) => {
|
||||
if (notification.type === 'connection_request' && notification.source_id) {
|
||||
showConnectionRequestToast(notification);
|
||||
} else {
|
||||
toast(notification.title || 'New Notification', {
|
||||
description: notification.message || undefined,
|
||||
icon: <Bell className="h-4 w-4" />,
|
||||
duration: 8000,
|
||||
});
|
||||
}
|
||||
});
|
||||
}, [notifications, handleConnectionRespond]);
|
||||
|
||||
const showConnectionRequestToast = (notification: AppNotification) => {
|
||||
const requestId = notification.source_id!;
|
||||
const senderName =
|
||||
(notification.data as Record<string, string>)?.sender_umbral_name || 'Someone';
|
||||
|
||||
toast.custom(
|
||||
(id) => (
|
||||
<div className="w-[356px] rounded-lg border border-border bg-card p-4 shadow-lg">
|
||||
<div className="flex items-start gap-3">
|
||||
<div className="h-9 w-9 rounded-full bg-violet-500/15 flex items-center justify-center shrink-0">
|
||||
<UserPlus className="h-4 w-4 text-violet-400" />
|
||||
</div>
|
||||
<div className="flex-1 min-w-0">
|
||||
<p className="text-sm font-medium text-foreground">Connection Request</p>
|
||||
<p className="text-xs text-muted-foreground mt-0.5">
|
||||
{notification.message || `${senderName} wants to connect with you`}
|
||||
</p>
|
||||
<div className="flex items-center gap-2 mt-3">
|
||||
<button
|
||||
onClick={() => handleConnectionRespond(requestId, 'accept', id)}
|
||||
className="flex items-center gap-1 px-3 py-1.5 text-xs font-medium rounded-md bg-accent text-accent-foreground hover:bg-accent/90 transition-colors"
|
||||
>
|
||||
<Check className="h-3.5 w-3.5" />
|
||||
Accept
|
||||
</button>
|
||||
<button
|
||||
onClick={() => handleConnectionRespond(requestId, 'reject', id)}
|
||||
className="flex items-center gap-1 px-3 py-1.5 text-xs font-medium rounded-md text-muted-foreground hover:bg-card-elevated transition-colors"
|
||||
>
|
||||
<X className="h-3.5 w-3.5" />
|
||||
Reject
|
||||
</button>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
),
|
||||
{ duration: 30000 },
|
||||
);
|
||||
};
|
||||
|
||||
return null;
|
||||
}
|
||||
@ -1,10 +1,13 @@
|
||||
import { useState, useMemo } from 'react';
|
||||
import { useNavigate } from 'react-router-dom';
|
||||
import { Bell, Check, CheckCheck, Trash2, UserPlus, Info, AlertCircle } from 'lucide-react';
|
||||
import { Bell, Check, CheckCheck, Trash2, UserPlus, Info, AlertCircle, X, Loader2 } from 'lucide-react';
|
||||
import { formatDistanceToNow } from 'date-fns';
|
||||
import { toast } from 'sonner';
|
||||
import { useNotifications } from '@/hooks/useNotifications';
|
||||
import { useConnections } from '@/hooks/useConnections';
|
||||
import { Button } from '@/components/ui/button';
|
||||
import { cn } from '@/lib/utils';
|
||||
import { getErrorMessage } from '@/lib/api';
|
||||
import { ListSkeleton } from '@/components/ui/skeleton';
|
||||
import type { AppNotification } from '@/types';
|
||||
|
||||
@ -27,9 +30,16 @@ export default function NotificationsPage() {
|
||||
deleteNotification,
|
||||
} = useNotifications();
|
||||
|
||||
const { incomingRequests, respond, isResponding } = useConnections();
|
||||
const navigate = useNavigate();
|
||||
const [filter, setFilter] = useState<Filter>('all');
|
||||
|
||||
// Build a set of pending connection request IDs for quick lookup
|
||||
const pendingRequestIds = useMemo(
|
||||
() => new Set(incomingRequests.map((r) => r.id)),
|
||||
[incomingRequests],
|
||||
);
|
||||
|
||||
const filtered = useMemo(() => {
|
||||
if (filter === 'unread') return notifications.filter((n) => !n.is_read);
|
||||
return notifications;
|
||||
@ -58,7 +68,31 @@ export default function NotificationsPage() {
|
||||
return config;
|
||||
};
|
||||
|
||||
const handleConnectionRespond = async (
|
||||
notification: AppNotification,
|
||||
action: 'accept' | 'reject',
|
||||
) => {
|
||||
if (!notification.source_id) return;
|
||||
try {
|
||||
await respond({ requestId: notification.source_id, action });
|
||||
if (!notification.is_read) {
|
||||
await markRead([notification.id]).catch(() => {});
|
||||
}
|
||||
toast.success(action === 'accept' ? 'Connection accepted' : 'Request declined');
|
||||
} catch (err) {
|
||||
toast.error(getErrorMessage(err, 'Failed to respond'));
|
||||
}
|
||||
};
|
||||
|
||||
const handleNotificationClick = async (notification: AppNotification) => {
|
||||
// Don't navigate for pending connection requests — let user act inline
|
||||
if (
|
||||
notification.type === 'connection_request' &&
|
||||
notification.source_id &&
|
||||
pendingRequestIds.has(notification.source_id)
|
||||
) {
|
||||
return;
|
||||
}
|
||||
if (!notification.is_read) {
|
||||
await markRead([notification.id]).catch(() => {});
|
||||
}
|
||||
@ -168,6 +202,32 @@ export default function NotificationsPage() {
|
||||
</div>
|
||||
</div>
|
||||
|
||||
{/* Connection request actions (inline) */}
|
||||
{notification.type === 'connection_request' &&
|
||||
notification.source_id &&
|
||||
pendingRequestIds.has(notification.source_id) && (
|
||||
<div className="flex items-center gap-1.5 shrink-0">
|
||||
<Button
|
||||
size="sm"
|
||||
onClick={(e) => { e.stopPropagation(); handleConnectionRespond(notification, 'accept'); }}
|
||||
disabled={isResponding}
|
||||
className="gap-1 h-7 text-xs"
|
||||
>
|
||||
{isResponding ? <Loader2 className="h-3 w-3 animate-spin" /> : <Check className="h-3 w-3" />}
|
||||
Accept
|
||||
</Button>
|
||||
<Button
|
||||
variant="ghost"
|
||||
size="sm"
|
||||
onClick={(e) => { e.stopPropagation(); handleConnectionRespond(notification, 'reject'); }}
|
||||
disabled={isResponding}
|
||||
className="h-7 text-xs"
|
||||
>
|
||||
<X className="h-3 w-3" />
|
||||
</Button>
|
||||
</div>
|
||||
)}
|
||||
|
||||
{/* Timestamp + actions */}
|
||||
<div className="flex items-center gap-1.5 shrink-0">
|
||||
<span className="text-[11px] text-muted-foreground tabular-nums">
|
||||
|
||||
@ -6,6 +6,7 @@ import type { NotificationListResponse } from '@/types';
|
||||
export function useNotifications() {
|
||||
const queryClient = useQueryClient();
|
||||
const visibleRef = useRef(true);
|
||||
const prevUnreadRef = useRef<number | undefined>(undefined);
|
||||
|
||||
// Track tab visibility to pause polling when hidden
|
||||
useEffect(() => {
|
||||
@ -22,10 +23,20 @@ export function useNotifications() {
|
||||
const { data } = await api.get<{ count: number }>('/notifications/unread-count');
|
||||
return data.count;
|
||||
},
|
||||
refetchInterval: () => (visibleRef.current ? 60_000 : false),
|
||||
staleTime: 30_000,
|
||||
refetchInterval: () => (visibleRef.current ? 30_000 : false),
|
||||
staleTime: 15_000,
|
||||
});
|
||||
|
||||
// When unread count increases, immediately refetch the notification list
|
||||
useEffect(() => {
|
||||
const count = unreadQuery.data;
|
||||
if (count === undefined) return;
|
||||
if (prevUnreadRef.current !== undefined && count > prevUnreadRef.current) {
|
||||
queryClient.invalidateQueries({ queryKey: ['notifications', 'list'] });
|
||||
}
|
||||
prevUnreadRef.current = count;
|
||||
}, [unreadQuery.data, queryClient]);
|
||||
|
||||
const listQuery = useQuery({
|
||||
queryKey: ['notifications', 'list'],
|
||||
queryFn: async () => {
|
||||
@ -34,7 +45,8 @@ export function useNotifications() {
|
||||
});
|
||||
return data;
|
||||
},
|
||||
staleTime: 30_000,
|
||||
staleTime: 15_000,
|
||||
refetchInterval: () => (visibleRef.current ? 60_000 : false),
|
||||
});
|
||||
|
||||
const markReadMutation = useMutation({
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user