fix(photo-approval): resolve group photo lifecycle gaps

app/core/config.py

@@ -33,6 +33,8 @@ class Settings(BaseSettings):
     POSTGRES_HOST: str = "localhost"
     POSTGRES_PORT: int = 5432
 
+    PHOTO_APPROVAL_TIMEOUT_DAYS: int = 7
+
     # Mobile auth/session defaults
     MOBILE_SESSION_LIMIT: int = 3
     MOBILE_SESSION_TTL_SECONDS: int = 180

app/main.py

@@ -8,6 +8,7 @@
 from fastapi.middleware.cors import CORSMiddleware
 from starlette.middleware.base import BaseHTTPMiddleware, RequestResponseEndpoint
 from app.core.config import settings
+from app.infra.database import engine
 from app.infra.minio import init_minio_client
 from app.infra.nats import NatsClient
 from app.infra.redis import RedisClient
@@ -48,6 +49,18 @@ async def dispatch(
 
 
 
+async def _approval_expiry_loop() -> None:
+    while True:
+        await asyncio.sleep(3600)
+        try:
+            async with engine.begin() as conn:
+                from app.container import Container
+                container = Container(conn)
+                await container.photo_approval_service.expire_stale(settings.PHOTO_APPROVAL_TIMEOUT_DAYS)
+        except Exception as exc:
+            logger.warning("Approval expiry task failed: %s", exc)
+
+
 MAX_RETRIES = 5
 RETRY_DELAY = 2  # seconds
 @asynccontextmanager
@@ -77,8 +90,11 @@ async def lifespan(app: FastAPI) -> AsyncIterator[None]:
     await NatsClient.connect()
     get_face_embedding_service()
 
+    expiry_task = asyncio.create_task(_approval_expiry_loop())
+
     yield
 
+    expiry_task.cancel()
     await RedisClient.get_instance().close()
     await NatsClient.close()
 

app/service/photo_approval.py

@@ -64,6 +64,14 @@ async def decide(
         await self._photo_querier.update_photo_status(id=photo_id, status="approved")
         return "approved"
 
+    async def expire_stale(self, timeout_days: int) -> int:
+        count = 0
+        async for _ in self._approval_querier.expire_stale_approvals(timeout_days=timeout_days):
+            count += 1
+        if count:
+            logger.info("Auto-expired %d stale pending photo(s)", count)
+        return count
+
     async def _delete_photo_storage(self, photo_id: UUID) -> None:
         photo = await self._photo_querier.get_photo_by_id(id=photo_id)
         if photo is None:

app/worker/photo_worker/main.py

@@ -126,6 +126,8 @@ async def _handle_single_face(self, event: PhotoProcessEvent, face: DetectedFace
     async def _handle_group_photo(self, event: PhotoProcessEvent, faces: list[DetectedFace]) -> None:
         logger.info("Processing group photo %s with %d faces", event.photo_id, len(faces))
 
+        approvals_created = 0
+
         for face_index, face in enumerate(faces):
             bbox_json = json.dumps({
                 "x1": float(face.bbox[0]),
@@ -151,6 +153,8 @@ async def _handle_group_photo(self, event: PhotoProcessEvent, faces: list[Detect
                 logger.info("No match for face %d in photo %s", face_index, event.photo_id)
                 continue
 
+            approvals_created += 1
+
             try:
                 await self._notification_service.create_notification(
                     user_id=approval.user_id,
@@ -174,6 +178,11 @@ async def _handle_group_photo(self, event: PhotoProcessEvent, faces: list[Detect
                     approval.user_id, event.photo_id, exc,
                 )
 
+        if approvals_created == 0:
+            logger.info("No users matched in group photo %s, auto-approving as public", event.photo_id)
+            await self._photo_querier.update_photo_status(id=event.photo_id, status="approved")
+            await self._photo_querier.update_photo_visibility(id=event.photo_id, visibility="public")
+
 
     async def _create_job(self, event: PhotoProcessEvent) -> models.ProcessingJob | None:
         if self._pj_querier is None:

db/generated/photo_approvals.py

@@ -11,6 +11,25 @@
 from db.generated import models
 
 
+EXPIRE_STALE_APPROVALS = """-- name: expire_stale_approvals \\:many
+WITH stale_photos AS (
+    SELECT id FROM photos
+    WHERE status = 'pending'
+      AND created_at < now() - make_interval(days => :p1::int)
+),
+_update_approvals AS (
+    UPDATE photo_approvals
+    SET decision = 'approved', decided_at = now()
+    WHERE photo_id IN (SELECT id FROM stale_photos)
+      AND decision = 'pending'
+)
+UPDATE photos
+SET status = 'approved'
+WHERE id IN (SELECT id FROM stale_photos)
+RETURNING id
+"""
+
+
 CREATE_PHOTO_APPROVAL = """-- name: create_photo_approval \\:one
 INSERT INTO photo_approvals (
     photo_id,
@@ -49,6 +68,11 @@ class AsyncQuerier:
     def __init__(self, conn: sqlalchemy.ext.asyncio.AsyncConnection):
         self._conn = conn
 
+    async def expire_stale_approvals(self, *, timeout_days: int) -> AsyncIterator[uuid.UUID]:
+        result = await self._conn.stream(sqlalchemy.text(EXPIRE_STALE_APPROVALS), {"p1": timeout_days})
+        async for row in result:
+            yield row[0]
+
     async def create_photo_approval(self, *, photo_id: uuid.UUID, user_id: uuid.UUID, decision: str) -> Optional[models.PhotoApproval]:
         row = (await self._conn.execute(sqlalchemy.text(CREATE_PHOTO_APPROVAL), {"p1": photo_id, "p2": user_id, "p3": decision})).first()
         if row is None:

db/queries/photo_approvals.sql

@@ -17,6 +17,23 @@ RETURNING *;
 -- name: GetPhotoApprovalsByPhotoId :many
 SELECT * FROM photo_approvals WHERE photo_id = $1;
 
+-- name: ExpireStaleApprovals :many
+WITH stale_photos AS (
+    SELECT id FROM photos
+    WHERE status = 'pending'
+      AND created_at < now() - make_interval(days => $1::int)
+),
+_update_approvals AS (
+    UPDATE photo_approvals
+    SET decision = 'approved', decided_at = now()
+    WHERE photo_id IN (SELECT id FROM stale_photos)
+      AND decision = 'pending'
+)
+UPDATE photos
+SET status = 'approved'
+WHERE id IN (SELECT id FROM stale_photos)
+RETURNING id;
+
 -- name: ListApprovalsByUserAndStatus :many
 SELECT * FROM photo_approvals
 WHERE user_id = $1

tests/unit/test_photo_approval_lifecycle.py

@@ -0,0 +1,166 @@
+import uuid
+from unittest.mock import AsyncMock, MagicMock
+import pytest
+
+from app.worker.photo_worker.main import PhotoWorker
+from app.worker.photo_worker.schema.event import PhotoProcessEvent
+from app.service.face_embedding import DetectedFace
+from app.service.photo_approval import PhotoApprovalService
+
+@pytest.fixture
+def mock_conn() -> AsyncMock:
+    return AsyncMock()
+
+@pytest.fixture
+def mock_face_embedding_service() -> AsyncMock:
+    return AsyncMock()
+
+@pytest.fixture
+def mock_single_face_service() -> AsyncMock:
+    return AsyncMock()
+
+@pytest.fixture
+def mock_notification_service() -> AsyncMock:
+    return AsyncMock()
+
+@pytest.fixture
+def mock_photo_face_querier() -> AsyncMock:
+    return AsyncMock()
+
+@pytest.fixture
+def mock_photo_querier() -> AsyncMock:
+    return AsyncMock()
+
+@pytest.fixture
+def mock_photo_approval_querier() -> AsyncMock:
+    return AsyncMock()
+
+@pytest.fixture
+def mock_processing_job_querier() -> AsyncMock:
+    return AsyncMock()
+
+@pytest.fixture
+def mock_staged_upload_storage_service() -> AsyncMock:
+    return AsyncMock()
+
+
+@pytest.mark.asyncio
+async def test_group_photo_auto_approve_no_users(
+    mock_conn: AsyncMock,
+    mock_face_embedding_service: AsyncMock,
+    mock_single_face_service: AsyncMock,
+    mock_notification_service: AsyncMock,
+    mock_photo_face_querier: AsyncMock,
+    mock_photo_querier: AsyncMock,
+    mock_processing_job_querier: AsyncMock,
+) -> None:
+    """
+    Test: Group photo with no enrolled users -> photo becomes approved + public.
+    """
+    worker = PhotoWorker(
+        conn=mock_conn,
+        face_embedding_service=mock_face_embedding_service,
+        single_face_service=mock_single_face_service,
+        user_notification_service=mock_notification_service,
+        photo_face_querier=mock_photo_face_querier,
+        photo_querier=mock_photo_querier,
+        processing_job_querier=mock_processing_job_querier,
+    )
+
+    photo_id = uuid.uuid4()
+    event = PhotoProcessEvent(photo_id=photo_id, image_ref="test.jpg")
+
+    # 2 faces detected
+    faces = [
+        DetectedFace(bbox=(0.0, 0.0, 10.0, 10.0), embedding=[0.1, 0.2]),
+        DetectedFace(bbox=(10.0, 10.0, 20.0, 20.0), embedding=[0.3, 0.4]),
+    ]
+
+    # No face matches any user -> insert_photo_face_with_approval returns None
+    mock_photo_face_querier.insert_photo_face_with_approval.return_value = None
+
+    await worker._handle_group_photo(event, faces)
+
+    # Verify notifications were NOT sent
+    mock_notification_service.create_notification.assert_not_called()
+
+    # Verify photo is marked public and approved
+    mock_photo_querier.update_photo_status.assert_called_once_with(id=photo_id, status="approved")
+    mock_photo_querier.update_photo_visibility.assert_called_once_with(id=photo_id, visibility="public")
+
+
+@pytest.mark.asyncio
+async def test_group_photo_pending_with_enrolled_users(
+    mock_conn: AsyncMock,
+    mock_face_embedding_service: AsyncMock,
+    mock_single_face_service: AsyncMock,
+    mock_notification_service: AsyncMock,
+    mock_photo_face_querier: AsyncMock,
+    mock_photo_querier: AsyncMock,
+    mock_processing_job_querier: AsyncMock,
+) -> None:
+    """
+    Test: Group photo with at least one enrolled user -> approval records created, notifications sent, photo stays pending.
+    """
+    worker = PhotoWorker(
+        conn=mock_conn,
+        face_embedding_service=mock_face_embedding_service,
+        single_face_service=mock_single_face_service,
+        user_notification_service=mock_notification_service,
+        photo_face_querier=mock_photo_face_querier,
+        photo_querier=mock_photo_querier,
+        processing_job_querier=mock_processing_job_querier,
+    )
+
+    photo_id = uuid.uuid4()
+    event = PhotoProcessEvent(photo_id=photo_id, image_ref="test.jpg")
+
+    faces = [
+        DetectedFace(bbox=(0.0, 0.0, 10.0, 10.0), embedding=[0.1, 0.2]),
+    ]
+
+    # Mock DB returning an approval record
+    mock_approval = MagicMock()
+    mock_approval.user_id = uuid.uuid4()
+    mock_approval.photo_id = photo_id
+    mock_photo_face_querier.insert_photo_face_with_approval.return_value = mock_approval
+
+    await worker._handle_group_photo(event, faces)
+
+    # Verify notification WAS sent
+    mock_notification_service.create_notification.assert_called_once()
+    args, kwargs = mock_notification_service.create_notification.call_args
+    assert kwargs["user_id"] == mock_approval.user_id
+
+    # Verify photo status is NOT updated to approved (stays pending)
+    mock_photo_querier.update_photo_status.assert_not_called()
+    mock_photo_querier.update_photo_visibility.assert_not_called()
+
+
+@pytest.mark.asyncio
+async def test_expire_stale_marks_photos_approved(
+    mock_photo_approval_querier: AsyncMock,
+    mock_photo_querier: AsyncMock,
+    mock_staged_upload_storage_service: AsyncMock,
+) -> None:
+    """
+    Test: After PHOTO_APPROVAL_TIMEOUT_DAYS days, expire_stale marks photos approved
+    """
+    service = PhotoApprovalService(
+        photo_approval_querier=mock_photo_approval_querier,
+        photo_querier=mock_photo_querier,
+        storage_service=mock_staged_upload_storage_service,
+    )
+
+    from typing import Any, AsyncIterator
+    # Mock the generator for expire_stale_approvals
+    async def mock_generator(*args: Any, **kwargs: Any) -> AsyncIterator[uuid.UUID]:
+        yield uuid.uuid4()
+        yield uuid.uuid4()
+
+    mock_photo_approval_querier.expire_stale_approvals = MagicMock(side_effect=mock_generator)
+
+    count = await service.expire_stale(timeout_days=7)
+
+    assert count == 2
+    mock_photo_approval_querier.expire_stale_approvals.assert_called_once_with(timeout_days=7)