mirrors/zotero
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages1 Wiki Activity Actions

Revert "Validate fields in ItemTree::getSortFields()"

Browse source 
This reverts commit a8ed30ce80 and related
commits.

We'll address breakage from invalid sort fields another way, without
inconveniencing plugin authors.

https://groups.google.com/g/zotero-dev/c/kc0-C6-SA74/m/bhHniGceAQAJ
This commit is contained in:
Dan Stillman 2022-07-11 00:42:06 -04:00
parent f7dbcdde4b
commit 34d66381d1
2 changed files with 9 additions and 93 deletions
chrome/content/zotero
itemTree.jsx
test/tests
itemTreeTest.js

54
chrome/content/zotero/itemTree.jsx
View file

@ -1820,41 +1820,24 @@ var ItemTree = class ItemTree extends LibraryTree {
getSortFields() {
var fields = [this.getSortField()];
if (!this._isValidSortField(fields[0])) {
Zotero.logError(`'${fields[0]}' is not a valid sort field -- skipping`);
fields.shift();
}
var secondaryField = this._getSecondarySortField();
if (secondaryField) {
fields.push(secondaryField);
}
var fallbackFields;
try {
fallbackFields = Zotero.Prefs.get('fallbackSort')
var fallbackFields = Zotero.Prefs.get('fallbackSort')
.split(',')
.map(x => x.trim())
.filter(x => x !== '');
for (let field of fallbackFields) {
if (!this._isValidSortField(field)) {
// eslint-disable-next-line no-throw-literal
throw `'${field}' is not a valid field in fallbackSort -- resetting`;
}
}
.map((x) => x.trim())
.filter((x) => x !== '');
}
catch (e) {
Zotero.debug(e, 1);
Zotero.logError(e);
Zotero.Prefs.clear('fallbackSort');
fallbackFields = Zotero.Prefs.get('fallbackSort').split(',');
// This should match the default value for the fallbackSort pref
var fallbackFields = ['firstCreator', 'date', 'title', 'dateAdded'];
}
fields = Zotero.Utilities.arrayUnique(fields.concat(fallbackFields));
// If no valid fields, use default fallback
if (!fields.length) {
Zotero.logError(`No valid fields in getSortFields() (${fields.join(',')}) `
+ '-- resetting');
Zotero.Prefs.clear('fallbackSort');
fields = Zotero.Prefs.get('fallbackSort').split(',');
}
// If date appears after year, remove it, unless it's the explicit secondary sort
var yearPos = fields.indexOf('year');
if (yearPos != -1) {
@ -1866,21 +1849,7 @@ var ItemTree = class ItemTree extends LibraryTree {
return fields;
}
_isValidSortField(field) {
switch (field) {
// Non-standard columns
case 'itemType':
case 'year':
case 'hasAttachment':
case 'numNotes':
// Feed items
case 'id':
return true;
}
return !!Zotero.ItemFields.getID(field) || Zotero.Items.primaryFields.includes(field);
}
/**
* @param index {Integer}
* @param selectAll {Boolean} Whether the selection is part of a select-all event
@ -3753,13 +3722,6 @@ var ItemTree = class ItemTree extends LibraryTree {
if (!secondaryField || secondaryField == primaryField) {
return false;
}
// Make sure specified field is valid
if (!this._isValidSortField(secondaryField)) {
Zotero.logError(`'${secondaryField}' is not a valid field in `
+ `secondarySort.${primaryField} -- resetting`);
Zotero.Prefs.clear('secondarySort.' + primaryField);
return false;
}
return secondaryField;
}

48
test/tests/itemTreeTest.js
View file

@ -151,49 +151,6 @@ describe("Zotero.ItemTree", function() {
})
})
describe("#getSortFields()", function () {
beforeEach(async function () {
itemsView = zp.itemsView;
// Sort by title
const colIndex = itemsView.tree._getColumns().findIndex(column => column.dataKey == 'title');
await itemsView.tree._columns.toggleSort(colIndex);
});
after(function () {
Zotero.Prefs.clear('fallbackSort');
});
it("should ignore invalid secondary-sort field", function () {
Zotero.Prefs.set('secondarySort.title', 'invalidField');
var fields = itemsView.getSortFields();
assert.isUndefined(Zotero.Prefs.get('secondarySort.title'));
// fallbackSort pref with title moved to beginning
assert.sameMembers(['title', 'firstCreator', 'date', 'dateAdded'], fields);
Zotero.Prefs.clear('secondarySort.title');
});
it("should ignore invalid fallback-sort field", function () {
Zotero.Prefs.clear('fallbackSort');
var originalFallback = Zotero.Prefs.get('fallbackSort');
Zotero.Prefs.set('fallbackSort', 'invalidField,' + originalFallback);
var fields = itemsView.getSortFields();
assert.equal(Zotero.Prefs.get('fallbackSort'), originalFallback);
// fallbackSort pref with title moved to beginning
assert.sameMembers(['title', 'firstCreator', 'date', 'dateAdded'], fields);
});
it("should sort by item type", async function () {
// Sort by item type
const colIndex = itemsView.tree._getColumns().findIndex(column => column.dataKey == 'itemType');
await itemsView.tree._columns.toggleSort(colIndex);
var fields = itemsView.getSortFields();
assert.sameMembers(['itemType', 'firstCreator', 'date', 'title', 'dateAdded'], fields);
});
});
describe("#notify()", function () {
beforeEach(function () {
sinon.spy(win.ZoteroPane, "itemSelected");
@ -625,10 +582,7 @@ describe("Zotero.ItemTree", function() {
var item3 = await createDataObject('item', { title: title + " 5" });
var item4 = await createDataObject('item', { title: title + " 7" });
// Sort by creator and then title, to make sure we're sorting by title ascending
var colIndex = itemsView.tree._getColumns().findIndex(column => column.dataKey == 'firstCreator');
await itemsView.tree._columns.toggleSort(colIndex);
colIndex = itemsView.tree._getColumns().findIndex(column => column.dataKey == 'title');
const colIndex = itemsView.tree._getColumns().findIndex(column => column.dataKey == 'title');
await itemsView.tree._columns.toggleSort(colIndex);
// Check initial sort order