Fix bugs and action remaining QA suggestions

Bugs fixed:
- formatUpdatedAt treats naive UTC timestamps as UTC (append Z before parsing)
- PersonForm/LocationForm X button now inline with star toggle, matching panel style
- LocationForm contact placeholder changed from +44 to +61

QA suggestions actioned:
- CategoryAutocomplete: replace blur setTimeout with onPointerDown preventDefault
- CategoryFilterBar: replace hardcoded 600px maxWidth with 100vw
- Location "other" category shows dash instead of styled badge
- Delete dead legacy constants files (people/constants.ts, locations/constants.ts)
- EntityTable rows: add tabIndex, Enter/Space keyboard navigation, focus ring
- Replace Record<string, unknown> casts with typed keyof accessors
- Add email validation (field_validator) to Person and Location schemas

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Kyle 2026-02-24 21:46:38 +08:00
parent 8cbc95939a
commit 1b78dadf75
12 changed files with 134 additions and 94 deletions

View File

@ -1,7 +1,10 @@
from pydantic import BaseModel, ConfigDict import re
from pydantic import BaseModel, ConfigDict, field_validator
from datetime import datetime from datetime import datetime
from typing import Optional, Literal from typing import Optional, Literal
_EMAIL_RE = re.compile(r'^[^@\s]+@[^@\s]+\.[^@\s]+$')
class LocationSearchResult(BaseModel): class LocationSearchResult(BaseModel):
source: Literal["local", "nominatim"] source: Literal["local", "nominatim"]
@ -19,6 +22,13 @@ class LocationCreate(BaseModel):
contact_number: Optional[str] = None contact_number: Optional[str] = None
email: Optional[str] = None email: Optional[str] = None
@field_validator('email')
@classmethod
def validate_email(cls, v: str | None) -> str | None:
if v and not _EMAIL_RE.match(v):
raise ValueError('Invalid email address')
return v
class LocationUpdate(BaseModel): class LocationUpdate(BaseModel):
name: Optional[str] = None name: Optional[str] = None
@ -29,6 +39,13 @@ class LocationUpdate(BaseModel):
contact_number: Optional[str] = None contact_number: Optional[str] = None
email: Optional[str] = None email: Optional[str] = None
@field_validator('email')
@classmethod
def validate_email(cls, v: str | None) -> str | None:
if v and not _EMAIL_RE.match(v):
raise ValueError('Invalid email address')
return v
class LocationResponse(BaseModel): class LocationResponse(BaseModel):
id: int id: int

View File

@ -1,7 +1,10 @@
from pydantic import BaseModel, ConfigDict, model_validator import re
from pydantic import BaseModel, ConfigDict, model_validator, field_validator
from datetime import datetime, date from datetime import datetime, date
from typing import Optional from typing import Optional
_EMAIL_RE = re.compile(r'^[^@\s]+@[^@\s]+\.[^@\s]+$')
class PersonCreate(BaseModel): class PersonCreate(BaseModel):
name: Optional[str] = None # legacy fallback — auto-split into first/last if provided alone name: Optional[str] = None # legacy fallback — auto-split into first/last if provided alone
@ -25,6 +28,13 @@ class PersonCreate(BaseModel):
raise ValueError('At least one name field is required') raise ValueError('At least one name field is required')
return self return self
@field_validator('email')
@classmethod
def validate_email(cls, v: str | None) -> str | None:
if v and not _EMAIL_RE.match(v):
raise ValueError('Invalid email address')
return v
class PersonUpdate(BaseModel): class PersonUpdate(BaseModel):
# name is intentionally omitted — always computed from first/last/nickname # name is intentionally omitted — always computed from first/last/nickname
@ -42,6 +52,13 @@ class PersonUpdate(BaseModel):
job_title: Optional[str] = None job_title: Optional[str] = None
notes: Optional[str] = None notes: Optional[str] = None
@field_validator('email')
@classmethod
def validate_email(cls, v: str | None) -> str | None:
if v and not _EMAIL_RE.match(v):
raise ValueError('Invalid email address')
return v
class PersonResponse(BaseModel): class PersonResponse(BaseModel):
id: int id: int

View File

@ -1,7 +1,7 @@
import { useState, FormEvent } from 'react'; import { useState, FormEvent } from 'react';
import { useMutation, useQueryClient } from '@tanstack/react-query'; import { useMutation, useQueryClient } from '@tanstack/react-query';
import { toast } from 'sonner'; import { toast } from 'sonner';
import { Star, StarOff } from 'lucide-react'; import { Star, StarOff, X } from 'lucide-react';
import api, { getErrorMessage } from '@/lib/api'; import api, { getErrorMessage } from '@/lib/api';
import type { Location } from '@/types'; import type { Location } from '@/types';
import { import {
@ -10,7 +10,6 @@ import {
SheetHeader, SheetHeader,
SheetTitle, SheetTitle,
SheetFooter, SheetFooter,
SheetClose,
} from '@/components/ui/sheet'; } from '@/components/ui/sheet';
import { Input } from '@/components/ui/input'; import { Input } from '@/components/ui/input';
import { Textarea } from '@/components/ui/textarea'; import { Textarea } from '@/components/ui/textarea';
@ -73,15 +72,15 @@ export default function LocationForm({ location, categories, onClose }: Location
return ( return (
<Sheet open={true} onOpenChange={onClose}> <Sheet open={true} onOpenChange={onClose}>
<SheetContent> <SheetContent>
<SheetClose onClick={onClose} />
<SheetHeader> <SheetHeader>
<div className="flex items-center justify-between pr-8"> <div className="flex items-center justify-between">
<SheetTitle>{location ? 'Edit Location' : 'New Location'}</SheetTitle> <SheetTitle>{location ? 'Edit Location' : 'New Location'}</SheetTitle>
<div className="flex items-center gap-1">
<Button <Button
type="button" type="button"
variant="ghost" variant="ghost"
size="icon" size="icon"
className="h-8 w-8" className="h-7 w-7"
aria-label={formData.is_frequent ? 'Remove from frequent' : 'Mark as frequent'} aria-label={formData.is_frequent ? 'Remove from frequent' : 'Mark as frequent'}
onClick={() => onClick={() =>
setFormData((prev) => ({ ...prev, is_frequent: !prev.is_frequent })) setFormData((prev) => ({ ...prev, is_frequent: !prev.is_frequent }))
@ -93,6 +92,17 @@ export default function LocationForm({ location, categories, onClose }: Location
<StarOff className="h-4 w-4" /> <StarOff className="h-4 w-4" />
)} )}
</Button> </Button>
<Button
type="button"
variant="ghost"
size="icon"
onClick={onClose}
aria-label="Close"
className="h-7 w-7 shrink-0"
>
<X className="h-4 w-4" />
</Button>
</div>
</div> </div>
</SheetHeader> </SheetHeader>
@ -139,7 +149,7 @@ export default function LocationForm({ location, categories, onClose }: Location
onChange={(e) => onChange={(e) =>
setFormData({ ...formData, contact_number: e.target.value }) setFormData({ ...formData, contact_number: e.target.value })
} }
placeholder="+44..." placeholder="+61..."
/> />
</div> </div>
<div className="space-y-2"> <div className="space-y-2">

View File

@ -63,8 +63,8 @@ export default function LocationsPage() {
const sortedLocations = useMemo(() => { const sortedLocations = useMemo(() => {
return [...locations].sort((a, b) => { return [...locations].sort((a, b) => {
const aVal = String((a as unknown as Record<string, unknown>)[sortKey] ?? ''); const aVal = String(a[sortKey as keyof Location] ?? '');
const bVal = String((b as unknown as Record<string, unknown>)[sortKey] ?? ''); const bVal = String(b[sortKey as keyof Location] ?? '');
const cmp = aVal.localeCompare(bVal); const cmp = aVal.localeCompare(bVal);
return sortDir === 'asc' ? cmp : -cmp; return sortDir === 'asc' ? cmp : -cmp;
}); });
@ -187,7 +187,8 @@ export default function LocationsPage() {
label: 'Category', label: 'Category',
sortable: true, sortable: true,
visibilityLevel: 'all', visibilityLevel: 'all',
render: (l) => ( render: (l) =>
l.category && l.category.toLowerCase() !== 'other' ? (
<span <span
className="px-2 py-0.5 text-xs rounded" className="px-2 py-0.5 text-xs rounded"
style={{ style={{
@ -197,6 +198,8 @@ export default function LocationsPage() {
> >
{l.category} {l.category}
</span> </span>
) : (
<span className="text-muted-foreground"></span>
), ),
}, },
]; ];
@ -233,7 +236,7 @@ export default function LocationsPage() {
)} )}
getUpdatedAt={(l) => l.updated_at} getUpdatedAt={(l) => l.updated_at}
getValue={(l, key) => getValue={(l, key) =>
(l as unknown as Record<string, string | undefined>)[key] ?? undefined (l[key as keyof Location] as string | undefined) ?? undefined
} }
/> />
); );

View File

@ -1,13 +0,0 @@
const FALLBACK = 'bg-gray-500/10 text-gray-400 border-gray-500/20';
export function getCategoryColor(category: string | undefined): string {
if (!category) return FALLBACK;
const colors: Record<string, string> = {
home: 'bg-blue-500/10 text-blue-400 border-blue-500/20',
work: 'bg-purple-500/10 text-purple-400 border-purple-500/20',
restaurant: 'bg-orange-500/10 text-orange-400 border-orange-500/20',
shop: 'bg-green-500/10 text-green-400 border-green-500/20',
other: FALLBACK,
};
return colors[category] ?? FALLBACK;
}

View File

@ -63,8 +63,8 @@ function sortPeople(people: Person[], key: string, dir: 'asc' | 'desc'): Person[
const bD = b.birthday ? getDaysUntilBirthday(b.birthday) : Infinity; const bD = b.birthday ? getDaysUntilBirthday(b.birthday) : Infinity;
cmp = aD - bD; cmp = aD - bD;
} else { } else {
const aVal = (a as unknown as Record<string, unknown>)[key]; const aVal = a[key as keyof Person];
const bVal = (b as unknown as Record<string, unknown>)[key]; const bVal = b[key as keyof Person];
const aStr = aVal != null ? String(aVal) : ''; const aStr = aVal != null ? String(aVal) : '';
const bStr = bVal != null ? String(bVal) : ''; const bStr = bVal != null ? String(bVal) : '';
cmp = aStr.localeCompare(bStr); cmp = aStr.localeCompare(bStr);
@ -323,7 +323,7 @@ export default function PeoplePage() {
const age = differenceInYears(new Date(), parseISO(p.birthday)); const age = differenceInYears(new Date(), parseISO(p.birthday));
return `${format(parseISO(p.birthday), 'MMM d, yyyy')} (${age})`; return `${format(parseISO(p.birthday), 'MMM d, yyyy')} (${age})`;
} }
const val = (p as unknown as Record<string, unknown>)[key]; const val = p[key as keyof Person];
return val != null ? String(val) : undefined; return val != null ? String(val) : undefined;
}; };

View File

@ -1,7 +1,7 @@
import { useState, useMemo, FormEvent } from 'react'; import { useState, useMemo, FormEvent } from 'react';
import { useMutation, useQueryClient } from '@tanstack/react-query'; import { useMutation, useQueryClient } from '@tanstack/react-query';
import { toast } from 'sonner'; import { toast } from 'sonner';
import { Star, StarOff } from 'lucide-react'; import { Star, StarOff, X } from 'lucide-react';
import { parseISO, differenceInYears } from 'date-fns'; import { parseISO, differenceInYears } from 'date-fns';
import api, { getErrorMessage } from '@/lib/api'; import api, { getErrorMessage } from '@/lib/api';
import type { Person } from '@/types'; import type { Person } from '@/types';
@ -11,7 +11,6 @@ import {
SheetHeader, SheetHeader,
SheetTitle, SheetTitle,
SheetFooter, SheetFooter,
SheetClose,
} from '@/components/ui/sheet'; } from '@/components/ui/sheet';
import { Input } from '@/components/ui/input'; import { Input } from '@/components/ui/input';
import { Textarea } from '@/components/ui/textarea'; import { Textarea } from '@/components/ui/textarea';
@ -96,15 +95,15 @@ export default function PersonForm({ person, categories, onClose }: PersonFormPr
return ( return (
<Sheet open={true} onOpenChange={onClose}> <Sheet open={true} onOpenChange={onClose}>
<SheetContent> <SheetContent>
<SheetClose onClick={onClose} />
<SheetHeader> <SheetHeader>
<div className="flex items-center justify-between"> <div className="flex items-center justify-between">
<SheetTitle>{person ? 'Edit Person' : 'New Person'}</SheetTitle> <SheetTitle>{person ? 'Edit Person' : 'New Person'}</SheetTitle>
<div className="flex items-center gap-1">
<Button <Button
type="button" type="button"
variant="ghost" variant="ghost"
size="icon" size="icon"
className={`h-8 w-8 ${formData.is_favourite ? 'text-yellow-400' : 'text-muted-foreground'}`} className={`h-7 w-7 ${formData.is_favourite ? 'text-yellow-400' : 'text-muted-foreground'}`}
onClick={() => set('is_favourite', !formData.is_favourite)} onClick={() => set('is_favourite', !formData.is_favourite)}
aria-label={formData.is_favourite ? 'Remove from favourites' : 'Add to favourites'} aria-label={formData.is_favourite ? 'Remove from favourites' : 'Add to favourites'}
> >
@ -114,6 +113,17 @@ export default function PersonForm({ person, categories, onClose }: PersonFormPr
<StarOff className="h-4 w-4" /> <StarOff className="h-4 w-4" />
)} )}
</Button> </Button>
<Button
type="button"
variant="ghost"
size="icon"
onClick={onClose}
aria-label="Close"
className="h-7 w-7 shrink-0"
>
<X className="h-4 w-4" />
</Button>
</div>
</div> </div>
</SheetHeader> </SheetHeader>

View File

@ -1,14 +0,0 @@
// Legacy — kept for backward compatibility during transition
const FALLBACK = 'bg-gray-500/10 text-gray-400 border-gray-500/20';
export function getRelationshipColor(relationship: string | undefined): string {
if (!relationship) return FALLBACK;
const colors: Record<string, string> = {
Family: 'bg-rose-500/10 text-rose-400 border-rose-500/20',
Friend: 'bg-blue-500/10 text-blue-400 border-blue-500/20',
Colleague: 'bg-purple-500/10 text-purple-400 border-purple-500/20',
Partner: 'bg-pink-500/10 text-pink-400 border-pink-500/20',
Other: FALLBACK,
};
return colors[relationship] ?? FALLBACK;
}

View File

@ -35,12 +35,9 @@ export default function CategoryAutocomplete({
}, []); }, []);
const handleBlur = () => { const handleBlur = () => {
// Normalise casing if input matches an existing category
setTimeout(() => {
const match = categories.find((c) => c.toLowerCase() === value.toLowerCase()); const match = categories.find((c) => c.toLowerCase() === value.toLowerCase());
if (match && match !== value) onChange(match); if (match && match !== value) onChange(match);
setOpen(false); setOpen(false);
}, 150);
}; };
const handleSelect = (cat: string) => { const handleSelect = (cat: string) => {
@ -74,7 +71,10 @@ export default function CategoryAutocomplete({
key={cat} key={cat}
role="option" role="option"
aria-selected={cat === value} aria-selected={cat === value}
onMouseDown={() => handleSelect(cat)} onPointerDown={(e) => {
e.preventDefault();
handleSelect(cat);
}}
className="px-3 py-1.5 text-sm hover:bg-card-elevated cursor-pointer transition-colors duration-150" className="px-3 py-1.5 text-sm hover:bg-card-elevated cursor-pointer transition-colors duration-150"
> >
{cat} {cat}

View File

@ -99,7 +99,7 @@ export default function CategoryFilterBar({
<div <div
className="flex items-center gap-1.5 overflow-x-auto transition-all duration-200 ease-out" className="flex items-center gap-1.5 overflow-x-auto transition-all duration-200 ease-out"
style={{ style={{
maxWidth: otherOpen ? '600px' : '0px', maxWidth: otherOpen ? '100vw' : '0px',
opacity: otherOpen ? 1 : 0, opacity: otherOpen ? 1 : 0,
overflow: 'hidden', overflow: 'hidden',
}} }}

View File

@ -71,10 +71,18 @@ export function EntityTable<T extends { id: number }>({
const DataRow = ({ item }: { item: T }) => ( const DataRow = ({ item }: { item: T }) => (
<tr <tr
className={`border-b border-border/50 cursor-pointer hover:bg-card-elevated transition-colors duration-150 ${ className={`border-b border-border/50 cursor-pointer hover:bg-card-elevated transition-colors duration-150 outline-none focus-visible:ring-1 focus-visible:ring-ring ${
selectedId === item.id ? 'bg-accent/10' : '' selectedId === item.id ? 'bg-accent/10' : ''
}`} }`}
onClick={() => onRowClick(item.id)} onClick={() => onRowClick(item.id)}
onKeyDown={(e) => {
if (e.key === 'Enter' || e.key === ' ') {
e.preventDefault();
onRowClick(item.id);
}
}}
tabIndex={0}
role="row"
aria-selected={selectedId === item.id} aria-selected={selectedId === item.id}
> >
{visibleColumns.map((col) => ( {visibleColumns.map((col) => (

View File

@ -29,7 +29,9 @@ export function getAvatarColor(name: string): string {
export function formatUpdatedAt(updatedAt: string): string { export function formatUpdatedAt(updatedAt: string): string {
try { try {
return `Updated ${formatDistanceToNow(parseISO(updatedAt), { addSuffix: true })}`; // Backend stores naive UTC timestamps — append Z so date-fns treats as UTC
const utcString = updatedAt.endsWith('Z') ? updatedAt : updatedAt + 'Z';
return `Updated ${formatDistanceToNow(parseISO(utcString), { addSuffix: true })}`;
} catch { } catch {
return ''; return '';
} }