Merge branch 'master' into drupal-new

Conflicts:
	lms/envs/aws.py
	lms/envs/common.py
	lms/static/sass/base/_base.scss
	lms/templates/forgot_password_modal.html

common/djangoapps/heartbeat/urls.py

@@ -1,5 +1,5 @@
 from django.conf.urls import *
 
-urlpatterns = patterns('',
+urlpatterns = patterns('',  # nopep8
     url(r'^$', 'heartbeat.views.heartbeat', name='heartbeat'),
 )

common/djangoapps/student/tests/factories.py

@@ -2,7 +2,7 @@ from student.models import (User, UserProfile, Registration,
                             CourseEnrollmentAllowed, CourseEnrollment)
 from django.contrib.auth.models import Group
 from datetime import datetime
-from factory import DjangoModelFactory, Factory, SubFactory, PostGenerationMethodCall
+from factory import DjangoModelFactory, Factory, SubFactory, PostGenerationMethodCall, post_generation
 from uuid import uuid4
 
 
@@ -45,6 +45,16 @@ class UserFactory(DjangoModelFactory):
     last_login = datetime(2012, 1, 1)
     date_joined = datetime(2011, 1, 1)
 
+    @post_generation
+    def profile(obj, create, extracted, **kwargs):
+        if create:
+            obj.save()
+            return UserProfileFactory.create(user=obj, **kwargs)
+        elif kwargs:
+            raise Exception("Cannot build a user profile without saving the user")
+        else:
+            return None
+
 
 class AdminFactory(UserFactory):
     is_staff = True

common/djangoapps/student/views.py

@@ -77,8 +77,9 @@ def index(request, extra_context={}, user=None):
     '''
 
     # The course selection work is done in courseware.courses.
-    domain = settings.MITX_FEATURES.get('FORCE_UNIVERSITY_DOMAIN')  	# normally False
-    if domain == False:				# do explicit check, because domain=None is valid
+    domain = settings.MITX_FEATURES.get('FORCE_UNIVERSITY_DOMAIN')     # normally False
+    # do explicit check, because domain=None is valid
+    if domain == False:
         domain = request.META.get('HTTP_HOST')
 
     courses = get_courses(None, domain=domain)

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.
 
-    if len(safe_key) > 250:
-        safe_key = fasthash(safe_key)
+    `key`, `key_prefix`, and `version` can be numbers, strings, or unicode.
+    """
 
-    return ":".join([key_prefix, str(version), safe_key])
+    # 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 the total length is too long for memcache, hash it
+    if len(combined) > 250:
+        combined = fasthash(combined)
+
+    # Return the result
+    return combined

common/djangoapps/util/tests.py

@@ -1,16 +0,0 @@
-"""
-This file demonstrates writing tests using the unittest module. These will pass
-when you run "manage.py test".
-
-Replace this with more appropriate tests for your application.
-"""
-
-from django.test import TestCase
-
-
-class SimpleTest(TestCase):
-    def test_basic_addition(self):
-        """
-        Tests that 1 + 1 always equals 2.
-        """
-        self.assertEqual(1 + 1, 2)

common/djangoapps/util/tests/__init__.py

@@ -0,0 +1 @@
+

common/djangoapps/util/tests/test_memcache.py

@@ -0,0 +1,124 @@
+"""
+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/test_zendesk.py

@@ -0,0 +1,280 @@
+"""Tests for the Zendesk"""
+
+from django.conf import settings
+from django.contrib.auth.models import AnonymousUser
+from django.http import Http404
+from django.test import TestCase
+from django.test.client import RequestFactory
+from django.test.utils import override_settings
+from student.tests.factories import UserFactory
+from util import views
+from zendesk import ZendeskError
+import json
+import mock
+
+
+@mock.patch.dict("django.conf.settings.MITX_FEATURES", {"ENABLE_FEEDBACK_SUBMISSION": True})
+@override_settings(ZENDESK_URL="dummy", ZENDESK_USER="dummy", ZENDESK_API_KEY="dummy")
+@mock.patch("util.views._ZendeskApi", autospec=True)
+class SubmitFeedbackViaZendeskTest(TestCase):
+    def setUp(self):
+        """Set up data for the test case"""
+        self._request_factory = RequestFactory()
+        self._anon_user = AnonymousUser()
+        self._auth_user = UserFactory.create(
+            email="test@edx.org",
+            username="test",
+            profile__name="Test User"
+        )
+        # This contains a tag to ensure that tags are submitted correctly
+        self._anon_fields = {
+            "email": "test@edx.org",
+            "name": "Test User",
+            "subject": "a subject",
+            "details": "some details",
+            "tag": "a tag"
+        }
+        # This does not contain a tag to ensure that tag is optional
+        self._auth_fields = {"subject": "a subject", "details": "some details"}
+
+    def _test_request(self, user, fields):
+        """
+        Generate a request and invoke the view, returning the response.
+
+        The request will be a POST request from the given `user`, with the given
+        `fields` in the POST body.
+        """
+        req = self._request_factory.post(
+            "/submit_feedback",
+            data=fields,
+            HTTP_REFERER="test_referer",
+            HTTP_USER_AGENT="test_user_agent"
+        )
+        req.user = user
+        return views.submit_feedback_via_zendesk(req)
+
+    def _assert_bad_request(self, response, field, zendesk_mock_class):
+        """
+        Assert that the given `response` contains correct failure data.
+
+        It should have a 400 status code, and its content should be a JSON
+        object containing the specified `field` and an `error`.
+        """
+        self.assertEqual(response.status_code, 400)
+        resp_json = json.loads(response.content)
+        self.assertTrue("field" in resp_json)
+        self.assertEqual(resp_json["field"], field)
+        self.assertTrue("error" in resp_json)
+        # There should be absolutely no interaction with Zendesk
+        self.assertFalse(zendesk_mock_class.return_value.mock_calls)
+
+    def _test_bad_request_omit_field(self, user, fields, omit_field, zendesk_mock_class):
+        """
+        Invoke the view with a request missing a field and assert correctness.
+
+        The request will be a POST request from the given `user`, with POST
+        fields taken from `fields` minus the entry specified by `omit_field`.
+        The response should have a 400 (bad request) status code and specify
+        the invalid field and an error message, and the Zendesk API should not
+        have been invoked.
+        """
+        filtered_fields = {k: v for (k, v) in fields.items() if k != omit_field}
+        resp = self._test_request(user, filtered_fields)
+        self._assert_bad_request(resp, omit_field, zendesk_mock_class)
+
+    def _test_bad_request_empty_field(self, user, fields, empty_field, zendesk_mock_class):
+        """
+        Invoke the view with an empty field and assert correctness.
+
+        The request will be a POST request from the given `user`, with POST
+        fields taken from `fields`, replacing the entry specified by
+        `empty_field` with the empty string. The response should have a 400
+        (bad request) status code and specify the invalid field and an error
+        message, and the Zendesk API should not have been invoked.
+        """
+        altered_fields = fields.copy()
+        altered_fields[empty_field] = ""
+        resp = self._test_request(user, altered_fields)
+        self._assert_bad_request(resp, empty_field, zendesk_mock_class)
+
+    def _test_success(self, user, fields):
+        """
+        Generate a request, invoke the view, and assert success.
+
+        The request will be a POST request from the given `user`, with the given
+        `fields` in the POST body. The response should have a 200 (success)
+        status code.
+        """
+        resp = self._test_request(user, fields)
+        self.assertEqual(resp.status_code, 200)
+
+    def test_bad_request_anon_user_no_name(self, zendesk_mock_class):
+        """Test a request from an anonymous user not specifying `name`."""
+        self._test_bad_request_omit_field(self._anon_user, self._anon_fields, "name", zendesk_mock_class)
+        self._test_bad_request_empty_field(self._anon_user, self._anon_fields, "name", zendesk_mock_class)
+
+    def test_bad_request_anon_user_no_email(self, zendesk_mock_class):
+        """Test a request from an anonymous user not specifying `email`."""
+        self._test_bad_request_omit_field(self._anon_user, self._anon_fields, "email", zendesk_mock_class)
+        self._test_bad_request_empty_field(self._anon_user, self._anon_fields, "email", zendesk_mock_class)
+
+    def test_bad_request_anon_user_no_subject(self, zendesk_mock_class):
+        """Test a request from an anonymous user not specifying `subject`."""
+        self._test_bad_request_omit_field(self._anon_user, self._anon_fields, "subject", zendesk_mock_class)
+        self._test_bad_request_empty_field(self._anon_user, self._anon_fields, "subject", zendesk_mock_class)
+
+    def test_bad_request_anon_user_no_details(self, zendesk_mock_class):
+        """Test a request from an anonymous user not specifying `details`."""
+        self._test_bad_request_omit_field(self._anon_user, self._anon_fields, "details", zendesk_mock_class)
+        self._test_bad_request_empty_field(self._anon_user, self._anon_fields, "details", zendesk_mock_class)
+
+    def test_valid_request_anon_user(self, zendesk_mock_class):
+        """
+        Test a valid request from an anonymous user.
+
+        The response should have a 200 (success) status code, and a ticket with
+        the given information should have been submitted via the Zendesk API.
+        """
+        zendesk_mock_instance = zendesk_mock_class.return_value
+        zendesk_mock_instance.create_ticket.return_value = 42
+        self._test_success(self._anon_user, self._anon_fields)
+        expected_calls = [
+            mock.call.create_ticket(
+                {
+                    "ticket": {
+                        "requester": {"name": "Test User", "email": "test@edx.org"},
+                        "subject": "a subject",
+                        "comment": {"body": "some details"},
+                        "tags": ["a tag"]
+                    }
+                }
+            ),
+            mock.call.update_ticket(
+                42,
+                {
+                    "ticket": {
+                        "comment": {
+                            "public": False,
+                            "body":
+                            "Additional information:\n\n"
+                            "HTTP_USER_AGENT: test_user_agent\n"
+                            "HTTP_REFERER: test_referer"
+                        }
+                    }
+                }
+            )
+        ]
+        self.assertEqual(zendesk_mock_instance.mock_calls, expected_calls)
+
+    def test_bad_request_auth_user_no_subject(self, zendesk_mock_class):
+        """Test a request from an authenticated user not specifying `subject`."""
+        self._test_bad_request_omit_field(self._auth_user, self._auth_fields, "subject", zendesk_mock_class)
+        self._test_bad_request_empty_field(self._auth_user, self._auth_fields, "subject", zendesk_mock_class)
+
+    def test_bad_request_auth_user_no_details(self, zendesk_mock_class):
+        """Test a request from an authenticated user not specifying `details`."""
+        self._test_bad_request_omit_field(self._auth_user, self._auth_fields, "details", zendesk_mock_class)
+        self._test_bad_request_empty_field(self._auth_user, self._auth_fields, "details", zendesk_mock_class)
+
+    def test_valid_request_auth_user(self, zendesk_mock_class):
+        """
+        Test a valid request from an authenticated user.
+
+        The response should have a 200 (success) status code, and a ticket with
+        the given information should have been submitted via the Zendesk API.
+        """
+        zendesk_mock_instance = zendesk_mock_class.return_value
+        zendesk_mock_instance.create_ticket.return_value = 42
+        self._test_success(self._auth_user, self._auth_fields)
+        expected_calls = [
+            mock.call.create_ticket(
+                {
+                    "ticket": {
+                        "requester": {"name": "Test User", "email": "test@edx.org"},
+                        "subject": "a subject",
+                        "comment": {"body": "some details"},
+                        "tags": []
+                    }
+                }
+            ),
+            mock.call.update_ticket(
+                42,
+                {
+                    "ticket": {
+                        "comment": {
+                            "public": False,
+                            "body":
+                            "Additional information:\n\n"
+                            "username: test\n"
+                            "HTTP_USER_AGENT: test_user_agent\n"
+                            "HTTP_REFERER: test_referer"
+                        }
+                    }
+                }
+            )
+        ]
+        self.assertEqual(zendesk_mock_instance.mock_calls, expected_calls)
+
+    def test_get_request(self, zendesk_mock_class):
+        """Test that a GET results in a 405 even with all required fields"""
+        req = self._request_factory.get("/submit_feedback", data=self._anon_fields)
+        req.user = self._anon_user
+        resp = views.submit_feedback_via_zendesk(req)
+        self.assertEqual(resp.status_code, 405)
+        self.assertIn("Allow", resp)
+        self.assertEqual(resp["Allow"], "POST")
+        # There should be absolutely no interaction with Zendesk
+        self.assertFalse(zendesk_mock_class.mock_calls)
+
+    def test_zendesk_error_on_create(self, zendesk_mock_class):
+        """
+        Test Zendesk returning an error on ticket creation.
+
+        We should return a 500 error with no body
+        """
+        err = ZendeskError(msg="", error_code=404)
+        zendesk_mock_instance = zendesk_mock_class.return_value
+        zendesk_mock_instance.create_ticket.side_effect = err
+        resp = self._test_request(self._anon_user, self._anon_fields)
+        self.assertEqual(resp.status_code, 500)
+        self.assertFalse(resp.content)
+
+    def test_zendesk_error_on_update(self, zendesk_mock_class):
+        """
+        Test for Zendesk returning an error on ticket update.
+
+        If Zendesk returns any error on ticket update, we return a 200 to the
+        browser because the update contains additional information that is not
+        necessary for the user to have submitted their feedback.
+        """
+        err = ZendeskError(msg="", error_code=500)
+        zendesk_mock_instance = zendesk_mock_class.return_value
+        zendesk_mock_instance.update_ticket.side_effect = err
+        resp = self._test_request(self._anon_user, self._anon_fields)
+        self.assertEqual(resp.status_code, 200)
+
+    @mock.patch.dict("django.conf.settings.MITX_FEATURES", {"ENABLE_FEEDBACK_SUBMISSION": False})
+    def test_not_enabled(self, zendesk_mock_class):
+        """
+        Test for Zendesk submission not enabled in `settings`.
+
+        We should raise Http404.
+        """
+        with self.assertRaises(Http404):
+            self._test_request(self._anon_user, self._anon_fields)
+
+    def test_zendesk_not_configured(self, zendesk_mock_class):
+        """
+        Test for Zendesk not fully configured in `settings`.
+
+        For each required configuration parameter, test that setting it to
+        `None` causes an otherwise valid request to return a 500 error.
+        """
+        def test_case(missing_config):
+            with mock.patch(missing_config, None):
+                with self.assertRaises(Exception):
+                    self._test_request(self._anon_user, self._anon_fields)
+
+        test_case("django.conf.settings.ZENDESK_URL")
+        test_case("django.conf.settings.ZENDESK_USER")
+        test_case("django.conf.settings.ZENDESK_API_KEY")

common/djangoapps/util/views.py

@@ -1,5 +1,6 @@
 import datetime
 import json
+import logging
 import pprint
 import sys
 
@@ -7,15 +8,21 @@ from django.conf import settings
 from django.contrib.auth.models import User
 from django.core.context_processors import csrf
 from django.core.mail import send_mail
-from django.http import Http404
-from django.http import HttpResponse
+from django.core.validators import ValidationError, validate_email
+from django.http import Http404, HttpResponse, HttpResponseBadRequest, HttpResponseNotAllowed, HttpResponseServerError
 from django.shortcuts import redirect
+from django_future.csrf import ensure_csrf_cookie
 from mitxmako.shortcuts import render_to_response, render_to_string
+from urllib import urlencode
+import zendesk
 
 import capa.calc
 import track.views
 
 
+log = logging.getLogger(__name__)
+
+
 def calculate(request):
     ''' Calculator in footer of every page. '''
     equation = request.GET['equation']
@@ -29,36 +36,145 @@ def calculate(request):
     return HttpResponse(json.dumps({'result': str(result)}))
 
 
-def send_feedback(request):
-    ''' Feeback mechanism in footer of every page. '''
-    try:
-        username = request.user.username
+class _ZendeskApi(object):
+    def __init__(self):
+        """
+        Instantiate the Zendesk API.
+
+        All of `ZENDESK_URL`, `ZENDESK_USER`, and `ZENDESK_API_KEY` must be set
+        in `django.conf.settings`.
+        """
+        self._zendesk_instance = zendesk.Zendesk(
+            settings.ZENDESK_URL,
+            settings.ZENDESK_USER,
+            settings.ZENDESK_API_KEY,
+            use_api_token=True,
+            api_version=2,
+            # As of 2012-05-08, Zendesk is using a CA that is not
+            # installed on our servers
+            client_args={"disable_ssl_certificate_validation": True}
+        )
+
+    def create_ticket(self, ticket):
+        """
+        Create the given `ticket` in Zendesk.
+
+        The ticket should have the format specified by the zendesk package.
+        """
+        ticket_url = self._zendesk_instance.create_ticket(data=ticket)
+        return zendesk.get_id_from_url(ticket_url)
+
+    def update_ticket(self, ticket_id, update):
+        """
+        Update the Zendesk ticket with id `ticket_id` using the given `update`.
+
+        The update should have the format specified by the zendesk package.
+        """
+        self._zendesk_instance.update_ticket(ticket_id=ticket_id, data=update)
+
+
+def submit_feedback_via_zendesk(request):
+    """
+    Create a new user-requested Zendesk ticket.
+
+    If Zendesk submission is not enabled, any request will raise `Http404`.
+    If any configuration parameter (`ZENDESK_URL`, `ZENDESK_USER`, or
+    `ZENDESK_API_KEY`) is missing, any request will raise an `Exception`.
+    The request must be a POST request specifying `subject` and `details`.
+    If the user is not authenticated, the request must also specify `name` and
+    `email`. If the user is authenticated, the `name` and `email` will be
+    populated from the user's information. If any required parameter is
+    missing, a 400 error will be returned indicating which field is missing and
+    providing an error message. If Zendesk returns any error on ticket
+    creation, a 500 error will be returned with no body. Once created, the
+    ticket will be updated with a private comment containing additional
+    information from the browser and server, such as HTTP headers and user
+    state. Whether or not the update succeeds, if the user's ticket is
+    successfully created, an empty successful response (200) will be returned.
+    """
+    if not settings.MITX_FEATURES.get('ENABLE_FEEDBACK_SUBMISSION', False):
+        raise Http404()
+    if request.method != "POST":
+        return HttpResponseNotAllowed(["POST"])
+    if (
+        not settings.ZENDESK_URL or
+        not settings.ZENDESK_USER or
+        not settings.ZENDESK_API_KEY
+    ):
+        raise Exception("Zendesk enabled but not configured")
+
+    def build_error_response(status_code, field, err_msg):
+        return HttpResponse(json.dumps({"field": field, "error": err_msg}), status=status_code)
+
+    additional_info = {}
+
+    required_fields = ["subject", "details"]
+    if not request.user.is_authenticated():
+        required_fields += ["name", "email"]
+    required_field_errs = {
+        "subject": "Please provide a subject.",
+        "details": "Please provide details.",
+        "name": "Please provide your name.",
+        "email": "Please provide a valid e-mail.",
+    }
+
+    for field in required_fields:
+        if field not in request.POST or not request.POST[field]:
+            return build_error_response(400, field, required_field_errs[field])
+
+    subject = request.POST["subject"]
+    details = request.POST["details"]
+    tags = []
+    if "tag" in request.POST:
+        tags = [request.POST["tag"]]
+
+    if request.user.is_authenticated():
+        realname = request.user.profile.name
         email = request.user.email
-    except:
-        username = "anonymous"
-        email = "anonymous"
+        additional_info["username"] = request.user.username
+    else:
+        realname = request.POST["name"]
+        email = request.POST["email"]
+        try:
+            validate_email(email)
+        except ValidationError:
+            return build_error_response(400, "email", required_field_errs["email"])
 
+    for header in ["HTTP_REFERER", "HTTP_USER_AGENT"]:
+        additional_info[header] = request.META.get(header)
+
+    zendesk_api = _ZendeskApi()
+
+    additional_info_string = (
+        "Additional information:\n\n" +
+        "\n".join("%s: %s" % (key, value) for (key, value) in additional_info.items() if value is not None)
+    )
+
+    new_ticket = {
+        "ticket": {
+            "requester": {"name": realname, "email": email},
+            "subject": subject,
+            "comment": {"body": details},
+            "tags": tags
+        }
+    }
     try:
-        browser = request.META['HTTP_USER_AGENT']
-    except:
-        browser = "Unknown"
+        ticket_id = zendesk_api.create_ticket(new_ticket)
+    except zendesk.ZendeskError as err:
+        log.error("%s", str(err))
+        return HttpResponse(status=500)
 
-    feedback = render_to_string("feedback_email.txt",
-                                {"subject": request.POST['subject'],
-                                 "url": request.POST['url'],
-                                 "time": datetime.datetime.now().isoformat(),
-                                 "feedback": request.POST['message'],
-                                 "email": email,
-                                 "browser": browser,
-                                 "user": username})
+    # Additional information is provided as a private update so the information
+    # is not visible to the user.
+    ticket_update = {"ticket": {"comment": {"public": False, "body": additional_info_string}}}
+    try:
+        zendesk_api.update_ticket(ticket_id, ticket_update)
+    except zendesk.ZendeskError as err:
+        log.error("%s", str(err))
+        # The update is not strictly necessary, so do not indicate failure to the user
+        pass
 
-    send_mail("MITx Feedback / " + request.POST['subject'],
-              feedback,
-              settings.DEFAULT_FROM_EMAIL,
-              [settings.DEFAULT_FEEDBACK_EMAIL],
-              fail_silently=False
-              )
-    return HttpResponse(json.dumps({'success': True}))
+    return HttpResponse()
 
 
 def info(request):

common/lib/capa/capa/responsetypes.py

@@ -1147,9 +1147,9 @@ def sympy_check2():
                     messages = []
                     for input_dict in input_list:
                         correct.append('correct'
-                                if input_dict['ok'] else 'incorrect')
+                                       if input_dict['ok'] else 'incorrect')
                         msg = (self.clean_message_html(input_dict['msg'])
-                                if 'msg' in input_dict else None)
+                               if 'msg' in input_dict else None)
                         messages.append(msg)
 
                 # Otherwise, we do not recognize the dictionary
@@ -1174,7 +1174,7 @@ def sympy_check2():
 
         for k in range(len(idset)):
             npoints = (self.maxpoints[idset[k]]
-                    if correct[k] == 'correct' else 0)
+                       if correct[k] == 'correct' else 0)
             correct_map.set(idset[k], correct[k], msg=messages[k],
                             npoints=npoints)
         return correct_map
@@ -1851,6 +1851,19 @@ class FormulaResponse(LoncapaResponse):
                     'formularesponse: undefined variable in given=%s' % given)
                 raise StudentInputError(
                     "Invalid input: " + uv.message + " not permitted in answer")
+            except ValueError as ve:
+                if 'factorial' in ve.message:
+                    # This is thrown when fact() or factorial() is used in a formularesponse answer
+                    #   that tests on negative and/or non-integer inputs
+                    # ve.message will be: `factorial() only accepts integral values` or `factorial() not defined for negative values`
+                    log.debug(
+                        'formularesponse: factorial function used in response that tests negative and/or non-integer inputs. given={0}'.format(given))
+                    raise StudentInputError(
+                        "factorial function not permitted in answer for this problem. Provided answer was: {0}".format(given))
+                # If non-factorial related ValueError thrown, handle it the same as any other Exception
+                log.debug('formularesponse: error {0} in formula'.format(ve))
+                raise StudentInputError("Invalid input: Could not parse '%s' as a formula" %
+                                        cgi.escape(given))
             except Exception as err:
                 # traceback.print_exc()
                 log.debug('formularesponse: error %s in formula' % err)
@@ -1983,7 +1996,6 @@ class ImageResponse(LoncapaResponse):
         self.ielements = self.inputfields
         self.answer_ids = [ie.get('id') for ie in self.ielements]
 
-
     def get_score(self, student_answers):
         correct_map = CorrectMap()
         expectedset = self.get_mapped_answers()
@@ -2052,7 +2064,7 @@ class ImageResponse(LoncapaResponse):
                 rectangles (dict) - a map of inputs to the defined rectangle for that input
                 regions (dict) - a map of inputs to the defined region for that input
         '''
-        answers =  (
+        answers = (
             dict([(ie.get('id'), ie.get(
                 'rectangle')) for ie in self.ielements]),
             dict([(ie.get('id'), ie.get('regions')) for ie in self.ielements]))
@@ -2074,8 +2086,6 @@ class ImageResponse(LoncapaResponse):
             answers[ie_id] = (ie.get('rectangle'), ie.get('regions'))
 
         return answers
-            
-
 
 #-----------------------------------------------------------------------------
 

common/lib/xmodule/xmodule/capa_module.py

@@ -65,7 +65,8 @@ class CapaFields(object):
     max_attempts = StringyInteger(help="Maximum number of attempts that a student is allowed", scope=Scope.settings)
     due = Date(help="Date that this problem is due by", scope=Scope.settings)
     graceperiod = Timedelta(help="Amount of time after the due date that submissions will be accepted", scope=Scope.settings)
-    showanswer = String(help="When to show the problem answer to the student", scope=Scope.settings, default="closed")
+    showanswer = String(help="When to show the problem answer to the student", scope=Scope.settings, default="closed",
+                        values=["answered", "always", "attempted", "closed", "never"])
     force_save_button = Boolean(help="Whether to force the save button to appear on the page", scope=Scope.settings, default=False)
     rerandomize = Randomization(help="When to rerandomize the problem", default="always", scope=Scope.settings)
     data = String(help="XML data for the problem", scope=Scope.content)
@@ -882,16 +883,6 @@ class CapaDescriptor(CapaFields, RawDescriptor):
                          'enable_markdown': self.markdown is not None})
         return _context
 
-    @property
-    def editable_metadata_fields(self):
-        """Remove metadata from the editable fields since it has its own editor"""
-        subset = super(CapaDescriptor, self).editable_metadata_fields
-        if 'markdown' in subset:
-            del subset['markdown']
-        if 'empty' in subset:
-            del subset['empty']
-        return subset
-
     # VS[compat]
     # TODO (cpennington): Delete this method once all fall 2012 course are being
     # edited in the cms
@@ -901,3 +892,10 @@ class CapaDescriptor(CapaFields, RawDescriptor):
             'problems/' + path[8:],
             path[8:],
         ]
+
+    @property
+    def non_editable_metadata_fields(self):
+        non_editable_fields = super(CapaDescriptor, self).non_editable_metadata_fields
+        non_editable_fields.extend([CapaDescriptor.due, CapaDescriptor.graceperiod,
+                                    CapaDescriptor.force_save_button, CapaDescriptor.markdown])
+        return non_editable_fields

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/course_module.py

@@ -162,8 +162,7 @@ class CourseFields(object):
     discussion_blackouts = List(help="List of pairs of start/end dates for discussion blackouts", scope=Scope.settings)
     discussion_topics = Object(
         help="Map of topics names to ids",
-        scope=Scope.settings,
-        computed_default=lambda c: {'General': {'id': c.location.html_id()}},
+        scope=Scope.settings
         )
     testcenter_info = Object(help="Dictionary of Test Center info", scope=Scope.settings)
     announcement = Date(help="Date this course is announced", scope=Scope.settings)
@@ -234,6 +233,8 @@ class CourseDescriptor(CourseFields, SequenceDescriptor):
         self._grading_policy = {}
 
         self.set_grading_policy(self.grading_policy)
+        if self.discussion_topics == {}:
+            self.discussion_topics = {'General': {'id': self.location.html_id()}}
 
         self.test_center_exams = []
         test_center_info = self.testcenter_info

common/lib/xmodule/xmodule/discussion_module.py

@@ -37,3 +37,10 @@ class DiscussionDescriptor(DiscussionFields, MetadataOnlyEditingDescriptor, RawD
     metadata_translations = dict(RawDescriptor.metadata_translations)
     metadata_translations['id'] = 'discussion_id'
     metadata_translations['for'] = 'discussion_target'
+
+    @property
+    def non_editable_metadata_fields(self):
+        non_editable_fields = super(DiscussionDescriptor, self).non_editable_metadata_fields
+        # We may choose to enable sort_keys in the future, but while Kevin is investigating....
+        non_editable_fields.extend([DiscussionDescriptor.discussion_id, DiscussionDescriptor.sort_key])
+        return non_editable_fields

common/lib/xmodule/xmodule/html_module.py

@@ -19,6 +19,7 @@ log = logging.getLogger("mitx.courseware")
 
 class HtmlFields(object):
     data = String(help="Html contents to display for this module", scope=Scope.content)
+    source_code = String(help="Source code for LaTeX documents. This feature is not well-supported.", scope=Scope.settings)
 
 
 class HtmlModule(HtmlFields, XModule):
@@ -166,16 +167,6 @@ class HtmlDescriptor(HtmlFields, XmlDescriptor, EditingDescriptor):
         elt.set("filename", relname)
         return elt
 
-    @property
-    def editable_metadata_fields(self):
-        """Remove any metadata from the editable fields which have their own editor or shouldn't be edited by user."""
-        subset = super(HtmlDescriptor, self).editable_metadata_fields
-
-        if 'empty' in subset:
-            del subset['empty']
-
-        return subset
-
 
 class AboutDescriptor(HtmlDescriptor):
     """

common/lib/xmodule/xmodule/js/src/.gitignore

@@ -1 +1,4 @@
-# Please do not ignore *.js files. Some xmodules are written in JS.
+# Ignore .js files in this folder as they are compiled from coffeescript
+# For each of the xmodules subdirectories, add a .gitignore file that
+# will version any *.js file that is specifically written, not compiled.
+*.js

common/lib/xmodule/xmodule/js/src/capa/.gitignore

@@ -0,0 +1,2 @@
+!imageinput.js
+!schematic.js

common/lib/xmodule/xmodule/js/src/graphical_slider_tool/.gitignore

@@ -0,0 +1 @@
+!*.js

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'))
-
-    @problem_button = $('.problem-button')
-    @problem_button.click @show_results
-
-    @problem_list = $('.problem-list')
-    @construct_progress_bar()
-
-    if @use_single_location
+    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'))
+
+      @message_container = $('.message-container')
+      @message_container.toggle(not @message_container.is(':empty'))
+
+      @problem_button = $('.problem-button')
+      @problem_button.click @show_results
+
+      @problem_list = $('.problem-list')
+      @construct_progress_bar()
 
   construct_progress_bar: () =>
     problems = @problem_list.find('tr').next()

common/lib/xmodule/xmodule/js/src/poll/.gitignore

@@ -0,0 +1 @@
+!*.js

common/lib/xmodule/xmodule/js/src/sequence/display/.gitignore

@@ -0,0 +1 @@
+!*.js

common/lib/xmodule/xmodule/js/src/videoalpha/display/.gitignore

@@ -0,0 +1 @@
+!html5_video.js

common/lib/xmodule/xmodule/mako_module.py

@@ -1,5 +1,4 @@
 from .x_module import XModuleDescriptor, DescriptorSystem
-from .modulestore.inheritance import own_metadata
 
 
 class MakoDescriptorSystem(DescriptorSystem):
@@ -34,20 +33,10 @@ class MakoModuleDescriptor(XModuleDescriptor):
         """
         return {
             'module': self,
-            'editable_metadata_fields': self.editable_metadata_fields,
+            'editable_metadata_fields': self.editable_metadata_fields
         }
 
     def get_html(self):
         return self.system.render_template(
             self.mako_template, self.get_context())
 
-    # cdodge: encapsulate a means to expose "editable" metadata fields (i.e. not internal system metadata)
-    @property
-    def editable_metadata_fields(self):
-        fields = {}
-        for field, value in own_metadata(self).items():
-            if field in self.system_metadata_fields:
-                continue
-
-            fields[field] = value
-        return fields

common/lib/xmodule/xmodule/modulestore/inheritance.py

@@ -31,15 +31,22 @@ def inherit_metadata(descriptor, model_data):
     Only metadata specified in self.inheritable_metadata will
     be inherited
     """
+    # The inherited values that are actually being used.
     if not hasattr(descriptor, '_inherited_metadata'):
         setattr(descriptor, '_inherited_metadata', {})
 
+    # All inheritable metadata values (for which a value exists in model_data).
+    if not hasattr(descriptor, '_inheritable_metadata'):
+        setattr(descriptor, '_inheritable_metadata', {})
+
     # Set all inheritable metadata from kwargs that are
     # in self.inheritable_metadata and aren't already set in metadata
     for attr in INHERITABLE_METADATA:
-        if attr not in descriptor._model_data and attr in model_data:
-            descriptor._inherited_metadata[attr] = model_data[attr]
-            descriptor._model_data[attr] = model_data[attr]
+        if attr in model_data:
+            descriptor._inheritable_metadata[attr] = model_data[attr]
+            if attr not in descriptor._model_data:
+                descriptor._inherited_metadata[attr] = model_data[attr]
+                descriptor._model_data[attr] = model_data[attr]
 
 
 def own_metadata(module):

common/lib/xmodule/xmodule/modulestore/tests/factories.py

@@ -37,11 +37,17 @@ class XModuleCourseFactory(Factory):
             new_course.display_name = display_name
 
         new_course.lms.start = gmtime()
-        new_course.tabs = [{"type": "courseware"},
-                           {"type": "course_info", "name": "Course Info"},
-                           {"type": "discussion", "name": "Discussion"},
-                           {"type": "wiki", "name": "Wiki"},
-                           {"type": "progress", "name": "Progress"}]
+        new_course.tabs = kwargs.get(
+            'tabs',
+            [
+                {"type": "courseware"},
+                {"type": "course_info", "name": "Course Info"},
+                {"type": "discussion", "name": "Discussion"},
+                {"type": "wiki", "name": "Wiki"},
+                {"type": "progress", "name": "Progress"}
+            ]
+        )
+        new_course.discussion_link = kwargs.get('discussion_link')
 
         # Update the data in the mongo datastore
         store.update_metadata(new_course.location.url(), own_metadata(new_course))

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)

common/lib/xmodule/xmodule/tests/test_course_module.py

@@ -40,34 +40,20 @@ class DummySystem(ImportSystem):
         )
 
 
-class IsNewCourseTestCase(unittest.TestCase):
-    """Make sure the property is_new works on courses"""
+def get_dummy_course(start, announcement=None, is_new=None, advertised_start=None, end=None):
+    """Get a dummy course"""
 
-    def setUp(self):
-        # Needed for test_is_newish
-        datetime_patcher = patch.object(
-            xmodule.course_module, 'datetime',
-            Mock(wraps=datetime.datetime)
-        )
-        mocked_datetime = datetime_patcher.start()
-        mocked_datetime.utcnow.return_value = time_to_datetime(NOW)
-        self.addCleanup(datetime_patcher.stop)
+    system = DummySystem(load_error_modules=True)
 
-    @staticmethod
-    def get_dummy_course(start, announcement=None, is_new=None, advertised_start=None, end=None):
-        """Get a dummy course"""
+    def to_attrb(n, v):
+        return '' if v is None else '{0}="{1}"'.format(n, v).lower()
 
-        system = DummySystem(load_error_modules=True)
+    is_new = to_attrb('is_new', is_new)
+    announcement = to_attrb('announcement', announcement)
+    advertised_start = to_attrb('advertised_start', advertised_start)
+    end = to_attrb('end', end)
 
-        def to_attrb(n, v):
-            return '' if v is None else '{0}="{1}"'.format(n, v).lower()
-
-        is_new = to_attrb('is_new', is_new)
-        announcement = to_attrb('announcement', announcement)
-        advertised_start = to_attrb('advertised_start', advertised_start)
-        end = to_attrb('end', end)
-
-        start_xml = '''
+    start_xml = '''
          <course org="{org}" course="{course}"
                 graceperiod="1 day" url_name="test"
                 start="{start}"
@@ -80,9 +66,23 @@ class IsNewCourseTestCase(unittest.TestCase):
             </chapter>
          </course>
          '''.format(org=ORG, course=COURSE, start=start, is_new=is_new,
-                    announcement=announcement, advertised_start=advertised_start, end=end)
+        announcement=announcement, advertised_start=advertised_start, end=end)
 
-        return system.process_xml(start_xml)
+    return system.process_xml(start_xml)
+
+
+class IsNewCourseTestCase(unittest.TestCase):
+    """Make sure the property is_new works on courses"""
+
+    def setUp(self):
+        # Needed for test_is_newish
+        datetime_patcher = patch.object(
+            xmodule.course_module, 'datetime',
+            Mock(wraps=datetime.datetime)
+        )
+        mocked_datetime = datetime_patcher.start()
+        mocked_datetime.utcnow.return_value = time_to_datetime(NOW)
+        self.addCleanup(datetime_patcher.stop)
 
     @patch('xmodule.course_module.time.gmtime')
     def test_sorting_score(self, gmtime_mock):
@@ -120,8 +120,8 @@ class IsNewCourseTestCase(unittest.TestCase):
         ]
 
         for a, b, assertion in dates:
-            a_score = self.get_dummy_course(start=a[0], announcement=a[1], advertised_start=a[2]).sorting_score
-            b_score = self.get_dummy_course(start=b[0], announcement=b[1], advertised_start=b[2]).sorting_score
+            a_score = get_dummy_course(start=a[0], announcement=a[1], advertised_start=a[2]).sorting_score
+            b_score = get_dummy_course(start=b[0], announcement=b[1], advertised_start=b[2]).sorting_score
             print "Comparing %s to %s" % (a, b)
             assertion(a_score, b_score)
 
@@ -138,36 +138,42 @@ class IsNewCourseTestCase(unittest.TestCase):
         ]
 
         for s in settings:
-            d = self.get_dummy_course(start=s[0], advertised_start=s[1])
+            d = get_dummy_course(start=s[0], advertised_start=s[1])
             print "Checking start=%s advertised=%s" % (s[0], s[1])
             self.assertEqual(d.start_date_text, s[2])
 
     def test_is_newish(self):
-        descriptor = self.get_dummy_course(start='2012-12-02T12:00', is_new=True)
+        descriptor = get_dummy_course(start='2012-12-02T12:00', is_new=True)
         assert(descriptor.is_newish is True)
 
-        descriptor = self.get_dummy_course(start='2013-02-02T12:00', is_new=False)
+        descriptor = get_dummy_course(start='2013-02-02T12:00', is_new=False)
         assert(descriptor.is_newish is False)
 
-        descriptor = self.get_dummy_course(start='2013-02-02T12:00', is_new=True)
+        descriptor = get_dummy_course(start='2013-02-02T12:00', is_new=True)
         assert(descriptor.is_newish is True)
 
-        descriptor = self.get_dummy_course(start='2013-01-15T12:00')
+        descriptor = get_dummy_course(start='2013-01-15T12:00')
         assert(descriptor.is_newish is True)
 
-        descriptor = self.get_dummy_course(start='2013-03-01T12:00')
+        descriptor = get_dummy_course(start='2013-03-01T12:00')
         assert(descriptor.is_newish is True)
 
-        descriptor = self.get_dummy_course(start='2012-10-15T12:00')
+        descriptor = get_dummy_course(start='2012-10-15T12:00')
         assert(descriptor.is_newish is False)
 
-        descriptor = self.get_dummy_course(start='2012-12-31T12:00')
+        descriptor = get_dummy_course(start='2012-12-31T12:00')
         assert(descriptor.is_newish is True)
 
     def test_end_date_text(self):
         # No end date set, returns empty string.
-        d = self.get_dummy_course('2012-12-02T12:00')
+        d = get_dummy_course('2012-12-02T12:00')
         self.assertEqual('', d.end_date_text)
 
-        d = self.get_dummy_course('2012-12-02T12:00', end='2014-9-04T12:00')
+        d = get_dummy_course('2012-12-02T12:00', end='2014-9-04T12:00')
         self.assertEqual('Sep 04, 2014', d.end_date_text)
+
+
+class DiscussionTopicsTestCase(unittest.TestCase):
+    def test_default_discussion_topics(self):
+        d = get_dummy_course('2012-12-02T12:00')
+        self.assertEqual({'General': {'id': 'i4x-test_org-test_course-course-test'}}, d.discussion_topics)

common/lib/xmodule/xmodule/tests/test_import.py

@@ -151,6 +151,10 @@ class ImportTestCase(BaseCourseTestCase):
         # Check that the child inherits due correctly
         child = descriptor.get_children()[0]
         self.assertEqual(child.lms.due, Date().from_json(v))
+        self.assertEqual(child._inheritable_metadata, child._inherited_metadata)
+        self.assertEqual(2, len(child._inherited_metadata))
+        self.assertEqual('1970-01-01T00:00:00Z', child._inherited_metadata['start'])
+        self.assertEqual(v, child._inherited_metadata['due'])
 
         # Now export and check things
         resource_fs = MemoryFS()
@@ -184,6 +188,60 @@ class ImportTestCase(BaseCourseTestCase):
         self.assertEqual(chapter_xml.tag, 'chapter')
         self.assertFalse('due' in chapter_xml.attrib)
 
+    def test_metadata_no_inheritance(self):
+        """
+        Checks that default value of None (for due) does not get marked as inherited.
+        """
+        system = self.get_system()
+        url_name = 'test1'
+        start_xml = '''
+        <course org="{org}" course="{course}"
+                url_name="{url_name}" unicorn="purple">
+            <chapter url="hi" url_name="ch" display_name="CH">
+                <html url_name="h" display_name="H">Two houses, ...</html>
+            </chapter>
+        </course>'''.format(org=ORG, course=COURSE, url_name=url_name)
+        descriptor = system.process_xml(start_xml)
+        compute_inherited_metadata(descriptor)
+
+        self.assertEqual(descriptor.lms.due, None)
+
+        # Check that the child does not inherit a value for due
+        child = descriptor.get_children()[0]
+        self.assertEqual(child.lms.due, None)
+        self.assertEqual(child._inheritable_metadata, child._inherited_metadata)
+        self.assertEqual(1, len(child._inherited_metadata))
+        self.assertEqual('1970-01-01T00:00:00Z', child._inherited_metadata['start'])
+
+    def test_metadata_override_default(self):
+        """
+        Checks that due date can be overriden at child level.
+        """
+        system = self.get_system()
+        course_due = 'March 20 17:00'
+        child_due = 'April 10 00:00'
+        url_name = 'test1'
+        start_xml = '''
+        <course org="{org}" course="{course}"
+                due="{due}" url_name="{url_name}" unicorn="purple">
+            <chapter url="hi" url_name="ch" display_name="CH">
+                <html url_name="h" display_name="H">Two houses, ...</html>
+            </chapter>
+        </course>'''.format(due=course_due, org=ORG, course=COURSE, url_name=url_name)
+        descriptor = system.process_xml(start_xml)
+        child = descriptor.get_children()[0]
+        child._model_data['due'] = child_due
+        compute_inherited_metadata(descriptor)
+
+        self.assertEqual(descriptor.lms.due, Date().from_json(course_due))
+        self.assertEqual(child.lms.due, Date().from_json(child_due))
+        # Test inherited metadata. Due does not appear here (because explicitly set on child).
+        self.assertEqual(1, len(child._inherited_metadata))
+        self.assertEqual('1970-01-01T00:00:00Z', child._inherited_metadata['start'])
+        # Test inheritable metadata. This has the course inheritable value for due.
+        self.assertEqual(2, len(child._inheritable_metadata))
+        self.assertEqual(course_due, child._inheritable_metadata['due'])
+
     def test_is_pointer_tag(self):
         """
         Check that is_pointer_tag works properly.

common/lib/xmodule/xmodule/tests/test_xml_module.py

@@ -0,0 +1,93 @@
+from xmodule.x_module import XModuleFields
+from xblock.core import Scope, String, Object
+from xmodule.fields import Date, StringyInteger
+from xmodule.xml_module import XmlDescriptor
+import unittest
+from . import test_system
+from mock import Mock
+
+
+class TestFields(object):
+    # Will be returned by editable_metadata_fields.
+    max_attempts = StringyInteger(scope=Scope.settings, default=1000)
+    # Will not be returned by editable_metadata_fields because filtered out by non_editable_metadata_fields.
+    due = Date(scope=Scope.settings)
+    # Will not be returned by editable_metadata_fields because is not Scope.settings.
+    student_answers = Object(scope=Scope.user_state)
+    # Will be returned, and can override the inherited value from XModule.
+    display_name = String(scope=Scope.settings, default='local default')
+
+
+class EditableMetadataFieldsTest(unittest.TestCase):
+
+    def test_display_name_field(self):
+        editable_fields = self.get_xml_editable_fields({})
+        # Tests that the xblock fields (currently tags and name) get filtered out.
+        # Also tests that xml_attributes is filtered out of XmlDescriptor.
+        self.assertEqual(1, len(editable_fields), "Expected only 1 editable field for xml descriptor.")
+        self.assert_field_values(editable_fields, 'display_name', XModuleFields.display_name,
+            explicitly_set=False, inheritable=False, value=None, default_value=None)
+
+    def test_override_default(self):
+        # Tests that explicitly_set is correct when a value overrides the default (not inheritable).
+        editable_fields = self.get_xml_editable_fields({'display_name': 'foo'})
+        self.assert_field_values(editable_fields, 'display_name', XModuleFields.display_name,
+            explicitly_set=True, inheritable=False, value='foo', default_value=None)
+
+    def test_additional_field(self):
+        descriptor = self.get_descriptor({'max_attempts' : '7'})
+        editable_fields = descriptor.editable_metadata_fields
+        self.assertEqual(2, len(editable_fields))
+        self.assert_field_values(editable_fields, 'max_attempts', TestFields.max_attempts,
+            explicitly_set=True, inheritable=False, value=7, default_value=1000)
+        self.assert_field_values(editable_fields, 'display_name', XModuleFields.display_name,
+            explicitly_set=False, inheritable=False, value='local default', default_value='local default')
+
+        editable_fields = self.get_descriptor({}).editable_metadata_fields
+        self.assert_field_values(editable_fields, 'max_attempts', TestFields.max_attempts,
+            explicitly_set=False, inheritable=False, value=1000, default_value=1000)
+
+    def test_inherited_field(self):
+        model_val = {'display_name' : 'inherited'}
+        descriptor = self.get_descriptor(model_val)
+        # Mimic an inherited value for display_name (inherited and inheritable are the same in this case).
+        descriptor._inherited_metadata = model_val
+        descriptor._inheritable_metadata = model_val
+        editable_fields = descriptor.editable_metadata_fields
+        self.assert_field_values(editable_fields, 'display_name', XModuleFields.display_name,
+            explicitly_set=False, inheritable=True, value='inherited', default_value='inherited')
+
+        descriptor = self.get_descriptor({'display_name' : 'explicit'})
+        # Mimic the case where display_name WOULD have been inherited, except we explicitly set it.
+        descriptor._inheritable_metadata = {'display_name' : 'inheritable value'}
+        descriptor._inherited_metadata = {}
+        editable_fields = descriptor.editable_metadata_fields
+        self.assert_field_values(editable_fields, 'display_name', XModuleFields.display_name,
+            explicitly_set=True, inheritable=True, value='explicit', default_value='inheritable value')
+
+    # Start of helper methods
+    def get_xml_editable_fields(self, model_data):
+        system = test_system()
+        system.render_template = Mock(return_value="<div>Test Template HTML</div>")
+        return XmlDescriptor(system=system, location=None, model_data=model_data).editable_metadata_fields
+
+    def get_descriptor(self, model_data):
+        class TestModuleDescriptor(TestFields, XmlDescriptor):
+
+            @property
+            def non_editable_metadata_fields(self):
+                non_editable_fields = super(TestModuleDescriptor, self).non_editable_metadata_fields
+                non_editable_fields.append(TestModuleDescriptor.due)
+                return non_editable_fields
+
+        system = test_system()
+        system.render_template = Mock(return_value="<div>Test Template HTML</div>")
+        return TestModuleDescriptor(system=system, location=None, model_data=model_data)
+
+    def assert_field_values(self, editable_fields, name, field, explicitly_set, inheritable, value, default_value):
+        test_field = editable_fields[name]
+        self.assertEqual(field, test_field['field'])
+        self.assertEqual(explicitly_set, test_field['explicitly_set'])
+        self.assertEqual(inheritable, test_field['inheritable'])
+        self.assertEqual(value, test_field['value'])
+        self.assertEqual(default_value, test_field['default_value'])

common/lib/xmodule/xmodule/x_module.py

@@ -82,7 +82,7 @@ class XModuleFields(object):
     display_name = String(
         help="Display name for this module",
         scope=Scope.settings,
-        default=None,
+        default=None
     )
 
 
@@ -334,12 +334,6 @@ class XModuleDescriptor(XModuleFields, HTMLSnippet, ResourceTemplates, XBlock):
     # (like a practice problem).
     has_score = False
 
-    # cdodge: this is a list of metadata names which are 'system' metadata
-    # and should not be edited by an end-user
-
-    system_metadata_fields = ['data_dir', 'published_date', 'published_by', 'is_draft',
-        'discussion_id', 'xml_attributes']
-
     # A list of descriptor attributes that must be equal for the descriptors to
     # be equal
     equality_attributes = ('_model_data', 'location')
@@ -612,6 +606,49 @@ class XModuleDescriptor(XModuleFields, HTMLSnippet, ResourceTemplates, XBlock):
             model_data=self._model_data,
         ))
 
+    @property
+    def non_editable_metadata_fields(self):
+        """
+        Return the list of fields that should not be editable in Studio.
+
+        When overriding, be sure to append to the superclasses' list.
+        """
+        # We are not allowing editing of xblock tag and name fields at this time (for any component).
+        return [XBlock.tags, XBlock.name]
+
+    @property
+    def editable_metadata_fields(self):
+        """
+        Returns the metadata fields to be edited in Studio. These are fields with scope `Scope.settings`.
+
+        Can be limited by extending `non_editable_metadata_fields`.
+        """
+        inherited_metadata = getattr(self, '_inherited_metadata', {})
+        inheritable_metadata = getattr(self, '_inheritable_metadata', {})
+        metadata = {}
+        for field in self.fields:
+
+            if field.scope != Scope.settings or field in self.non_editable_metadata_fields:
+                continue
+
+            inheritable = False
+            value = getattr(self, field.name)
+            default_value = field.default
+            explicitly_set = field.name in self._model_data
+            if field.name in inheritable_metadata:
+                inheritable = True
+                default_value = field.from_json(inheritable_metadata.get(field.name))
+                if field.name in inherited_metadata:
+                    explicitly_set = False
+
+            metadata[field.name] = {'field': field,
+                                    'value': value,
+                                    'default_value': default_value,
+                                    'inheritable': inheritable,
+                                    'explicitly_set': explicitly_set }
+
+        return metadata
+
 
 class DescriptorSystem(object):
     def __init__(self, load_item, resources_fs, error_tracker, **kwargs):

common/lib/xmodule/xmodule/xml_module.py

@@ -84,7 +84,8 @@ class XmlDescriptor(XModuleDescriptor):
     Mixin class for standardized parsing of from xml
     """
 
-    xml_attributes = Object(help="Map of unhandled xml attributes, used only for storage between import and export", default={}, scope=Scope.settings)
+    xml_attributes = Object(help="Map of unhandled xml attributes, used only for storage between import and export",
+        default={}, scope=Scope.settings)
 
     # Extension to append to filename paths
     filename_extension = 'xml'
@@ -418,3 +419,9 @@ class XmlDescriptor(XModuleDescriptor):
         """
         raise NotImplementedError(
             "%s does not implement definition_to_xml" % self.__class__.__name__)
+
+    @property
+    def non_editable_metadata_fields(self):
+        non_editable_fields = super(XmlDescriptor, self).non_editable_metadata_fields
+        non_editable_fields.append(XmlDescriptor.xml_attributes)
+        return non_editable_fields