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

Rework object type checking in items list

Browse source 
a532cfb475 added `isCollection()`, `isSearch()`, and `isItem()` methods
to data objects to handle collections and searches in the trash, with
`isItem()` checking whether `._ObjectType` was `Item`. That left out
feed items (`._ObjectType` == `FeedItem`), and when c384fef867 made
`getSelectedItems()` return only items, it used `isItem()`, so feed
items were excluded, which broke feed-item toggling between read and
unread [1] and possibly some other things.

The simple fix would be to make `isItem` match feed items as well (which
could potentially fix other bugs related to feed items), but there was
actually no need to add new methods (which can get confused with
`CollectionTreeRow` methods) when we can just check the object type with
`obj instanceof Zotero.Item`, which gets the benefit of inheritance and
matches `Zotero.FeedItem` instances as well.

[1] https://forums.zotero.org/discussion/115571/cannot-change-the-status-of-title-in-subscribtion
This commit is contained in:
Dan Stillman 2024-06-28 02:09:26 -04:00
parent 20e9f20580
commit 6942506eba
8 changed files with 72 additions and 48 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
chrome/content/zotero/collectionTree.jsx
View file

@ -905,8 +905,8 @@ var CollectionTree = class CollectionTree extends LibraryTree {
await this._addSortedRow('collection', id);
await this.selectByID(currentTreeRow.id);
// Invalidate parent in case it's become non-empty
let parentRow = this.getRowIndexByID("C" + collection.parentID);
if (parentRow !== false) {
if (collection.parentID) {
let parentRow = this.getRowIndexByID("C" + collection.parentID);
this.tree.invalidateRow(parentRow);
}
}

8
chrome/content/zotero/elements/itemPane.js
View file

@ -113,7 +113,7 @@
let item = this.data[0];
// If a collection or search is selected, it must be in the trash.
if (item.isCollection() || item.isSearch()) {
if (item instanceof Zotero.Collection || item instanceof Zotero.Search) {
renderStatus = this.renderMessage();
}
else if (item.isNote()) {
@ -228,13 +228,13 @@
// In the trash, we have to check the object type
if (this.collectionTreeRow.isTrash()) {
let obj = this.data[0];
if (this.data.every(x => x.isCollection())) {
if (this.data.every(x => x instanceof Zotero.Collection)) {
key = 'item-pane-message-collections-selected';
}
else if (this.data.every(x => x.isSearch())) {
else if (this.data.every(x => x instanceof Zotero.Search)) {
key = 'item-pane-message-searches-selected';
}
else if (this.data.every(x => x.isItem())) {
else if (this.data.every(x => x instanceof Zotero.Item)) {
key = 'item-pane-message-items-selected';
}
else {

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

@ -224,7 +224,11 @@ var ItemTree = class ItemTree extends LibraryTree {
newSearchItems = newSearchItems.filter(item => !item.isAnnotation());
// Remove notes and attachments if necessary
if (this.regularOnly) {
newSearchItems = newSearchItems.filter(item => item.isCollection() || item.isRegularItem());
newSearchItems = newSearchItems.filter((item) => {
return item instanceof Zotero.Collection
|| item instanceof Zotero.Search
|| item.isRegularItem();
});
}
let newSearchItemIDs = new Set(newSearchItems.map(item => item.id));
// Find the items that aren't yet in the tree
@ -255,7 +259,7 @@ var ItemTree = class ItemTree extends LibraryTree {
if (row.level == 0) {
// A top-level attachment moved into a parent. Don't copy, it will be added
// via this loop for the parent item.
if (row.ref.parentID && row.ref.isItem()) {
if (row.ref instanceof Zotero.Item && row.ref.parentID) {
continue;
}
let isSearchParent = newSearchParentIDs.has(row.ref.treeViewID);
@ -1761,22 +1765,22 @@ var ItemTree = class ItemTree extends LibraryTree {
this._refreshRowMap();
this.tree.invalidate();
// Create an array of selected items
var ids = Array.from(this.selection.selected).filter(index => this.getRow(index).ref.isItem()).map(index => this.getRow(index).id);
let selectedObjects = [...this.selection.selected].map(index => this.getRow(index).ref);
let selectedItems = selectedObjects.filter(o => o instanceof Zotero.Item);
let selectedItemIDs = selectedItems.map(o => o.id);
var collectionTreeRow = this.collectionTreeRow;
let collectionTreeRow = this.collectionTreeRow;
if (collectionTreeRow.isBucket()) {
collectionTreeRow.ref.deleteItems(ids);
}
if (collectionTreeRow.isTrash()) {
let selectedObjects = Array.from(this.selection.selected).map(index => this.getRow(index).ref);
else if (collectionTreeRow.isTrash()) {
let [trashedCollectionIDs, trashedSearches] = [[], []];
for (let obj of selectedObjects) {
if (obj.isCollection()) {
if (obj instanceof Zotero.Collection) {
trashedCollectionIDs.push(obj.id);
}
if (obj.isSearch()) {
if (obj instanceof Zotero.Search) {
trashedSearches.push(obj.id);
}
}
@ -1786,8 +1790,8 @@ var ItemTree = class ItemTree extends LibraryTree {
if (trashedSearches.length > 0) {
await Zotero.Searches.erase(trashedSearches);
}
if (ids.length > 0) {
await Zotero.Items.erase(ids);
if (selectedItemIDs.length > 0) {
await Zotero.Items.erase(selectedItemIDs);
}
}
else if (collectionTreeRow.isLibrary(true)
@ -1796,7 +1800,7 @@ var ItemTree = class ItemTree extends LibraryTree {
|| collectionTreeRow.isRetracted()
|| collectionTreeRow.isDuplicates()
|| force) {
await Zotero.Items.trashTx(ids);
await Zotero.Items.trashTx(selectedItemIDs);
}
else if (collectionTreeRow.isCollection()) {
let collectionIDs = [collectionTreeRow.ref.id];
@ -1805,8 +1809,7 @@ var ItemTree = class ItemTree extends LibraryTree {
}
await Zotero.DB.executeTransaction(async () => {
for (let itemID of ids) {
let item = Zotero.Items.get(itemID);
for (let item of selectedItems) {
for (let collectionID of collectionIDs) {
item.removeFromCollection(collectionID);
}
@ -1817,7 +1820,7 @@ var ItemTree = class ItemTree extends LibraryTree {
});
}
else if (collectionTreeRow.isPublications()) {
await Zotero.Items.removeFromPublications(ids.map(id => Zotero.Items.get(id)));
await Zotero.Items.removeFromPublications(selectedItems);
}
}
finally {
@ -1844,7 +1847,7 @@ var ItemTree = class ItemTree extends LibraryTree {
* Get selected items, omitting collections and searches in the trash
*/
getSelectedItems(asIDs) {
var items = this.getSelectedObjects().filter(x => x.isItem());
var items = this.getSelectedObjects().filter(o => o instanceof Zotero.Item);
return asIDs ? items.map(x => x.id) : items;
}
@ -2821,10 +2824,10 @@ var ItemTree = class ItemTree extends LibraryTree {
try {
// Special treatment for trashed collections or searches since they are not an actual
// item and do not have an item type
if (item.isSearch()) {
if (item instanceof Zotero.Collection) {
itemTypeAriaLabel = Zotero.getString('searchConditions.collection') + '.';
}
else if (item.isCollection()) {
else if (item instanceof Zotero.Search) {
itemTypeAriaLabel = Zotero.getString('searchConditions.savedSearch') + '.';
}
else {
@ -3181,10 +3184,13 @@ var ItemTree = class ItemTree extends LibraryTree {
return this._rowCache[itemID];
}
let row = {};
let row = {
// Not a collection or search in the trash
isItem: treeRow.ref instanceof Zotero.Item
};
// Mark items not matching search as context rows, displayed in gray
if (this._searchMode && !this._searchItemIDs.has(itemID) && treeRow.ref.isItem()) {
if (row.isItem && this._searchMode && !this._searchItemIDs.has(itemID)) {
row.contextRow = true;
}
@ -3201,7 +3207,7 @@ var ItemTree = class ItemTree extends LibraryTree {
row.unread = true;
}
if (!(treeRow.ref.isCollection() || treeRow.ref.isSearch())) {
if (!(treeRow.ref instanceof Zotero.Collection || treeRow.ref instanceof Zotero.Search)) {
row.itemType = Zotero.ItemTypes.getLocalizedString(treeRow.ref.itemTypeID);
}
// Year column is just date field truncated
@ -3257,7 +3263,6 @@ var ItemTree = class ItemTree extends LibraryTree {
}
row[key] = val;
}
row.isItem = treeRow.ref.isItem();
return this._rowCache[itemID] = row;
}
@ -3833,12 +3838,12 @@ var ItemTree = class ItemTree extends LibraryTree {
var item = this.getRow(index).ref;
// Non-item objects that can be appear in the trash
if (item.isCollection() || item.isSearch()) {
if (item instanceof Zotero.Collection || item instanceof Zotero.Search) {
let icon;
if (item.isCollection()) {
if (item instanceof Zotero.Collection) {
icon = getCSSIcon('collection');
}
else if (item.isSearch()) {
else if (item instanceof Zotero.Search) {
icon = getCSSIcon('search');
}
icon.classList.add('icon-item-type');

9
chrome/content/zotero/xpcom/data/dataObjectUtilities.js
View file

@ -734,9 +734,6 @@ Zotero.DataObjectUtilities = {
* Most of these are overriden by Zotero.Item.
*/
itemTreeMockProperties: {
isCollection: function () {
return this._ObjectType == "Collection";
},
isAnnotation: () => false,
isNote: () => false,
numNotes: () => 0,
@ -744,16 +741,10 @@ Zotero.DataObjectUtilities = {
numAttachments: () => false,
getColoredTags: () => false,
isRegularItem: () => false, // Should be false to prevent items dropped into deleted searches
isSearch: function () {
return this._ObjectType == "Search";
},
getNotes: () => [],
getAttachments: () => [],
isFileAttachment: () => false,
isTopLevelItem: () => false,
isItem: function () {
return this._ObjectType == "Item";
},
getField: function (field, _) {
return this['_' + field] || "";
},

4
chrome/content/zotero/xpcom/data/feedItems.js
View file

@ -133,6 +133,10 @@ Zotero.FeedItems = new Proxy(function() {
ids = [ids];
}
if (!ids.length) {
throw new Error("No ids passed");
}
let items = yield this.getAsync(ids);
if (state == undefined) {

4
chrome/content/zotero/xpcom/data/item.js
View file

@ -4994,10 +4994,6 @@ Zotero.Item.prototype._eraseData = Zotero.Promise.coroutine(function* (env) {
});
Zotero.Item.prototype.isCollection = function() {
return false;
}
/**
* Populate the object's data from an API JSON data object
*

8
chrome/content/zotero/zoteroPane.js
View file

@ -1232,8 +1232,8 @@ var ZoteroPane = new function()
// If no items or an unreasonable number, don't try
if (!objects.length || objects.length > 100) return;
var collections = objects.filter(o => o.isCollection());
var items = objects.filter(o => o.isItem());
var collections = objects.filter(o => o instanceof Zotero.Collection);
var items = objects.filter(o => o instanceof Zotero.Item);
// Get parent collections of collections
var toHighlight = [];
@ -2421,7 +2421,7 @@ var ZoteroPane = new function()
let parent = this.itemsView.getRow(row).ref;
let childIDs = [];
let subcollections = [];
if (parent.isCollection()) {
if (parent instanceof Zotero.Collection) {
// If the restored item is a collection, restore its subcollections too
if (isSelected(parent)) {
subcollections = parent.getDescendents(false, 'collection', true).map(col => col.id);
@ -3924,7 +3924,7 @@ var ZoteroPane = new function()
menu.childNodes[m.showInLibrary].setAttribute('label', Zotero.getString('general.showInLibrary'));
}
// For collections and search, only keep restore/delete options
if (items.some(item => item.isCollection() || item.isSearch())) {
if (items.some(item => item instanceof Zotero.Collection || item instanceof Zotero.Search)) {
for (let option of options) {
if (!['restoreToLibrary', 'deleteFromLibrary'].includes(option)) {
show.delete(m[option]);

28
test/tests/itemPaneTest.js
View file

@ -1215,6 +1215,34 @@ describe("Item pane", function () {
describe("Feed buttons", function() {
describe("Mark as Read/Unread", function() {
it("should change an item from unread to read", async function () {
var feed = await createFeed();
await select(win, feed);
var item = await createDataObject('feedItem', { libraryID: feed.libraryID });
// Skip timed mark-as-read
var stub = sinon.stub(win.ZoteroPane, 'startItemReadTimeout');
await select(win, item);
// Click "Mark as Read"
var promise = waitForItemEvent('modify');
var button = ZoteroPane.itemPane.getCurrentPane().querySelector('.feed-item-toggleRead-button');
assert.equal(button.label, Zotero.getString('pane.item.markAsRead'));
assert.isFalse(item.isRead);
button.click();
var ids = await promise;
assert.sameMembers(ids, [item.id]);
assert.isTrue(item.isRead);
// Button is re-created
button = ZoteroPane.itemPane.getCurrentPane().querySelector('.feed-item-toggleRead-button');
assert.equal(button.label, Zotero.getString('pane.item.markAsUnread'));
stub.restore();
});
it("should update label when state of an item changes", function* () {
let feed = yield createFeed();
yield selectLibrary(win, feed.libraryID);