Tags overhaul [DB reupgrade]

- Simplified schema
- Tags are now added without reloading entire tag selector
  - On my system, adding 400 tags to an item (separately, with the tag
    selector updating each time) went from 59 seconds to 42. (Given that
    it takes only 13 seconds with the tag selector closed, though,
    there's clearly more work to be done.)
- Tag selector now uses HTML flexbox (in identical fashion, for now, but
  with the possibility of fancier changes later, and with streamlined
  logic thanks to the flexbox 'order' property)
- Various async fixes
- Tests

test/tests/supportTest.js

@@ -77,7 +77,7 @@ describe("Support Functions for Unit Testing", function() {
 
 			let tags = data.itemWithTags.tags;
 			for (let i=0; i<tags.length; i++) {
-				let tagID = Zotero.Tags.getID(zItem.libraryID, tags[i].tag);
+				let tagID = yield Zotero.Tags.getID(tags[i].tag);
 				assert.ok(tagID, '"' + tags[i].tag + '" tag was inserted into the database');
 				assert.ok(zItem.hasTag(tags[i].tag), '"' + tags[i].tag + '" tag was assigned to item');
 			}

test/tests/tagSelectorTest.js

@@ -3,6 +3,40 @@
 describe("Tag Selector", function () {
 	var win, doc, collectionsView;
 	
+	var clearTagColors = Zotero.Promise.coroutine(function* (libraryID) {
+		var tagColors = yield Zotero.Tags.getColors(libraryID);
+		for (let name of tagColors.keys()) {
+			yield Zotero.Tags.setColor(libraryID, name, false);
+		}
+	});
+	
+	function getColoredTags() {
+		var tagSelector = doc.getElementById('zotero-tag-selector');
+		var tagsBox = tagSelector.id('tags-box');
+		var elems = tagsBox.getElementsByTagName('button');
+		var names = [];
+		for (let i = 0; i < elems.length; i++) {
+			if (elems[i].style.order < 0) {
+				names.push(elems[i].textContent);
+			}
+		}
+		return names;
+	}
+	
+	function getRegularTags() {
+		var tagSelector = doc.getElementById('zotero-tag-selector');
+		var tagsBox = tagSelector.id('tags-box');
+		var elems = tagsBox.getElementsByTagName('button');
+		var names = [];
+		for (let i = 0; i < elems.length; i++) {
+			if (elems[i].style.order >= 0 && elems[i].style.display != 'none') {
+				names.push(elems[i].textContent);
+			}
+		}
+		return names;
+	}
+	
+	
 	before(function* () {
 		win = yield loadZoteroPane();
 		doc = win.document;
@@ -11,6 +45,10 @@ describe("Tag Selector", function () {
 		// Wait for things to settle
 		yield Zotero.Promise.delay(100);
 	});
+	beforeEach(function* () {
+		var libraryID = Zotero.Libraries.userLibraryID;
+		yield clearTagColors(libraryID);
+	})
 	after(function () {
 		win.close();
 	});
@@ -27,7 +65,7 @@ describe("Tag Selector", function () {
 	}
 	
 	describe("#notify()", function () {
-		it("should add a tag when added to an item in the current view", function* () {
+		it("should add a tag when added to an item in the library root", function* () {
 			var promise, tagSelector;
 			
 			if (collectionsView.selection.currentIndex != 0) {
@@ -41,6 +79,10 @@ describe("Tag Selector", function () {
 			item.setTags([
 				{
 					tag: 'A'
+				},
+				{
+					tag: 'B',
+					type: 1
 				}
 			]);
 			promise = waitForTagSelector();
@@ -48,8 +90,11 @@ describe("Tag Selector", function () {
 			yield promise;
 			
 			// Tag selector should have at least one tag
-			tagSelector = doc.getElementById('zotero-tag-selector');
-			assert.isAbove(tagSelector.getVisible().length, 0);
+			assert.isAbove(getRegularTags().length, 1);
+		});
+		
+		it("should add a tag when an item is added in a collection", function* () {
+			var promise, tagSelector;
 			
 			// Add collection
 			promise = waitForTagSelector();
@@ -57,14 +102,13 @@ describe("Tag Selector", function () {
 			yield promise;
 			
 			// Tag selector should be empty in new collection
-			tagSelector = doc.getElementById('zotero-tag-selector');
-			assert.equal(tagSelector.getVisible().length, 0);
+			assert.equal(getRegularTags().length, 0);
 			
 			// Add item with tag to collection
 			var item = createUnsavedDataObject('item');
 			item.setTags([
 				{
-					tag: 'B'
+					tag: 'C'
 				}
 			]);
 			item.setCollections([collection.id]);
@@ -72,9 +116,80 @@ describe("Tag Selector", function () {
 			yield item.saveTx();
 			yield promise;
 			
+			// Tag selector should show the new item's tag
+			assert.equal(getRegularTags().length, 1);
+		})
+		
+		it("should add a tag when an item is added to a collection", function* () {
+			var promise, tagSelector;
+			
+			// Add collection
+			promise = waitForTagSelector();
+			var collection = yield createDataObject('collection');
+			yield promise;
+			
+			// Tag selector should be empty in new collection
+			assert.equal(getRegularTags().length, 0);
+			
+			// Add item with tag to library root
+			var item = createUnsavedDataObject('item');
+			item.setTags([
+				{
+					tag: 'C'
+				}
+			]);
+			promise = waitForTagSelector()
+			yield item.saveTx();
+			yield promise;
+			
+			// Tag selector should still be empty in collection
+			assert.equal(getRegularTags().length, 0);
+			
+			item.setCollections([collection.id]);
+			promise = waitForTagSelector();
+			yield item.saveTx();
+			yield promise;
+			
 			// Tag selector should show the new item's tag
 			tagSelector = doc.getElementById('zotero-tag-selector');
-			assert.equal(tagSelector.getVisible().length, 1);
+			assert.equal(getRegularTags().length, 1);
+		})
+		
+		it("shouldn't re-insert a new tag that matches an existing color", function* () {
+			var libraryID = Zotero.Libraries.userLibraryID;
+			
+			/*// Remove all tags in library
+			var tags = yield Zotero.Tags.getAll(libraryID);
+			tags.forEach(function (tag) {
+				var tagID = yield Zotero.Tags.getID(tag);
+				yield Zotero.Tags.removeFromLibrary(libraryID, tagID);
+			});*/
+			
+			// Add B and A as colored tags without any items
+			yield Zotero.Tags.setColor(libraryID, "B", '#990000');
+			yield Zotero.Tags.setColor(libraryID, "A", '#CC9933');
+			
+			// Add A to an item to make it a real tag
+			var item = createUnsavedDataObject('item');
+			item.setTags([
+				{
+					tag: "A"
+				}
+			]);
+			var promise = waitForTagSelector();
+			yield item.saveTx();
+			yield promise;
+			
+			var tagSelector = doc.getElementById('zotero-tag-selector');
+			var tagElems = tagSelector.id('tags-box').childNodes;
+			
+			// Make sure the colored tags are still in the right position
+			var tags = new Map();
+			for (let i = 0; i < tagElems.length; i++) {
+				tags.set(tagElems[i].textContent, tagElems[i].style.order);
+			}
+			assert.isBelow(parseInt(tags.get("B")), 0);
+			assert.isBelow(parseInt(tags.get("B")), parseInt(tags.get("A")));
 		})
 		
 		it("should remove a tag when an item is removed from a collection", function* () {
@@ -91,13 +206,12 @@ describe("Tag Selector", function () {
 				}
 			]);
 			item.setCollections([collection.id]);
-			promise = waitForTagSelector()
+			promise = waitForTagSelector();
 			yield item.saveTx();
 			yield promise;
 			
 			// Tag selector should show the new item's tag
-			var tagSelector = doc.getElementById('zotero-tag-selector');
-			assert.equal(tagSelector.getVisible().length, 1);
+			assert.equal(getRegularTags().length, 1);
 			
 			item.setCollections();
 			promise = waitForTagSelector();
@@ -105,8 +219,7 @@ describe("Tag Selector", function () {
 			yield promise;
 			
 			// Tag selector shouldn't show the removed item's tag
-			tagSelector = doc.getElementById('zotero-tag-selector');
-			assert.equal(tagSelector.getVisible().length, 0);
+			assert.equal(getRegularTags().length, 0);
 		})
 		
 		it("should remove a tag when an item in a collection is moved to the trash", function* () {
@@ -128,8 +241,7 @@ describe("Tag Selector", function () {
 			yield promise;
 			
 			// Tag selector should show the new item's tag
-			var tagSelector = doc.getElementById('zotero-tag-selector');
-			assert.equal(tagSelector.getVisible().length, 1);
+			assert.equal(getRegularTags().length, 1);
 			
 			// Move item to trash
 			item.deleted = true;
@@ -138,8 +250,96 @@ describe("Tag Selector", function () {
 			yield promise;
 			
 			// Tag selector shouldn't show the deleted item's tag
-			tagSelector = doc.getElementById('zotero-tag-selector');
-			assert.equal(tagSelector.getVisible().length, 0);
+			assert.equal(getRegularTags().length, 0);
+		})
+		
+		it("should remove a tag when a tag is deleted for a library", function* () {
+			yield selectLibrary(win);
+			
+			var item = createUnsavedDataObject('item');
+			item.setTags([
+				{
+					tag: 'A'
+				}
+			]);
+			var promise = waitForTagSelector();
+			yield item.saveTx();
+			yield promise;
+			
+			// Tag selector should show the new item's tag
+			assert.include(getRegularTags(), "A");
+			
+			// Remove tag from library
+			promise = waitForTagSelector();
+			var dialogPromise = waitForDialog();
+			var tagSelector = doc.getElementById('zotero-tag-selector');
+			yield tagSelector.delete("A");
+			yield promise;
+			
+			// Tag selector shouldn't show the deleted item's tag
+			assert.notInclude(getRegularTags(), "A");
 		})
 	})
+	
+	describe("#rename()", function () {
+		it("should rename a tag and update the tag selector", function* () {
+			yield selectLibrary(win);
+			
+			var tag = Zotero.Utilities.randomString();
+			var newTag = Zotero.Utilities.randomString();
+			var item = createUnsavedDataObject('item');
+			item.setTags([
+				{
+					tag: tag
+				}
+			]);
+			var promise = waitForTagSelector();
+			yield item.saveTx();
+			yield promise;
+			
+			var tagSelector = doc.getElementById('zotero-tag-selector');
+			promise = waitForTagSelector();
+			var promptPromise = waitForWindow("chrome://global/content/commonDialog.xul", function (dialog) {
+				dialog.document.getElementById('loginTextbox').value = newTag;
+				dialog.document.documentElement.acceptDialog();
+			})
+			yield tagSelector.rename(tag);
+			yield promise;
+			
+			var tags = getRegularTags();
+			assert.include(tags, newTag);
+		})
+	})
+	
+	describe("#_openColorPickerWindow()", function () {
+		it("should assign a color to a tag", function* () {
+			yield selectLibrary(win);
+			var tag = "b " + Zotero.Utilities.randomString();
+			var item = createUnsavedDataObject('item');
+			item.setTags([
+				{
+					tag: "a"
+				},
+				{
+					tag: tag
+				}
+			]);
+			var promise = waitForTagSelector();
+			yield item.saveTx();
+			yield promise;
+			
+			var tagSelector = doc.getElementById('zotero-tag-selector');
+			
+			assert.include(getRegularTags(), "a");
+			
+			var dialogPromise = waitForDialog(false, undefined, 'chrome://zotero/content/tagColorChooser.xul');
+			var tagSelectorPromise = waitForTagSelector();
+			yield tagSelector._openColorPickerWindow(tag);
+			yield dialogPromise;
+			yield tagSelectorPromise;
+			
+			assert.include(getColoredTags(), tag);
+			assert.notInclude(getRegularTags(), tag);
+		})
+	});
 })

test/tests/tagsTest.js

@@ -8,7 +8,7 @@ describe("Zotero.Tags", function () {
 			item.addTag(tagName);
 			yield item.saveTx();
 			
-			assert.typeOf(Zotero.Tags.getID(Zotero.Libraries.userLibraryID, tagName), "number");
+			assert.typeOf((yield Zotero.Tags.getID(tagName)), "number");
 		})
 	})
 	
@@ -19,32 +19,49 @@ describe("Zotero.Tags", function () {
 			item.addTag(tagName);
 			yield item.saveTx();
 			
-			var tagID = Zotero.Tags.getID(Zotero.Libraries.userLibraryID, tagName);
-			assert.equal(Zotero.Tags.getName(Zotero.Libraries.userLibraryID, tagID), tagName);
+			var libraryID = Zotero.Libraries.userLibraryID;
+			var tagID = yield Zotero.Tags.getID(tagName);
+			assert.equal((yield Zotero.Tags.getName(tagID)), tagName);
+		})
+	})
+	
+	describe("#removeFromLibrary()", function () {
+		it("should reload tags of associated items", function* () {
+			var libraryID = Zotero.Libraries.userLibraryID;
+			
+			var tagName = Zotero.Utilities.randomString();
+			var item = createUnsavedDataObject('item');
+			item.addTag(tagName);
+			yield item.saveTx();
+			assert.lengthOf(item.getTags(), 1);
+			
+			var tagID = yield Zotero.Tags.getID(tagName);
+			yield Zotero.Tags.removeFromLibrary(libraryID, tagID);
+			assert.lengthOf(item.getTags(), 0);
 		})
 	})
 	
 	describe("#purge()", function () {
 		it("should remove orphaned tags", function* () {
 			var libraryID = Zotero.Libraries.userLibraryID;
+			
 			var tagName = Zotero.Utilities.randomString();
 			var item = createUnsavedDataObject('item');
 			item.addTag(tagName);
 			yield item.saveTx();
 			
-			var tagID = Zotero.Tags.getID(libraryID, tagName);
+			var tagID = yield Zotero.Tags.getID(tagName);
 			assert.typeOf(tagID, "number");
 			
 			yield item.eraseTx();
 			
-			assert.equal(Zotero.Tags.getName(libraryID, tagID), tagName);
+			assert.equal((yield Zotero.Tags.getName(tagID)), tagName);
 			
 			yield Zotero.DB.executeTransaction(function* () {
 				yield Zotero.Tags.purge();
 			});
 			
-			yield Zotero.Tags.load(libraryID);
-			assert.isFalse(Zotero.Tags.getName(libraryID, tagID));
+			assert.isFalse(yield Zotero.Tags.getName(tagID));
 		})
 	})
 })

test/tests/tagsboxTest.js

@@ -0,0 +1,125 @@
+"use strict";
+
+describe("Item Tags Box", function () {
+	var win, doc, collectionsView;
+	
+	before(function* () {
+		win = yield loadZoteroPane();
+		doc = win.document;
+		
+		// Wait for things to settle
+		yield Zotero.Promise.delay(100);
+	});
+	after(function () {
+		win.close();
+	});
+	
+	function waitForTagsBox() {
+		var deferred = Zotero.Promise.defer();
+		var tagsbox = doc.getElementById('zotero-editpane-tags');
+		var onRefresh = function (event) {
+			tagsbox.removeEventListener('refresh', onRefresh);
+			deferred.resolve();
+		}
+		tagsbox.addEventListener('refresh', onRefresh);
+		return deferred.promise;
+	}
+	
+	describe("#notify()", function () {
+		it("should update an existing tag on rename", function* () {
+			var tag = Zotero.Utilities.randomString();
+			var newTag = Zotero.Utilities.randomString();
+			
+			var tabbox = doc.getElementById('zotero-view-tabbox');
+			tabbox.selectedIndex = 0;
+			
+			var item = createUnsavedDataObject('item');
+			item.setTags([
+				{
+					tag: tag
+				}
+			]);
+			yield item.saveTx();
+			
+			var tabbox = doc.getElementById('zotero-view-tabbox');
+			tabbox.selectedIndex = 2;
+			yield waitForTagsBox();
+			var tagsbox = doc.getElementById('zotero-editpane-tags');
+			var rows = tagsbox.id('tagRows').getElementsByTagName('row');
+			assert.equal(rows.length, 1);
+			assert.equal(rows[0].textContent, tag);
+			
+			yield Zotero.Tags.rename(Zotero.Libraries.userLibraryID, tag, newTag);
+			
+			var rows = tagsbox.id('tagRows').getElementsByTagName('row');
+			assert.equal(rows.length, 1);
+			assert.equal(rows[0].textContent, newTag);
+		})
+		
+		it("should update when a tag's color is removed", function* () {
+			var libraryID = Zotero.Libraries.userLibraryID;
+			
+			var tag = Zotero.Utilities.randomString();
+			var tabbox = doc.getElementById('zotero-view-tabbox');
+			tabbox.selectedIndex = 0;
+			
+			yield Zotero.Tags.setColor(libraryID, tag, "#990000");
+			var item = createUnsavedDataObject('item');
+			item.setTags([
+				{
+					tag: tag,
+				},
+				{
+					tag: "_A"
+				}
+			]);
+			yield item.saveTx();
+			
+			var tabbox = doc.getElementById('zotero-view-tabbox');
+			tabbox.selectedIndex = 2;
+			yield waitForTagsBox();
+			var tagsbox = doc.getElementById('zotero-editpane-tags');
+			var rows = tagsbox.id('tagRows').getElementsByTagName('row');
+			
+			// Colored tags aren't sorted first, for now
+			assert.notOk(rows[0].getElementsByTagName('label')[0].style.color);
+			assert.ok(rows[1].getElementsByTagName('label')[0].style.color);
+			assert.equal(rows[0].textContent, "_A");
+			assert.equal(rows[1].textContent, tag);
+			
+			yield Zotero.Tags.setColor(libraryID, tag, false);
+			
+			assert.notOk(rows[1].getElementsByTagName('label')[0].style.color);
+		})
+		
+		it("should update when a tag is removed from the library", function* () {
+			var tag = Zotero.Utilities.randomString();
+			
+			var tabbox = doc.getElementById('zotero-view-tabbox');
+			tabbox.selectedIndex = 0;
+			
+			var item = createUnsavedDataObject('item');
+			item.setTags([
+				{
+					tag: tag
+				}
+			]);
+			yield item.saveTx();
+			
+			var tabbox = doc.getElementById('zotero-view-tabbox');
+			tabbox.selectedIndex = 2;
+			yield waitForTagsBox();
+			var tagsbox = doc.getElementById('zotero-editpane-tags');
+			var rows = tagsbox.id('tagRows').getElementsByTagName('row');
+			assert.equal(rows.length, 1);
+			assert.equal(rows[0].textContent, tag);
+			
+			yield Zotero.Tags.removeFromLibrary(
+				Zotero.Libraries.userLibraryID, (yield Zotero.Tags.getID(tag))
+			);
+			
+			var rows = tagsbox.id('tagRows').getElementsByTagName('row');
+			assert.equal(rows.length, 0);
+		})
+	})
+})