From fe41e50d7473e1a38b37080c6ab1bf18ff23ac2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Behmo?= Date: Mon, 22 Feb 2016 17:03:00 +0100 Subject: [PATCH] Include non-obsolete location info in student profile report The student profile report that can be downloaded from the course staff dashboard included the "location" field (which is obsolete) and the mailing address, which is seldom completed. Here, we add the "country" and "city" fields to the csv report. To do so, we need to be able to dump the new fields to JSON so we convert the user fields to unicode when needed. Note that this breaks compatibility with earlier reports. --- lms/djangoapps/instructor/views/api.py | 4 ++- lms/djangoapps/instructor_analytics/basic.py | 17 +++++++++-- .../instructor_analytics/tests/test_basic.py | 28 +++++++++++++++---- 3 files changed, 40 insertions(+), 9 deletions(-) diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index cca2c8ca1c..8501243f86 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 7e0fff8806..feeaccf39c 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 6cdecf141d..3e8db3731c 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): """