wbrawner/server
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix sorting of favorite files

Browse source 
The sort comparator checks the "isFavorite" property of the FileInfo
objects to compare. That property is set when the file list is loaded
and the response from the server is parsed, and thus a freshly loaded
file list has the proper sorting for favorite files. However, the
property is not set in other cases, like when the FileInfo objects are
derived from FileInfoModels due to a file being marked as a favorite or
a text editor being closed, which causes the file to be sorted in the
wrong position.

There is no need to add the property in those situations, though; in all
cases the TagsPlugin adds a "tags" array property that contains an
OC.TAG_FAVORITE tag, so that tag can be checked instead of "isFavorite".
Moreover, although "isFavorite" was added by the main "_parseFileInfo"
function it did not really belong there but to the "FileInfoParser" from
the TagsPlugin; however, as that property now is not used anywhere it
was removed altogether.

A cleaner solution would have been to make the sort comparator
extensible by plugins like other behaviours of the file list and then
add the sorting logic related to favorite files to the TagsPlugin.
However, right now only the TagsPlugin would need to alter the main
sorting logic, and it seems like a corner case anyway. Even if it is
implemented as a plugin, favorite files is a core feature, so for the
time being it will be taken into account directly in the main sorting
logic; making the sort comparator extensible by plugins is defered until
there are other use cases for that.

Fixes #5410

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
This commit is contained in:
Daniel Calviño Sánchez 2017-07-05 15:01:23 +02:00
parent 6fa5c33af4
commit be56374c51
4 changed files with 122 additions and 15 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

9
apps/files/js/filelist.js
View file

@ -1566,11 +1566,16 @@
this._sort = sort;
this._sortDirection = (direction === 'desc')?'desc':'asc';
this._sortComparator = function(fileInfo1, fileInfo2) {
if(fileInfo1.isFavorite && !fileInfo2.isFavorite) {
var isFavorite = function(fileInfo) {
return fileInfo.tags && fileInfo.tags.indexOf(OC.TAG_FAVORITE) >= 0;
};
if (isFavorite(fileInfo1) && !isFavorite(fileInfo2)) {
return -1;
} else if(!fileInfo1.isFavorite && fileInfo2.isFavorite) {
} else if (!isFavorite(fileInfo1) && isFavorite(fileInfo2)) {
return 1;
}
return direction === 'asc' ? comparator(fileInfo1, fileInfo2) : -comparator(fileInfo1, fileInfo2);
};

114
apps/files/tests/js/filelistSpec.js
View file

@ -2476,6 +2476,120 @@ describe('OCA.Files.FileList tests', function() {
sortStub.restore();
});
describe('with favorites', function() {
it('shows favorite files on top', function() {
testFiles.push(new FileInfo({
id: 5,
type: 'file',
name: 'ZZY Before last file in ascending order',
mimetype: 'text/plain',
mtime: 999999998,
size: 9999998,
// Tags would be added by TagsPlugin
tags: [OC.TAG_FAVORITE],
}), new FileInfo({
id: 6,
type: 'file',
name: 'ZZZ Last file in ascending order',
mimetype: 'text/plain',
mtime: 999999999,
size: 9999999,
// Tags would be added by TagsPlugin
tags: [OC.TAG_FAVORITE],
}));
fileList.setFiles(testFiles);
// Sort by name in ascending order (default sorting is by name
// in ascending order, but setFiles does not trigger a sort, so
// the files must be sorted before being set or a sort must be
// triggered afterwards by clicking on the header).
fileList.$el.find('.column-name .columntitle').click();
fileList.$el.find('.column-name .columntitle').click();
expect(fileList.findFileEl('ZZY Before last file in ascending order').index()).toEqual(0);
expect(fileList.findFileEl('ZZZ Last file in ascending order').index()).toEqual(1);
expect(fileList.findFileEl('somedir').index()).toEqual(2);
expect(fileList.findFileEl('One.txt').index()).toEqual(3);
expect(fileList.findFileEl('Three.pdf').index()).toEqual(4);
expect(fileList.findFileEl('Two.jpg').index()).toEqual(5);
// Sort by size in ascending order
fileList.$el.find('.column-size .columntitle').click();
fileList.$el.find('.column-size .columntitle').click();
expect(fileList.findFileEl('ZZY Before last file in ascending order').index()).toEqual(0);
expect(fileList.findFileEl('ZZZ Last file in ascending order').index()).toEqual(1);
expect(fileList.findFileEl('One.txt').index()).toEqual(2);
expect(fileList.findFileEl('somedir').index()).toEqual(3);
expect(fileList.findFileEl('Two.jpg').index()).toEqual(4);
expect(fileList.findFileEl('Three.pdf').index()).toEqual(5);
// Sort by modification time in ascending order
fileList.$el.find('.column-mtime .columntitle').click();
fileList.$el.find('.column-mtime .columntitle').click();
expect(fileList.findFileEl('ZZY Before last file in ascending order').index()).toEqual(0);
expect(fileList.findFileEl('ZZZ Last file in ascending order').index()).toEqual(1);
expect(fileList.findFileEl('One.txt').index()).toEqual(2);
expect(fileList.findFileEl('somedir').index()).toEqual(3);
expect(fileList.findFileEl('Three.pdf').index()).toEqual(4);
expect(fileList.findFileEl('Two.jpg').index()).toEqual(5);
});
it('shows favorite files on top also when using descending order', function() {
testFiles.push(new FileInfo({
id: 5,
type: 'file',
name: 'AAB Before last file in descending order',
mimetype: 'text/plain',
mtime: 2,
size: 2,
// Tags would be added by TagsPlugin
tags: [OC.TAG_FAVORITE],
}), new FileInfo({
id: 6,
type: 'file',
name: 'AAA Last file in descending order',
mimetype: 'text/plain',
mtime: 1,
size: 1,
// Tags would be added by TagsPlugin
tags: [OC.TAG_FAVORITE],
}));
fileList.setFiles(testFiles);
// Sort by name in descending order
fileList.$el.find('.column-name .columntitle').click();
expect(fileList.findFileEl('AAB Before last file in descending order').index()).toEqual(0);
expect(fileList.findFileEl('AAA Last file in descending order').index()).toEqual(1);
expect(fileList.findFileEl('Two.jpg').index()).toEqual(2);
expect(fileList.findFileEl('Three.pdf').index()).toEqual(3);
expect(fileList.findFileEl('One.txt').index()).toEqual(4);
expect(fileList.findFileEl('somedir').index()).toEqual(5);
// Sort by size in descending order
fileList.$el.find('.column-size .columntitle').click();
expect(fileList.findFileEl('AAB Before last file in descending order').index()).toEqual(0);
expect(fileList.findFileEl('AAA Last file in descending order').index()).toEqual(1);
expect(fileList.findFileEl('Three.pdf').index()).toEqual(2);
expect(fileList.findFileEl('Two.jpg').index()).toEqual(3);
expect(fileList.findFileEl('somedir').index()).toEqual(4);
expect(fileList.findFileEl('One.txt').index()).toEqual(5);
// Sort by modification time in descending order
fileList.$el.find('.column-mtime .columntitle').click();
expect(fileList.findFileEl('AAB Before last file in descending order').index()).toEqual(0);
expect(fileList.findFileEl('AAA Last file in descending order').index()).toEqual(1);
expect(fileList.findFileEl('Two.jpg').index()).toEqual(2);
expect(fileList.findFileEl('Three.pdf').index()).toEqual(3);
expect(fileList.findFileEl('somedir').index()).toEqual(4);
expect(fileList.findFileEl('One.txt').index()).toEqual(5);
});
});
});
describe('create file', function() {
var deferredCreate;

7
core/js/files/client.js
View file

@ -304,13 +304,6 @@
data.hasPreview = true;
}
var isFavorite = props['{' + Client.NS_OWNCLOUD + '}favorite'];
if (!_.isUndefined(isFavorite)) {
data.isFavorite = isFavorite === '1';
} else {
data.isFavorite = false;
}
var contentType = props[Client.PROPERTY_GETCONTENTTYPE];
if (!_.isUndefined(contentType)) {
data.mimetype = contentType;

7
core/js/files/fileinfo.js
View file

@ -132,12 +132,7 @@
/**
* @type boolean
*/
hasPreview: true,
/**
* @type boolean
*/
isFavorite: false
hasPreview: true
};
if (!OC.Files) {