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

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

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)
 

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