From 05c0510cd3cf3173ff2f9945a04bccc71f118a1e Mon Sep 17 00:00:00 2001
From: Michael Roytman <mroytman@edx.org>
Date: Fri, 7 Jun 2019 11:18:26 -0400
Subject: [PATCH] add external_user_key to response and add ability to search
by username, email, or external user key
Code review comments - EDUCATOR-4319
undoing changes temporarily
undoing changes temporarily
Fixed exception handling
re-added changes after hard reset
removed waffle flag (wrong merge
removed waffle flag (wrong merge
---
cms/djangoapps/contentstore/config/waffle.py | 11 --
common/djangoapps/student/models.py | 20 ++++
lms/djangoapps/grades/rest_api/serializers.py | 2 +
.../grades/rest_api/v1/gradebook_views.py | 35 ++++--
.../grades/rest_api/v1/tests/mixins.py | 39 ++++++-
.../rest_api/v1/tests/test_gradebook_views.py | 106 ++++++++++++++++--
.../grades/rest_api/v1/tests/test_views.py | 8 ++
lms/djangoapps/grades/rest_api/v1/utils.py | 15 +--
8 files changed, 197 insertions(+), 39 deletions(-)
diff --git a/cms/djangoapps/contentstore/config/waffle.py b/cms/djangoapps/contentstore/config/waffle.py
index 27d984309be..6457e77b03e 100644
--- a/cms/djangoapps/contentstore/config/waffle.py
+++ b/cms/djangoapps/contentstore/config/waffle.py
@@ -10,17 +10,6 @@ WAFFLE_NAMESPACE = u'studio'
# Switches
ENABLE_ACCESSIBILITY_POLICY_PAGE = u'enable_policy_page'
-# Global dictionary to store proctoring provider specific waffle flags
-REVIEW_RULES_PER_PROCTORING_PROVIDER = {}
-
-def create_review_rules_for_provider_waffle_flag(provider_name):
- name_format = u'show_review_rules_for'
- new_flag = CourseWaffleFlag(
- waffle_namespace=waffle_flags(),
- flag_name=u'show_review_rules',
- flag_undefined_default=False
- )
- return new_flag
def waffle():
"""
diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py
index 8919100d2f5..2a5fd003188 100644
--- a/common/djangoapps/student/models.py
+++ b/common/djangoapps/student/models.py
@@ -1244,6 +1244,26 @@ class CourseEnrollment(models.Model):
except cls.DoesNotExist:
return None
+ @classmethod
+ def get_program_enrollment(cls, user, course_id):
+ """
+ Return the ProgramEnrollment associated with the CourseEnrollment specified
+ by the user and course_id.
+ Return None if there is no ProgramEnrollment.
+
+ Arguments:
+ user (User): the user for whom we want the program enrollment
+ coure_id (CourseKey): the id of the course the user has a course enrollment in
+
+ Returns:
+ ProgramEnrollment object or None
+ """
+ try:
+ course_enrollment = cls.objects.get(user=user, course_id=course_id)
+ return course_enrollment.programcourseenrollment.program_enrollment
+ except (ObjectDoesNotExist):
+ return None
+
@classmethod
def is_enrollment_closed(cls, user, course):
"""
diff --git a/lms/djangoapps/grades/rest_api/serializers.py b/lms/djangoapps/grades/rest_api/serializers.py
index a86cf1b43b6..6de2617000b 100644
--- a/lms/djangoapps/grades/rest_api/serializers.py
+++ b/lms/djangoapps/grades/rest_api/serializers.py
@@ -50,6 +50,8 @@ class StudentGradebookEntrySerializer(serializers.Serializer):
"""
user_id = serializers.IntegerField()
username = serializers.CharField()
+ email = serializers.EmailField()
+ external_user_key = serializers.CharField(required=False)
percent = serializers.FloatField()
section_breakdown = SectionBreakdownSerializer(many=True)
diff --git a/lms/djangoapps/grades/rest_api/v1/gradebook_views.py b/lms/djangoapps/grades/rest_api/v1/gradebook_views.py
index 63743ddeef8..2c0175b47c5 100644
--- a/lms/djangoapps/grades/rest_api/v1/gradebook_views.py
+++ b/lms/djangoapps/grades/rest_api/v1/gradebook_views.py
@@ -9,6 +9,7 @@ from contextlib import contextmanager
from functools import wraps
import six
+from django.db.models import Q
from django.urls import reverse
from opaque_keys import InvalidKeyError
from opaque_keys.edx.keys import CourseKey, UsageKey
@@ -332,12 +333,15 @@ class GradebookView(GradeViewMixin, PaginatedAPIView):
**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}/?user_contains={user_contains}
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.
+ * user_contains: (optional) A substring against which a case-insensitive substring filter will be performed
+ on the USER_MODEL.username, or the USER_MODEL.email, or the PROGRAM_ENROLLMENT.external_user_key fields.
* 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
@@ -484,8 +488,17 @@ class GradebookView(GradeViewMixin, PaginatedAPIView):
user_entry['user_id'] = user.id
user_entry['full_name'] = user.get_full_name()
+ external_user_key = self._get_external_user_key(user, course.id)
+ if external_user_key:
+ user_entry['external_user_key'] = external_user_key
+
return user_entry
+ @staticmethod
+ def _get_external_user_key(user, course_id):
+ program_enrollment = CourseEnrollment.get_program_enrollment(user, course_id)
+ return getattr(program_enrollment, 'external_user_key', None)
+
@verify_course_exists
@verify_writable_gradebook_enabled
@course_author_access_required
@@ -515,22 +528,28 @@ class GradebookView(GradeViewMixin, PaginatedAPIView):
serializer = StudentGradebookEntrySerializer(entry)
return Response(serializer.data)
else:
- filter_kwargs = {}
- related_models = []
+ q_objects = []
+ if request.GET.get('user_contains'):
+ search_term = request.GET.get('user_contains')
+ q_objects.append(
+ Q(user__username__icontains=search_term) |
+ Q(programcourseenrollment__program_enrollment__external_user_key__icontains=search_term) |
+ Q(user__email__icontains=search_term)
+ )
if request.GET.get('username_contains'):
- filter_kwargs['user__username__icontains'] = request.GET.get('username_contains')
- related_models.append('user')
+ q_objects.append(Q(user__username__icontains=request.GET.get('username_contains')))
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()
+ q_objects.append(Q(user__in=cohort.users.all()))
else:
- filter_kwargs['user__in'] = []
+ q_objects.append(Q(user__in=[]))
if request.GET.get('enrollment_mode'):
- filter_kwargs['mode'] = request.GET.get('enrollment_mode')
+ q_objects.append(Q(mode=request.GET.get('enrollment_mode')))
entries = []
- users = self._paginate_users(course_key, filter_kwargs, related_models)
+ related_models = ['user']
+ users = self._paginate_users(course_key, q_objects, related_models)
with bulk_gradebook_view_context(course_key, users):
for user, course_grade, exc in CourseGradeFactory().iter(
diff --git a/lms/djangoapps/grades/rest_api/v1/tests/mixins.py b/lms/djangoapps/grades/rest_api/v1/tests/mixins.py
index c4576f13cbb..2db3d389491 100644
--- a/lms/djangoapps/grades/rest_api/v1/tests/mixins.py
+++ b/lms/djangoapps/grades/rest_api/v1/tests/mixins.py
@@ -9,6 +9,7 @@ from pytz import UTC
from six.moves import range
from lms.djangoapps.courseware.tests.factories import GlobalStaffFactory
+from lms.djangoapps.program_enrollments.tests.factories import ProgramEnrollmentFactory, ProgramCourseEnrollmentFactory
from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory
from student.tests.factories import CourseEnrollmentFactory, UserFactory
from xmodule.modulestore.tests.django_utils import TEST_DATA_SPLIT_MODULESTORE, SharedModuleStoreTestCase
@@ -59,7 +60,7 @@ class GradeViewTestMixin(SharedModuleStoreTestCase):
@classmethod
def setUpClass(cls):
super(GradeViewTestMixin, cls).setUpClass()
-
+ cls.date = datetime(2013, 1, 22, tzinfo=UTC)
cls.course = cls._create_test_course_with_default_grading_policy(
display_name='test course', run="Testing_course"
)
@@ -69,21 +70,49 @@ class GradeViewTestMixin(SharedModuleStoreTestCase):
cls.course_key = cls.course.id
def _create_user_enrollments(self, *users):
- date = datetime(2013, 1, 22, tzinfo=UTC)
for user in users:
CourseEnrollmentFactory(
course_id=self.course.id,
user=user,
- created=date,
+ created=self.date,
+ )
+
+ def _create_user_program_enrollments(self, *users):
+ for index, user in enumerate(users):
+ course_enrollment = CourseEnrollmentFactory(
+ course_id=self.course.id,
+ user=user,
+ created=self.date,
+ )
+
+ program_enrollment = ProgramEnrollmentFactory(
+ user=user,
+ external_user_key='program_user_key_{}'.format(index),
+ )
+
+ ProgramCourseEnrollmentFactory(
+ program_enrollment=program_enrollment,
+ course_enrollment=course_enrollment,
+ course_key=self.course.id,
)
def setUp(self):
super(GradeViewTestMixin, self).setUp()
self.password = 'test'
self.global_staff = GlobalStaffFactory.create()
- self.student = UserFactory(password=self.password, username='student')
- self.other_student = UserFactory(password=self.password, username='other_student')
+ self.student = UserFactory(password=self.password, username='student', email='student@example.com')
+ self.other_student = UserFactory(
+ password=self.password,
+ username='other_student',
+ email='i_like_learning@example.com',
+ )
+ self.program_student = UserFactory(
+ password=self.password,
+ username='program_student',
+ email='i_love_learning@example.com',
+ )
self._create_user_enrollments(self.student, self.other_student)
+ self._create_user_program_enrollments(self.program_student)
@classmethod
def _create_test_course_with_default_grading_policy(cls, display_name, run):
diff --git a/lms/djangoapps/grades/rest_api/v1/tests/test_gradebook_views.py b/lms/djangoapps/grades/rest_api/v1/tests/test_gradebook_views.py
index 583ca5dec86..fc805bb5223 100644
--- a/lms/djangoapps/grades/rest_api/v1/tests/test_gradebook_views.py
+++ b/lms/djangoapps/grades/rest_api/v1/tests/test_gradebook_views.py
@@ -386,15 +386,15 @@ class GradebookViewTest(GradebookViewTestBase):
),
}
- def get_url(self, course_key=None, username=None, username_contains=None): # pylint: disable=arguments-differ
+ def get_url(self, course_key=None, username=None, user_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)
+ if user_contains:
+ return "{0}?user_contains={1}".format(base_url, user_contains)
return base_url
@staticmethod
@@ -465,22 +465,32 @@ 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.
+ Helper method to assert that self.student, self.other_student, and
+ self.program_student have the expected gradebook data.
"""
expected_results = [
OrderedDict([
('user_id', self.student.id),
('username', self.student.username),
+ ('email', self.student.email),
('percent', 0.85),
('section_breakdown', self.expected_subsection_grades()),
]),
OrderedDict([
('user_id', self.other_student.id),
('username', self.other_student.username),
+ ('email', self.other_student.email),
('percent', 0.45),
('section_breakdown', self.expected_subsection_grades()),
]),
+ OrderedDict([
+ ('user_id', self.program_student.id),
+ ('username', self.program_student.username),
+ ('email', self.program_student.email),
+ ('external_user_key', 'program_user_key_0'),
+ ('percent', 0.75),
+ ('section_breakdown', self.expected_subsection_grades()),
+ ])
]
self.assertEqual(status.HTTP_200_OK, response.status_code)
@@ -566,6 +576,7 @@ class GradebookViewTest(GradebookViewTestBase):
mock_grade.side_effect = [
self.mock_course_grade(self.student, passed=True, percent=0.85),
self.mock_course_grade(self.other_student, passed=False, percent=0.45),
+ self.mock_course_grade(self.program_student, passed=True, percent=0.75)
]
with override_waffle_flag(self.waffle_flag, active=True):
@@ -592,6 +603,7 @@ class GradebookViewTest(GradebookViewTestBase):
expected_results = OrderedDict([
('user_id', self.student.id),
('username', self.student.username),
+ ('email', self.student.email),
('percent', 0.85),
('section_breakdown', self.expected_subsection_grades()),
])
@@ -674,6 +686,7 @@ class GradebookViewTest(GradebookViewTestBase):
expected_results = OrderedDict([
('user_id', self.student.id),
('username', self.student.username),
+ ('email', self.student.email),
('percent', 0.85),
('section_breakdown', self.expected_subsection_grades()),
])
@@ -688,6 +701,41 @@ class GradebookViewTest(GradebookViewTestBase):
'login_course_staff',
)
def test_gradebook_data_filter_username_contains(self, login_method):
+ with patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.read') as mock_grade:
+ mock_grade.return_value = self.mock_course_grade(
+ self.program_student, passed=True, percent=0.75
+ )
+
+ with override_waffle_flag(self.waffle_flag, active=True):
+ getattr(self, login_method)()
+
+ # check username contains "program"
+ resp = self.client.get(
+ self.get_url(course_key=self.course.id, user_contains='program')
+ )
+ expected_results = [
+ OrderedDict([
+ ('user_id', self.program_student.id),
+ ('username', self.program_student.username),
+ ('email', self.program_student.email),
+ ('external_user_key', 'program_user_key_0'),
+ ('percent', 0.75),
+ ('section_breakdown', self.expected_subsection_grades()),
+ ]),
+ ]
+
+ 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'])
+
+ @ddt.data(
+ 'login_staff',
+ 'login_course_admin',
+ 'login_course_staff',
+ )
+ def test_gradebook_data_filter_email_contains(self, login_method):
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, percent=0.85
@@ -695,13 +743,16 @@ class GradebookViewTest(GradebookViewTestBase):
with override_waffle_flag(self.waffle_flag, active=True):
getattr(self, login_method)()
+
+ # check email contains "like"
resp = self.client.get(
- self.get_url(course_key=self.course.id, username_contains='other')
+ self.get_url(course_key=self.course.id, user_contains='like')
)
expected_results = [
OrderedDict([
('user_id', self.other_student.id),
('username', self.other_student.username),
+ ('email', self.other_student.email),
('percent', 0.85),
('section_breakdown', self.expected_subsection_grades()),
]),
@@ -718,7 +769,43 @@ class GradebookViewTest(GradebookViewTestBase):
'login_course_admin',
'login_course_staff',
)
- def test_gradebook_data_filter_username_contains_no_match(self, login_method):
+ def test_gradebook_data_filter_external_user_key_contains(self, login_method):
+ with patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.read') as mock_grade:
+ mock_grade.return_value = self.mock_course_grade(
+ self.program_student, passed=True, percent=0.75
+ )
+
+ with override_waffle_flag(self.waffle_flag, active=True):
+ getattr(self, login_method)()
+
+ # check external user key contains "key"
+ resp = self.client.get(
+ self.get_url(course_key=self.course.id, user_contains='key')
+ )
+
+ expected_results = [
+ OrderedDict([
+ ('user_id', self.program_student.id),
+ ('username', self.program_student.username),
+ ('email', self.program_student.email),
+ ('external_user_key', 'program_user_key_0'),
+ ('percent', 0.75),
+ ('section_breakdown', self.expected_subsection_grades()),
+ ]),
+ ]
+
+ 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'])
+
+ @ddt.data(
+ 'login_staff',
+ 'login_course_admin',
+ 'login_course_staff',
+ )
+ def test_gradebook_data_filter_user_contains_no_match(self, login_method):
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, percent=0.85
@@ -727,7 +814,7 @@ class GradebookViewTest(GradebookViewTestBase):
with override_waffle_flag(self.waffle_flag, active=True):
getattr(self, login_method)()
resp = self.client.get(
- self.get_url(course_key=self.course.id, username_contains='fooooooooooooooooo')
+ self.get_url(course_key=self.course.id, user_contains='fooooooooooooooooo')
)
self._assert_empty_response(resp)
@@ -754,6 +841,7 @@ class GradebookViewTest(GradebookViewTestBase):
OrderedDict([
('user_id', self.student.id),
('username', self.student.username),
+ ('email', self.student.email),
('percent', 0.85),
('section_breakdown', self.expected_subsection_grades()),
]),
@@ -792,6 +880,7 @@ class GradebookViewTest(GradebookViewTestBase):
mock_grade.side_effect = [
self.mock_course_grade(self.student, passed=True, percent=0.85),
self.mock_course_grade(self.other_student, passed=False, percent=0.45),
+ self.mock_course_grade(self.program_student, passed=True, percent=0.75),
]
# Enroll a verified student, for whom data should not be returned.
@@ -820,6 +909,7 @@ class GradebookViewTest(GradebookViewTestBase):
mock_grade.side_effect = [
self.mock_course_grade(self.student, passed=True, percent=0.85),
self.mock_course_grade(self.other_student, passed=False, percent=0.45),
+ self.mock_course_grade(self.program_student, passed=True, percent=0.75),
]
with override_waffle_flag(self.waffle_flag, active=True):
diff --git a/lms/djangoapps/grades/rest_api/v1/tests/test_views.py b/lms/djangoapps/grades/rest_api/v1/tests/test_views.py
index 977b1249c4a..60e45a164f4 100644
--- a/lms/djangoapps/grades/rest_api/v1/tests/test_views.py
+++ b/lms/djangoapps/grades/rest_api/v1/tests/test_views.py
@@ -246,6 +246,14 @@ class CourseGradesViewTest(GradeViewTestMixin, APITestCase):
'percent': 0.0,
'letter_grade': None
},
+ {
+ 'username': self.program_student.username,
+ 'email': self.program_student.email,
+ 'course_id': str(self.course.id),
+ 'passed': False,
+ 'percent': 0.0,
+ 'letter_grade': None,
+ },
]),
])
diff --git a/lms/djangoapps/grades/rest_api/v1/utils.py b/lms/djangoapps/grades/rest_api/v1/utils.py
index c6253e74e59..d518431f830 100644
--- a/lms/djangoapps/grades/rest_api/v1/utils.py
+++ b/lms/djangoapps/grades/rest_api/v1/utils.py
@@ -6,6 +6,7 @@ from __future__ import absolute_import
from contextlib import contextmanager
from django.contrib.auth import get_user_model
+from django.db.models import Q
from rest_framework import status
from rest_framework.exceptions import AuthenticationFailed
from rest_framework.pagination import CursorPagination
@@ -119,20 +120,20 @@ class GradeViewMixin(DeveloperErrorViewMixin):
"""
Args:
course_key (CourseLocator): The course to retrieve grades for.
- course_enrollment_filter: Optional dictionary of keyword arguments to pass
+ course_enrollment_filter: Optional list of Q objects to pass
to `CourseEnrollment.filter()`.
related_models: Optional list of related models to join to the CourseEnrollment table.
Returns:
A list of users, pulled from a paginated queryset of enrollments, who are enrolled in the given course.
"""
- filter_kwargs = {
- 'course_id': course_key,
- 'is_active': True,
- }
- filter_kwargs.update(course_enrollment_filter or {})
+ filter_args = [
+ Q(course_id=course_key) & Q(is_active=True)
+ ]
+ filter_args.extend(course_enrollment_filter or [])
+
enrollments_in_course = use_read_replica_if_available(
- CourseEnrollment.objects.filter(**filter_kwargs)
+ CourseEnrollment.objects.filter(*filter_args)
)
if related_models:
enrollments_in_course = enrollments_in_course.select_related(*related_models)
--
GitLab