Action QA findings: refactor sync to accept resolved values

C-01: sync_birthday_to_contacts now accepts (share_birthday, date_of_birth)
      directly — no internal re-query, no stale-read risk with autoflush.
W-01: Eliminated redundant User/Settings SELECTs inside the service.
W-02: Removed scalar_one() on User query (no longer queries internally).
W-03: Settings router only syncs when share_birthday value actually changes.
S-02: Added logger.info with rowcount for observability.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Kyle 2026-03-07 06:13:21 +08:00
parent 8aec5a5078
commit 66cc1a0457
3 changed files with 26 additions and 14 deletions

View File

@ -687,7 +687,12 @@ async def update_profile(
current_user.email = update_data["email"]
if "date_of_birth" in update_data:
current_user.date_of_birth = update_data["date_of_birth"]
await sync_birthday_to_contacts(db, current_user.id)
settings_result = await db.execute(
select(Settings).where(Settings.user_id == current_user.id)
)
user_settings = settings_result.scalar_one_or_none()
share = user_settings.share_birthday if user_settings else False
await sync_birthday_to_contacts(db, current_user.id, share_birthday=share, date_of_birth=update_data["date_of_birth"])
if "umbral_name" in update_data:
current_user.umbral_name = update_data["umbral_name"]

View File

@ -79,6 +79,7 @@ async def get_settings(
async def update_settings(
settings_update: SettingsUpdate,
db: AsyncSession = Depends(get_db),
current_user: User = Depends(get_current_user),
current_settings: Settings = Depends(get_current_settings)
):
"""Update settings."""
@ -92,11 +93,17 @@ async def update_settings(
except ValueError as e:
raise HTTPException(status_code=400, detail=str(e))
old_share_birthday = current_settings.share_birthday
for key, value in update_data.items():
setattr(current_settings, key, value)
if "share_birthday" in update_data:
await sync_birthday_to_contacts(db, current_settings.user_id)
if "share_birthday" in update_data and update_data["share_birthday"] != old_share_birthday:
await sync_birthday_to_contacts(
db, current_user.id,
share_birthday=update_data["share_birthday"],
date_of_birth=current_user.date_of_birth,
)
await db.commit()
await db.refresh(current_settings)

View File

@ -9,7 +9,7 @@ from datetime import date as date_type
from types import SimpleNamespace
from typing import Optional
from sqlalchemy import select, update
from sqlalchemy import update
from sqlalchemy.ext.asyncio import AsyncSession
from app.models.person import Person
@ -136,22 +136,22 @@ def create_person_from_connection(
async def sync_birthday_to_contacts(db: AsyncSession, user_id: int) -> None:
async def sync_birthday_to_contacts(
db: AsyncSession,
user_id: int,
share_birthday: bool,
date_of_birth: Optional[date_type],
) -> None:
"""Sync user's DOB to all Person records where linked_user_id == user_id.
Respects share_birthday setting if disabled, clears birthday on linked records."""
user = await db.execute(select(User).where(User.id == user_id))
user_obj = user.scalar_one()
settings_result = await db.execute(select(Settings).where(Settings.user_id == user_id))
settings_obj = settings_result.scalar_one_or_none()
Caller passes resolved values no internal re-query."""
new_birthday = date_of_birth if share_birthday else None
share = settings_obj.share_birthday if settings_obj else False
new_birthday = user_obj.date_of_birth if share else None
await db.execute(
result = await db.execute(
update(Person)
.where(Person.linked_user_id == user_id)
.values(birthday=new_birthday)
)
logger.info("sync_birthday_to_contacts user_id=%s updated %s person(s)", user_id, result.rowcount)
async def detach_umbral_contact(person: Person) -> None: