Merge pull request #1155 from edx/sarina/ins-dash-student-admin-2

lms/djangoapps/instructor/tests/test_api.py

@@ -512,7 +512,7 @@ class TestInstructorAPILevelsDataDump(ModuleStoreTestCase, LoginEnrollmentTestCa
     def test_get_student_progress_url(self):
         """ Test that progress_url is in the successful response. """
         url = reverse('get_student_progress_url', kwargs={'course_id': self.course.id})
-        url += "?student_email={}".format(
+        url += "?unique_student_identifier={}".format(
             quote(self.students[0].email.encode("utf-8"))
         )
         print url
@@ -522,6 +522,19 @@ class TestInstructorAPILevelsDataDump(ModuleStoreTestCase, LoginEnrollmentTestCa
         res_json = json.loads(response.content)
         self.assertIn('progress_url', res_json)
 
+    def test_get_student_progress_url_from_uname(self):
+        """ Test that progress_url is in the successful response. """
+        url = reverse('get_student_progress_url', kwargs={'course_id': self.course.id})
+        url += "?unique_student_identifier={}".format(
+            quote(self.students[0].username.encode("utf-8"))
+        )
+        print url
+        response = self.client.get(url)
+        print response
+        self.assertEqual(response.status_code, 200)
+        res_json = json.loads(response.content)
+        self.assertIn('progress_url', res_json)
+
     def test_get_student_progress_url_noparams(self):
         """ Test that the endpoint 404's without the required query params. """
         url = reverse('get_student_progress_url', kwargs={'course_id': self.course.id})
@@ -579,7 +592,7 @@ class TestInstructorAPIRegradeTask(ModuleStoreTestCase, LoginEnrollmentTestCase)
         url = reverse('reset_student_attempts', kwargs={'course_id': self.course.id})
         response = self.client.get(url, {
             'problem_to_reset': self.problem_urlname,
-            'student_email': self.student.email,
+            'unique_student_identifier': self.student.email,
         })
         print response.content
         self.assertEqual(response.status_code, 200)
@@ -608,7 +621,7 @@ class TestInstructorAPIRegradeTask(ModuleStoreTestCase, LoginEnrollmentTestCase)
         url = reverse('reset_student_attempts', kwargs={'course_id': self.course.id})
         response = self.client.get(url, {
             'problem_to_reset': 'robot-not-a-real-module',
-            'student_email': self.student.email,
+            'unique_student_identifier': self.student.email,
         })
         print response.content
         self.assertEqual(response.status_code, 400)
@@ -618,7 +631,7 @@ class TestInstructorAPIRegradeTask(ModuleStoreTestCase, LoginEnrollmentTestCase)
         url = reverse('reset_student_attempts', kwargs={'course_id': self.course.id})
         response = self.client.get(url, {
             'problem_to_reset': self.problem_urlname,
-            'student_email': self.student.email,
+            'unique_student_identifier': self.student.email,
             'delete_module': True,
         })
         print response.content
@@ -634,11 +647,11 @@ class TestInstructorAPIRegradeTask(ModuleStoreTestCase, LoginEnrollmentTestCase)
         )
 
     def test_reset_student_attempts_nonsense(self):
-        """ Test failure with both student_email and all_students. """
+        """ Test failure with both unique_student_identifier and all_students. """
         url = reverse('reset_student_attempts', kwargs={'course_id': self.course.id})
         response = self.client.get(url, {
             'problem_to_reset': self.problem_urlname,
-            'student_email': self.student.email,
+            'unique_student_identifier': self.student.email,
             'all_students': True,
         })
         print response.content
@@ -650,7 +663,19 @@ class TestInstructorAPIRegradeTask(ModuleStoreTestCase, LoginEnrollmentTestCase)
         url = reverse('rescore_problem', kwargs={'course_id': self.course.id})
         response = self.client.get(url, {
             'problem_to_reset': self.problem_urlname,
-            'student_email': self.student.email,
+            'unique_student_identifier': self.student.email,
+        })
+        print response.content
+        self.assertEqual(response.status_code, 200)
+        self.assertTrue(act.called)
+
+    @patch.object(instructor_task.api, 'submit_rescore_problem_for_student')
+    def test_rescore_problem_single_from_uname(self, act):
+        """ Test rescoring of a single student. """
+        url = reverse('rescore_problem', kwargs={'course_id': self.course.id})
+        response = self.client.get(url, {
+            'problem_to_reset': self.problem_urlname,
+            'unique_student_identifier': self.student.username,
         })
         print response.content
         self.assertEqual(response.status_code, 200)
@@ -747,7 +772,7 @@ class TestInstructorAPITaskLists(ModuleStoreTestCase, LoginEnrollmentTestCase):
         url = reverse('list_instructor_tasks', kwargs={'course_id': self.course.id})
         response = self.client.get(url, {
             'problem_urlname': self.problem_urlname,
-            'student_email': self.student.email,
+            'unique_student_identifier': self.student.email,
         })
         print response.content
         self.assertEqual(response.status_code, 200)

lms/djangoapps/instructor/views/api.py

@@ -33,7 +33,7 @@ import instructor_task.api
 from instructor_task.api_helper import AlreadyRunningError
 import instructor.enrollment as enrollment
 from instructor.enrollment import enroll_email, unenroll_email
-from instructor.views.tools import strip_if_string
+from instructor.views.tools import strip_if_string, get_student_from_identifier
 import instructor.access as access
 import analytics.basic
 import analytics.distributions
@@ -456,20 +456,19 @@ def get_distribution(request, course_id):
 @common_exceptions_400
 @require_level('staff')
 @require_query_params(
-    student_email="email of student for whom to get progress url"
+    unique_student_identifier="email or username of student for whom to get progress url"
 )
 def get_student_progress_url(request, course_id):
     """
     Get the progress url of a student.
     Limited to staff access.
 
-    Takes query paremeter student_email and if the student exists
+    Takes query paremeter unique_student_identifier and if the student exists
     returns e.g. {
         'progress_url': '/../...'
     }
     """
-    student_email = strip_if_string(request.GET.get('student_email'))
-    user = User.objects.get(email=student_email)
+    user = get_student_from_identifier(request.GET.get('unique_student_identifier'))
 
     progress_url = reverse('student_progress', kwargs={'course_id': course_id, 'student_id': user.id})
 
@@ -496,7 +495,7 @@ def reset_student_attempts(request, course_id):
 
     Takes some of the following query paremeters
         - problem_to_reset is a urlname of a problem
-        - student_email is an email
+        - unique_student_identifier is an email or username
         - all_students is a boolean
             requires instructor access
             mutually exclusive with delete_module
@@ -510,14 +509,17 @@ def reset_student_attempts(request, course_id):
     )
 
     problem_to_reset = strip_if_string(request.GET.get('problem_to_reset'))
-    student_email = strip_if_string(request.GET.get('student_email'))
+    student_identifier = request.GET.get('unique_student_identifier', None)
+    student = None
+    if student_identifier is not None:
+        student = get_student_from_identifier(student_identifier)
     all_students = request.GET.get('all_students', False) in ['true', 'True', True]
     delete_module = request.GET.get('delete_module', False) in ['true', 'True', True]
 
     # parameter combinations
-    if all_students and student_email:
+    if all_students and student:
         return HttpResponseBadRequest(
-            "all_students and student_email are mutually exclusive."
+            "all_students and unique_student_identifier are mutually exclusive."
         )
     if all_students and delete_module:
         return HttpResponseBadRequest(
@@ -534,15 +536,16 @@ def reset_student_attempts(request, course_id):
     response_payload = {}
     response_payload['problem_to_reset'] = problem_to_reset
 
-    if student_email:
+    if student:
         try:
-            student = User.objects.get(email=student_email)
             enrollment.reset_student_attempts(course_id, student, module_state_key, delete_module=delete_module)
         except StudentModule.DoesNotExist:
             return HttpResponseBadRequest("Module does not exist.")
+        response_payload['student'] = student_identifier
     elif all_students:
         instructor_task.api.submit_reset_problem_attempts_for_all_students(request, course_id, module_state_key)
         response_payload['task'] = 'created'
+        response_payload['student'] = 'All Students'
     else:
         return HttpResponseBadRequest()
 
@@ -561,21 +564,25 @@ def rescore_problem(request, course_id):
 
     Takes either of the following query paremeters
         - problem_to_reset is a urlname of a problem
-        - student_email is an email
+        - unique_student_identifier is an email or username
         - all_students is a boolean
 
-    all_students and student_email cannot both be present.
+    all_students and unique_student_identifier cannot both be present.
     """
     problem_to_reset = strip_if_string(request.GET.get('problem_to_reset'))
-    student_email = strip_if_string(request.GET.get('student_email', False))
+    student_identifier = request.GET.get('unique_student_identifier', None)
+    student = None
+    if student_identifier is not None:
+        student = get_student_from_identifier(student_identifier)
+
     all_students = request.GET.get('all_students') in ['true', 'True', True]
 
-    if not (problem_to_reset and (all_students or student_email)):
+    if not (problem_to_reset and (all_students or student)):
         return HttpResponseBadRequest("Missing query parameters.")
 
-    if all_students and student_email:
+    if all_students and student:
         return HttpResponseBadRequest(
-            "Cannot rescore with all_students and student_email."
+            "Cannot rescore with all_students and unique_student_identifier."
         )
 
     module_state_key = _msk_from_problem_urlname(course_id, problem_to_reset)
@@ -583,9 +590,8 @@ def rescore_problem(request, course_id):
     response_payload = {}
     response_payload['problem_to_reset'] = problem_to_reset
 
-    if student_email:
-        response_payload['student_email'] = student_email
-        student = User.objects.get(email=student_email)
+    if student:
+        response_payload['student'] = student_identifier
         instructor_task.api.submit_rescore_problem_for_student(request, course_id, module_state_key, student)
         response_payload['task'] = 'created'
     elif all_students:
@@ -608,21 +614,22 @@ def list_instructor_tasks(request, course_id):
     Takes optional query paremeters.
         - With no arguments, lists running tasks.
         - `problem_urlname` lists task history for problem
-        - `problem_urlname` and `student_email` lists task
+        - `problem_urlname` and `unique_student_identifier` lists task
             history for problem AND student (intersection)
     """
     problem_urlname = strip_if_string(request.GET.get('problem_urlname', False))
-    student_email = strip_if_string(request.GET.get('student_email', False))
+    student = request.GET.get('unique_student_identifier', None)
+    if student is not None:
+        student = get_student_from_identifier(student)
 
-    if student_email and not problem_urlname:
+    if student and not problem_urlname:
         return HttpResponseBadRequest(
-            "student_email must accompany problem_urlname"
+            "unique_student_identifier must accompany problem_urlname"
         )
 
     if problem_urlname:
         module_state_key = _msk_from_problem_urlname(course_id, problem_urlname)
-        if student_email:
-            student = User.objects.get(email=student_email)
+        if student:
             tasks = instructor_task.api.get_instructor_task_history(course_id, module_state_key, student)
         else:
             tasks = instructor_task.api.get_instructor_task_history(course_id, module_state_key)

lms/djangoapps/instructor/views/instructor_dashboard.py

@@ -38,7 +38,7 @@ def instructor_dashboard_2(request, course_id):
         raise Http404()
 
     sections = [
-        _section_course_info(course_id),
+        _section_course_info(course_id, access),
         _section_membership(course_id, access),
         _section_student_admin(course_id, access),
         _section_data_download(course_id),
@@ -67,18 +67,21 @@ section_display_name will be used to generate link titles in the nav bar.
 """  # pylint: disable=W0105
 
 
-def _section_course_info(course_id):
+def _section_course_info(course_id, access):
     """ Provide data for the corresponding dashboard section """
     course = get_course_by_id(course_id, depth=None)
 
-    section_data = {}
-    section_data['section_key'] = 'course_info'
-    section_data['section_display_name'] = _('Course Info')
-    section_data['course_id'] = course_id
-    section_data['course_display_name'] = course.display_name
-    section_data['enrollment_count'] = CourseEnrollment.objects.filter(course_id=course_id).count()
-    section_data['has_started'] = course.has_started()
-    section_data['has_ended'] = course.has_ended()
+    section_data = {
+        'section_key': 'course_info',
+        'section_display_name': _('Course Info'),
+        'course_id': course_id,
+        'access': access,
+        'course_display_name': course.display_name,
+        'enrollment_count': CourseEnrollment.objects.filter(course_id=course_id).count(),
+        'has_started': course.has_started(),
+        'has_ended': course.has_ended(),
+        'list_instructor_tasks_url': reverse('list_instructor_tasks', kwargs={'course_id': course_id}),
+    }
 
     try:
         advance = lambda memo, (letter, score): "{}: {}, ".format(letter, score) + memo

lms/djangoapps/instructor/views/tools.py

 """
 Tools for the instructor dashboard
 """
+from django.contrib.auth.models import User
+
 def strip_if_string(value):
     if isinstance(value, basestring):
         return value.strip()
     return value
+
+
+def get_student_from_identifier(unique_student_identifier):
+    """
+    Gets a student object using either an email address or username.
+
+    Returns the student object associated with `unique_student_identifier`
+
+    Raises User.DoesNotExist if no user object can be found.
+    """
+    unique_student_identifier = strip_if_string(unique_student_identifier)
+    if "@" in unique_student_identifier:
+        student = User.objects.get(email=unique_student_identifier)
+    else:
+        student = User.objects.get(username=unique_student_identifier)
+    return student

lms/templates/courseware/instructor_dashboard.html

@@ -240,7 +240,7 @@ function goto( mode)
     <hr width="40%" style="align:left">
   %endif
 
-    <H2>${_("Student-specific grade inspection and adjustment")}</h2>
+    <h2>${_("Student-specific grade inspection and adjustment")}</h2>
     <p>
       ${_("Specify the {platform_name} email address or username of a student here:").format(platform_name=settings.PLATFORM_NAME)}
       <input type="text" name="unique_student_identifier">

lms/templates/instructor/instructor_dashboard_2/course_info.html

@@ -38,8 +38,21 @@
 ##   ${ section_data['offline_grades'] }
 ## </div>
 
+%if settings.MITX_FEATURES.get('ENABLE_INSTRUCTOR_BACKGROUND_TASKS') and section_data['access']['instructor']:
+  <div class="running-tasks-container action-type-container">
+    <hr>
+    <h2> ${_("Pending Instructor Tasks")} </h2>
+    <p>${_("The status for any active tasks appears in a table below.")} </p>
+
+    <div class="running-tasks-table" data-endpoint="${ section_data['list_instructor_tasks_url'] }"></div>
+  </div>
+
+%endif
+
 %if len(section_data['course_errors']):
   <div class="course-errors-wrapper">
+    <hr>
+    <p>
     <div class="toggle-wrapper">
       <h2 class="title">${_("Course Warnings")}:</h2>
       <div class="triangle"></div>
@@ -52,5 +65,10 @@
         </div>
       %endfor
     </div>
+    <p>
   </div>
+<br>
 %endif
+
+
+

lms/templates/instructor/instructor_dashboard_2/student_admin.html

@@ -2,25 +2,49 @@
 <%page args="section_data"/>
 
 <div class="student-specific-container action-type-container">
-  <H2>${_("Student-specific grade adjustment")}</h2>
+  <h2>${_("Student-specific grade inspection")}</h2>
   <div class="request-response-error"></div>
-
-  <input type="text" name="student-select" placeholder="${_("Student Email")}">
+  <p>
+    <!-- Doesn't work for username but this MUST work -->
+  ${_("Specify the {platform_name} email address or username of a student here:").format(platform_name=settings.PLATFORM_NAME)}
+  <input type="text" name="student-select-progress" placeholder="${_("Student Email or Username")}">
+  </p>
   <br>
 
   <div class="progress-link-wrapper">
+  <p>
+    ${_("Click this link to view the student's progress page:")}
+
     <a href="" class="progress-link" data-endpoint="${ section_data['get_student_progress_url_url'] }"> ${_("Student Progress Page")} </a>
+  </p>
   </div>
-
   <br>
+
+  <!-- These buttons don't appear to be working
+  <p>
+    ${_("Click to enroll or unenroll this student from the course:")}
   <input type="button" name="enroll" value="${_("Enroll")}" data-endpoint="${ section_data['enrollment_url'] }">
   <input type="button" name="unenroll" value="${_("Unenroll")}" data-endpoint="${ section_data['enrollment_url'] }">
-  ## <select class="problems">
-  ##   <option>Getting problems...</option>
-  ## </select>
+  </p>
+  -->
 
-  <p> ${_('Specify a particular problem in the course here by its url:')} </p>
+  <hr>
+</div>
+
+<div class="student-grade-container action-type-container">
+  <h2>${_("Student-specific grade adjustment")}</h2>
+  <div class="request-response-error"></div>
+  <p>
+    <!-- Doesn't work for username but this MUST work -->
+  ${_("Specify the {platform_name} email address or username of a student here:").format(platform_name=settings.PLATFORM_NAME)}
+  <input type="text" name="student-select-grade" placeholder="${_("Student Email or Username")}">
+  </p>
+  <br>
+
+  <p> ${_('Specify a particular problem in the course here by its url:')} 
   <input type="text" name="problem-select-single" placeholder="${_("Problem urlname")}">
+  </p>
+
   <p>
     ${_('You may use just the "urlname" if a problem, or "modulename/urlname" if not. (For example, if the location is {location1}, then just provide the {urlname1}. If the location is {location2}, then provide {urlname2}.)').format(
     location1="<tt>i4x://university/course/problem/problemname</tt>",
@@ -29,20 +53,31 @@
     urlname2="<tt>notaproblem/someothername</tt>")
     }
   </p>
-  <input type="button" name="reset-attempts-single" value="${_("Reset Student Attempts")}" data-endpoint="${ section_data['reset_student_attempts_url'] }">
 
-  %if section_data['access']['instructor']:
-    <p> ${_('You may also delete the entire state of a student for the specified module:')} </p>
-    <input type="button" class="molly-guard" name="delete-state-single" value="${_("Delete Student State for Module")}" data-endpoint="${ section_data['reset_student_attempts_url'] }">
-  %endif
+  <p>
+  ${_("Next, select an action to perform for the given user and problem:")}
+  </p>
+
+  <p>
+    <!-- Doesn't give any type of notification upon success -->
+  <input type="button" name="reset-attempts-single" value="${_("Reset Student Attempts")}" data-endpoint="${ section_data['reset_student_attempts_url'] }">
 
   %if settings.MITX_FEATURES.get('ENABLE_INSTRUCTOR_BACKGROUND_TASKS') and section_data['access']['instructor']:
     <input type="button" name="rescore-problem-single" value="${_("Rescore Student Submission")}" data-endpoint="${ section_data['rescore_problem_url'] }">
   %endif
+  </p>
+
+  <p>
+  %if section_data['access']['instructor']:
+    <p> ${_('You may also delete the entire state of a student for the specified problem:')} </p>
+    <input type="button" class="molly-guard" name="delete-state-single" value="${_("Delete Student State for Problem")}" data-endpoint="${ section_data['reset_student_attempts_url'] }">
+  %endif
+  </p>
+
 
   %if settings.MITX_FEATURES.get('ENABLE_INSTRUCTOR_BACKGROUND_TASKS') and section_data['access']['instructor']:
     <p>
-      ${_("Rescoring runs in the background, and status for active tasks will appear in a table below. "
+      ${_("Rescoring runs in the background, and status for active tasks will appear in a table on the Course Info tab. "
       "To see status for all tasks submitted for this problem and student, click on this button:")}
     </p>
 
@@ -76,18 +111,11 @@
     </p>
     <p>
     <p>
-      ${_("These actions run in the background, and status for active tasks will appear in a table below. "
+      ${_("These actions run in the background, and status for active tasks will appear in a table on the Course Info tab. "
       "To see status for all tasks submitted for this problem, click on this button")}:
     </p>
       <input type="button" name="task-history-all" value="${_("Show Background Task History for Problem")}" data-endpoint="${ section_data['list_instructor_tasks_url'] }">
       <div class="task-history-all-table"></div>
     </p>
   </div>
-
-
-  <div class="running-tasks-container action-type-container">
-    <hr>
-    <h2> ${_("Pending Instructor Tasks")} </h2>
-    <div class="running-tasks-table" data-endpoint="${ section_data['list_instructor_tasks_url'] }"></div>
-  </div>
 %endif