www

commit d9c32a8e90a1ae616f9034badd4efee3767460ed
parent 59773f3f6d629bd4d37a5254892d29d8f3dfd023
Author: Dan Stillman <dstillman@zotero.org>
Date:   Fri, 17 Apr 2015 00:20:16 -0400

Fix search saving, and add some unit tests

Also:

- Return an object from `Zotero.Search.prototype.getConditions()`
  instead of an array.
- Add support function `getPromiseError(promise)` to return the error
  thrown from a chain of promises, or false if none. (We could make an
  `assert.throwsAsync()`, but this allows testing of various properties
  such as `.name`, which even the built-in `assert.throws()` can't
  test.)
- Clarify some search save errors

Diffstat:
M
chrome/content/zotero/xpcom/data/dataObject.js
 | 
8
+++++++-
M
chrome/content/zotero/xpcom/search.js
 | 
15
+++++++--------
M
test/content/support.js
 | 
7
+++++++
A
test/tests/searchTests.js
 | 
47
+++++++++++++++++++++++++++++++++++++++++++++++

4 files changed, 68 insertions(+), 9 deletions(-)
diff --git a/chrome/content/zotero/xpcom/data/dataObject.js b/chrome/content/zotero/xpcom/data/dataObject.js
@@ -569,9 +569,15 @@ Zotero.DataObject.prototype._recoverFromSaveError = Zotero.Promise.coroutine(fun
 });
 
 Zotero.DataObject.prototype._initSave = Zotero.Promise.coroutine(function* (env) {
+	if (!this.libraryID) {
+		throw new Error("libraryID must be set before saving " + this._objectType);
+	}
+	
 	env.isNew = !this.id;
 	
-	if (!env.options.skipEditCheck) this.editCheck();
+	if (!env.options.skipEditCheck) {
+		this.editCheck();
+	}
 	
 	if (!this.hasChanged()) {
 		Zotero.debug(this._ObjectType + ' ' + this.id + ' has not changed', 4);
diff --git a/chrome/content/zotero/xpcom/search.js b/chrome/content/zotero/xpcom/search.js
@@ -172,7 +172,7 @@ Zotero.Search.prototype.loadFromRow = function (row) {
 
 Zotero.Search.prototype._initSave = Zotero.Promise.coroutine(function* (env) {
 	if (!this.name) {
-		throw('Name not provided for saved search');
+		throw new Error('Name not provided for saved search');
 	}
 	
 	return Zotero.Search._super.prototype._initSave.apply(this, arguments);
@@ -199,7 +199,6 @@ Zotero.Search.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
 	var sqlValues = [
 		searchID ? { int: searchID } : null,
 		{ string: this.name },
-		Zotero.DB.transactionDateTime,
 		this.libraryID,
 		key,
 		this.version ? this.version : 0,
@@ -478,14 +477,14 @@ Zotero.Search.prototype.getCondition = function(searchConditionID){
 
 
 /*
- * Returns a multidimensional array of conditions/operator/value sets
- * used in the search, indexed by searchConditionID
+ * Returns an object of conditions/operator/value sets used in the search,
+ * indexed by searchConditionID
  */
 Zotero.Search.prototype.getConditions = function(){
 	this._requireData('conditions');
-	var conditions = [];
-	for (var id in this._conditions) {
-		var condition = this._conditions[id];
+	var conditions = {};
+	for (let id in this._conditions) {
+		let condition = this._conditions[id];
 		conditions[id] = {
 			id: id,
 			condition: condition.condition,
@@ -2340,7 +2339,7 @@ Zotero.SearchConditions = new function(){
 		}
 		
 		if (!_conditions[condition]){
-			throw ("Invalid condition '" + condition + "' in hasOperator()");
+			throw new Error("Invalid condition '" + condition + "' in hasOperator()");
 		}
 		
 		if (!operator && typeof _conditions[condition]['operators'] == 'undefined'){
diff --git a/test/content/support.js b/test/content/support.js
@@ -115,6 +115,13 @@ function waitForCallback(cb, interval, timeout) {
 }
 
 /**
+ * Return a promise for the error thrown by a promise, or false if none
+ */
+function getPromiseError(promise) {
+	return promise.thenReturn(false).catch(e => e);
+}
+
+/**
  * Ensures that the PDF tools are installed, or installs them if not.
  * Returns a promise.
  */
diff --git a/test/tests/searchTests.js b/test/tests/searchTests.js
@@ -0,0 +1,47 @@
+describe("Zotero.Search", function() {
+	describe("#save()", function () {
+		it("should fail without a libraryID", function* () {
+			var s = new Zotero.Search;
+			s.name = "Test";
+			s.addCondition('title', 'is', 'test');
+			var e = yield getPromiseError(s.save());
+			assert.ok(e);
+			assert.equal(e.constructor.name, Error.prototype.constructor.name); // TEMP: Error mismatch
+			assert.equal(e.message, "libraryID must be set before saving search");
+		});
+		
+		it("should fail without a name", function* () {
+			var s = new Zotero.Search;
+			s.libraryID = Zotero.Libraries.userLibraryID;
+			s.addCondition('title', 'is', 'test');
+			var e = yield getPromiseError(s.save());
+			assert.ok(e);
+			assert.equal(e.constructor.name, Error.prototype.constructor.name); // TEMP: Error mismatch
+			assert.equal(e.message, "Name not provided for saved search");
+		});
+		
+		it("should save a new search", function* () {
+			// Save search
+			var s = new Zotero.Search;
+			s.libraryID = Zotero.Libraries.userLibraryID;
+			s.name = "Test";
+			s.addCondition('title', 'is', 'test');
+			var id = yield s.save();
+			assert.typeOf(id, 'number');
+			
+			// Check saved search
+			s = yield Zotero.Searches.getAsync(id);
+			assert.ok(s);
+			assert.instanceOf(s, Zotero.Search);
+			assert.equal(s.name, "Test");
+			yield s.loadConditions();
+			var conditions = s.getConditions();
+			assert.lengthOf(Object.keys(conditions), 1);
+			assert.property(conditions, "1"); // searchConditionIDs start at 1
+			var condition = conditions[1];
+			assert.propertyVal(condition, 'condition', 'title')
+			assert.propertyVal(condition, 'operator', 'is')
+			assert.propertyVal(condition, 'value', 'test')
+		});
+	});
+});