Add sorting to Studio's Files & Uploads page

CHANGELOG.rst

@@ -5,6 +5,9 @@ These are notable changes in edx-platform.  This is a rolling list of changes,
 in roughly chronological order, most recent first.  Add your entries at or near
 the top.  Include a label indicating the component affected.
 
+Studio: Add sorting by column to the Files & Uploads page.
+See mongo_indexes.md for new indices that should be added.
+
 Studio: Newly-created courses default to being published on Jan 1, 2030
 
 Studio: Added pagination to the Files & Uploads page.

cms/djangoapps/contentstore/tests/test_assets.py

@@ -85,6 +85,10 @@ def test_json_responses(self):
         self.assert_correct_asset_response(self.url, 0, 3, 3)
         self.assert_correct_asset_response(self.url + "?page_size=2", 0, 2, 3)
         self.assert_correct_asset_response(self.url + "?page_size=2&page=1", 2, 1, 3)
+        self.assert_correct_sort_response(self.url, 'date_added', 'asc')
+        self.assert_correct_sort_response(self.url, 'date_added', 'desc')
+        self.assert_correct_sort_response(self.url, 'display_name', 'asc')
+        self.assert_correct_sort_response(self.url, 'display_name', 'desc')
 
         # Verify querying outside the range of valid pages
         self.assert_correct_asset_response(self.url + "?page_size=2&page=-1", 0, 2, 3)
@@ -99,6 +103,19 @@ def assert_correct_asset_response(self, url, expected_start, expected_length, ex
         self.assertEquals(len(assets), expected_length)
         self.assertEquals(json_response['totalCount'], expected_total)
 
+    def assert_correct_sort_response(self, url, sort, direction):
+        resp = self.client.get(url + '?sort=' + sort + '&direction=' + direction, HTTP_ACCEPT='application/json')
+        json_response = json.loads(resp.content)
+        assets = json_response['assets']
+        name1 = assets[0][sort]
+        name2 = assets[1][sort]
+        name3 = assets[2][sort]
+        if direction == 'asc':
+            self.assertLessEqual(name1, name2)
+            self.assertLessEqual(name2, name3)
+        else:
+            self.assertGreaterEqual(name1, name2)
+            self.assertGreaterEqual(name2, name3)
 
 class UploadTestCase(AssetsTestCase):
     """

cms/djangoapps/contentstore/tests/test_contentstore.py

@@ -176,7 +176,7 @@ def _lock_an_asset(self, content_store, course_location):
         Lock an arbitrary asset in the course
         :param course_location:
         """
-        course_assets,__ = content_store.get_all_content_for_course(course_location)
+        course_assets, __ = content_store.get_all_content_for_course(course_location)
         self.assertGreater(len(course_assets), 0, "No assets to lock")
         content_store.set_attr(course_assets[0]['_id'], 'locked', True)
         return course_assets[0]['_id']
@@ -585,7 +585,7 @@ def test_asset_import(self):
         self.assertIsNotNone(course)
 
         # make sure we have some assets in our contentstore
-        all_assets,__ = content_store.get_all_content_for_course(course_location)
+        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
@@ -698,7 +698,7 @@ def test_empty_trashcan(self):
 
         # make sure there's something in the trashcan
         course_location = CourseDescriptor.id_to_location('edX/toy/6.002_Spring_2012')
-        all_assets,__ = trash_store.get_all_content_for_course(course_location)
+        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
@@ -713,7 +713,7 @@ def test_empty_trashcan(self):
         empty_asset_trashcan([course_location])
 
         # make sure trashcan is empty
-        all_assets,count = trash_store.get_all_content_for_course(course_location)
+        all_assets, count = trash_store.get_all_content_for_course(course_location)
         self.assertEqual(len(all_assets), 0)
         self.assertEqual(count, 0)
 
@@ -924,7 +924,7 @@ def test_delete_course(self):
         self.assertEqual(len(items), 0)
 
         # assert that all content in the asset library is also deleted
-        assets,count = content_store.get_all_content_for_course(location)
+        assets, count = content_store.get_all_content_for_course(location)
         self.assertEqual(len(assets), 0)
         self.assertEqual(count, 0)
 

cms/djangoapps/contentstore/tests/test_import_nostatic.py

@@ -84,7 +84,7 @@ def test_static_import(self):
         _, content_store, course, course_location = self.load_test_import_course()
 
         # make sure we have ONE asset in our contentstore ("should_be_imported.html")
-        all_assets,count = content_store.get_all_content_for_course(course_location)
+        all_assets, count = content_store.get_all_content_for_course(course_location)
         print "len(all_assets)=%d" % len(all_assets)
         self.assertEqual(len(all_assets), 1)
         self.assertEqual(count, 1)
@@ -115,8 +115,7 @@ def test_asset_import_nostatic(self):
         module_store.get_item(course_location)
 
         # make sure we have NO assets in our contentstore
-        all_assets,count = content_store.get_all_content_for_course(course_location)
-        print "len(all_assets)=%d" % len(all_assets)
+        all_assets, count = content_store.get_all_content_for_course(course_location)
         self.assertEqual(len(all_assets), 0)
         self.assertEqual(count, 0)
 

cms/djangoapps/contentstore/views/assets.py

@@ -1,5 +1,7 @@
 import logging
 from functools import partial
+import math
+import json
 
 from django.http import HttpResponseBadRequest
 from django.contrib.auth.decorators import login_required
@@ -24,10 +26,8 @@
 
 from util.json_request import JsonResponse
 from django.http import HttpResponseNotFound
-import json
 from django.utils.translation import ugettext as _
-from pymongo import DESCENDING
-import math
+from pymongo import ASCENDING, DESCENDING
 
 __all__ = ['assets_handler']
 
@@ -41,10 +41,13 @@ def assets_handler(request, tag=None, package_id=None, branch=None, version_guid
     deleting assets, and changing the "locked" state of an asset.
 
     GET
-        html: return html page which will show all course assets. Note that only the asset container
+        html: return an html page which will show all course assets. Note that only the asset container
             is returned and that the actual assets are filled in with a client-side request.
-        json: returns a page of assets. A page parameter specifies the desired page, and the
-            optional page_size parameter indicates the number of items per page (defaults to 50).
+        json: returns a page of assets. The following parameters are supported:
+            page: the desired page of results (defaults to 0)
+            page_size: the number of items per page (defaults to 50)
+            sort: the asset field to sort by (defaults to "date_added")
+            direction: the sort direction (defaults to "descending")
     POST
         json: create (or update?) an asset. The only updating that can be done is changing the lock state.
     PUT
@@ -91,7 +94,17 @@ def _assets_json(request, location):
     """
     requested_page = int(request.REQUEST.get('page', 0))
     requested_page_size = int(request.REQUEST.get('page_size', 50))
-    sort = [('uploadDate', DESCENDING)]
+    requested_sort = request.REQUEST.get('sort', 'date_added')
+    sort_direction = DESCENDING
+    if request.REQUEST.get('direction', '').lower() == 'asc':
+        sort_direction = ASCENDING
+
+    # Convert the field name to the Mongo name
+    if requested_sort == 'date_added':
+        requested_sort = 'uploadDate'
+    elif requested_sort == 'display_name':
+        requested_sort = 'displayname'
+    sort = [(requested_sort, sort_direction)]
 
     current_page = max(requested_page, 0)
     start = current_page * requested_page_size
@@ -122,7 +135,8 @@ def _assets_json(request, location):
         'page': current_page,
         'pageSize': requested_page_size,
         'totalCount': total_count,
-        'assets': asset_json
+        'assets': asset_json,
+        'sort': requested_sort,
     })
 
 

cms/static/coffee/spec/views/assets_spec.coffee

@@ -198,21 +198,51 @@ define ["jasmine", "js/spec/create_sinon", "squire"],
             @injector.clean()
             @injector.remove()
 
+        addMockAsset = (requests) ->
+            model = new @AssetModel
+                display_name: "new asset"
+                url: 'new_actual_asset_url'
+                portable_url: 'portable_url'
+                date_added: 'date'
+                thumbnail: null
+                id: 'idx'
+            @view.addAsset(model)
+            create_sinon.respondWithJson(requests,
+                {
+                    assets: [
+                        @mockAsset1, @mockAsset2,
+                        {
+                            display_name: "new asset"
+                            url: 'new_actual_asset_url'
+                            portable_url: 'portable_url'
+                            date_added: 'date'
+                            thumbnail: null
+                            id: 'idx'
+                        }
+                    ],
+                    start: 0,
+                    end: 2,
+                    page: 0,
+                    pageSize: 5,
+                    totalCount: 3
+                })
+
+
         describe "Basic", ->
             # Separate setup method to work-around mis-parenting of beforeEach methods
-            setup = (response) ->
+            setup = ->
                 requests = create_sinon.requests(this)
                 @view.setPage(0)
-                create_sinon.respondWithJson(requests, response)
+                create_sinon.respondWithJson(requests, @mockAssetsResponse)
                 return requests
 
             it "should render both assets", ->
-                requests = setup.call(this, @mockAssetsResponse)
+                requests = setup.call(this)
                 expect(@view.$el).toContainText("test asset 1")
                 expect(@view.$el).toContainText("test asset 2")
 
             it "should remove the deleted asset from the view", ->
-                requests = setup.call(this, @mockAssetsResponse)
+                requests = setup.call(this)
                 # Delete the 2nd asset with success from server.
                 @view.$(".remove-asset-button")[1].click()
                 @promptSpies.constructor.mostRecentCall.args[0].actions.primary.click(@promptSpies)
@@ -221,7 +251,7 @@ define ["jasmine", "js/spec/create_sinon", "squire"],
                 expect(@view.$el).not.toContainText("test asset 2")
 
             it "does not remove asset if deletion failed", ->
-                requests = setup.call(this, @mockAssetsResponse)
+                requests = setup.call(this)
                 # Delete the 2nd asset, but mimic a failure from the server.
                 @view.$(".remove-asset-button")[1].click()
                 @promptSpies.constructor.mostRecentCall.args[0].actions.primary.click(@promptSpies)
@@ -230,39 +260,60 @@ define ["jasmine", "js/spec/create_sinon", "squire"],
                 expect(@view.$el).toContainText("test asset 2")
 
             it "adds an asset if asset does not already exist", ->
-                requests = setup.call(this, @mockAssetsResponse)
-                model = new @AssetModel
-                    display_name: "new asset"
-                    url: 'new_actual_asset_url'
-                    portable_url: 'portable_url'
-                    date_added: 'date'
-                    thumbnail: null
-                    id: 'idx'
-                @view.addAsset(model)
-                create_sinon.respondWithJson(requests,
-                    {
-                        assets: [ @mockAsset1, @mockAsset2,
-                            {
-                                display_name: "new asset"
-                                url: 'new_actual_asset_url'
-                                portable_url: 'portable_url'
-                                date_added: 'date'
-                                thumbnail: null
-                                id: 'idx'
-                            }
-                        ],
-                        start: 0,
-                        end: 2,
-                        page: 0,
-                        pageSize: 5,
-                        totalCount: 3
-                    })
+                requests = setup.call(this)
+                addMockAsset.call(this, requests)
                 expect(@view.$el).toContainText("new asset")
                 expect(@collection.models.length).toBe(3)
 
             it "does not add an asset if asset already exists", ->
-                setup.call(this, @mockAssetsResponse)
+                setup.call(this)
                 spyOn(@collection, "add").andCallThrough()
                 model = @collection.models[1]
                 @view.addAsset(model)
                 expect(@collection.add).not.toHaveBeenCalled()
+
+        describe "Sorting", ->
+            # Separate setup method to work-around mis-parenting of beforeEach methods
+            setup = ->
+                requests = create_sinon.requests(this)
+                @view.setPage(0)
+                create_sinon.respondWithJson(requests, @mockAssetsResponse)
+                return requests
+
+            it "should have the correct default sort order", ->
+                requests = setup.call(this)
+                expect(@view.sortDisplayName()).toBe("Date Added")
+                expect(@view.collection.sortDirection).toBe("desc")
+
+            it "should toggle the sort order when clicking on the currently sorted column", ->
+                requests = setup.call(this)
+                expect(@view.sortDisplayName()).toBe("Date Added")
+                expect(@view.collection.sortDirection).toBe("desc")
+                @view.$("#js-asset-date-col").click()
+                create_sinon.respondWithJson(requests, @mockAssetsResponse)
+                expect(@view.sortDisplayName()).toBe("Date Added")
+                expect(@view.collection.sortDirection).toBe("asc")
+                @view.$("#js-asset-date-col").click()
+                create_sinon.respondWithJson(requests, @mockAssetsResponse)
+                expect(@view.sortDisplayName()).toBe("Date Added")
+                expect(@view.collection.sortDirection).toBe("desc")
+
+            it "should switch the sort order when clicking on a different column", ->
+                requests = setup.call(this)
+                @view.$("#js-asset-name-col").click()
+                create_sinon.respondWithJson(requests, @mockAssetsResponse)
+                expect(@view.sortDisplayName()).toBe("Name")
+                expect(@view.collection.sortDirection).toBe("asc")
+                @view.$("#js-asset-name-col").click()
+                create_sinon.respondWithJson(requests, @mockAssetsResponse)
+                expect(@view.sortDisplayName()).toBe("Name")
+                expect(@view.collection.sortDirection).toBe("desc")
+
+            it "should switch sort to most recent date added when a new asset is added", ->
+                requests = setup.call(this)
+                @view.$("#js-asset-name-col").click()
+                create_sinon.respondWithJson(requests, @mockAssetsResponse)
+                addMockAsset.call(this, requests)
+                create_sinon.respondWithJson(requests, @mockAssetsResponse)
+                expect(@view.sortDisplayName()).toBe("Date Added")
+                expect(@view.collection.sortDirection).toBe("desc")

cms/static/js/collections/asset.js

@@ -15,6 +15,8 @@ define(["backbone.paginator", "js/models/asset"], function(BackbonePaginator, As
         server_api: {
             'page': function() { return this.currentPage; },
             'page_size': function() { return this.perPage; },
+            'sort': function() { return this.sortField; },
+            'direction': function() { return this.sortDirection; },
             'format': 'json'
         },