Skip to content

Commit c6eaa18

Browse files
Address code review feedback: cache dictionaries instead of model instances
Co-authored-by: alexeygrigorev <875246+alexeygrigorev@users.noreply.github.com>
1 parent 783ed2e commit c6eaa18

3 files changed

Lines changed: 25 additions & 10 deletions

File tree

courses/tests/test_course.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ class CourseDetailViewTests(TestCase):
3030
def setUp(self):
3131
# Clear cache before each test to ensure fresh state
3232
cache.clear()
33-
33+
3434
self.client = Client()
3535

3636
self.user = User.objects.create_user(**credentials)
@@ -317,7 +317,7 @@ def test_leaderboard_order(self):
317317
self.enrollment.display_name,
318318
]
319319

320-
actual_order = [e.display_name for e in enrollments]
320+
actual_order = [e['display_name'] for e in enrollments]
321321

322322
self.assertEqual(actual_order, expected_order)
323323

@@ -353,13 +353,13 @@ def test_new_enrollment_at_the_end_of_leaderboard(self):
353353
e5.display_name,
354354
]
355355

356-
actual_order = [e.display_name for e in enrollments]
356+
actual_order = [e['display_name'] for e in enrollments]
357357

358358
self.assertEqual(actual_order, expected_order)
359359

360360
expected_positions = [1, 2, 3, 4, None, None]
361361
actual_positions = [
362-
e.position_on_leaderboard for e in enrollments
362+
e['position_on_leaderboard'] for e in enrollments
363363
]
364364
self.assertEqual(actual_positions, expected_positions)
365365

@@ -394,7 +394,7 @@ def test_not_enrolled_yet_but_leaderboard_displays(self):
394394

395395
# Verify the order is correct
396396
expected_order = ["e1", "e2", "e3", "e4", "e5"]
397-
actual_order = [e.display_name for e in enrollments]
397+
actual_order = [e['display_name'] for e in enrollments]
398398
self.assertEqual(actual_order, expected_order)
399399

400400
def test_not_enrolled_but_can_edit_details(self):

courses/tests/test_leaderboard.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,9 @@ def submit_homework(self, homework, enrollment, score):
5959
)
6060

6161
def setUp(self):
62+
# Clear cache before each test to ensure fresh state
63+
cache.clear()
64+
6265
self.course = Course.objects.create(
6366
slug="test-course",
6467
title="Test Course",

courses/views/course.py

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -261,25 +261,37 @@ def leaderboard_view(request, course_slug: str):
261261

262262
# Try to get enrollments from cache
263263
cache_key = f"leaderboard:{course.id}"
264-
enrollments = cache.get(cache_key)
264+
enrollments_data = cache.get(cache_key)
265265

266-
if enrollments is None:
266+
if enrollments_data is None:
267267
logger.info(f"Cache miss for leaderboard of course {course.slug}")
268268
# Cache miss, fetch from database
269269
enrollments = list(
270-
Enrollment.objects.filter(course=course).order_by(
270+
Enrollment.objects.filter(course=course)
271+
.select_related('student')
272+
.order_by(
271273
Coalesce("position_on_leaderboard", Value(999999)),
272274
"id",
273275
)
274276
)
277+
# Store as list of dictionaries to avoid stale model instances
278+
enrollments_data = [
279+
{
280+
'id': e.id,
281+
'display_name': e.display_name,
282+
'total_score': e.total_score,
283+
'position_on_leaderboard': e.position_on_leaderboard,
284+
}
285+
for e in enrollments
286+
]
275287
# Cache for 1 hour (3600 seconds)
276288
# The cache will be invalidated when leaderboard is recalculated
277-
cache.set(cache_key, enrollments, 3600)
289+
cache.set(cache_key, enrollments_data, 3600)
278290
else:
279291
logger.info(f"Cache hit for leaderboard of course {course.slug}")
280292

281293
context = {
282-
"enrollments": enrollments,
294+
"enrollments": enrollments_data,
283295
"course": course,
284296
"current_student_enrollment": current_student_enrollment,
285297
"current_student_enrollment_id": current_student_enrollment_id,

0 commit comments

Comments
 (0)