diff --git a/cms/djangoapps/contentstore/module_info_model.py b/cms/djangoapps/contentstore/module_info_model.py index 91f722a699..f7d1bbd8fe 100644 --- a/cms/djangoapps/contentstore/module_info_model.py +++ b/cms/djangoapps/contentstore/module_info_model.py @@ -75,11 +75,7 @@ def set_module_info(store, location, post_data): # IMPORTANT NOTE: if the client passed pack 'null' (None) for a piece of metadata that means 'remove it' for metadata_key, value in posted_metadata.items(): - # let's strip out any metadata fields from the postback which have been identified as system metadata - # and therefore should not be user-editable, so we should accept them back from the client - if metadata_key in module.system_metadata_fields: - del posted_metadata[metadata_key] - elif posted_metadata[metadata_key] is None: + if posted_metadata[metadata_key] is None: # remove both from passed in collection as well as the collection read in from the modulestore if metadata_key in module._model_data: del module._model_data[metadata_key] diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py index af31fb841b..d77af8aeb1 100644 --- a/cms/djangoapps/contentstore/views.py +++ b/cms/djangoapps/contentstore/views.py @@ -678,11 +678,7 @@ def save_item(request): # IMPORTANT NOTE: if the client passed pack 'null' (None) for a piece of metadata that means 'remove it' for metadata_key, value in posted_metadata.items(): - # let's strip out any metadata fields from the postback which have been identified as system metadata - # and therefore should not be user-editable, so we should accept them back from the client - if metadata_key in existing_item.system_metadata_fields: - del posted_metadata[metadata_key] - elif posted_metadata[metadata_key] is None: + if posted_metadata[metadata_key] is None: # remove both from passed in collection as well as the collection read in from the modulestore if metadata_key in existing_item._model_data: del existing_item._model_data[metadata_key] diff --git a/cms/djangoapps/models/settings/course_metadata.py b/cms/djangoapps/models/settings/course_metadata.py index 4429e35692..708e79f0a3 100644 --- a/cms/djangoapps/models/settings/course_metadata.py +++ b/cms/djangoapps/models/settings/course_metadata.py @@ -14,13 +14,14 @@ class CourseMetadata(object): The objects have no predefined attrs but instead are obj encodings of the editable metadata. ''' - FILTERED_LIST = XModuleDescriptor.system_metadata_fields + ['start', - 'end', - 'enrollment_start', - 'enrollment_end', - 'tabs', - 'graceperiod', - 'checklists'] + FILTERED_LIST = ['xml_attributes', + 'start', + 'end', + 'enrollment_start', + 'enrollment_end', + 'tabs', + 'graceperiod', + 'checklists'] @classmethod def fetch(cls, course_location): diff --git a/cms/static/coffee/spec/views/module_edit_spec.coffee b/cms/static/coffee/spec/views/module_edit_spec.coffee index 5e83ecb42d..baf9ee9c20 100644 --- a/cms/static/coffee/spec/views/module_edit_spec.coffee +++ b/cms/static/coffee/spec/views/module_edit_spec.coffee @@ -72,3 +72,14 @@ describe "CMS.Views.ModuleEdit", -> it "loads the .xmodule-display inside the module editor", -> expect(XModule.loadModule).toHaveBeenCalled() expect(XModule.loadModule.mostRecentCall.args[0]).toBe($('.xmodule_display')) + + describe "changedMetadata", -> + it "returns empty if no metadata loaded", -> + expect(@moduleEdit.changedMetadata()).toEqual({}) + + it "returns only changed values", -> + @moduleEdit.originalMetadata = {'foo', 'bar'} + spyOn(@moduleEdit, 'metadata').andReturn({'a': '', 'b': 'before', 'c': ''}) + @moduleEdit.loadEdit() + @moduleEdit.metadata.andReturn({'a': '', 'b': 'after', 'd': 'only_after'}) + expect(@moduleEdit.changedMetadata()).toEqual({'b' : 'after', 'd' : 'only_after'}) diff --git a/cms/static/coffee/src/views/module_edit.coffee b/cms/static/coffee/src/views/module_edit.coffee index 3cb3b1703f..bf56807f66 100644 --- a/cms/static/coffee/src/views/module_edit.coffee +++ b/cms/static/coffee/src/views/module_edit.coffee @@ -20,6 +20,7 @@ class CMS.Views.ModuleEdit extends Backbone.View loadEdit: -> if not @module @module = XModule.loadModule(@$el.find('.xmodule_edit')) + @originalMetadata = @metadata() metadata: -> # cdodge: package up metadata which is separated into a number of input fields @@ -35,6 +36,14 @@ class CMS.Views.ModuleEdit extends Backbone.View return _metadata + changedMetadata: -> + currentMetadata = @metadata() + changedMetadata = {} + for key of currentMetadata + if currentMetadata[key] != @originalMetadata[key] + changedMetadata[key] = currentMetadata[key] + return changedMetadata + cloneTemplate: (parent, template) -> $.post("/clone_item", { parent_location: parent @@ -60,7 +69,7 @@ class CMS.Views.ModuleEdit extends Backbone.View course: course_location_analytics id: _this.model.id - data.metadata = _.extend(data.metadata || {}, @metadata()) + data.metadata = _.extend(data.metadata || {}, @changedMetadata()) @hideModal() @model.save(data).done( => # # showToastMessage("Your changes have been saved.", null, 3) diff --git a/cms/templates/widgets/metadata-edit.html b/cms/templates/widgets/metadata-edit.html index 51fe400f88..9693c18e9c 100644 --- a/cms/templates/widgets/metadata-edit.html +++ b/cms/templates/widgets/metadata-edit.html @@ -1,5 +1,6 @@ <% import hashlib + from xmodule.fields import StringyInteger, StringyFloat hlskey = hashlib.md5(module.location.url()).hexdigest() %>
@@ -7,17 +8,40 @@ % for field_name, field_value in editable_metadata_fields.items():
  • % if field_name == 'source_code': - Edit High Level Source + % if field_value['is_default'] is False: + Edit High Level Source + % endif % else: - - + + + ## Change to True to see all the information being passed through. + % if False: + + + + + % if field_value['field'].values: + + % for value in field_value['field'].values: + + % endfor + % endif + % endif % endif
    • % endfor - % if 'source_code' in editable_metadata_fields: - <%include file="source-edit.html" /> + % if 'source_code' in editable_metadata_fields and not editable_metadata_fields['source_code']['is_default']: + <%include file="source-edit.html" /> % endif
    diff --git a/cms/templates/widgets/source-edit.html b/cms/templates/widgets/source-edit.html index 883190d6b3..b7ee6c9db9 100644 --- a/cms/templates/widgets/source-edit.html +++ b/cms/templates/widgets/source-edit.html @@ -12,7 +12,7 @@
    - +
    diff --git a/common/djangoapps/util/memcache.py b/common/djangoapps/util/memcache.py index 540cf96539..ee450e68cb 100644 --- a/common/djangoapps/util/memcache.py +++ b/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 diff --git a/common/djangoapps/util/tests/__init__.py b/common/djangoapps/util/tests/__init__.py new file mode 100644 index 0000000000..8b13789179 --- /dev/null +++ b/common/djangoapps/util/tests/__init__.py @@ -0,0 +1 @@ + diff --git a/common/djangoapps/util/tests/test_memcache.py b/common/djangoapps/util/tests/test_memcache.py new file mode 100644 index 0000000000..de8d352c38 --- /dev/null +++ b/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 diff --git a/common/djangoapps/util/tests.py b/common/djangoapps/util/tests/test_zendesk.py similarity index 99% rename from common/djangoapps/util/tests.py rename to common/djangoapps/util/tests/test_zendesk.py index d829676eaf..51d06a92ed 100644 --- a/common/djangoapps/util/tests.py +++ b/common/djangoapps/util/tests/test_zendesk.py @@ -1,4 +1,4 @@ -"""Tests for the util package""" +"""Tests for the Zendesk""" from django.conf import settings from django.contrib.auth.models import AnonymousUser diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index 4143345196..479cd5a759 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/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 diff --git a/common/lib/xmodule/xmodule/combined_open_ended_module.py b/common/lib/xmodule/xmodule/combined_open_ended_module.py index 239adcaa41..67ff206e89 100644 --- a/common/lib/xmodule/xmodule/combined_open_ended_module.py +++ b/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): diff --git a/common/lib/xmodule/xmodule/discussion_module.py b/common/lib/xmodule/xmodule/discussion_module.py index 8968e221b2..98082ddea2 100644 --- a/common/lib/xmodule/xmodule/discussion_module.py +++ b/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 diff --git a/common/lib/xmodule/xmodule/html_module.py b/common/lib/xmodule/xmodule/html_module.py index d901fc5fbe..bbf24a6320 100644 --- a/common/lib/xmodule/xmodule/html_module.py +++ b/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): """ diff --git a/common/lib/xmodule/xmodule/js/src/.gitignore b/common/lib/xmodule/xmodule/js/src/.gitignore index bbd93c90e3..456e71bf8b 100644 --- a/common/lib/xmodule/xmodule/js/src/.gitignore +++ b/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 diff --git a/common/lib/xmodule/xmodule/js/src/capa/.gitignore b/common/lib/xmodule/xmodule/js/src/capa/.gitignore new file mode 100644 index 0000000000..13b8deb002 --- /dev/null +++ b/common/lib/xmodule/xmodule/js/src/capa/.gitignore @@ -0,0 +1,2 @@ +!imageinput.js +!schematic.js diff --git a/common/lib/xmodule/xmodule/js/src/graphical_slider_tool/.gitignore b/common/lib/xmodule/xmodule/js/src/graphical_slider_tool/.gitignore new file mode 100644 index 0000000000..d4aa116a26 --- /dev/null +++ b/common/lib/xmodule/xmodule/js/src/graphical_slider_tool/.gitignore @@ -0,0 +1 @@ +!*.js diff --git a/common/lib/xmodule/xmodule/js/src/peergrading/peer_grading.coffee b/common/lib/xmodule/xmodule/js/src/peergrading/peer_grading.coffee index 45c678bad9..48980c7d88 100644 --- a/common/lib/xmodule/xmodule/js/src/peergrading/peer_grading.coffee +++ b/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() diff --git a/common/lib/xmodule/xmodule/js/src/poll/.gitignore b/common/lib/xmodule/xmodule/js/src/poll/.gitignore new file mode 100644 index 0000000000..d4aa116a26 --- /dev/null +++ b/common/lib/xmodule/xmodule/js/src/poll/.gitignore @@ -0,0 +1 @@ +!*.js diff --git a/common/lib/xmodule/xmodule/js/src/sequence/display/.gitignore b/common/lib/xmodule/xmodule/js/src/sequence/display/.gitignore new file mode 100644 index 0000000000..d4aa116a26 --- /dev/null +++ b/common/lib/xmodule/xmodule/js/src/sequence/display/.gitignore @@ -0,0 +1 @@ +!*.js diff --git a/common/lib/xmodule/xmodule/js/src/videoalpha/display/.gitignore b/common/lib/xmodule/xmodule/js/src/videoalpha/display/.gitignore new file mode 100644 index 0000000000..c7a88ce092 --- /dev/null +++ b/common/lib/xmodule/xmodule/js/src/videoalpha/display/.gitignore @@ -0,0 +1 @@ +!html5_video.js diff --git a/common/lib/xmodule/xmodule/mako_module.py b/common/lib/xmodule/xmodule/mako_module.py index 84db6ad779..8abb1d7777 100644 --- a/common/lib/xmodule/xmodule/mako_module.py +++ b/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 diff --git a/common/lib/xmodule/xmodule/peer_grading_module.py b/common/lib/xmodule/xmodule/peer_grading_module.py index 35f2fa2d76..1ad31922f5 100644 --- a/common/lib/xmodule/xmodule/peer_grading_module.py +++ b/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) diff --git a/common/lib/xmodule/xmodule/tests/test_xml_module.py b/common/lib/xmodule/xmodule/tests/test_xml_module.py new file mode 100644 index 0000000000..06d5b0b0a3 --- /dev/null +++ b/common/lib/xmodule/xmodule/tests/test_xml_module.py @@ -0,0 +1,79 @@ +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) + # 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) + + +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_display_name_default(editable_fields) + + def test_override_default(self): + # Tests that is_default is correct when a value overrides the default. + editable_fields = self.get_xml_editable_fields({'display_name': 'foo'}) + display_name = editable_fields['display_name'] + self.assertFalse(display_name['is_default']) + self.assertEqual('foo', display_name['value']) + + def test_additional_field(self): + editable_fields = self.get_module_editable_fields({'max_attempts' : '7'}) + self.assertEqual(2, len(editable_fields)) + self.assert_field_values(editable_fields, 'max_attempts', TestFields.max_attempts, False, False, 7) + self.assert_display_name_default(editable_fields) + + editable_fields = self.get_module_editable_fields({}) + self.assert_field_values(editable_fields, 'max_attempts', TestFields.max_attempts, True, False, None) + + def test_inherited_field(self): + editable_fields = self.get_module_editable_fields({'display_name' : 'inherited'}) + self.assert_field_values(editable_fields, 'display_name', XModuleFields.display_name, False, True, 'inherited') + + # Start of helper methods + def get_xml_editable_fields(self, model_data): + system = test_system() + system.render_template = Mock(return_value="
    Test Template HTML
    ") + return XmlDescriptor(system=system, location=None, model_data=model_data).editable_metadata_fields + + def get_module_editable_fields(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="
    Test Template HTML
    ") + descriptor = TestModuleDescriptor(system=system, location=None, model_data=model_data) + descriptor._inherited_metadata = {'display_name' : 'inherited'} + return descriptor.editable_metadata_fields + + def assert_display_name_default(self, editable_fields): + self.assert_field_values(editable_fields, 'display_name', XModuleFields.display_name, True, False, None) + + def assert_field_values(self, editable_fields, name, field, is_default, is_inherited, value): + test_field = editable_fields[name] + self.assertEqual(field, test_field['field']) + self.assertEqual(is_default, test_field['is_default']) + self.assertEqual(is_inherited, test_field['is_inherited']) + self.assertEqual(value, test_field['value']) diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index 1fd0b8e138..749ca66258 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/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,48 @@ 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', {}) + metadata = {} + for field in self.fields: + + if field.scope != Scope.settings or field in self.non_editable_metadata_fields: + continue + + inherited = False + default = False + value = getattr(self, field.name) + if field.name in self._model_data: + default = False + if field.name in inherited_metadata: + if self._model_data.get(field.name) == inherited_metadata.get(field.name): + inherited = True + else: + default = True + + metadata[field.name] = {'field': field, + 'value': value, + 'is_inherited': inherited, + 'is_default': default} + + return metadata + class DescriptorSystem(object): def __init__(self, load_item, resources_fs, error_tracker, **kwargs): diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py index f9de929c05..7480cda0c5 100644 --- a/common/lib/xmodule/xmodule/xml_module.py +++ b/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 diff --git a/lms/djangoapps/courseware/tests/tests.py b/lms/djangoapps/courseware/tests/tests.py index 4c9f592797..d5064ec5e5 100644 --- a/lms/djangoapps/courseware/tests/tests.py +++ b/lms/djangoapps/courseware/tests/tests.py @@ -1,13 +1,13 @@ ''' Test for lms courseware app ''' - import logging import json import time import random from urlparse import urlsplit, urlunsplit +from uuid import uuid4 from django.contrib.auth.models import User, Group from django.test import TestCase @@ -62,7 +62,7 @@ def mongo_store_config(data_dir): 'default_class': 'xmodule.raw_module.RawDescriptor', 'host': 'localhost', 'db': 'test_xmodule', - 'collection': 'modulestore', + 'collection': 'modulestore_%s' % uuid4().hex, 'fs_root': data_dir, 'render_template': 'mitxmako.shortcuts.render_to_string', } @@ -81,7 +81,7 @@ def draft_mongo_store_config(data_dir): 'default_class': 'xmodule.raw_module.RawDescriptor', 'host': 'localhost', 'db': 'test_xmodule', - 'collection': 'modulestore', + 'collection': 'modulestore_%s' % uuid4().hex, 'fs_root': data_dir, 'render_template': 'mitxmako.shortcuts.render_to_string', } @@ -92,7 +92,7 @@ def draft_mongo_store_config(data_dir): 'default_class': 'xmodule.raw_module.RawDescriptor', 'host': 'localhost', 'db': 'test_xmodule', - 'collection': 'modulestore', + 'collection': 'modulestore_%s' % uuid4().hex, 'fs_root': data_dir, 'render_template': 'mitxmako.shortcuts.render_to_string', } diff --git a/lms/djangoapps/open_ended_grading/open_ended_notifications.py b/lms/djangoapps/open_ended_grading/open_ended_notifications.py index 6d5f2a3eb4..1d6fa22929 100644 --- a/lms/djangoapps/open_ended_grading/open_ended_notifications.py +++ b/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 + #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} - pending_grading = False - - 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 diff --git a/lms/lib/comment_client/utils.py b/lms/lib/comment_client/utils.py index 860035dc06..53bdd462ad 100644 --- a/lms/lib/comment_client/utils.py +++ b/lms/lib/comment_client/utils.py @@ -37,6 +37,10 @@ def perform_request(method, url, data_or_params=None, *args, **kwargs): else: response = requests.request(method, url, params=data_or_params, timeout=5) except Exception as err: + # remove API key if it is in the params + if 'api_key' in data_or_params: + log.info('Deleting API key from params') + del data_or_params['api_key'] log.exception("Trying to call {method} on {url} with params {params}".format( method=method, url=url, params=data_or_params)) # Reraise with a single exception type diff --git a/lms/static/sass/shared/_modal.scss b/lms/static/sass/shared/_modal.scss index 2da64d54a6..8ff58c1c14 100644 --- a/lms/static/sass/shared/_modal.scss +++ b/lms/static/sass/shared/_modal.scss @@ -149,7 +149,7 @@ } label { - color: #999; + color: #646464; &.field-error { display: block; diff --git a/lms/templates/courseware/courseware.html b/lms/templates/courseware/courseware.html index 33dc9562a7..4889da25ca 100644 --- a/lms/templates/courseware/courseware.html +++ b/lms/templates/courseware/courseware.html @@ -156,7 +156,7 @@
    - +
    Hints @@ -176,8 +176,8 @@
    - - + +
    diff --git a/lms/templates/forgot_password_modal.html b/lms/templates/forgot_password_modal.html index 97b41d4be9..f0f571a817 100644 --- a/lms/templates/forgot_password_modal.html +++ b/lms/templates/forgot_password_modal.html @@ -12,19 +12,19 @@
    - - + +
    -
    +

    ✕

    -
    + @@ -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) diff --git a/lms/templates/login_modal.html b/lms/templates/login_modal.html index 1587cca767..de1c437caf 100644 --- a/lms/templates/login_modal.html +++ b/lms/templates/login_modal.html @@ -9,14 +9,17 @@
    - - - - -