From 21dc4d863e92388193875955aaa34ae889674353 Mon Sep 17 00:00:00 2001 From: cahrens Date: Fri, 14 Jun 2013 14:10:40 -0400 Subject: [PATCH 01/32] Put back in some older code to save source_code property. --- cms/static/coffee/src/views/module_edit.coffee | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) 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", { From 2cf71b7ff328c79e57c9a709b9b604db05885d04 Mon Sep 17 00:00:00 2001 From: cahrens Date: Fri, 14 Jun 2013 15:12:25 -0400 Subject: [PATCH 02/32] Add helper method for typing in CodeMirror, make it robust to Mac and Unix. --- .../features/advanced-settings.py | 24 ++----------- .../contentstore/features/common.py | 11 ++++++ .../features/problem-editor.feature | 6 ++++ .../contentstore/features/problem-editor.py | 35 ++++++++++++++++--- common/djangoapps/terrain/ui_helpers.py | 6 ++++ 5 files changed, 57 insertions(+), 25 deletions(-) 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 494192ad06..0ebdd75c09 100644 --- a/cms/djangoapps/contentstore/features/common.py +++ b/cms/djangoapps/contentstore/features/common.py @@ -169,3 +169,14 @@ def open_new_unit(step): step.given('I have added a new subsection') step.given('I expand the first section') world.css_click('a.new-unit-item') + + +def type_in_codemirror(index, text): + world.css_find(".CodeMirror")[index].click() + 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) \ No newline at end of file diff --git a/cms/djangoapps/contentstore/features/problem-editor.feature b/cms/djangoapps/contentstore/features/problem-editor.feature index bde350d8a3..893f9b48f9 100644 --- a/cms/djangoapps/contentstore/features/problem-editor.feature +++ b/cms/djangoapps/contentstore/features/problem-editor.feature @@ -65,3 +65,9 @@ Feature: Problem Editor Given I have created a LaTeX Problem And 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 + And I edit 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..ff8efcdb5e 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') @@ -159,7 +160,33 @@ def create_latex_problem(step): 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 the High Level Source') +def edit_latex_source(step): + world.css_click('a.edit-button') + world.css_find('.launch-latex-compiler').find_by_css('a').click() + # Without the wait, the second code mirror is not yet clickable. + world.wait(1) + 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): + world.css_click('a.edit-button') + world.css_find('.launch-latex-compiler').find_by_css('a').click() + world.wait(2) + 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')) diff --git a/common/djangoapps/terrain/ui_helpers.py b/common/djangoapps/terrain/ui_helpers.py index ecd43eb719..f933d342c8 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 @@ -158,3 +159,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 '' From bb70907e206b25b882368c2a59e0157f291b6bcb Mon Sep 17 00:00:00 2001 From: Valera Rozuvan Date: Mon, 17 Jun 2013 08:48:55 +0300 Subject: [PATCH 03/32] Added info to changelog for recent pull requests. --- CHANGELOG.rst | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index bbbb023527..03a3b8c809 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -10,6 +10,11 @@ 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. +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. + 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 @@ -32,7 +37,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. @@ -53,7 +58,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. From 184c0a5c40ecd2e62793f8beeb2c0e2117833775 Mon Sep 17 00:00:00 2001 From: Valera Rozuvan Date: Mon, 17 Jun 2013 09:17:48 +0300 Subject: [PATCH 04/32] Added comment regarding the fact that the temporary bug fix should be eventually removed. --- .../xmodule/js/src/videoalpha/display/video_player.coffee | 6 ++++++ 1 file changed, 6 insertions(+) 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) From 770b9f1f872674b280c88b50b4faa04e8d86f137 Mon Sep 17 00:00:00 2001 From: John Jarvis Date: Mon, 17 Jun 2013 09:01:20 -0400 Subject: [PATCH 05/32] fixing segment io settings bug * Moving to the auth tokens section * Removing the feature flag logic since all features are loaded earlier ``` for feature, value in ENV_TOKENS.get('MITX_FEATURES', {}).items(): MITX_FEATURES[feature] = value ``` --- lms/envs/aws.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/lms/envs/aws.py b/lms/envs/aws.py index 73f49a6209..da1e9fb40f 100644 --- a/lms/envs/aws.py +++ b/lms/envs/aws.py @@ -172,18 +172,14 @@ 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) +SEGMENT_IO_LMS_KEY = AUTH_TOKENS.get('SEGMENT_IO_LMS_KEY', None) + SECRET_KEY = AUTH_TOKENS['SECRET_KEY'] AWS_ACCESS_KEY_ID = AUTH_TOKENS["AWS_ACCESS_KEY_ID"] From ffeabae069cd88e136b6bbfa66d3e5a9d6becd4d Mon Sep 17 00:00:00 2001 From: John Jarvis Date: Mon, 17 Jun 2013 09:07:02 -0400 Subject: [PATCH 06/32] moving settings into the auth tokens section --- lms/envs/aws.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lms/envs/aws.py b/lms/envs/aws.py index da1e9fb40f..b9d3f58e8f 100644 --- a/lms/envs/aws.py +++ b/lms/envs/aws.py @@ -178,7 +178,12 @@ COURSES_WITH_UNSAFE_CODE = ENV_TOKENS.get("COURSES_WITH_UNSAFE_CODE", []) with open(ENV_ROOT / CONFIG_PREFIX + "auth.json") as auth_file: AUTH_TOKENS = json.load(auth_file) -SEGMENT_IO_LMS_KEY = AUTH_TOKENS.get('SEGMENT_IO_LMS_KEY', None) +############### 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) + SECRET_KEY = AUTH_TOKENS['SECRET_KEY'] From 1e3efadc28d09306a7211783255001085e04f061 Mon Sep 17 00:00:00 2001 From: Renzo Lucioni Date: Mon, 17 Jun 2013 10:58:37 -0400 Subject: [PATCH 07/32] switch to using environment variables to get key for Segment.io --- cms/envs/aws.py | 6 ++++-- cms/envs/common.py | 16 +++++++++++++--- cms/envs/dev.py | 8 ++++++-- 3 files changed, 23 insertions(+), 7 deletions(-) diff --git a/cms/envs/aws.py b/cms/envs/aws.py index 35b15fe6ba..dbade8bba5 100644 --- a/cms/envs/aws.py +++ b/cms/envs/aws.py @@ -112,8 +112,10 @@ 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 +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..7b9c0c52e4 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, diff --git a/cms/envs/dev.py b/cms/envs/dev.py index cbe47a1fe1..9de42656c6 100644 --- a/cms/envs/dev.py +++ b/cms/envs/dev.py @@ -163,8 +163,12 @@ 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 +SEGMENT_IO_KEY = os.environ.get('SEGMENT_IO_KEY') +if SEGMENT_IO_KEY: + MITX_FEATURES['SEGMENT_IO'] = True ##################################################################### From 3fa6e77dff6a5f4c33b6fc28d0555f20c7f846d2 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Thu, 13 Jun 2013 11:14:56 -0400 Subject: [PATCH 08/32] add new collection configuration to support a 'trashcan' for assets. Update class factory to allow callers to specify if they want the default set of assets or the trashcan --- cms/envs/dev.py | 7 ++++++- common/lib/xmodule/xmodule/contentstore/django.py | 13 ++++++++----- common/lib/xmodule/xmodule/contentstore/mongo.py | 8 ++++---- 3 files changed, 18 insertions(+), 10 deletions(-) diff --git a/cms/envs/dev.py b/cms/envs/dev.py index cbe47a1fe1..988856dbf4 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', 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..7d96e132ee 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() From 9b749c4201fc898e17719f39e3885b201b563a1d Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Thu, 13 Jun 2013 11:59:19 -0400 Subject: [PATCH 09/32] add ajax callback entry point to remove the asset --- cms/djangoapps/contentstore/views/assets.py | 51 +++++++++++++++++++++ cms/urls.py | 5 ++ 2 files changed, 56 insertions(+) diff --git a/cms/djangoapps/contentstore/views/assets.py b/cms/djangoapps/contentstore/views/assets.py index 229788f24d..f1f51b3ca9 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 @@ -82,6 +84,8 @@ def asset_index(request, org, course, 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 +149,53 @@ def upload_asset(request, org, course, coursename): return response +@ensure_csrf_cookie +@login_required +def remove_asset(request, org, course, name, location): + ''' + 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) + + # make sure the location is valid + try: + loc = StaticContent.get_location_from_path(request.path) + 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) + + @ensure_csrf_cookie @login_required def import_course(request, org, course, name): diff --git a/cms/urls.py b/cms/urls.py index e7444de4e9..ebd5e33323 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/remove/(?P.*?)$', + 'contentstore.views.remove_asset', name='remove_asset'), # this is a generic method to return the data/metadata associated with a xmodule url(r'^module_info/(?P.*)$', From 85b904f176c11cd4af52afaf1bdd35509ce49193 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Thu, 13 Jun 2013 16:01:06 -0400 Subject: [PATCH 10/32] fix sizing of the delete column --- .../commands/empty_asset_trashcan.py | 28 +++++++++++ .../commands/restore_asset_from_trashcan.py | 17 +++++++ cms/djangoapps/contentstore/views/assets.py | 16 ++++-- cms/static/js/base.js | 24 +++++++++ cms/static/sass/views/_assets.scss | 4 ++ cms/templates/asset_index.html | 28 ++++++++++- cms/urls.py | 4 +- .../lib/xmodule/xmodule/contentstore/utils.py | 49 +++++++++++++++++++ 8 files changed, 164 insertions(+), 6 deletions(-) create mode 100644 cms/djangoapps/contentstore/management/commands/empty_asset_trashcan.py create mode 100644 cms/djangoapps/contentstore/management/commands/restore_asset_from_trashcan.py create mode 100644 common/lib/xmodule/xmodule/contentstore/utils.py 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..c10700c7af --- /dev/null +++ b/cms/djangoapps/contentstore/management/commands/empty_asset_trashcan.py @@ -0,0 +1,28 @@ +### +### Script for cloning a course +### +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..0a4be40efc --- /dev/null +++ b/cms/djangoapps/contentstore/management/commands/restore_asset_from_trashcan.py @@ -0,0 +1,17 @@ +### +### Script for cloning a course +### +from django.core.management.base import BaseCommand, CommandError +from xmodule.contentstore.utils import restore_asset_from_trashcan +from xmodule.modulestore import Location + + +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/views/assets.py b/cms/djangoapps/contentstore/views/assets.py index f1f51b3ca9..62dee1ba21 100644 --- a/cms/djangoapps/contentstore/views/assets.py +++ b/cms/djangoapps/contentstore/views/assets.py @@ -80,7 +80,12 @@ 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 + }) }) @@ -151,16 +156,19 @@ def upload_asset(request, org, course, coursename): @ensure_csrf_cookie @login_required -def remove_asset(request, org, course, name, location): +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'] + logging.debug('location = {0}'.format(location)) + # make sure the location is valid try: - loc = StaticContent.get_location_from_path(request.path) + loc = StaticContent.get_location_from_path(location) except InvalidLocationError: # return a 'Bad Request' to browser as we have a malformed Location response = HttpResponse() @@ -195,6 +203,8 @@ def remove_asset(request, org, course, name, location): # remove from cache del_cached_content(content.location) + return HttpResponse() + @ensure_csrf_cookie @login_required diff --git a/cms/static/js/base.js b/cms/static/js/base.js index c626fa1b3f..9cb70592cb 100644 --- a/cms/static/js/base.js +++ b/cms/static/js/base.js @@ -146,6 +146,7 @@ $(document).ready(function() { $('.edit-section-start-save').bind('click', saveSetSectionScheduleDate); $('.upload-modal .choose-file-button').bind('click', showFileSelectionMenu); + $('.remove-asset-button').bind('click', removeAsset); $body.on('click', '.section-published-date .edit-button', editSectionPublishDate); $body.on('click', '.section-published-date .schedule-button', editSectionPublishDate); @@ -398,6 +399,29 @@ function _deleteItem($el) { }); } +function removeAsset(e) { + e.preventDefault(); + + // replace with new notification moodal + if (!confirm('Are you sure you wish to delete this item. It cannot be reversed!')) return; + + var remove_asset_url = $('.asset-library').data('remove-asset-callback-url'); + var location = $(this).closest('tr').data('id'); + var that = this; + $.post(remove_asset_url, + { 'location': location }, + function() { + // show the alert + $(".wrapper-alert-confirmation").addClass("is-shown").attr('aria-hidden','false'); + $(that).closest('tr').remove(); + analytics.track('Deleted Asset', { + 'course': course_location_analytics, + 'id': location + }); + } + ); +} + function showUploadModal(e) { e.preventDefault(); $modal = $('.upload-modal').show(); 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..0de38510f5 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 @@ -30,6 +31,9 @@ + + + @@ -56,7 +60,7 @@
-
+
@@ -64,6 +68,7 @@ + @@ -86,6 +91,9 @@ + % endfor @@ -129,3 +137,21 @@ + +<%block name="view_alerts"> + +
+
+ + +
+

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

+
+ + + + close alert + +
+
+ diff --git a/cms/urls.py b/cms/urls.py index ebd5e33323..a9a7f0a68a 100644 --- a/cms/urls.py +++ b/cms/urls.py @@ -76,8 +76,8 @@ urlpatterns = ('', # nopep8 url(r'^(?P[^/]+)/(?P[^/]+)/assets/(?P[^/]+)$', 'contentstore.views.asset_index', name='asset_index'), - url(r'^(?P[^/]+)/(?P[^/]+)/assets/remove/(?P.*?)$', - 'contentstore.views.remove_asset', name='remove_asset'), + 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/lib/xmodule/xmodule/contentstore/utils.py b/common/lib/xmodule/xmodule/contentstore/utils.py new file mode 100644 index 0000000000..fadc06c84e --- /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=None): + ''' + 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 From 443d6a382f0e43f18c1b3f6d61e069c8c3542e79 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Thu, 13 Jun 2013 16:06:09 -0400 Subject: [PATCH 11/32] add CHANGELOG entry for work --- CHANGELOG.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 03a3b8c809..84e1bb474b 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -15,6 +15,8 @@ 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 + 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 From 274d8997a21baaf9ed7310f2693eb9ab961ccc3b Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Thu, 13 Jun 2013 16:15:48 -0400 Subject: [PATCH 12/32] update warning message --- cms/static/js/base.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cms/static/js/base.js b/cms/static/js/base.js index 9cb70592cb..aa34f1b7c2 100644 --- a/cms/static/js/base.js +++ b/cms/static/js/base.js @@ -403,7 +403,7 @@ function removeAsset(e) { e.preventDefault(); // replace with new notification moodal - if (!confirm('Are you sure you wish to delete this item. It cannot be reversed!')) return; + if (!confirm('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)')) return; var remove_asset_url = $('.asset-library').data('remove-asset-callback-url'); var location = $(this).closest('tr').data('id'); From 70ee4b80602e0508e91c25c1d034698ef0adacd4 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Fri, 14 Jun 2013 10:47:32 -0400 Subject: [PATCH 13/32] add unit tests for asset delete/restore --- .../contentstore/tests/test_contentstore.py | 108 ++++++++++++++++++ cms/djangoapps/contentstore/views/assets.py | 1 - cms/envs/test.py | 8 +- .../xmodule/modulestore/xml_importer.py | 2 +- 4 files changed, 116 insertions(+), 3 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index 03449fc22f..4e96b6cb5f 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 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,111 @@ 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) + 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) + 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) + + 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) + + # 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 = None + thumbnail = None + try: + content = content_store.find(asset_location) + thumbnail = trash_store.find(content.thumbnail_location) + except NotFoundError: + pass + self.assertIsNone(content) + self.assertIsNone(thumbnail) + + # now try to find it and the thumbnail in trashcan + content = None + thumbnail = None + try: + content = trash_store.find(asset_location) + thumbnail = trash_store.find(content.thumbnail_location) + except NotFoundError: + pass + + # should be in trashcan + self.assertIsNotNone(content) + 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 = None + thumbnail = None + try: + content = content_store.find(asset_location) + thumbnail = trash_store.find(content.thumbnail_location) + except NotFoundError: + pass + self.assertIsNotNone(content) + self.assertIsNotNone(thumbnail) + def test_clone_course(self): course_data = { diff --git a/cms/djangoapps/contentstore/views/assets.py b/cms/djangoapps/contentstore/views/assets.py index 62dee1ba21..400013b59b 100644 --- a/cms/djangoapps/contentstore/views/assets.py +++ b/cms/djangoapps/contentstore/views/assets.py @@ -164,7 +164,6 @@ def remove_asset(request, org, course, name): get_location_and_verify_access(request, org, course, name) location = request.POST['location'] - logging.debug('location = {0}'.format(location)) # make sure the location is valid try: 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/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py index 71c6983644..8d6aa86918 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py @@ -30,7 +30,7 @@ def import_static_content(modules, course_loc, course_data_path, static_content_ try: content_path = os.path.join(dirname, filename) - if verbose: + if True: log.debug('importing static content {0}...'.format(content_path)) fullname_with_subpath = content_path.replace(static_dir, '') # strip away leading path from the name From cd0087ca531c1db822515c5dc6464e5b893f0869 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Fri, 14 Jun 2013 12:39:34 -0400 Subject: [PATCH 14/32] use the new notification tools for the confirmation dialog --- cms/static/js/base.js | 67 ++++++++++++++++++++++++-------- cms/static/js/models/feedback.js | 6 +++ cms/templates/asset_index.html | 5 +++ 3 files changed, 61 insertions(+), 17 deletions(-) diff --git a/cms/static/js/base.js b/cms/static/js/base.js index aa34f1b7c2..ed0bdff8bd 100644 --- a/cms/static/js/base.js +++ b/cms/static/js/base.js @@ -402,24 +402,54 @@ function _deleteItem($el) { function removeAsset(e) { e.preventDefault(); - // replace with new notification moodal - if (!confirm('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)')) return; - - var remove_asset_url = $('.asset-library').data('remove-asset-callback-url'); - var location = $(this).closest('tr').data('id'); var that = this; - $.post(remove_asset_url, - { 'location': location }, - function() { - // show the alert - $(".wrapper-alert-confirmation").addClass("is-shown").attr('aria-hidden','false'); - $(that).closest('tr').remove(); - analytics.track('Deleted Asset', { - 'course': course_location_analytics, - 'id': location - }); - } - ); + 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) { @@ -482,6 +512,9 @@ function displayFinishedUpload(xhr) { 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 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/templates/asset_index.html b/cms/templates/asset_index.html index 0de38510f5..0fcfee0b83 100644 --- a/cms/templates/asset_index.html +++ b/cms/templates/asset_index.html @@ -8,6 +8,11 @@ <%block name="jsextra"> + + <%block name="content"> From b443629ee8c8dd37a14643cd52ed66c636b48583 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Fri, 14 Jun 2013 15:26:37 -0400 Subject: [PATCH 15/32] remove invalid comments --- .../contentstore/management/commands/empty_asset_trashcan.py | 3 --- .../management/commands/restore_asset_from_trashcan.py | 4 ---- 2 files changed, 7 deletions(-) diff --git a/cms/djangoapps/contentstore/management/commands/empty_asset_trashcan.py b/cms/djangoapps/contentstore/management/commands/empty_asset_trashcan.py index c10700c7af..9af3277a2b 100644 --- a/cms/djangoapps/contentstore/management/commands/empty_asset_trashcan.py +++ b/cms/djangoapps/contentstore/management/commands/empty_asset_trashcan.py @@ -1,6 +1,3 @@ -### -### Script for cloning a course -### from django.core.management.base import BaseCommand, CommandError from xmodule.course_module import CourseDescriptor from xmodule.contentstore.utils import empty_asset_trashcan diff --git a/cms/djangoapps/contentstore/management/commands/restore_asset_from_trashcan.py b/cms/djangoapps/contentstore/management/commands/restore_asset_from_trashcan.py index 0a4be40efc..6770bfaf44 100644 --- a/cms/djangoapps/contentstore/management/commands/restore_asset_from_trashcan.py +++ b/cms/djangoapps/contentstore/management/commands/restore_asset_from_trashcan.py @@ -1,9 +1,5 @@ -### -### Script for cloning a course -### from django.core.management.base import BaseCommand, CommandError from xmodule.contentstore.utils import restore_asset_from_trashcan -from xmodule.modulestore import Location class Command(BaseCommand): From ab94b8618c49a16e29d39683eee433acf26db4e9 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Fri, 14 Jun 2013 15:32:02 -0400 Subject: [PATCH 16/32] forgot to internationalize one string --- cms/templates/asset_index.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cms/templates/asset_index.html b/cms/templates/asset_index.html index 0fcfee0b83..0006d29d38 100644 --- a/cms/templates/asset_index.html +++ b/cms/templates/asset_index.html @@ -155,7 +155,7 @@ - close alert + ${_('close alert')} From 7f1768f8f8891f8098a56ad908c0693520232006 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Fri, 14 Jun 2013 15:37:14 -0400 Subject: [PATCH 17/32] put back the verbose switch in xml_importer.py --- common/lib/xmodule/xmodule/modulestore/xml_importer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py index 8d6aa86918..71c6983644 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py @@ -30,7 +30,7 @@ def import_static_content(modules, course_loc, course_data_path, static_content_ try: content_path = os.path.join(dirname, filename) - if True: + if verbose: log.debug('importing static content {0}...'.format(content_path)) fullname_with_subpath = content_path.replace(static_dir, '') # strip away leading path from the name From 7e46447cd5a80b692f405747035118923f5e7dbe Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Fri, 14 Jun 2013 16:08:19 -0400 Subject: [PATCH 18/32] there is no default for empty_asset_trashcan, caller must provide a list --- common/lib/xmodule/xmodule/contentstore/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/lib/xmodule/xmodule/contentstore/utils.py b/common/lib/xmodule/xmodule/contentstore/utils.py index fadc06c84e..74c4242bd9 100644 --- a/common/lib/xmodule/xmodule/contentstore/utils.py +++ b/common/lib/xmodule/xmodule/contentstore/utils.py @@ -3,7 +3,7 @@ from xmodule.contentstore.content import StaticContent from .django import contentstore -def empty_asset_trashcan(course_locs=None): +def empty_asset_trashcan(course_locs): ''' This method will hard delete all assets (optionally within a course_id) from the trashcan ''' From dc94ea10415557214739ce7f38408f1d3052f456 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Fri, 14 Jun 2013 16:19:17 -0400 Subject: [PATCH 19/32] add unit test for emptying the trashcan --- .../contentstore/tests/test_contentstore.py | 46 ++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index 4e96b6cb5f..5b829781c4 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -29,7 +29,7 @@ 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 +from xmodule.contentstore.utils import restore_asset_from_trashcan, empty_asset_trashcan from xmodule.capa_module import CapaDescriptor from xmodule.course_module import CourseDescriptor @@ -490,6 +490,50 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): self.assertIsNotNone(content) 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') + + 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) + + # 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) + 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) + all_thumbnails = trash_store.get_all_content_thumbnails_for_course(course_location) + self.assertEqual(len(all_assets), 0) + self.assertEqual(len(all_thumbnails), 0) + def test_clone_course(self): course_data = { From 7cebb873df947a96a02cb38f7cc465f8c5d040b5 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Fri, 14 Jun 2013 16:35:21 -0400 Subject: [PATCH 20/32] allow for an optional throw_on_not_found on contentstore().find() so we can clean up caller logic in the tests --- .../contentstore/tests/test_contentstore.py | 52 +++++-------------- .../lib/xmodule/xmodule/contentstore/mongo.py | 7 ++- 2 files changed, 19 insertions(+), 40 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index 5b829781c4..a503f22c26 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -435,14 +435,12 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): import_from_xml(module_store, 'common/test/data/', ['full'], static_content_store=content_store) - content = None - try: - location = StaticContent.get_location_from_path('/c4x/edX/full/asset/circuits_duality.gif') - content = content_store.find(location) - except NotFoundError: - pass - + # 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) + self.assertIsNotNone(thumbnail_location) # go through the website to do the delete, since the soft-delete logic is in the view @@ -453,26 +451,14 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): 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 = None - thumbnail = None - try: - content = content_store.find(asset_location) - thumbnail = trash_store.find(content.thumbnail_location) - except NotFoundError: - pass + content = content_store.find(asset_location, throw_on_not_found=False) + thumbnail = content_store.find(thumbnail_location, throw_on_not_found=False) self.assertIsNone(content) self.assertIsNone(thumbnail) - # now try to find it and the thumbnail in trashcan - content = None - thumbnail = None - try: - content = trash_store.find(asset_location) - thumbnail = trash_store.find(content.thumbnail_location) - except NotFoundError: - pass - - # should be in trashcan + # 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) + thumbnail = trash_store.find(thumbnail_location, throw_on_not_found=False) self.assertIsNotNone(content) self.assertIsNotNone(thumbnail) @@ -480,13 +466,8 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): 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 = None - thumbnail = None - try: - content = content_store.find(asset_location) - thumbnail = trash_store.find(content.thumbnail_location) - except NotFoundError: - pass + content = content_store.find(asset_location, throw_on_not_found=False) + thumbnail = content_store.find(thumbnail_location, throw_on_not_found=False) self.assertIsNotNone(content) self.assertIsNotNone(thumbnail) @@ -502,13 +483,8 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): course_location = CourseDescriptor.id_to_location('edX/full/6.002_Spring_2012') - content = None - try: - location = StaticContent.get_location_from_path('/c4x/edX/full/asset/circuits_duality.gif') - content = content_store.find(location) - except NotFoundError: - pass - + 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 diff --git a/common/lib/xmodule/xmodule/contentstore/mongo.py b/common/lib/xmodule/xmodule/contentstore/mongo.py index 7d96e132ee..fa0fc95181 100644 --- a/common/lib/xmodule/xmodule/contentstore/mongo.py +++ b/common/lib/xmodule/xmodule/contentstore/mongo.py @@ -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) From 16e476e8e4fae72771bcea117fed41429c206473 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Mon, 17 Jun 2013 10:34:09 -0400 Subject: [PATCH 21/32] refactored asset page related JS into it's own page --- cms/static/js/base.js | 128 --------------------------------- cms/static/js/views/assets.js | 128 +++++++++++++++++++++++++++++++++ cms/templates/asset_index.html | 1 + 3 files changed, 129 insertions(+), 128 deletions(-) create mode 100644 cms/static/js/views/assets.js diff --git a/cms/static/js/base.js b/cms/static/js/base.js index ed0bdff8bd..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,9 +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); - $('.remove-asset-button').bind('click', removeAsset); - $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); @@ -399,129 +394,6 @@ function _deleteItem($el) { }); } -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 - }); - -} - function markAsLoaded() { $('.upload-modal .copy-button').css('display', 'inline-block'); $('.upload-modal .progress-bar').addClass('loaded'); 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/templates/asset_index.html b/cms/templates/asset_index.html index 0006d29d38..e8dc523ba7 100644 --- a/cms/templates/asset_index.html +++ b/cms/templates/asset_index.html @@ -8,6 +8,7 @@ <%block name="jsextra"> +
Name Date Added URL
+ +