Commit 93675304 by Andy Armstrong Committed by Brian Talbot

Add sorting to Studio's Files & Uploads page

Added sorting to the new pagination logic for STUD-995.
parent 5d63cd0e
......@@ -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.
......
......@@ -85,6 +85,10 @@ class PaginationTestCase(AssetsTestCase):
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 @@ class PaginationTestCase(AssetsTestCase):
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):
"""
......
......@@ -176,7 +176,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
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 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
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 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
# 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 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
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 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
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)
......
......@@ -84,7 +84,7 @@ class ContentStoreImportNoStaticTest(ModuleStoreTestCase):
_, 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 @@ class ContentStoreImportNoStaticTest(ModuleStoreTestCase):
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)
......
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 xmodule.modulestore.locator import BlockUsageLocator
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,
})
......
......@@ -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")
......@@ -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'
},
......
......@@ -4,11 +4,18 @@ define(["js/views/paging", "js/views/asset", "js/views/paging_header", "js/views
var AssetsView = PagingView.extend({
// takes AssetCollection as model
events : {
"click .column-sort-link": "onToggleColumn"
},
initialize : function() {
PagingView.prototype.initialize.call(this);
var collection = this.collection;
this.template = _.template($("#asset-library-tpl").text());
this.listenTo(collection, 'destroy', this.handleDestroy);
this.registerSortableColumn('js-asset-name-col', gettext('Name'), 'display_name', 'asc');
this.registerSortableColumn('js-asset-date-col', gettext('Date Added'), 'date_added', 'desc');
this.setInitialSortColumn('js-asset-date-col');
},
render: function() {
......@@ -52,12 +59,20 @@ var AssetsView = PagingView.extend({
},
addAsset: function (model) {
// Switch the sort column back to the default (most recent date added) and show the first page
// so that the new asset is shown at the top of the page.
this.setInitialSortColumn('js-asset-date-col');
this.setPage(0);
analytics.track('Uploaded a File', {
'course': course_location_analytics,
'asset_url': model.get('url')
});
},
onToggleColumn: function(event) {
var columnName = event.target.id;
this.toggleSortOrder(columnName);
}
});
......
......@@ -3,12 +3,21 @@ define(["backbone", "js/views/feedback_alert", "gettext"], function(Backbone, Al
var PagingView = Backbone.View.extend({
// takes a Backbone Paginator as a model
sortableColumns: {},
initialize: function() {
Backbone.View.prototype.initialize.call(this);
var collection = this.collection;
collection.bind('add', _.bind(this.renderPageItems, this));
collection.bind('remove', _.bind(this.renderPageItems, this));
collection.bind('reset', _.bind(this.renderPageItems, this));
collection.bind('add', _.bind(this.onPageRefresh, this));
collection.bind('remove', _.bind(this.onPageRefresh, this));
collection.bind('reset', _.bind(this.onPageRefresh, this));
},
onPageRefresh: function() {
var sortColumn = this.sortColumn;
this.renderPageItems();
this.$('.column-sort-link').removeClass('current-sort');
this.$('#' + sortColumn).addClass('current-sort');
},
setPage: function(page) {
......@@ -41,6 +50,67 @@ define(["backbone", "js/views/feedback_alert", "gettext"], function(Backbone, Al
if (currentPage > 0) {
this.setPage(currentPage - 1);
}
},
/**
* Registers information about a column that can be sorted.
* @param columnName The element name of the column.
* @param displayName The display name for the column in the current locale.
* @param fieldName The database field name that is represented by this column.
* @param defaultSortDirection The default sort direction for the column
*/
registerSortableColumn: function(columnName, displayName, fieldName, defaultSortDirection) {
this.sortableColumns[columnName] = {
displayName: displayName,
fieldName: fieldName,
defaultSortDirection: defaultSortDirection
};
},
sortableColumnInfo: function(sortColumn) {
var sortInfo = this.sortableColumns[sortColumn];
if (!sortInfo) {
throw "Unregistered sort column '" + sortColumn + '"';
}
return sortInfo;
},
sortDisplayName: function() {
var sortColumn = this.sortColumn,
sortInfo = this.sortableColumnInfo(sortColumn);
return sortInfo.displayName;
},
sortDirectionName: function() {
var collection = this.collection;
if (collection.sortDirection === 'asc') {
return gettext("ascending");
} else {
return gettext("descending");
}
},
setInitialSortColumn: function(sortColumn) {
var collection = this.collection,
sortInfo = this.sortableColumns[sortColumn];
collection.sortField = sortInfo.fieldName;
collection.sortDirection = sortInfo.defaultSortDirection;
this.sortColumn = sortColumn;
},
toggleSortOrder: function(sortColumn) {
var collection = this.collection,
sortInfo = this.sortableColumnInfo(sortColumn),
sortField = sortInfo.fieldName,
defaultSortDirection = sortInfo.defaultSortDirection;
if (collection.sortField === sortField) {
collection.sortDirection = collection.sortDirection === 'asc' ? 'desc' : 'asc';
} else {
collection.sortField = sortField;
collection.sortDirection = defaultSortDirection;
}
this.sortColumn = sortColumn;
this.setPage(0);
}
});
......
......@@ -35,15 +35,18 @@ define(["backbone", "underscore", "gettext"], function(Backbone, _, gettext) {
collection = view.collection,
start = collection.start,
count = collection.size(),
sortName = view.sortDisplayName(),
sortDirectionName = view.sortDirectionName(),
end = start + count,
total = collection.totalCount,
fmts = gettext('Showing %(current_span)s%(start)s-%(end)s%(end_span)s out of %(total_span)s%(total)s total%(end_span)s, sorted by %(order_span)s%(sort_order)s%(end_span)s');
fmts = gettext('Showing %(current_span)s%(start)s-%(end)s%(end_span)s out of %(total_span)s%(total)s total%(end_span)s, sorted by %(order_span)s%(sort_order)s%(end_span)s %(sort_direction)s');
return '<p>' + interpolate(fmts, {
start: Math.min(start + 1, end),
end: end,
total: total,
sort_order: gettext('Date Added'),
sort_order: sortName,
sort_direction: sortDirectionName,
current_span: '<span class="count-current-shown">',
total_span: '<span class="count-total">',
order_span: '<span class="sort-order">',
......
......@@ -87,7 +87,7 @@
.nav-link {
@include transition(all $tmg-f2 ease-in-out 0s);
display: block;
padding: ($baseline/4) ($baseline*.75);
padding: ($baseline/4) ($baseline*0.75);
&.previous {
margin-right: ($baseline/2);
......@@ -126,7 +126,7 @@
.total-pages {
@extend %t-copy-base;
width: ($baseline*2.5);
margin: 0 ($baseline*.75);
margin: 0 ($baseline*0.75);
padding: ($baseline/4);
text-align: center;
color: $gray;
......@@ -195,14 +195,26 @@
th {
@extend %t-copy-sub2;
background-color: $gray-l5;
padding: 0 $baseline ($baseline*.75) $baseline;
padding: 0 ($baseline/2) ($baseline*0.75) ($baseline/2);
vertical-align: middle;
text-align: left;
color: $gray;
.column-sort-link {
cursor: pointer;
color: $blue;
}
.current-sort {
font-weight: 700;
border-bottom: 1px solid $gray-l3;
font-weight: 700;
}
// CASE: embed column
&.embed-col {
padding-left: ($baseline*0.75);
padding-right: ($baseline*0.75);
}
}
......@@ -285,7 +297,7 @@
.embed-col {
@include transition(all $tmg-f2 ease-in-out 0s);
padding-left: ($baseline*.75);
padding-left: ($baseline*0.75);
color: $gray-l2;
.embeddable-xml-input {
......@@ -455,7 +467,7 @@
@include transition(color $tmg-f2 ease-in-out 0s);
position: absolute;
top: 0;
right: ($baseline*.75);
right: ($baseline*0.75);
border: none;
background: none;
padding: 0;
......
......@@ -14,8 +14,8 @@
<thead>
<tr>
<th class="thumb-col"><%= gettext("Preview") %></th>
<th class="name-col"><%= gettext("Name") %></th>
<th class="date-col"><span class="current-sort" href=""><%= gettext("Date Added") %></span></th>
<th class="name-col sortable-column"><span class="column-sort-link" id="js-asset-name-col"><%= gettext("Name") %></span></th>
<th class="date-col sortable-column"><span class="column-sort-link" id="js-asset-date-col"><%= gettext("Date Added") %></span></th>
<th class="embed-col"><%= gettext("URL") %></th>
<th class="actions-col"><span class="sr"><%= gettext("Actions") %></span></th>
</tr>
......
......@@ -128,7 +128,7 @@ class MongoContentStore(ContentStore):
directory as the other policy files.
"""
policy = {}
assets,__ = self.get_all_content_for_course(course_location)
assets, __ = self.get_all_content_for_course(course_location)
for asset in assets:
asset_location = Location(asset['_id'])
......
......@@ -19,7 +19,7 @@ def empty_asset_trashcan(course_locs):
store.delete(id)
# then delete all of the assets
assets,__ = store.get_all_content_for_course(course_loc)
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)
......
......@@ -199,7 +199,7 @@ def clone_course(modulestore, contentstore, source_location, dest_location, dele
# now iterate through all of the assets, also updating the thumbnail pointer
assets,__ = contentstore.get_all_content_for_course(source_location)
assets, __ = contentstore.get_all_content_for_course(source_location)
for asset in assets:
asset_loc = Location(asset["_id"])
content = contentstore.find(asset_loc)
......@@ -260,7 +260,7 @@ def delete_course(modulestore, contentstore, source_location, commit=False):
_delete_assets(contentstore, thumbs, commit)
# then delete all of the assets
assets,__ = contentstore.get_all_content_for_course(source_location)
assets, __ = contentstore.get_all_content_for_course(source_location)
_delete_assets(contentstore, assets, commit)
# then delete all course modules
......
......@@ -223,7 +223,7 @@ class TestMongoModuleStore(object):
Test getting, setting, and defaulting the locked attr and arbitrary attrs.
"""
location = Location('i4x', 'edX', 'toy', 'course', '2012_Fall')
course_content,__ = TestMongoModuleStore.content_store.get_all_content_for_course(location)
course_content, __ = TestMongoModuleStore.content_store.get_all_content_for_course(location)
assert len(course_content) > 0
# a bit overkill, could just do for content[0]
for content in course_content:
......
......@@ -19,3 +19,10 @@ location_map:
ensureIndex({'_id.org': 1, '_id.course': 1})
ensureIndex({'course_id': 1})
```
fs.files:
=========
```
ensureIndex({'displayname': 1})
```
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment