scriptoriumadmin/zotero
1
0
Fork 0
mirror of https://github.com/zotero/zotero.git synced 2026-10-08 03:08:19 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

Handle multiple-library item selections dropped on a collection or library

Browse source 
Route each item by its own library -- items already in the target
library are added directly (or skipped, for a library root), while
items from other libraries are copied in -- instead of attempting an
invalid cross-library insert. Disallow a move of such a selection
rather than silently copying.

Fixes #5961
This commit is contained in:
Dan Stillman 2026-06-18 15:51:31 -04:00
parent 6d1cd85211
commit 08d875c7f7
2 changed files with 161 additions and 25 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

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

@ -1583,7 +1583,19 @@ var CollectionTree = class CollectionTree extends LibraryTree {
}
}
if ((Zotero.isMac && event.metaKey) || (!Zotero.isMac && event.shiftKey)) {
let move = (Zotero.isMac && event.metaKey) || (!Zotero.isMac && event.shiftKey);
// A selection from a multiple-collection view can span libraries. Those items can
// only be copied, never moved (a move can't coherently move some items and copy
// others), so disallow a move rather than silently substituting a copy.
let ids = Zotero.DragDrop.getDataFromDataTransfer(event.dataTransfer).data;
let items = Zotero.Items.get(ids);
if (new Set(items.map(item => item.libraryID)).size > 1) {
this.setDropEffect(event, move ? "none" : "copy");
return false;
}
if (move) {
this.setDropEffect(event, "move");
}
else {
@ -1784,11 +1796,14 @@ var CollectionTree = class CollectionTree extends LibraryTree {
}
// Intra-library drag
// Don't allow drag onto root of same library
// An item can't be added to the root of its own library, but skip it rather
// than rejecting the whole drag, so a mixed-library selection can still copy
// its out-of-library items here. (If every item is already in this library,
// `skip` stays true and the drag is refused below.)
if (treeRow.isLibrary(true)) {
Zotero.debug("Can't drag into same library root");
return false;
Zotero.debug("Item " + item.id + " already in library " + treeRow.ref.libraryID);
continue;
}
// Make sure there's at least one item that's not already in this destination
@ -2338,42 +2353,46 @@ var CollectionTree = class CollectionTree extends LibraryTree {
});
}
let newItems = [];
let newIDs = [];
// Route each item by its own library: items already in the target library are added
// directly, while items from other libraries are copied into the target library. A
// selection can span multiple libraries when dragging from a multiple-collection view.
let sameLibraryItems = [];
let otherLibraryItems = [];
let toMove = [];
// TODO: support items coming from different sources?
let sameLibrary = items[0].libraryID == targetLibraryID
for (let item of items) {
if (!item.isTopLevelItem()) {
continue;
}
newItems.push(item);
if (sameLibrary) {
newIDs.push(item.id);
if (item.libraryID == targetLibraryID) {
sameLibraryItems.push(item);
toMove.push(item.id);
}
else {
otherLibraryItems.push(item);
}
}
if (sameLibrary) {
// Add items to target container in the same library.
// Add same-library items to the target container
if (sameLibraryItems.length) {
if (targetCollectionID) {
let ids = newIDs.filter(itemID => Zotero.Items.get(itemID).isTopLevelItem());
let ids = sameLibraryItems.map(item => item.id);
await Zotero.DB.executeTransaction(async function () {
let collection = await Zotero.Collections.getAsync(targetCollectionID);
await collection.addItems(ids);
}.bind(this));
}
else if (targetTreeRow.isPublications()) {
await Zotero.Items.addToPublications(newItems, copyOptions);
await Zotero.Items.addToPublications(sameLibraryItems, copyOptions);
}
}
else {
// Copy items from other libraries into the target library
if (otherLibraryItems.length) {
let toReconcile = [];
await Zotero.Utilities.Internal.forEachChunkAsync(
newItems,
otherLibraryItems,
100,
function (chunk) {
return Zotero.DB.executeTransaction(async () => {
@ -2442,11 +2461,10 @@ var CollectionTree = class CollectionTree extends LibraryTree {
}
// If moving, remove items from source collection
if (dropEffect == 'move' && toMove.length) {
if (!sameLibrary) {
throw new Error("Cannot move items between libraries");
}
// If moving, remove items from source collection. A move of a mixed-library selection
// is disallowed in onDragOver(), so it shouldn't reach here; guard against a partial
// move just in case, since only the same-library items would be moved.
if (dropEffect == 'move' && toMove.length && !otherLibraryItems.length) {
if (!sourceTreeRow || !sourceTreeRow.isCollection()) {
throw new Error("Drag source must be a collection for move action");
}

118
test/tests/collectionTreeTest.js
View file

@ -896,6 +896,46 @@ describe("Zotero.CollectionTree", function () {
return canDrop;
};
// Simulate a drag over a row and return the resulting dropEffect ('copy', 'move', or
// 'none'). Pass { move: true } to simulate the platform's move modifier being held.
var dragOver = function (objectType, targetRowID, ids, { move = false } = {}) {
var index = cv.getRowIndexByID(targetRowID);
Zotero.DragDrop.currentDragSource = objectType == "item"
? zp.itemsView.collectionTreeRows[0]
: null;
// Drop directly onto the middle of the row (orient 0)
var rowEl = {
classList: { contains: () => true },
getBoundingClientRect: () => ({ y: 0, height: 100 })
};
var dataTransfer = {
dropEffect: 'copy',
effectAllowed: 'copyMove',
types: [`zotero/${objectType}`],
getData: function (type) {
if (type == `zotero/${objectType}`) {
return ids.join(",");
}
return "";
},
setDragImage: () => {}
};
cv.onDragOver({
preventDefault: () => {},
stopPropagation: () => {},
currentTarget: rowEl,
target: rowEl,
clientY: 50,
metaKey: move && Zotero.isMac,
shiftKey: move && !Zotero.isMac,
dataTransfer
}, index);
Zotero.DragDrop.currentDragSource = null;
return dataTransfer.dropEffect;
};
describe("with items", function () {
it("should add an item to a collection", async function () {
var collection = await createDataObject('collection');
@ -962,6 +1002,84 @@ describe("Zotero.CollectionTree", function () {
assert.equal(treeRow.ref.id, item.id);
});
it("should add a multiple-library item selection to a collection, copying out-of-library items", async function () {
await Zotero.Users.setCurrentUserID(1);
await Zotero.Users.setName(1, 'Name');
var collection = await createDataObject('collection');
var libraryItem = await createDataObject('item', false, { skipSelect: true });
var group = await createGroup();
var groupItem = await createDataObject('item', { libraryID: group.libraryID });
// Drop one item from the personal library and one from the group onto a
// personal-library collection
await onDrop('item', 'C' + collection.id, [libraryItem.id, groupItem.id]);
await collection.loadDataType('childItems');
// The collection now contains the same-library item plus a copy of the group item
var childItemIDs = collection.getChildItems(true);
assert.lengthOf(childItemIDs, 2);
assert.include(childItemIDs, libraryItem.id);
// The group item was copied into the personal library, and the copy links back to it
var copiedItem = Zotero.Items.get(childItemIDs.find(id => id != libraryItem.id));
assert.equal(copiedItem.libraryID, collection.libraryID);
assert.equal((await copiedItem.getLinkedItem(group.libraryID)).id, groupItem.id);
await group.eraseTx();
});
it("should disallow moving a multiple-library item selection", async function () {
var sourceCollection = await createDataObject('collection');
var targetCollection = await createDataObject('collection');
var libraryItem = await createDataObject('item', { collections: [sourceCollection.id] });
var group = await createGroup();
var groupItem = await createDataObject('item', { libraryID: group.libraryID });
// Source collection has to be selected so it's used as the drag source
await select(win, sourceCollection);
await waitForItemsLoad(win);
var ids = [libraryItem.id, groupItem.id];
// A plain drag copies the selection
assert.equal(dragOver('item', 'C' + targetCollection.id, ids), 'copy');
// A move is disallowed, since the out-of-library item can't be moved
assert.equal(dragOver('item', 'C' + targetCollection.id, ids, { move: true }), 'none');
await group.eraseTx();
});
it("should copy out-of-library items from a multiple-library selection dropped on a library root", async function () {
await Zotero.Users.setCurrentUserID(1);
await Zotero.Users.setName(1, 'Name');
var libraryItem = await createDataObject('item', false, { skipSelect: true });
var group = await createGroup();
var groupItem = await createDataObject('item', { libraryID: group.libraryID });
// Drop a personal-library item and a group item onto the personal library root: the
// item already in the library is a no-op, and the group item is copied in
var ids = (await onDrop('item', 'L' + userLibraryID, [libraryItem.id, groupItem.id])).ids;
assert.lengthOf(ids, 1);
var copiedItem = Zotero.Items.get(ids[0]);
assert.equal(copiedItem.libraryID, userLibraryID);
assert.equal((await copiedItem.getLinkedItem(group.libraryID)).id, groupItem.id);
await group.eraseTx();
});
it("should refuse a single-library selection dropped on its own library root", async function () {
var item1 = await createDataObject('item', false, { skipSelect: true });
var item2 = await createDataObject('item', false, { skipSelect: true });
// With no out-of-library items to copy, the drag is refused
assert.isFalse(await canDrop('item', 'L' + userLibraryID, [item1.id, item2.id]));
});
describe("My Publications", function () {
function getItemModifyPromise(item) {
// Add observer to wait for item modification