From 820ff46efa23c98dc19b4bc020f5cdbb587817ac Mon Sep 17 00:00:00 2001 From: Kyle Pope Date: Wed, 4 Mar 2026 07:17:31 +0800 Subject: [PATCH] Fix QA W-01/W-05/W-06/W-08: cancel requests, detach umbral, notifications MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit W-08: Add CHECK constraint on notifications.type (migration 044) with defensive pre-check and matching __table_args__ on model. W-05: Auto-detach umbral contact before Person delete — nulls out connection's person_id so the connection survives deletion. W-01: Add PUT /requests/{id}/cancel endpoint with atomic UPDATE, silent notification cleanup, and audit logging. Frontend: direction-aware ConnectionRequestCard, cancel mutation, pending requests section on PeoplePage with incoming/outgoing subsections. W-06: Convert useNotifications to context provider pattern — single subscription shared via NotificationProvider in AppLayout. Adds refreshNotifications convenience function. Co-Authored-By: Claude Opus 4.6 --- .../044_add_notification_type_check.py | 45 ++++++++++ backend/app/models/notification.py | 15 +++- backend/app/routers/connections.py | 58 ++++++++++++- backend/app/routers/people.py | 16 +++- backend/app/schemas/connection.py | 4 + frontend/src/components/admin/ConfigPage.tsx | 1 + .../connections/ConnectionRequestCard.tsx | 83 ++++++++++++++----- frontend/src/components/layout/AppLayout.tsx | 51 ++++++------ frontend/src/components/people/PeoplePage.tsx | 39 +++++++++ frontend/src/hooks/useConnections.ts | 13 +++ frontend/src/hooks/useNotifications.ts | 39 ++++++++- 11 files changed, 312 insertions(+), 52 deletions(-) create mode 100644 backend/alembic/versions/044_add_notification_type_check.py diff --git a/backend/alembic/versions/044_add_notification_type_check.py b/backend/alembic/versions/044_add_notification_type_check.py new file mode 100644 index 0000000..7f45b14 --- /dev/null +++ b/backend/alembic/versions/044_add_notification_type_check.py @@ -0,0 +1,45 @@ +"""Add CHECK constraint on notifications.type column. + +Revision ID: 044 +Revises: 043 +""" +from alembic import op +import sqlalchemy as sa + +revision = "044" +down_revision = "043" +branch_labels = None +depends_on = None + +ALLOWED_TYPES = ( + "connection_request", + "connection_accepted", + "connection_rejected", + "info", + "warning", + "reminder", + "system", +) + + +def upgrade() -> None: + # Defensive: ensure no existing rows violate the constraint + conn = op.get_bind() + placeholders = ", ".join(f"'{t}'" for t in ALLOWED_TYPES) + bad = conn.execute( + sa.text(f"SELECT COUNT(*) FROM notifications WHERE type NOT IN ({placeholders})") + ).scalar() + if bad: + raise RuntimeError( + f"Cannot apply CHECK constraint: {bad} notification(s) have types outside the allowed list" + ) + + op.create_check_constraint( + "ck_notifications_type", + "notifications", + f"type IN ({placeholders})", + ) + + +def downgrade() -> None: + op.drop_constraint("ck_notifications_type", "notifications", type_="check") diff --git a/backend/app/models/notification.py b/backend/app/models/notification.py index f98ae7d..58d71c3 100644 --- a/backend/app/models/notification.py +++ b/backend/app/models/notification.py @@ -1,13 +1,26 @@ -from sqlalchemy import String, Text, Integer, Boolean, ForeignKey, func +from sqlalchemy import CheckConstraint, String, Text, Integer, Boolean, ForeignKey, func from sqlalchemy.dialects.postgresql import JSONB from sqlalchemy.orm import Mapped, mapped_column from datetime import datetime from typing import Optional from app.database import Base +# Active: connection_request, connection_accepted +# Reserved: connection_rejected, info, warning, reminder, system +_NOTIFICATION_TYPES = ( + "connection_request", "connection_accepted", "connection_rejected", + "info", "warning", "reminder", "system", +) + class Notification(Base): __tablename__ = "notifications" + __table_args__ = ( + CheckConstraint( + f"type IN ({', '.join(repr(t) for t in _NOTIFICATION_TYPES)})", + name="ck_notifications_type", + ), + ) id: Mapped[int] = mapped_column(primary_key=True, index=True) user_id: Mapped[int] = mapped_column( diff --git a/backend/app/routers/connections.py b/backend/app/routers/connections.py index 2a7eeea..a8fe237 100644 --- a/backend/app/routers/connections.py +++ b/backend/app/routers/connections.py @@ -12,7 +12,7 @@ import asyncio from datetime import datetime, timedelta, timezone from fastapi import APIRouter, BackgroundTasks, Depends, HTTPException, Path, Query, Request -from sqlalchemy import select, func, and_, update +from sqlalchemy import delete, select, func, and_, update from sqlalchemy.exc import IntegrityError from sqlalchemy.ext.asyncio import AsyncSession from sqlalchemy.orm import selectinload @@ -26,6 +26,7 @@ from app.models.user import User from app.models.user_connection import UserConnection from app.routers.auth import get_current_user from app.schemas.connection import ( + CancelResponse, ConnectionRequestResponse, ConnectionResponse, RespondAcceptResponse, @@ -459,6 +460,61 @@ async def respond_to_request( return {"message": "Connection request rejected"} +# ── PUT /requests/{id}/cancel ────────────────────────────────────── + +@router.put("/requests/{request_id}/cancel", response_model=CancelResponse) +async def cancel_request( + request: Request, + request_id: int = Path(ge=1, le=2147483647), + db: AsyncSession = Depends(get_db), + current_user: User = Depends(get_current_user), +): + """Cancel an outgoing connection request. Atomic via UPDATE...WHERE status='pending'.""" + now = datetime.now() + + # Atomic update — only succeeds if sender is current user and status is still pending + result = await db.execute( + update(ConnectionRequest) + .where( + ConnectionRequest.id == request_id, + ConnectionRequest.sender_id == current_user.id, + ConnectionRequest.status == "pending", + ) + .values(status="cancelled", resolved_at=now) + .returning(ConnectionRequest.id, ConnectionRequest.receiver_id) + ) + row = result.first() + if not row: + raise HTTPException(status_code=409, detail="Request not found or already resolved") + + receiver_id = row.receiver_id + + # Silent cleanup: remove the notification sent to the receiver + await db.execute( + delete(Notification).where( + Notification.source_type == "connection_request", + Notification.source_id == request_id, + Notification.user_id == receiver_id, + ) + ) + + # Look up receiver umbral_name for audit detail + receiver_result = await db.execute(select(User.umbral_name).where(User.id == receiver_id)) + receiver_umbral_name = receiver_result.scalar_one_or_none() or "unknown" + + await log_audit_event( + db, + action="connection.request_cancelled", + actor_id=current_user.id, + target_id=receiver_id, + detail={"request_id": request_id, "receiver_umbral_name": receiver_umbral_name}, + ip=get_client_ip(request), + ) + + await db.commit() + return {"message": "Connection request cancelled"} + + # ── GET / ─────────────────────────────────────────────────────────── @router.get("/", response_model=list[ConnectionResponse]) diff --git a/backend/app/routers/people.py b/backend/app/routers/people.py index 3fff4b4..6dfe41c 100644 --- a/backend/app/routers/people.py +++ b/backend/app/routers/people.py @@ -12,7 +12,7 @@ from app.models.user import User from app.models.user_connection import UserConnection from app.schemas.person import PersonCreate, PersonUpdate, PersonResponse from app.routers.auth import get_current_user -from app.services.connection import resolve_shared_profile +from app.services.connection import detach_umbral_contact, resolve_shared_profile router = APIRouter() @@ -231,6 +231,20 @@ async def delete_person( if not person: raise HTTPException(status_code=404, detail="Person not found") + # Auto-detach umbral contact before delete + if person.is_umbral_contact: + await detach_umbral_contact(person) + # Null out the current user's connection person_id so the connection survives + conn_result = await db.execute( + select(UserConnection).where( + UserConnection.user_id == current_user.id, + UserConnection.person_id == person.id, + ) + ) + conn = conn_result.scalar_one_or_none() + if conn: + conn.person_id = None + await db.delete(person) await db.commit() diff --git a/backend/app/schemas/connection.py b/backend/app/schemas/connection.py index d2048dc..c313e72 100644 --- a/backend/app/schemas/connection.py +++ b/backend/app/schemas/connection.py @@ -72,6 +72,10 @@ class RespondRejectResponse(BaseModel): message: str +class CancelResponse(BaseModel): + message: str + + class SharingOverrideUpdate(BaseModel): model_config = ConfigDict(extra="forbid") preferred_name: Optional[bool] = None diff --git a/frontend/src/components/admin/ConfigPage.tsx b/frontend/src/components/admin/ConfigPage.tsx index f699e96..2b1131a 100644 --- a/frontend/src/components/admin/ConfigPage.tsx +++ b/frontend/src/components/admin/ConfigPage.tsx @@ -31,6 +31,7 @@ const ACTION_TYPES = [ 'auth.registration', 'auth.mfa_enforce_prompted', 'connection.request_sent', + 'connection.request_cancelled', 'connection.accepted', 'connection.rejected', 'connection.removed', diff --git a/frontend/src/components/connections/ConnectionRequestCard.tsx b/frontend/src/components/connections/ConnectionRequestCard.tsx index c55ad3e..d4bebdf 100644 --- a/frontend/src/components/connections/ConnectionRequestCard.tsx +++ b/frontend/src/components/connections/ConnectionRequestCard.tsx @@ -1,4 +1,4 @@ -import { useState } from 'react'; +import { useState, useEffect } from 'react'; import { Check, X, Loader2 } from 'lucide-react'; import { toast } from 'sonner'; import { formatDistanceToNow } from 'date-fns'; @@ -10,12 +10,23 @@ import type { ConnectionRequest } from '@/types'; interface ConnectionRequestCardProps { request: ConnectionRequest; + direction: 'incoming' | 'outgoing'; } -export default function ConnectionRequestCard({ request }: ConnectionRequestCardProps) { - const { respond, isResponding } = useConnections(); +export default function ConnectionRequestCard({ request, direction }: ConnectionRequestCardProps) { + const { respond, isResponding, cancelRequest, isCancelling } = useConnections(); const [resolved, setResolved] = useState(false); + // Clean up invisible DOM element after fade-out transition + const [hidden, setHidden] = useState(false); + useEffect(() => { + if (!resolved) return; + const timer = setTimeout(() => setHidden(true), 300); + return () => clearTimeout(timer); + }, [resolved]); + + if (hidden) return null; + const handleRespond = async (action: 'accept' | 'reject') => { try { await respond({ requestId: request.id, action }); @@ -26,7 +37,21 @@ export default function ConnectionRequestCard({ request }: ConnectionRequestCard } }; - const displayName = request.sender_preferred_name || request.sender_umbral_name; + const handleCancel = async () => { + setResolved(true); + try { + await cancelRequest(request.id); + toast.success('Request cancelled'); + } catch (err) { + setResolved(false); + toast.error(getErrorMessage(err, 'Failed to cancel request')); + } + }; + + const isIncoming = direction === 'incoming'; + const displayName = isIncoming + ? request.sender_preferred_name || request.sender_umbral_name + : request.receiver_preferred_name || request.receiver_umbral_name; return (

{displayName}

- wants to connect · {formatDistanceToNow(new Date(request.created_at), { addSuffix: true })} + {isIncoming ? 'wants to connect' : 'request pending'} · {formatDistanceToNow(new Date(request.created_at), { addSuffix: true })}

{/* Actions */}
- - + {isIncoming ? ( + <> + + + + ) : ( + + )}
); diff --git a/frontend/src/components/layout/AppLayout.tsx b/frontend/src/components/layout/AppLayout.tsx index 55ee07c..7b7a89c 100644 --- a/frontend/src/components/layout/AppLayout.tsx +++ b/frontend/src/components/layout/AppLayout.tsx @@ -4,6 +4,7 @@ import { Menu } from 'lucide-react'; import { useTheme } from '@/hooks/useTheme'; import { AlertsProvider } from '@/hooks/useAlerts'; import { LockProvider } from '@/hooks/useLock'; +import { NotificationProvider } from '@/hooks/useNotifications'; import { Button } from '@/components/ui/button'; import Sidebar from './Sidebar'; import LockOverlay from './LockOverlay'; @@ -20,32 +21,34 @@ export default function AppLayout() { return ( -
- { - const next = !collapsed; - setCollapsed(next); - localStorage.setItem('umbra-sidebar-collapsed', JSON.stringify(next)); - }} - mobileOpen={mobileOpen} - onMobileClose={() => setMobileOpen(false)} - /> -
- {/* Mobile header */} -
- -

UMBRA

+ +
+ { + const next = !collapsed; + setCollapsed(next); + localStorage.setItem('umbra-sidebar-collapsed', JSON.stringify(next)); + }} + mobileOpen={mobileOpen} + onMobileClose={() => setMobileOpen(false)} + /> +
+ {/* Mobile header */} +
+ +

UMBRA

+
+
+ +
-
- -
-
- - + + + ); diff --git a/frontend/src/components/people/PeoplePage.tsx b/frontend/src/components/people/PeoplePage.tsx index ed4b3dd..aa1cd7a 100644 --- a/frontend/src/components/people/PeoplePage.tsx +++ b/frontend/src/components/people/PeoplePage.tsx @@ -24,6 +24,8 @@ import { useTableVisibility } from '@/hooks/useTableVisibility'; import { useCategoryOrder } from '@/hooks/useCategoryOrder'; import PersonForm from './PersonForm'; import ConnectionSearch from '@/components/connections/ConnectionSearch'; +import ConnectionRequestCard from '@/components/connections/ConnectionRequestCard'; +import { useConnections } from '@/hooks/useConnections'; // --------------------------------------------------------------------------- // StatCounter — inline helper @@ -205,6 +207,9 @@ export default function PeoplePage() { const [showAddDropdown, setShowAddDropdown] = useState(false); const addDropdownRef = useRef(null); + const { incomingRequests, outgoingRequests } = useConnections(); + const hasRequests = incomingRequests.length > 0 || outgoingRequests.length > 0; + const { data: people = [], isLoading } = useQuery({ queryKey: ['people'], queryFn: async () => { @@ -583,6 +588,40 @@ export default function PeoplePage() {
)} + {/* Pending requests */} + {hasRequests && ( +
+
+ + Pending Requests + + + {incomingRequests.length + outgoingRequests.length} + +
+
+ {incomingRequests.length > 0 && outgoingRequests.length > 0 && ( +

Incoming

+ )} + {incomingRequests.slice(0, 5).map((req) => ( + + ))} + {incomingRequests.length > 5 && ( +

+{incomingRequests.length - 5} more

+ )} + {incomingRequests.length > 0 && outgoingRequests.length > 0 && ( +

Outgoing

+ )} + {outgoingRequests.slice(0, 5).map((req) => ( + + ))} + {outgoingRequests.length > 5 && ( +

+{outgoingRequests.length - 5} more

+ )} +
+
+ )} + {/* Main content: table + panel */}
{/* Table */} diff --git a/frontend/src/hooks/useConnections.ts b/frontend/src/hooks/useConnections.ts index fae3a46..d92eec4 100644 --- a/frontend/src/hooks/useConnections.ts +++ b/frontend/src/hooks/useConnections.ts @@ -62,6 +62,17 @@ export function useConnections() { }, }); + const cancelMutation = useMutation({ + mutationFn: async (requestId: number) => { + const { data } = await api.put(`/connections/requests/${requestId}/cancel`); + return data; + }, + onSuccess: () => { + queryClient.invalidateQueries({ queryKey: ['connections'] }); + queryClient.invalidateQueries({ queryKey: ['notifications'] }); + }, + }); + const removeConnectionMutation = useMutation({ mutationFn: async (connectionId: number) => { await api.delete(`/connections/${connectionId}`); @@ -83,6 +94,8 @@ export function useConnections() { isSending: sendRequestMutation.isPending, respond: respondMutation.mutateAsync, isResponding: respondMutation.isPending, + cancelRequest: cancelMutation.mutateAsync, + isCancelling: cancelMutation.isPending, removeConnection: removeConnectionMutation.mutateAsync, }; } diff --git a/frontend/src/hooks/useNotifications.ts b/frontend/src/hooks/useNotifications.ts index 978a4ce..e2663ad 100644 --- a/frontend/src/hooks/useNotifications.ts +++ b/frontend/src/hooks/useNotifications.ts @@ -1,13 +1,39 @@ +import { createContext, useContext, type ReactNode } from 'react'; import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query'; -import { useEffect, useRef } from 'react'; +import { useEffect, useRef, createElement } from 'react'; import api from '@/lib/api'; import type { NotificationListResponse } from '@/types'; +interface NotificationContextValue { + unreadCount: number; + notifications: NotificationListResponse['notifications']; + total: number; + isLoading: boolean; + markRead: (ids: number[]) => Promise; + markAllRead: () => Promise; + deleteNotification: (id: number) => Promise; + refreshNotifications: () => void; +} + +const NotificationContext = createContext({ + unreadCount: 0, + notifications: [], + total: 0, + isLoading: true, + markRead: async () => {}, + markAllRead: async () => {}, + deleteNotification: async () => {}, + refreshNotifications: () => {}, +}); + export function useNotifications() { + return useContext(NotificationContext); +} + +export function NotificationProvider({ children }: { children: ReactNode }) { const queryClient = useQueryClient(); const visibleRef = useRef(true); - // Track tab visibility to pause polling when hidden useEffect(() => { const handler = () => { visibleRef.current = document.visibilityState === 'visible'; @@ -65,7 +91,11 @@ export function useNotifications() { }, }); - return { + const refreshNotifications = () => { + queryClient.invalidateQueries({ queryKey: ['notifications'] }); + }; + + const value: NotificationContextValue = { unreadCount: unreadQuery.data ?? 0, notifications: listQuery.data?.notifications ?? [], total: listQuery.data?.total ?? 0, @@ -73,5 +103,8 @@ export function useNotifications() { markRead: markReadMutation.mutateAsync, markAllRead: markAllReadMutation.mutateAsync, deleteNotification: deleteMutation.mutateAsync, + refreshNotifications, }; + + return createElement(NotificationContext.Provider, { value }, children); }