Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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: |<location>|")

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)
Original file line number Diff line number Diff line change
@@ -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: <location>")

restore_asset_from_trashcan(args[0])

128 changes: 128 additions & 0 deletions cms/djangoapps/contentstore/tests/test_contentstore.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,16 @@
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
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
Expand Down Expand Up @@ -382,6 +385,131 @@ def test_remove_hide_progress_tab(self):
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)

# 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

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)
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 - 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)

# 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)
thumbnail = content_store.find(thumbnail_location, throw_on_not_found=False)
self.assertIsNotNone(content)
self.assertIsNotNone(thumbnail)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's remove_asset's behavior if the asset doesn't exist (or was already deleted)? Should you test using a garbage asset url? (Use case: 2 users try to delete same asset w/o reloading screen)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove_asset will return a HTTP 404 to the caller

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a test? Not crucial. I'm thinking of situations where such queries end up deleting everything or the wrong thing, but testing for that would be a pain and that outcome is very unlikely. (It's not like the client is sending a cursor back.)

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)
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 = {
Expand Down
62 changes: 61 additions & 1 deletion cms/djangoapps/contentstore/views/assets.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may want to return a "204 No Content" HTTP status code.



@ensure_csrf_cookie
@login_required
def import_course(request, org, course, name):
Expand Down
3 changes: 2 additions & 1 deletion cms/envs/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,8 @@
) + ['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
},
Expand Down
7 changes: 6 additions & 1 deletion cms/envs/dev.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,15 @@
'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',
Expand Down
8 changes: 7 additions & 1 deletion cms/envs/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,13 @@
'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'
}
}
}

Expand Down
Loading