Fix QA: IntegrityError handling, dict mutation, birthday sync, None guard
- C-01: Wrap accept flow flush/commit in IntegrityError handling (409) - C-02: Use separate remote_timestamps dict instead of pop() on shared profile - W-01: Add birthday sync in Link conversion path (existing person → umbral) - W-02: Add None guard on max(updated_at) comparison in get_person Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
568a78e64b
commit
b554ba7151
@ -9,7 +9,7 @@ Security:
|
|||||||
- Audit logging for all connection events
|
- Audit logging for all connection events
|
||||||
"""
|
"""
|
||||||
import asyncio
|
import asyncio
|
||||||
from datetime import datetime, timedelta, timezone
|
from datetime import date as date_type, datetime, timedelta, timezone
|
||||||
|
|
||||||
from fastapi import APIRouter, BackgroundTasks, Depends, HTTPException, Path, Query, Request
|
from fastapi import APIRouter, BackgroundTasks, Depends, HTTPException, Path, Query, Request
|
||||||
from sqlalchemy import delete, select, func, and_, update
|
from sqlalchemy import delete, select, func, and_, update
|
||||||
@ -415,7 +415,7 @@ async def respond_to_request(
|
|||||||
select(Person).where(Person.id == request_person_id)
|
select(Person).where(Person.id == request_person_id)
|
||||||
)
|
)
|
||||||
existing_person = existing_result.scalar_one_or_none()
|
existing_person = existing_result.scalar_one_or_none()
|
||||||
# Re-validate at accept time (C-01, W-01): ownership must match sender,
|
# Re-validate at accept time: ownership must match sender,
|
||||||
# and must not already be umbral (prevents double-conversion races)
|
# and must not already be umbral (prevents double-conversion races)
|
||||||
if existing_person and existing_person.user_id == sender_id and not existing_person.is_umbral_contact:
|
if existing_person and existing_person.user_id == sender_id and not existing_person.is_umbral_contact:
|
||||||
# Convert existing standard contact to umbral
|
# Convert existing standard contact to umbral
|
||||||
@ -433,6 +433,13 @@ async def respond_to_request(
|
|||||||
existing_person.address = receiver_shared.get("address") or existing_person.address
|
existing_person.address = receiver_shared.get("address") or existing_person.address
|
||||||
existing_person.company = receiver_shared.get("company") or existing_person.company
|
existing_person.company = receiver_shared.get("company") or existing_person.company
|
||||||
existing_person.job_title = receiver_shared.get("job_title") or existing_person.job_title
|
existing_person.job_title = receiver_shared.get("job_title") or existing_person.job_title
|
||||||
|
# Sync birthday from shared profile
|
||||||
|
birthday_str = receiver_shared.get("birthday")
|
||||||
|
if birthday_str:
|
||||||
|
try:
|
||||||
|
existing_person.birthday = date_type.fromisoformat(birthday_str)
|
||||||
|
except (ValueError, TypeError):
|
||||||
|
pass
|
||||||
# Recompute display name
|
# Recompute display name
|
||||||
full = ((first_name or '') + ' ' + (last_name or '')).strip()
|
full = ((first_name or '') + ' ' + (last_name or '')).strip()
|
||||||
existing_person.name = full or current_user.umbral_name
|
existing_person.name = full or current_user.umbral_name
|
||||||
@ -444,7 +451,11 @@ async def respond_to_request(
|
|||||||
)
|
)
|
||||||
db.add(person_for_sender)
|
db.add(person_for_sender)
|
||||||
|
|
||||||
|
try:
|
||||||
await db.flush() # populate person IDs
|
await db.flush() # populate person IDs
|
||||||
|
except IntegrityError:
|
||||||
|
await db.rollback()
|
||||||
|
raise HTTPException(status_code=409, detail="Connection already exists")
|
||||||
|
|
||||||
# Create bidirectional connections
|
# Create bidirectional connections
|
||||||
conn_a = UserConnection(
|
conn_a = UserConnection(
|
||||||
@ -460,7 +471,11 @@ async def respond_to_request(
|
|||||||
db.add(conn_a)
|
db.add(conn_a)
|
||||||
db.add(conn_b)
|
db.add(conn_b)
|
||||||
|
|
||||||
|
try:
|
||||||
await db.flush() # populate conn_a.id for source_id
|
await db.flush() # populate conn_a.id for source_id
|
||||||
|
except IntegrityError:
|
||||||
|
await db.rollback()
|
||||||
|
raise HTTPException(status_code=409, detail="Connection already exists")
|
||||||
|
|
||||||
# Notification to sender
|
# Notification to sender
|
||||||
receiver_display = (receiver_settings.preferred_name if receiver_settings else None) or current_user.umbral_name
|
receiver_display = (receiver_settings.preferred_name if receiver_settings else None) or current_user.umbral_name
|
||||||
|
|||||||
@ -91,8 +91,9 @@ async def get_people(
|
|||||||
for c in conns_result.scalars().all()
|
for c in conns_result.scalars().all()
|
||||||
}
|
}
|
||||||
|
|
||||||
# Build shared profiles
|
# Build shared profiles and track remote timestamps separately
|
||||||
shared_profiles: dict[int, dict] = {}
|
shared_profiles: dict[int, dict] = {}
|
||||||
|
remote_timestamps: dict[int, datetime] = {}
|
||||||
for uid in linked_user_ids:
|
for uid in linked_user_ids:
|
||||||
user = users_by_id.get(uid)
|
user = users_by_id.get(uid)
|
||||||
user_settings = settings_by_user.get(uid)
|
user_settings = settings_by_user.get(uid)
|
||||||
@ -102,19 +103,17 @@ async def get_people(
|
|||||||
)
|
)
|
||||||
# umbral_name is always visible (public identity), not a shareable field
|
# umbral_name is always visible (public identity), not a shareable field
|
||||||
shared_profiles[uid]["umbral_name"] = user.umbral_name
|
shared_profiles[uid]["umbral_name"] = user.umbral_name
|
||||||
shared_profiles[uid]["_updated_at"] = max(
|
if user.updated_at and user_settings.updated_at:
|
||||||
user.updated_at, user_settings.updated_at
|
remote_timestamps[uid] = max(user.updated_at, user_settings.updated_at)
|
||||||
)
|
|
||||||
|
|
||||||
# Attach to response
|
# Attach to response
|
||||||
responses = []
|
responses = []
|
||||||
for p in people:
|
for p in people:
|
||||||
resp = PersonResponse.model_validate(p)
|
resp = PersonResponse.model_validate(p)
|
||||||
if p.linked_user_id and p.linked_user_id in shared_profiles:
|
if p.linked_user_id and p.linked_user_id in shared_profiles:
|
||||||
profile = shared_profiles[p.linked_user_id]
|
resp.shared_fields = shared_profiles[p.linked_user_id]
|
||||||
resp.shared_fields = profile
|
|
||||||
# Show the latest update time across local record and connected user's profile
|
# Show the latest update time across local record and connected user's profile
|
||||||
remote_updated = profile.pop("_updated_at", None)
|
remote_updated = remote_timestamps.get(p.linked_user_id)
|
||||||
if remote_updated and remote_updated > p.updated_at:
|
if remote_updated and remote_updated > p.updated_at:
|
||||||
resp.updated_at = remote_updated
|
resp.updated_at = remote_updated
|
||||||
responses.append(resp)
|
responses.append(resp)
|
||||||
@ -188,6 +187,7 @@ async def get_person(
|
|||||||
)
|
)
|
||||||
resp.shared_fields["umbral_name"] = linked_user.umbral_name
|
resp.shared_fields["umbral_name"] = linked_user.umbral_name
|
||||||
# Show the latest update time across local record and connected user's profile
|
# Show the latest update time across local record and connected user's profile
|
||||||
|
if linked_user.updated_at and linked_settings.updated_at:
|
||||||
remote_updated = max(linked_user.updated_at, linked_settings.updated_at)
|
remote_updated = max(linked_user.updated_at, linked_settings.updated_at)
|
||||||
if remote_updated > person.updated_at:
|
if remote_updated > person.updated_at:
|
||||||
resp.updated_at = remote_updated
|
resp.updated_at = remote_updated
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user