perf: speed up the CCX grade report and fix its CSV output

lms/djangoapps/ccx/tests/test_views.py

@@ -25,6 +25,7 @@
 from common.djangoapps.student.models import CourseEnrollment, CourseEnrollmentAllowed
 from common.djangoapps.student.roles import CourseCcxCoachRole, CourseInstructorRole, CourseStaffRole
 from common.djangoapps.student.tests.factories import AdminFactory, CourseEnrollmentFactory, UserFactory
+from common.djangoapps.util.file import course_filename_prefix_generator
 from lms.djangoapps.ccx.models import CustomCourseForEdX
 from lms.djangoapps.ccx.overrides import get_override_for_ccx, override_field_for_ccx
 from lms.djangoapps.ccx.tests.factories import CcxFactory
@@ -36,7 +37,7 @@
 from lms.djangoapps.courseware.tests.helpers import LoginEnrollmentTestCase
 from lms.djangoapps.courseware.testutils import FieldOverrideTestMixin
 from lms.djangoapps.discussion.django_comment_client.utils import has_forum_access
-from lms.djangoapps.grades.api import task_compute_all_grades_for_course
+from lms.djangoapps.grades.api import prefetch_course_and_subsection_grades, task_compute_all_grades_for_course
 from lms.djangoapps.instructor.access import allow_access, list_with_level
 from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
 from openedx.core.djangoapps.django_comment_common.models import FORUM_ROLE_ADMINISTRATOR
@@ -1050,9 +1051,20 @@ def test_grades_csv(self):
         )
         response = self.client.get(url)
         assert response.status_code == 200
-        # Are the grades downloaded as an attachment?
-        assert response['content-disposition'] == 'attachment'
-        rows = response.content.decode('utf-8').strip().split('\r')
+        # The grades download as an attachment named after the course, matching
+        # the naming used by the asynchronous course grade report.
+        expected_prefix = course_filename_prefix_generator(self.ccx_key)
+        assert re.match(
+            rf'attachment; filename="{re.escape(expected_prefix)}_grade_report_'
+            r'\d{4}-\d{2}-\d{2}-\d{4}\.csv"$',
+            response['content-disposition']
+        )
+        body = response.content.decode('utf-8')
+        # Emails and usernames are written as plain text, not as a bytes repr.
+        assert "b'" not in body
+        assert self.student.email in body
+        assert self.student2.username in body
+        rows = body.strip().split('\r')
         headers = rows[0]
         # picking first student records
         data = dict(list(zip(headers.strip().split(','), rows[1].strip().split(','))))  # noqa: B905
@@ -1062,6 +1074,32 @@ def test_grades_csv(self):
         assert data['HW 03'] == '0.25'
         assert data['HW Avg'] == '0.5'
 
+    def test_grades_csv_prefetches_persisted_grades(self):
+        """
+        The report bulk-prefetches persisted course/subsection grades once for
+        the whole class instead of reading them one student at a time (N+1).
+        """
+        self.course.enable_ccx = True
+        RequestCache.clear_all_namespaces()
+
+        url = reverse(
+            'ccx_grades_csv',
+            kwargs={'course_id': self.ccx_key}
+        )
+        with patch(
+            'lms.djangoapps.ccx.views.prefetch_course_and_subsection_grades',
+            wraps=prefetch_course_and_subsection_grades,
+        ) as mock_prefetch:
+            response = self.client.get(url)
+
+        assert response.status_code == 200
+        # A single bulk prefetch is issued for the CCX, covering every enrolled student.
+        mock_prefetch.assert_called_once()
+        called_course_key, called_users = mock_prefetch.call_args[0]
+        assert called_course_key == self.ccx_key
+        prefetched_ids = {user.id for user in called_users}
+        assert {self.student.id, self.student2.id} <= prefetched_ids
+
     @patch('lms.djangoapps.courseware.views.views.render_to_response', intercept_renderer)
     def test_student_progress(self):
         self.course.enable_ccx = True

lms/djangoapps/ccx/views.py

@@ -26,6 +26,7 @@
 from common.djangoapps.edxmako.shortcuts import render_to_response
 from common.djangoapps.student.models import CourseEnrollment
 from common.djangoapps.student.roles import CourseCcxCoachRole
+from common.djangoapps.util.file import course_filename_prefix_generator
 from lms.djangoapps.ccx.models import CustomCourseForEdX
 from lms.djangoapps.ccx.overrides import (
     bulk_delete_ccx_override_fields,
@@ -47,13 +48,13 @@
     parse_date,
 )
 from lms.djangoapps.courseware.field_overrides import disable_overrides
-from lms.djangoapps.grades.api import CourseGradeFactory
+from lms.djangoapps.grades.api import CourseGradeFactory, prefetch_course_and_subsection_grades
 from lms.djangoapps.instructor.enrollment import enroll_email, get_email_params
 from lms.djangoapps.instructor.views.gradebook_api import get_grade_book_page
 from openedx.core.djangoapps.django_comment_common.models import FORUM_ROLE_ADMINISTRATOR, assign_role
 from openedx.core.djangoapps.django_comment_common.utils import seed_permissions_roles
 from openedx.core.lib.courses import get_course_by_id
-from xmodule.modulestore.django import SignalHandler  # pylint: disable=wrong-import-order
+from xmodule.modulestore.django import SignalHandler, modulestore  # pylint: disable=wrong-import-order
 
 log = logging.getLogger(__name__)
 TODAY = datetime.datetime.today  # for patching in tests
@@ -520,41 +521,47 @@ def ccx_grades_csv(request, course, ccx=None):
     ccx_key = CCXLocator.from_course_locator(course.id, str(ccx.id))
     with ccx_course(ccx_key) as course:  # pylint: disable=redefined-argument-from-local
 
-        enrolled_students = User.objects.filter(
+        enrolled_students = list(User.objects.filter(
             courseenrollment__course_id=ccx_key,
             courseenrollment__is_active=1
-        ).order_by('username').select_related("profile")
-        grades = CourseGradeFactory().iter(enrolled_students, course)
+        ).order_by('username').select_related("profile"))
 
         header = None
         rows = []
-        for student, course_grade, __ in grades:
-            if course_grade:
-                # We were able to successfully grade this student for this
-                # course.
-                if not header:
-                    # Encode the header row in utf-8 encoding in case there are
-                    # unicode characters
-                    header = [section['label'] for section in course_grade.summary['section_breakdown']]
-                    rows.append(["id", "email", "username", "grade"] + header)
-
-                percents = {
-                    section['label']: section.get('percent', 0.0)
-                    for section in course_grade.summary['section_breakdown']
-                    if 'label' in section
-                }
-
-                row_percents = [percents.get(label, 0.0) for label in header]
-                rows.append([student.id, student.email.encode('utf-8'),
-                             student.username.encode('utf-8'),
-                             course_grade.percent] + row_percents)
+        # Bulk-read the persisted course and subsection grades for the whole
+        # class in a single pass instead of reading them once per student,
+        # mirroring the asynchronous CourseGradeReport.
+        with modulestore().bulk_operations(ccx_key):
+            prefetch_course_and_subsection_grades(ccx_key, enrolled_students)
+            grades = CourseGradeFactory().iter(enrolled_students, course)
+
+            for student, course_grade, __ in grades:
+                if course_grade:
+                    # We were able to successfully grade this student for this
+                    # course.
+                    if not header:
+                        header = [section['label'] for section in course_grade.summary['section_breakdown']]
+                        rows.append(["id", "email", "username", "grade"] + header)
+
+                    percents = {
+                        section['label']: section.get('percent', 0.0)
+                        for section in course_grade.summary['section_breakdown']
+                        if 'label' in section
+                    }
+
+                    row_percents = [percents.get(label, 0.0) for label in header]
+                    rows.append([student.id, student.email,
+                                 student.username,
+                                 course_grade.percent] + row_percents)
 
         buf = StringIO()
         writer = csv.writer(buf)
         for row in rows:
             writer.writerow(row)
 
         response = HttpResponse(buf.getvalue(), content_type='text/csv')
-        response['Content-Disposition'] = 'attachment'
+        timestamp = datetime.datetime.now(pytz.UTC).strftime('%Y-%m-%d-%H%M')
+        report_name = f'{course_filename_prefix_generator(ccx_key)}_grade_report_{timestamp}.csv'
+        response['Content-Disposition'] = f'attachment; filename="{report_name}"'
 
         return response