diff --git a/cms/djangoapps/contentstore/tests/test_assets.py b/cms/djangoapps/contentstore/tests/test_assets.py index 2f158cfda6..7dd266b3eb 100644 --- a/cms/djangoapps/contentstore/tests/test_assets.py +++ b/cms/djangoapps/contentstore/tests/test_assets.py @@ -2,7 +2,10 @@ Unit tests for the asset upload endpoint. """ -import json +#pylint: disable=C0111 +#pylint: disable=W0621 +#pylint: disable=W0212 + from datetime import datetime from io import BytesIO from pytz import UTC @@ -12,7 +15,9 @@ from django.core.urlresolvers import reverse from contentstore.views import assets from xmodule.contentstore.content import StaticContent from xmodule.modulestore import Location - +from xmodule.contentstore.django import contentstore +from xmodule.modulestore.django import modulestore +from xmodule.modulestore.xml_importer import import_from_xml class AssetsTestCase(CourseTestCase): def setUp(self): @@ -27,22 +32,27 @@ class AssetsTestCase(CourseTestCase): resp = self.client.get(self.url) self.assertEquals(resp.status_code, 200) - def test_json(self): - resp = self.client.get( - self.url, - HTTP_ACCEPT="application/json", - HTTP_X_REQUESTED_WITH="XMLHttpRequest", - ) - self.assertEquals(resp.status_code, 200) - content = json.loads(resp.content) - self.assertIsInstance(content, list) - def test_static_url_generation(self): location = Location(['i4x', 'foo', 'bar', 'asset', 'my_file_name.jpg']) path = StaticContent.get_static_path_from_location(location) self.assertEquals(path, '/static/my_file_name.jpg') +class AssetsToyCourseTestCase(CourseTestCase): + """ + Tests the assets returned from asset_index for the toy test course. + """ + def test_toy_assets(self): + module_store = modulestore('direct') + import_from_xml(module_store, 'common/test/data/', ['toy'], static_content_store=contentstore(), verbose=True) + url = reverse("asset_index", kwargs={'org': 'edX', 'course': 'toy', 'name': '2012_Fall'}) + + resp = self.client.get(url) + # Test a small portion of the asset data passed to the client. + self.assertContains(resp, "new CMS.Models.AssetCollection([{") + self.assertContains(resp, "/c4x/edX/toy/asset/handouts_sample_handout.txt") + + class UploadTestCase(CourseTestCase): """ Unit tests for uploading a file @@ -71,32 +81,25 @@ class UploadTestCase(CourseTestCase): self.assertEquals(resp.status_code, 405) -class AssetsToJsonTestCase(TestCase): +class AssetToJsonTestCase(TestCase): """ - Unit tests for transforming the results of a database call into something + Unit test for transforming asset information into something we can send out to the client via JSON. """ def test_basic(self): upload_date = datetime(2013, 6, 1, 10, 30, tzinfo=UTC) - asset = { - "displayname": "foo", - "chunkSize": 512, - "filename": "foo.png", - "length": 100, - "uploadDate": upload_date, - "_id": { - "course": "course", - "org": "org", - "revision": 12, - "category": "category", - "name": "name", - "tag": "tag", - } - } - output = assets.assets_to_json_dict([asset]) - self.assertEquals(len(output), 1) - compare = output[0] - self.assertEquals(compare["name"], "foo") - self.assertEquals(compare["path"], "foo.png") - self.assertEquals(compare["uploaded"], upload_date.isoformat()) - self.assertEquals(compare["id"], "/tag/org/course/12/category/name") + + location = Location(['i4x', 'foo', 'bar', 'asset', 'my_file_name.jpg']) + thumbnail_location = Location(['i4x', 'foo', 'bar', 'asset', 'my_file_name_thumb.jpg']) + + output = assets._get_asset_json("my_file", upload_date, location, thumbnail_location) + + self.assertEquals(output["display_name"], "my_file") + self.assertEquals(output["date_added"], "Jun 01, 2013 at 10:30 UTC") + self.assertEquals(output["url"], "/i4x/foo/bar/asset/my_file_name.jpg") + self.assertEquals(output["portable_url"], "/static/my_file_name.jpg") + self.assertEquals(output["thumbnail"], "/i4x/foo/bar/asset/my_file_name_thumb.jpg") + self.assertEquals(output["id"], output["url"]) + + output = assets._get_asset_json("name", upload_date, location, None) + self.assertIsNone(output["thumbnail"]) diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index 9d39c2a443..74c1c3367c 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -593,9 +593,9 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): # 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': 'toy', 'name': '2012_Fall'}) - resp = self.client.post(url, {'location': '/c4x/edX/toy/asset/sample_static.txt'}) - self.assertEqual(resp.status_code, 200) + url = reverse('update_asset', kwargs={'org': 'edX', 'course': 'toy', 'name': '2012_Fall', 'asset_id': '/c4x/edX/toy/asset/sample_static.txt'}) + resp = self.client.delete(url) + self.assertEqual(resp.status_code, 204) asset_location = StaticContent.get_location_from_path('/c4x/edX/toy/asset/sample_static.txt') @@ -628,7 +628,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): def test_empty_trashcan(self): ''' - This test will exercise the empting of the asset trashcan + This test will exercise the emptying of the asset trashcan ''' content_store = contentstore() trash_store = contentstore('trashcan') @@ -644,9 +644,9 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): # 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': 'toy', 'name': '2012_Fall'}) - resp = self.client.post(url, {'location': '/c4x/edX/toy/asset/sample_static.txt'}) - self.assertEqual(resp.status_code, 200) + url = reverse('update_asset', kwargs={'org': 'edX', 'course': 'toy', 'name': '2012_Fall', 'asset_id': '/c4x/edX/toy/asset/sample_static.txt'}) + resp = self.client.delete(url) + self.assertEqual(resp.status_code, 204) # make sure there's something in the trashcan all_assets = trash_store.get_all_content_for_course(course_location) diff --git a/cms/djangoapps/contentstore/views/assets.py b/cms/djangoapps/contentstore/views/assets.py index 4743622fa8..b1fb148300 100644 --- a/cms/djangoapps/contentstore/views/assets.py +++ b/cms/djangoapps/contentstore/views/assets.py @@ -1,76 +1,33 @@ import logging -import json -import os -import tarfile -import shutil -import cgi -import re from functools import partial -from tempfile import mkdtemp -from path import path -from django.conf import settings -from django.http import HttpResponse, HttpResponseBadRequest +from django.http import HttpResponseBadRequest from django.contrib.auth.decorators import login_required +from django.views.decorators.http import require_http_methods from django_future.csrf import ensure_csrf_cookie from django.core.urlresolvers import reverse -from django.core.servers.basehttp import FileWrapper -from django.core.files.temp import NamedTemporaryFile -from django.views.decorators.http import require_POST, require_http_methods +from django.views.decorators.http import require_POST from mitxmako.shortcuts import render_to_response from cache_toolbox.core import del_cached_content from auth.authz import create_all_course_groups -from xmodule.modulestore.xml_importer import import_from_xml from xmodule.contentstore.django import contentstore -from xmodule.modulestore.xml_exporter import export_to_xml 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, SerializationError +from xmodule.exceptions import NotFoundError from .access import get_location_and_verify_access from util.json_request import JsonResponse +import json +from django.utils.translation import ugettext as _ __all__ = ['asset_index', 'upload_asset'] -def assets_to_json_dict(assets): - """ - Transform the results of a contentstore query into something appropriate - for output via JSON. - """ - ret = [] - for asset in assets: - obj = { - "name": asset.get("displayname", ""), - "chunkSize": asset.get("chunkSize", 0), - "path": asset.get("filename", ""), - "length": asset.get("length", 0), - } - uploaded = asset.get("uploadDate") - if uploaded: - obj["uploaded"] = uploaded.isoformat() - thumbnail = asset.get("thumbnail_location") - if thumbnail: - obj["thumbnail"] = thumbnail - id_info = asset.get("_id") - if id_info: - obj["id"] = "/{tag}/{org}/{course}/{revision}/{category}/{name}" \ - .format( - org=id_info.get("org", ""), - course=id_info.get("course", ""), - revision=id_info.get("revision", ""), - tag=id_info.get("tag", ""), - category=id_info.get("category", ""), - name=id_info.get("name", ""), - ) - ret.append(obj) - return ret - @login_required @ensure_csrf_cookie @@ -96,32 +53,21 @@ def asset_index(request, org, course, name): # sort in reverse upload date order assets = sorted(assets, key=lambda asset: asset['uploadDate'], reverse=True) - if request.META.get('HTTP_ACCEPT', "").startswith("application/json"): - return JsonResponse(assets_to_json_dict(assets)) - - asset_display = [] + asset_json = [] for asset in assets: asset_id = asset['_id'] - display_info = {} - display_info['displayname'] = asset['displayname'] - display_info['uploadDate'] = get_default_time_display(asset['uploadDate']) - asset_location = StaticContent.compute_location(asset_id['org'], asset_id['course'], asset_id['name']) - display_info['url'] = StaticContent.get_url_path_from_location(asset_location) - display_info['portable_url'] = StaticContent.get_static_path_from_location(asset_location) - # note, due to the schema change we may not have a 'thumbnail_location' in the result set _thumbnail_location = asset.get('thumbnail_location', None) thumbnail_location = Location(_thumbnail_location) if _thumbnail_location is not None else None - display_info['thumb_url'] = StaticContent.get_url_path_from_location(thumbnail_location) if thumbnail_location is not None else None - asset_display.append(display_info) + asset_json.append(_get_asset_json(asset['displayname'], asset['uploadDate'], asset_location, thumbnail_location)) return render_to_response('asset_index.html', { 'context_course': course_module, - 'assets': asset_display, + 'asset_list': json.dumps(asset_json), 'upload_asset_callback_url': upload_asset_callback_url, - 'remove_asset_callback_url': reverse('remove_asset', kwargs={ + 'update_asset_callback_url': reverse('update_asset', kwargs={ 'org': org, 'course': course, 'name': name @@ -171,9 +117,6 @@ def upload_asset(request, org, course, coursename): content = sc_partial(upload_file.read()) tempfile_path = None - thumbnail_content = None - thumbnail_location = None - # first let's see if a thumbnail can be created (thumbnail_content, thumbnail_location) = contentstore().generate_thumbnail( content, @@ -195,46 +138,38 @@ def upload_asset(request, org, course, coursename): readback = contentstore().find(content.location) response_payload = { - 'displayname': content.name, - 'uploadDate': get_default_time_display(readback.last_modified_at), - 'url': StaticContent.get_url_path_from_location(content.location), - 'portable_url': StaticContent.get_static_path_from_location(content.location), - 'thumb_url': StaticContent.get_url_path_from_location(thumbnail_location) - if thumbnail_content is not None else None, - 'msg': 'Upload completed' + 'asset': _get_asset_json(content.name, readback.last_modified_at, content.location, content.thumbnail_location), + 'msg': _('Upload completed') } - response = JsonResponse(response_payload) - return response + return JsonResponse(response_payload) -@ensure_csrf_cookie +@require_http_methods(("DELETE",)) @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) +@ensure_csrf_cookie +def update_asset(request, org, course, name, asset_id): + """ + restful CRUD operations for a course asset. + Currently only the DELETE method is implemented. - location = request.POST['location'] + org, course, name: Attributes of the Location for the item to edit + asset_id: the URL of the asset (used by Backbone as the id) + """ + get_location_and_verify_access(request, org, course, name) # make sure the location is valid try: - loc = StaticContent.get_location_from_path(location) - except InvalidLocationError: + loc = StaticContent.get_location_from_path(asset_id) + except InvalidLocationError as err: # return a 'Bad Request' to browser as we have a malformed Location - response = HttpResponse() - response.status_code = 400 - return response + return JsonResponse({"error": err.message}, status=400) # also make sure the item to delete actually exists try: content = contentstore().find(loc) except NotFoundError: - response = HttpResponse() - response.status_code = 404 - return response + return JsonResponse(status=404) # ok, save the content into the trashcan contentstore('trashcan').save(content) @@ -249,13 +184,26 @@ def remove_asset(request, org, course, name): # remove from any caching del_cached_content(thumbnail_content.location) except: - pass # OK if this is left dangling + logging.warning('Could not delete thumbnail: ' + content.thumbnail_location) # delete the original contentstore().delete(content.get_id()) # remove from cache del_cached_content(content.location) - - return HttpResponse() + return JsonResponse() +def _get_asset_json(display_name, date, location, thumbnail_location): + """ + Helper method for formatting the asset information to send to client. + """ + asset_url = StaticContent.get_url_path_from_location(location) + return { + 'display_name': display_name, + 'date_added': get_default_time_display(date), + 'url': asset_url, + 'portable_url': StaticContent.get_static_path_from_location(location), + 'thumbnail': StaticContent.get_url_path_from_location(thumbnail_location) if thumbnail_location is not None else None, + # Needed for Backbone delete/update. + 'id': asset_url + } diff --git a/cms/envs/common.py b/cms/envs/common.py index ef3c8654d4..38913f537a 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -254,8 +254,10 @@ PIPELINE_JS = { 'js/models/metadata_model.js', 'js/views/metadata_editor_view.js', 'js/models/uploads.js', 'js/views/uploads.js', 'js/models/textbook.js', 'js/views/textbook.js', - 'js/views/assets.js', 'js/src/utility.js', - 'js/models/settings/course_grading_policy.js'], + 'js/src/utility.js', + 'js/models/settings/course_grading_policy.js', + 'js/models/asset.js', 'js/models/assets.js', + 'js/views/assets_view.js', 'js/views/asset_view.js'], 'output_filename': 'js/cms-application.js', 'test_order': 0 }, diff --git a/cms/static/coffee/spec/views/assets_spec.coffee b/cms/static/coffee/spec/views/assets_spec.coffee new file mode 100644 index 0000000000..f4f67f682c --- /dev/null +++ b/cms/static/coffee/spec/views/assets_spec.coffee @@ -0,0 +1,135 @@ +feedbackTpl = readFixtures('system-feedback.underscore') +assetTpl = readFixtures('asset.underscore') + +describe "CMS.Views.Asset", -> + beforeEach -> + setFixtures($(" + + <%block name="jsextra"> - - - + + + + + <%block name="content"> - -

@@ -62,7 +136,7 @@
-
+
@@ -73,31 +147,8 @@ - - % for asset in assets: - - - - - - - - % endfor + +
-
- % if asset['thumb_url'] is not None: - - % endif -
-
- ${asset['displayname']} -
-
- ${asset['uploadDate']} - - - - -