diff --git a/CHANGELOG.rst b/CHANGELOG.rst index ce9c08221e..ba64552973 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -13,6 +13,20 @@ instance, Boolean, Integer, String), but also allow them to hold either the typed value, or a String that can be converted to their typed value. For example, an Integer can contain 3 or '3'. This changed an update to the xblock library. +LMS: Courses whose id matches a regex in the COURSES_WITH_UNSAFE_CODE Django +setting now run entirely outside the Python sandbox. + +Blades: Video Alpha bug fix for speed changing to 1.0 in Firefox. + +Blades: Additional event tracking added to Video Alpha: fullscreen switch, show/hide +captions. + +CMS: Allow editors to delete uploaded files/assets + +XModules: `XModuleDescriptor.__init__` and `XModule.__init__` dropped the +`location` parameter (and added it as a field), and renamed `system` to `runtime`, +to accord more closely to `XBlock.__init__` + LMS: Some errors handling Non-ASCII data in XML courses have been fixed. LMS: Add page-load tracking using segment-io (if SEGMENT_IO_LMS_KEY and @@ -35,7 +49,7 @@ student. Blades: Staff debug info is now accessible for Graphical Slider Tool problems. Blades: For Video Alpha the events ready, play, pause, seek, and speed change -are logged on the server (in the logs). +are logged on the server (in the logs). Common: Developers can now have private Django settings files. @@ -56,7 +70,7 @@ Common: The "duplicate email" error message is more informative. Studio: Component metadata settings editor. -Studio: Autoplay is disabled (only in Studio). +Studio: Autoplay for Video Alpha is disabled (only in Studio). Studio: Single-click creation for video and discussion components. diff --git a/cms/CHANGELOG.md b/cms/CHANGELOG.md deleted file mode 100644 index d21d08d23c..0000000000 --- a/cms/CHANGELOG.md +++ /dev/null @@ -1,21 +0,0 @@ -Instructions -============ -For each pull request, add one or more lines to the bottom of the change list. When -code is released to production, change the `Upcoming` entry to todays date, and add -a new block at the bottom of the file. - - Upcoming - -------- - -Change log entries should be targeted at end users. A good place to start is the -user story that instigated the pull request. - - -Changes -======= - -Upcoming --------- -* Fix: Deleting last component in a unit does not work -* Fix: Unit name is editable when a unit is public -* Fix: Visual feedback inconsistent when saving a unit name change diff --git a/cms/djangoapps/contentstore/features/advanced-settings.py b/cms/djangoapps/contentstore/features/advanced-settings.py index 049103db27..4995f3505d 100644 --- a/cms/djangoapps/contentstore/features/advanced-settings.py +++ b/cms/djangoapps/contentstore/features/advanced-settings.py @@ -3,11 +3,7 @@ from lettuce import world, step from nose.tools import assert_false, assert_equal, assert_regexp_matches - -""" -http://selenium.googlecode.com/svn/trunk/docs/api/py/webdriver/selenium.webdriver.common.keys.html -""" -from selenium.webdriver.common.keys import Keys +from common import type_in_codemirror KEY_CSS = '.key input.policy-key' VALUE_CSS = 'textarea.json' @@ -37,13 +33,7 @@ def press_the_notification_button(step, name): @step(u'I edit the value of a policy key$') def edit_the_value_of_a_policy_key(step): - """ - It is hard to figure out how to get into the CodeMirror - area, so cheat and do it from the policy key field :) - """ - world.css_find(".CodeMirror")[get_index_of(DISPLAY_NAME_KEY)].click() - g = world.css_find("div.CodeMirror.CodeMirror-focused > div > textarea") - g._element.send_keys(Keys.ARROW_LEFT, ' ', 'X') + type_in_codemirror(get_index_of(DISPLAY_NAME_KEY), 'X') @step(u'I edit the value of a policy key and save$') @@ -132,13 +122,5 @@ def change_display_name_value(step, new_value): def change_value(step, key, new_value): - index = get_index_of(key) - world.css_find(".CodeMirror")[index].click() - g = world.css_find("div.CodeMirror.CodeMirror-focused > div > textarea") - current_value = world.css_find(VALUE_CSS)[index].value - g._element.send_keys(Keys.CONTROL + Keys.END) - for count in range(len(current_value)): - g._element.send_keys(Keys.END, Keys.BACK_SPACE) - # Must delete "" before typing the JSON value - g._element.send_keys(Keys.END, Keys.BACK_SPACE, Keys.BACK_SPACE, new_value) + type_in_codemirror(get_index_of(key), new_value) press_the_notification_button(step, "Save") diff --git a/cms/djangoapps/contentstore/features/common.py b/cms/djangoapps/contentstore/features/common.py index fec1b90040..e7187e83d8 100644 --- a/cms/djangoapps/contentstore/features/common.py +++ b/cms/djangoapps/contentstore/features/common.py @@ -179,3 +179,14 @@ def shows_captions(step, show_captions): assert world.css_find('.video')[0].has_class('closed') else: assert world.is_css_not_present('.video.closed') + + +def type_in_codemirror(index, text): + world.css_click(".CodeMirror", index=index) + g = world.css_find("div.CodeMirror.CodeMirror-focused > div > textarea") + if world.is_mac(): + g._element.send_keys(Keys.COMMAND + 'a') + else: + g._element.send_keys(Keys.CONTROL + 'a') + g._element.send_keys(Keys.DELETE) + g._element.send_keys(text) diff --git a/cms/djangoapps/contentstore/features/problem-editor.feature b/cms/djangoapps/contentstore/features/problem-editor.feature index bde350d8a3..cc1d766d2e 100644 --- a/cms/djangoapps/contentstore/features/problem-editor.feature +++ b/cms/djangoapps/contentstore/features/problem-editor.feature @@ -3,65 +3,71 @@ Feature: Problem Editor Scenario: User can view metadata Given I have created a Blank Common Problem - And I edit and select Settings + When I edit and select Settings Then I see five alphabetized settings and their expected values And Edit High Level Source is not visible Scenario: User can modify String values Given I have created a Blank Common Problem - And I edit and select Settings + When I edit and select Settings Then I can modify the display name And my display name change is persisted on save Scenario: User can specify special characters in String values Given I have created a Blank Common Problem - And I edit and select Settings + When I edit and select Settings Then I can specify special characters in the display name And my special characters and persisted on save Scenario: User can revert display name to unset Given I have created a Blank Common Problem - And I edit and select Settings + When I edit and select Settings Then I can revert the display name to unset And my display name is unset on save Scenario: User can select values in a Select Given I have created a Blank Common Problem - And I edit and select Settings + When I edit and select Settings Then I can select Per Student for Randomization And my change to randomization is persisted And I can revert to the default value for randomization Scenario: User can modify float input values Given I have created a Blank Common Problem - And I edit and select Settings + When I edit and select Settings Then I can set the weight to "3.5" And my change to weight is persisted And I can revert to the default value of unset for weight Scenario: User cannot type letters in float number field Given I have created a Blank Common Problem - And I edit and select Settings + When I edit and select Settings Then if I set the weight to "abc", it remains unset Scenario: User cannot type decimal values integer number field Given I have created a Blank Common Problem - And I edit and select Settings + When I edit and select Settings Then if I set the max attempts to "2.34", it displays initially as "234", and is persisted as "234" Scenario: User cannot type out of range values in an integer number field Given I have created a Blank Common Problem - And I edit and select Settings + When I edit and select Settings Then if I set the max attempts to "-3", it displays initially as "-3", and is persisted as "0" Scenario: Settings changes are not saved on Cancel Given I have created a Blank Common Problem - And I edit and select Settings + When I edit and select Settings Then I can set the weight to "3.5" And I can modify the display name Then If I press Cancel my changes are not persisted Scenario: Edit High Level source is available for LaTeX problem Given I have created a LaTeX Problem - And I edit and select Settings + When I edit and select Settings Then Edit High Level Source is visible + + Scenario: High Level source is persisted for LaTeX problem (bug STUD-280) + Given I have created a LaTeX Problem + When I edit and compile the High Level Source + Then my change to the High Level Source is persisted + And when I view the High Level Source I see my changes diff --git a/cms/djangoapps/contentstore/features/problem-editor.py b/cms/djangoapps/contentstore/features/problem-editor.py index 7679128beb..8691a6772e 100644 --- a/cms/djangoapps/contentstore/features/problem-editor.py +++ b/cms/djangoapps/contentstore/features/problem-editor.py @@ -3,6 +3,7 @@ from lettuce import world, step from nose.tools import assert_equal +from common import type_in_codemirror DISPLAY_NAME = "Display Name" MAXIMUM_ATTEMPTS = "Maximum Attempts" @@ -135,12 +136,12 @@ def set_the_max_attempts(step, max_attempts_set, max_attempts_displayed, max_att @step('Edit High Level Source is not visible') def edit_high_level_source_not_visible(step): - verify_high_level_source(step, False) + verify_high_level_source_links(step, False) @step('Edit High Level Source is visible') -def edit_high_level_source_visible(step): - verify_high_level_source(step, True) +def edit_high_level_source_links_visible(step): + verify_high_level_source_links(step, True) @step('If I press Cancel my changes are not persisted') @@ -153,13 +154,33 @@ def cancel_does_not_save_changes(step): @step('I have created a LaTeX Problem') def create_latex_problem(step): world.click_new_component_button(step, '.large-problem-icon') - # Go to advanced tab (waiting for the tab to be visible) - world.css_find('#ui-id-2') + # Go to advanced tab. world.css_click('#ui-id-2') world.click_component_from_menu("i4x://edx/templates/problem/Problem_Written_in_LaTeX", '.xmodule_CapaModule') -def verify_high_level_source(step, visible): +@step('I edit and compile the High Level Source') +def edit_latex_source(step): + open_high_level_source() + type_in_codemirror(1, "hi") + world.css_click('.hls-compile') + + +@step('my change to the High Level Source is persisted') +def high_level_source_persisted(step): + def verify_text(driver): + return world.css_find('.problem').text == 'hi' + + world.wait_for(verify_text) + + +@step('I view the High Level Source I see my changes') +def high_level_source_in_editor(step): + open_high_level_source() + assert_equal('hi', world.css_find('.source-edit-box').value) + + +def verify_high_level_source_links(step, visible): assert_equal(visible, world.is_css_present('.launch-latex-compiler')) world.cancel_component(step) assert_equal(visible, world.is_css_present('.upload-button')) @@ -187,3 +208,8 @@ def verify_unset_display_name(): def set_weight(weight): world.get_setting_entry(PROBLEM_WEIGHT).find_by_css('.setting-input')[0].fill(weight) + + +def open_high_level_source(): + world.css_click('a.edit-button') + world.css_click('.launch-latex-compiler > a') diff --git a/cms/djangoapps/contentstore/management/commands/empty_asset_trashcan.py b/cms/djangoapps/contentstore/management/commands/empty_asset_trashcan.py new file mode 100644 index 0000000000..9af3277a2b --- /dev/null +++ b/cms/djangoapps/contentstore/management/commands/empty_asset_trashcan.py @@ -0,0 +1,25 @@ +from django.core.management.base import BaseCommand, CommandError +from xmodule.course_module import CourseDescriptor +from xmodule.contentstore.utils import empty_asset_trashcan +from xmodule.modulestore.django import modulestore +from .prompt import query_yes_no + + +class Command(BaseCommand): + help = '''Empty the trashcan. Can pass an optional course_id to limit the damage.''' + + def handle(self, *args, **options): + if len(args) != 1 and len(args) != 0: + raise CommandError("empty_asset_trashcan requires one or no arguments: ||") + + locs = [] + + if len(args) == 1: + locs.append(CourseDescriptor.id_to_location(args[0])) + else: + courses = modulestore('direct').get_courses() + for course in courses: + locs.append(course.location) + + if query_yes_no("Emptying trashcan. Confirm?", default="no"): + empty_asset_trashcan(locs) diff --git a/cms/djangoapps/contentstore/management/commands/restore_asset_from_trashcan.py b/cms/djangoapps/contentstore/management/commands/restore_asset_from_trashcan.py new file mode 100644 index 0000000000..6770bfaf44 --- /dev/null +++ b/cms/djangoapps/contentstore/management/commands/restore_asset_from_trashcan.py @@ -0,0 +1,13 @@ +from django.core.management.base import BaseCommand, CommandError +from xmodule.contentstore.utils import restore_asset_from_trashcan + + +class Command(BaseCommand): + help = '''Restore a deleted asset from the trashcan back to it's original course''' + + def handle(self, *args, **options): + if len(args) != 1 and len(args) != 0: + raise CommandError("restore_asset_from_trashcan requires one argument: ") + + restore_asset_from_trashcan(args[0]) + diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index 03449fc22f..9346d2189d 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -28,6 +28,8 @@ from xmodule.templates import update_templates from xmodule.modulestore.xml_exporter import export_to_xml from xmodule.modulestore.xml_importer import import_from_xml, perform_xlint from xmodule.modulestore.inheritance import own_metadata +from xmodule.contentstore.content import StaticContent +from xmodule.contentstore.utils import restore_asset_from_trashcan, empty_asset_trashcan from xmodule.capa_module import CapaDescriptor from xmodule.course_module import CourseDescriptor @@ -35,6 +37,7 @@ from xmodule.seq_module import SequenceDescriptor from xmodule.modulestore.exceptions import ItemNotFoundError from contentstore.views.component import ADVANCED_COMPONENT_TYPES +from xmodule.exceptions import NotFoundError from django_comment_common.utils import are_permissions_roles_seeded from xmodule.exceptions import InvalidVersionError @@ -382,6 +385,159 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): course = module_store.get_item(source_location) self.assertFalse(course.hide_progress_tab) + def test_asset_import(self): + ''' + This test validates that an image asset is imported and a thumbnail was generated for a .gif + ''' + content_store = contentstore() + + module_store = modulestore('direct') + import_from_xml(module_store, 'common/test/data/', ['full'], static_content_store=content_store) + + course_location = CourseDescriptor.id_to_location('edX/full/6.002_Spring_2012') + course = module_store.get_item(course_location) + + self.assertIsNotNone(course) + + # make sure we have some assets in our contentstore + all_assets = content_store.get_all_content_for_course(course_location) + self.assertGreater(len(all_assets), 0) + + # make sure we have some thumbnails in our contentstore + all_thumbnails = content_store.get_all_content_thumbnails_for_course(course_location) + + # + # cdodge: temporarily comment out assertion on thumbnails because many environments + # will not have the jpeg converter installed and this test will fail + # + # + # self.assertGreater(len(all_thumbnails), 0) + + content = None + try: + location = StaticContent.get_location_from_path('/c4x/edX/full/asset/circuits_duality.gif') + content = content_store.find(location) + except NotFoundError: + pass + + self.assertIsNotNone(content) + + # + # cdodge: temporarily comment out assertion on thumbnails because many environments + # will not have the jpeg converter installed and this test will fail + # + # self.assertIsNotNone(content.thumbnail_location) + # + # thumbnail = None + # try: + # thumbnail = content_store.find(content.thumbnail_location) + # except: + # pass + # + # self.assertIsNotNone(thumbnail) + + def test_asset_delete_and_restore(self): + ''' + This test will exercise the soft delete/restore functionality of the assets + ''' + content_store = contentstore() + trash_store = contentstore('trashcan') + module_store = modulestore('direct') + + import_from_xml(module_store, 'common/test/data/', ['full'], static_content_store=content_store) + + # look up original (and thumbnail) in content store, should be there after import + location = StaticContent.get_location_from_path('/c4x/edX/full/asset/circuits_duality.gif') + content = content_store.find(location, throw_on_not_found=False) + thumbnail_location = content.thumbnail_location + self.assertIsNotNone(content) + + # + # cdodge: temporarily comment out assertion on thumbnails because many environments + # will not have the jpeg converter installed and this test will fail + # + # self.assertIsNotNone(thumbnail_location) + + # go through the website to do the delete, since the soft-delete logic is in the view + + url = reverse('remove_asset', kwargs={'org': 'edX', 'course': 'full', 'name': '6.002_Spring_2012'}) + resp = self.client.post(url, {'location': '/c4x/edX/full/asset/circuits_duality.gif'}) + self.assertEqual(resp.status_code, 200) + + asset_location = StaticContent.get_location_from_path('/c4x/edX/full/asset/circuits_duality.gif') + + # now try to find it in store, but they should not be there any longer + content = content_store.find(asset_location, throw_on_not_found=False) + self.assertIsNone(content) + + if thumbnail_location: + thumbnail = content_store.find(thumbnail_location, throw_on_not_found=False) + self.assertIsNone(thumbnail) + + # now try to find it and the thumbnail in trashcan - should be in there + content = trash_store.find(asset_location, throw_on_not_found=False) + self.assertIsNotNone(content) + + if thumbnail_location: + thumbnail = trash_store.find(thumbnail_location, throw_on_not_found=False) + self.assertIsNotNone(thumbnail) + + # let's restore the asset + restore_asset_from_trashcan('/c4x/edX/full/asset/circuits_duality.gif') + + # now try to find it in courseware store, and they should be back after restore + content = content_store.find(asset_location, throw_on_not_found=False) + self.assertIsNotNone(content) + + if thumbnail_location: + thumbnail = content_store.find(thumbnail_location, throw_on_not_found=False) + self.assertIsNotNone(thumbnail) + + def test_empty_trashcan(self): + ''' + This test will exercise the empting of the asset trashcan + ''' + content_store = contentstore() + trash_store = contentstore('trashcan') + module_store = modulestore('direct') + + import_from_xml(module_store, 'common/test/data/', ['full'], static_content_store=content_store) + + course_location = CourseDescriptor.id_to_location('edX/full/6.002_Spring_2012') + + location = StaticContent.get_location_from_path('/c4x/edX/full/asset/circuits_duality.gif') + content = content_store.find(location, throw_on_not_found=False) + self.assertIsNotNone(content) + + # go through the website to do the delete, since the soft-delete logic is in the view + + url = reverse('remove_asset', kwargs={'org': 'edX', 'course': 'full', 'name': '6.002_Spring_2012'}) + resp = self.client.post(url, {'location': '/c4x/edX/full/asset/circuits_duality.gif'}) + self.assertEqual(resp.status_code, 200) + + # make sure there's something in the trashcan + all_assets = trash_store.get_all_content_for_course(course_location) + self.assertGreater(len(all_assets), 0) + + # make sure we have some thumbnails in our trashcan + all_thumbnails = trash_store.get_all_content_thumbnails_for_course(course_location) + # + # cdodge: temporarily comment out assertion on thumbnails because many environments + # will not have the jpeg converter installed and this test will fail + # + # self.assertGreater(len(all_thumbnails), 0) + + # empty the trashcan + empty_asset_trashcan([course_location]) + + # make sure trashcan is empty + all_assets = trash_store.get_all_content_for_course(course_location) + self.assertEqual(len(all_assets), 0) + + + all_thumbnails = trash_store.get_all_content_thumbnails_for_course(course_location) + self.assertEqual(len(all_thumbnails), 0) + def test_clone_course(self): course_data = { diff --git a/cms/djangoapps/contentstore/views/assets.py b/cms/djangoapps/contentstore/views/assets.py index 229788f24d..400013b59b 100644 --- a/cms/djangoapps/contentstore/views/assets.py +++ b/cms/djangoapps/contentstore/views/assets.py @@ -25,6 +25,8 @@ from xmodule.modulestore.django import modulestore from xmodule.modulestore import Location from xmodule.contentstore.content import StaticContent from xmodule.util.date_utils import get_default_time_display +from xmodule.modulestore import InvalidLocationError +from xmodule.exceptions import NotFoundError from ..utils import get_url_reverse from .access import get_location_and_verify_access @@ -78,10 +80,17 @@ def asset_index(request, org, course, name): 'active_tab': 'assets', 'context_course': course_module, 'assets': asset_display, - 'upload_asset_callback_url': upload_asset_callback_url + 'upload_asset_callback_url': upload_asset_callback_url, + 'remove_asset_callback_url': reverse('remove_asset', kwargs={ + 'org': org, + 'course': course, + 'name': name + }) }) +@login_required +@ensure_csrf_cookie def upload_asset(request, org, course, coursename): ''' cdodge: this method allows for POST uploading of files into the course asset library, which will @@ -145,6 +154,57 @@ def upload_asset(request, org, course, coursename): return response +@ensure_csrf_cookie +@login_required +def remove_asset(request, org, course, name): + ''' + This method will perform a 'soft-delete' of an asset, which is basically to copy the asset from + the main GridFS collection and into a Trashcan + ''' + get_location_and_verify_access(request, org, course, name) + + location = request.POST['location'] + + # make sure the location is valid + try: + loc = StaticContent.get_location_from_path(location) + except InvalidLocationError: + # return a 'Bad Request' to browser as we have a malformed Location + response = HttpResponse() + response.status_code = 400 + return response + + # also make sure the item to delete actually exists + try: + content = contentstore().find(loc) + except NotFoundError: + response = HttpResponse() + response.status_code = 404 + return response + + # ok, save the content into the trashcan + contentstore('trashcan').save(content) + + # see if there is a thumbnail as well, if so move that as well + if content.thumbnail_location is not None: + try: + thumbnail_content = contentstore().find(content.thumbnail_location) + contentstore('trashcan').save(thumbnail_content) + # hard delete thumbnail from origin + contentstore().delete(thumbnail_content.get_id()) + # remove from any caching + del_cached_content(thumbnail_content.location) + except: + pass # OK if this is left dangling + + # delete the original + contentstore().delete(content.get_id()) + # remove from cache + del_cached_content(content.location) + + return HttpResponse() + + @ensure_csrf_cookie @login_required def import_course(request, org, course, name): diff --git a/cms/envs/aws.py b/cms/envs/aws.py index 35b15fe6ba..a6d6e2f847 100644 --- a/cms/envs/aws.py +++ b/cms/envs/aws.py @@ -112,8 +112,11 @@ TIME_ZONE = ENV_TOKENS.get('TIME_ZONE', TIME_ZONE) for feature, value in ENV_TOKENS.get('MITX_FEATURES', {}).items(): MITX_FEATURES[feature] = value -# load segment.io key, provide a dummy if it does not exist -SEGMENT_IO_KEY = ENV_TOKENS.get('SEGMENT_IO_KEY', '***REMOVED***') +# If Segment.io key specified, load it and turn on Segment.io if the feature flag is set +# Note that this is the Studio key. There is a separate key for the LMS. +SEGMENT_IO_KEY = AUTH_TOKENS.get('SEGMENT_IO_KEY') +if SEGMENT_IO_KEY: + MITX_FEATURES['SEGMENT_IO'] = ENV_TOKENS.get('SEGMENT_IO', False) LOGGING = get_logger_config(LOG_DIR, logging_env=ENV_TOKENS['LOGGING_ENV'], diff --git a/cms/envs/common.py b/cms/envs/common.py index 5962cec340..8551a56c41 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -32,13 +32,23 @@ from path import path MITX_FEATURES = { 'USE_DJANGO_PIPELINE': True, + 'GITHUB_PUSH': False, + 'ENABLE_DISCUSSION_SERVICE': False, + 'AUTH_USE_MIT_CERTIFICATES': False, - 'STUB_VIDEO_FOR_TESTING': False, # do not display video when running automated acceptance tests - 'STAFF_EMAIL': '', # email address for staff (eg to request course creation) + + # do not display video when running automated acceptance tests + 'STUB_VIDEO_FOR_TESTING': False, + + # email address for staff (eg to request course creation) + 'STAFF_EMAIL': '', + 'STUDIO_NPS_SURVEY': True, - 'SEGMENT_IO': True, + + # Segment.io - must explicitly turn it on for production + 'SEGMENT_IO': False, # Enable URL that shows information about the status of various services 'ENABLE_SERVICE_STATUS': False, @@ -228,7 +238,8 @@ PIPELINE_JS = { ) + ['js/hesitate.js', 'js/base.js', 'js/models/feedback.js', 'js/views/feedback.js', 'js/models/section.js', 'js/views/section.js', - 'js/models/metadata_model.js', 'js/views/metadata_editor_view.js'], + 'js/models/metadata_model.js', 'js/views/metadata_editor_view.js', + 'js/views/assets.js'], 'output_filename': 'js/cms-application.js', 'test_order': 0 }, diff --git a/cms/envs/dev.py b/cms/envs/dev.py index cbe47a1fe1..6327d442d2 100644 --- a/cms/envs/dev.py +++ b/cms/envs/dev.py @@ -43,10 +43,15 @@ CONTENTSTORE = { 'OPTIONS': { 'host': 'localhost', 'db': 'xcontent', + }, + # allow for additional options that can be keyed on a name, e.g. 'trashcan' + 'ADDITIONAL_OPTIONS': { + 'trashcan': { + 'bucket': 'trash_fs' + } } } - DATABASES = { 'default': { 'ENGINE': 'django.db.backends.sqlite3', @@ -163,8 +168,13 @@ MITX_FEATURES['STUDIO_NPS_SURVEY'] = False # Enable URL that shows information about the status of variuous services MITX_FEATURES['ENABLE_SERVICE_STATUS'] = True -# segment-io key for dev -SEGMENT_IO_KEY = 'mty8edrrsg' +############################# SEGMENT-IO ################################## + +# If there's an environment variable set, grab it and turn on Segment.io +# Note that this is the Studio key. There is a separate key for the LMS. +SEGMENT_IO_KEY = os.environ.get('SEGMENT_IO_KEY') +if SEGMENT_IO_KEY: + MITX_FEATURES['SEGMENT_IO'] = True ##################################################################### diff --git a/cms/envs/test.py b/cms/envs/test.py index 1569d0a42d..954a553e10 100644 --- a/cms/envs/test.py +++ b/cms/envs/test.py @@ -70,7 +70,13 @@ CONTENTSTORE = { 'ENGINE': 'xmodule.contentstore.mongo.MongoContentStore', 'OPTIONS': { 'host': 'localhost', - 'db': 'xcontent', + 'db': 'test_xmodule', + }, + # allow for additional options that can be keyed on a name, e.g. 'trashcan' + 'ADDITIONAL_OPTIONS': { + 'trashcan': { + 'bucket': 'trash_fs' + } } } diff --git a/cms/static/coffee/src/views/module_edit.coffee b/cms/static/coffee/src/views/module_edit.coffee index d0a76a6c15..5154591d6f 100644 --- a/cms/static/coffee/src/views/module_edit.coffee +++ b/cms/static/coffee/src/views/module_edit.coffee @@ -44,8 +44,17 @@ class CMS.Views.ModuleEdit extends Backbone.View [@metadataEditor.getDisplayName()]) @$el.find('.component-name').html(title) + customMetadata: -> + # Hack to support metadata fields that aren't part of the metadata editor (ie, LaTeX high level source). + # Walk through the set of elements which have the 'data-metadata_name' attribute and + # build up an object to pass back to the server on the subsequent POST. + # Note that these values will always be sent back on POST, even if they did not actually change. + _metadata = {} + _metadata[$(el).data("metadata-name")] = el.value for el in $('[data-metadata-name]', @$component_editor()) + return _metadata + changedMetadata: -> - return @metadataEditor.getModifiedMetadataValues() + return _.extend(@metadataEditor.getModifiedMetadataValues(), @customMetadata()) cloneTemplate: (parent, template) -> $.post("/clone_item", { diff --git a/cms/static/js/base.js b/cms/static/js/base.js index c626fa1b3f..92a16b8417 100644 --- a/cms/static/js/base.js +++ b/cms/static/js/base.js @@ -32,8 +32,6 @@ $(document).ready(function() { $modal.bind('click', hideModal); $modalCover.bind('click', hideModal); - $('.uploads .upload-button').bind('click', showUploadModal); - $('.upload-modal .close-button').bind('click', hideModal); $body.on('click', '.embeddable-xml-input', function() { $(this).select(); @@ -145,8 +143,6 @@ $(document).ready(function() { $('.edit-section-start-cancel').bind('click', cancelSetSectionScheduleDate); $('.edit-section-start-save').bind('click', saveSetSectionScheduleDate); - $('.upload-modal .choose-file-button').bind('click', showFileSelectionMenu); - $body.on('click', '.section-published-date .edit-button', editSectionPublishDate); $body.on('click', '.section-published-date .schedule-button', editSectionPublishDate); $body.on('click', '.edit-subsection-publish-settings .save-button', saveSetSectionScheduleDate); @@ -398,73 +394,6 @@ function _deleteItem($el) { }); } -function showUploadModal(e) { - e.preventDefault(); - $modal = $('.upload-modal').show(); - $('.file-input').bind('change', startUpload); - $modalCover.show(); -} - -function showFileSelectionMenu(e) { - e.preventDefault(); - $('.file-input').click(); -} - -function startUpload(e) { - var files = $('.file-input').get(0).files; - if (files.length === 0) - return; - - $('.upload-modal h1').html(gettext('Uploading…')); - $('.upload-modal .file-name').html(files[0].name); - $('.upload-modal .file-chooser').ajaxSubmit({ - beforeSend: resetUploadBar, - uploadProgress: showUploadFeedback, - complete: displayFinishedUpload - }); - $('.upload-modal .choose-file-button').hide(); - $('.upload-modal .progress-bar').removeClass('loaded').show(); -} - -function resetUploadBar() { - var percentVal = '0%'; - $('.upload-modal .progress-fill').width(percentVal); - $('.upload-modal .progress-fill').html(percentVal); -} - -function showUploadFeedback(event, position, total, percentComplete) { - var percentVal = percentComplete + '%'; - $('.upload-modal .progress-fill').width(percentVal); - $('.upload-modal .progress-fill').html(percentVal); -} - -function displayFinishedUpload(xhr) { - if (xhr.status = 200) { - markAsLoaded(); - } - - var resp = JSON.parse(xhr.responseText); - $('.upload-modal .embeddable-xml-input').val(xhr.getResponseHeader('asset_url')); - $('.upload-modal .embeddable').show(); - $('.upload-modal .file-name').hide(); - $('.upload-modal .progress-fill').html(resp.msg); - $('.upload-modal .choose-file-button').html(gettext('Load Another File')).show(); - $('.upload-modal .progress-fill').width('100%'); - - // see if this id already exists, if so, then user must have updated an existing piece of content - $("tr[data-id='" + resp.url + "']").remove(); - - var template = $('#new-asset-element').html(); - var html = Mustache.to_html(template, resp); - $('table > tbody').prepend(html); - - analytics.track('Uploaded a File', { - 'course': course_location_analytics, - 'asset_url': resp.url - }); - -} - function markAsLoaded() { $('.upload-modal .copy-button').css('display', 'inline-block'); $('.upload-modal .progress-bar').addClass('loaded'); diff --git a/cms/static/js/models/feedback.js b/cms/static/js/models/feedback.js index 1f1ee57000..d57cffa779 100644 --- a/cms/static/js/models/feedback.js +++ b/cms/static/js/models/feedback.js @@ -42,6 +42,12 @@ CMS.Models.ErrorMessage = CMS.Models.SystemFeedback.extend({ }) }); +CMS.Models.ConfirmAssetDeleteMessage = CMS.Models.SystemFeedback.extend({ + defaults: $.extend({}, CMS.Models.SystemFeedback.prototype.defaults, { + "intent": "warning" + }) +}); + CMS.Models.ConfirmationMessage = CMS.Models.SystemFeedback.extend({ defaults: $.extend({}, CMS.Models.SystemFeedback.prototype.defaults, { "intent": "confirmation" diff --git a/cms/static/js/views/assets.js b/cms/static/js/views/assets.js new file mode 100644 index 0000000000..9eb521dcb6 --- /dev/null +++ b/cms/static/js/views/assets.js @@ -0,0 +1,128 @@ +$(document).ready(function() { + $('.uploads .upload-button').bind('click', showUploadModal); + $('.upload-modal .close-button').bind('click', hideModal); + $('.upload-modal .choose-file-button').bind('click', showFileSelectionMenu); + $('.remove-asset-button').bind('click', removeAsset); +}); + +function removeAsset(e){ + e.preventDefault(); + + var that = this; + var msg = new CMS.Models.ConfirmAssetDeleteMessage({ + title: gettext("Delete File Confirmation"), + message: gettext("Are you sure you wish to delete this item. It cannot be reversed!\n\nAlso any content that links/refers to this item will no longer work (e.g. broken images and/or links)"), + actions: { + primary: { + text: gettext("OK"), + click: function(view) { + // call the back-end to actually remove the asset + $.post(view.model.get('remove_asset_url'), + { 'location': view.model.get('asset_location') }, + function() { + // show the post-commit confirmation + $(".wrapper-alert-confirmation").addClass("is-shown").attr('aria-hidden','false'); + view.model.get('row_to_remove').remove(); + analytics.track('Deleted Asset', { + 'course': course_location_analytics, + 'id': view.model.get('asset_location') + }); + } + ); + view.hide(); + } + }, + secondary: [{ + text: gettext("Cancel"), + click: function(view) { + view.hide(); + } + }] + }, + remove_asset_url: $('.asset-library').data('remove-asset-callback-url'), + asset_location: $(this).closest('tr').data('id'), + row_to_remove: $(this).closest('tr') + }); + + // workaround for now. We can't spawn multiple instances of the Prompt View + // so for now, a bit of hackery to just make sure we have a single instance + // note: confirm_delete_prompt is in asset_index.html + if (confirm_delete_prompt === null) + confirm_delete_prompt = new CMS.Views.Prompt({model: msg}); + else + { + confirm_delete_prompt.model = msg; + confirm_delete_prompt.show(); + } + + return; +} + +function showUploadModal(e) { + e.preventDefault(); + $modal = $('.upload-modal').show(); + $('.file-input').bind('change', startUpload); + $modalCover.show(); +} + +function showFileSelectionMenu(e) { + e.preventDefault(); + $('.file-input').click(); +} + +function startUpload(e) { + var files = $('.file-input').get(0).files; + if (files.length === 0) + return; + + $('.upload-modal h1').html(gettext('Uploading…')); + $('.upload-modal .file-name').html(files[0].name); + $('.upload-modal .file-chooser').ajaxSubmit({ + beforeSend: resetUploadBar, + uploadProgress: showUploadFeedback, + complete: displayFinishedUpload + }); + $('.upload-modal .choose-file-button').hide(); + $('.upload-modal .progress-bar').removeClass('loaded').show(); +} + +function resetUploadBar() { + var percentVal = '0%'; + $('.upload-modal .progress-fill').width(percentVal); + $('.upload-modal .progress-fill').html(percentVal); +} + +function showUploadFeedback(event, position, total, percentComplete) { + var percentVal = percentComplete + '%'; + $('.upload-modal .progress-fill').width(percentVal); + $('.upload-modal .progress-fill').html(percentVal); +} + +function displayFinishedUpload(xhr) { + if (xhr.status == 200) { + markAsLoaded(); + } + + var resp = JSON.parse(xhr.responseText); + $('.upload-modal .embeddable-xml-input').val(xhr.getResponseHeader('asset_url')); + $('.upload-modal .embeddable').show(); + $('.upload-modal .file-name').hide(); + $('.upload-modal .progress-fill').html(resp.msg); + $('.upload-modal .choose-file-button').html(gettext('Load Another File')).show(); + $('.upload-modal .progress-fill').width('100%'); + + // see if this id already exists, if so, then user must have updated an existing piece of content + $("tr[data-id='" + resp.url + "']").remove(); + + var template = $('#new-asset-element').html(); + var html = Mustache.to_html(template, resp); + $('table > tbody').prepend(html); + + // re-bind the listeners to delete it + $('.remove-asset-button').bind('click', removeAsset); + + analytics.track('Uploaded a File', { + 'course': course_location_analytics, + 'asset_url': resp.url + }); +} \ No newline at end of file diff --git a/cms/static/sass/views/_assets.scss b/cms/static/sass/views/_assets.scss index d01dd988ef..d4cff42ee9 100644 --- a/cms/static/sass/views/_assets.scss +++ b/cms/static/sass/views/_assets.scss @@ -76,6 +76,10 @@ body.course.uploads { width: 250px; } + .delete-col { + width: 20px; + } + .embeddable-xml-input { @include box-shadow(none); width: 100%; diff --git a/cms/templates/asset_index.html b/cms/templates/asset_index.html index f03a9012f8..e8dc523ba7 100644 --- a/cms/templates/asset_index.html +++ b/cms/templates/asset_index.html @@ -1,5 +1,6 @@ <%inherit file="base.html" /> <%! from django.core.urlresolvers import reverse %> +<%! from django.utils.translation import ugettext as _ %> <%block name="bodyclass">is-signedin course uploads <%block name="title">Files & Uploads @@ -7,6 +8,12 @@ <%block name="jsextra"> + + + <%block name="content"> @@ -30,6 +37,9 @@ + + + @@ -56,7 +66,7 @@
-
+
@@ -64,6 +74,7 @@ + @@ -86,6 +97,9 @@ + % endfor @@ -129,3 +143,21 @@ + +<%block name="view_alerts"> + +
+
+ + +
+

${_('Your file has been deleted.')}

+
+ + + + ${_('close alert')} + +
+
+ diff --git a/cms/urls.py b/cms/urls.py index e7444de4e9..a9a7f0a68a 100644 --- a/cms/urls.py +++ b/cms/urls.py @@ -35,6 +35,8 @@ urlpatterns = ('', # nopep8 'contentstore.views.preview_dispatch', name='preview_dispatch'), url(r'^(?P[^/]+)/(?P[^/]+)/course/(?P[^/]+)/upload_asset$', 'contentstore.views.upload_asset', name='upload_asset'), + + url(r'^manage_users/(?P.*?)$', 'contentstore.views.manage_users', name='manage_users'), url(r'^add_user/(?P.*?)$', 'contentstore.views.add_user', name='add_user'), @@ -71,8 +73,11 @@ urlpatterns = ('', # nopep8 'contentstore.views.edit_static', name='edit_static'), url(r'^edit_tabs/(?P[^/]+)/(?P[^/]+)/course/(?P[^/]+)$', 'contentstore.views.edit_tabs', name='edit_tabs'), + url(r'^(?P[^/]+)/(?P[^/]+)/assets/(?P[^/]+)$', 'contentstore.views.asset_index', name='asset_index'), + url(r'^(?P[^/]+)/(?P[^/]+)/assets/(?P[^/]+)/remove$', + 'contentstore.views.assets.remove_asset', name='remove_asset'), # this is a generic method to return the data/metadata associated with a xmodule url(r'^module_info/(?P.*)$', diff --git a/common/djangoapps/terrain/ui_helpers.py b/common/djangoapps/terrain/ui_helpers.py index ecd43eb719..b1c5f30467 100644 --- a/common/djangoapps/terrain/ui_helpers.py +++ b/common/djangoapps/terrain/ui_helpers.py @@ -3,6 +3,7 @@ from lettuce import world import time +import platform from urllib import quote_plus from selenium.common.exceptions import WebDriverException, StaleElementReferenceException from selenium.webdriver.support import expected_conditions as EC @@ -57,20 +58,28 @@ def css_find(css, wait_time=5): @world.absorb -def css_click(css_selector): +def css_click(css_selector, index=0, attempts=5): """ Perform a click on a CSS selector, retrying if it initially fails + This function will return if the click worked (since it is try/excepting all errors) """ assert is_css_present(css_selector) - try: - world.browser.find_by_css(css_selector).click() - - except WebDriverException: - # Occassionally, MathJax or other JavaScript can cover up - # an element temporarily. - # If this happens, wait a second, then try again - world.wait(1) - world.browser.find_by_css(css_selector).click() + attempt = 0 + result = False + while attempt < attempts: + try: + world.css_find(css_selector)[index].click() + result = True + break + except WebDriverException: + # Occasionally, MathJax or other JavaScript can cover up + # an element temporarily. + # If this happens, wait a second, then try again + world.wait(1) + attempt += 1 + except: + attempt += 1 + return result @world.absorb @@ -158,3 +167,8 @@ def click_tools(): tools_css = 'li.nav-course-tools' if world.browser.is_element_present_by_css(tools_css): world.css_click(tools_css) + + +@world.absorb +def is_mac(): + return platform.mac_ver()[0] is not '' diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index ca10d4d2a0..fee80a34ff 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -117,6 +117,8 @@ class CapaModule(CapaFields, XModule): ''' An XModule implementing LonCapa format problems, implemented by way of capa.capa_problem.LoncapaProblem + + CapaModule.__init__ takes the same arguments as xmodule.x_module:XModule.__init__ ''' icon_class = 'problem' @@ -131,8 +133,9 @@ class CapaModule(CapaFields, XModule): js_module_name = "Problem" css = {'scss': [resource_string(__name__, 'css/capa/display.scss')]} - def __init__(self, system, location, descriptor, model_data): - XModule.__init__(self, system, location, descriptor, model_data) + def __init__(self, *args, **kwargs): + """ Accepts the same arguments as xmodule.x_module:XModule.__init__ """ + XModule.__init__(self, *args, **kwargs) due_date = self.due diff --git a/common/lib/xmodule/xmodule/combined_open_ended_module.py b/common/lib/xmodule/xmodule/combined_open_ended_module.py index dc753a64b8..ac95567946 100644 --- a/common/lib/xmodule/xmodule/combined_open_ended_module.py +++ b/common/lib/xmodule/xmodule/combined_open_ended_module.py @@ -116,6 +116,8 @@ class CombinedOpenEndedModule(CombinedOpenEndedFields, XModule): incorporates multiple children (tasks): openendedmodule selfassessmentmodule + + CombinedOpenEndedModule.__init__ takes the same arguments as xmodule.x_module:XModule.__init__ """ STATE_VERSION = 1 @@ -139,8 +141,7 @@ class CombinedOpenEndedModule(CombinedOpenEndedFields, XModule): css = {'scss': [resource_string(__name__, 'css/combinedopenended/display.scss')]} - def __init__(self, system, location, descriptor, model_data): - XModule.__init__(self, system, location, descriptor, model_data) + def __init__(self, *args, **kwargs): """ Definition file should have one or many task blocks, a rubric block, and a prompt block: @@ -175,9 +176,9 @@ class CombinedOpenEndedModule(CombinedOpenEndedFields, XModule): """ + XModule.__init__(self, *args, **kwargs) - self.system = system - self.system.set('location', location) + self.system.set('location', self.location) if self.task_states is None: self.task_states = [] @@ -189,13 +190,11 @@ class CombinedOpenEndedModule(CombinedOpenEndedFields, XModule): attributes = self.student_attributes + self.settings_attributes - static_data = { - 'rewrite_content_links': self.rewrite_content_links, - } + static_data = {} instance_state = {k: getattr(self, k) for k in attributes} self.child_descriptor = version_tuple.descriptor(self.system) self.child_definition = version_tuple.descriptor.definition_from_xml(etree.fromstring(self.data), self.system) - self.child_module = version_tuple.module(self.system, location, self.child_definition, self.child_descriptor, + self.child_module = version_tuple.module(self.system, self.location, self.child_definition, self.child_descriptor, instance_state=instance_state, static_data=static_data, attributes=attributes) self.save_instance_data() diff --git a/common/lib/xmodule/xmodule/contentstore/django.py b/common/lib/xmodule/xmodule/contentstore/django.py index 83a2508d96..f163348cc8 100644 --- a/common/lib/xmodule/xmodule/contentstore/django.py +++ b/common/lib/xmodule/xmodule/contentstore/django.py @@ -3,7 +3,7 @@ from importlib import import_module from django.conf import settings -_CONTENTSTORE = None +_CONTENTSTORE = {} def load_function(path): @@ -17,13 +17,16 @@ def load_function(path): return getattr(import_module(module_path), name) -def contentstore(): +def contentstore(name='default'): global _CONTENTSTORE - if _CONTENTSTORE is None: + if name not in _CONTENTSTORE: class_ = load_function(settings.CONTENTSTORE['ENGINE']) options = {} options.update(settings.CONTENTSTORE['OPTIONS']) - _CONTENTSTORE = class_(**options) + if 'ADDITIONAL_OPTIONS' in settings.CONTENTSTORE: + if name in settings.CONTENTSTORE['ADDITIONAL_OPTIONS']: + options.update(settings.CONTENTSTORE['ADDITIONAL_OPTIONS'][name]) + _CONTENTSTORE[name] = class_(**options) - return _CONTENTSTORE + return _CONTENTSTORE[name] diff --git a/common/lib/xmodule/xmodule/contentstore/mongo.py b/common/lib/xmodule/xmodule/contentstore/mongo.py index 58fadb7957..fa0fc95181 100644 --- a/common/lib/xmodule/xmodule/contentstore/mongo.py +++ b/common/lib/xmodule/xmodule/contentstore/mongo.py @@ -1,4 +1,3 @@ -from bson.son import SON from pymongo import Connection import gridfs from gridfs.errors import NoFile @@ -15,15 +14,16 @@ import os class MongoContentStore(ContentStore): - def __init__(self, host, db, port=27017, user=None, password=None, **kwargs): + def __init__(self, host, db, port=27017, user=None, password=None, bucket='fs', **kwargs): logging.debug('Using MongoDB for static content serving at host={0} db={1}'.format(host, db)) _db = Connection(host=host, port=port, **kwargs)[db] if user is not None and password is not None: _db.authenticate(user, password) - self.fs = gridfs.GridFS(_db) - self.fs_files = _db["fs.files"] # the underlying collection GridFS uses + self.fs = gridfs.GridFS(_db, bucket) + + self.fs_files = _db[bucket + ".files"] # the underlying collection GridFS uses def save(self, content): id = content.get_id() @@ -43,7 +43,7 @@ class MongoContentStore(ContentStore): if self.fs.exists({"_id": id}): self.fs.delete(id) - def find(self, location): + def find(self, location, throw_on_not_found=True): id = StaticContent.get_id_from_location(location) try: with self.fs.get(id) as fp: @@ -52,7 +52,10 @@ class MongoContentStore(ContentStore): thumbnail_location=fp.thumbnail_location if hasattr(fp, 'thumbnail_location') else None, import_path=fp.import_path if hasattr(fp, 'import_path') else None) except NoFile: - raise NotFoundError() + if throw_on_not_found: + raise NotFoundError() + else: + return None def export(self, location, output_directory): content = self.find(location) diff --git a/common/lib/xmodule/xmodule/contentstore/utils.py b/common/lib/xmodule/xmodule/contentstore/utils.py new file mode 100644 index 0000000000..74c4242bd9 --- /dev/null +++ b/common/lib/xmodule/xmodule/contentstore/utils.py @@ -0,0 +1,49 @@ +from xmodule.modulestore import Location +from xmodule.contentstore.content import StaticContent +from .django import contentstore + + +def empty_asset_trashcan(course_locs): + ''' + This method will hard delete all assets (optionally within a course_id) from the trashcan + ''' + store = contentstore('trashcan') + + for course_loc in course_locs: + # first delete all of the thumbnails + thumbs = store.get_all_content_thumbnails_for_course(course_loc) + for thumb in thumbs: + thumb_loc = Location(thumb["_id"]) + id = StaticContent.get_id_from_location(thumb_loc) + print "Deleting {0}...".format(id) + store.delete(id) + + # then delete all of the assets + assets = store.get_all_content_for_course(course_loc) + for asset in assets: + asset_loc = Location(asset["_id"]) + id = StaticContent.get_id_from_location(asset_loc) + print "Deleting {0}...".format(id) + store.delete(id) + + +def restore_asset_from_trashcan(location): + ''' + This method will restore an asset which got soft deleted and put back in the original course + ''' + trash = contentstore('trashcan') + store = contentstore() + + loc = StaticContent.get_location_from_path(location) + content = trash.find(loc) + + # ok, save the content into the courseware + store.save(content) + + # see if there is a thumbnail as well, if so move that as well + if content.thumbnail_location is not None: + try: + thumbnail_content = trash.find(content.thumbnail_location) + store.save(thumbnail_content) + except: + pass # OK if this is left dangling diff --git a/common/lib/xmodule/xmodule/error_module.py b/common/lib/xmodule/xmodule/error_module.py index e998ceb58e..a37081c447 100644 --- a/common/lib/xmodule/xmodule/error_module.py +++ b/common/lib/xmodule/xmodule/error_module.py @@ -94,11 +94,11 @@ class ErrorDescriptor(ErrorFields, JSONEditingDescriptor): model_data = { 'error_msg': str(error_msg), 'contents': contents, - 'display_name': 'Error: ' + location.name + 'display_name': 'Error: ' + location.name, + 'location': location, } return cls( system, - location, model_data, ) diff --git a/common/lib/xmodule/xmodule/js/src/videoalpha/display/video_player.coffee b/common/lib/xmodule/xmodule/js/src/videoalpha/display/video_player.coffee index 6e08589187..31dd115fa6 100644 --- a/common/lib/xmodule/xmodule/js/src/videoalpha/display/video_player.coffee +++ b/common/lib/xmodule/xmodule/js/src/videoalpha/display/video_player.coffee @@ -74,6 +74,9 @@ class @VideoPlayerAlpha extends SubviewAlpha # when this change is requested (instead of simply requesting a speed change to 1.0). This has to # be done only when the video is being watched in Firefox. We need to figure out what browser is # currently executing this code. + # + # TODO: Check the status of http://code.google.com/p/gdata-issues/issues/detail?id=4654 + # When the YouTube team fixes the API bug, we can remove this temporary bug fix. @video.isFirefox = navigator.userAgent.toLowerCase().indexOf('firefox') > -1 if @video.videoType is 'html5' @@ -252,6 +255,9 @@ class @VideoPlayerAlpha extends SubviewAlpha # or when we are in Firefox, and the new speed is 1.0. The second case is necessary to # avoid the bug where in Firefox speed switching to 1.0 in HTML5 player mode is handled # incorrectly by YouTube API. + # + # TODO: Check the status of http://code.google.com/p/gdata-issues/issues/detail?id=4654 + # When the YouTube team fixes the API bug, we can remove this temporary bug fix. if (@video.videoType is 'youtube') or ((@video.isFirefox) and (@video.playerType is 'youtube') and (newSpeed is '1.0')) if @isPlaying() @player.loadVideoById(@video.youtubeId(), @currentTime) diff --git a/common/lib/xmodule/xmodule/mako_module.py b/common/lib/xmodule/xmodule/mako_module.py index 8abb1d7777..526fc1a0eb 100644 --- a/common/lib/xmodule/xmodule/mako_module.py +++ b/common/lib/xmodule/xmodule/mako_module.py @@ -18,14 +18,16 @@ class MakoModuleDescriptor(XModuleDescriptor): Expects the descriptor to have the `mako_template` attribute set with the name of the template to render, and it will pass the descriptor as the `module` parameter to that template + + MakoModuleDescriptor.__init__ takes the same arguments as xmodule.x_module:XModuleDescriptor.__init__ """ - def __init__(self, system, location, model_data): - if getattr(system, 'render_template', None) is None: - raise TypeError('{system} must have a render_template function' + def __init__(self, *args, **kwargs): + super(MakoModuleDescriptor, self).__init__(*args, **kwargs) + if getattr(self.runtime, 'render_template', None) is None: + raise TypeError('{runtime} must have a render_template function' ' in order to use a MakoDescriptor'.format( - system=system)) - super(MakoModuleDescriptor, self).__init__(system, location, model_data) + runtime=self.runtime)) def get_context(self): """ diff --git a/common/lib/xmodule/xmodule/modulestore/mongo.py b/common/lib/xmodule/xmodule/modulestore/mongo.py index 87a995cd06..422abbdd73 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo.py @@ -37,15 +37,23 @@ def get_course_id_no_run(location): return "/".join([location.org, location.course]) +class InvalidWriteError(Exception): + """ + Raised to indicate that writing to a particular key + in the KeyValueStore is disabled + """ + + class MongoKeyValueStore(KeyValueStore): """ A KeyValueStore that maps keyed data access to one of the 3 data areas known to the MongoModuleStore (data, children, and metadata) """ - def __init__(self, data, children, metadata): + def __init__(self, data, children, metadata, location): self._data = data self._children = children self._metadata = metadata + self._location = location def get(self, key): if key.scope == Scope.children: @@ -55,7 +63,9 @@ class MongoKeyValueStore(KeyValueStore): elif key.scope == Scope.settings: return self._metadata[key.field_name] elif key.scope == Scope.content: - if key.field_name == 'data' and not isinstance(self._data, dict): + if key.field_name == 'location': + return self._location + elif key.field_name == 'data' and not isinstance(self._data, dict): return self._data else: return self._data[key.field_name] @@ -68,7 +78,9 @@ class MongoKeyValueStore(KeyValueStore): elif key.scope == Scope.settings: self._metadata[key.field_name] = value elif key.scope == Scope.content: - if key.field_name == 'data' and not isinstance(self._data, dict): + if key.field_name == 'location': + self._location = value + elif key.field_name == 'data' and not isinstance(self._data, dict): self._data = value else: self._data[key.field_name] = value @@ -82,7 +94,9 @@ class MongoKeyValueStore(KeyValueStore): if key.field_name in self._metadata: del self._metadata[key.field_name] elif key.scope == Scope.content: - if key.field_name == 'data' and not isinstance(self._data, dict): + if key.field_name == 'location': + self._location = Location(None) + elif key.field_name == 'data' and not isinstance(self._data, dict): self._data = None else: del self._data[key.field_name] @@ -95,7 +109,9 @@ class MongoKeyValueStore(KeyValueStore): elif key.scope == Scope.settings: return key.field_name in self._metadata elif key.scope == Scope.content: - if key.field_name == 'data' and not isinstance(self._data, dict): + if key.field_name == 'location': + return True + elif key.field_name == 'data' and not isinstance(self._data, dict): return True else: return key.field_name in self._data @@ -171,10 +187,11 @@ class CachingDescriptorSystem(MakoDescriptorSystem): definition.get('data', {}), definition.get('children', []), metadata, + location, ) model_data = DbModel(kvs, class_, None, MongoUsage(self.course_id, location)) - module = class_(self, location, model_data) + module = class_(self, model_data) if self.cached_metadata is not None: # parent container pointers don't differentiate between draft and non-draft # so when we do the lookup, we should do so with a non-draft location diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py index 8a479fbf9e..07e6124537 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py @@ -1,11 +1,14 @@ import pymongo from mock import Mock -from nose.tools import assert_equals, assert_raises, assert_not_equals, with_setup, assert_false +from nose.tools import assert_equals, assert_raises, assert_not_equals, assert_false from pprint import pprint +from xblock.core import Scope +from xblock.runtime import KeyValueStore, InvalidScopeError + from xmodule.modulestore import Location -from xmodule.modulestore.mongo import MongoModuleStore +from xmodule.modulestore.mongo import MongoModuleStore, MongoKeyValueStore from xmodule.modulestore.xml_importer import import_from_xml from xmodule.templates import update_templates @@ -114,3 +117,75 @@ class TestMongoModuleStore(object): course.location.org == 'edx' and course.location.course == 'templates', '{0} is a template course'.format(course) ) + +class TestMongoKeyValueStore(object): + + def setUp(self): + self.data = {'foo': 'foo_value'} + self.location = Location('i4x://org/course/category/name@version') + self.children = ['i4x://org/course/child/a', 'i4x://org/course/child/b'] + self.metadata = {'meta': 'meta_val'} + self.kvs = MongoKeyValueStore(self.data, self.children, self.metadata, self.location) + + def _check_read(self, key, expected_value): + assert_equals(expected_value, self.kvs.get(key)) + assert self.kvs.has(key) + + def test_read(self): + assert_equals(self.data['foo'], self.kvs.get(KeyValueStore.Key(Scope.content, None, None, 'foo'))) + assert_equals(self.location, self.kvs.get(KeyValueStore.Key(Scope.content, None, None, 'location'))) + assert_equals(self.children, self.kvs.get(KeyValueStore.Key(Scope.children, None, None, 'children'))) + assert_equals(self.metadata['meta'], self.kvs.get(KeyValueStore.Key(Scope.settings, None, None, 'meta'))) + assert_equals(None, self.kvs.get(KeyValueStore.Key(Scope.parent, None, None, 'parent'))) + + def test_read_invalid_scope(self): + for scope in (Scope.preferences, Scope.user_info, Scope.user_state): + key = KeyValueStore.Key(scope, None, None, 'foo') + with assert_raises(InvalidScopeError): + self.kvs.get(key) + assert_false(self.kvs.has(key)) + + def test_read_non_dict_data(self): + self.kvs._data = 'xml_data' + assert_equals('xml_data', self.kvs.get(KeyValueStore.Key(Scope.content, None, None, 'data'))) + + def _check_write(self, key, value): + self.kvs.set(key, value) + assert_equals(value, self.kvs.get(key)) + + def test_write(self): + yield (self._check_write, KeyValueStore.Key(Scope.content, None, None, 'foo'), 'new_data') + yield (self._check_write, KeyValueStore.Key(Scope.content, None, None, 'location'), Location('i4x://org/course/category/name@new_version')) + yield (self._check_write, KeyValueStore.Key(Scope.children, None, None, 'children'), []) + yield (self._check_write, KeyValueStore.Key(Scope.settings, None, None, 'meta'), 'new_settings') + + def test_write_non_dict_data(self): + self.kvs._data = 'xml_data' + self._check_write(KeyValueStore.Key(Scope.content, None, None, 'data'), 'new_data') + + def test_write_invalid_scope(self): + for scope in (Scope.preferences, Scope.user_info, Scope.user_state, Scope.parent): + with assert_raises(InvalidScopeError): + self.kvs.set(KeyValueStore.Key(scope, None, None, 'foo'), 'new_value') + + def _check_delete_default(self, key, default_value): + self.kvs.delete(key) + assert_equals(default_value, self.kvs.get(key)) + assert self.kvs.has(key) + + def _check_delete_key_error(self, key): + self.kvs.delete(key) + with assert_raises(KeyError): + self.kvs.get(key) + assert_false(self.kvs.has(key)) + + def test_delete(self): + yield (self._check_delete_key_error, KeyValueStore.Key(Scope.content, None, None, 'foo')) + yield (self._check_delete_default, KeyValueStore.Key(Scope.content, None, None, 'location'), Location(None)) + yield (self._check_delete_default, KeyValueStore.Key(Scope.children, None, None, 'children'), []) + yield (self._check_delete_key_error, KeyValueStore.Key(Scope.settings, None, None, 'meta')) + + def test_delete_invalid_scope(self): + for scope in (Scope.preferences, Scope.user_info, Scope.user_state, Scope.parent): + with assert_raises(InvalidScopeError): + self.kvs.delete(KeyValueStore.Key(scope, None, None, 'foo')) diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index 6ab6843216..a704fc2ae8 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -463,7 +463,7 @@ class XMLModuleStore(ModuleStoreBase): # tabs are referenced in policy.json through a 'slug' which is just the filename without the .html suffix slug = os.path.splitext(os.path.basename(filepath))[0] loc = Location('i4x', course_descriptor.location.org, course_descriptor.location.course, category, slug) - module = HtmlDescriptor(system, loc, {'data': html}) + module = HtmlDescriptor(system, {'data': html, 'location': loc}) # VS[compat]: # Hack because we need to pull in the 'display_name' for static tabs (because we need to edit them) # from the course policy 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 b48c70c8d5..45e73442d0 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 @@ -117,7 +117,6 @@ class CombinedOpenEndedV1Module(): self.instance_state = instance_state self.display_name = instance_state.get('display_name', "Open Ended") - self.rewrite_content_links = static_data.get('rewrite_content_links', "") #We need to set the location here so the child modules can use it system.set('location', location) @@ -354,17 +353,7 @@ class CombinedOpenEndedV1Module(): Output: Child task HTML """ self.update_task_states() - html = self.current_task.get_html(self.system) - return_html = html - try: - #Without try except block, get this error: - # File "/home/vik/mitx_all/mitx/common/lib/xmodule/xmodule/x_module.py", line 263, in rewrite_content_links - # if link.startswith(XASSET_SRCREF_PREFIX): - # Placing try except so that if the error is fixed, this code will start working again. - return_html = rewrite_links(html, self.rewrite_content_links) - except Exception: - pass - return return_html + return self.current_task.get_html(self.system) def get_current_attributes(self, task_number): """ diff --git a/common/lib/xmodule/xmodule/peer_grading_module.py b/common/lib/xmodule/xmodule/peer_grading_module.py index 979bc95cb0..a13fef8e40 100644 --- a/common/lib/xmodule/xmodule/peer_grading_module.py +++ b/common/lib/xmodule/xmodule/peer_grading_module.py @@ -62,6 +62,9 @@ class PeerGradingFields(object): class PeerGradingModule(PeerGradingFields, XModule): + """ + PeerGradingModule.__init__ takes the same arguments as xmodule.x_module:XModule.__init__ + """ _VERSION = 1 js = {'coffee': [resource_string(__name__, 'js/src/peergrading/peer_grading.coffee'), @@ -73,12 +76,11 @@ class PeerGradingModule(PeerGradingFields, XModule): css = {'scss': [resource_string(__name__, 'css/combinedopenended/display.scss')]} - def __init__(self, system, location, descriptor, model_data): - XModule.__init__(self, system, location, descriptor, model_data) + def __init__(self, *args, **kwargs): + super(PeerGradingModule, self).__init__(*args, **kwargs) - # We need to set the location here so the child modules can use it - system.set('location', location) - self.system = system + #We need to set the location here so the child modules can use it + self.runtime.set('location', self.location) if (self.system.open_ended_grading_interface): self.peer_gs = PeerGradingService(self.system.open_ended_grading_interface, self.system) else: diff --git a/common/lib/xmodule/xmodule/tests/test_annotatable_module.py b/common/lib/xmodule/xmodule/tests/test_annotatable_module.py index 43eae8e43e..268f743421 100644 --- a/common/lib/xmodule/xmodule/tests/test_annotatable_module.py +++ b/common/lib/xmodule/xmodule/tests/test_annotatable_module.py @@ -29,10 +29,10 @@ class AnnotatableModuleTestCase(unittest.TestCase): ''' descriptor = Mock() - module_data = {'data': sample_xml} + module_data = {'data': sample_xml, 'location': location} def setUp(self): - self.annotatable = AnnotatableModule(test_system(), self.location, self.descriptor, self.module_data) + self.annotatable = AnnotatableModule(test_system(), self.descriptor, self.module_data) def test_annotation_data_attr(self): el = etree.fromstring('test') diff --git a/common/lib/xmodule/xmodule/tests/test_capa_module.py b/common/lib/xmodule/xmodule/tests/test_capa_module.py index 3153250527..7cba4a76b3 100644 --- a/common/lib/xmodule/xmodule/tests/test_capa_module.py +++ b/common/lib/xmodule/xmodule/tests/test_capa_module.py @@ -86,7 +86,7 @@ class CapaFactory(object): """ location = Location(["i4x", "edX", "capa_test", "problem", "SampleProblem{0}".format(CapaFactory.next_num())]) - model_data = {'data': CapaFactory.sample_problem_xml} + model_data = {'data': CapaFactory.sample_problem_xml, 'location': location} if graceperiod is not None: model_data['graceperiod'] = graceperiod @@ -113,7 +113,7 @@ class CapaFactory(object): system = test_system() system.render_template = Mock(return_value="
Test Template HTML
") - module = CapaModule(system, location, descriptor, model_data) + module = CapaModule(system, descriptor, model_data) if correct: # TODO: probably better to actually set the internal state properly, but... 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 409347882f..7dbaf6fe3d 100644 --- a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py +++ b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py @@ -175,7 +175,6 @@ class OpenEndedModuleTest(unittest.TestCase): 'max_score': max_score, 'display_name': 'Name', 'accept_file_upload': False, - 'rewrite_content_links': "", 'close_date': None, 's3_interface': test_util_open_ended.S3_INTERFACE, 'open_ended_grading_interface': test_util_open_ended.OPEN_ENDED_GRADING_INTERFACE, @@ -332,7 +331,6 @@ class CombinedOpenEndedModuleTest(unittest.TestCase): 'max_score': max_score, 'display_name': 'Name', 'accept_file_upload': False, - 'rewrite_content_links': "", 'close_date': "", 's3_interface': test_util_open_ended.S3_INTERFACE, 'open_ended_grading_interface': test_util_open_ended.OPEN_ENDED_GRADING_INTERFACE, @@ -370,10 +368,15 @@ class CombinedOpenEndedModuleTest(unittest.TestCase): full_definition = definition_template.format(prompt=prompt, rubric=rubric, task1=task_xml1, task2=task_xml2) descriptor = Mock(data=full_definition) test_system = test_system() - combinedoe_container = CombinedOpenEndedModule(test_system, - location, - descriptor, - model_data={'data': full_definition, 'weight': '1'}) + combinedoe_container = CombinedOpenEndedModule( + test_system, + descriptor, + model_data={ + 'data': full_definition, + 'weight': '1', + 'location': location + } + ) def setUp(self): # TODO: this constructor call is definitely wrong, but neither branch diff --git a/common/lib/xmodule/xmodule/tests/test_conditional.py b/common/lib/xmodule/xmodule/tests/test_conditional.py index 320b94efb7..e88bf0c588 100644 --- a/common/lib/xmodule/xmodule/tests/test_conditional.py +++ b/common/lib/xmodule/xmodule/tests/test_conditional.py @@ -60,9 +60,9 @@ class ConditionalFactory(object): source_location = Location(["i4x", "edX", "conditional_test", "problem", "SampleProblem"]) if source_is_error_module: # Make an error descriptor and module - source_descriptor = NonStaffErrorDescriptor.from_xml('some random xml data', + source_descriptor = NonStaffErrorDescriptor.from_xml('some random xml data', system, - org=source_location.org, + org=source_location.org, course=source_location.course, error_msg='random error message') source_module = source_descriptor.xmodule(system) @@ -87,8 +87,8 @@ class ConditionalFactory(object): # construct conditional module: cond_location = Location(["i4x", "edX", "conditional_test", "conditional", "SampleConditional"]) - model_data = {'data': ''} - cond_module = ConditionalModule(system, cond_location, cond_descriptor, model_data) + model_data = {'data': '', 'location': cond_location} + cond_module = ConditionalModule(system, cond_descriptor, model_data) # return dict: return {'cond_module': cond_module, @@ -106,7 +106,7 @@ class ConditionalModuleBasicTest(unittest.TestCase): self.test_system = test_system() def test_icon_class(self): - '''verify that get_icon_class works independent of condition satisfaction''' + '''verify that get_icon_class works independent of condition satisfaction''' modules = ConditionalFactory.create(self.test_system) for attempted in ["false", "true"]: for icon_class in [ 'other', 'problem', 'video']: @@ -186,7 +186,6 @@ class ConditionalModuleXmlTest(unittest.TestCase): if isinstance(descriptor, Location): location = descriptor descriptor = self.modulestore.get_instance(course.id, location, depth=None) - location = descriptor.location return descriptor.xmodule(self.test_system) # edx - HarvardX diff --git a/common/lib/xmodule/xmodule/tests/test_html_module.py b/common/lib/xmodule/xmodule/tests/test_html_module.py index e56e9babe7..ea6b358f3b 100644 --- a/common/lib/xmodule/xmodule/tests/test_html_module.py +++ b/common/lib/xmodule/xmodule/tests/test_html_module.py @@ -8,14 +8,13 @@ from xmodule.modulestore import Location from . import test_system class HtmlModuleSubstitutionTestCase(unittest.TestCase): - location = Location(["i4x", "edX", "toy", "html", "simple_html"]) descriptor = Mock() def test_substitution_works(self): sample_xml = '''%%USER_ID%%''' module_data = {'data': sample_xml} module_system = test_system() - module = HtmlModule(module_system, self.location, self.descriptor, module_data) + module = HtmlModule(module_system, self.descriptor, module_data) self.assertEqual(module.get_html(), str(module_system.anonymous_student_id)) @@ -26,7 +25,7 @@ class HtmlModuleSubstitutionTestCase(unittest.TestCase): ''' module_data = {'data': sample_xml} - module = HtmlModule(test_system(), self.location, self.descriptor, module_data) + module = HtmlModule(test_system(), self.descriptor, module_data) self.assertEqual(module.get_html(), sample_xml) @@ -35,6 +34,6 @@ class HtmlModuleSubstitutionTestCase(unittest.TestCase): module_data = {'data': sample_xml} module_system = test_system() module_system.anonymous_student_id = None - module = HtmlModule(module_system, self.location, self.descriptor, module_data) + module = HtmlModule(module_system, self.descriptor, module_data) self.assertEqual(module.get_html(), sample_xml) diff --git a/common/lib/xmodule/xmodule/tests/test_logic.py b/common/lib/xmodule/xmodule/tests/test_logic.py index e60af63921..ff17f88dfc 100644 --- a/common/lib/xmodule/xmodule/tests/test_logic.py +++ b/common/lib/xmodule/xmodule/tests/test_logic.py @@ -30,13 +30,13 @@ class LogicTest(unittest.TestCase): pass self.system = None - self.location = None self.descriptor = EmptyClass() self.xmodule_class = self.descriptor_class.module_class self.xmodule = self.xmodule_class( - self.system, self.location, - self.descriptor, self.raw_model_data + self.system, + self.descriptor, + self.raw_model_data ) def ajax_request(self, dispatch, get): diff --git a/common/lib/xmodule/xmodule/tests/test_mako_module.py b/common/lib/xmodule/xmodule/tests/test_mako_module.py new file mode 100644 index 0000000000..7ba023bda7 --- /dev/null +++ b/common/lib/xmodule/xmodule/tests/test_mako_module.py @@ -0,0 +1,22 @@ +""" Test mako_module.py """ + +from unittest import TestCase +from mock import Mock + +from xmodule.mako_module import MakoModuleDescriptor + + +class MakoModuleTest(TestCase): + """ Test MakoModuleDescriptor """ + + def test_render_template_check(self): + mock_system = Mock() + mock_system.render_template = None + + with self.assertRaises(TypeError): + MakoModuleDescriptor(mock_system, {}) + + del mock_system.render_template + + with self.assertRaises(TypeError): + MakoModuleDescriptor(mock_system, {}) diff --git a/common/lib/xmodule/xmodule/tests/test_progress.py b/common/lib/xmodule/xmodule/tests/test_progress.py index 4bb663ad85..97ec00b13e 100644 --- a/common/lib/xmodule/xmodule/tests/test_progress.py +++ b/common/lib/xmodule/xmodule/tests/test_progress.py @@ -134,6 +134,6 @@ class ModuleProgressTest(unittest.TestCase): ''' def test_xmodule_default(self): '''Make sure default get_progress exists, returns None''' - xm = x_module.XModule(test_system(), 'a://b/c/d/e', None, {}) + xm = x_module.XModule(test_system(), None, {'location': 'a://b/c/d/e'}) p = xm.get_progress() self.assertEqual(p, None) diff --git a/lms/djangoapps/courseware/tests/test_video_xml.py b/common/lib/xmodule/xmodule/tests/test_video_xml.py similarity index 97% rename from lms/djangoapps/courseware/tests/test_video_xml.py rename to common/lib/xmodule/xmodule/tests/test_video_xml.py index 169e5be2d9..0dcec78d08 100644 --- a/lms/djangoapps/courseware/tests/test_video_xml.py +++ b/common/lib/xmodule/xmodule/tests/test_video_xml.py @@ -47,13 +47,13 @@ class VideoFactory(object): """Method return Video Xmodule instance.""" location = Location(["i4x", "edX", "video", "default", "SampleProblem1"]) - model_data = {'data': VideoFactory.sample_problem_xml_youtube} + model_data = {'data': VideoFactory.sample_problem_xml_youtube, 'location': location} descriptor = Mock(weight="1", url_name="SampleProblem1") system = test_system() system.render_template = lambda template, context: context - module = VideoModule(system, location, descriptor, model_data) + module = VideoModule(system, descriptor, model_data) return module diff --git a/common/lib/xmodule/xmodule/tests/test_xml_module.py b/common/lib/xmodule/xmodule/tests/test_xml_module.py index eb715b44bc..46410def8e 100644 --- a/common/lib/xmodule/xmodule/tests/test_xml_module.py +++ b/common/lib/xmodule/xmodule/tests/test_xml_module.py @@ -142,7 +142,7 @@ class EditableMetadataFieldsTest(unittest.TestCase): 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 + return XmlDescriptor(runtime=system, model_data=model_data).editable_metadata_fields def get_descriptor(self, model_data): class TestModuleDescriptor(TestFields, XmlDescriptor): @@ -154,7 +154,7 @@ class EditableMetadataFieldsTest(unittest.TestCase): system = test_system() system.render_template = Mock(return_value="
Test Template HTML
") - return TestModuleDescriptor(system=system, location=None, model_data=model_data) + return TestModuleDescriptor(runtime=system, model_data=model_data) def assert_field_values(self, editable_fields, name, field, explicitly_set, inheritable, value, default_value, type='Generic', options=[]): diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index a1813a795a..3edc22df43 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -10,7 +10,7 @@ from pkg_resources import resource_listdir, resource_string, resource_isdir from xmodule.modulestore import Location from xmodule.modulestore.exceptions import ItemNotFoundError -from xblock.core import XBlock, Scope, String, Integer, Float +from xblock.core import XBlock, Scope, String, Integer, Float, ModelType log = logging.getLogger(__name__) @@ -19,6 +19,23 @@ def dummy_track(event_type, event): pass +class LocationField(ModelType): + """ + XBlock field for storing Location values + """ + def from_json(self, value): + """ + Parse the json value as a Location + """ + return Location(value) + + def to_json(self, value): + """ + Store the Location as a url string in json + """ + return value.url() + + class HTMLSnippet(object): """ A base class defining an interface for an object that is able to present an @@ -87,6 +104,16 @@ class XModuleFields(object): default=None ) + # Please note that in order to be compatible with XBlocks more generally, + # the LMS and CMS shouldn't be using this field. It's only for internal + # consumption by the XModules themselves + location = LocationField( + display_name="Location", + help="This is the location id for the XModule.", + scope=Scope.content, + default=Location(None), + ) + class XModule(XModuleFields, HTMLSnippet, XBlock): ''' Implements a generic learning module. @@ -106,24 +133,20 @@ class XModule(XModuleFields, HTMLSnippet, XBlock): icon_class = 'other' - def __init__(self, system, location, descriptor, model_data): + def __init__(self, runtime, descriptor, model_data): ''' Construct a new xmodule - system: A ModuleSystem allowing access to external resources - - location: Something Location-like that identifies this xmodule + runtime: An XBlock runtime allowing access to external resources descriptor: the XModuleDescriptor that this module is an instance of. - TODO (vshnayder): remove the definition parameter and location--they - can come from the descriptor. model_data: A dictionary-like object that maps field names to values for those fields. ''' + super(XModule, self).__init__(runtime, model_data) self._model_data = model_data - self.system = system - self.location = Location(location) + self.system = runtime self.descriptor = descriptor self.url_name = self.location.name self.category = self.location.category @@ -254,19 +277,6 @@ class XModule(XModuleFields, HTMLSnippet, XBlock): get is a dictionary-like object ''' return "" - # cdodge: added to support dynamic substitutions of - # links for courseware assets (e.g. images). is passed through from lxml.html parser - def rewrite_content_links(self, link): - # see if we start with our format, e.g. 'xasset:' - if link.startswith(XASSET_SRCREF_PREFIX): - # yes, then parse out the name - name = link[len(XASSET_SRCREF_PREFIX):] - loc = Location(self.location) - # resolve the reference to our internal 'filepath' which - link = StaticContent.compute_location_filename(loc.org, loc.course, name) - - return link - def policy_key(location): """ @@ -340,13 +350,12 @@ class XModuleDescriptor(XModuleFields, HTMLSnippet, ResourceTemplates, XBlock): template_dir_name = "default" # Class level variable + + # True if this descriptor always requires recalculation of grades, for + # example if the score can change via an extrnal service, not just when the + # student interacts with the module on the page. A specific example is + # FoldIt, which posts grade-changing updates through a separate API. always_recalculate_grades = False - """ - Return whether this descriptor always requires recalculation of grades, for - example if the score can change via an extrnal service, not just when the - student interacts with the module on the page. A specific example is - FoldIt, which posts grade-changing updates through a separate API. - """ # VS[compat]. Backwards compatibility code that can go away after # importing 2012 courses. @@ -357,10 +366,7 @@ class XModuleDescriptor(XModuleFields, HTMLSnippet, ResourceTemplates, XBlock): } # ============================= STRUCTURAL MANIPULATION =================== - def __init__(self, - system, - location, - model_data): + def __init__(self, *args, **kwargs): """ Construct a new XModuleDescriptor. The only required arguments are the system, used for interaction with external resources, and the @@ -371,19 +377,17 @@ class XModuleDescriptor(XModuleFields, HTMLSnippet, ResourceTemplates, XBlock): This allows for maximal flexibility to add to the interface while preserving backwards compatibility. - system: A DescriptorSystem for interacting with external resources - - location: Something Location-like that identifies this xmodule + runtime: A DescriptorSystem for interacting with external resources model_data: A dictionary-like object that maps field names to values for those fields. + + XModuleDescriptor.__init__ takes the same arguments as xblock.core:XBlock.__init__ """ - self.system = system - self.location = Location(location) + super(XModuleDescriptor, self).__init__(*args, **kwargs) + self.system = self.runtime self.url_name = self.location.name self.category = self.location.category - self._model_data = model_data - self._child_instances = None @property @@ -441,7 +445,6 @@ class XModuleDescriptor(XModuleFields, HTMLSnippet, ResourceTemplates, XBlock): """ return self.module_class( system, - self.location, self, system.xblock_model_data(self), ) @@ -510,7 +513,9 @@ class XModuleDescriptor(XModuleFields, HTMLSnippet, ResourceTemplates, XBlock): else: model_data['data'] = definition['data'] - return cls(system=system, location=json_data['location'], model_data=model_data) + model_data['location'] = json_data['location'] + + return cls(system, model_data) @classmethod def _translate(cls, key): diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py index 9b5de9d7e7..e1a0e0cf08 100644 --- a/common/lib/xmodule/xmodule/xml_module.py +++ b/common/lib/xmodule/xmodule/xml_module.py @@ -254,7 +254,7 @@ class XmlDescriptor(XModuleDescriptor): definition, children = cls.definition_from_xml(definition_xml, system) if definition_metadata: definition['definition_metadata'] = definition_metadata - definition['filename'] = [ filepath, filename ] + definition['filename'] = [ filepath, filename ] return definition, children @@ -352,10 +352,10 @@ class XmlDescriptor(XModuleDescriptor): for key, value in metadata.items(): if key not in set(f.name for f in cls.fields + cls.lms.fields): model_data['xml_attributes'][key] = value + model_data['location'] = location return cls( system, - location, model_data, ) diff --git a/lms/CHANGELOG.md b/lms/CHANGELOG.md deleted file mode 100644 index 0794d379b9..0000000000 --- a/lms/CHANGELOG.md +++ /dev/null @@ -1,18 +0,0 @@ -Instructions -============ -For each pull request, add one or more lines to the bottom of the change list. When -code is released to production, change the `Upcoming` entry to todays date, and add -a new block at the bottom of the file. - - Upcoming - -------- - -Change log entries should be targeted at end users. A good place to start is the -user story that instigated the pull request. - -Changes -======= - -Upcoming --------- -* Created changelog \ No newline at end of file diff --git a/lms/djangoapps/courseware/tests/__init__.py b/lms/djangoapps/courseware/tests/__init__.py index dc61239c36..cc53bf735a 100644 --- a/lms/djangoapps/courseware/tests/__init__.py +++ b/lms/djangoapps/courseware/tests/__init__.py @@ -77,14 +77,15 @@ class BaseTestXmodule(ModuleStoreTestCase): data=self.DATA ) - location = self.item_descriptor.location system = test_system() system.render_template = lambda template, context: context + model_data = {'location': self.item_descriptor.location} + model_data.update(self.MODEL_DATA) self.item_module = self.item_descriptor.module_class( - system, location, self.item_descriptor, self.MODEL_DATA + system, self.item_descriptor, model_data ) - self.item_url = Location(location).url() + self.item_url = Location(self.item_module.location).url() # login all users for acces to Xmodule self.clients = {user.username: Client() for user in self.users} diff --git a/lms/djangoapps/courseware/tests/test_video_mongo.py b/lms/djangoapps/courseware/tests/test_video_mongo.py index c041ccc151..a0fdecc77a 100644 --- a/lms/djangoapps/courseware/tests/test_video_mongo.py +++ b/lms/djangoapps/courseware/tests/test_video_mongo.py @@ -15,8 +15,8 @@ class TestVideo(BaseTestXmodule): user.username: self.clients[user.username].post( self.get_url('whatever'), {}, - HTTP_X_REQUESTED_WITH='XMLHttpRequest') - for user in self.users + HTTP_X_REQUESTED_WITH='XMLHttpRequest' + ) for user in self.users } self.assertEqual( diff --git a/lms/djangoapps/open_ended_grading/tests.py b/lms/djangoapps/open_ended_grading/tests.py index 13d780df12..3b6c992881 100644 --- a/lms/djangoapps/open_ended_grading/tests.py +++ b/lms/djangoapps/open_ended_grading/tests.py @@ -160,7 +160,7 @@ class TestPeerGradingService(LoginEnrollmentTestCase): self.course_id = "edX/toy/2012_Fall" self.toy = modulestore().get_course(self.course_id) location = "i4x://edX/toy/peergrading/init" - model_data = {'data': ""} + model_data = {'data': "", 'location': location} self.mock_service = peer_grading_service.MockPeerGradingService() self.system = ModuleSystem( ajax_url=location, @@ -172,9 +172,9 @@ class TestPeerGradingService(LoginEnrollmentTestCase): s3_interface=test_util_open_ended.S3_INTERFACE, open_ended_grading_interface=test_util_open_ended.OPEN_ENDED_GRADING_INTERFACE ) - self.descriptor = peer_grading_module.PeerGradingDescriptor(self.system, location, model_data) - model_data = {} - self.peer_module = peer_grading_module.PeerGradingModule(self.system, location, self.descriptor, model_data) + self.descriptor = peer_grading_module.PeerGradingDescriptor(self.system, model_data) + model_data = {'location': location} + self.peer_module = peer_grading_module.PeerGradingModule(self.system, self.descriptor, model_data) self.peer_module.peer_gs = self.mock_service self.logout() diff --git a/lms/envs/aws.py b/lms/envs/aws.py index 73f49a6209..c8c49c2b1e 100644 --- a/lms/envs/aws.py +++ b/lms/envs/aws.py @@ -172,18 +172,19 @@ for name, value in ENV_TOKENS.get("CODE_JAIL", {}).items(): COURSES_WITH_UNSAFE_CODE = ENV_TOKENS.get("COURSES_WITH_UNSAFE_CODE", []) -############### Mixed Related(Secure/Not-Secure) Items ########## -# If segment.io key specified, load it and turn on segment IO if the feature flag is set -SEGMENT_IO_LMS_KEY = AUTH_TOKENS.get('SEGMENT_IO_LMS_KEY') -if SEGMENT_IO_LMS_KEY: - MITX_FEATURES['SEGMENT_IO_LMS'] = ENV_TOKENS.get('SEGMENT_IO_LMS', False) - ############################## SECURE AUTH ITEMS ############### # Secret things: passwords, access keys, etc. with open(ENV_ROOT / CONFIG_PREFIX + "auth.json") as auth_file: AUTH_TOKENS = json.load(auth_file) +############### Mixed Related(Secure/Not-Secure) Items ########## +# If Segment.io key specified, load it and enable Segment.io if the feature flag is set +SEGMENT_IO_LMS_KEY = AUTH_TOKENS.get('SEGMENT_IO_LMS_KEY') +if SEGMENT_IO_LMS_KEY: + MITX_FEATURES['SEGMENT_IO_LMS'] = ENV_TOKENS.get('SEGMENT_IO_LMS', False) + + SECRET_KEY = AUTH_TOKENS['SECRET_KEY'] AWS_ACCESS_KEY_ID = AUTH_TOKENS["AWS_ACCESS_KEY_ID"]
Name Date Added URL
+ +