Merge pull request #1976 from edx/vik/fix/oe-bugs

Vik/fix/oe bugs

common/lib/xmodule/xmodule/combined_open_ended_module.py

@@ -203,9 +203,7 @@ class CombinedOpenEndedModule(CombinedOpenEndedFields, XModule):
 
     def save_instance_data(self):
         for attribute in self.student_attributes:
-            child_attr = getattr(self.child_module, attribute)
-            if child_attr != getattr(self, attribute):
-                setattr(self, attribute, getattr(self.child_module, attribute))
+            setattr(self, attribute, getattr(self.child_module, attribute))
 
 
 class CombinedOpenEndedDescriptor(CombinedOpenEndedFields, RawDescriptor):

common/lib/xmodule/xmodule/js/src/peergrading/peer_grading.coffee

@@ -8,20 +8,23 @@ class @PeerGrading
     @use_single_location = @peer_grading_container.data('use-single-location')
     @peer_grading_outer_container = $('.peer-grading-container')
     @ajax_url = @peer_grading_container.data('ajax-url')
-    @error_container = $('.error-container')
-    @error_container.toggle(not @error_container.is(':empty'))
 
-    @message_container = $('.message-container')
-    @message_container.toggle(not @message_container.is(':empty'))
+    if @use_single_location.toLowerCase() == "true"
+      #If the peer grading element is linked to a single location, then activate the backend for that location
+      @activate_problem()
+    else
+      #Otherwise, activate the panel view.
+      @error_container = $('.error-container')
+      @error_container.toggle(not @error_container.is(':empty'))
 
-    @problem_button = $('.problem-button')
-    @problem_button.click @show_results
+      @message_container = $('.message-container')
+      @message_container.toggle(not @message_container.is(':empty'))
 
-    @problem_list = $('.problem-list')
-    @construct_progress_bar()
+      @problem_button = $('.problem-button')
+      @problem_button.click @show_results
 
-    if @use_single_location
-      @activate_problem()
+      @problem_list = $('.problem-list')
+      @construct_progress_bar()
 
   construct_progress_bar: () =>
     problems = @problem_list.find('tr').next()

common/lib/xmodule/xmodule/peer_grading_module.py

@@ -11,7 +11,7 @@ from xmodule.raw_module import RawDescriptor
 from xmodule.modulestore.django import modulestore
 from .timeinfo import TimeInfo
 from xblock.core import Object, Integer, Boolean, String, Scope
-from xmodule.fields import Date, StringyFloat
+from xmodule.fields import Date, StringyFloat, StringyInteger, StringyBoolean
 
 from xmodule.open_ended_grading_classes.peer_grading_service import PeerGradingService, GradingServiceError, MockPeerGradingService
 from open_ended_grading_classes import combined_open_ended_rubric
@@ -28,14 +28,14 @@ EXTERNAL_GRADER_NO_CONTACT_ERROR = "Failed to contact external graders.  Please
 
 
 class PeerGradingFields(object):
-    use_for_single_location = Boolean(help="Whether to use this for a single location or as a panel.",
+    use_for_single_location = StringyBoolean(help="Whether to use this for a single location or as a panel.",
                                       default=USE_FOR_SINGLE_LOCATION, scope=Scope.settings)
     link_to_location = String(help="The location this problem is linked to.", default=LINK_TO_LOCATION,
                               scope=Scope.settings)
-    is_graded = Boolean(help="Whether or not this module is scored.", default=IS_GRADED, scope=Scope.settings)
+    is_graded = StringyBoolean(help="Whether or not this module is scored.", default=IS_GRADED, scope=Scope.settings)
     due_date = Date(help="Due date that should be displayed.", default=None, scope=Scope.settings)
     grace_period_string = String(help="Amount of grace to give on the due date.", default=None, scope=Scope.settings)
-    max_grade = Integer(help="The maximum grade that a student can receieve for this problem.", default=MAX_SCORE,
+    max_grade = StringyInteger(help="The maximum grade that a student can receieve for this problem.", default=MAX_SCORE,
                         scope=Scope.settings)
     student_data_for_location = Object(help="Student data for a given peer grading problem.",
                                        scope=Scope.user_state)
@@ -93,9 +93,9 @@ class PeerGradingModule(PeerGradingFields, XModule):
         if not self.ajax_url.endswith("/"):
             self.ajax_url = self.ajax_url + "/"
 
-        if not isinstance(self.max_grade, (int, long)):
-            #This could result in an exception, but not wrapping in a try catch block so it moves up the stack
-            self.max_grade = int(self.max_grade)
+        #StringyInteger could return None, so keep this check.
+        if not isinstance(self.max_grade, int):
+            raise TypeError("max_grade needs to be an integer.")
 
     def closed(self):
         return self._closed(self.timeinfo)

lms/djangoapps/open_ended_grading/open_ended_notifications.py

@@ -11,6 +11,7 @@ from util.cache import cache
 import datetime
 from xmodule.x_module import ModuleSystem
 from mitxmako.shortcuts import render_to_string
+import datetime
 
 log = logging.getLogger(__name__)
 
@@ -104,6 +105,25 @@ def peer_grading_notifications(course, user):
 
 
 def combined_notifications(course, user):
+    """
+    Show notifications to a given user for a given course.  Get notifications from the cache if possible,
+    or from the grading controller server if not.
+    @param course: The course object for which we are getting notifications
+    @param user: The user object for which we are getting notifications
+    @return: A dictionary with boolean pending_grading (true if there is pending grading), img_path (for notification
+    image), and response (actual response from grading controller server).
+    """
+    #Set up return values so that we can return them for error cases
+    pending_grading = False
+    img_path = ""
+    notifications={}
+    notification_dict = {'pending_grading': pending_grading, 'img_path': img_path, 'response': notifications}
+
+    #We don't want to show anonymous users anything.
+    if not user.is_authenticated():
+        return notification_dict
+
+    #Define a mock modulesystem
     system = ModuleSystem(
         ajax_url=None,
         track_function=None,
@@ -112,41 +132,44 @@ def combined_notifications(course, user):
         replace_urls=None,
         xblock_model_data= {}
     )
+    #Initialize controller query service using our mock system
     controller_qs = ControllerQueryService(settings.OPEN_ENDED_GRADING_INTERFACE, system)
     student_id = unique_id_for_user(user)
     user_is_staff = has_access(user, course, 'staff')
     course_id = course.id
     notification_type = "combined"
 
+    #See if we have a stored value in the cache
     success, notification_dict = get_value_from_cache(student_id, course_id, notification_type)
     if success:
         return notification_dict
 
-    min_time_to_query = user.last_login
+    #Get the time of the last login of the user
+    last_login = user.last_login
+
+    #Find the modules they have seen since they logged in
     last_module_seen = StudentModule.objects.filter(student=user, course_id=course_id,
-                                                    modified__gt=min_time_to_query).values('modified').order_by(
+                                                    modified__gt=last_login).values('modified').order_by(
         '-modified')
     last_module_seen_count = last_module_seen.count()
 
     if last_module_seen_count > 0:
+        #The last time they viewed an updated notification (last module seen minus how long notifications are cached)
         last_time_viewed = last_module_seen[0]['modified'] - datetime.timedelta(seconds=(NOTIFICATION_CACHE_TIME + 60))
     else:
-        last_time_viewed = user.last_login
-
-    pending_grading = False
+        #If they have not seen any modules since they logged in, then don't refresh
+        return {'pending_grading': False, 'img_path': img_path, 'response': notifications}
 
-    img_path = ""
     try:
+        #Get the notifications from the grading controller
         controller_response = controller_qs.check_combined_notifications(course.id, student_id, user_is_staff,
                                                                          last_time_viewed)
-        log.debug(controller_response)
         notifications = json.loads(controller_response)
         if notifications['success']:
             if notifications['overall_need_to_check']:
                 pending_grading = True
     except:
         #Non catastrophic error, so no real action
-        notifications = {}
         #This is a dev_facing_error
         log.exception(
             "Problem with getting notifications from controller query service for course {0} user {1}.".format(
@@ -157,6 +180,7 @@ def combined_notifications(course, user):
 
     notification_dict = {'pending_grading': pending_grading, 'img_path': img_path, 'response': notifications}
 
+    #Store the notifications in the cache
     set_value_in_cache(student_id, course_id, notification_type, notification_dict)
 
     return notification_dict