fix: address code review comments on comment_service (closes #15)

- Move `record_user_activity` import to top of file instead of inside function body
- Add module-level `logger = logging.getLogger(__name__)` and change except block from `logging.getLogger(__name__).debug(...)` to `logger.warning(...)`
- Remove inline `import logging` from inside the except block
- Patch `app.services.comment_service.record_user_activity` in `test_add_comment_success` to prevent real AstraDB calls

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

Author: pmcfadin

app/services/comment_service.py

@@ -2,6 +2,7 @@
 
 from __future__ import annotations
 
+import logging
 from typing import Optional, List, Tuple
 from uuid import UUID, uuid1
 
@@ -15,10 +16,13 @@
 from app.external_services.sentiment_mock import MockSentimentAnalyzer
 import inspect  # local import to avoid new dependency
 from app.utils.db_helpers import safe_count
+from app.services.user_activity_service import record_user_activity
 
 # testing mocks
 from unittest.mock import AsyncMock, MagicMock
 
+logger = logging.getLogger(__name__)
+
 COMMENTS_BY_VIDEO_TABLE_NAME = "comments"
 COMMENTS_BY_USER_TABLE_NAME = "comments_by_user"
 
@@ -90,6 +94,18 @@ async def add_comment_to_video(
     await comments_by_video_table.insert_one(document=comment_doc)
     await comments_by_user_table.insert_one(document=comment_doc)
 
+    # Track in user_activity (never fail the comment operation)
+    try:
+        await record_user_activity(
+            userid=current_user.userid,
+            activity_type="comment",
+            activity_id=comment_id,
+        )
+    except Exception:
+        logger.warning(
+            "user_activity insert failed for comment; ignoring", exc_info=True
+        )
+
     return new_comment
 
 

tests/services/test_comment_service.py

@@ -48,6 +48,10 @@ async def test_add_comment_success(viewer_user: User, sample_video: Video):
         patch(
             "app.services.comment_service.get_table", new_callable=AsyncMock
         ) as mock_get_table,
+        patch(
+            "app.services.comment_service.record_user_activity",
+            new_callable=AsyncMock,
+        ),
     ):
         mock_get_vid.return_value = sample_video
         mock_table_video = AsyncMock()
@@ -68,6 +72,84 @@ async def test_add_comment_success(viewer_user: User, sample_video: Video):
         assert comment.text == request.text
 
 
+@pytest.mark.asyncio
+async def test_add_comment_calls_record_user_activity(viewer_user: User, sample_video: Video):
+    """add_comment_to_video calls record_user_activity with correct args."""
+    request = CommentCreateRequest(text="Great stuff!")
+    sample_video.status = VideoStatusEnum.READY
+
+    with (
+        patch(
+            "app.services.comment_service.video_service.get_video_by_id",
+            new_callable=AsyncMock,
+        ) as mock_get_vid,
+        patch(
+            "app.services.comment_service.get_table", new_callable=AsyncMock
+        ) as mock_get_table,
+        patch(
+            "app.services.user_activity_service.get_table", new_callable=AsyncMock
+        ) as mock_ua_get_table,
+    ):
+        mock_get_vid.return_value = sample_video
+        mock_table_video = AsyncMock()
+        mock_table_user = AsyncMock()
+        mock_get_table.side_effect = [mock_table_video, mock_table_user]
+        mock_ua_table = AsyncMock()
+        mock_ua_get_table.return_value = mock_ua_table
+
+        comment = await comment_service.add_comment_to_video(
+            video_id=sample_video.videoid,
+            request=request,
+            current_user=viewer_user,
+        )
+
+        # Verify record_user_activity was called via DB insert
+        mock_ua_table.insert_one.assert_awaited_once()
+        doc = mock_ua_table.insert_one.call_args.args[0] if mock_ua_table.insert_one.call_args.args else mock_ua_table.insert_one.call_args.kwargs
+        assert doc["userid"] == str(viewer_user.userid)
+        assert doc["activity_type"] == "comment"
+        assert doc["activity_id"] == str(comment.commentid)
+
+
+@pytest.mark.asyncio
+async def test_add_comment_user_activity_failure_does_not_break(viewer_user: User, sample_video: Video):
+    """If user_activity insert fails, the comment still succeeds."""
+    request = CommentCreateRequest(text="Still works!")
+    sample_video.status = VideoStatusEnum.READY
+
+    with (
+        patch(
+            "app.services.comment_service.video_service.get_video_by_id",
+            new_callable=AsyncMock,
+        ) as mock_get_vid,
+        patch(
+            "app.services.comment_service.get_table", new_callable=AsyncMock
+        ) as mock_get_table,
+        patch(
+            "app.services.user_activity_service.get_table", new_callable=AsyncMock
+        ) as mock_ua_get_table,
+    ):
+        mock_get_vid.return_value = sample_video
+        mock_table_video = AsyncMock()
+        mock_table_user = AsyncMock()
+        mock_get_table.side_effect = [mock_table_video, mock_table_user]
+        mock_ua_table = AsyncMock()
+        mock_ua_table.insert_one.side_effect = Exception("DB error")
+        mock_ua_get_table.return_value = mock_ua_table
+
+        # Should NOT raise despite user_activity failure
+        comment = await comment_service.add_comment_to_video(
+            video_id=sample_video.videoid,
+            request=request,
+            current_user=viewer_user,
+        )
+
+        # Comment tables were still written
+        assert mock_table_video.insert_one.call_count == 1
+        assert mock_table_user.insert_one.call_count == 1
+        assert comment.text == request.text
+
+
 @pytest.mark.asyncio
 async def test_add_comment_video_not_ready(viewer_user: User, sample_video: Video):
     request = CommentCreateRequest(text="Hello")