Fix handling of old-style 'condition'/'savedSearch' conditions

Strip library id prefix in addCondition() and _loadConditions(), so the internal code can always expect just a key.
Juris-M · Feb 21, 2017 · 64d73cf · 64d73cf
1 parent d32f234
commit 64d73cf
Show file tree

Hide file tree

Showing 3 changed files with 52 additions and 33 deletions.
diff --git a/chrome/content/zotero/xpcom/data/search.js b/chrome/content/zotero/xpcom/data/search.js
@@ -374,6 +374,14 @@ Zotero.Search.prototype.addCondition = function (condition, operator, value, req
 		}
 		return this.addCondition('savedSearch', operator, key, required);
 	}
+	// Parse old-style collection/savedSearch conditions ('0_ABCD2345' -> 'ABCD2345')
+	else if (condition == 'collection' || condition == 'savedSearch') {
+		if (value.includes('_')) {
+			Zotero.logError(`'condition' value '${value}' should be an object key`);
+			let [_, objKey] = value.split('_');
+			value = objKey;
+		}
+	}
 
 	var searchConditionID = ++this._maxSearchConditionID;
 
@@ -1150,15 +1158,8 @@ Zotero.Search.prototype._buildQuery = Zotero.Promise.coroutine(function* () {
 						let objectType = condition.name == 'collection' ? 'collection' : 'search';
 						let objectTypeClass = Zotero.DataObjectUtilities.getObjectsClassForObjectType(objectType);
 
-						// Old-style library-key hash
-						if (objKey.indexOf('_') != -1) {
-							[objLibraryID, objKey] = objKey.split('_');
-							if (objLibraryID === "0") {
-								objLibraryID = Zotero.Libraries.userLibraryID;
-							}
-						}
 						// libraryID assigned on search
-						else if (this.libraryID !== null) {
+						if (this.libraryID !== null) {
 							objLibraryID = this.libraryID;
 						}
 

diff --git a/chrome/content/zotero/xpcom/data/searches.js b/chrome/content/zotero/xpcom/data/searches.js
@@ -136,6 +136,13 @@ Zotero.Searches = function() {
 					conditionName = 'itemType';
 					condition.value = Zotero.ItemTypes.getName(condition.value);
 				}
+				// Parse old-style collection/savedSearch conditions ('0_ABCD2345' -> 'ABCD2345')
+				else if (conditionName == 'collection' || conditionName == 'savedSearch') {
+					if (condition.value.includes('_')) {
+						let [_, objKey] = condition.value.split('_');
+						condition.value = objKey;
+					}
+				}
 
 				search._conditions[i] = {
 					id: i,

diff --git a/test/tests/searchTest.js b/test/tests/searchTest.js
@@ -1,4 +1,40 @@
 describe("Zotero.Search", function() {
+	describe("#addCondition()", function () {
+		it("should convert old-style 'collection' condition value", function* () {
+			var col = yield createDataObject('collection');
+			var item = yield createDataObject('item', { collections: [col.id] });
+
+			var s = new Zotero.Search();
+			s.libraryID = item.libraryID;
+			s.name = "Test";
+			s.addCondition('collection', 'is', '0_' + col.key);
+			var matches = yield s.search();
+			assert.sameMembers(matches, [item.id]);
+		});
+	});
+
+	// This is for Zotero.Search._loadConditions()
+	describe("Loading", function () {
+		it("should convert old-style 'collection' condition value", function* () {
+			var col = yield createDataObject('collection');
+			var item = yield createDataObject('item', { collections: [col.id] });
+
+			var s = new Zotero.Search();
+			s.libraryID = item.libraryID;
+			s.name = "Test";
+			s.addCondition('collection', 'is', col.key);
+			yield s.saveTx();
+			yield Zotero.DB.queryAsync(
+				"UPDATE savedSearchConditions SET value=? WHERE savedSearchID=? AND condition=?",
+				["0_" + col.key, s.id, 'collection']
+			);
+			yield s.reload(['conditions'], true);
+			var matches = yield s.search();
+			assert.sameMembers(matches, [item.id]);
+		});
+	});
+
+
 	describe("#save()", function () {
 		it("should fail without a name", function* () {
 			var s = new Zotero.Search;
@@ -135,17 +171,6 @@ describe("Zotero.Search", function() {
 					assert.sameMembers(matches, [item.id]);
 				});
 
-				it("should find item in collection in old-style format", function* () {
-					var col = yield createDataObject('collection');
-					var item = yield createDataObject('item', { collections: [col.id] });
-
-					var s = new Zotero.Search();
-					s.libraryID = item.libraryID;
-					s.addCondition('collection', 'is', "0_" + col.key);
-					var matches = yield s.search();
-					assert.sameMembers(matches, [item.id]);
-				});
-
 				it("should find items not in collection", function* () {
 					var col = yield createDataObject('collection');
 					var item = yield createDataObject('item', { collections: [col.id] });
@@ -180,20 +205,6 @@ describe("Zotero.Search", function() {
 					var matches = yield s.search();
 					assert.sameMembers(matches, [item.id]);
 				});
-
-				it("should find item in subcollection in recursive mode with old-style key", function* () {
-					var col1 = yield createDataObject('collection');
-					var col2 = yield createDataObject('collection', { parentID: col1.id });
-					var item = yield createDataObject('item', { collections: [col2.id] });
-
-					var s = new Zotero.Search();
-					s.libraryID = item.libraryID;
-					s.addCondition('collection', 'is', "0_" + col1.key);
-					s.addCondition('recursive', 'true');
-					var matches = yield s.search();
-					assert.sameMembers(matches, [item.id]);
-				});
-
 			});
 
 			describe("fileTypeID", function () {