diff --git a/lms/djangoapps/grades/api/v1/tests/test_views.py b/lms/djangoapps/grades/api/v1/tests/test_views.py index 38063958fc..d296a40223 100644 --- a/lms/djangoapps/grades/api/v1/tests/test_views.py +++ b/lms/djangoapps/grades/api/v1/tests/test_views.py @@ -15,6 +15,7 @@ from rest_framework import status from rest_framework.test import APITestCase from six import text_type +from course_modes.models import CourseMode from lms.djangoapps.courseware.tests.factories import GlobalStaffFactory from lms.djangoapps.grades.api.v1.views import CourseGradesView from lms.djangoapps.grades.config.waffle import waffle_flags, WRITABLE_GRADEBOOK @@ -23,6 +24,7 @@ from lms.djangoapps.grades.course_grade import CourseGrade from lms.djangoapps.grades.models import PersistentSubsectionGrade from lms.djangoapps.grades.subsection_grade import ReadSubsectionGrade from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory +from openedx.core.djangoapps.course_groups.tests.helpers import CohortFactory from openedx.core.djangoapps.user_authn.tests.utils import AuthAndScopesTestMixin from openedx.core.djangoapps.waffle_utils.testutils import override_waffle_flag from student.tests.factories import CourseEnrollmentFactory, UserFactory @@ -621,6 +623,80 @@ class GradebookViewTest(GradebookViewTestBase): ]), ] + def _assert_data_all_users(self, response): + """ + Helper method to assert that self.student and self.other_student + have the expected gradebook data. + """ + expected_results = [ + OrderedDict([ + ('course_id', text_type(self.course.id)), + ('email', self.student.email), + ('user_id', self.student.id), + ('username', self.student.username), + ('full_name', self.student.get_full_name()), + ('passed', True), + ('percent', 0.85), + ('letter_grade', 'A'), + ('progress_page_url', reverse( + 'student_progress', + kwargs=dict(course_id=text_type(self.course.id), student_id=self.student.id) + )), + ('section_breakdown', self.expected_subsection_grades(letter_grade='A')), + ('aggregates', { + 'Lab': { + 'score_earned': 2.0, + 'score_possible': 4.0, + }, + 'Homework': { + 'score_earned': 2.0, + 'score_possible': 4.0, + }, + }), + ]), + OrderedDict([ + ('course_id', text_type(self.course.id)), + ('email', self.other_student.email), + ('user_id', self.other_student.id), + ('username', self.other_student.username), + ('full_name', self.other_student.get_full_name()), + ('passed', False), + ('percent', 0.45), + ('letter_grade', None), + ('progress_page_url', reverse( + 'student_progress', + kwargs=dict(course_id=text_type(self.course.id), student_id=self.other_student.id) + )), + ('section_breakdown', self.expected_subsection_grades()), + ('aggregates', { + 'Lab': { + 'score_earned': 2.0, + 'score_possible': 4.0, + }, + 'Homework': { + 'score_earned': 2.0, + 'score_possible': 4.0, + }, + }), + ]), + ] + + self.assertEqual(status.HTTP_200_OK, response.status_code) + actual_data = dict(response.data) + self.assertIsNone(actual_data['next']) + self.assertIsNone(actual_data['previous']) + self.assertEqual(expected_results, actual_data['results']) + + def _assert_empty_response(self, response): + """ + Helper method for assertions about OK, empty responses. + """ + self.assertEqual(status.HTTP_200_OK, response.status_code) + actual_data = dict(response.data) + self.assertIsNone(actual_data['next']) + self.assertIsNone(actual_data['previous']) + self.assertEqual([], actual_data['results']) + def test_feature_not_enabled(self): self.client.login(username=self.global_staff.username, password=self.password) with override_waffle_flag(self.waffle_flag, active=False): @@ -670,13 +746,7 @@ class GradebookViewTest(GradebookViewTestBase): resp = self.client.get( self.get_url(course_key=self.empty_course.id) ) - expected_data = { - 'next': None, - 'previous': None, - 'results': [], - } - self.assertEqual(status.HTTP_200_OK, resp.status_code) - self.assertEqual(expected_data, dict(resp.data)) + self._assert_empty_response(resp) def test_gradebook_data_for_course(self): with patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.read') as mock_grade: @@ -690,64 +760,7 @@ class GradebookViewTest(GradebookViewTestBase): resp = self.client.get( self.get_url(course_key=self.course.id) ) - expected_results = [ - OrderedDict([ - ('course_id', text_type(self.course.id)), - ('email', self.student.email), - ('user_id', self.student.id), - ('username', self.student.username), - ('full_name', self.student.get_full_name()), - ('passed', True), - ('percent', 0.85), - ('letter_grade', 'A'), - ('progress_page_url', reverse( - 'student_progress', - kwargs=dict(course_id=text_type(self.course.id), student_id=self.student.id) - )), - ('section_breakdown', self.expected_subsection_grades(letter_grade='A')), - ('aggregates', { - 'Lab': { - 'score_earned': 2.0, - 'score_possible': 4.0, - }, - 'Homework': { - 'score_earned': 2.0, - 'score_possible': 4.0, - }, - }), - ]), - OrderedDict([ - ('course_id', text_type(self.course.id)), - ('email', self.other_student.email), - ('user_id', self.other_student.id), - ('username', self.other_student.username), - ('full_name', self.other_student.get_full_name()), - ('passed', False), - ('percent', 0.45), - ('letter_grade', None), - ('progress_page_url', reverse( - 'student_progress', - kwargs=dict(course_id=text_type(self.course.id), student_id=self.other_student.id) - )), - ('section_breakdown', self.expected_subsection_grades()), - ('aggregates', { - 'Lab': { - 'score_earned': 2.0, - 'score_possible': 4.0, - }, - 'Homework': { - 'score_earned': 2.0, - 'score_possible': 4.0, - }, - }), - ]), - ] - - self.assertEqual(status.HTTP_200_OK, resp.status_code) - actual_data = dict(resp.data) - self.assertIsNone(actual_data['next']) - self.assertIsNone(actual_data['previous']) - self.assertEqual(expected_results, actual_data['results']) + self._assert_data_all_users(resp) def test_gradebook_data_for_single_learner(self): with patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.read') as mock_grade: @@ -844,14 +857,102 @@ class GradebookViewTest(GradebookViewTestBase): resp = self.client.get( self.get_url(course_key=self.course.id, username_contains='fooooooooooooooooo') ) + self._assert_empty_response(resp) + + def test_filter_cohort_id(self): + with patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.read') as mock_grade: + mock_grade.return_value = self.mock_course_grade(self.student, passed=True, letter_grade='A', percent=0.85) + + cohort = CohortFactory(course_id=self.course.id, name="TestCohort", users=[self.student]) + with override_waffle_flag(self.waffle_flag, active=True): + self.login_staff() + resp = self.client.get( + self.get_url(course_key=self.course.id) + '?cohort_id={}'.format(cohort.id) + ) + + expected_results = [ + OrderedDict([ + ('course_id', text_type(self.course.id)), + ('email', self.student.email), + ('user_id', self.student.id), + ('username', self.student.username), + ('full_name', self.student.get_full_name()), + ('passed', True), + ('percent', 0.85), + ('letter_grade', 'A'), + ('progress_page_url', reverse( + 'student_progress', + kwargs=dict(course_id=text_type(self.course.id), student_id=self.student.id) + )), + ('section_breakdown', self.expected_subsection_grades(letter_grade='A')), + ('aggregates', { + 'Lab': { + 'score_earned': 2.0, + 'score_possible': 4.0, + }, + 'Homework': { + 'score_earned': 2.0, + 'score_possible': 4.0, + }, + }), + ]), + ] - expected_results = [] self.assertEqual(status.HTTP_200_OK, resp.status_code) actual_data = dict(resp.data) self.assertIsNone(actual_data['next']) self.assertIsNone(actual_data['previous']) self.assertEqual(expected_results, actual_data['results']) + def test_filter_cohort_id_does_not_exist(self): + with patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.read') as mock_grade: + mock_grade.return_value = self.mock_course_grade(self.student, passed=True, letter_grade='A', percent=0.85) + + empty_cohort = CohortFactory(course_id=self.course.id, name="TestCohort", users=[]) + with override_waffle_flag(self.waffle_flag, active=True): + self.login_staff() + resp = self.client.get( + self.get_url(course_key=self.course.id) + '?cohort_id={}'.format(empty_cohort.id) + ) + self._assert_empty_response(resp) + + def test_filter_enrollment_mode(self): + with patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.read') as mock_grade: + mock_grade.side_effect = [ + self.mock_course_grade(self.student, passed=True, letter_grade='A', percent=0.85), + self.mock_course_grade(self.other_student, passed=False, letter_grade=None, percent=0.45), + ] + + # Enroll a verified student, for whom data should not be returned. + verified_student = UserFactory() + _ = CourseEnrollmentFactory( + course_id=self.course.id, + user=verified_student, + created=datetime(2013, 1, 1, tzinfo=UTC), + mode=CourseMode.VERIFIED, + ) + with override_waffle_flag(self.waffle_flag, active=True): + self.login_staff() + resp = self.client.get( + self.get_url(course_key=self.course.id) + '?enrollment_mode={}'.format(CourseMode.AUDIT) + ) + + self._assert_data_all_users(resp) + + def test_filter_enrollment_mode_no_students(self): + with patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.read') as mock_grade: + mock_grade.side_effect = [ + self.mock_course_grade(self.student, passed=True, letter_grade='A', percent=0.85), + self.mock_course_grade(self.other_student, passed=False, letter_grade=None, percent=0.45), + ] + + with override_waffle_flag(self.waffle_flag, active=True): + self.login_staff() + resp = self.client.get( + self.get_url(course_key=self.course.id) + '?enrollment_mode={}'.format(CourseMode.VERIFIED) + ) + self._assert_empty_response(resp) + class GradebookBulkUpdateViewTest(GradebookViewTestBase): """ diff --git a/lms/djangoapps/grades/api/v1/views.py b/lms/djangoapps/grades/api/v1/views.py index a3a7747779..323cfd3b1e 100644 --- a/lms/djangoapps/grades/api/v1/views.py +++ b/lms/djangoapps/grades/api/v1/views.py @@ -30,6 +30,7 @@ from lms.djangoapps.grades.tasks import recalculate_subsection_grade_v3, are_gra from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey, UsageKey from openedx.core.djangoapps.content.course_overviews.models import CourseOverview +from openedx.core.djangoapps.course_groups import cohorts from openedx.core.lib.api.authentication import OAuth2AuthenticationAllowInactiveUser from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin from student.models import CourseEnrollment @@ -188,12 +189,13 @@ class GradeViewMixin(DeveloperErrorViewMixin): course_grade = CourseGradeFactory().read(grade_user, course_key=course_key) return Response([self._serialize_user_grade(grade_user, course_key, course_grade)]) - def _iter_user_grades(self, course_key, course_enrollment_filter=None): + def _iter_user_grades(self, course_key, course_enrollment_filter=None, related_models=None): """ Args: course_key (CourseLocator): The course to retrieve grades for. course_enrollment_filter: Optional dictionary of keyword arguments to pass to `CourseEnrollment.filter()`. + related_models: Optional list of related models to join to the CourseEnrollment table. Returns: An iterator of CourseGrade objects for users enrolled in the given course. @@ -204,6 +206,8 @@ class GradeViewMixin(DeveloperErrorViewMixin): } filter_kwargs.update(course_enrollment_filter or {}) enrollments_in_course = CourseEnrollment.objects.filter(**filter_kwargs) + if related_models: + enrollments_in_course = enrollments_in_course.select_related(*related_models) paged_enrollments = self.paginate_queryset(enrollments_in_course) users = (enrollment.user for enrollment in paged_enrollments) @@ -378,17 +382,23 @@ class GradebookView(GradeViewMixin, GenericAPIView): """ **Use Case** * Get course gradebook entries of a single user in a course, - or of all users who are enrolled in a course. The currently logged-in user may request + or of all users who are actively enrolled in a course. The currently logged-in user may request all enrolled user's grades information if they are allowed. **Example Request** GET /api/grades/v1/gradebook/{course_id}/ - Get gradebook entries for all users in course GET /api/grades/v1/gradebook/{course_id}/?username={username} - Get grades for specific user in course GET /api/grades/v1/gradebook/{course_id}/?username_contains={username_contains} + GET /api/grades/v1/gradebook/{course_id}/?cohort_id={cohort_id} + GET /api/grades/v1/gradebook/{course_id}/?enrollment_mode={enrollment_mode} **GET Parameters** A GET request may include the following query parameters. * username: (optional) A string representation of a user's username. * username_contains: (optional) A substring against which a case-insensitive substring filter will be performed on the USER_MODEL.username field. + * cohort_id: (optional) The id of a cohort in this course. If present, will return grades + only for course enrollees who belong to that cohort. + * enrollment_mode: (optional) The slug of an enrollment mode (e.g. "verified"). If present, will return grades + only for course enrollees with the given enrollment mode. **GET Response Values** If the request for gradebook data is successful, an HTTP 200 "OK" response is returned. @@ -586,13 +596,21 @@ class GradebookView(GradeViewMixin, GenericAPIView): serializer = StudentGradebookEntrySerializer(entry) return Response(serializer.data) else: + filter_kwargs = {} + related_models = [] if request.GET.get('username_contains'): - users = USER_MODEL.objects.filter(username__icontains=request.GET.get('username_contains')) - filter_kwargs = {'user__in': users} - user_grades = self._iter_user_grades(course_key, filter_kwargs) - else: - # list gradebook data for all course enrollees - user_grades = self._iter_user_grades(course_key) + filter_kwargs = {'user__username__icontains': request.GET.get('username_contains')} + related_models.append('user') + elif request.GET.get('cohort_id'): + cohort = cohorts.get_cohort_by_id(course_key, request.GET.get('cohort_id')) + if cohort: + filter_kwargs = {'user__in': cohort.users.all()} + else: + filter_kwargs = {'user__in': []} + elif request.GET.get('enrollment_mode'): + filter_kwargs = {'mode': request.GET.get('enrollment_mode')} + + user_grades = self._iter_user_grades(course_key, filter_kwargs, related_models) entries = [] for user, course_grade, exc in user_grades: