Make event handlers fire properly

Respond to review comments

LMS-1242

lms/djangoapps/instructor/tests/test_api.py

@@ -770,8 +770,8 @@ class MockCompletionInfo(object):
         """Mock for get_task_completion_info"""
         self.times_called += 1
         if self.times_called % 2 == 0:
-            return (True, 'Task Completed')
-        return (False, 'Task Errored In Some Way')
+            return True, 'Task Completed'
+        return False, 'Task Errored In Some Way'
 
 
 @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
@@ -791,34 +791,36 @@ class TestInstructorAPITaskLists(ModuleStoreTestCase, LoginEnrollmentTestCase):
             'created',
             'status',
             'task_message',
-            'duration_sec',
-            'task_output'
+            'duration_sec'
         ]
 
         def __init__(self, completion):
             for feature in self.FEATURES:
                 setattr(self, feature, 'expected')
-            # Make 'created' into a datetime
-            setattr(self, 'created', datetime.datetime(2013, 10, 25, 11, 42, 35))
+            # created needs to be a datetime
+            self.created = datetime.datetime(2013, 10, 25, 11, 42, 35)
             # set 'status' and 'task_message' attrs
             success, task_message = completion()
             if success:
-                setattr(self, 'status', "Complete")
+                self.status = "Complete"
             else:
-                setattr(self, 'status', "Incomplete")
-            setattr(self, 'task_message', task_message)
+                self.status = "Incomplete"
+            self.task_message = task_message
             # Set 'task_output' attr, which will be parsed to the 'duration_sec' attr.
-            setattr(self, 'task_output', '{"duration_ms": 1035000}')
-            setattr(self, 'duration_sec', 1035000 / 1000.0)
+            self.task_output = '{"duration_ms": 1035000}'
+            self.duration_sec = 1035000 / 1000.0
 
+        def make_invalid_output(self):
+            """Munge task_output to be invalid json"""
+            self.task_output = 'HI MY NAME IS INVALID JSON'
+            # This should be given the value of 'unknown' if the task output
+            # can't be properly parsed
+            self.duration_sec = 'unknown'
 
         def to_dict(self):
             """ Convert fake task to dictionary representation. """
             attr_dict = {key: getattr(self, key) for key in self.FEATURES}
             attr_dict['created'] = attr_dict['created'].isoformat()
-            # Don't actually want task_output in the attribute dictionary, as this
-            # is not explicitly extracted in extract_task_features
-            del attr_dict['task_output']
             return attr_dict
 
     def setUp(self):
@@ -840,7 +842,8 @@ class TestInstructorAPITaskLists(ModuleStoreTestCase, LoginEnrollmentTestCase):
             state=json.dumps({'attempts': 10}),
         )
         mock_factory = MockCompletionInfo()
-        self.tasks = [self.FakeTask(mock_factory.mock_get_task_completion_info) for _ in xrange(6)]
+        self.tasks = [self.FakeTask(mock_factory.mock_get_task_completion_info) for _ in xrange(7)]
+        self.tasks[-1].make_invalid_output()
 
     def tearDown(self):
         """

lms/djangoapps/instructor/views/api.py

@@ -690,21 +690,25 @@ def list_instructor_tasks(request, course_id):
         """
         # Pull out information from the task
         features = ['task_type', 'task_input', 'task_id', 'requester', 'task_state']
-        task_feature_dict = dict((feature, str(getattr(task, feature))) for feature in features)
+        task_feature_dict = {feature: str(getattr(task, feature)) for feature in features}
         # Some information (created, duration, status, task message) require additional formatting
         task_feature_dict['created'] = task.created.isoformat()
 
         # Get duration info, if known
         duration_sec = 'unknown'
         if hasattr(task, 'task_output') and task.task_output is not None:
-            task_output = json.loads(task.task_output)
-            if 'duration_ms' in task_output:
-                duration_sec = int(task_output['duration_ms'] / 1000.0)
+            try:
+                task_output = json.loads(task.task_output)
+            except ValueError:
+                log.error("Could not parse task output as valid json; task output: %s", task.task_output)
+            else:
+                if 'duration_ms' in task_output:
+                    duration_sec = int(task_output['duration_ms'] / 1000.0)
         task_feature_dict['duration_sec'] = duration_sec
 
         # Get progress status message & success information
         success, task_message = get_task_completion_info(task)
-        status = "Complete" if success else "Incomplete"
+        status = _("Complete") if success else _("Incomplete")
         task_feature_dict['status'] = status
         task_feature_dict['task_message'] = task_message
 

lms/djangoapps/instructor/views/instructor_dashboard.py

@@ -95,7 +95,7 @@ def _section_course_info(course_id):
     """ Provide data for the corresponding dashboard section """
     course = get_course_by_id(course_id, depth=None)
 
-    (course_org, course_num, course_name) = course_id.split('/')
+    course_org, course_num, course_name = course_id.split('/')
 
     section_data = {
         'section_key': 'course_info',

lms/static/coffee/src/instructor_dashboard/analytics.coffee

@@ -230,9 +230,7 @@ class Analytics
 
 # export for use
 # create parent namespaces if they do not already exist.
-# abort if underscore can not be found.
-if _?
-  _.defaults window, InstructorDashboard: {}
-  _.defaults window.InstructorDashboard, sections: {}
-  _.defaults window.InstructorDashboard.sections,
-    Analytics: Analytics
+_.defaults window, InstructorDashboard: {}
+_.defaults window.InstructorDashboard, sections: {}
+_.defaults window.InstructorDashboard.sections,
+  Analytics: Analytics

lms/static/coffee/src/instructor_dashboard/course_info.coffee

@@ -7,8 +7,6 @@ such that the value can be defined later than this assignment (file load order).
 ###
 
 # Load utilities
-plantTimeout = -> window.InstructorDashboard.util.plantTimeout.apply this, arguments
-std_ajax_err = -> window.InstructorDashboard.util.std_ajax_err.apply this, arguments
 PendingInstructorTasks = -> window.InstructorDashboard.util.PendingInstructorTasks
 
 # A typical section object.
@@ -16,6 +14,12 @@ PendingInstructorTasks = -> window.InstructorDashboard.util.PendingInstructorTas
 # which holds the section body container.
 class CourseInfo
   constructor: (@$section) ->
+    # attach self to html so that instructor_dashboard.coffee can find
+    #  this object to call event handlers like 'onClickTitle'
+    @$section.data 'wrapper', @
+
+    # gather elements
+    @instructor_tasks = new (PendingInstructorTasks()) @$section
     @$course_errors_wrapper = @$section.find '.course-errors-wrapper'
 
     # if there are errors
@@ -37,19 +41,15 @@ class CourseInfo
         else
           @$course_errors_wrapper.addClass 'open'
 
-    @instructor_tasks = new (PendingInstructorTasks()) @$section
-
   # handler for when the section title is clicked.
-  onClickTitle: -> @instructor_tasks.task_poller?.start()
+  onClickTitle: -> @instructor_tasks.task_poller.start()
 
   # handler for when the section is closed
-  onExit: -> @instructor_tasks.task_poller?.stop()
+  onExit: -> @instructor_tasks.task_poller.stop()
 
 # export for use
 # create parent namespaces if they do not already exist.
-# abort if underscore can not be found.
-if _?
-  _.defaults window, InstructorDashboard: {}
-  _.defaults window.InstructorDashboard, sections: {}
-  _.defaults window.InstructorDashboard.sections,
-    CourseInfo: CourseInfo
+_.defaults window, InstructorDashboard: {}
+_.defaults window.InstructorDashboard, sections: {}
+_.defaults window.InstructorDashboard.sections,
+  CourseInfo: CourseInfo

lms/static/coffee/src/instructor_dashboard/data_download.coffee

@@ -6,13 +6,16 @@ wrap in (-> ... apply) to defer evaluation
 such that the value can be defined later than this assignment (file load order).
 ###
 
-plantTimeout = -> window.InstructorDashboard.util.plantTimeout.apply this, arguments
+# Load utilities
 std_ajax_err = -> window.InstructorDashboard.util.std_ajax_err.apply this, arguments
 PendingInstructorTasks = -> window.InstructorDashboard.util.PendingInstructorTasks
 
 # Data Download Section
 class DataDownload
   constructor: (@$section) ->
+    # attach self to html so that instructor_dashboard.coffee can find
+    #  this object to call event handlers like 'onClickTitle'
+    @$section.data 'wrapper', @
     # gather elements
     @$display                = @$section.find '.data-display'
     @$display_text           = @$display.find '.data-display-text'
@@ -21,9 +24,9 @@ class DataDownload
     @$list_studs_btn = @$section.find("input[name='list-profiles']'")
     @$list_anon_btn = @$section.find("input[name='list-anon-ids']'")
     @$grade_config_btn = @$section.find("input[name='dump-gradeconf']'")
+    @instructor_tasks = new (PendingInstructorTasks()) @$section
 
     # attach click handlers
-
     # The list-anon case is always CSV
     @$list_anon_btn.click (e) =>
       url = @$list_anon_btn.data 'endpoint'
@@ -80,13 +83,11 @@ class DataDownload
           @clear_display()
           @$display_text.html data['grading_config_summary']
 
-    @instructor_tasks = new (PendingInstructorTasks()) @$section
-
   # handler for when the section title is clicked.
-  onClickTitle: -> @instructor_tasks.task_poller?.start()
+  onClickTitle: -> @instructor_tasks.task_poller.start()
 
   # handler for when the section is closed
-  onExit: -> @instructor_tasks.task_poller?.stop()
+  onExit: -> @instructor_tasks.task_poller.stop()
 
   clear_display: ->
     @$display_text.empty()
@@ -96,9 +97,7 @@ class DataDownload
 
 # export for use
 # create parent namespaces if they do not already exist.
-# abort if underscore can not be found.
-if _?
-  _.defaults window, InstructorDashboard: {}
-  _.defaults window.InstructorDashboard, sections: {}
-  _.defaults window.InstructorDashboard.sections,
-    DataDownload: DataDownload
+_.defaults window, InstructorDashboard: {}
+_.defaults window.InstructorDashboard, sections: {}
+_.defaults window.InstructorDashboard.sections,
+  DataDownload: DataDownload

lms/static/coffee/src/instructor_dashboard/instructor_dashboard.coffee

@@ -118,7 +118,7 @@ setup_instructor_dashboard = (idash_content) =>
       location.hash = "#{HASH_LINK_PREFIX}#{section_name}"
 
       sections_have_loaded.after ->
-        $section.data('wrapper')?.onClickTitle?()
+        $section.data('wrapper').onClickTitle()
 
       # call onExit handler if exiting a section to a different section.
       unless $section.is $active_section

lms/static/coffee/src/instructor_dashboard/membership.coffee

@@ -487,9 +487,7 @@ class Membership
 
 # export for use
 # create parent namespaces if they do not already exist.
-# abort if underscore can not be found.
-if _?
-  _.defaults window, InstructorDashboard: {}
-  _.defaults window.InstructorDashboard, sections: {}
-  _.defaults window.InstructorDashboard.sections,
-    Membership: Membership
+_.defaults window, InstructorDashboard: {}
+_.defaults window.InstructorDashboard, sections: {}
+_.defaults window.InstructorDashboard.sections,
+  Membership: Membership

lms/static/coffee/src/instructor_dashboard/send_email.coffee

@@ -81,9 +81,8 @@ class SendEmail
 class Email
   # enable subsections.
   constructor: (@$section) ->
-    # attach self to html
-    # so that instructor_dashboard.coffee can find this object
-    # to call event handlers like 'onClickTitle'
+    # attach self to html so that instructor_dashboard.coffee can find
+    #  this object to call event handlers like 'onClickTitle'
     @$section.data 'wrapper', @
 
     # isolate # initialize SendEmail subsection
@@ -92,17 +91,15 @@ class Email
     @instructor_tasks = new (PendingInstructorTasks()) @$section
 
   # handler for when the section title is clicked.
-  onClickTitle: -> @instructor_tasks.task_poller?.start()
+  onClickTitle: -> @instructor_tasks.task_poller.start()
 
   # handler for when the section is closed
-  onExit: -> @instructor_tasks.task_poller?.stop()
+  onExit: -> @instructor_tasks.task_poller.stop()
 
 
 # export for use
 # create parent namespaces if they do not already exist.
-# abort if underscore can not be found.
-if _?
-  _.defaults window, InstructorDashboard: {}
-  _.defaults window.InstructorDashboard, sections: {}
-  _.defaults window.InstructorDashboard.sections,
-    Email: Email
+_.defaults window, InstructorDashboard: {}
+_.defaults window.InstructorDashboard, sections: {}
+_.defaults window.InstructorDashboard.sections,
+  Email: Email

lms/static/coffee/src/instructor_dashboard/student_admin.coffee

@@ -7,10 +7,7 @@ such that the value can be defined later than this assignment (file load order).
 ###
 
 # Load utilities
-plantTimeout = -> window.InstructorDashboard.util.plantTimeout.apply this, arguments
-plantInterval = -> window.InstructorDashboard.util.plantInterval.apply this, arguments
 std_ajax_err = -> window.InstructorDashboard.util.std_ajax_err.apply this, arguments
-load_IntervalManager = -> window.InstructorDashboard.util.IntervalManager
 create_task_list_table = -> window.InstructorDashboard.util.create_task_list_table.apply this, arguments
 PendingInstructorTasks = -> window.InstructorDashboard.util.PendingInstructorTasks
 
@@ -27,6 +24,8 @@ find_and_assert = ($root, selector) ->
 
 class StudentAdmin
   constructor: (@$section) ->
+    # attach self to html so that instructor_dashboard.coffee can find
+    #  this object to call event handlers like 'onClickTitle'
     @$section.data 'wrapper', @
 
     # gather buttons
@@ -255,17 +254,15 @@ class StudentAdmin
     @$request_response_error_all.empty()
 
   # handler for when the section title is clicked.
-  onClickTitle: -> @instructor_tasks.task_poller?.start()
+  onClickTitle: -> @instructor_tasks.task_poller.start()
 
   # handler for when the section is closed
-  onExit: -> @instructor_tasks.task_poller?.stop()
+  onExit: -> @instructor_tasks.task_poller.stop()
 
 
 # export for use
 # create parent namespaces if they do not already exist.
-# abort if underscore can not be found.
-if _?
-  _.defaults window, InstructorDashboard: {}
-  _.defaults window.InstructorDashboard, sections: {}
-  _.defaults window.InstructorDashboard.sections,
-    StudentAdmin: StudentAdmin
+_.defaults window, InstructorDashboard: {}
+_.defaults window.InstructorDashboard, sections: {}
+_.defaults window.InstructorDashboard.sections,
+  StudentAdmin: StudentAdmin

lms/static/coffee/src/instructor_dashboard/util.coffee

@@ -101,8 +101,8 @@ class IntervalManager
     @intervalID = null
 
   # Start or restart firing every `ms` milliseconds.
-  # Soes not fire immediately.
   start: ->
+    @fn()
     if @intervalID is null
       @intervalID = setInterval @fn, @ms
 

lms/templates/instructor/instructor_dashboard_2/course_info.html

@@ -37,11 +37,7 @@
     <li class="field text is-not-editable" id="field-course-started">
       <label for="start-date">${_("Has the course started?")}</label>
 
-      %if section_data['has_started']:
-      <b>${_("Yes")}</b>
-      %else:
-      <b>${_("No")}</b>
-      %endif
+      <b>${_("Yes") if section_data['grade_cutoffs'] else _("No")}</b>
 
     </li>