Skip to content

Commit f0d8203

Browse files
Fix code review issues: N+1 query and bulk_update
Co-authored-by: alexeygrigorev <875246+alexeygrigorev@users.noreply.github.com>
1 parent 9046dbd commit f0d8203

2 files changed

Lines changed: 43 additions & 29 deletions

File tree

cadmin/views.py

Lines changed: 25 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -352,17 +352,15 @@ def project_submission_edit(request, course_slug, project_slug, submission_id):
352352
@staff_required
353353
def enrollments_list(request, course_slug):
354354
"""List all enrollments for a course"""
355-
course = get_object_or_404(Course, slug=course_slug)
355+
from django.db.models import Count
356356

357-
# Get all enrollments with related student data, ordered by leaderboard position
358-
enrollments = Enrollment.objects.filter(course=course).select_related('student').order_by(
359-
'position_on_leaderboard', 'id'
360-
)
357+
course = get_object_or_404(Course, slug=course_slug)
361358

362-
# Get submission counts for each enrollment
363-
for enrollment in enrollments:
364-
enrollment.homework_count = Submission.objects.filter(enrollment=enrollment).count()
365-
enrollment.project_count = ProjectSubmission.objects.filter(enrollment=enrollment).count()
359+
# Get all enrollments with related student data and submission counts, ordered by leaderboard position
360+
enrollments = Enrollment.objects.filter(course=course).select_related('student').annotate(
361+
homework_count=Count('submission', distinct=True),
362+
project_count=Count('projectsubmission', distinct=True)
363+
).order_by('position_on_leaderboard', 'id')
366364

367365
context = {
368366
"course": course,
@@ -390,7 +388,8 @@ def enrollment_edit(request, course_slug, enrollment_id):
390388
# If we're disabling, zero out all learning in public scores
391389
if enrollment.disable_learning_in_public:
392390
# Zero out homework learning in public scores
393-
homework_submissions = Submission.objects.filter(enrollment=enrollment)
391+
homework_submissions = list(Submission.objects.filter(enrollment=enrollment))
392+
submissions_to_update = []
394393
for submission in homework_submissions:
395394
if submission.learning_in_public_score > 0:
396395
submission.learning_in_public_score = 0
@@ -400,13 +399,17 @@ def enrollment_edit(request, course_slug, enrollment_id):
400399
submission.faq_score +
401400
submission.learning_in_public_score
402401
)
403-
Submission.objects.bulk_update(
404-
homework_submissions,
405-
['learning_in_public_score', 'total_score']
406-
)
402+
submissions_to_update.append(submission)
403+
404+
if submissions_to_update:
405+
Submission.objects.bulk_update(
406+
submissions_to_update,
407+
['learning_in_public_score', 'total_score']
408+
)
407409

408410
# Zero out project learning in public scores
409-
project_submissions = ProjectSubmission.objects.filter(enrollment=enrollment)
411+
project_submissions = list(ProjectSubmission.objects.filter(enrollment=enrollment))
412+
project_submissions_to_update = []
410413
for submission in project_submissions:
411414
if submission.project_learning_in_public_score > 0 or submission.peer_review_learning_in_public_score > 0:
412415
submission.project_learning_in_public_score = 0
@@ -419,10 +422,13 @@ def enrollment_edit(request, course_slug, enrollment_id):
419422
submission.peer_review_score +
420423
submission.peer_review_learning_in_public_score
421424
)
422-
ProjectSubmission.objects.bulk_update(
423-
project_submissions,
424-
['project_learning_in_public_score', 'peer_review_learning_in_public_score', 'total_score']
425-
)
425+
project_submissions_to_update.append(submission)
426+
427+
if project_submissions_to_update:
428+
ProjectSubmission.objects.bulk_update(
429+
project_submissions_to_update,
430+
['project_learning_in_public_score', 'peer_review_learning_in_public_score', 'total_score']
431+
)
426432

427433
messages.success(
428434
request,

courses/tests/test_disable_learning_in_public.py

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,8 @@ def test_zeroing_scores_when_disabling(self):
131131
self.enrollment1.save()
132132

133133
# Zero out homework learning in public scores
134-
homework_submissions = Submission.objects.filter(enrollment=self.enrollment1)
134+
homework_submissions = list(Submission.objects.filter(enrollment=self.enrollment1))
135+
submissions_to_update = []
135136
for submission in homework_submissions:
136137
if submission.learning_in_public_score > 0:
137138
submission.learning_in_public_score = 0
@@ -140,13 +141,17 @@ def test_zeroing_scores_when_disabling(self):
140141
submission.faq_score +
141142
submission.learning_in_public_score
142143
)
143-
Submission.objects.bulk_update(
144-
homework_submissions,
145-
['learning_in_public_score', 'total_score']
146-
)
144+
submissions_to_update.append(submission)
145+
146+
if submissions_to_update:
147+
Submission.objects.bulk_update(
148+
submissions_to_update,
149+
['learning_in_public_score', 'total_score']
150+
)
147151

148152
# Zero out project learning in public scores
149-
project_submissions = ProjectSubmission.objects.filter(enrollment=self.enrollment1)
153+
project_submissions = list(ProjectSubmission.objects.filter(enrollment=self.enrollment1))
154+
project_submissions_to_update = []
150155
for submission in project_submissions:
151156
if submission.project_learning_in_public_score > 0 or submission.peer_review_learning_in_public_score > 0:
152157
submission.project_learning_in_public_score = 0
@@ -158,10 +163,13 @@ def test_zeroing_scores_when_disabling(self):
158163
submission.peer_review_score +
159164
submission.peer_review_learning_in_public_score
160165
)
161-
ProjectSubmission.objects.bulk_update(
162-
project_submissions,
163-
['project_learning_in_public_score', 'peer_review_learning_in_public_score', 'total_score']
164-
)
166+
project_submissions_to_update.append(submission)
167+
168+
if project_submissions_to_update:
169+
ProjectSubmission.objects.bulk_update(
170+
project_submissions_to_update,
171+
['project_learning_in_public_score', 'peer_review_learning_in_public_score', 'total_score']
172+
)
165173

166174
# Verify scores are zeroed
167175
submission1.refresh_from_db()

0 commit comments

Comments
 (0)