diff --git a/chrome/content/zotero/duplicatesMerge.js b/chrome/content/zotero/duplicatesMerge.js index 0d951cde22..3c0947ffe1 100644 --- a/chrome/content/zotero/duplicatesMerge.js +++ b/chrome/content/zotero/duplicatesMerge.js @@ -141,7 +141,7 @@ var Zotero_Duplicates_Pane = new function () { } _masterItem = item; - itembox.item = item.clone(); + itembox.item = item.clone(null, { includeCollections: true }); } diff --git a/chrome/content/zotero/xpcom/collectionTreeView.js b/chrome/content/zotero/xpcom/collectionTreeView.js index 9696e75f24..327f3dd35e 100644 --- a/chrome/content/zotero/xpcom/collectionTreeView.js +++ b/chrome/content/zotero/xpcom/collectionTreeView.js @@ -1853,7 +1853,7 @@ Zotero.CollectionTreeView.prototype.drop = Zotero.Promise.coroutine(function* (r } // Create new clone item in target library - var newItem = item.clone(targetLibraryID, false, !options.tags); + var newItem = item.clone(targetLibraryID, { skipTags: !options.tags }); // Set Rights field for My Publications if (options.license) { @@ -1880,7 +1880,7 @@ Zotero.CollectionTreeView.prototype.drop = Zotero.Promise.coroutine(function* (r var noteIDs = item.getNotes(); var notes = Zotero.Items.get(noteIDs); for each(var note in notes) { - let newNote = note.clone(targetLibraryID); + let newNote = note.clone(targetLibraryID, { skipTags: !options.tags }); newNote.parentID = newItemID; yield newNote.save({ skipSelect: true diff --git a/chrome/content/zotero/xpcom/data/item.js b/chrome/content/zotero/xpcom/data/item.js index 4bad5204cf..f9ef40539c 100644 --- a/chrome/content/zotero/xpcom/data/item.js +++ b/chrome/content/zotero/xpcom/data/item.js @@ -3802,15 +3802,16 @@ Zotero.Item.prototype.multiDiff = function (otherItems, ignoreFields) { /** - * Returns an unsaved copy of the item without an itemID or key + * Returns an unsaved copy of the item without itemID and key * * This is used to duplicate items and copy them between libraries. * * @param {Number} [libraryID] - libraryID of the new item, or the same as original if omitted - * @param {Boolean} [skipTags=false] - Skip tags + * @param {Boolean} [options.skipTags=false] - Skip tags + * @param {Boolean} [options.includeCollections=false] - Add new item to all collections * @return {Promise} */ -Zotero.Item.prototype.clone = function (libraryID, skipTags) { +Zotero.Item.prototype.clone = function (libraryID, options = {}) { Zotero.debug('Cloning item ' + this.id); if (libraryID !== undefined && libraryID !== null && typeof libraryID !== 'number') { @@ -3857,10 +3858,17 @@ Zotero.Item.prototype.clone = function (libraryID, skipTags) { } } - if (!skipTags) { + if (!options.skipTags) { newItem.setTags(this.getTags()); } + if (options.includeCollections) { + if (!sameLibrary) { + throw new Error("Can't include collections when cloning to different library"); + } + newItem.setCollections(this.getCollections()); + } + if (sameLibrary) { // DEBUG: this will add reverse-only relateds too newItem.setRelations(this.getRelations()); diff --git a/chrome/content/zotero/xpcom/data/items.js b/chrome/content/zotero/xpcom/data/items.js index 018fcf9f34..6ab168e56e 100644 --- a/chrome/content/zotero/xpcom/data/items.js +++ b/chrome/content/zotero/xpcom/data/items.js @@ -789,11 +789,7 @@ Zotero.Items = function() { // old item, which will be put in the trash // Add collections to master - var collectionIDs = otherItem.getCollections(); - for each(var collectionID in collectionIDs) { - let collection = yield Zotero.Collections.getAsync(collectionID); - yield collection.addItem(item.id); - } + otherItem.getCollections().forEach(id => item.addToCollection(id)); // Add tags to master var tags = otherItem.getTags(); diff --git a/chrome/content/zotero/zoteroPane.js b/chrome/content/zotero/zoteroPane.js index fb3cf204c3..448a2a682f 100644 --- a/chrome/content/zotero/zoteroPane.js +++ b/chrome/content/zotero/zoteroPane.js @@ -1719,12 +1719,12 @@ var ZoteroPane = new function() var newItem; yield Zotero.DB.executeTransaction(function* () { - newItem = item.clone(null, !Zotero.Prefs.get('groups.copyTags')); - yield newItem.save(); - + newItem = item.clone(); + // If in a collection, add new item to it if (self.collectionsView.selectedTreeRow.isCollection() && newItem.isTopLevelItem()) { - yield self.collectionsView.selectedTreeRow.ref.addItem(newItem.id); + newItem.setCollections([self.collectionsView.selectedTreeRow.ref.id]); } + yield newItem.save(); }); yield self.selectItem(newItem.id); diff --git a/test/tests/duplicatesTest.js b/test/tests/duplicatesTest.js index 0e1a327149..946b96ad23 100644 --- a/test/tests/duplicatesTest.js +++ b/test/tests/duplicatesTest.js @@ -58,5 +58,39 @@ describe("Duplicate Items", function () { assert.property(rels, pred); assert.equal(rels[pred], uri2); }); + + it("should combine collections from all items", function* () { + var collection1 = yield createDataObject('collection'); + var collection2 = yield createDataObject('collection'); + + var item1 = yield createDataObject('item', { setTitle: true, collections: [collection1.id] }); + var item2 = item1.clone(); + item2.setCollections([collection2.id]); + yield item2.saveTx(); + + var userLibraryID = Zotero.Libraries.userLibraryID; + + var selected = yield cv.selectByID('D' + userLibraryID); + assert.ok(selected); + yield waitForItemsLoad(win); + + // Select the first item, which should select both + var iv = zp.itemsView; + var row = iv.getRowIndexByID(item1.id); + clickOnItemsRow(iv, row); + + // Click merge button + var button = win.document.getElementById('zotero-duplicates-merge-button'); + button.click(); + + yield waitForNotifierEvent('refresh', 'trash'); + + // Items should be gone + assert.isFalse(iv.getRowIndexByID(item1.id)); + assert.isFalse(iv.getRowIndexByID(item2.id)); + assert.isTrue(item2.deleted); + assert.isTrue(collection1.hasItem(item1.id)); + assert.isTrue(collection2.hasItem(item1.id)); + }); }); });