www

Unnamed repository; edit this file 'description' to name the repository.
Log | Files | Refs | Submodules | README | LICENSE

commit de3b47fd782dc0dd19f6bb392ede8da46b8b535b
parent 941ae5499c3fce34fad5981d76292bc5e73a616e
Author: Dan Stillman <dstillman@zotero.org>
Date:   Fri,  7 Jul 2017 18:14:30 -0400

Add "Delete Automatic Tags in This Library…" option to tag selector menu

I think it might be worth having a tag management window that lets you
view tags as a grid, sort by column (e.g., type), select ranges, delete,
consolidate, etc., but until then, this fulfills a popular request.

Diffstat:
Mchrome/content/zotero/bindings/tagselector.xml | 59+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Mchrome/content/zotero/xpcom/data/tags.js | 183++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------------------
Mchrome/locale/en-US/zotero/zotero.dtd | 1+
Mchrome/locale/en-US/zotero/zotero.properties | 2++
Mtest/tests/tagsTest.js | 30++++++++++++++++++++++++++++++
5 files changed, 217 insertions(+), 58 deletions(-)

diff --git a/chrome/content/zotero/bindings/tagselector.xml b/chrome/content/zotero/bindings/tagselector.xml @@ -833,6 +833,59 @@ </method> + <method name="_updateDeleteAutomaticMenuOption"> + <body><![CDATA[ + (async function () { + var hasAutomatic = !!(await Zotero.Tags.getAutomaticInLibrary(this.libraryID)).length; + var menuitem = this.id('delete-automatic-tags'); + menuitem.disabled = !hasAutomatic; + }.bind(this))(); + ]]></body> + </method> + + + <method name="_deleteAutomatic"> + <body><![CDATA[ + (async function () { + var num = (await Zotero.Tags.getAutomaticInLibrary(this.libraryID)).length; + if (!num) { + return; + } + + var ps = Components.classes["@mozilla.org/embedcomp/prompt-service;1"] + .getService(Components.interfaces.nsIPromptService); + var confirmed = ps.confirm( + window, + Zotero.getString('pane.tagSelector.deleteAutomatic.title'), + Zotero.getString( + 'pane.tagSelector.deleteAutomatic.message', + new Intl.NumberFormat().format(num), + num + ) + + "\n\n" + + Zotero.getString('general.actionCannotBeUndone') + ); + if (confirmed) { + Zotero.showZoteroPaneProgressMeter(null, true); + try { + await Zotero.Tags.removeAutomaticFromLibrary( + this.libraryID, + (progress, progressMax) => { + Zotero.updateZoteroPaneProgressMeter( + Math.round(progress / progressMax * 100) + ); + } + ); + } + finally { + Zotero.hideZoteroPaneOverlays(); + } + } + }.bind(this))(); + ]]></body> + </method> + + <method name="_insertClickableTag"> <parameter name="tagsBox"/> <parameter name="tagData"/> @@ -1113,6 +1166,7 @@ <toolbarbutton id="view-settings-menu" tooltiptext="&zotero.toolbar.actions.label;" image="chrome://zotero/skin/tag-selector-menu.png" type="menu"> <menupopup id="view-settings-popup" + onpopupshowing="document.getBindingParent(this)._updateDeleteAutomaticMenuOption()" onpopupshown="/* This is necessary to fix a bug with Display All Tags not being checked if enabled before menuu is shown (OS X only?) @@ -1136,6 +1190,11 @@ this.setAttribute('checked', !displayAll); document.getBindingParent(this).filterToScope = !displayAll; event.stopPropagation();"/> + <menuseparator/> + <menuitem id="delete-automatic-tags" label="&zotero.tagSelector.deleteAutomaticInLibrary;" type="checkbox" + oncommand="document.getBindingParent(this)._deleteAutomatic(); + this.setAttribute('checked', false); + event.stopPropagation();"/> </menupopup> </toolbarbutton> </hbox> diff --git a/chrome/content/zotero/xpcom/data/tags.js b/chrome/content/zotero/xpcom/data/tags.js @@ -304,64 +304,93 @@ Zotero.Tags = new function() { /** * @return {Promise} */ - this.removeFromLibrary = Zotero.Promise.coroutine(function* (libraryID, tagIDs) { + this.removeFromLibrary = Zotero.Promise.coroutine(function* (libraryID, tagIDs, onProgress) { + var d = new Date(); + tagIDs = Zotero.flattenArguments(tagIDs); var deletedNames = []; - var oldItemIDs = []; - - yield Zotero.DB.executeTransaction(function* () { - var notifierPairs = []; - var notifierData = {}; - for (let i=0; i<tagIDs.length; i++) { - let tagID = tagIDs[i]; - let name = this.getName(tagID); - if (name === false) { - continue; - } - deletedNames.push(name); - - // Since we're performing the DELETE query directly, - // get the list of items that will need their tags reloaded, - // and generate data for item-tag notifications - let tagItems = yield this.getTagItems(libraryID, tagID); - for (let j = 0; j < tagItems.length; j++) { - let itemID = tagItems[i]; - let pair = itemID + "-" + tagID; - notifierPairs.push(pair); - notifierData[pair] = { - libraryID: libraryID, - tag: name - }; - } - oldItemIDs = oldItemIDs.concat(tagItems); - } - if (oldItemIDs.length) { - Zotero.Notifier.queue('remove', 'item-tag', notifierPairs, notifierData); - } - - var sql = "DELETE FROM itemTags WHERE tagID IN (" - + tagIDs.map(x => '?').join(', ') + ") AND itemID IN " - + "(SELECT itemID FROM items WHERE libraryID=?)"; - yield Zotero.DB.queryAsync(sql, tagIDs.concat([libraryID])); - - yield this.purge(tagIDs); - - // Update internal timestamps on all items that had these tags - yield Zotero.Utilities.Internal.forEachChunkAsync( - Zotero.Utilities.arrayUnique(oldItemIDs), - Zotero.DB.MAX_BOUND_PARAMETERS - 1, - Zotero.Promise.coroutine(function* (chunk) { - let placeholders = chunk.map(() => '?').join(','); + var done = 0; + + yield Zotero.Utilities.Internal.forEachChunkAsync( + tagIDs, + 100, + async function (chunk) { + await Zotero.DB.executeTransaction(function* () { + var oldItemIDs = []; - sql = 'UPDATE items SET synced=0, clientDateModified=? ' - + 'WHERE itemID IN (' + placeholders + ')' - yield Zotero.DB.queryAsync(sql, [Zotero.DB.transactionDateTime].concat(chunk)); + var notifierPairs = []; + var notifierData = {}; + var a = new Date(); - yield Zotero.Items.reload(oldItemIDs, ['primaryData', 'tags'], true); - }) - ); - }.bind(this)); + var sql = 'SELECT tagID, itemID FROM itemTags JOIN items USING (itemID) ' + + 'WHERE libraryID=? AND tagID IN (' + + Array(chunk.length).fill('?').join(', ') + + ') ORDER BY tagID'; + var chunkTagItems = yield Zotero.DB.queryAsync(sql, [libraryID, ...chunk]); + var i = 0; + + chunk.sort((a, b) => a - b); + + for (let tagID of chunk) { + let name = this.getName(tagID); + if (name === false) { + continue; + } + deletedNames.push(name); + + // Since we're performing the DELETE query directly, + // get the list of items that will need their tags reloaded, + // and generate data for item-tag notifications + let itemIDs = [] + while (i < chunkTagItems.length && chunkTagItems[i].tagID == tagID) { + itemIDs.push(chunkTagItems[i].itemID); + i++; + } + for (let itemID of itemIDs) { + let pair = itemID + "-" + tagID; + notifierPairs.push(pair); + notifierData[pair] = { + libraryID: libraryID, + tag: name + }; + } + oldItemIDs = oldItemIDs.concat(itemIDs); + } + if (oldItemIDs.length) { + Zotero.Notifier.queue('remove', 'item-tag', notifierPairs, notifierData); + } + + var sql = "DELETE FROM itemTags WHERE tagID IN (" + + Array(chunk.length).fill('?').join(', ') + ") AND itemID IN " + + "(SELECT itemID FROM items WHERE libraryID=?)"; + yield Zotero.DB.queryAsync(sql, chunk.concat([libraryID])); + + yield this.purge(chunk); + + // Update internal timestamps on all items that had these tags + yield Zotero.Utilities.Internal.forEachChunkAsync( + Zotero.Utilities.arrayUnique(oldItemIDs), + Zotero.DB.MAX_BOUND_PARAMETERS - 1, + Zotero.Promise.coroutine(function* (chunk2) { + var placeholders = Array(chunk2.length).fill('?').join(','); + + sql = 'UPDATE items SET synced=0, clientDateModified=? ' + + 'WHERE itemID IN (' + placeholders + ')' + yield Zotero.DB.queryAsync(sql, [Zotero.DB.transactionDateTime].concat(chunk2)); + + yield Zotero.Items.reload(oldItemIDs, ['primaryData', 'tags'], true); + }) + ); + + done += chunk.length; + }.bind(this)); + + if (onProgress) { + onProgress(done, tagIDs.length); + } + }.bind(this) + ); // Also delete tag color setting // @@ -371,16 +400,44 @@ Zotero.Tags = new function() { for (let i=0; i<deletedNames.length; i++) { yield this.setColor(libraryID, deletedNames[i], false); } + + Zotero.debug(`Removed ${tagIDs.length} ${Zotero.Utilities.pluralize(tagIDs.length, 'tag')} ` + + `in ${new Date() - d} ms`); }); /** + * @param {Integer} libraryID + * @return {Integer[]} - An array of tagIDs + */ + this.getAutomaticInLibrary = function (libraryID) { + var sql = "SELECT DISTINCT tagID FROM itemTags JOIN items USING (itemID) " + + "WHERE type=1 AND libraryID=?" + return Zotero.DB.columnQueryAsync(sql, libraryID); + }; + + + /** + * Remove all automatic tags in the given library + */ + this.removeAutomaticFromLibrary = async function (libraryID, onProgress) { + var tagIDs = await this.getAutomaticInLibrary(libraryID); + if (onProgress) { + onProgress(0, tagIDs.length); + } + return this.removeFromLibrary(libraryID, tagIDs, onProgress); + }; + + + /** * Delete obsolete tags from database * * @param {Number|Number[]} [tagIDs] - tagID or array of tagIDs to purge * @return {Promise} */ this.purge = Zotero.Promise.coroutine(function* (tagIDs) { + var d = new Date(); + if (!_initialized) { throw new Zotero.Exception.UnloadedDataException("Tags not yet loaded"); } @@ -399,9 +456,17 @@ Zotero.Tags = new function() { if (tagIDs) { let sql = "CREATE TEMPORARY TABLE tagDelete (tagID INT PRIMARY KEY)"; yield Zotero.DB.queryAsync(sql); - for (let i=0; i<tagIDs.length; i++) { - yield Zotero.DB.queryAsync("INSERT OR IGNORE INTO tagDelete VALUES (?)", tagIDs[i]); - } + yield Zotero.Utilities.Internal.forEachChunkAsync( + tagIDs, + Zotero.DB.MAX_BOUND_PARAMETERS, + function (chunk) { + return Zotero.DB.queryAsync( + "INSERT OR IGNORE INTO tagDelete VALUES " + + Array(chunk.length).fill('(?)').join(', '), + chunk + ); + } + ); sql = "SELECT tagID AS id, name FROM tagDelete JOIN tags USING (tagID) " + "WHERE tagID NOT IN (SELECT tagID FROM itemTags)"; var toDelete = yield Zotero.DB.queryAsync(sql); @@ -409,8 +474,7 @@ Zotero.Tags = new function() { // Look for orphaned tags else { var sql = "CREATE TEMPORARY TABLE tagDelete AS " - + "SELECT tagID FROM tags WHERE tagID " - + "NOT IN (SELECT tagID FROM itemTags)"; + + "SELECT tagID FROM tags WHERE tagID NOT IN (SELECT tagID FROM itemTags)"; yield Zotero.DB.queryAsync(sql); sql = "CREATE INDEX tagDelete_tagID ON tagDelete(tagID)"; @@ -452,6 +516,9 @@ Zotero.Tags = new function() { Zotero.Notifier.queue('delete', 'tag', ids, notifierData); Zotero.Prefs.set('purge.tags', false); + + Zotero.debug(`Purged ${toDelete.length} ${Zotero.Utilities.pluralize(toDelete.length, 'tag')} ` + + `in ${new Date() - d} ms`); }); diff --git a/chrome/locale/en-US/zotero/zotero.dtd b/chrome/locale/en-US/zotero/zotero.dtd @@ -156,6 +156,7 @@ <!ENTITY zotero.tagSelector.loadingTags "Loading tags…"> <!ENTITY zotero.tagSelector.showAutomatic "Show Automatic"> <!ENTITY zotero.tagSelector.displayAllInLibrary "Display All Tags in This Library"> +<!ENTITY zotero.tagSelector.deleteAutomaticInLibrary "Delete Automatic Tags in This Library…"> <!ENTITY zotero.tagSelector.clearAll "Deselect All"> <!ENTITY zotero.tagSelector.assignColor "Assign Color…"> <!ENTITY zotero.tagSelector.renameTag "Rename Tag…"> diff --git a/chrome/locale/en-US/zotero/zotero.properties b/chrome/locale/en-US/zotero/zotero.properties @@ -234,6 +234,8 @@ pane.tagSelector.rename.title = Rename Tag pane.tagSelector.rename.message = Please enter a new name for this tag.\n\nThe tag will be changed in all associated items. pane.tagSelector.delete.title = Delete Tag pane.tagSelector.delete.message = Are you sure you want to delete this tag?\n\nThe tag will be removed from all items. +pane.tagSelector.deleteAutomatic.title = Delete Automatic Tags +pane.tagSelector.deleteAutomatic.message = Are you sure you want to delete %1$S automatic tag in this library?;Are you sure you want to delete %1$S automatic tags in this library? pane.tagSelector.numSelected.none = 0 tags selected pane.tagSelector.numSelected.singular = %S tag selected pane.tagSelector.numSelected.plural = %S tags selected diff --git a/test/tests/tagsTest.js b/test/tests/tagsTest.js @@ -39,6 +39,36 @@ describe("Zotero.Tags", function () { }); describe("#removeFromLibrary()", function () { + it("should remove tags in given library", function* () { + var libraryID = Zotero.Libraries.userLibraryID; + var groupLibraryID = (yield getGroup()).libraryID; + + var tags = []; + var items = []; + yield Zotero.DB.executeTransaction(function* () { + for (let i = 0; i < 10; i++) { + let tagName = Zotero.Utilities.randomString(); + tags.push(tagName); + let item = createUnsavedDataObject('item'); + item.addTag(tagName); + yield item.save(); + items.push(item); + } + }); + + var groupTagName = Zotero.Utilities.randomString(); + var groupItem = createUnsavedDataObject('item', { libraryID: groupLibraryID }); + groupItem.addTag(groupTagName); + yield groupItem.saveTx(); + + var tagIDs = tags.map(tag => Zotero.Tags.getID(tag)); + yield Zotero.Tags.removeFromLibrary(libraryID, tagIDs); + items.forEach(item => assert.lengthOf(item.getTags(), 0)); + + // Group item should still have the tag + assert.lengthOf(groupItem.getTags(), 1); + }) + it("should reload tags of associated items", function* () { var libraryID = Zotero.Libraries.userLibraryID;