pull from master

common/djangoapps/util/memcache.py

@@ -8,15 +8,42 @@ import urllib
 
 
 def fasthash(string):
-    m = hashlib.new("md4")
-    m.update(string)
-    return m.hexdigest()
+    """
+    Hashes `string` into a string representation of a 128-bit digest.
+    """
+    md4 = hashlib.new("md4")
+    md4.update(string)
+    return md4.hexdigest()
+
+
+def cleaned_string(val):
+    """
+    Converts `val` to unicode and URL-encodes special characters
+    (including quotes and spaces)
+    """
+    return urllib.quote_plus(smart_str(val))
 
 
 def safe_key(key, key_prefix, version):
-    safe_key = urllib.quote_plus(smart_str(key))
+    """
+    Given a `key`, `key_prefix`, and `version`,
+    return a key that is safe to use with memcache.
+
+    `key`, `key_prefix`, and `version` can be numbers, strings, or unicode.
+    """
+
+    # Clean for whitespace and control characters, which
+    # cause memcache to raise an exception
+    key = cleaned_string(key)
+    key_prefix = cleaned_string(key_prefix)
+    version = cleaned_string(version)
+
+    # Attempt to combine the prefix, version, and key
+    combined = ":".join([key_prefix, version, key])
 
-    if len(safe_key) > 250:
-        safe_key = fasthash(safe_key)
+    # If the total length is too long for memcache, hash it
+    if len(combined) > 250:
+        combined = fasthash(combined)
 
-    return ":".join([key_prefix, str(version), safe_key])
+    # Return the result
+    return combined

common/djangoapps/util/tests/__init__.py

+

common/djangoapps/util/tests/test_memcache.py

+"""
+Tests for memcache in util app
+"""
+
+from django.test import TestCase
+from django.core.cache import get_cache
+from django.conf import settings
+from util.memcache import safe_key
+
+
+class MemcacheTest(TestCase):
+    """
+    Test memcache key cleanup
+    """
+
+    # Test whitespace, control characters, and some non-ASCII UTF-16
+    UNICODE_CHAR_CODES = ([c for c in range(0, 30)] + [127] +
+                          [129, 500, 2 ** 8 - 1, 2 ** 8 + 1, 2 ** 16 - 1])
+
+    def setUp(self):
+        self.cache = get_cache('default')
+
+    def test_safe_key(self):
+        key = safe_key('test', 'prefix', 'version')
+        self.assertEqual(key, 'prefix:version:test')
+
+    def test_numeric_inputs(self):
+
+        # Numeric key
+        self.assertEqual(safe_key(1, 'prefix', 'version'), 'prefix:version:1')
+
+        # Numeric prefix
+        self.assertEqual(safe_key('test', 5, 'version'), '5:version:test')
+
+        # Numeric version
+        self.assertEqual(safe_key('test', 'prefix', 5), 'prefix:5:test')
+
+    def test_safe_key_long(self):
+
+        # Choose lengths close to memcached's cutoff (250)
+        for length in [248, 249, 250, 251, 252]:
+
+            # Generate a key of that length
+            key = 'a' * length
+
+            # Make the key safe
+            key = safe_key(key, '', '')
+
+            # The key should now be valid
+            self.assertTrue(self._is_valid_key(key),
+                            msg="Failed for key length {0}".format(length))
+
+    def test_long_key_prefix_version(self):
+
+        # Long key
+        key = safe_key('a' * 300, 'prefix', 'version')
+        self.assertTrue(self._is_valid_key(key))
+
+        # Long prefix
+        key = safe_key('key', 'a' * 300, 'version')
+        self.assertTrue(self._is_valid_key(key))
+
+        # Long version
+        key = safe_key('key', 'prefix', 'a' * 300)
+        self.assertTrue(self._is_valid_key(key))
+
+    def test_safe_key_unicode(self):
+
+        for unicode_char in self.UNICODE_CHAR_CODES:
+
+            # Generate a key with that character
+            key = unichr(unicode_char)
+
+            # Make the key safe
+            key = safe_key(key, '', '')
+
+            # The key should now be valid
+            self.assertTrue(self._is_valid_key(key),
+                            msg="Failed for unicode character {0}".format(unicode_char))
+
+    def test_safe_key_prefix_unicode(self):
+
+        for unicode_char in self.UNICODE_CHAR_CODES:
+
+            # Generate a prefix with that character
+            prefix = unichr(unicode_char)
+
+            # Make the key safe
+            key = safe_key('test', prefix, '')
+
+            # The key should now be valid
+            self.assertTrue(self._is_valid_key(key),
+                            msg="Failed for unicode character {0}".format(unicode_char))
+
+    def test_safe_key_version_unicode(self):
+
+        for unicode_char in self.UNICODE_CHAR_CODES:
+
+            # Generate a version with that character
+            version = unichr(unicode_char)
+
+            # Make the key safe
+            key = safe_key('test', '', version)
+
+            # The key should now be valid
+            self.assertTrue(self._is_valid_key(key),
+                            msg="Failed for unicode character {0}".format(unicode_char))
+
+    def _is_valid_key(self, key):
+        """
+        Test that a key is memcache-compatible.
+        Based on Django's validator in core.cache.backends.base
+        """
+
+        # Check the length
+        if len(key) > 250:
+            return False
+
+        # Check that there are no spaces or control characters
+        for char in key:
+            if ord(char) < 33 or ord(char) == 127:
+                return False
+
+        return True

common/djangoapps/util/tests.py → common/djangoapps/util/tests/test_zendesk.py

-"""Tests for the util package"""
+"""Tests for the Zendesk"""
 
 from django.conf import settings
 from django.contrib.auth.models import AnonymousUser

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

lms/static/sass/shared/_modal.scss

@@ -149,7 +149,7 @@
       }
 
       label {
-        color: #999;
+        color: #646464;
 
         &.field-error {
           display: block;

lms/templates/courseware/courseware.html

@@ -156,7 +156,7 @@
         <div id="calculator_wrapper">
             <form id="calculator">
                 <div class="input-wrapper">
-                    <input type="text" id="calculator_input" />
+                    <input type="text" id="calculator_input" title="Calculator Input Field" />
 
                     <div class="help-wrapper">
                         <a href="#">Hints</a>
@@ -176,8 +176,8 @@
                         </dl>
                     </div>
                 </div>
-                <input id="calculator_button" type="submit" value="="/>
-                <input type="text" id="calculator_output"  readonly />
+                <input id="calculator_button" type="submit" title="Calculate" value="="/>
+                <input type="text" id="calculator_output" title="Calculator Output Field" readonly />
             </form>
 
         </div>

lms/templates/forgot_password_modal.html

@@ -12,19 +12,19 @@
       </div>
 
       <form id="pwd_reset_form" action="${reverse('password_reset')}" method="post" data-remote="true">
-        <label for="id_email">E-mail address:</label>
-        <input id="id_email" type="email" name="email" maxlength="75" placeholder="Your E-mail"/>
+        <label for="pwd_reset_email">E-mail address:</label>
+        <input id="pwd_reset_email" type="email" name="email" maxlength="75" placeholder="Your E-mail"/>
         <div class="submit">
           <input type="submit" id="pwd_reset_button" value="Reset my password" />
         </div>
       </form>
     </div>
 
-    <div class="close-modal">
+    <a href="#" class="close-modal" title="Close Modal">
       <div class="inner">
         <p>&#10005;</p>
       </div>
-    </div>
+    </a>
   </div>
 </section>
 
@@ -40,5 +40,10 @@
        $('#pwd_error').stop().css("display", "block");
      }
    });
+   
+   // removing close link's default behavior
+   $('#login-modal .close-modal').click(function(e) {
+    e.preventDefault();
+   });
   })(this)
 </script>

lms/templates/login_modal.html

@@ -9,14 +9,17 @@
     </header>
 
     <form id="login_form" class="login_form" method="post" data-remote="true" action="/login">
-      <label>E-mail</label>
-      <input name="email" type="email">
-      <label>Password</label>
-      <input name="password" type="password">
-      <label class="remember-me">
-        <input name="remember" type="checkbox" value="true">
+      <label for="login_email">E-mail</label>
+        <input id="login_email" type="email" name="email" placeholder="e.g. yourname@domain.com" />
+      
+      <label for="login_password">Password</label>
+        <input id="login_password" type="password" name="password" placeholder="&bull;&bull;&bull;&bull;&bull;&bull;&bull;&bull;" />  
+      
+      <label for="login_remember_me" class="remember-me">
+        <input id="login_remember_me" type="checkbox" name="remember" value="true" />
         Remember me
       </label>
+      
       <div class="submit">
         <input name="submit" type="submit" value="Access My Courses">
       </div>
@@ -34,11 +37,11 @@
 % endif
     </section>
 
-    <div class="close-modal">
+    <a href="#" class="close-modal" title="Close Modal">
       <div class="inner">
         <p>&#10005;</p>
       </div>
-    </div>
+    </a>
   </div>
 </section>
 
@@ -59,5 +62,10 @@
        $('#login_error').html(json.value).stop().css("display", "block");
      }
    });
+   
+   // removing close link's default behavior
+   $('#login-modal .close-modal').click(function(e) {
+    e.preventDefault();
+   });
 })(this)
 </script>

lms/templates/seq_module.html

@@ -10,7 +10,8 @@
         <li>
         <a class="seq_${item['type']} inactive progress-${item['progress_status']}"
            data-id="${item['id']}"
-           data-element="${idx+1}">
+           data-element="${idx+1}"
+           href="javascript:void(0);">
             <p>${item['title']}</p>
           </a>
         </li>

lms/templates/signup_modal.html

@@ -20,27 +20,31 @@
 
         <div class="input-group">
 	  % if has_extauth_info is UNDEFINED:
-          <label data-field="email">E-mail*</label>
-          <input name="email" type="email" placeholder="eg. yourname@domain.com">
-          <label data-field="password">Password*</label>
-          <input name="password" type="password" placeholder="****">
-          <label data-field="username">Public Username*</label>
-          <input name="username" type="text" placeholder="Shown on forums">
-          <label data-field="name">Full Name*</label>
-          <input name="name" type="text" placeholder="For your certificate">
+          <label data-field="email" for="signup_email">E-mail *</label>
+	    <input id="signup_email" type="email" name="email" placeholder="e.g. yourname@domain.com" required />
+	  
+          <label data-field="password" for="signup_password">Password *</label>
+	    <input id="signup_password" type="password" name="password" placeholder="&bull;&bull;&bull;&bull;&bull;&bull;&bull;&bull;" required />
+          
+	  <label data-field="username" for="signup_username">Public Username *</label>
+	    <input id="signup_username" type="text" name="username" placeholder="e.g. yourname (shown on forums)" required />
+	  
+          <label data-field="name" for="signup_fullname">Full Name *</label>
+	    <input id="signup_fullname" type="text" name="name" placeholder="e.g. Your Name (for certificates)" required />
 	  % else:
 	  <p><i>Welcome</i> ${extauth_email}</p><br/>
 	  <p><i>Enter a public username:</i></p>
-          <label data-field="username">Public Username*</label>
-          <input name="username" type="text" value="${extauth_username}" placeholder="Shown on forums">
+	  
+          <label data-field="username" for="signup_username">Public Username *</label>
+	    <input id="signup_username" type="text" name="username" value="${extauth_username}" placeholder="e.g. yourname (shown on forums)" required />
 	  % endif
         </div>
 
         <div class="input-group">
           <section class="citizenship">
-            <label data-field="level_of_education">Ed. completed</label>
+            <label data-field="level_of_education" for="signup_ed_level">Ed. Completed</label>
             <div class="input-wrapper">
-              <select name="level_of_education">
+              <select id="signup_ed_level" name="level_of_education">
                   <option value="">--</option>
                   %for code, ed_level in UserProfile.LEVEL_OF_EDUCATION_CHOICES:
                       <option value="${code}">${ed_level}</option>
@@ -50,9 +54,9 @@
           </section>
 
           <section class="gender">
-            <label data-field="gender">Gender</label>
+            <label data-field="gender" for="signup_gender">Gender</label>
             <div class="input-wrapper">
-              <select name="gender">
+              <select id="signup_gender" name="gender">
                   <option value="">--</option>
                   %for code, gender in UserProfile.GENDER_CHOICES:
                       <option value="${code}">${gender}</option>
@@ -62,9 +66,9 @@
           </section>
 
           <section class="date-of-birth">
-            <label data-field="date-of-birth">Year of birth</label>
+            <label data-field="date-of-birth" for="signup_birth_year">Year of birth</label>
             <div class="input-wrapper">
-              <select name="year_of_birth">
+              <select id="signup_birth_year" name="year_of_birth">
                 <option value="">--</option>
                 %for year in UserProfile.VALID_YEARS:
                   <option value="${year}">${year}</option>
@@ -74,22 +78,23 @@
             </div>
           </section>
 
-          <label data-field="mailing_address">Mailing address</label>
-          <textarea name="mailing_address"></textarea>
-          <label data-field="goals">Goals in signing up for edX</label>
-          <textarea name="goals"></textarea>
+          <label data-field="mailing_address" for="signup_mailing_address">Mailing address</label>
+	    <textarea id="signup_mailing_address" name="mailing_address"></textarea>
+	  
+          <label data-field="goals" for="signup_goals">Goals in signing up for edX</label>
+	    <textarea name="goals" id="signup_goals"></textarea>
 
         </div>
 
         <div class="input-group">
-          <label data-field="terms_of_service" class="terms-of-service">
-            <input name="terms_of_service" type="checkbox" value="true">
+          <label data-field="terms_of_service" class="terms-of-service" for="signup_tos">
+            <input id="signup_tos" name="terms_of_service" type="checkbox" value="true">
             I agree to the
             <a href="${reverse('tos')}" target="_blank">Terms of Service</a>*
           </label>
 
-          <label data-field="honor_code" class="honor-code">
-            <input name="honor_code" type="checkbox" value="true">
+          <label data-field="honor_code" class="honor-code" for="signup_honor">
+            <input id="signup_honor" name="honor_code" type="checkbox" value="true">
             I agree to the
             <a href="${reverse('honor')}" target="_blank">Honor Code</a>*
           </label>
@@ -110,11 +115,11 @@
 
     </div>
 
-    <div class="close-modal">
+    <a href="#" class="close-modal" title="Close Modal">
       <div class="inner">
         <p>&#10005;</p>
       </div>
-    </div>
+    </a>
   </div>
 </section>
 
@@ -129,5 +134,10 @@
        $("[data-field='"+json.field+"']").addClass('field-error')
      }
     });
+   
+   // removing close link's default behavior
+   $('#login-modal .close-modal').click(function(e) {
+    e.preventDefault();
+   });
   })(this)
 </script>