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
This commit is contained in:
Michael Roytman
2019-06-07 11:18:26 -04:00
committed by atesker
parent dff3eec0d1
commit 05c0510cd3
8 changed files with 197 additions and 39 deletions

View File

@@ -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():
"""

View File

@@ -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):
"""

View File

@@ -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)

View File

@@ -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(

View File

@@ -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):

View File

@@ -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):

View File

@@ -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,
},
]),
])

View File

@@ -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)