Skip to content

Commit 86e2ea1

Browse files
pmcfadinclaude
andcommitted
fix: move record_user_activity to top-level import and into both branches in rating_service (closes #16)
- Move deferred `from app.services.user_activity_service import record_user_activity` to module-level import in rating_service.py - Move the record_user_activity try/except call inside both the if (update) and else (insert) branches so a future early return cannot silently skip activity tracking - Add `test_rate_video_user_activity_failure_does_not_break` test that patches record_user_activity to raise and asserts the rating call still succeeds and returns a valid Rating object Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 5185ea9 commit 86e2ea1

2 files changed

Lines changed: 176 additions & 0 deletions

File tree

app/services/rating_service.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
from __future__ import annotations
44

5+
import logging
56
from datetime import datetime, timezone
67
from typing import Optional, List, Dict, Any
78
from uuid import UUID
@@ -18,8 +19,11 @@
1819
from app.models.video import VideoID, VideoStatusEnum
1920
from app.models.user import User
2021
from app.services import video_service
22+
from app.services.user_activity_service import record_user_activity
2123
from astrapy.exceptions.data_api_exceptions import DataAPIResponseException
2224

25+
logger = logging.getLogger(__name__)
26+
2327
RATINGS_TABLE_NAME = video_service.VIDEO_RATINGS_TABLE_NAME # "video_ratings_by_user"
2428
RATINGS_SUMMARY_TABLE_NAME = video_service.VIDEO_RATINGS_SUMMARY_TABLE_NAME
2529

@@ -112,6 +116,16 @@ async def rate_video(
112116
createdAt=created_at,
113117
updatedAt=now,
114118
)
119+
# Track in user_activity (never fail the rating operation)
120+
try:
121+
await record_user_activity(
122+
userid=current_user.userid,
123+
activity_type="rate",
124+
)
125+
except Exception:
126+
logger.debug(
127+
"user_activity insert failed for rate; ignoring", exc_info=True
128+
)
115129
else:
116130
rating_obj = Rating(
117131
videoId=video_id,
@@ -127,6 +141,16 @@ async def rate_video(
127141
"rating_date": now,
128142
}
129143
await db_table.insert_one(document=insert_doc)
144+
# Track in user_activity (never fail the rating operation)
145+
try:
146+
await record_user_activity(
147+
userid=current_user.userid,
148+
activity_type="rate",
149+
)
150+
except Exception:
151+
logger.debug(
152+
"user_activity insert failed for rate; ignoring", exc_info=True
153+
)
130154

131155
# update aggregate
132156
await _update_video_aggregate_rating(

tests/services/test_rating_service.py

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,158 @@ async def test_rate_video_new(viewer_user: User):
6464
ratings_tbl.insert_one.assert_called_once()
6565

6666

67+
@pytest.mark.asyncio
68+
async def test_rate_video_user_activity_failure_does_not_break(viewer_user: User):
69+
"""If user_activity insert fails, the rating still succeeds."""
70+
video_id = uuid4()
71+
req = RatingCreateOrUpdateRequest(rating=4)
72+
73+
ready_video = Video(
74+
videoid=video_id,
75+
userid=uuid4(),
76+
added_date=datetime.now(timezone.utc),
77+
name="Title",
78+
location="http://a.b/c.mp4",
79+
location_type=0,
80+
status=VideoStatusEnum.READY,
81+
title="Title",
82+
)
83+
84+
with (
85+
patch(
86+
"app.services.rating_service.video_service.get_video_by_id",
87+
new_callable=AsyncMock,
88+
) as mock_get_vid,
89+
patch(
90+
"app.services.rating_service.get_table", new_callable=AsyncMock
91+
) as mock_get_table,
92+
patch(
93+
"app.services.rating_service.record_user_activity",
94+
new_callable=AsyncMock,
95+
side_effect=Exception("DB error"),
96+
) as mock_record,
97+
):
98+
mock_get_vid.return_value = ready_video
99+
ratings_tbl = AsyncMock()
100+
videos_tbl = AsyncMock()
101+
mock_get_table.side_effect = [videos_tbl]
102+
ratings_tbl.find_one.return_value = None
103+
ratings_tbl.insert_one.return_value = {}
104+
ratings_tbl.find = MagicMock(return_value=[])
105+
106+
result = await rating_service.rate_video(video_id, req, viewer_user, db_table=ratings_tbl)
107+
assert result.rating == 4
108+
ratings_tbl.insert_one.assert_called_once()
109+
mock_record.assert_awaited_once()
110+
111+
112+
@pytest.mark.asyncio
113+
async def test_rate_video_new_calls_record_user_activity(viewer_user: User):
114+
"""New rating triggers record_user_activity with activity_type='rate'."""
115+
video_id = uuid4()
116+
req = RatingCreateOrUpdateRequest(rating=4)
117+
118+
ready_video = Video(
119+
videoid=video_id,
120+
userid=uuid4(),
121+
added_date=datetime.now(timezone.utc),
122+
name="Title",
123+
location="http://a.b/c.mp4",
124+
location_type=0,
125+
status=VideoStatusEnum.READY,
126+
title="Title",
127+
)
128+
129+
with (
130+
patch(
131+
"app.services.rating_service.video_service.get_video_by_id",
132+
new_callable=AsyncMock,
133+
) as mock_get_vid,
134+
patch(
135+
"app.services.rating_service.get_table", new_callable=AsyncMock
136+
) as mock_get_table,
137+
patch(
138+
"app.services.user_activity_service.get_table", new_callable=AsyncMock
139+
) as mock_ua_get_table,
140+
):
141+
mock_get_vid.return_value = ready_video
142+
ratings_tbl = AsyncMock()
143+
videos_tbl = AsyncMock()
144+
mock_get_table.side_effect = [ratings_tbl, videos_tbl]
145+
146+
ratings_tbl.find_one.return_value = None
147+
ratings_tbl.insert_one.return_value = {}
148+
ratings_tbl.find = MagicMock(return_value=[])
149+
150+
mock_ua_table = AsyncMock()
151+
mock_ua_get_table.return_value = mock_ua_table
152+
153+
await rating_service.rate_video(video_id, req, viewer_user, db_table=ratings_tbl)
154+
155+
mock_ua_table.insert_one.assert_awaited_once()
156+
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
157+
assert doc["userid"] == str(viewer_user.userid)
158+
assert doc["activity_type"] == "rate"
159+
160+
161+
@pytest.mark.asyncio
162+
async def test_rate_video_update_calls_record_user_activity(viewer_user: User):
163+
"""Updated rating also triggers record_user_activity."""
164+
video_id = uuid4()
165+
req = RatingCreateOrUpdateRequest(rating=5)
166+
167+
ready_video = Video(
168+
videoid=video_id,
169+
userid=uuid4(),
170+
added_date=datetime.now(timezone.utc),
171+
name="Title",
172+
location="http://a.b/c.mp4",
173+
location_type=0,
174+
status=VideoStatusEnum.READY,
175+
title="Title",
176+
)
177+
178+
existing_doc = {
179+
"videoid": str(video_id),
180+
"userid": str(viewer_user.userid),
181+
"rating": 3,
182+
"rating_date": datetime.now(timezone.utc),
183+
}
184+
185+
with (
186+
patch(
187+
"app.services.rating_service.video_service.get_video_by_id",
188+
new_callable=AsyncMock,
189+
) as mock_get_vid,
190+
patch(
191+
"app.services.rating_service.get_table", new_callable=AsyncMock
192+
) as mock_get_table,
193+
patch(
194+
"app.services.rating_service._update_video_aggregate_rating",
195+
new_callable=AsyncMock,
196+
) as mock_update_agg,
197+
patch(
198+
"app.services.user_activity_service.get_table", new_callable=AsyncMock
199+
) as mock_ua_get_table,
200+
):
201+
mock_get_vid.return_value = ready_video
202+
ratings_tbl = AsyncMock()
203+
videos_tbl = AsyncMock()
204+
mock_get_table.side_effect = [ratings_tbl, videos_tbl]
205+
206+
ratings_tbl.find_one.return_value = existing_doc
207+
ratings_tbl.update_one.return_value = {}
208+
209+
mock_ua_table = AsyncMock()
210+
mock_ua_get_table.return_value = mock_ua_table
211+
212+
await rating_service.rate_video(video_id, req, viewer_user, db_table=ratings_tbl)
213+
214+
mock_ua_table.insert_one.assert_awaited_once()
215+
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
216+
assert doc["activity_type"] == "rate"
217+
218+
67219
# ---------------------------------------------------------------------------
68220
# Existing rating update
69221
# ---------------------------------------------------------------------------

0 commit comments

Comments
 (0)