Skip to content

Commit b8420f3

Browse files
pmcfadinclaude
andcommitted
fix: activity tracking persists on views-counter failure, passes video_id (closes #14)
- Remove early `return` from UNKNOWN_TABLE_COLUMNS/UNSUPPORTED_UPDATE_OPERATIONS branch so video_activity and user_activity inserts still run even when the views counter update is unavailable. - Pass `activity_id=video_id` to `record_user_activity` so each user_activity row links back to the video that was viewed. - Add `record_user_activity` patch + `assert_awaited_once` to `test_record_video_view_success` as a regression guard. - Update `test_record_video_view_authenticated_user_activity` and `test_record_video_view_anonymous_user_activity` assertions to include the new `activity_id=vid` kwarg. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 4610d55 commit b8420f3

2 files changed

Lines changed: 18 additions & 6 deletions

File tree

app/services/video_service.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -427,7 +427,7 @@ async def record_video_view(
427427
"UNSUPPORTED_UPDATE_OPERATIONS_FOR_TABLE"
428428
)
429429
_logged_views_disabled = True
430-
return # Gracefully no-op without breaking the API contract
430+
pass # Gracefully no-op for views counter; continue to activity tracking below
431431

432432
# Some deployments (Astra *tables*) currently reject $inc on bigint –
433433
# fall back to a manual read-modify-write cycle.
@@ -477,6 +477,7 @@ async def record_video_view(
477477
await record_user_activity(
478478
userid=effective_user_id,
479479
activity_type="view",
480+
activity_id=video_id,
480481
)
481482
except Exception:
482483
logger.warning("user_activity insert failed for view; ignoring", exc_info=True)

tests/services/test_video_service.py

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -236,9 +236,15 @@ async def test_record_video_view_success():
236236
# Mock the activity table returned via get_table
237237
mock_activity_table = AsyncMock()
238238

239-
with patch(
240-
"app.services.video_service.get_table", new_callable=AsyncMock
241-
) as mock_get_table:
239+
with (
240+
patch(
241+
"app.services.video_service.get_table", new_callable=AsyncMock
242+
) as mock_get_table,
243+
patch(
244+
"app.services.user_activity_service.record_user_activity",
245+
new_callable=AsyncMock,
246+
) as mock_record_user_activity,
247+
):
242248
# First call inside record_video_view is for VIDEO_PLAYBACK_STATS_TABLE_NAME
243249
# but we already pass mock_stats_table, so get_table will be used only once
244250
mock_get_table.return_value = mock_activity_table
@@ -253,6 +259,9 @@ async def test_record_video_view_success():
253259
# Validate activity table log
254260
mock_activity_table.insert_one.assert_called_once()
255261

262+
# Validate user activity was tracked (regression guard)
263+
mock_record_user_activity.assert_awaited_once()
264+
256265

257266
@pytest.mark.asyncio
258267
async def test_record_video_view_authenticated_user_activity():
@@ -277,10 +286,11 @@ async def test_record_video_view_authenticated_user_activity():
277286
vid, viewer_user_id=viewer_id, db_table=mock_stats_table
278287
)
279288

280-
# record_user_activity should be called with the real user ID
289+
# record_user_activity should be called with the real user ID and video_id
281290
mock_record_user_activity.assert_awaited_once_with(
282291
userid=viewer_id,
283292
activity_type="view",
293+
activity_id=vid,
284294
)
285295

286296

@@ -308,10 +318,11 @@ async def test_record_video_view_anonymous_user_activity():
308318
vid, viewer_user_id=None, db_table=mock_stats_table
309319
)
310320

311-
# record_user_activity should be called with the anonymous sentinel UUID
321+
# record_user_activity should be called with the anonymous sentinel UUID and video_id
312322
mock_record_user_activity.assert_awaited_once_with(
313323
userid=ANONYMOUS_USER_ID,
314324
activity_type="view",
325+
activity_id=vid,
315326
)
316327

317328

0 commit comments

Comments
 (0)