Only show student emails and full names to instructors in internal courses

- includes a bit of test refactoring: look up courses by id, not name

common/lib/xmodule/xmodule/modulestore/__init__.py

@@ -312,6 +312,11 @@ class ModuleStore(object):
         '''
         raise NotImplementedError
 
+    def get_course(self, course_id):
+        '''
+        Look for a specific course id.  Returns the course descriptor, or None if not found.
+        '''
+        raise NotImplementedError
 
     def get_parent_locations(self, location):
         '''Find all locations that are the parents of this location.  Needed
@@ -357,3 +362,10 @@ class ModuleStoreBase(ModuleStore):
 
         errorlog = self._get_errorlog(location)
         return errorlog.errors
+
+    def get_course(self, course_id):
+        """Default impl--linear search through course list"""
+        for c in self.get_courses():
+            if c.id == course_id:
+                return c
+        return None

lms/djangoapps/courseware/tests/tests.py

@@ -292,14 +292,10 @@ class TestNavigation(PageLoader):
 
     def setUp(self):
         xmodule.modulestore.django._MODULESTORES = {}
-        courses = modulestore().get_courses()
 
-        def find_course(course_id):
-            """Assumes the course is present"""
-            return [c for c in courses if c.id==course_id][0]
-
-        self.full = find_course("edX/full/6.002_Spring_2012")
-        self.toy = find_course("edX/toy/2012_Fall")
+        # Assume courses are there
+        self.full = modulestore().get_course("edX/full/6.002_Spring_2012")
+        self.toy = modulestore().get_course("edX/toy/2012_Fall")
 
         # Create two accounts
         self.student = 'view@test.com'
@@ -350,14 +346,9 @@ class TestViewAuth(PageLoader):
 
     def setUp(self):
         xmodule.modulestore.django._MODULESTORES = {}
-        courses = modulestore().get_courses()
-
-        def find_course(course_id):
-            """Assumes the course is present"""
-            return [c for c in courses if c.id==course_id][0]
 
-        self.full = find_course("edX/full/6.002_Spring_2012")
-        self.toy = find_course("edX/toy/2012_Fall")
+        self.full = modulestore().get_course("edX/full/6.002_Spring_2012")
+        self.toy = modulestore().get_course("edX/toy/2012_Fall")
 
         # Create two accounts
         self.student = 'view@test.com'

lms/djangoapps/instructor/tests.py

@@ -35,12 +35,8 @@ class TestInstructorDashboardGradeDownloadCSV(ct.PageLoader):
         xmodule.modulestore.django._MODULESTORES = {}
         courses = modulestore().get_courses()
 
-        def find_course(name):
-            """Assumes the course is present"""
-            return [c for c in courses if c.location.course==name][0]
-
-        self.full = find_course("full")
-        self.toy = find_course("toy")
+        self.full = modulestore().get_course("edX/full/6.002_Spring_2012")
+        self.toy = modulestore().get_course("edX/toy/2012_Fall")
 
         # Create two accounts
         self.student = 'view@test.com'
@@ -51,9 +47,12 @@ class TestInstructorDashboardGradeDownloadCSV(ct.PageLoader):
         self.activate_user(self.student)
         self.activate_user(self.instructor)
 
-        group_name = _course_staff_group_name(self.toy.location)
-        g = Group.objects.create(name=group_name)
-        g.user_set.add(ct.user(self.instructor))
+        def make_instructor(course):
+            group_name = _course_staff_group_name(course.location)
+            g = Group.objects.create(name=group_name)
+            g.user_set.add(ct.user(self.instructor))
+
+        make_instructor(self.toy)
 
         self.logout()
         self.login(self.instructor, self.password)
@@ -62,7 +61,21 @@ class TestInstructorDashboardGradeDownloadCSV(ct.PageLoader):
 
     def test_download_grades_csv(self):
         print "running test_download_grades_csv"
-        course = self.toy
+        toy = self.toy
+        self.check_download_grades_csv(toy, toy.id in settings.INTERNAL_COURSE_IDS)
+
+
+    @override_settings(INTERNAL_COURSE_IDS=set())
+    def test_download_external_grades_csv(self):
+        """
+        Test with an external course--should return less stuff.
+        """
+        print "running test_download_grades_csv"
+        toy = self.toy
+        self.check_download_grades_csv(toy, toy.id in settings.INTERNAL_COURSE_IDS)
+
+    def check_download_grades_csv(self, course, internal):
+        print "checking course {0}".format(course.id)
         url = reverse('instructor_dashboard', kwargs={'course_id': course.id})
         msg = "url = %s\n" % url
         response = self.client.post(url, {'action': 'Download CSV of all student grades for this course',
@@ -71,14 +84,20 @@ class TestInstructorDashboardGradeDownloadCSV(ct.PageLoader):
 
         self.assertEqual(response['Content-Type'],'text/csv',msg)
 
-        cdisp = response['Content-Disposition'].replace('TT_2012','2012')  # jenkins course_id is TT_2012_Fall instead of 2012_Fall?
-        msg += "cdisp = '%s'\n" % cdisp
-        self.assertEqual(cdisp,'attachment; filename=grades_edX/toy/2012_Fall.csv',msg)
+        cdisp = response['Content-Disposition']
+        msg += "Content-Disposition = '%s'\n" % cdisp
+        self.assertEqual(cdisp, 'attachment; filename=grades_{0}.csv'.format(course.id), msg)
 
         body = response.content.replace('\r','')
         msg += "body = '%s'\n" % body
 
-        expected_body = '''"ID","Username","Full Name","edX email","External email","HW 01","HW 02","HW 03","HW 04","HW 05","HW 06","HW 07","HW 08","HW 09","HW 10","HW 11","HW 12","HW Avg","Lab 01","Lab 02","Lab 03","Lab 04","Lab 05","Lab 06","Lab 07","Lab 08","Lab 09","Lab 10","Lab 11","Lab 12","Lab Avg","Midterm","Final"
-"2","u2","Fred Weasley","view2@test.com","","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0.0","0.0"
-'''
+        # if internal, expect some more info.
+        extra1 = '"Full Name","edX email","External email",' if internal else ''
+        extra2 = '"Fred Weasley","view2@test.com","",' if internal else ''
+
+        # All the not-actually-in-the-course hw and labs come from the
+        # default grading policy string in graders.py
+        expected_body = '''"ID","Username",{0}"HW 01","HW 02","HW 03","HW 04","HW 05","HW 06","HW 07","HW 08","HW 09","HW 10","HW 11","HW 12","HW Avg","Lab 01","Lab 02","Lab 03","Lab 04","Lab 05","Lab 06","Lab 07","Lab 08","Lab 09","Lab 10","Lab 11","Lab 12","Lab Avg","Midterm","Final"
+"2","u2",{1}"0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0.0","0.0"
+'''.format(extra1, extra2)
         self.assertEqual(body, expected_body, msg)

lms/djangoapps/instructor/views.py

@@ -81,7 +81,7 @@ def instructor_dashboard(request, course_id):
         try:
             group = Group.objects.get(name=staffgrp)
         except Group.DoesNotExist:
-            group = Group(name=staffgrp)		# create the group
+            group = Group(name=staffgrp)     # create the group
             group.save()
         return group
 
@@ -217,11 +217,18 @@ def get_student_grade_summary_data(request, course, course_id, get_grades=True,
 
     If get_raw_scores=True, then instead of grade summaries, the raw grades for all graded modules are returned.
 
+    If the course_id is in INTERNAL_COURSE_IDS, includes student
+    emails and full names.  Otherwise, just returns the usernames and
+    grades.  This may change pending future policy decisions about data access.
     '''
     enrolled_students = User.objects.filter(courseenrollment__course_id=course_id).order_by('username')
 
-    header = ['ID', 'Username', 'Full Name', 'edX email', 'External email']
-    if get_grades:
+    internal = course_id in settings.INTERNAL_COURSE_IDS
+    if internal:
+        header = ['ID', 'Username', 'Full Name', 'edX email', 'External email']
+    else:
+        header = ['ID', 'Username']
+    if get_grades and enrolled_students.count() > 0:
         # just to construct the header
         gradeset = grades.grade(enrolled_students[0], request, course, keep_raw_scores=get_raw_scores)
         # log.debug('student %s gradeset %s' % (enrolled_students[0], gradeset))
@@ -234,11 +241,14 @@ def get_student_grade_summary_data(request, course, course_id, get_grades=True,
     data = []
 
     for student in enrolled_students:
-        datarow = [ student.id, student.username, student.profile.name, student.email ]
-        try:
-            datarow.append(student.externalauthmap.external_email)
-        except:	# ExternalAuthMap.DoesNotExist
-            datarow.append('')
+        if internal:
+            datarow = [student.id, student.username, student.profile.name, student.email]
+            try:
+                datarow.append(student.externalauthmap.external_email)
+            except:	# ExternalAuthMap.DoesNotExist
+                datarow.append('')
+        else:
+            datarow = [student.id, student.username]
 
         if get_grades:
             gradeset = grades.grade(student, request, course, keep_raw_scores=get_raw_scores)
@@ -267,11 +277,16 @@ def gradebook(request, course_id):
     # TODO (vshnayder): implement pagination.
     enrolled_students = enrolled_students[:1000]   # HACK!
 
+    internal = course_id in settings.INTERNAL_COURSE_IDS
+    def tooltip(student):
+        if internal:
+            return '{0} <{1}>'.format(student.profile.name, student.email)
+        return ""
+
     student_info = [{'username': student.username,
                      'id': student.id,
-                     'email': student.email,
                      'grade_summary': grades.grade(student, request, course),
-                     'realname': student.profile.name,
+                     'tooltip': tooltip(student),
                      }
                      for student in enrolled_students]
 

lms/envs/common.py

@@ -93,6 +93,11 @@ GENERATE_PROFILE_SCORES = False
 # Used with XQueue
 XQUEUE_WAITTIME_BETWEEN_REQUESTS = 5 # seconds
 
+# Used for various access control decisions.  Should be a set of course_ids.
+# This is not a good long-term solution--eventually this needs to be set in a db somewhere
+INTERNAL_COURSE_IDS = set()
+
+
 ############################# SET PATH INFORMATION #############################
 PROJECT_ROOT = path(__file__).abspath().dirname().dirname()  # /mitx/lms
 REPO_ROOT = PROJECT_ROOT.dirname()
@@ -126,7 +131,7 @@ NODE_PATH = ':'.join(node_paths)
 
 
 ############################ OpenID Provider  ##################################
-OPENID_PROVIDER_TRUSTED_ROOTS = ['cs50.net', '*.cs50.net'] 
+OPENID_PROVIDER_TRUSTED_ROOTS = ['cs50.net', '*.cs50.net']
 
 ################################## MITXWEB #####################################
 # This is where we stick our compiled template files. Most of the app uses Mako
@@ -156,7 +161,7 @@ TEMPLATE_CONTEXT_PROCESSORS = (
     'askbot.user_messages.context_processors.user_messages',#must be before auth
     'django.contrib.auth.context_processors.auth', #this is required for admin
     'django.core.context_processors.csrf', #necessary for csrf protection
-    
+
     # Added for django-wiki
     'django.core.context_processors.media',
     'django.core.context_processors.tz',
@@ -343,7 +348,7 @@ WIKI_CAN_ASSIGN = lambda article, user: user.is_staff or user.is_superuser
 
 WIKI_USE_BOOTSTRAP_SELECT_WIDGET = False
 WIKI_LINK_LIVE_LOOKUPS = False
-WIKI_LINK_DEFAULT_LEVEL = 2 
+WIKI_LINK_DEFAULT_LEVEL = 2
 
 ################################# Jasmine ###################################
 JASMINE_TEST_DIRECTORY = PROJECT_ROOT + '/static/coffee'
@@ -360,10 +365,10 @@ STATICFILES_FINDERS = (
 TEMPLATE_LOADERS = (
     'mitxmako.makoloader.MakoFilesystemLoader',
     'mitxmako.makoloader.MakoAppDirectoriesLoader',
- 
+
     # 'django.template.loaders.filesystem.Loader',
     # 'django.template.loaders.app_directories.Loader',
-    
+
     #'askbot.skins.loaders.filesystem_load_template_source',
     # 'django.template.loaders.eggs.Loader',
 )
@@ -381,7 +386,7 @@ MIDDLEWARE_CLASSES = (
     'django.contrib.messages.middleware.MessageMiddleware',
     'track.middleware.TrackMiddleware',
     'mitxmako.middleware.MakoMiddleware',
-    
+
     'course_wiki.course_nav.Middleware',
 
     'askbot.middleware.anon_user.ConnectToSessionMessagesMiddleware',
@@ -609,7 +614,7 @@ INSTALLED_APPS = (
     'util',
     'certificates',
     'instructor',
-    
+
     #For the wiki
     'wiki', # The new django-wiki from benjaoming
     'django_notify',

lms/envs/dev.py

@@ -92,6 +92,12 @@ COURSE_LISTINGS = {
 }
 
 
+INTERNAL_COURSE_IDS = set(['BerkeleyX/CS169/fa12',
+                           'BerkeleyX/CS188/fa12',
+                           'HarvardX/CS50x/2012H',
+                           'MITx/3.091/MIT_2012_Fall',
+                           'MITx/6.002x-EE98/2012_Fall_SJSU'])
+
 SUBDOMAIN_BRANDING = {
     'sjsu': 'MITx',
     'mit': 'MITx',

lms/envs/test.py

@@ -25,6 +25,8 @@ WIKI_ENABLED = True
 # Makes the tests run much faster...
 SOUTH_TESTS_MIGRATE = False # To disable migrations and use syncdb instead
 
+INTERNAL_COURSE_IDS = set(['edX/toy/2012_Fall'])
+
 # Nose Test Runner
 INSTALLED_APPS += ('django_nose',)
 NOSE_ARGS = ['--cover-erase', '--with-xunit', '--with-xcoverage', '--cover-html',

lms/templates/courseware/gradebook.html

@@ -49,7 +49,7 @@
         %for student in students:
         <tr>
           <td>
-            <a href="${reverse('student_progress', kwargs=dict(course_id=course_id, student_id=student['id']))}">${student['username']}</a>
+            <a href="${reverse('student_progress', kwargs=dict(course_id=course_id, student_id=student['id']))}" title="${student['tooltip']}">${student['username']}</a>
           </td>
         </tr>
         %endfor
@@ -90,7 +90,7 @@
 
         <tbody>
           %for student in students:
-          <tr>          
+          <tr>
             %for section in student['grade_summary']['section_breakdown']:
               ${percent_data( section['percent'] )}
             %endfor