From 66cc1a045737de45421c8964df5c28e3312a616f Mon Sep 17 00:00:00 2001 From: Kyle Pope Date: Sat, 7 Mar 2026 06:13:21 +0800 Subject: [PATCH] Action QA findings: refactor sync to accept resolved values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- backend/app/routers/auth.py | 7 ++++++- backend/app/routers/settings.py | 11 +++++++++-- backend/app/services/connection.py | 22 +++++++++++----------- 3 files changed, 26 insertions(+), 14 deletions(-) diff --git a/backend/app/routers/auth.py b/backend/app/routers/auth.py index 777833e..c998f32 100644 --- a/backend/app/routers/auth.py +++ b/backend/app/routers/auth.py @@ -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"] diff --git a/backend/app/routers/settings.py b/backend/app/routers/settings.py index 877a393..5b4777b 100644 --- a/backend/app/routers/settings.py +++ b/backend/app/routers/settings.py @@ -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) diff --git a/backend/app/services/connection.py b/backend/app/services/connection.py index 270d487..01f2323 100644 --- a/backend/app/services/connection.py +++ b/backend/app/services/connection.py @@ -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: