From 12c502b35f3ff78dabc8cd57afc2552ab3d2f3c0 Mon Sep 17 00:00:00 2001 From: Alex Dusenbery Date: Tue, 13 Nov 2018 14:19:51 -0500 Subject: [PATCH] Change gradebook GET endpoint to allow filtering by multiple attributes. --- lms/djangoapps/grades/api/v1/tests/test_views.py | 7 +++++-- lms/djangoapps/grades/api/v1/views.py | 12 ++++++------ lms/djangoapps/mobile_api/users/tests.py | 12 ++++++------ 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/lms/djangoapps/grades/api/v1/tests/test_views.py b/lms/djangoapps/grades/api/v1/tests/test_views.py index 366599b140..15022ba104 100644 --- a/lms/djangoapps/grades/api/v1/tests/test_views.py +++ b/lms/djangoapps/grades/api/v1/tests/test_views.py @@ -859,15 +859,18 @@ class GradebookViewTest(GradebookViewTestBase): ) self._assert_empty_response(resp) - def test_filter_cohort_id(self): + def test_filter_cohort_id_and_enrollment_mode(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() + # both of our test users are in the audit track, so this is functionally equivalent + # to just `?cohort_id=cohort.id`. + query = '?cohort_id={}&enrollment_mode={}'.format(cohort.id, CourseMode.AUDIT) resp = self.client.get( - self.get_url(course_key=self.course.id) + '?cohort_id={}'.format(cohort.id) + self.get_url(course_key=self.course.id) + query ) expected_results = [ diff --git a/lms/djangoapps/grades/api/v1/views.py b/lms/djangoapps/grades/api/v1/views.py index 92277aef59..fa7eb01e6d 100644 --- a/lms/djangoapps/grades/api/v1/views.py +++ b/lms/djangoapps/grades/api/v1/views.py @@ -633,16 +633,16 @@ class GradebookView(GradeViewMixin, PaginatedAPIView): filter_kwargs = {} related_models = [] if request.GET.get('username_contains'): - filter_kwargs = {'user__username__icontains': request.GET.get('username_contains')} + filter_kwargs['user__username__icontains'] = request.GET.get('username_contains') related_models.append('user') - elif request.GET.get('cohort_id'): + if 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()} + 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')} + filter_kwargs['user__in'] = [] + if 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) diff --git a/lms/djangoapps/mobile_api/users/tests.py b/lms/djangoapps/mobile_api/users/tests.py index b017ad4ac7..dd16ade8e8 100644 --- a/lms/djangoapps/mobile_api/users/tests.py +++ b/lms/djangoapps/mobile_api/users/tests.py @@ -16,9 +16,9 @@ from mock import patch from lms.djangoapps.certificates.api import generate_user_certificates from lms.djangoapps.certificates.models import CertificateStatuses from lms.djangoapps.certificates.tests.factories import GeneratedCertificateFactory +from lms.djangoapps.grades.tests.utils import mock_passing_grade from course_modes.models import CourseMode from courseware.access_response import MilestoneAccessError, StartDateError, VisibilityError -from lms.djangoapps.grades.tests.utils import mock_passing_grade from mobile_api.testutils import ( MobileAPITestCase, MobileAuthTestMixin, @@ -277,9 +277,9 @@ class TestUserEnrollmentApi(UrlResetMixin, MobileAPITestCase, MobileAuthUserTest if api_version == API_V05: if num_courses_returned: - self.assertFalse('audit_access_expires' in courses[0]) + self.assertNotIn('audit_access_expires', courses[0]) else: - self.assertTrue('audit_access_expires' in courses[0]) + self.assertIn('audit_access_expires', courses[0]) self.assertIsNotNone(courses[0].get('audit_access_expires')) @ddt.data( @@ -452,7 +452,7 @@ class TestCourseStatusPATCH(CourseStatusAPITestCase, MobileAuthUserTestMixin, """ Tests for PATCH of /api/mobile/v0.5/users//course_status_info/{course_id} """ - def url_method(self, url, **kwargs): + def url_method(self, url, **kwargs): # pylint: disable=arguments-differ # override implementation to use PATCH method. return self.client.patch(url, data=kwargs.get('data', None)) @@ -586,10 +586,10 @@ class TestCourseEnrollmentSerializer(MobileAPITestCase, MilestonesTestCaseMixin) based on version of api being used ''' if api_version != API_V05: - self.assertTrue('audit_access_expires' in response) + self.assertIn('audit_access_expires', response) self.assertIsNotNone(response.get('audit_access_expires')) else: - self.assertFalse('audit_access_expires' in response) + self.assertNotIn('audit_access_expires', response) @ddt.data(API_V05, API_V1) def test_success(self, api_version):