fix: wrap video_activity in try/except, move import, fix log level, fix tests (closes #14)

- Wrap video_activity insert in its own try/except so a DB failure there
  never kills the 204 response after the views counter already succeeded
- Move import of record_user_activity/ANONYMOUS_USER_ID outside the try
  block so ImportError is not silently swallowed
- Upgrade logger.debug to logger.warning for both non-critical insert
  failures for production visibility
- Update user_activity tests to patch record_user_activity directly
  instead of patching get_table — tests the contract, not implementation
- Fix test_list_latest_videos: assert page_size=10 (was 3) to match call
- Fix test_search_videos_by_keyword: patch search_videos_by_semantic
  (keyword search now delegates there) instead of list_videos_with_query

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Author: pmcfadin

app/services/video_service.py

@@ -383,6 +383,7 @@ async def update_video_details(
 
 async def record_video_view(
     video_id: VideoID,
+    viewer_user_id: Optional[UUID] = None,
     db_table: Optional[AstraDBCollection] = None,
 ) -> None:
     """Increment the view counter stored directly in the *videos* table.
@@ -449,19 +450,37 @@ async def record_video_view(
         else:
             raise
 
-    # Log individual view event in the time-series activity table (unchanged)
-    activity_table = await get_table(VIDEO_ACTIVITY_TABLE_NAME)
-    now_utc = datetime.now(timezone.utc)
-    day_partition = now_utc.strftime("%Y-%m-%d")  # Cassandra date literal format
-
-    await activity_table.insert_one(
-        {
-            "videoid": _uuid_for_db(video_id, db_table),
-            "day": day_partition,
-            "watch_time": str(uuid1()),  # time-based UUID for clustering order
-        }
+    from app.services.user_activity_service import (
+        record_user_activity,
+        ANONYMOUS_USER_ID,
     )
 
+    # Log individual view event in the time-series activity table (non-critical)
+    try:
+        activity_table = await get_table(VIDEO_ACTIVITY_TABLE_NAME)
+        now_utc = datetime.now(timezone.utc)
+        day_partition = now_utc.strftime("%Y-%m-%d")  # Cassandra date literal format
+
+        await activity_table.insert_one(
+            {
+                "videoid": _uuid_for_db(video_id, db_table),
+                "day": day_partition,
+                "watch_time": str(uuid1()),  # time-based UUID for clustering order
+            }
+        )
+    except Exception:
+        logger.warning("video_activity insert failed for view; ignoring", exc_info=True)
+
+    # Track in user_activity (never fail the view operation)
+    try:
+        effective_user_id = viewer_user_id if viewer_user_id else ANONYMOUS_USER_ID
+        await record_user_activity(
+            userid=effective_user_id,
+            activity_type="view",
+        )
+    except Exception:
+        logger.warning("user_activity insert failed for view; ignoring", exc_info=True)
+
 
 async def list_videos_with_query(
     query_filter: Dict[str, Any],

tests/services/test_video_service.py

@@ -243,7 +243,7 @@ async def test_record_video_view_success():
         # but we already pass mock_stats_table, so get_table will be used only once
         mock_get_table.return_value = mock_activity_table
 
-        await video_service.record_video_view(vid, mock_stats_table)
+        await video_service.record_video_view(vid, db_table=mock_stats_table)
 
         # Validate stats table increment
         mock_stats_table.update_one.assert_called_once_with(
@@ -254,6 +254,95 @@ async def test_record_video_view_success():
         mock_activity_table.insert_one.assert_called_once()
 
 
+@pytest.mark.asyncio
+async def test_record_video_view_authenticated_user_activity():
+    """Authenticated view calls record_user_activity with real user ID."""
+    vid = uuid4()
+    viewer_id = uuid4()
+    mock_stats_table = AsyncMock()
+    mock_activity_table = AsyncMock()
+
+    with (
+        patch(
+            "app.services.video_service.get_table", new_callable=AsyncMock
+        ) as mock_get_table,
+        patch(
+            "app.services.user_activity_service.record_user_activity",
+            new_callable=AsyncMock,
+        ) as mock_record_user_activity,
+    ):
+        mock_get_table.return_value = mock_activity_table
+
+        await video_service.record_video_view(
+            vid, viewer_user_id=viewer_id, db_table=mock_stats_table
+        )
+
+        # record_user_activity should be called with the real user ID
+        mock_record_user_activity.assert_awaited_once_with(
+            userid=viewer_id,
+            activity_type="view",
+        )
+
+
+@pytest.mark.asyncio
+async def test_record_video_view_anonymous_user_activity():
+    """Anonymous view calls record_user_activity with nil UUID sentinel."""
+    from app.services.user_activity_service import ANONYMOUS_USER_ID
+
+    vid = uuid4()
+    mock_stats_table = AsyncMock()
+    mock_activity_table = AsyncMock()
+
+    with (
+        patch(
+            "app.services.video_service.get_table", new_callable=AsyncMock
+        ) as mock_get_table,
+        patch(
+            "app.services.user_activity_service.record_user_activity",
+            new_callable=AsyncMock,
+        ) as mock_record_user_activity,
+    ):
+        mock_get_table.return_value = mock_activity_table
+
+        await video_service.record_video_view(
+            vid, viewer_user_id=None, db_table=mock_stats_table
+        )
+
+        # record_user_activity should be called with the anonymous sentinel UUID
+        mock_record_user_activity.assert_awaited_once_with(
+            userid=ANONYMOUS_USER_ID,
+            activity_type="view",
+        )
+
+
+@pytest.mark.asyncio
+async def test_record_video_view_user_activity_failure_does_not_break():
+    """If record_user_activity raises, the video view still succeeds."""
+    vid = uuid4()
+    mock_stats_table = AsyncMock()
+    mock_activity_table = AsyncMock()
+
+    with (
+        patch(
+            "app.services.video_service.get_table", new_callable=AsyncMock
+        ) as mock_get_table,
+        patch(
+            "app.services.user_activity_service.record_user_activity",
+            new_callable=AsyncMock,
+        ) as mock_record_user_activity,
+    ):
+        mock_get_table.return_value = mock_activity_table
+        mock_record_user_activity.side_effect = Exception("DB error")
+
+        # Should NOT raise despite user_activity failure
+        await video_service.record_video_view(
+            vid, viewer_user_id=uuid4(), db_table=mock_stats_table
+        )
+
+        # video_activity insert still happened
+        mock_activity_table.insert_one.assert_called_once()
+
+
 # ------------------------------------------------------------
 # list_latest_videos (delegate to generic) – just verify query call
 # ------------------------------------------------------------
@@ -276,7 +365,7 @@ async def test_list_latest_videos():
         mock_list_with_query.assert_called_once_with(
             {},
             1,
-            3,
+            10,
             sort_options={"added_date": -1},
             db_table=mock_db,
             source_table_name=video_service.VIDEOS_TABLE_NAME,
@@ -293,20 +382,18 @@ async def test_list_latest_videos():
 @pytest.mark.asyncio
 async def test_search_videos_by_keyword():
     mock_db = AsyncMock()
-    mock_db.find.return_value = []
-    mock_db.count_documents.return_value = 0
 
     with patch(
-        "app.services.video_service.list_videos_with_query",
+        "app.services.video_service.search_videos_by_semantic",
         new_callable=AsyncMock,
-    ) as mock_list_with_query:
-        mock_list_with_query.return_value = ([], 0)
+    ) as mock_semantic:
+        mock_semantic.return_value = ([], 0)
 
         summaries, total = await video_service.search_videos_by_keyword(
             query="test", page=1, page_size=10, db_table=mock_db
         )
 
-        mock_list_with_query.assert_called_once()
+        mock_semantic.assert_called_once()
         assert summaries == []
         assert total == 0