fix formatting, address comments from PR #322

lms/djangoapps/analytics/distributions.py

@@ -31,7 +31,10 @@ def profile_distribution(course_id, feature):
     feature_results = {}
 
     if not feature in AVAILABLE_PROFILE_FEATURES:
-        raise ValueError("unsupported feature requested for distribution '{}'".format(feature))
+        raise ValueError(
+            "unsupported feature requested for distribution '{}'".format(
+                feature)
+        )
 
     if feature in _EASY_CHOICE_FEATURES:
         if feature == 'gender':
@@ -39,25 +42,34 @@ def profile_distribution(course_id, feature):
         elif feature == 'level_of_education':
             raw_choices = UserProfile.LEVEL_OF_EDUCATION_CHOICES
 
-        choices = [(short, full) for (short, full) in raw_choices] + [('no_data', 'No Data')]
+        choices = [(short, full)
+                   for (short, full) in raw_choices] + [('no_data', 'No Data')]
 
         data = {}
         for (short, full) in choices:
             if feature == 'gender':
-                count = CourseEnrollment.objects.filter(course_id=course_id, user__profile__gender=short).count()
+                count = CourseEnrollment.objects.filter(
+                    course_id=course_id, user__profile__gender=short
+                ).count()
             elif feature == 'level_of_education':
-                count = CourseEnrollment.objects.filter(course_id=course_id, user__profile__level_of_education=short).count()
+                count = CourseEnrollment.objects.filter(
+                    course_id=course_id, user__profile__level_of_education=short
+                ).count()
             data[short] = count
 
         feature_results['data'] = data
         feature_results['type'] = 'EASY_CHOICE'
         feature_results['display_names'] = dict(choices)
     elif feature in _OPEN_CHOICE_FEATURES:
-        profiles = UserProfile.objects.filter(user__courseenrollment__course_id=course_id)
-        query_distribution = profiles.values(feature).annotate(Count(feature)).order_by()
-        # query_distribution is of the form [{'featureval': 'value1', 'featureval__count': 4}, {'featureval': 'value2', 'featureval__count': 2}, ...]
-
-        distribution = dict((vald[feature], vald[feature + '__count']) for vald in query_distribution)
+        profiles = UserProfile.objects.filter(
+            user__courseenrollment__course_id=course_id)
+        query_distribution = profiles.values(
+            feature).annotate(Count(feature)).order_by()
+        # query_distribution is of the form [{'featureval': 'value1', 'featureval__count': 4},
+        #    {'featureval': 'value2', 'featureval__count': 2}, ...]
+
+        distribution = dict((vald[feature], vald[feature + '__count'])
+                            for vald in query_distribution)
         # distribution is of the form {'value1': 4, 'value2': 2, ...}
 
         # change none to no_data for valid json key
@@ -65,7 +77,9 @@ def profile_distribution(course_id, feature):
             distribution['no_data'] = distribution.pop(None)
             # django does not properly count NULL values, so the above will alwasy be 0.
             # this correctly counts null values
-            distribution['no_data'] = profiles.filter(**{feature: None}).count()
+            distribution['no_data'] = profiles.filter(
+                **{feature: None}
+            ).count()
 
         feature_results['data'] = distribution
         feature_results['type'] = 'OPEN_CHOICE'

lms/djangoapps/instructor/tests/test_api.py

@@ -17,3 +17,10 @@ class TestInstructorAPIHelpers(TestCase):
 
         for (stng, lst) in zip(strings, lists):
             self.assertEqual(_split_input_list(stng), lst)
+
+    def test_split_input_list_unicode(self):
+        self.assertEqual(_split_input_list('robot@robot.edu, robot2@robot.edu'), ['robot@robot.edu', 'robot2@robot.edu'])
+        self.assertEqual(_split_input_list(u'robot@robot.edu, robot2@robot.edu'), ['robot@robot.edu', 'robot2@robot.edu'])
+        self.assertEqual(_split_input_list(u'robot@robot.edu, robot2@robot.edu'), [u'robot@robot.edu', 'robot2@robot.edu'])
+        scary_unistuff = unichr(40960) + u'abcd' + unichr(1972)
+        self.assertEqual(_split_input_list(scary_unistuff), [scary_unistuff])

lms/djangoapps/instructor/views/api.py

@@ -98,7 +98,10 @@ def students_update_enrollment(request, course_id):
     )
 
     action = request.GET.get('action')
-    emails = _split_input_list(request.GET.get('emails'))
+    emails_raw = request.GET.get('emails')
+    print "@@@@"
+    print type(emails_raw)
+    emails = _split_input_list(emails_raw)
     auto_enroll = request.GET.get('auto_enroll') in ['true', 'True', True]
 
     def format_result(func, email):
@@ -340,6 +343,9 @@ def profile_distribution(request, course_id):
 
 @ensure_csrf_cookie
 @cache_control(no_cache=True, no_store=True, must_revalidate=True)
+@require_query_params(
+    student_email="email of student for whom to get progress url"
+)
 def get_student_progress_url(request, course_id):
     """
     Get the progress url of a student.
@@ -355,8 +361,6 @@ def get_student_progress_url(request, course_id):
     )
 
     student_email = request.GET.get('student_email')
-    if not student_email:
-        return HttpResponseBadRequest()
     user = User.objects.get(email=student_email)
 
     progress_url = reverse('student_progress', kwargs={'course_id': course_id, 'student_id': user.id})
@@ -650,7 +654,7 @@ def _msk_from_problem_urlname(course_id, urlname):
     Convert a 'problem urlname' (instructor input name)
     to a module state key (db field)
     """
-    if urlname[-4:] == ".xml":
+    if urlname.endswith(".xml"):
         urlname = urlname[:-4]
 
     urlname = "problem/" + urlname