From fe6ed085189a605085b06dd80d027a4c13c8aebd Mon Sep 17 00:00:00 2001
From: cahrens <christina@edx.org>
Date: Thu, 19 Sep 2013 17:09:02 -0400
Subject: [PATCH] Code review feedback.

---
 cms/djangoapps/contentstore/views/assets.py | 65 ++++++++++-----------
 cms/static/js/views/assets_view.js          |  2 +-
 cms/urls.py                                 |  2 +-
 3 files changed, 34 insertions(+), 35 deletions(-)

diff --git a/cms/djangoapps/contentstore/views/assets.py b/cms/djangoapps/contentstore/views/assets.py
index 67cfcc447e..89c73d3299 100644
--- a/cms/djangoapps/contentstore/views/assets.py
+++ b/cms/djangoapps/contentstore/views/assets.py
@@ -145,7 +145,7 @@ def upload_asset(request, org, course, coursename):
     return JsonResponse(response_payload)
 
 
-@require_http_methods("DELETE")
+@require_http_methods(("DELETE",))
 @login_required
 @ensure_csrf_cookie
 def update_asset(request, org, course, name, asset_id):
@@ -158,40 +158,39 @@ def update_asset(request, org, course, name, asset_id):
     """
     get_location_and_verify_access(request, org, course, name)
 
-    if request.method == 'DELETE':
-        # make sure the location is valid
+    # make sure the location is valid
+    try:
+        loc = StaticContent.get_location_from_path(asset_id)
+    except InvalidLocationError as err:
+        # return a 'Bad Request' to browser as we have a malformed Location
+        return JsonResponse({"error": err.message}, status=400)
+
+    # also make sure the item to delete actually exists
+    try:
+        content = contentstore().find(loc)
+    except NotFoundError:
+        return JsonResponse(status=404)
+
+    # 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:
-            loc = StaticContent.get_location_from_path(asset_id)
-        except InvalidLocationError as err:
-            # return a 'Bad Request' to browser as we have a malformed Location
-            return JsonResponse({"error": err.message}, status=400)
+            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
 
-        # also make sure the item to delete actually exists
-        try:
-            content = contentstore().find(loc)
-        except NotFoundError:
-            return JsonResponse(status=404)
-
-        # 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 JsonResponse()
+    # delete the original
+    contentstore().delete(content.get_id())
+    # remove from cache
+    del_cached_content(content.location)
+    return JsonResponse()
 
 
 def _get_asset_json(display_name, date, location, thumbnail_location):
diff --git a/cms/static/js/views/assets_view.js b/cms/static/js/views/assets_view.js
index bcc41d8f49..3acdb09673 100644
--- a/cms/static/js/views/assets_view.js
+++ b/cms/static/js/views/assets_view.js
@@ -10,7 +10,7 @@ CMS.Views.Assets = Backbone.View.extend({
         this.$el.empty();
 
         var self = this;
-        _.each(this.collection.models,
+        this.collection.each(
             function(asset) {
                 var view = new CMS.Views.Asset({model: asset});
                 self.$el.append(view.render().el);
diff --git a/cms/urls.py b/cms/urls.py
index 147536c84f..1f7da09a8f 100644
--- a/cms/urls.py
+++ b/cms/urls.py
@@ -76,7 +76,7 @@ urlpatterns = ('',  # nopep8
 
     url(r'^(?P<org>[^/]+)/(?P<course>[^/]+)/assets/(?P<name>[^/]+)$',
         'contentstore.views.asset_index', name='asset_index'),
-    url(r'^(?P<org>[^/]+)/(?P<course>[^/]+)/assets/(?P<name>[^/]+)/update/(?P<asset_id>.+)?.*$',
+    url(r'^(?P<org>[^/]+)/(?P<course>[^/]+)/assets/(?P<name>[^/]+)/(?P<asset_id>.+)?.*$',
         'contentstore.views.assets.update_asset', name='update_asset'),
     url(r'^(?P<org>[^/]+)/(?P<course>[^/]+)/textbooks/(?P<name>[^/]+)$',
         'contentstore.views.textbook_index', name='textbook_index'),