Fix QA W-01/W-05/W-06/W-08: cancel requests, detach umbral, notifications
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 <noreply@anthropic.com>
This commit is contained in:
parent
0e94b6e1f7
commit
820ff46efa
45
backend/alembic/versions/044_add_notification_type_check.py
Normal file
45
backend/alembic/versions/044_add_notification_type_check.py
Normal file
@ -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")
|
||||
@ -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(
|
||||
|
||||
@ -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])
|
||||
|
||||
@ -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()
|
||||
|
||||
|
||||
@ -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
|
||||
|
||||
@ -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',
|
||||
|
||||
@ -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 (
|
||||
<div
|
||||
@ -46,12 +71,14 @@ export default function ConnectionRequestCard({ request }: ConnectionRequestCard
|
||||
<div className="flex-1 min-w-0">
|
||||
<p className="text-sm font-medium truncate">{displayName}</p>
|
||||
<p className="text-xs text-muted-foreground">
|
||||
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 })}
|
||||
</p>
|
||||
</div>
|
||||
|
||||
{/* Actions */}
|
||||
<div className="flex items-center gap-1.5 shrink-0">
|
||||
{isIncoming ? (
|
||||
<>
|
||||
<Button
|
||||
size="sm"
|
||||
onClick={() => handleRespond('accept')}
|
||||
@ -69,6 +96,18 @@ export default function ConnectionRequestCard({ request }: ConnectionRequestCard
|
||||
>
|
||||
<X className="h-3.5 w-3.5" />
|
||||
</Button>
|
||||
</>
|
||||
) : (
|
||||
<Button
|
||||
variant="ghost"
|
||||
size="sm"
|
||||
onClick={handleCancel}
|
||||
disabled={isCancelling}
|
||||
className="text-muted-foreground hover:text-destructive hover:bg-destructive/10"
|
||||
>
|
||||
{isCancelling ? <Loader2 className="h-3.5 w-3.5 animate-spin" /> : <X className="h-3.5 w-3.5" />}
|
||||
</Button>
|
||||
)}
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
|
||||
@ -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,6 +21,7 @@ export default function AppLayout() {
|
||||
return (
|
||||
<LockProvider>
|
||||
<AlertsProvider>
|
||||
<NotificationProvider>
|
||||
<div className="flex h-screen overflow-hidden bg-background">
|
||||
<Sidebar
|
||||
collapsed={collapsed}
|
||||
@ -46,6 +48,7 @@ export default function AppLayout() {
|
||||
</div>
|
||||
<LockOverlay />
|
||||
<NotificationToaster />
|
||||
</NotificationProvider>
|
||||
</AlertsProvider>
|
||||
</LockProvider>
|
||||
);
|
||||
|
||||
@ -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<HTMLDivElement>(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() {
|
||||
</div>
|
||||
)}
|
||||
|
||||
{/* Pending requests */}
|
||||
{hasRequests && (
|
||||
<div className="px-6 pb-3">
|
||||
<div className="flex items-center gap-2 mb-2">
|
||||
<span className="text-xs text-muted-foreground uppercase tracking-wider font-medium">
|
||||
Pending Requests
|
||||
</span>
|
||||
<span className="text-[10px] tabular-nums bg-accent/15 text-accent px-1.5 py-0.5 rounded-full font-medium">
|
||||
{incomingRequests.length + outgoingRequests.length}
|
||||
</span>
|
||||
</div>
|
||||
<div className="space-y-2">
|
||||
{incomingRequests.length > 0 && outgoingRequests.length > 0 && (
|
||||
<p className="text-[11px] text-muted-foreground font-medium uppercase tracking-wider">Incoming</p>
|
||||
)}
|
||||
{incomingRequests.slice(0, 5).map((req) => (
|
||||
<ConnectionRequestCard key={req.id} request={req} direction="incoming" />
|
||||
))}
|
||||
{incomingRequests.length > 5 && (
|
||||
<p className="text-xs text-muted-foreground">+{incomingRequests.length - 5} more</p>
|
||||
)}
|
||||
{incomingRequests.length > 0 && outgoingRequests.length > 0 && (
|
||||
<p className="text-[11px] text-muted-foreground font-medium uppercase tracking-wider mt-3">Outgoing</p>
|
||||
)}
|
||||
{outgoingRequests.slice(0, 5).map((req) => (
|
||||
<ConnectionRequestCard key={req.id} request={req} direction="outgoing" />
|
||||
))}
|
||||
{outgoingRequests.length > 5 && (
|
||||
<p className="text-xs text-muted-foreground">+{outgoingRequests.length - 5} more</p>
|
||||
)}
|
||||
</div>
|
||||
</div>
|
||||
)}
|
||||
|
||||
{/* Main content: table + panel */}
|
||||
<div className="flex-1 overflow-hidden flex">
|
||||
{/* Table */}
|
||||
|
||||
@ -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,
|
||||
};
|
||||
}
|
||||
|
||||
@ -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<void>;
|
||||
markAllRead: () => Promise<void>;
|
||||
deleteNotification: (id: number) => Promise<void>;
|
||||
refreshNotifications: () => void;
|
||||
}
|
||||
|
||||
const NotificationContext = createContext<NotificationContextValue>({
|
||||
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);
|
||||
}
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user