From cb54081d2eb4795b334d812edc76433d36d11935 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Tue, 6 Aug 2013 09:55:49 -0400 Subject: [PATCH 001/125] Fix notification problem --- .../open_ended_grading/open_ended_notifications.py | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/lms/djangoapps/open_ended_grading/open_ended_notifications.py b/lms/djangoapps/open_ended_grading/open_ended_notifications.py index 1d6fa22929..44ff41be22 100644 --- a/lms/djangoapps/open_ended_grading/open_ended_notifications.py +++ b/lms/djangoapps/open_ended_grading/open_ended_notifications.py @@ -146,19 +146,7 @@ def combined_notifications(course, user): #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=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: - #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} + last_time_viewed = last_login - datetime.timedelta(seconds=(NOTIFICATION_CACHE_TIME + 60)) try: #Get the notifications from the grading controller From 1be6ce3ee387ead051b001112c0ef68a7a172b03 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Thu, 8 Aug 2013 08:34:08 -0400 Subject: [PATCH 002/125] Add in and route control options --- .../xmodule/combined_open_ended_module.py | 33 +++++++++++++++++-- .../combined_open_ended_modulev1.py | 11 +++++++ .../open_ended_module.py | 1 + .../openendedchild.py | 1 + 4 files changed, 44 insertions(+), 2 deletions(-) diff --git a/common/lib/xmodule/xmodule/combined_open_ended_module.py b/common/lib/xmodule/xmodule/combined_open_ended_module.py index e01ae49149..f8ae7a3f13 100644 --- a/common/lib/xmodule/xmodule/combined_open_ended_module.py +++ b/common/lib/xmodule/xmodule/combined_open_ended_module.py @@ -14,7 +14,8 @@ import textwrap log = logging.getLogger("mitx.courseware") V1_SETTINGS_ATTRIBUTES = ["display_name", "max_attempts", "graded", "accept_file_upload", - "skip_spelling_checks", "due", "graceperiod", "weight"] + "skip_spelling_checks", "due", "graceperiod", "weight", "min_to_calibrate", + "max_to_calibrate", "peer_grader_count", "required_peer_grading"] V1_STUDENT_ATTRIBUTES = ["current_task_number", "task_states", "state", "student_attempts", "ready_to_reset"] @@ -37,7 +38,7 @@ DEFAULT_DATA = textwrap.dedent("""\

- Write a persuasive essay to a newspaper reflecting your vies on censorship in libraries. Do you believe that certain materials, such as books, music, movies, magazines, etc., should be removed from the shelves if they are found offensive? Support your position with convincing arguments from your own experience, observations, and/or reading. + Write a persuasive essay to a newspaper reflecting your views on censorship in libraries. Do you believe that certain materials, such as books, music, movies, magazines, etc., should be removed from the shelves if they are found offensive? Support your position with convincing arguments from your own experience, observations, and/or reading.

@@ -244,6 +245,34 @@ class CombinedOpenEndedFields(object): values={"min" : 0 , "step": ".1"}, default=1 ) + min_to_calibrate = Integer( + display_name="Minimum Peer Grading Calibrations", + help="The minimum number of calibration essays each student will need to complete for peer grading.", + default=1, + scope=Scope.settings, + values={"min" : 1, "step" : "1"} + ) + max_to_calibrate = Integer( + display_name="Maximum Peer Grading Calibrations", + help="The maximum number of calibration essays each student will need to complete for peer grading.", + default=1, + scope=Scope.settings, + values={"max" : 20, "step" : "1"} + ) + peer_grader_count = Integer( + display_name="Peer Graders per Response", + help="The number of peers who will grade each submission.", + default=1, + scope=Scope.settings, + values={"min" : 1, "step" : "1", "max" : 5} + ) + required_peer_grading = Integer( + display_name="Required Peer Grading", + help="The number of other students each student making a submission will have to grade.", + default=1, + scope=Scope.settings, + values={"min" : 1, "step" : "1", "max" : 5} + ) markdown = String( help="Markdown source of this module", default=textwrap.dedent("""\ diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py index 933eb0b5bb..c65d30968d 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py @@ -106,6 +106,11 @@ class CombinedOpenEndedV1Module(): self.accept_file_upload = instance_state.get('accept_file_upload', ACCEPT_FILE_UPLOAD) in TRUE_DICT self.skip_basic_checks = instance_state.get('skip_spelling_checks', SKIP_BASIC_CHECKS) in TRUE_DICT + self.required_peer_grading = instance_state.get('required_peer_grading', 3) + self.peer_grader_count = instance_state.get('peer_grader_count', 3) + self.min_to_calibrate = instance_state.get('min_to_calibrate', 3) + self.max_to_calibrate = instance_state.get('max_to_calibrate', 6) + due_date = instance_state.get('due', None) grace_period_string = instance_state.get('graceperiod', None) @@ -131,6 +136,12 @@ class CombinedOpenEndedV1Module(): 'close_date': self.timeinfo.close_date, 's3_interface': self.system.s3_interface, 'skip_basic_checks': self.skip_basic_checks, + 'control': { + 'required_peer_grading': self.required_peer_grading, + 'peer_grader_count': self.peer_grader_count, + 'min_to_calibrate': self.min_to_calibrate, + 'max_to_calibrate': self.max_to_calibrate, + } } self.task_xml = definition['task_xml'] diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py index 2e7a3eaf89..924ca2c23d 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py @@ -118,6 +118,7 @@ class OpenEndedModule(openendedchild.OpenEndedChild): 'answer': self.answer, 'problem_id': self.display_name, 'skip_basic_checks': self.skip_basic_checks, + 'control': json.dumps(self.control), }) updated_grader_payload = json.dumps(parsed_grader_payload) diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/openendedchild.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/openendedchild.py index 10f939b270..7138dcc723 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/openendedchild.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/openendedchild.py @@ -92,6 +92,7 @@ class OpenEndedChild(object): self.s3_interface = static_data['s3_interface'] self.skip_basic_checks = static_data['skip_basic_checks'] self._max_score = static_data['max_score'] + self.control = static_data['control'] # Used for progress / grading. Currently get credit just for # completion (doesn't matter if you self-assessed correct/incorrect). From 4b5aba29ca3fc0b24ea4195bf5cf5f50065ff7db Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Thu, 8 Aug 2013 09:52:13 -0400 Subject: [PATCH 003/125] Fix defaults --- common/lib/xmodule/xmodule/combined_open_ended_module.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/common/lib/xmodule/xmodule/combined_open_ended_module.py b/common/lib/xmodule/xmodule/combined_open_ended_module.py index f8ae7a3f13..faf22d1926 100644 --- a/common/lib/xmodule/xmodule/combined_open_ended_module.py +++ b/common/lib/xmodule/xmodule/combined_open_ended_module.py @@ -248,28 +248,28 @@ class CombinedOpenEndedFields(object): min_to_calibrate = Integer( display_name="Minimum Peer Grading Calibrations", help="The minimum number of calibration essays each student will need to complete for peer grading.", - default=1, + default=3, scope=Scope.settings, values={"min" : 1, "step" : "1"} ) max_to_calibrate = Integer( display_name="Maximum Peer Grading Calibrations", help="The maximum number of calibration essays each student will need to complete for peer grading.", - default=1, + default=6, scope=Scope.settings, values={"max" : 20, "step" : "1"} ) peer_grader_count = Integer( display_name="Peer Graders per Response", help="The number of peers who will grade each submission.", - default=1, + default=3, scope=Scope.settings, values={"min" : 1, "step" : "1", "max" : 5} ) required_peer_grading = Integer( display_name="Required Peer Grading", help="The number of other students each student making a submission will have to grade.", - default=1, + default=3, scope=Scope.settings, values={"min" : 1, "step" : "1", "max" : 5} ) From 6e41c082b11a89ec4f54fe5b8e362dd460dff570 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Fri, 9 Aug 2013 12:30:05 -0400 Subject: [PATCH 004/125] Fix tests --- .../xmodule/tests/test_combined_open_ended.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py index 4fd0ddccf7..268f8f0b69 100644 --- a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py +++ b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py @@ -64,6 +64,12 @@ class OpenEndedChildTest(unittest.TestCase): 's3_interface': "", 'open_ended_grading_interface': {}, 'skip_basic_checks': False, + 'control': { + 'required_peer_grading': 1, + 'peer_grader_count': 1, + 'min_to_calibrate': 3, + 'max_to_calibrate': 6, + } } definition = Mock() descriptor = Mock() @@ -180,6 +186,12 @@ class OpenEndedModuleTest(unittest.TestCase): 's3_interface': test_util_open_ended.S3_INTERFACE, 'open_ended_grading_interface': test_util_open_ended.OPEN_ENDED_GRADING_INTERFACE, 'skip_basic_checks': False, + 'control': { + 'required_peer_grading': 1, + 'peer_grader_count': 1, + 'min_to_calibrate': 3, + 'max_to_calibrate': 6, + } } oeparam = etree.XML(''' From 8cf82f297371df61f93b346bf736b53daec9f381 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Fri, 9 Aug 2013 14:38:30 -0400 Subject: [PATCH 005/125] Fix self assessment test --- common/lib/xmodule/xmodule/tests/test_self_assessment.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/common/lib/xmodule/xmodule/tests/test_self_assessment.py b/common/lib/xmodule/xmodule/tests/test_self_assessment.py index 0ccc6864cd..c9140d643a 100644 --- a/common/lib/xmodule/xmodule/tests/test_self_assessment.py +++ b/common/lib/xmodule/xmodule/tests/test_self_assessment.py @@ -49,6 +49,12 @@ class SelfAssessmentTest(unittest.TestCase): 's3_interface': test_util_open_ended.S3_INTERFACE, 'open_ended_grading_interface': test_util_open_ended.OPEN_ENDED_GRADING_INTERFACE, 'skip_basic_checks': False, + 'control': { + 'required_peer_grading': 1, + 'peer_grader_count': 1, + 'min_to_calibrate': 3, + 'max_to_calibrate': 6, + } } self.module = SelfAssessmentModule(get_test_system(), self.location, From 835edbf32f5554f1e5cb7829f0ce988f4ec8fa8d Mon Sep 17 00:00:00 2001 From: cahrens Date: Fri, 9 Aug 2013 14:46:18 -0400 Subject: [PATCH 006/125] Change locators to a restful interface. Don't use ; @ and # as separators. --- .../xmodule/xmodule/modulestore/locator.py | 39 +++++----- .../xmodule/xmodule/modulestore/parsers.py | 34 +++++---- .../modulestore/tests/test_locators.py | 74 ++++++++++++------- 3 files changed, 86 insertions(+), 61 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/locator.py b/common/lib/xmodule/xmodule/modulestore/locator.py index 591ef3115f..3e20f3e1b4 100644 --- a/common/lib/xmodule/xmodule/modulestore/locator.py +++ b/common/lib/xmodule/xmodule/modulestore/locator.py @@ -1,8 +1,7 @@ """ -Created on Mar 13, 2013 +Identifier for course resources. +""" -@author: dmitchell -""" from __future__ import absolute_import import logging import inspect @@ -15,6 +14,7 @@ from bson.errors import InvalidId from xmodule.modulestore.exceptions import InsufficientSpecificationError, OverSpecificationError from .parsers import parse_url, parse_course_id, parse_block_ref +from .parsers import BRANCH_PREFIX, BLOCK_PREFIX, URL_VERSION_PREFIX log = logging.getLogger(__name__) @@ -37,9 +37,6 @@ class Locator(object): """ raise InsufficientSpecificationError() - def quoted_url(self): - return quote(self.url(), '@;#') - def __eq__(self, other): return self.__dict__ == other.__dict__ @@ -90,11 +87,11 @@ class CourseLocator(Locator): Examples of valid CourseLocator specifications: CourseLocator(version_guid=ObjectId('519665f6223ebd6980884f2b')) CourseLocator(course_id='mit.eecs.6002x') - CourseLocator(course_id='mit.eecs.6002x;published') + CourseLocator(course_id='mit.eecs.6002x/branch/published') CourseLocator(course_id='mit.eecs.6002x', branch='published') - CourseLocator(url='edx://@519665f6223ebd6980884f2b') + CourseLocator(url='edx://version/519665f6223ebd6980884f2b') CourseLocator(url='edx://mit.eecs.6002x') - CourseLocator(url='edx://mit.eecs.6002x;published') + CourseLocator(url='edx://mit.eecs.6002x/branch/published') Should have at lease a specific course_id (id for the course as if it were a project w/ versions) with optional 'branch', @@ -115,10 +112,10 @@ class CourseLocator(Locator): if self.course_id: result = self.course_id if self.branch: - result += ';' + self.branch + result += BRANCH_PREFIX + self.branch return result elif self.version_guid: - return '@' + str(self.version_guid) + return URL_VERSION_PREFIX + str(self.version_guid) else: # raise InsufficientSpecificationError("missing course_id or version_guid") return '' @@ -224,7 +221,7 @@ class CourseLocator(Locator): """ url must be a string beginning with 'edx://' and containing either a valid version_guid or course_id (with optional branch) - If a block ('#HW3') is present, it is ignored. + If a block ('/block/HW3') is present, it is ignored. """ if isinstance(url, Locator): url = url.url() @@ -253,14 +250,14 @@ class CourseLocator(Locator): def init_from_course_id(self, course_id, explicit_branch=None): """ - Course_id is a string like 'mit.eecs.6002x' or 'mit.eecs.6002x;published'. + Course_id is a string like 'mit.eecs.6002x' or 'mit.eecs.6002x/branch/published'. Revision (optional) is a string like 'published'. It may be provided explicitly (explicit_branch) or embedded into course_id. - If branch is part of course_id ("...;published"), parse it out separately. + If branch is part of course_id (".../branch/published"), parse it out separately. If branch is provided both ways, that's ok as long as they are the same value. - If a block ('#HW3') is a part of course_id, it is ignored. + If a block ('/block/HW3') is a part of course_id, it is ignored. """ @@ -411,9 +408,9 @@ class BlockUsageLocator(CourseLocator): rep = CourseLocator.__unicode__(self) if self.usage_id is None: # usage_id has not been initialized - return rep + '#NONE' + return rep + BLOCK_PREFIX + 'NONE' else: - return rep + '#' + self.usage_id + return rep + BLOCK_PREFIX + self.usage_id class DescriptionLocator(Locator): @@ -427,14 +424,14 @@ class DescriptionLocator(Locator): def __unicode__(self): ''' Return a string representing this location. - unicode(self) returns something like this: "@519665f6223ebd6980884f2b" + unicode(self) returns something like this: "version/519665f6223ebd6980884f2b" ''' - return '@' + str(self.definition_guid) + return URL_VERSION_PREFIX + str(self.definition_id) def url(self): """ Return a string containing the URL for this location. - url(self) returns something like this: 'edx://@519665f6223ebd6980884f2b' + url(self) returns something like this: 'edx://version/519665f6223ebd6980884f2b' """ return 'edx://' + unicode(self) @@ -442,7 +439,7 @@ class DescriptionLocator(Locator): """ Returns the ObjectId referencing this specific location. """ - return self.definition_guid + return self.definition_id class VersionTree(object): diff --git a/common/lib/xmodule/xmodule/modulestore/parsers.py b/common/lib/xmodule/xmodule/modulestore/parsers.py index 8e5b685cec..efdf1c9e18 100644 --- a/common/lib/xmodule/xmodule/modulestore/parsers.py +++ b/common/lib/xmodule/xmodule/modulestore/parsers.py @@ -1,5 +1,12 @@ import re +# Prefix for the branch portion of a locator URL +BRANCH_PREFIX = "/branch/" +# Prefix for the block portion of a locator URL +BLOCK_PREFIX = "/block/" +# Prefix for when a course URL begins with a version ID +URL_VERSION_PREFIX = 'version/' + URL_RE = re.compile(r'^edx://(.+)$', re.IGNORECASE) @@ -9,10 +16,10 @@ def parse_url(string): followed by either a version_guid or a course_id. Examples: - 'edx://@0123FFFF' + 'edx://version/0123FFFF' 'edx://edu.mit.eecs.6002x' - 'edx://edu.mit.eecs.6002x;published' - 'edx://edu.mit.eecs.6002x;published#HW3' + 'edx://edu.mit.eecs.6002x/branch/published' + 'edx://edu.mit.eecs.6002x/branch/published/block/HW3' This returns None if string cannot be parsed. @@ -27,8 +34,8 @@ def parse_url(string): if not match: return None path = match.group(1) - if path[0] == '@': - return parse_guid(path[1:]) + if path.startswith(URL_VERSION_PREFIX): + return parse_guid(path[len(URL_VERSION_PREFIX):]) return parse_course_id(path) @@ -52,8 +59,7 @@ def parse_block_ref(string): return None -GUID_RE = re.compile(r'^(?P[A-F0-9]+)(#(?P\w+))?$', re.IGNORECASE) - +GUID_RE = re.compile(r'^(?P[A-F0-9]+)(' + BLOCK_PREFIX + '(?P\w+))?$', re.IGNORECASE) def parse_guid(string): """ @@ -69,27 +75,27 @@ def parse_guid(string): return None -COURSE_ID_RE = re.compile(r'^(?P(\w+)(\.\w+\w*)*)(;(?P\w+))?(#(?P\w+))?$', re.IGNORECASE) +COURSE_ID_RE = re.compile(r'^(?P(\w+)(\.\w+\w*)*)('+ BRANCH_PREFIX + '(?P\w+))?(' + BLOCK_PREFIX + '(?P\w+))?$', re.IGNORECASE) def parse_course_id(string): r""" A course_id has a main id component. - There may also be an optional branch (;published or ;draft). - There may also be an optional block (#HW3 or #Quiz2). + There may also be an optional branch (/branch/published or /branch/draft). + There may also be an optional block (/block/HW3 or /block/Quiz2). Examples of valid course_ids: 'edu.mit.eecs.6002x' - 'edu.mit.eecs.6002x;published' - 'edu.mit.eecs.6002x#HW3' - 'edu.mit.eecs.6002x;published#HW3' + 'edu.mit.eecs.6002x/branch/published' + 'edu.mit.eecs.6002x/block/HW3' + 'edu.mit.eecs.6002x/branch/published/block/HW3' Syntax: - course_id = main_id [; branch] [# block] + course_id = main_id [/branch/ branch] [/block/ block] main_id = name [. name]* diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_locators.py b/common/lib/xmodule/xmodule/modulestore/tests/test_locators.py index bb41131234..0f39a4c66f 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_locators.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_locators.py @@ -1,12 +1,11 @@ -''' -Created on Mar 14, 2013 - -@author: dmitchell -''' +""" +Tests for xmodule.modulestore.locator. +""" from unittest import TestCase from bson.objectid import ObjectId -from xmodule.modulestore.locator import Locator, CourseLocator, BlockUsageLocator +from xmodule.modulestore.locator import Locator, CourseLocator, BlockUsageLocator, DescriptionLocator +from xmodule.modulestore.parsers import BRANCH_PREFIX, BLOCK_PREFIX, URL_VERSION_PREFIX from xmodule.modulestore.exceptions import InsufficientSpecificationError, OverSpecificationError @@ -32,12 +31,12 @@ class LocatorTest(TestCase): self.assertRaises( OverSpecificationError, CourseLocator, - url='edx://mit.eecs.6002x;published', + url='edx://mit.eecs.6002x' + BRANCH_PREFIX + 'published', branch='draft') self.assertRaises( OverSpecificationError, CourseLocator, - course_id='mit.eecs.6002x;published', + course_id='mit.eecs.6002x' + BRANCH_PREFIX + 'published', branch='draft') def test_course_constructor_underspecified(self): @@ -55,8 +54,8 @@ class LocatorTest(TestCase): testobj_1 = CourseLocator(version_guid=test_id_1) self.check_course_locn_fields(testobj_1, 'version_guid', version_guid=test_id_1) self.assertEqual(str(testobj_1.version_guid), test_id_1_loc) - self.assertEqual(str(testobj_1), '@' + test_id_1_loc) - self.assertEqual(testobj_1.url(), 'edx://@' + test_id_1_loc) + self.assertEqual(str(testobj_1), URL_VERSION_PREFIX + test_id_1_loc) + self.assertEqual(testobj_1.url(), 'edx://' + URL_VERSION_PREFIX + test_id_1_loc) # Test using a given string test_id_2_loc = '519665f6223ebd6980884f2b' @@ -64,8 +63,8 @@ class LocatorTest(TestCase): testobj_2 = CourseLocator(version_guid=test_id_2) self.check_course_locn_fields(testobj_2, 'version_guid', version_guid=test_id_2) self.assertEqual(str(testobj_2.version_guid), test_id_2_loc) - self.assertEqual(str(testobj_2), '@' + test_id_2_loc) - self.assertEqual(testobj_2.url(), 'edx://@' + test_id_2_loc) + self.assertEqual(str(testobj_2), URL_VERSION_PREFIX + test_id_2_loc) + self.assertEqual(testobj_2.url(), 'edx://'+ URL_VERSION_PREFIX + test_id_2_loc) def test_course_constructor_bad_course_id(self): """ @@ -74,20 +73,20 @@ class LocatorTest(TestCase): for bad_id in ('mit.', ' mit.eecs', 'mit.eecs ', - '@mit.eecs', - '#mit.eecs', + URL_VERSION_PREFIX + 'mit.eecs', + BLOCK_PREFIX + 'block/mit.eecs', 'mit.ee cs', 'mit.ee,cs', 'mit.ee/cs', 'mit.ee$cs', 'mit.ee&cs', 'mit.ee()cs', - ';this', - 'mit.eecs;', - 'mit.eecs;this;that', - 'mit.eecs;this;', - 'mit.eecs;this ', - 'mit.eecs;th%is ', + BRANCH_PREFIX + 'this', + 'mit.eecs' + BRANCH_PREFIX, + 'mit.eecs' + BRANCH_PREFIX + 'this' + BRANCH_PREFIX +'that', + 'mit.eecs' + BRANCH_PREFIX + 'this' + BRANCH_PREFIX , + 'mit.eecs' + BRANCH_PREFIX + 'this ', + 'mit.eecs' + BRANCH_PREFIX + 'th%is ', ): self.assertRaises(AssertionError, CourseLocator, course_id=bad_id) self.assertRaises(AssertionError, CourseLocator, url='edx://' + bad_id) @@ -106,7 +105,7 @@ class LocatorTest(TestCase): self.check_course_locn_fields(testobj, 'course_id', course_id=testurn) def test_course_constructor_redundant_002(self): - testurn = 'mit.eecs.6002x;published' + testurn = 'mit.eecs.6002x' + BRANCH_PREFIX + 'published' expected_urn = 'mit.eecs.6002x' expected_rev = 'published' testobj = CourseLocator(course_id=testurn, url='edx://' + testurn) @@ -114,6 +113,17 @@ class LocatorTest(TestCase): course_id=expected_urn, branch=expected_rev) + def test_course_constructor_url(self): + # Test parsing a url when it starts with a version ID and there is also a block ID. + # This hits the parsers parse_guid method. + test_id_loc = '519665f6223ebd6980884f2b' + testobj = CourseLocator(url="edx://" + URL_VERSION_PREFIX + test_id_loc + BLOCK_PREFIX + "hw3") + self.check_course_locn_fields( + testobj, + 'test_block constructor', + version_guid=ObjectId(test_id_loc) + ) + def test_course_constructor_course_id_no_branch(self): testurn = 'mit.eecs.6002x' testobj = CourseLocator(course_id=testurn) @@ -123,7 +133,7 @@ class LocatorTest(TestCase): self.assertEqual(testobj.url(), 'edx://' + testurn) def test_course_constructor_course_id_with_branch(self): - testurn = 'mit.eecs.6002x;published' + testurn = 'mit.eecs.6002x' + BRANCH_PREFIX + 'published' expected_id = 'mit.eecs.6002x' expected_branch = 'published' testobj = CourseLocator(course_id=testurn) @@ -139,7 +149,7 @@ class LocatorTest(TestCase): def test_course_constructor_course_id_separate_branch(self): test_id = 'mit.eecs.6002x' test_branch = 'published' - expected_urn = 'mit.eecs.6002x;published' + expected_urn = 'mit.eecs.6002x' + BRANCH_PREFIX + 'published' testobj = CourseLocator(course_id=test_id, branch=test_branch) self.check_course_locn_fields(testobj, 'course_id with separate branch', course_id=test_id, @@ -154,10 +164,10 @@ class LocatorTest(TestCase): """ The same branch appears in the course_id and the branch field. """ - test_id = 'mit.eecs.6002x;published' + test_id = 'mit.eecs.6002x' + BRANCH_PREFIX + 'published' test_branch = 'published' expected_id = 'mit.eecs.6002x' - expected_urn = 'mit.eecs.6002x;published' + expected_urn = test_id testobj = CourseLocator(course_id=test_id, branch=test_branch) self.check_course_locn_fields(testobj, 'course_id with repeated branch', course_id=expected_id, @@ -169,7 +179,7 @@ class LocatorTest(TestCase): self.assertEqual(testobj.url(), 'edx://' + expected_urn) def test_block_constructor(self): - testurn = 'mit.eecs.6002x;published#HW3' + testurn = 'mit.eecs.6002x' + BRANCH_PREFIX + 'published' + BLOCK_PREFIX + 'HW3' expected_id = 'mit.eecs.6002x' expected_branch = 'published' expected_block_ref = 'HW3' @@ -181,6 +191,18 @@ class LocatorTest(TestCase): self.assertEqual(str(testobj), testurn) self.assertEqual(testobj.url(), 'edx://' + testurn) + def test_repr(self): + testurn = 'mit.eecs.6002x' + BRANCH_PREFIX + 'published' + BLOCK_PREFIX + 'HW3' + testobj = BlockUsageLocator(course_id=testurn) + self.assertEqual('BlockUsageLocator("mit.eecs.6002x/branch/published/block/HW3")', repr(testobj)) + + def test_description_locator_url(self): + definition_locator=DescriptionLocator("chapter12345_2") + self.assertEqual('edx://' + URL_VERSION_PREFIX + 'chapter12345_2', definition_locator.url()) + + def test_description_locator_version(self): + definition_locator=DescriptionLocator("chapter12345_2") + self.assertEqual("chapter12345_2", definition_locator.version()) # ------------------------------------------------------------------ # Utilities From 5c9538a9a112129d9d44551e23bdcd2cad35949c Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Fri, 9 Aug 2013 16:46:25 -0400 Subject: [PATCH 007/125] Address review comments --- .../xmodule/xmodule/combined_open_ended_module.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/common/lib/xmodule/xmodule/combined_open_ended_module.py b/common/lib/xmodule/xmodule/combined_open_ended_module.py index faf22d1926..2856c98127 100644 --- a/common/lib/xmodule/xmodule/combined_open_ended_module.py +++ b/common/lib/xmodule/xmodule/combined_open_ended_module.py @@ -13,9 +13,11 @@ import textwrap log = logging.getLogger("mitx.courseware") -V1_SETTINGS_ATTRIBUTES = ["display_name", "max_attempts", "graded", "accept_file_upload", - "skip_spelling_checks", "due", "graceperiod", "weight", "min_to_calibrate", - "max_to_calibrate", "peer_grader_count", "required_peer_grading"] +V1_SETTINGS_ATTRIBUTES = [ + "display_name", "max_attempts", "graded", "accept_file_upload", + "skip_spelling_checks", "due", "graceperiod", "weight", "min_to_calibrate", + "max_to_calibrate", "peer_grader_count", "required_peer_grading", +] V1_STUDENT_ATTRIBUTES = ["current_task_number", "task_states", "state", "student_attempts", "ready_to_reset"] @@ -250,14 +252,14 @@ class CombinedOpenEndedFields(object): help="The minimum number of calibration essays each student will need to complete for peer grading.", default=3, scope=Scope.settings, - values={"min" : 1, "step" : "1"} + values={"min" : 1, "max" : 20, "step" : "1"} ) max_to_calibrate = Integer( display_name="Maximum Peer Grading Calibrations", help="The maximum number of calibration essays each student will need to complete for peer grading.", default=6, scope=Scope.settings, - values={"max" : 20, "step" : "1"} + values={"min" : 1, "max" : 20, "step" : "1"} ) peer_grader_count = Integer( display_name="Peer Graders per Response", From 8fa4b4dbd1e0ce95845f3464ca5684e3ed53e88f Mon Sep 17 00:00:00 2001 From: Peter Fogg Date: Tue, 6 Aug 2013 16:59:15 -0400 Subject: [PATCH 008/125] Change course create form to synchronous validation. --- .../contentstore/features/courses.feature | 2 +- cms/static/js/base.js | 157 +++++++++++------- cms/static/sass/elements/_forms.scss | 1 - cms/templates/index.html | 2 +- 4 files changed, 102 insertions(+), 60 deletions(-) diff --git a/cms/djangoapps/contentstore/features/courses.feature b/cms/djangoapps/contentstore/features/courses.feature index 455313b0e2..a0ba8099ac 100644 --- a/cms/djangoapps/contentstore/features/courses.feature +++ b/cms/djangoapps/contentstore/features/courses.feature @@ -8,6 +8,6 @@ Feature: Create Course And I am logged into Studio When I click the New Course button And I fill in the new course information - And I press the "Save" button + And I press the "Create" button Then the Courseware page has loaded in Studio And I see a link for adding a new section diff --git a/cms/static/js/base.js b/cms/static/js/base.js index 80b24776da..72ef5991a3 100644 --- a/cms/static/js/base.js +++ b/cms/static/js/base.js @@ -605,80 +605,118 @@ function cancelNewSection(e) { function addNewCourse(e) { e.preventDefault(); $('.new-course-button').addClass('is-disabled'); + $('.new-course-save').addClass('is-disabled'); var $newCourse = $('.wrapper-create-course').addClass('is-shown'); var $cancelButton = $newCourse.find('.new-course-cancel'); - $newCourse.find('.new-course-name').focus().select(); - $newCourse.find('form').bind('submit', saveNewCourse); + var $courseName = $('.new-course-name'); + $courseName.focus().select(); + $('.new-course-save').on('click', saveNewCourse); $cancelButton.bind('click', cancelNewCourse); $body.bind('keyup', { $cancelButton: $cancelButton }, checkForCancel); + // Handle validation asynchronously + _.each( + ['.new-course-org', '.new-course-number', '.new-course-run'], + function(ele) { + var $ele = $(ele); + $ele.on('keyup', function(event) { + // Don't bother showing "required field" error when + // the user tabs into a new field; this is distracting + // and unnecessary + if(event.keyCode === 9) { + return; + } + var error = validateCourseItemEncoding($ele.val()); + setNewCourseFieldInErr($ele.parent('li'), error); + validateTotalCourseItemsLength(); + }); + } + ); + var $name = $('.new-course-name'); + $name.on('keyup', function() { + var error = validateCourseName($name.val()); + setNewCourseFieldInErr($name.parent('li'), error); + validateTotalCourseItemsLength(); + }); +} + +function setNewCourseFieldInErr(el, msg) { + el.children('.tip-error').remove(); + if(msg) { + el.addClass('error'); + el.append('' + msg + ''); + $('.new-course-save').addClass('is-disabled'); + } + else { + el.removeClass('error'); + // One "error" div is always present, but hidden or shown + if($('.error').length === 1) { + $('.new-course-save').removeClass('is-disabled'); + } + } +}; + +function validateCourseName(name) { + if(name.length === 0) { + return gettext('Required field.'); + } + return ''; +} + +// Check that a course (org, number, run) doesn't use any special characters +function validateCourseItemEncoding(item) { + if(item === '') { + return gettext('Required field.'); + } + if(item !== encodeURIComponent(item)) { + return gettext('Please do not use any spaces or special characters in this field.'); + } + return ''; +} + +// Ensure that all items are less than 80 characters. +function validateTotalCourseItemsLength() { + var totalLength = _.reduce( + ['.new-course-name', '.new-course-org', '.new-course-number', '.new-course-run'], + function(sum, ele) { + return sum + $(ele).val().length; + }, 0 + ); + if(totalLength > 80) { + $('.wrap-error').addClass('is-shown'); + $('#course_creation_error').html('

' + gettext('Course fields must have a combined length of no more than 80 characters.') + '

'); + $('.new-course-save').addClass('is-disabled'); + } + else { + $('.wrap-error').removeClass('is-shown'); + } } function saveNewCourse(e) { e.preventDefault(); + // One final check for empty values + var errors = _.reduce( + ['.new-course-name', '.new-course-org', '.new-course-number', '.new-course-run'], + function(acc, ele) { + var $ele = $(ele); + var error = $ele.val().length === 0 ? gettext('Required field.') : ''; + setNewCourseFieldInErr($ele.parent('li'), error); + return error ? true : acc; + }, + false + ); + var $newCourseForm = $(this).closest('#create-course-form'); var display_name = $newCourseForm.find('.new-course-name').val(); var org = $newCourseForm.find('.new-course-org').val(); var number = $newCourseForm.find('.new-course-number').val(); var run = $newCourseForm.find('.new-course-run').val(); - var required_field_text = gettext('Required field'); - - var display_name_errMsg = (display_name === '') ? required_field_text : null; - var org_errMsg = (org === '') ? required_field_text : null; - var number_errMsg = (number === '') ? required_field_text : null; - var run_errMsg = (run === '') ? required_field_text : null; - - var bInErr = (display_name_errMsg || org_errMsg || number_errMsg || run_errMsg); - - // check for suitable encoding - if (!bInErr) { - var encoding_errMsg = gettext('Please do not use any spaces or special characters in this field.'); - - if (encodeURIComponent(org) != org) - org_errMsg = encoding_errMsg; - if (encodeURIComponent(number) != number) - number_errMsg = encoding_errMsg; - if (encodeURIComponent(run) != run) - run_errMsg = encoding_errMsg; - - bInErr = (org_errMsg || number_errMsg || run_errMsg); - } - - var header_err_msg = (bInErr) ? gettext('Please correct the fields below.') : null; - - var setNewCourseErrMsgs = function(header_err_msg, display_name_errMsg, org_errMsg, number_errMsg, run_errMsg) { - if (header_err_msg) { - $('.wrapper-create-course').addClass('has-errors'); - $('.wrap-error').addClass('is-shown'); - $('#course_creation_error').html('

' + header_err_msg + '

'); - } else { - $('.wrap-error').removeClass('is-shown'); - $('#course_creation_error').html(''); - } - - var setNewCourseFieldInErr = function(el, msg) { - el.children('.tip-error').remove(); - if (msg !== null && msg !== '') { - el.addClass('error'); - el.append('' + msg + ''); - } else { - el.removeClass('error'); - } - }; - - setNewCourseFieldInErr($('#field-course-name'), display_name_errMsg); - setNewCourseFieldInErr($('#field-organization'), org_errMsg); - setNewCourseFieldInErr($('#field-course-number'), number_errMsg); - setNewCourseFieldInErr($('#field-course-run'), run_errMsg); - }; - - setNewCourseErrMsgs(header_err_msg, display_name_errMsg, org_errMsg, number_errMsg, run_errMsg); - - if (bInErr) + if(errors) { return; + } analytics.track('Created a Course', { 'org': org, @@ -698,8 +736,13 @@ function saveNewCourse(e) { window.location = '/' + data.id.replace(/.*:\/\//, ''); } else if (data.ErrMsg !== undefined) { var orgErrMsg = (data.OrgErrMsg !== undefined) ? data.OrgErrMsg : null; + if(orgErrMsg) { + setNewCourseFieldInErr($('.new-course-org').parent('li'), orgErrMsg); + } var courseErrMsg = (data.CourseErrMsg !== undefined) ? data.CourseErrMsg : null; - setNewCourseErrMsgs(data.ErrMsg, null, orgErrMsg, courseErrMsg, null); + if(courseErrMsg) { + setNewCourseFieldInErr($('.new-course-number').parent('li'), orgErrMsg); + } } } ); diff --git a/cms/static/sass/elements/_forms.scss b/cms/static/sass/elements/_forms.scss index c78e2f3692..0d2dec68a0 100644 --- a/cms/static/sass/elements/_forms.scss +++ b/cms/static/sass/elements/_forms.scss @@ -226,7 +226,6 @@ form[class^="create-"] { } .tip-error { - @extend .anim-fadeIn; display: block; color: $red; } diff --git a/cms/templates/index.html b/cms/templates/index.html index 9702eedada..5a88054563 100644 --- a/cms/templates/index.html +++ b/cms/templates/index.html @@ -123,7 +123,7 @@
- +
From 05e1ffb190df99b84e03926f18df928e12d1525b Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Mon, 12 Aug 2013 12:49:15 -0400 Subject: [PATCH 009/125] remove unused code from xml_import.py --- .../xmodule/modulestore/xml_importer.py | 54 ------------------- 1 file changed, 54 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py index 0b30a884be..fa9228bed3 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py @@ -490,57 +490,3 @@ def perform_xlint(data_dir, course_dirs, print "This course can be imported successfully." return err_cnt - - -# -# UNSURE IF THIS IS UNUSED CODE - IF SO NEEDS TO BE PRUNED. TO BE INVESTIGATED. -# -def import_module_from_xml(modulestore, static_content_store, course_data_path, module, target_location_namespace=None, verbose=False): - # remap module to the new namespace - if target_location_namespace is not None: - # This looks a bit wonky as we need to also change the 'name' of the imported course to be what - # the caller passed in - if module.location.category != 'course': - module.location = module.location._replace(tag=target_location_namespace.tag, org=target_location_namespace.org, - course=target_location_namespace.course) - else: - module.location = module.location._replace(tag=target_location_namespace.tag, org=target_location_namespace.org, - course=target_location_namespace.course, name=target_location_namespace.name) - - # then remap children pointers since they too will be re-namespaced - if module.has_children: - children_locs = module.children - new_locs = [] - for child in children_locs: - child_loc = Location(child) - new_child_loc = child_loc._replace(tag=target_location_namespace.tag, org=target_location_namespace.org, - course=target_location_namespace.course) - - new_locs.append(new_child_loc.url()) - - module.children = new_locs - - if hasattr(module, 'data'): - modulestore.update_item(module.location, module.data) - - if module.has_children: - modulestore.update_children(module.location, module.children) - - modulestore.update_metadata(module.location, own_metadata(module)) - - -def import_course_from_xml(modulestore, static_content_store, course_data_path, module, target_location_namespace=None, verbose=False): - # CDODGE: Is this unused code (along with import_module_from_xml)? I can't find any references to it. If so, then - # we need to delete this apparently duplicate code. - - # cdodge: more hacks (what else). Seems like we have a problem when importing a course (like 6.002) which - # does not have any tabs defined in the policy file. The import goes fine and then displays fine in LMS, - # but if someone tries to add a new tab in the CMS, then the LMS barfs because it expects that - - # if there is *any* tabs - then there at least needs to be some predefined ones - if module.tabs is None or len(module.tabs) == 0: - module.tabs = [{"type": "courseware"}, - {"type": "course_info", "name": "Course Info"}, - {"type": "discussion", "name": "Discussion"}, - {"type": "wiki", "name": "Wiki"}] # note, add 'progress' when we can support it on Edge - - import_module_from_xml(modulestore, static_content_store, course_data_path, module, target_location_namespace, verbose=verbose) From 431eb8f4e7896d0aac32fc63bb1aba519def304c Mon Sep 17 00:00:00 2001 From: Adam Palay Date: Mon, 12 Aug 2013 14:03:27 -0400 Subject: [PATCH 010/125] remove 'preview' from urls in enrollment emails sent while previewing a course --- lms/djangoapps/instructor/views/legacy.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/lms/djangoapps/instructor/views/legacy.py b/lms/djangoapps/instructor/views/legacy.py index 6a02b5be7d..42f399c143 100644 --- a/lms/djangoapps/instructor/views/legacy.py +++ b/lms/djangoapps/instructor/views/legacy.py @@ -1119,13 +1119,14 @@ def _do_enroll_students(course, course_id, students, overload=False, auto_enroll ceaset.delete() if email_students: - registration_url = 'https://' + settings.SITE_NAME + reverse('student.views.register_user') + stripped_site_name = _remove_preview(settings.SITE_NAME) + registration_url = 'https://' + stripped_site_name + reverse('student.views.register_user') #Composition of email - d = {'site_name': settings.SITE_NAME, + d = {'site_name': stripped_site_name, 'registration_url': registration_url, 'course_id': course_id, 'auto_enroll': auto_enroll, - 'course_url': 'https://' + settings.SITE_NAME + '/courses/' + course_id, + 'course_url': 'https://' + stripped_site_name + '/courses/' + course_id, } for student in new_students: @@ -1209,9 +1210,10 @@ def _do_unenroll_students(course_id, students, email_students=False): old_students, _ = get_and_clean_student_list(students) status = dict([x, 'unprocessed'] for x in old_students) + stripped_site_name = _remove_preview(settings.SITE_NAME) if email_students: #Composition of email - d = {'site_name': settings.SITE_NAME, + d = {'site_name': stripped_site_name, 'course_id': course_id} for student in old_students: @@ -1301,6 +1303,12 @@ def send_mail_to_student(student, param_dict): return False +def _remove_preview(site_name): + if site_name[:8] == "preview.": + return site_name[8:] + return site_name + + def get_and_clean_student_list(students): """ Separate out individual student email from the comma, or space separated string. From 98a47857b3f95749b55092b0da9336320613e3ff Mon Sep 17 00:00:00 2001 From: cahrens Date: Mon, 12 Aug 2013 13:38:31 -0400 Subject: [PATCH 011/125] Allow version ID to appear after course ID. cleanup --- .../xmodule/xmodule/modulestore/locator.py | 38 +++++++------ .../xmodule/xmodule/modulestore/parsers.py | 21 +++++-- .../modulestore/tests/test_locators.py | 55 +++++++++++++++++-- 3 files changed, 86 insertions(+), 28 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/locator.py b/common/lib/xmodule/xmodule/modulestore/locator.py index 3e20f3e1b4..bdc9e61fce 100644 --- a/common/lib/xmodule/xmodule/modulestore/locator.py +++ b/common/lib/xmodule/xmodule/modulestore/locator.py @@ -92,6 +92,7 @@ class CourseLocator(Locator): CourseLocator(url='edx://version/519665f6223ebd6980884f2b') CourseLocator(url='edx://mit.eecs.6002x') CourseLocator(url='edx://mit.eecs.6002x/branch/published') + CourseLocator(url='edx://mit.eecs.6002x/branch/published/version/519665f6223ebd6980884f2b') Should have at lease a specific course_id (id for the course as if it were a project w/ versions) with optional 'branch', @@ -220,21 +221,18 @@ class CourseLocator(Locator): def init_from_url(self, url): """ url must be a string beginning with 'edx://' and containing - either a valid version_guid or course_id (with optional branch) - If a block ('/block/HW3') is present, it is ignored. + either a valid version_guid or course_id (with optional branch), or both. """ if isinstance(url, Locator): url = url.url() - assert isinstance(url, basestring), \ - '%s is not an instance of basestring' % url + assert isinstance(url, basestring), '%s is not an instance of basestring' % url parse = parse_url(url) assert parse, 'Could not parse "%s" as a url' % url - if 'version_guid' in parse: - new_guid = parse['version_guid'] - self.set_version_guid(self.as_object_id(new_guid)) - else: - self.set_course_id(parse['id']) - self.set_branch(parse['branch']) + self._set_value( + parse, 'version_guid', lambda (new_guid): self.set_version_guid(self.as_object_id(new_guid)) + ) + self._set_value(parse, 'id', lambda (new_id): self.set_course_id(new_id)) + self._set_value(parse, 'branch', lambda (new_branch): self.set_branch(new_branch)) def init_from_version_guid(self, version_guid): """ @@ -292,6 +290,16 @@ class CourseLocator(Locator): """ return self.course_id + def _set_value(self, parse, key, setter): + """ + Helper method that gets a value out of the dict returned by parse, + and then sets the corresponding bit of information in this locator + (via the supplied lambda 'setter'), unless the value is None. + """ + value = parse.get(key, None) + if value: + setter(value) + class BlockUsageLocator(CourseLocator): """ @@ -387,9 +395,7 @@ class BlockUsageLocator(CourseLocator): url = url.url() parse = parse_url(url) assert parse, 'Could not parse "%s" as a url' % url - block = parse.get('block', None) - if block: - self.set_usage_id(block) + self._set_value(parse, 'block', lambda(new_block): self.set_usage_id(new_block)) def init_block_ref_from_course_id(self, course_id): if isinstance(course_id, CourseLocator): @@ -397,9 +403,7 @@ class BlockUsageLocator(CourseLocator): assert course_id, "%s does not have a valid course_id" parse = parse_course_id(course_id) assert parse, 'Could not parse "%s" as a course_id' % course_id - block = parse.get('block', None) - if block: - self.set_usage_id(block) + self._set_value(parse, 'block', lambda(new_block): self.set_usage_id(new_block)) def __unicode__(self): """ @@ -415,7 +419,7 @@ class BlockUsageLocator(CourseLocator): class DescriptionLocator(Locator): """ - Container for how to locate a description + Container for how to locate a description (the course-independent content). """ def __init__(self, definition_id): diff --git a/common/lib/xmodule/xmodule/modulestore/parsers.py b/common/lib/xmodule/xmodule/modulestore/parsers.py index efdf1c9e18..9308894b86 100644 --- a/common/lib/xmodule/xmodule/modulestore/parsers.py +++ b/common/lib/xmodule/xmodule/modulestore/parsers.py @@ -4,7 +4,9 @@ import re BRANCH_PREFIX = "/branch/" # Prefix for the block portion of a locator URL BLOCK_PREFIX = "/block/" -# Prefix for when a course URL begins with a version ID +# Prefix for the version portion of a locator URL, when it is preceded by a course ID +VERSION_PREFIX = "/version/" +# Prefix for version when it begins the URL (no course ID). URL_VERSION_PREFIX = 'version/' URL_RE = re.compile(r'^edx://(.+)$', re.IGNORECASE) @@ -19,15 +21,16 @@ def parse_url(string): 'edx://version/0123FFFF' 'edx://edu.mit.eecs.6002x' 'edx://edu.mit.eecs.6002x/branch/published' + 'edx://edu.mit.eecs.6002x/branch/published/version/519665f6223ebd6980884f2b/block/HW3' 'edx://edu.mit.eecs.6002x/branch/published/block/HW3' This returns None if string cannot be parsed. - If it can be parsed as a version_guid, returns a dict + If it can be parsed as a version_guid with no preceding course_id, returns a dict with key 'version_guid' and the value, If it can be parsed as a course_id, returns a dict - with keys 'id' and 'branch' (value of 'branch' may be None), + with key 'id' and optional keys 'branch' and 'version_guid'. """ match = URL_RE.match(string) @@ -61,6 +64,7 @@ def parse_block_ref(string): GUID_RE = re.compile(r'^(?P[A-F0-9]+)(' + BLOCK_PREFIX + '(?P\w+))?$', re.IGNORECASE) + def parse_guid(string): """ A version_guid is a string of hex digits (0-F). @@ -75,7 +79,12 @@ def parse_guid(string): return None -COURSE_ID_RE = re.compile(r'^(?P(\w+)(\.\w+\w*)*)('+ BRANCH_PREFIX + '(?P\w+))?(' + BLOCK_PREFIX + '(?P\w+))?$', re.IGNORECASE) +COURSE_ID_RE = re.compile( + r'^(?P(\w+)(\.\w+\w*)*)(' + + BRANCH_PREFIX + '(?P\w+))?(' + + VERSION_PREFIX + '(?P[A-F0-9]+))?(' + + BLOCK_PREFIX + '(?P\w+))?$', re.IGNORECASE +) def parse_course_id(string): @@ -83,6 +92,7 @@ def parse_course_id(string): A course_id has a main id component. There may also be an optional branch (/branch/published or /branch/draft). + There may also be an optional version (/version/519665f6223ebd6980884f2b). There may also be an optional block (/block/HW3 or /block/Quiz2). Examples of valid course_ids: @@ -91,11 +101,12 @@ def parse_course_id(string): 'edu.mit.eecs.6002x/branch/published' 'edu.mit.eecs.6002x/block/HW3' 'edu.mit.eecs.6002x/branch/published/block/HW3' + 'edu.mit.eecs.6002x/branch/published/version/519665f6223ebd6980884f2b/block/HW3' Syntax: - course_id = main_id [/branch/ branch] [/block/ block] + course_id = main_id [/branch/ branch] [/version/ version ] [/block/ block] main_id = name [. name]* diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_locators.py b/common/lib/xmodule/xmodule/modulestore/tests/test_locators.py index 0f39a4c66f..654de26db4 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_locators.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_locators.py @@ -5,11 +5,14 @@ from unittest import TestCase from bson.objectid import ObjectId from xmodule.modulestore.locator import Locator, CourseLocator, BlockUsageLocator, DescriptionLocator -from xmodule.modulestore.parsers import BRANCH_PREFIX, BLOCK_PREFIX, URL_VERSION_PREFIX +from xmodule.modulestore.parsers import BRANCH_PREFIX, BLOCK_PREFIX, VERSION_PREFIX, URL_VERSION_PREFIX from xmodule.modulestore.exceptions import InsufficientSpecificationError, OverSpecificationError class LocatorTest(TestCase): + """ + Tests for subclasses of Locator. + """ def test_cant_instantiate_abstract_class(self): self.assertRaises(TypeError, Locator) @@ -64,7 +67,7 @@ class LocatorTest(TestCase): self.check_course_locn_fields(testobj_2, 'version_guid', version_guid=test_id_2) self.assertEqual(str(testobj_2.version_guid), test_id_2_loc) self.assertEqual(str(testobj_2), URL_VERSION_PREFIX + test_id_2_loc) - self.assertEqual(testobj_2.url(), 'edx://'+ URL_VERSION_PREFIX + test_id_2_loc) + self.assertEqual(testobj_2.url(), 'edx://' + URL_VERSION_PREFIX + test_id_2_loc) def test_course_constructor_bad_course_id(self): """ @@ -83,8 +86,8 @@ class LocatorTest(TestCase): 'mit.ee()cs', BRANCH_PREFIX + 'this', 'mit.eecs' + BRANCH_PREFIX, - 'mit.eecs' + BRANCH_PREFIX + 'this' + BRANCH_PREFIX +'that', - 'mit.eecs' + BRANCH_PREFIX + 'this' + BRANCH_PREFIX , + 'mit.eecs' + BRANCH_PREFIX + 'this' + BRANCH_PREFIX + 'that', + 'mit.eecs' + BRANCH_PREFIX + 'this' + BRANCH_PREFIX, 'mit.eecs' + BRANCH_PREFIX + 'this ', 'mit.eecs' + BRANCH_PREFIX + 'th%is ', ): @@ -124,6 +127,21 @@ class LocatorTest(TestCase): version_guid=ObjectId(test_id_loc) ) + def test_course_constructor_url_course_id_and_version_guid(self): + test_id_loc = '519665f6223ebd6980884f2b' + testobj = CourseLocator(url='edx://mit.eecs.6002x' + VERSION_PREFIX + test_id_loc) + self.check_course_locn_fields(testobj, 'error parsing url with both course ID and version GUID', + course_id='mit.eecs.6002x', + version_guid=ObjectId(test_id_loc)) + + def test_course_constructor_url_course_id_branch_and_version_guid(self): + test_id_loc = '519665f6223ebd6980884f2b' + testobj = CourseLocator(url='edx://mit.eecs.6002x' + BRANCH_PREFIX + 'draft' + VERSION_PREFIX + test_id_loc) + self.check_course_locn_fields(testobj, 'error parsing url with both course ID branch, and version GUID', + course_id='mit.eecs.6002x', + branch='draft', + version_guid=ObjectId(test_id_loc)) + def test_course_constructor_course_id_no_branch(self): testurn = 'mit.eecs.6002x' testobj = CourseLocator(course_id=testurn) @@ -191,17 +209,42 @@ class LocatorTest(TestCase): self.assertEqual(str(testobj), testurn) self.assertEqual(testobj.url(), 'edx://' + testurn) + def test_block_constructor_url_version_prefix(self): + test_id_loc = '519665f6223ebd6980884f2b' + testobj = BlockUsageLocator( + url='edx://mit.eecs.6002x' + VERSION_PREFIX + test_id_loc + BLOCK_PREFIX + 'lab2' + ) + self.check_block_locn_fields( + testobj, 'error parsing URL with version and block', + course_id='mit.eecs.6002x', + block='lab2', + version_guid=ObjectId(test_id_loc) + ) + + def test_block_constructor_url_kitchen_sink(self): + test_id_loc = '519665f6223ebd6980884f2b' + testobj = BlockUsageLocator( + url='edx://mit.eecs.6002x' + BRANCH_PREFIX + 'draft' + VERSION_PREFIX + test_id_loc + BLOCK_PREFIX + 'lab2' + ) + self.check_block_locn_fields( + testobj, 'error parsing URL with branch, version, and block', + course_id='mit.eecs.6002x', + branch='draft', + block='lab2', + version_guid=ObjectId(test_id_loc) + ) + def test_repr(self): testurn = 'mit.eecs.6002x' + BRANCH_PREFIX + 'published' + BLOCK_PREFIX + 'HW3' testobj = BlockUsageLocator(course_id=testurn) self.assertEqual('BlockUsageLocator("mit.eecs.6002x/branch/published/block/HW3")', repr(testobj)) def test_description_locator_url(self): - definition_locator=DescriptionLocator("chapter12345_2") + definition_locator = DescriptionLocator("chapter12345_2") self.assertEqual('edx://' + URL_VERSION_PREFIX + 'chapter12345_2', definition_locator.url()) def test_description_locator_version(self): - definition_locator=DescriptionLocator("chapter12345_2") + definition_locator = DescriptionLocator("chapter12345_2") self.assertEqual("chapter12345_2", definition_locator.version()) # ------------------------------------------------------------------ From 25abce865f2ad4223508bc149f01770d04c27432 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Mon, 12 Aug 2013 14:39:59 -0400 Subject: [PATCH 012/125] add changelog information on two features being deployed --- CHANGELOG.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 897ea3ae3a..8d04c44d88 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -222,6 +222,10 @@ XModules: Added "randomize" XModule to list of XModule types. XModules: Show errors with full descriptors. +Studio: Add feedback to end user if there is a problem exporting a course + +Studio: Improve link re-writing on imports into a different course-id + XQueue: Fixed (hopefully) worker crash when the connection to RabbitMQ is dropped suddenly. From 57d9ea3f654e072ed35787f5baf517ed2fd0a351 Mon Sep 17 00:00:00 2001 From: Miles Steele Date: Mon, 12 Aug 2013 15:03:38 -0400 Subject: [PATCH 013/125] add beta dash to changelog --- CHANGELOG.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 8d04c44d88..b900eeb71e 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -5,6 +5,10 @@ These are notable changes in edx-platform. This is a rolling list of changes, in roughly chronological order, most recent first. Add your entries at or near the top. Include a label indicating the component affected. +LMS: Enable beta instructor dashboard. The beta dashboard is a rearchitecture +of the existing instructor dashboard and is available by clicking a link at +the top right of the existing dashboard. + Studio: Email will be sent to admin address when a user requests course creator privileges for Studio (edge only). From 57e141827437a6122d9b70d0edc4c6f0ade7aed0 Mon Sep 17 00:00:00 2001 From: Peter Baratta Date: Mon, 12 Aug 2013 15:37:18 -0400 Subject: [PATCH 014/125] Add an entry to the CHANGELOG for #512 --- CHANGELOG.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index b900eeb71e..29f608ef9c 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -15,6 +15,10 @@ privileges for Studio (edge only). Studio: Studio course authors (both instructors and staff) will be auto-enrolled for their courses so that "View Live" works. +Common: Add a new input type ```` for Formula/Numerical +Responses. It periodically makes AJAX calls to preview and validate the +student's input. + Common: Added ratelimiting to our authentication backend. Common: Add additional logging to cover login attempts and logouts. From 13d54c0dd7b43718d6dfad7cd53c86e1fceeced7 Mon Sep 17 00:00:00 2001 From: Adam Palay Date: Mon, 12 Aug 2013 10:08:02 -0400 Subject: [PATCH 015/125] fix password reset templates --- .../registration/password_reset_complete.html | 9 ++-- .../registration/password_reset_confirm.html | 41 +++++++++---------- 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/lms/templates/registration/password_reset_complete.html b/lms/templates/registration/password_reset_complete.html index 3f301102b5..6ccb75ed26 100644 --- a/lms/templates/registration/password_reset_complete.html +++ b/lms/templates/registration/password_reset_complete.html @@ -1,4 +1,3 @@ -<%! from django.utils.translation import ugettext as _ %> {% load i18n %} {% load compressed %} {% load staticfiles %} @@ -6,7 +5,7 @@ - ${_("Your Password Reset is Complete")} + {% trans "Your Password Reset is Complete" %} {% compressed_css 'application' %} @@ -54,13 +53,15 @@
-

${_("Your Password Reset is Complete")}

+

{% trans "Your Password Reset is Complete" %}

{% block content %}
-

${_('Your password has been set. You may go ahead and {link_start}log in{link_end} now.').format(link_start='', link_end='')}

+ {% blocktrans with link_start='' link_end='' %} + Your password has been set. You may go ahead and {{ link_start }}log in{{ link_end }} now. + {% endblocktrans %}
{% endblock %}
diff --git a/lms/templates/registration/password_reset_confirm.html b/lms/templates/registration/password_reset_confirm.html index 6a568545d1..afbaa22683 100644 --- a/lms/templates/registration/password_reset_confirm.html +++ b/lms/templates/registration/password_reset_confirm.html @@ -1,11 +1,11 @@ -<%! from django.utils.translation import ugettext as _ %> +{% load i18n %} {% load compressed %} {% load staticfiles %} - ${_("Reset Your {platform_name} Password").format(platform_name=settings.PLATFORM_NAME)} +{% trans "Reset Your edX Password" %} {% compressed_css 'application' %} @@ -53,78 +53,77 @@
-

${_("Reset Your {platform_name} Password").format(platform_name=settings.PLATFORM_NAME)}

+

{% trans "Reset Your edX Password" %}

{% if validlink %}
-

${_("Password Reset Form")}

+

{% trans "Password Reset Form" %}

{% csrf_token %}

- ${_('Please enter your new password twice so we can verify you typed it in correctly.
' - 'Required fields are noted by bold text and an asterisk (*).')} + {% trans 'Please enter your new password twice so we can verify you typed it in correctly.
Required fields are noted by bold text and an asterisk (*).' %}

- ${_("Required Information")} + {% trans "Required Information" %}
  1. - +
  2. - +
- +
{% else %}
-

${_("Your Password Reset Was Unsuccessful")}

+

{% trans "Your Password Reset Was Unsuccessful" %}

-

${_('The password reset link was invalid, possibly because the link has already been used. Please return to the login page and start the password reset process again.')}

+

{% trans 'The password reset link was invalid, possibly because the link has already been used. Please return to the login page and start the password reset process again.' %}

{% endif %}
From c611470e97edab910ae57eee24acde0070ff460d Mon Sep 17 00:00:00 2001 From: Peter Fogg Date: Mon, 12 Aug 2013 16:28:04 -0400 Subject: [PATCH 016/125] Correct non-unique course validation; code cleanup; better error style. --- cms/static/js/base.js | 98 +++++++++++++--------------- cms/static/sass/elements/_forms.scss | 8 +++ cms/templates/index.html | 4 ++ 3 files changed, 58 insertions(+), 52 deletions(-) diff --git a/cms/static/js/base.js b/cms/static/js/base.js index 72ef5991a3..b68b82dfa6 100644 --- a/cms/static/js/base.js +++ b/cms/static/js/base.js @@ -615,6 +615,37 @@ function addNewCourse(e) { $body.bind('keyup', { $cancelButton: $cancelButton }, checkForCancel); + + // Check that a course (org, number, run) doesn't use any special characters + var validateCourseItemEncoding = function(item) { + var required = validateRequiredField(item); + if(required) { + return required; + } + if(item !== encodeURIComponent(item)) { + return gettext('Please do not use any spaces or special characters in this field.'); + } + return ''; + } + + // Ensure that all items are less than 80 characters. + var validateTotalCourseItemsLength = function() { + var totalLength = _.reduce( + ['.new-course-name', '.new-course-org', '.new-course-number', '.new-course-run'], + function(sum, ele) { + return sum + $(ele).val().length; + }, 0 + ); + if(totalLength > 80) { + $('.wrap-error').addClass('is-shown'); + $('#course_creation_error').html('

' + gettext('Course fields must have a combined length of no more than 80 characters.') + '

'); + $('.new-course-save').addClass('is-disabled'); + } + else { + $('.wrap-error').removeClass('is-shown'); + } + } + // Handle validation asynchronously _.each( ['.new-course-org', '.new-course-number', '.new-course-run'], @@ -635,21 +666,25 @@ function addNewCourse(e) { ); var $name = $('.new-course-name'); $name.on('keyup', function() { - var error = validateCourseName($name.val()); + var error = validateRequiredField($name.val()); setNewCourseFieldInErr($name.parent('li'), error); validateTotalCourseItemsLength(); }); } +function validateRequiredField(msg) { + return msg.length === 0 ? gettext('Required field.') : ''; +} + function setNewCourseFieldInErr(el, msg) { - el.children('.tip-error').remove(); if(msg) { el.addClass('error'); - el.append('' + msg + ''); + el.children('span.tip-error').addClass('is-showing').removeClass('is-hiding').text(msg); $('.new-course-save').addClass('is-disabled'); } else { el.removeClass('error'); + el.children('span.tip-error').addClass('is-hiding').removeClass('is-showing'); // One "error" div is always present, but hidden or shown if($('.error').length === 1) { $('.new-course-save').removeClass('is-disabled'); @@ -657,42 +692,6 @@ function setNewCourseFieldInErr(el, msg) { } }; -function validateCourseName(name) { - if(name.length === 0) { - return gettext('Required field.'); - } - return ''; -} - -// Check that a course (org, number, run) doesn't use any special characters -function validateCourseItemEncoding(item) { - if(item === '') { - return gettext('Required field.'); - } - if(item !== encodeURIComponent(item)) { - return gettext('Please do not use any spaces or special characters in this field.'); - } - return ''; -} - -// Ensure that all items are less than 80 characters. -function validateTotalCourseItemsLength() { - var totalLength = _.reduce( - ['.new-course-name', '.new-course-org', '.new-course-number', '.new-course-run'], - function(sum, ele) { - return sum + $(ele).val().length; - }, 0 - ); - if(totalLength > 80) { - $('.wrap-error').addClass('is-shown'); - $('#course_creation_error').html('

' + gettext('Course fields must have a combined length of no more than 80 characters.') + '

'); - $('.new-course-save').addClass('is-disabled'); - } - else { - $('.wrap-error').removeClass('is-shown'); - } -} - function saveNewCourse(e) { e.preventDefault(); @@ -701,23 +700,23 @@ function saveNewCourse(e) { ['.new-course-name', '.new-course-org', '.new-course-number', '.new-course-run'], function(acc, ele) { var $ele = $(ele); - var error = $ele.val().length === 0 ? gettext('Required field.') : ''; + var error = validateRequiredField($ele.val()); setNewCourseFieldInErr($ele.parent('li'), error); return error ? true : acc; }, false ); + if(errors) { + return; + } + var $newCourseForm = $(this).closest('#create-course-form'); var display_name = $newCourseForm.find('.new-course-name').val(); var org = $newCourseForm.find('.new-course-org').val(); var number = $newCourseForm.find('.new-course-number').val(); var run = $newCourseForm.find('.new-course-run').val(); - if(errors) { - return; - } - analytics.track('Created a Course', { 'org': org, 'number': number, @@ -735,14 +734,9 @@ function saveNewCourse(e) { if (data.id !== undefined) { window.location = '/' + data.id.replace(/.*:\/\//, ''); } else if (data.ErrMsg !== undefined) { - var orgErrMsg = (data.OrgErrMsg !== undefined) ? data.OrgErrMsg : null; - if(orgErrMsg) { - setNewCourseFieldInErr($('.new-course-org').parent('li'), orgErrMsg); - } - var courseErrMsg = (data.CourseErrMsg !== undefined) ? data.CourseErrMsg : null; - if(courseErrMsg) { - setNewCourseFieldInErr($('.new-course-number').parent('li'), orgErrMsg); - } + $('.wrap-error').addClass('is-shown'); + $('#course_creation_error').html('

' + data.ErrMsg + '

'); + $('.new-course-save').addClass('is-disabled'); } } ); diff --git a/cms/static/sass/elements/_forms.scss b/cms/static/sass/elements/_forms.scss index 0d2dec68a0..69bf3a6566 100644 --- a/cms/static/sass/elements/_forms.scss +++ b/cms/static/sass/elements/_forms.scss @@ -225,6 +225,14 @@ form[class^="create-"] { color: $red; } + .is-showing { + @extend .anim-fadeIn; + } + + .is-hiding { + @extend .anim-fadeOut; + } + .tip-error { display: block; color: $red; diff --git a/cms/templates/index.html b/cms/templates/index.html index 5a88054563..e5cf2fa54a 100644 --- a/cms/templates/index.html +++ b/cms/templates/index.html @@ -99,23 +99,27 @@ ${_("The public display name for your course.")} +
  • ${_("The name of the organization sponsoring the course")} - ${_("Note: No spaces or special characters are allowed. This cannot be changed.")} +
  • ${_("The unique number that identifies your course within your organization")} - ${_("Note: No spaces or special characters are allowed. This cannot be changed.")} +
  • ${_("The term in which your course will run")} - ${_("Note: No spaces or special characters are allowed. This cannot be changed.")} +
    • From 18a979bb4d02fd4b5d20f620775fa1aaea69a2ee Mon Sep 17 00:00:00 2001 From: Adam Palay Date: Mon, 12 Aug 2013 15:50:34 -0400 Subject: [PATCH 017/125] put wiki templates back into Django templating --- lms/templates/wiki/includes/cheatsheet.html | 53 +++++++++++-------- .../wiki/includes/editor_widget.html | 7 ++- lms/templates/wiki/preview_inline.html | 5 +- 3 files changed, 37 insertions(+), 28 deletions(-) diff --git a/lms/templates/wiki/includes/cheatsheet.html b/lms/templates/wiki/includes/cheatsheet.html index f8507d2011..7ff7ae009c 100644 --- a/lms/templates/wiki/includes/cheatsheet.html +++ b/lms/templates/wiki/includes/cheatsheet.html @@ -1,21 +1,24 @@ -<%! from django.utils.translation import ugettext as _ %> +{% load i18n %}