diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index cca2c8c..8501243 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -1246,7 +1246,7 @@ def get_students_features(request, course_id, csv=False): # pylint: disable=red query_features = [ 'id', 'username', 'name', 'email', 'language', 'location', 'year_of_birth', 'gender', 'level_of_education', 'mailing_address', - 'goals' + 'goals', 'city', 'country' ] # Provide human-friendly and translatable names for these features. These names @@ -1264,6 +1264,8 @@ def get_students_features(request, course_id, csv=False): # pylint: disable=red 'level_of_education': _('Level of Education'), 'mailing_address': _('Mailing Address'), 'goals': _('Goals'), + 'city': _('City'), + 'country': _('Country'), } if is_course_cohorted(course.id): diff --git a/lms/djangoapps/instructor_analytics/basic.py b/lms/djangoapps/instructor_analytics/basic.py index 7e0fff8..feeaccf 100644 --- a/lms/djangoapps/instructor_analytics/basic.py +++ b/lms/djangoapps/instructor_analytics/basic.py @@ -13,6 +13,7 @@ from django.db.models import Q from django.conf import settings from django.contrib.auth.models import User from django.core.exceptions import ObjectDoesNotExist +from django.core.serializers.json import DjangoJSONEncoder from django.core.urlresolvers import reverse from opaque_keys.edx.keys import UsageKey import xmodule.graders as xmgraders @@ -27,7 +28,8 @@ from certificates.models import CertificateStatuses STUDENT_FEATURES = ('id', 'username', 'first_name', 'last_name', 'is_staff', 'email') PROFILE_FEATURES = ('name', 'language', 'location', 'year_of_birth', 'gender', - 'level_of_education', 'mailing_address', 'goals', 'meta') + 'level_of_education', 'mailing_address', 'goals', 'meta', + 'city', 'country') ORDER_ITEM_FEATURES = ('list_price', 'unit_cost', 'status') ORDER_FEATURES = ('purchase_time',) @@ -222,6 +224,15 @@ def enrolled_students_features(course_key, features): if include_team_column: students = students.prefetch_related('teams') + def extract_attr(student, feature): + """Evaluate a student attribute that is ready for JSON serialization""" + attr = getattr(student, feature) + try: + DjangoJSONEncoder().default(attr) + return attr + except TypeError: + return unicode(attr) + def extract_student(student, features): """ convert student to dictionary """ student_features = [x for x in STUDENT_FEATURES if x in features] @@ -236,11 +247,11 @@ def enrolled_students_features(course_key, features): meta_key = feature.split('.')[1] meta_features.append((feature, meta_key)) - student_dict = dict((feature, getattr(student, feature)) + student_dict = dict((feature, extract_attr(student, feature)) for feature in student_features) profile = student.profile if profile is not None: - profile_dict = dict((feature, getattr(profile, feature)) + profile_dict = dict((feature, extract_attr(profile, feature)) for feature in profile_features) student_dict.update(profile_dict) diff --git a/lms/djangoapps/instructor_analytics/tests/test_basic.py b/lms/djangoapps/instructor_analytics/tests/test_basic.py index 6cdecf1..3e8db37 100644 --- a/lms/djangoapps/instructor_analytics/tests/test_basic.py +++ b/lms/djangoapps/instructor_analytics/tests/test_basic.py @@ -106,17 +106,35 @@ class TestAnalyticsBasic(ModuleStoreTestCase): self.assertIn(userreport['username'], [user.username for user in self.users]) def test_enrolled_students_features_keys(self): - query_features = ('username', 'name', 'email') + query_features = ('username', 'name', 'email', 'city', 'country',) + for user in self.users: + user.profile.city = "Mos Eisley {}".format(user.id) + user.profile.country = "Tatooine {}".format(user.id) + user.profile.save() for feature in query_features: self.assertIn(feature, AVAILABLE_FEATURES) with self.assertNumQueries(1): userreports = enrolled_students_features(self.course_key, query_features) self.assertEqual(len(userreports), len(self.users)) - for userreport in userreports: + + userreports = sorted(userreports, key=lambda u: u["username"]) + users = sorted(self.users, key=lambda u: u.username) + for userreport, user in zip(userreports, users): self.assertEqual(set(userreport.keys()), set(query_features)) - self.assertIn(userreport['username'], [user.username for user in self.users]) - self.assertIn(userreport['email'], [user.email for user in self.users]) - self.assertIn(userreport['name'], [user.profile.name for user in self.users]) + self.assertEqual(userreport['username'], user.username) + self.assertEqual(userreport['email'], user.email) + self.assertEqual(userreport['name'], user.profile.name) + self.assertEqual(userreport['city'], user.profile.city) + self.assertEqual(userreport['country'], user.profile.country) + + def test_enrolled_student_with_no_country_city(self): + userreports = enrolled_students_features(self.course_key, ('username', 'city', 'country',)) + for userreport in userreports: + # This behaviour is somewhat inconsistent: None string fields + # objects are converted to "None", but non-JSON serializable fields + # are converted to an empty string. + self.assertEqual(userreport['city'], "None") + self.assertEqual(userreport['country'], "") def test_enrolled_students_meta_features_keys(self): """