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

When clearing automatic tags, don't delete manual tags with same name

Browse source
This commit is contained in:
Dan Stillman 2019-03-05 07:35:07 -05:00
parent fc43514ff0
commit d7dc5670d5
2 changed files with 128 additions and 97 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

122
chrome/content/zotero/xpcom/data/tags.js
View file

@ -281,14 +281,23 @@ Zotero.Tags = new function() {
/** /**
* @param {Integer} libraryID
* @param {Integer[]} tagIDs
* @param {Function} [onProgress]
* @param {Integer[]} [types]
* @return {Promise} * @return {Promise}
*/ */
this.removeFromLibrary = Zotero.Promise.coroutine(function* (libraryID, tagIDs, onProgress) { this.removeFromLibrary = Zotero.Promise.coroutine(function* (libraryID, tagIDs, onProgress, types) {
var d = new Date(); var d = new Date();
tagIDs = Zotero.flattenArguments(tagIDs); if (!Array.isArray(tagIDs)) {
tagIDs = [tagIDs];
}
if (types && !Array.isArray(types)) {
types = [types];
}
var deletedNames = []; var colors = this.getColors(libraryID);
var done = 0; var done = 0;
yield Zotero.Utilities.Internal.forEachChunkAsync( yield Zotero.Utilities.Internal.forEachChunkAsync(
@ -296,90 +305,77 @@ Zotero.Tags = new function() {
100, 100,
async function (chunk) { async function (chunk) {
await Zotero.DB.executeTransaction(function* () { await Zotero.DB.executeTransaction(function* () {
var oldItemIDs = []; var rowIDs = [];
var itemIDs = [];
var notifierPairs = []; var uniqueTags = new Set();
var notifierIDs = [];
var notifierData = {}; var notifierData = {};
var a = new Date();
var sql = 'SELECT tagID, itemID FROM itemTags JOIN items USING (itemID) ' var sql = 'SELECT IT.ROWID AS rowID, tagID, itemID, type FROM itemTags IT '
+ 'JOIN items USING (itemID) '
+ 'WHERE libraryID=? AND tagID IN (' + 'WHERE libraryID=? AND tagID IN ('
+ Array(chunk.length).fill('?').join(', ') + Array(chunk.length).fill('?').join(', ')
+ ') ORDER BY tagID'; + ') ';
var chunkTagItems = yield Zotero.DB.queryAsync(sql, [libraryID, ...chunk]); if (types) {
var i = 0; sql += 'AND type IN (' + types.join(', ') + ') ';
}
chunk.sort((a, b) => a - b); sql += 'ORDER BY tagID, type';
var rows = yield Zotero.DB.queryAsync(sql, [libraryID, ...chunk]);
for (let tagID of chunk) { for (let { rowID, tagID, itemID, type } of rows) {
uniqueTags.add(tagID);
let name = this.getName(tagID); let name = this.getName(tagID);
if (name === false) { if (name === false) {
continue; continue;
} }
deletedNames.push(name);
// Since we're performing the DELETE query directly, rowIDs.push(rowID);
// get the list of items that will need their tags reloaded, itemIDs.push(itemID);
// and generate data for item-tag notifications
let itemIDs = [] let ids = itemID + '-' + tagID;
while (i < chunkTagItems.length && chunkTagItems[i].tagID == tagID) { notifierIDs.push(ids);
itemIDs.push(chunkTagItems[i].itemID); notifierData[ids] = {
i++; libraryID: libraryID,
tag: name,
type
};
// If we're deleting the tag and not just a specific type, also clear any
// tag color
if (colors.has(name) && !types) {
yield this.setColor(libraryID, name, false);
} }
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) { if (itemIDs.length) {
Zotero.Notifier.queue('remove', 'item-tag', notifierPairs, notifierData); Zotero.Notifier.queue('remove', 'item-tag', notifierIDs, notifierData);
} }
var sql = "DELETE FROM itemTags WHERE tagID IN (" sql = "DELETE FROM itemTags WHERE ROWID IN (" + rowIDs.join(", ") + ")";
+ Array(chunk.length).fill('?').join(', ') + ") AND itemID IN " yield Zotero.DB.queryAsync(sql);
+ "(SELECT itemID FROM items WHERE libraryID=?)";
yield Zotero.DB.queryAsync(sql, chunk.concat([libraryID]));
yield this.purge(chunk); yield this.purge(chunk);
// Update internal timestamps on all items that had these tags // Update internal timestamps on all items that had these tags
yield Zotero.Utilities.Internal.forEachChunkAsync( yield Zotero.Utilities.Internal.forEachChunkAsync(
Zotero.Utilities.arrayUnique(oldItemIDs), Zotero.Utilities.arrayUnique(itemIDs),
Zotero.DB.MAX_BOUND_PARAMETERS - 1, Zotero.DB.MAX_BOUND_PARAMETERS - 1,
Zotero.Promise.coroutine(function* (chunk2) { async function (chunk) {
var placeholders = Array(chunk2.length).fill('?').join(','); var sql = 'UPDATE items SET synced=0, clientDateModified=? '
+ 'WHERE itemID IN (' + Array(chunk.length).fill('?').join(',') + ')';
await Zotero.DB.queryAsync(sql, [Zotero.DB.transactionDateTime].concat(chunk));
sql = 'UPDATE items SET synced=0, clientDateModified=? ' await Zotero.Items.reload(itemIDs, ['primaryData', 'tags'], true);
+ '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; if (onProgress) {
done += uniqueTags.size;
onProgress(done, tagIDs.length);
}
}.bind(this)); }.bind(this));
if (onProgress) {
onProgress(done, tagIDs.length);
}
}.bind(this) }.bind(this)
); );
// Also delete tag color setting
//
// Note that this isn't done in purge(), so the setting will not
// be removed if the tag is just removed from all items without
// being explicitly deleted.
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')} ` Zotero.debug(`Removed ${tagIDs.length} ${Zotero.Utilities.pluralize(tagIDs.length, 'tag')} `
+ `in ${new Date() - d} ms`); + `in ${new Date() - d} ms`);
}); });
@ -429,6 +425,10 @@ Zotero.Tags = new function() {
tagIDs = Zotero.flattenArguments(tagIDs); tagIDs = Zotero.flattenArguments(tagIDs);
} }
if (tagIDs && !tagIDs.length) {
return;
}
Zotero.DB.requireTransaction(); Zotero.DB.requireTransaction();
var sql; var sql;

103
test/tests/tagsTest.js
View file

@ -39,60 +39,91 @@ describe("Zotero.Tags", function () {
}); });
describe("#removeFromLibrary()", function () { describe("#removeFromLibrary()", function () {
it("should remove tags in given library", async function () { it("should delete tags in given library", async function () {
var libraryID = Zotero.Libraries.userLibraryID; var libraryID = Zotero.Libraries.userLibraryID;
var groupLibraryID = (await getGroup()).libraryID; var groupLibraryID = (await getGroup()).libraryID;
var tags = []; var item1 = await createDataObject('item', { tags: [{ tag: 'a' }, { tag: 'b', type: 1 }] });
var items = []; var item2 = await createDataObject('item', { tags: [{ tag: 'b' }, { tag: 'c', type: 1 }] });
await Zotero.DB.executeTransaction(async function () { var item3 = await createDataObject('item', { tags: [{ tag: 'd', type: 1 }] });
for (let i = 0; i < 10; i++) { var item4 = await createDataObject('item', { libraryID: groupLibraryID, tags: [{ tag: 'a' }, { tag: 'b', type: 1 }] });
let tagName = Zotero.Utilities.randomString();
tags.push(tagName);
let item = createUnsavedDataObject('item', { tags: [tagName] });
await item.save();
items.push(item);
}
});
var groupTagName = tags[0]; var tagIDs = ['a', 'd'].map(x => Zotero.Tags.getID(x));
var groupItem = await createDataObject(
'item',
{
libraryID: groupLibraryID,
tags: [groupTagName]
}
);
var tagIDs = tags.map(tag => Zotero.Tags.getID(tag));
await Zotero.Tags.removeFromLibrary(libraryID, tagIDs); await Zotero.Tags.removeFromLibrary(libraryID, tagIDs);
items.forEach(item => assert.lengthOf(item.getTags(), 0));
// Group item should still have the tag assert.sameDeepMembers(item1.getTags(), [{ tag: 'b', type: 1 }]);
assert.sameDeepMembers(groupItem.getTags(), [{ tag: groupTagName }]); assert.sameDeepMembers(item2.getTags(), [{ tag: 'b' }, { tag: 'c', type: 1 }]);
assert.lengthOf(item3.getTags(), 0);
assert.equal(Zotero.Tags.getID('a'), tagIDs[0]);
assert.isFalse(Zotero.Tags.getID('d'));
// Group item should still have all tags
assert.sameDeepMembers(item4.getTags(), [{ tag: 'a' }, { tag: 'b', type: 1 }]);
assert.equal( assert.equal(
await Zotero.DB.valueQueryAsync( await Zotero.DB.valueQueryAsync(
"SELECT COUNT(*) FROM itemTags WHERE itemID=?", "SELECT COUNT(*) FROM itemTags WHERE itemID=?",
groupItem.id item4.id
), ),
1 2
); );
}); });
it("should reload tags of associated items", function* () { it("should remove tags of a given type", async function () {
var libraryID = Zotero.Libraries.userLibraryID;
var groupLibraryID = (await getGroup()).libraryID;
var item1 = await createDataObject('item', { tags: [{ tag: 'a' }, { tag: 'b', type: 1 }] });
var item2 = await createDataObject('item', { tags: [{ tag: 'b' }, { tag: 'c', type: 1 }] });
var item3 = await createDataObject('item', { tags: [{ tag: 'd', type: 1 }] });
var item4 = await createDataObject('item', { libraryID: groupLibraryID, tags: [{ tag: 'a' }, { tag: 'b', type: 1 }] });
var tagIDs = ['a', 'b', 'c', 'd'].map(x => Zotero.Tags.getID(x));
var tagType = 1;
await Zotero.Tags.removeFromLibrary(libraryID, tagIDs, null, tagType);
assert.sameDeepMembers(item1.getTags(), [{ tag: 'a' }]);
assert.sameDeepMembers(item2.getTags(), [{ tag: 'b' }]);
assert.lengthOf(item3.getTags(), 0);
assert.isFalse(Zotero.Tags.getID('d'));
// Group item should still have all tags
assert.sameDeepMembers(item4.getTags(), [{ tag: 'a' }, { tag: 'b', type: 1 }]);
assert.equal(
await Zotero.DB.valueQueryAsync(
"SELECT COUNT(*) FROM itemTags WHERE itemID=?",
item4.id
),
2
);
});
it("should delete colored tag when removing tag", async function () {
var libraryID = Zotero.Libraries.userLibraryID; var libraryID = Zotero.Libraries.userLibraryID;
var tagName = Zotero.Utilities.randomString(); var tag = Zotero.Utilities.randomString();
var item = createUnsavedDataObject('item'); var item = await createDataObject('item', { tags: [{ tag: tag, type: 1 }] });
item.addTag(tagName); await Zotero.Tags.setColor(libraryID, tag, '#ABCDEF', 0);
yield item.saveTx();
assert.lengthOf(item.getTags(), 1); await Zotero.Tags.removeFromLibrary(libraryID, [Zotero.Tags.getID(tag)]);
var tagID = Zotero.Tags.getID(tagName);
yield Zotero.Tags.removeFromLibrary(libraryID, tagID);
assert.lengthOf(item.getTags(), 0); assert.lengthOf(item.getTags(), 0);
}) assert.isFalse(Zotero.Tags.getColor(libraryID, tag));
});
it("shouldn't delete colored tag when removing tag of a given type", async function () {
var libraryID = Zotero.Libraries.userLibraryID;
var tag = Zotero.Utilities.randomString();
var item = await createDataObject('item', { tags: [{ tag: tag, type: 1 }] });
await Zotero.Tags.setColor(libraryID, tag, '#ABCDEF', 0);
await Zotero.Tags.removeFromLibrary(libraryID, [Zotero.Tags.getID(tag)], null, 1);
assert.lengthOf(item.getTags(), 0);
assert.ok(Zotero.Tags.getColor(libraryID, tag));
});
}) })
describe("#purge()", function () { describe("#purge()", function () {