From b528b8e5d720dc6b377cfdbbc9acea3d6d8544b4 Mon Sep 17 00:00:00 2001
From: Alex Dusenbery <adusenbery@edx.org>
Date: Fri, 26 Oct 2018 11:47:44 -0400
Subject: [PATCH] EDUCATOR-3622 | Add a username_contains query param to
gradebook endpoint
---
.../grades/api/v1/tests/test_views.py | 82 +++++++++++++++++--
lms/djangoapps/grades/api/v1/views.py | 41 ++++++----
.../docs/decisions/0001-gradebook-api.rst | 6 +-
3 files changed, 105 insertions(+), 24 deletions(-)
diff --git a/lms/djangoapps/grades/api/v1/tests/test_views.py b/lms/djangoapps/grades/api/v1/tests/test_views.py
index 46993c5f409..1f26c206434 100644
--- a/lms/djangoapps/grades/api/v1/tests/test_views.py
+++ b/lms/djangoapps/grades/api/v1/tests/test_views.py
@@ -96,8 +96,8 @@ class GradeViewTestMixin(SharedModuleStoreTestCase):
super(GradeViewTestMixin, self).setUp()
self.password = 'test'
self.global_staff = GlobalStaffFactory.create()
- self.student = UserFactory(password=self.password)
- self.other_student = UserFactory(password=self.password)
+ self.student = UserFactory(password=self.password, username='student')
+ self.other_student = UserFactory(password=self.password, username='other_student')
self._create_user_enrollments(self.student, self.other_student)
@classmethod
@@ -459,13 +459,15 @@ class GradebookViewTest(GradebookViewTestBase):
"""
Tests for the gradebook view.
"""
- def get_url(self, course_key=None, username=None): # pylint: disable=arguments-differ
+ def get_url(self, course_key=None, username=None, username_contains=None): # pylint: disable=arguments-differ
"""
Helper function to create the course gradebook API read url.
"""
base_url = super(GradebookViewTest, self).get_url(course_key)
if username:
return "{0}?username={1}".format(base_url, username)
+ if username_contains:
+ return "{0}?username_contains={1}".format(base_url, username_contains)
return base_url
def mock_subsection_grade(self, subsection, **kwargs):
@@ -786,6 +788,70 @@ class GradebookViewTest(GradebookViewTestBase):
actual_data = dict(resp.data)
self.assertEqual(expected_results, actual_data)
+ def test_gradebook_data_filter_username_contains(self):
+ with patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.read') as mock_grade:
+ mock_grade.return_value = self.mock_course_grade(
+ self.other_student, passed=True, letter_grade='A', percent=0.85
+ )
+
+ with override_waffle_flag(self.waffle_flag, active=True):
+ self.login_staff()
+ resp = self.client.get(
+ self.get_url(course_key=self.course.id, username_contains='other')
+ )
+ expected_results = [
+ 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', 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.other_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,
+ },
+ }),
+ ]),
+ ]
+
+ 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_gradebook_data_filter_username_contains_no_match(self):
+ with patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.read') as mock_grade:
+ mock_grade.return_value = self.mock_course_grade(
+ self.other_student, passed=True, letter_grade='A', percent=0.85
+ )
+
+ with override_waffle_flag(self.waffle_flag, active=True):
+ self.login_staff()
+ resp = self.client.get(
+ self.get_url(course_key=self.course.id, username_contains='fooooooooooooooooo')
+ )
+
+ 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'])
+
class GradebookBulkUpdateViewTest(GradebookViewTestBase):
"""
@@ -850,7 +916,7 @@ class GradebookBulkUpdateViewTest(GradebookViewTestBase):
},
]
self.assertEqual(status.HTTP_422_UNPROCESSABLE_ENTITY, resp.status_code)
- self.assertEqual(expected_data, json.loads(resp.data))
+ self.assertEqual(expected_data, resp.data)
def test_user_does_not_exist(self):
with override_waffle_flag(self.waffle_flag, active=True):
@@ -878,7 +944,7 @@ class GradebookBulkUpdateViewTest(GradebookViewTestBase):
},
]
self.assertEqual(status.HTTP_422_UNPROCESSABLE_ENTITY, resp.status_code)
- self.assertEqual(expected_data, json.loads(resp.data))
+ self.assertEqual(expected_data, resp.data)
def test_invalid_usage_key(self):
with override_waffle_flag(self.waffle_flag, active=True):
@@ -906,7 +972,7 @@ class GradebookBulkUpdateViewTest(GradebookViewTestBase):
},
]
self.assertEqual(status.HTTP_422_UNPROCESSABLE_ENTITY, resp.status_code)
- self.assertEqual(expected_data, json.loads(resp.data))
+ self.assertEqual(expected_data, resp.data)
def test_subsection_does_not_exist(self):
"""
@@ -939,7 +1005,7 @@ class GradebookBulkUpdateViewTest(GradebookViewTestBase):
},
]
self.assertEqual(status.HTTP_422_UNPROCESSABLE_ENTITY, resp.status_code)
- self.assertEqual(expected_data, json.loads(resp.data))
+ self.assertEqual(expected_data, resp.data)
def test_override_is_created(self):
"""
@@ -992,7 +1058,7 @@ class GradebookBulkUpdateViewTest(GradebookViewTestBase):
},
]
self.assertEqual(status.HTTP_202_ACCEPTED, resp.status_code)
- self.assertEqual(expected_data, json.loads(resp.data))
+ self.assertEqual(expected_data, resp.data)
second_post_data = [
{
diff --git a/lms/djangoapps/grades/api/v1/views.py b/lms/djangoapps/grades/api/v1/views.py
index 4744223287a..98f0b4b7584 100644
--- a/lms/djangoapps/grades/api/v1/views.py
+++ b/lms/djangoapps/grades/api/v1/views.py
@@ -1,5 +1,4 @@
""" API v0 views. """
-import json
import logging
from collections import defaultdict, namedtuple
from contextlib import contextmanager
@@ -128,12 +127,13 @@ class GradeViewMixin(DeveloperErrorViewMixin):
USER_MODEL.DoesNotExist if no such user exists.
CourseEnrollment.DoesNotExist if the user is not enrolled in the given course.
"""
+ # May raise USER_MODEL.DoesNotExist if no user matching the given query exists.
if user_id:
- # May raise USER_MODEL.DoesNotExist if no user with this id exists
grade_user = USER_MODEL.objects.get(id=user_id)
+ elif 'username' in request.GET:
+ grade_user = USER_MODEL.objects.get(username=request.GET.get('username'))
else:
- username = request.GET.get('username') or request.user.username
- grade_user = USER_MODEL.objects.get(username=username)
+ grade_user = request.user
# May raise CourseEnrollment.DoesNotExist if no enrollment exists for this user/course.
_ = CourseEnrollment.objects.get(user=grade_user, course_id=course_key)
@@ -181,18 +181,22 @@ 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):
+ def _iter_user_grades(self, course_key, course_enrollment_filter=None):
"""
Args:
course_key (CourseLocator): The course to retrieve grades for.
+ course_enrollment_filter: Optional dictionary of keyword arguments to pass
+ to `CourseEnrollment.filter()`.
Returns:
An iterator of CourseGrade objects for users enrolled in the given course.
"""
- enrollments_in_course = CourseEnrollment.objects.filter(
- course_id=course_key,
- is_active=True
- )
+ filter_kwargs = {
+ 'course_id': course_key,
+ 'is_active': True,
+ }
+ filter_kwargs.update(course_enrollment_filter or {})
+ enrollments_in_course = CourseEnrollment.objects.filter(**filter_kwargs)
paged_enrollments = self.paginate_queryset(enrollments_in_course)
users = (enrollment.user for enrollment in paged_enrollments)
@@ -372,9 +376,12 @@ class GradebookView(GradeViewMixin, GenericAPIView):
**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 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.
**GET Response Values**
If the request for gradebook data is successful,
an HTTP 200 "OK" response is returned.
@@ -561,11 +568,10 @@ class GradebookView(GradeViewMixin, GenericAPIView):
request: A Django request object.
course_id: A string representation of a CourseKey object.
"""
- username = request.GET.get('username')
course_key = get_course_key(request, course_id)
course = get_course_with_access(request.user, 'staff', course_key, depth=None)
- if username:
+ if request.GET.get('username'):
with self._get_user_or_raise(request, course_key) as grade_user:
course_grade = CourseGradeFactory().read(grade_user, course)
@@ -573,9 +579,16 @@ class GradebookView(GradeViewMixin, GenericAPIView):
serializer = StudentGradebookEntrySerializer(entry)
return Response(serializer.data)
else:
- # list gradebook data for all course enrollees
+ 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)
+
entries = []
- for user, course_grade, exc in self._iter_user_grades(course_key):
+ for user, course_grade, exc in user_grades:
if not exc:
entries.append(self._gradebook_entry(user, course, course_grade))
serializer = StudentGradebookEntrySerializer(entries, many=True)
@@ -735,7 +748,7 @@ class GradebookBulkUpdateView(GradeViewMixin, GenericAPIView):
status_code = status.HTTP_202_ACCEPTED
return Response(
- json.dumps([item._asdict() for item in result]),
+ [item._asdict() for item in result],
status=status_code,
content_type='application/json'
)
diff --git a/lms/djangoapps/grades/docs/decisions/0001-gradebook-api.rst b/lms/djangoapps/grades/docs/decisions/0001-gradebook-api.rst
index 7f7f7a5fe55..9b930c48653 100644
--- a/lms/djangoapps/grades/docs/decisions/0001-gradebook-api.rst
+++ b/lms/djangoapps/grades/docs/decisions/0001-gradebook-api.rst
@@ -28,8 +28,10 @@ Decisions
#. **The read (GET) API**
- a. The read API supports either fetching subsection scores for a single user, by `username`, or fetching
- a paginated result of subsection grade data for all enrollees in the requested course.
+ a. The read API supports either fetching subsection scores for a single user via ``?username=my-user-name``,
+ where we look up a user by their exact ``username`` value; via ``?username_contains=name-substring`` where
+ we do a case-insensitive substring query for a user, or fetching a paginated result of
+ subsection grade data for all enrollees in the requested course.
b. We will use the data schema required by the EE's front-end implementation. This will allow us to port
over much of EE's front-end code with only minor modifications. Note that there are some fields specified
--
GitLab