Fix QA issues: single AlertsProvider, null safety, snooze cleanup
- C1: Replaced duplicate useAlerts() calls with AlertsProvider context wrapping AppLayout — single hook instance, no double polling/toasts - C2: Added null guard on remind_at in Active Reminders card format() - W2: Clear snoozed_until when dismissing a reminder - W5: Extracted getRelativeTime to shared lib/date-utils.ts - S3: Replaced inline SVG with Lucide Bell component in toasts - S4: Clear snoozed_until when remind_at is updated via PUT Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
b2e336ab4a
commit
5b1b9cc5b7
@ -134,6 +134,10 @@ async def update_reminder(
|
|||||||
|
|
||||||
update_data = reminder_update.model_dump(exclude_unset=True)
|
update_data = reminder_update.model_dump(exclude_unset=True)
|
||||||
|
|
||||||
|
# Clear stale snooze if remind_at is being changed
|
||||||
|
if 'remind_at' in update_data:
|
||||||
|
reminder.snoozed_until = None
|
||||||
|
|
||||||
for key, value in update_data.items():
|
for key, value in update_data.items():
|
||||||
setattr(reminder, key, value)
|
setattr(reminder, key, value)
|
||||||
|
|
||||||
@ -176,6 +180,7 @@ async def dismiss_reminder(
|
|||||||
raise HTTPException(status_code=404, detail="Reminder not found")
|
raise HTTPException(status_code=404, detail="Reminder not found")
|
||||||
|
|
||||||
reminder.is_dismissed = True
|
reminder.is_dismissed = True
|
||||||
|
reminder.snoozed_until = None
|
||||||
|
|
||||||
await db.commit()
|
await db.commit()
|
||||||
await db.refresh(reminder)
|
await db.refresh(reminder)
|
||||||
|
|||||||
@ -1,4 +1,5 @@
|
|||||||
import { Bell, X } from 'lucide-react';
|
import { Bell, X } from 'lucide-react';
|
||||||
|
import { getRelativeTime } from '@/lib/date-utils';
|
||||||
import type { Reminder } from '@/types';
|
import type { Reminder } from '@/types';
|
||||||
|
|
||||||
interface AlertBannerProps {
|
interface AlertBannerProps {
|
||||||
@ -55,17 +56,3 @@ export default function AlertBanner({ alerts, onDismiss, onSnooze }: AlertBanner
|
|||||||
</div>
|
</div>
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
function getRelativeTime(dateStr: string): string {
|
|
||||||
const date = new Date(dateStr);
|
|
||||||
const now = new Date();
|
|
||||||
const diffMs = now.getTime() - date.getTime();
|
|
||||||
const diffMins = Math.floor(diffMs / 60000);
|
|
||||||
|
|
||||||
if (diffMins < 1) return 'Just now';
|
|
||||||
if (diffMins < 60) return `${diffMins}m ago`;
|
|
||||||
const diffHours = Math.floor(diffMins / 60);
|
|
||||||
if (diffHours < 24) return `${diffHours}h ago`;
|
|
||||||
const diffDays = Math.floor(diffHours / 24);
|
|
||||||
return `${diffDays}d ago`;
|
|
||||||
}
|
|
||||||
|
|||||||
@ -239,7 +239,7 @@ export default function DashboardPage() {
|
|||||||
<div className="w-1.5 h-1.5 rounded-full bg-orange-400 shrink-0" />
|
<div className="w-1.5 h-1.5 rounded-full bg-orange-400 shrink-0" />
|
||||||
<span className="font-medium text-sm truncate flex-1 min-w-0">{reminder.title}</span>
|
<span className="font-medium text-sm truncate flex-1 min-w-0">{reminder.title}</span>
|
||||||
<span className="text-xs text-muted-foreground shrink-0 whitespace-nowrap">
|
<span className="text-xs text-muted-foreground shrink-0 whitespace-nowrap">
|
||||||
{format(new Date(reminder.remind_at), 'MMM d, h:mm a')}
|
{reminder.remind_at ? format(new Date(reminder.remind_at), 'MMM d, h:mm a') : ''}
|
||||||
</span>
|
</span>
|
||||||
</div>
|
</div>
|
||||||
))}
|
))}
|
||||||
|
|||||||
@ -2,36 +2,37 @@ import { useState } from 'react';
|
|||||||
import { Outlet } from 'react-router-dom';
|
import { Outlet } from 'react-router-dom';
|
||||||
import { Menu } from 'lucide-react';
|
import { Menu } from 'lucide-react';
|
||||||
import { useTheme } from '@/hooks/useTheme';
|
import { useTheme } from '@/hooks/useTheme';
|
||||||
import { useAlerts } from '@/hooks/useAlerts';
|
import { AlertsProvider } from '@/hooks/useAlerts';
|
||||||
import { Button } from '@/components/ui/button';
|
import { Button } from '@/components/ui/button';
|
||||||
import Sidebar from './Sidebar';
|
import Sidebar from './Sidebar';
|
||||||
|
|
||||||
export default function AppLayout() {
|
export default function AppLayout() {
|
||||||
useTheme();
|
useTheme();
|
||||||
useAlerts();
|
|
||||||
const [collapsed, setCollapsed] = useState(false);
|
const [collapsed, setCollapsed] = useState(false);
|
||||||
const [mobileOpen, setMobileOpen] = useState(false);
|
const [mobileOpen, setMobileOpen] = useState(false);
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<div className="flex h-screen overflow-hidden bg-background">
|
<AlertsProvider>
|
||||||
<Sidebar
|
<div className="flex h-screen overflow-hidden bg-background">
|
||||||
collapsed={collapsed}
|
<Sidebar
|
||||||
onToggle={() => setCollapsed(!collapsed)}
|
collapsed={collapsed}
|
||||||
mobileOpen={mobileOpen}
|
onToggle={() => setCollapsed(!collapsed)}
|
||||||
onMobileClose={() => setMobileOpen(false)}
|
mobileOpen={mobileOpen}
|
||||||
/>
|
onMobileClose={() => setMobileOpen(false)}
|
||||||
<div className="flex-1 flex flex-col overflow-hidden">
|
/>
|
||||||
{/* Mobile header */}
|
<div className="flex-1 flex flex-col overflow-hidden">
|
||||||
<div className="flex md:hidden items-center h-14 border-b bg-card px-4">
|
{/* Mobile header */}
|
||||||
<Button variant="ghost" size="icon" onClick={() => setMobileOpen(true)}>
|
<div className="flex md:hidden items-center h-14 border-b bg-card px-4">
|
||||||
<Menu className="h-5 w-5" />
|
<Button variant="ghost" size="icon" onClick={() => setMobileOpen(true)}>
|
||||||
</Button>
|
<Menu className="h-5 w-5" />
|
||||||
<h1 className="text-lg font-bold text-accent ml-3">UMBRA</h1>
|
</Button>
|
||||||
|
<h1 className="text-lg font-bold text-accent ml-3">UMBRA</h1>
|
||||||
|
</div>
|
||||||
|
<main className="flex-1 overflow-y-auto">
|
||||||
|
<Outlet />
|
||||||
|
</main>
|
||||||
</div>
|
</div>
|
||||||
<main className="flex-1 overflow-y-auto">
|
|
||||||
<Outlet />
|
|
||||||
</main>
|
|
||||||
</div>
|
</div>
|
||||||
</div>
|
</AlertsProvider>
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|||||||
@ -1,13 +1,31 @@
|
|||||||
import { useRef, useEffect, useCallback } from 'react';
|
import { createContext, useContext, useRef, useEffect, useCallback, type ReactNode } from 'react';
|
||||||
import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query';
|
import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query';
|
||||||
import { useLocation } from 'react-router-dom';
|
import { useLocation } from 'react-router-dom';
|
||||||
import { toast } from 'sonner';
|
import { toast } from 'sonner';
|
||||||
|
import { Bell } from 'lucide-react';
|
||||||
import api from '@/lib/api';
|
import api from '@/lib/api';
|
||||||
|
import { getRelativeTime } from '@/lib/date-utils';
|
||||||
import type { Reminder } from '@/types';
|
import type { Reminder } from '@/types';
|
||||||
|
|
||||||
const MAX_TOASTS = 3;
|
const MAX_TOASTS = 3;
|
||||||
|
|
||||||
|
interface AlertsContextValue {
|
||||||
|
alerts: Reminder[];
|
||||||
|
dismiss: (id: number) => void;
|
||||||
|
snooze: (id: number, minutes: 5 | 10 | 15) => void;
|
||||||
|
}
|
||||||
|
|
||||||
|
const AlertsContext = createContext<AlertsContextValue>({
|
||||||
|
alerts: [],
|
||||||
|
dismiss: () => {},
|
||||||
|
snooze: () => {},
|
||||||
|
});
|
||||||
|
|
||||||
export function useAlerts() {
|
export function useAlerts() {
|
||||||
|
return useContext(AlertsContext);
|
||||||
|
}
|
||||||
|
|
||||||
|
export function AlertsProvider({ children }: { children: ReactNode }) {
|
||||||
const queryClient = useQueryClient();
|
const queryClient = useQueryClient();
|
||||||
const location = useLocation();
|
const location = useLocation();
|
||||||
const firedRef = useRef<Set<number>>(new Set());
|
const firedRef = useRef<Set<number>>(new Set());
|
||||||
@ -37,29 +55,89 @@ export function useAlerts() {
|
|||||||
});
|
});
|
||||||
}, [alerts]);
|
}, [alerts]);
|
||||||
|
|
||||||
// Handle route changes
|
const dismissMutation = useMutation({
|
||||||
useEffect(() => {
|
mutationFn: (id: number) => api.patch(`/reminders/${id}/dismiss`),
|
||||||
const wasOnDashboard = prevPathnameRef.current === '/' || prevPathnameRef.current === '/dashboard';
|
onSuccess: () => {
|
||||||
const nowOnDashboard = isDashboard;
|
queryClient.invalidateQueries({ queryKey: ['reminders'] });
|
||||||
prevPathnameRef.current = location.pathname;
|
queryClient.invalidateQueries({ queryKey: ['dashboard'] });
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
if (nowOnDashboard) {
|
const snoozeMutation = useMutation({
|
||||||
// Moving TO dashboard — dismiss all toasts, banner takes over
|
mutationFn: ({ id, minutes }: { id: number; minutes: 5 | 10 | 15 }) =>
|
||||||
alerts.forEach((a) => toast.dismiss(`reminder-${a.id}`));
|
api.patch(`/reminders/${id}/snooze`, { minutes }),
|
||||||
toast.dismiss('reminder-summary');
|
onSuccess: () => {
|
||||||
firedRef.current.clear();
|
queryClient.invalidateQueries({ queryKey: ['reminders'] });
|
||||||
} else if (wasOnDashboard && !nowOnDashboard) {
|
queryClient.invalidateQueries({ queryKey: ['dashboard'] });
|
||||||
// Moving AWAY from dashboard — fire toasts for current alerts
|
},
|
||||||
firedRef.current.clear();
|
});
|
||||||
fireToasts(alerts);
|
|
||||||
}
|
|
||||||
}, [location.pathname]); // eslint-disable-line react-hooks/exhaustive-deps
|
|
||||||
|
|
||||||
// Fire toasts for new alerts on non-dashboard pages
|
const handleDismiss = useCallback((id: number) => {
|
||||||
useEffect(() => {
|
toast.dismiss(`reminder-${id}`);
|
||||||
if (isDashboard) return;
|
toast.dismiss('reminder-summary');
|
||||||
fireToasts(alerts);
|
firedRef.current.delete(id);
|
||||||
}, [alerts, isDashboard]); // eslint-disable-line react-hooks/exhaustive-deps
|
dismissMutation.mutate(id);
|
||||||
|
}, [dismissMutation]);
|
||||||
|
|
||||||
|
const handleSnooze = useCallback((id: number, minutes: 5 | 10 | 15) => {
|
||||||
|
toast.dismiss(`reminder-${id}`);
|
||||||
|
toast.dismiss('reminder-summary');
|
||||||
|
firedRef.current.delete(id);
|
||||||
|
snoozeMutation.mutate({ id, minutes });
|
||||||
|
}, [snoozeMutation]);
|
||||||
|
|
||||||
|
// Store latest callbacks in refs so toast closures always call current versions
|
||||||
|
const dismissRef = useRef(handleDismiss);
|
||||||
|
const snoozeRef = useRef(handleSnooze);
|
||||||
|
useEffect(() => { dismissRef.current = handleDismiss; }, [handleDismiss]);
|
||||||
|
useEffect(() => { snoozeRef.current = handleSnooze; }, [handleSnooze]);
|
||||||
|
|
||||||
|
function renderToast(_t: string | number, reminder: Reminder) {
|
||||||
|
const timeAgo = reminder.remind_at ? getRelativeTime(reminder.remind_at) : '';
|
||||||
|
return (
|
||||||
|
<div className="flex items-start gap-3 bg-card border border-border rounded-lg p-3 shadow-lg w-[356px]">
|
||||||
|
<div className="p-1.5 rounded-md bg-orange-500/10 shrink-0 mt-0.5">
|
||||||
|
<Bell className="h-4 w-4 text-orange-400" />
|
||||||
|
</div>
|
||||||
|
<div className="flex-1 min-w-0">
|
||||||
|
<p className="text-sm font-medium text-foreground truncate">{reminder.title}</p>
|
||||||
|
<p className="text-xs text-muted-foreground mt-0.5">{timeAgo}</p>
|
||||||
|
<div className="flex items-center gap-1.5 mt-2">
|
||||||
|
<div className="flex items-center gap-0.5 text-[11px]">
|
||||||
|
{[5, 10, 15].map((m) => (
|
||||||
|
<button
|
||||||
|
key={m}
|
||||||
|
onClick={() => snoozeRef.current(reminder.id, m as 5 | 10 | 15)}
|
||||||
|
className="px-1.5 py-0.5 rounded bg-secondary hover:bg-card-elevated text-muted-foreground hover:text-foreground transition-colors"
|
||||||
|
>
|
||||||
|
{m}m
|
||||||
|
</button>
|
||||||
|
))}
|
||||||
|
</div>
|
||||||
|
<button
|
||||||
|
onClick={() => dismissRef.current(reminder.id)}
|
||||||
|
className="ml-auto px-2 py-0.5 rounded text-[11px] bg-secondary hover:bg-card-elevated text-muted-foreground hover:text-foreground transition-colors"
|
||||||
|
>
|
||||||
|
Dismiss
|
||||||
|
</button>
|
||||||
|
</div>
|
||||||
|
</div>
|
||||||
|
</div>
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
function renderSummaryToast(_t: string | number, count: number) {
|
||||||
|
return (
|
||||||
|
<div className="flex items-center gap-3 bg-card border border-border rounded-lg p-3 shadow-lg w-[356px]">
|
||||||
|
<div className="p-1.5 rounded-md bg-orange-500/10 shrink-0">
|
||||||
|
<Bell className="h-4 w-4 text-orange-400" />
|
||||||
|
</div>
|
||||||
|
<p className="text-sm text-muted-foreground">
|
||||||
|
+{count} more reminder{count > 1 ? 's' : ''} due
|
||||||
|
</p>
|
||||||
|
</div>
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
function fireToasts(reminders: Reminder[]) {
|
function fireToasts(reminders: Reminder[]) {
|
||||||
const newAlerts = reminders.filter((a) => !firedRef.current.has(a.id));
|
const newAlerts = reminders.filter((a) => !firedRef.current.has(a.id));
|
||||||
@ -87,102 +165,33 @@ export function useAlerts() {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
function renderToast(_t: string | number, reminder: Reminder) {
|
// Handle route changes
|
||||||
const timeAgo = reminder.remind_at ? getRelativeTime(reminder.remind_at) : '';
|
useEffect(() => {
|
||||||
return (
|
const wasOnDashboard = prevPathnameRef.current === '/' || prevPathnameRef.current === '/dashboard';
|
||||||
<div className="flex items-start gap-3 bg-card border border-border rounded-lg p-3 shadow-lg w-[356px]">
|
const nowOnDashboard = isDashboard;
|
||||||
<div className="p-1.5 rounded-md bg-orange-500/10 shrink-0 mt-0.5">
|
prevPathnameRef.current = location.pathname;
|
||||||
<svg className="h-4 w-4 text-orange-400" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" fill="none" stroke="currentColor" strokeWidth="2" strokeLinecap="round" strokeLinejoin="round">
|
|
||||||
<path d="M6 8a6 6 0 0 1 12 0c0 7 3 9 3 9H3s3-2 3-9" /><path d="M10.3 21a1.94 1.94 0 0 0 3.4 0" />
|
|
||||||
</svg>
|
|
||||||
</div>
|
|
||||||
<div className="flex-1 min-w-0">
|
|
||||||
<p className="text-sm font-medium text-foreground truncate">{reminder.title}</p>
|
|
||||||
<p className="text-xs text-muted-foreground mt-0.5">{timeAgo}</p>
|
|
||||||
<div className="flex items-center gap-1.5 mt-2">
|
|
||||||
<div className="flex items-center gap-0.5 text-[11px]">
|
|
||||||
{[5, 10, 15].map((m) => (
|
|
||||||
<button
|
|
||||||
key={m}
|
|
||||||
onClick={() => handleSnooze(reminder.id, m as 5 | 10 | 15)}
|
|
||||||
className="px-1.5 py-0.5 rounded bg-secondary hover:bg-card-elevated text-muted-foreground hover:text-foreground transition-colors"
|
|
||||||
>
|
|
||||||
{m}m
|
|
||||||
</button>
|
|
||||||
))}
|
|
||||||
</div>
|
|
||||||
<button
|
|
||||||
onClick={() => handleDismiss(reminder.id)}
|
|
||||||
className="ml-auto px-2 py-0.5 rounded text-[11px] bg-secondary hover:bg-card-elevated text-muted-foreground hover:text-foreground transition-colors"
|
|
||||||
>
|
|
||||||
Dismiss
|
|
||||||
</button>
|
|
||||||
</div>
|
|
||||||
</div>
|
|
||||||
</div>
|
|
||||||
);
|
|
||||||
}
|
|
||||||
|
|
||||||
function renderSummaryToast(_t: string | number, count: number) {
|
if (nowOnDashboard) {
|
||||||
return (
|
// Moving TO dashboard — dismiss all toasts, banner takes over
|
||||||
<div className="flex items-center gap-3 bg-card border border-border rounded-lg p-3 shadow-lg w-[356px]">
|
alerts.forEach((a) => toast.dismiss(`reminder-${a.id}`));
|
||||||
<div className="p-1.5 rounded-md bg-orange-500/10 shrink-0">
|
toast.dismiss('reminder-summary');
|
||||||
<svg className="h-4 w-4 text-orange-400" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" fill="none" stroke="currentColor" strokeWidth="2" strokeLinecap="round" strokeLinejoin="round">
|
firedRef.current.clear();
|
||||||
<path d="M6 8a6 6 0 0 1 12 0c0 7 3 9 3 9H3s3-2 3-9" /><path d="M10.3 21a1.94 1.94 0 0 0 3.4 0" />
|
} else if (wasOnDashboard && !nowOnDashboard) {
|
||||||
</svg>
|
// Moving AWAY from dashboard — fire toasts for current alerts
|
||||||
</div>
|
firedRef.current.clear();
|
||||||
<p className="text-sm text-muted-foreground">
|
fireToasts(alerts);
|
||||||
+{count} more reminder{count > 1 ? 's' : ''} due
|
}
|
||||||
</p>
|
}, [location.pathname, isDashboard, alerts]);
|
||||||
</div>
|
|
||||||
);
|
|
||||||
}
|
|
||||||
|
|
||||||
const dismissMutation = useMutation({
|
// Fire toasts for new alerts on non-dashboard pages
|
||||||
mutationFn: (id: number) => api.patch(`/reminders/${id}/dismiss`),
|
useEffect(() => {
|
||||||
onSuccess: () => {
|
if (isDashboard) return;
|
||||||
queryClient.invalidateQueries({ queryKey: ['reminders'] });
|
fireToasts(alerts);
|
||||||
queryClient.invalidateQueries({ queryKey: ['dashboard'] });
|
}, [alerts, isDashboard]);
|
||||||
},
|
|
||||||
});
|
|
||||||
|
|
||||||
const snoozeMutation = useMutation({
|
return (
|
||||||
mutationFn: ({ id, minutes }: { id: number; minutes: 5 | 10 | 15 }) =>
|
<AlertsContext.Provider value={{ alerts, dismiss: handleDismiss, snooze: handleSnooze }}>
|
||||||
api.patch(`/reminders/${id}/snooze`, { minutes }),
|
{children}
|
||||||
onSuccess: () => {
|
</AlertsContext.Provider>
|
||||||
queryClient.invalidateQueries({ queryKey: ['reminders'] });
|
);
|
||||||
queryClient.invalidateQueries({ queryKey: ['dashboard'] });
|
|
||||||
},
|
|
||||||
});
|
|
||||||
|
|
||||||
const handleDismiss = useCallback((id: number) => {
|
|
||||||
toast.dismiss(`reminder-${id}`);
|
|
||||||
firedRef.current.delete(id);
|
|
||||||
dismissMutation.mutate(id);
|
|
||||||
// If summary toast exists and we're reducing count, dismiss it too — next poll will re-evaluate
|
|
||||||
toast.dismiss('reminder-summary');
|
|
||||||
}, [dismissMutation]);
|
|
||||||
|
|
||||||
const handleSnooze = useCallback((id: number, minutes: 5 | 10 | 15) => {
|
|
||||||
toast.dismiss(`reminder-${id}`);
|
|
||||||
firedRef.current.delete(id);
|
|
||||||
snoozeMutation.mutate({ id, minutes });
|
|
||||||
toast.dismiss('reminder-summary');
|
|
||||||
}, [snoozeMutation]);
|
|
||||||
|
|
||||||
return { alerts, dismiss: handleDismiss, snooze: handleSnooze };
|
|
||||||
}
|
|
||||||
|
|
||||||
function getRelativeTime(dateStr: string): string {
|
|
||||||
const date = new Date(dateStr);
|
|
||||||
const now = new Date();
|
|
||||||
const diffMs = now.getTime() - date.getTime();
|
|
||||||
const diffMins = Math.floor(diffMs / 60000);
|
|
||||||
|
|
||||||
if (diffMins < 1) return 'Just now';
|
|
||||||
if (diffMins < 60) return `${diffMins}m ago`;
|
|
||||||
const diffHours = Math.floor(diffMins / 60);
|
|
||||||
if (diffHours < 24) return `${diffHours}h ago`;
|
|
||||||
const diffDays = Math.floor(diffHours / 24);
|
|
||||||
return `${diffDays}d ago`;
|
|
||||||
}
|
}
|
||||||
|
|||||||
13
frontend/src/lib/date-utils.ts
Normal file
13
frontend/src/lib/date-utils.ts
Normal file
@ -0,0 +1,13 @@
|
|||||||
|
export function getRelativeTime(dateStr: string): string {
|
||||||
|
const date = new Date(dateStr);
|
||||||
|
const now = new Date();
|
||||||
|
const diffMs = now.getTime() - date.getTime();
|
||||||
|
const diffMins = Math.floor(diffMs / 60000);
|
||||||
|
|
||||||
|
if (diffMins < 1) return 'Just now';
|
||||||
|
if (diffMins < 60) return `${diffMins}m ago`;
|
||||||
|
const diffHours = Math.floor(diffMins / 60);
|
||||||
|
if (diffHours < 24) return `${diffHours}h ago`;
|
||||||
|
const diffDays = Math.floor(diffHours / 24);
|
||||||
|
return `${diffDays}d ago`;
|
||||||
|
}
|
||||||
Loading…
x
Reference in New Issue
Block a user