Fix QA review findings: detach cleanup, sf() fallthrough, polling, commit guard

- W-01: Wrap accept flow db.commit() in IntegrityError handler (409)
- W-03: Remove refetchIntervalInBackground from unread count polling
- W-04: detach_umbral_contact now clears all shared fields on Person
- W-05: sf() callers no longer fall through via ?? to stale local data

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Kyle 2026-03-05 19:12:31 +08:00
parent 053c2ae85e
commit 4e2d48c50b
4 changed files with 23 additions and 15 deletions

View File

@ -522,7 +522,11 @@ async def _respond_to_request_inner(
# Extract ntfy config before commit (avoids detached SA object in background task) # Extract ntfy config before commit (avoids detached SA object in background task)
sender_ntfy = extract_ntfy_config(sender_settings) if sender_settings else None sender_ntfy = extract_ntfy_config(sender_settings) if sender_settings else None
await db.commit() try:
await db.commit()
except IntegrityError:
await db.rollback()
raise HTTPException(status_code=409, detail="Connection already exists")
# ntfy push in background # ntfy push in background
background_tasks.add_task( background_tasks.add_task(

View File

@ -129,11 +129,16 @@ async def detach_umbral_contact(person: Person) -> None:
"""Convert an umbral contact back to a standard contact. Does NOT commit.""" """Convert an umbral contact back to a standard contact. Does NOT commit."""
person.linked_user_id = None person.linked_user_id = None
person.is_umbral_contact = False person.is_umbral_contact = False
# Clear shared field values but preserve locally-entered data person.category = None
# If no first_name exists, fill from the old name # Clear all shareable fields — they were populated from the connection
if not person.first_name: # Preserve first_name from existing name so the contact is not left blank
person.first_name = person.name or None fallback_name = person.first_name or person.name or None
for field in SHAREABLE_FIELDS:
if hasattr(person, field):
setattr(person, field, None)
person.first_name = fallback_name
# Recompute display name
person.name = fallback_name or "Removed Contact"
def extract_ntfy_config(settings: Settings) -> dict | None: def extract_ntfy_config(settings: Settings) -> dict | None:
"""Extract ntfy config values into a plain dict safe for use after session close.""" """Extract ntfy config values into a plain dict safe for use after session close."""

View File

@ -100,8 +100,8 @@ const columns: ColumnDef<Person>[] = [
sortable: true, sortable: true,
visibilityLevel: 'essential', visibilityLevel: 'essential',
render: (p) => { render: (p) => {
const firstName = sf(p, 'first_name') ?? p.first_name; const firstName = sf(p, 'first_name');
const lastName = sf(p, 'last_name') ?? p.last_name; const lastName = sf(p, 'last_name');
const liveName = [firstName, lastName].filter(Boolean).join(' ') || p.nickname || p.name; const liveName = [firstName, lastName].filter(Boolean).join(' ') || p.nickname || p.name;
const initialsName = liveName || getPersonInitialsName(p); const initialsName = liveName || getPersonInitialsName(p);
return ( return (
@ -125,8 +125,8 @@ const columns: ColumnDef<Person>[] = [
sortable: false, sortable: false,
visibilityLevel: 'essential', visibilityLevel: 'essential',
render: (p) => { render: (p) => {
const mobile = sf(p, 'mobile') ?? p.mobile; const mobile = sf(p, 'mobile');
const phone = sf(p, 'phone') ?? p.phone; const phone = sf(p, 'phone');
return <span className="text-muted-foreground truncate">{mobile || phone || '—'}</span>; return <span className="text-muted-foreground truncate">{mobile || phone || '—'}</span>;
}, },
}, },
@ -136,7 +136,7 @@ const columns: ColumnDef<Person>[] = [
sortable: true, sortable: true,
visibilityLevel: 'essential', visibilityLevel: 'essential',
render: (p) => { render: (p) => {
const email = sf(p, 'email') ?? p.email; const email = sf(p, 'email');
return <span className="text-muted-foreground truncate">{email || '—'}</span>; return <span className="text-muted-foreground truncate">{email || '—'}</span>;
}, },
}, },
@ -146,8 +146,8 @@ const columns: ColumnDef<Person>[] = [
sortable: true, sortable: true,
visibilityLevel: 'filtered', visibilityLevel: 'filtered',
render: (p) => { render: (p) => {
const jobTitle = sf(p, 'job_title') ?? p.job_title; const jobTitle = sf(p, 'job_title');
const company = sf(p, 'company') ?? p.company; const company = sf(p, 'company');
const parts = [jobTitle, company].filter(Boolean); const parts = [jobTitle, company].filter(Boolean);
return <span className="text-muted-foreground truncate">{parts.join(', ') || '—'}</span>; return <span className="text-muted-foreground truncate">{parts.join(', ') || '—'}</span>;
}, },
@ -158,7 +158,7 @@ const columns: ColumnDef<Person>[] = [
sortable: true, sortable: true,
visibilityLevel: 'filtered', visibilityLevel: 'filtered',
render: (p) => { render: (p) => {
const birthday = sf(p, 'birthday') ?? p.birthday; const birthday = sf(p, 'birthday');
return birthday ? ( return birthday ? (
<span className="text-muted-foreground">{format(parseISO(birthday), 'MMM d')}</span> <span className="text-muted-foreground">{format(parseISO(birthday), 'MMM d')}</span>
) : ( ) : (

View File

@ -49,7 +49,6 @@ export function NotificationProvider({ children }: { children: ReactNode }) {
return data.count; return data.count;
}, },
refetchInterval: 15_000, refetchInterval: 15_000,
refetchIntervalInBackground: true,
staleTime: 10_000, staleTime: 10_000,
}); });