Remove .noteSchemaVersion

This leaves item.note in place, rather than reverting all the
`getNote()` → `.note` changes. We can consider which we want to keep.

chrome/content/zotero/xpcom/data/dataObject.js

@@ -763,7 +763,7 @@ Zotero.DataObject.prototype._getLatestField = function (field) {
  */
 Zotero.DataObject.prototype._markFieldChange = function (field, value) {
 	// New method (changedData)
-	if (['deleted', 'tags', 'note'].includes(field) || field.startsWith('annotation')) {
+	if (['deleted', 'tags'].includes(field) || field.startsWith('annotation')) {
 		if (Array.isArray(value)) {
 			this._changedData[field] = [...value];
 		}

chrome/content/zotero/xpcom/data/dataObjectUtilities.js

@@ -380,11 +380,6 @@ Zotero.DataObjectUtilities = {
 				continue;
 			}
 			
-			// Ignore noteSchemaVersion: 0 if no local note
-			if (field == 'noteSchemaVersion' && val === 0) {
-				continue;
-			}
-			
 			changeset.push({
 				field: field,
 				op: "add",

chrome/content/zotero/xpcom/data/item.js

@@ -56,18 +56,13 @@ Zotero.Item = function(itemTypeOrID) {
 	
 	// loadItemData
 	this._itemData = null;
+	this._noteTitle = null;
+	this._noteText = null;
 	this._displayTitle = null;
 	
-	this._note = {
-		data: null,
-		title: null,
-		schemaVersion: Zotero.Notes.schemaVersion
-	};
-	
 	// loadChildItems
 	this._attachments = null;
 	this._notes = null;
-	this._annotations = null;
 	
 	// loadAnnotation
 	this._annotationType = null;
@@ -760,7 +755,7 @@ Zotero.Item.prototype.setField = function(field, value, loadIn) {
 	}
 	
 	if (loadIn && this.isNote() && field == Zotero.ItemFields.getID('title')) {
-		this._note.title = value ? value : "";
+		this._noteTitle = value ? value : "";
 		return true;
 	}
 	
@@ -1678,44 +1673,34 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
 	}
 	
 	// Note
-	if ((isNew && this.isNote()) || this._hasFieldChanged('note')) {
+	if ((isNew && this.isNote()) || this._changed.note) {
-		let note = this._getLatestField('note');
+		if (!isNew) {
-		
+			if (this._noteText === null || this._noteTitle === null) {
-		// Since we override change detection for new notes and always save data, we have to mark
+				throw new Error("Cached note values not set with "
-		// the data type as loaded so it gets reloaded by Zotero.DataObject.reload()
+					+ "this._changed.note set to true");
-		if (isNew) {
-			this._loaded.note = true;
-		}
-		else {
-			if (note.data === null || note.title === null) {
-				throw new Error("Note marked as changed with cached values not set");
 			}
 		}
 		
 		let parent = this.isNote() ? this.parentID : null;
-		let noteData = note.data || '';
+		let noteText = this._noteText ? this._noteText : '';
 		// Add <div> wrapper if not present
-		if (!noteData.match(/^<div class="zotero-note znv[0-9]+">[\s\S]*<\/div>$/)) {
+		if (!noteText.match(/^<div class="zotero-note znv[0-9]+">[\s\S]*<\/div>$/)) {
-			noteData = Zotero.Notes.notePrefix + noteData + Zotero.Notes.noteSuffix;
+			noteText = Zotero.Notes.notePrefix + noteText + Zotero.Notes.noteSuffix;
 		}
 		
 		let params = [
 			parent ? parent : null,
-			noteData,
+			noteText,
-			note.title || '',
+			this._noteTitle ? this._noteTitle : ''
-			note.schemaVersion
 		];
-		this._clearChanged('note');
-		this._markForReload('note');
-		
 		let sql = "SELECT COUNT(*) FROM itemNotes WHERE itemID=?";
 		if (yield Zotero.DB.valueQueryAsync(sql, itemID)) {
-			sql = "UPDATE itemNotes SET parentItemID=?, note=?, title=?, schemaVersion=? WHERE itemID=?";
+			sql = "UPDATE itemNotes SET parentItemID=?, note=?, title=? WHERE itemID=?";
 			params.push(itemID);
 		}
 		else {
 			sql = "INSERT INTO itemNotes "
-					+ "(itemID, parentItemID, note, title, schemaVersion) VALUES (?,?,?,?,?)";
+					+ "(itemID, parentItemID, note, title) VALUES (?,?,?,?)";
 			params.unshift(itemID);
 		}
 		yield Zotero.DB.queryAsync(sql, params);
@@ -2028,16 +2013,10 @@ Zotero.Item.prototype.numNotes = function(includeTrashed, includeEmbedded) {
  */
 Zotero.Item.prototype.getNoteTitle = function() {
 	if (!this.isNote() && !this.isAttachment()) {
-		throw new Error("getNoteTitle() can only be called on notes and attachments");
+		throw ("getNoteTitle() can only be called on notes and attachments");
 	}
-	
+	if (this._noteTitle !== null) {
-	var note = this._getLatestField('note');
+		return this._noteTitle;
-	if (note.title !== null) {
-		return note.title;
-	}
-	if (note.data !== null) {
-		note.title = Zotero.Notes.noteToTitle(note.data);
-		return note.title;
 	}
 	this._requireData('itemData');
 	return "";
@@ -2068,89 +2047,66 @@ Zotero.Item.prototype.hasNote = Zotero.Promise.coroutine(function* () {
 
 Zotero.defineProperty(Zotero.Item.prototype, 'note', {
 	get: function () {
+		return this.getNote();
+	}
+});
+
+
+/**
+ * Get the text of an item note
+ **/
+Zotero.Item.prototype.getNote = function() {
 	if (!this.isNote() && !this.isAttachment()) {
-			throw new Error(".note can only be accessed on notes and attachments "
+		throw new Error("getNote() can only be called on notes and attachments "
 			+ `(${this.libraryID}/${this.key} is a ${Zotero.ItemTypes.getName(this.itemTypeID)})`);
 	}
 	
 	// Store access time for later garbage collection
 	this._noteAccessTime = new Date();
 	
-		var note = this._getLatestField('note');
+	if (this._noteText !== null) {
-		
+		return this._noteText;
-		if (note.data !== null) {
-			return note.data;
 	}
 	
 	this._requireData('note');
 	return "";
 }
-});
-
-
-/**
- * Get the contents of an item note
- **/
-Zotero.Item.prototype.getNote = function() {
-	Zotero.warn("Zotero.Item::getNote() is deprecated -- use .note");
-	return this.note;
-}
-
-
-Zotero.defineProperty(Zotero.Item.prototype, 'noteSchemaVersion', {
-	get: function() {
-		if (!this.isNote() && !this.isAttachment()) {
-			return undefined;
-		}
-		return this._getLatestField('note').schemaVersion;
-	}
-});
 
 
 /**
 * Set an item note
 *
 * Note: This can only be called on notes and attachments
- *
+**/
- * @param {String} data - Note contents
+Zotero.Item.prototype.setNote = function(text) {
- * @param {Number} [schemaVersion = Zotero.Notes.schemaVersion]
- */
-Zotero.Item.prototype.setNote = function (data, schemaVersion) {
 	if (!this.isNote() && !this.isAttachment()) {
 		throw ("updateNote() can only be called on notes and attachments");
 	}
 	
-	if (typeof data != 'string') {
+	if (typeof text != 'string') {
-		throw new Error("data must be a string (was " + typeof data + ")");
+		throw ("text must be a string in Zotero.Item.setNote() (was " + typeof text + ")");
 	}
 	
-	if (schemaVersion === undefined) {
+	text = text
-		schemaVersion = Zotero.Notes.schemaVersion;
-	}
-	if (typeof schemaVersion != 'number' || schemaVersion != parseInt(schemaVersion)) {
-		throw new Error(`schemaVersion must be an integer (was ${JSON.stringify(schemaVersion)})`);
-	}
-	
-	data = data
 		// Strip control characters
 		.replace(/[\u0000-\u0008\u000B\u000C\u000E-\u001F\u007F]/g, "")
 		.trim();
 	
-	var { data: oldData, schemaVersion: oldSchemaVersion } = this._getLatestField('note');
+	var oldText = this.getNote();
-	if (data === oldData && schemaVersion === oldSchemaVersion) {
+	if (text === oldText) {
 		Zotero.debug("Note hasn't changed", 4);
 		return false;
 	}
 	
-	this._hasNote = data !== '';
+	this._hasNote = text !== '';
-	var title = Zotero.Notes.noteToTitle(data);
+	this._noteText = text;
-	
+	this._noteTitle = Zotero.Notes.noteToTitle(text);
-	// This isn't quite correct because the save could fail
 	if (this.isNote()) {
-		this._displayTitle = title;
+		this._displayTitle = this._noteTitle;
 	}
 	
-	this._markFieldChange('note', { data, title, schemaVersion });
+	this._markFieldChange('note', oldText);
+	this._changed.note = true;
 	
 	return true;
 }
@@ -4316,7 +4272,7 @@ Zotero.Item.prototype.clone = function (libraryID, options = {}) {
 		newItem.setCreators(this.getCreators());
 	}
 	else {
-		newItem.setNote(this.note, this.noteSchemaVersion);
+		newItem.setNote(this.getNote());
 		if (sameLibrary) {
 			var parent = this.parentKey;
 			if (parent) {
@@ -4604,6 +4560,7 @@ Zotero.Item.prototype.fromJSON = function (json, options = {}) {
 		case 'key':
 		case 'version':
 		case 'itemType':
+		case 'note':
 		// Use?
 		case 'md5':
 		case 'mtime':
@@ -4612,7 +4569,6 @@ Zotero.Item.prototype.fromJSON = function (json, options = {}) {
 		// Handled below
 		//
 		case 'note':
-		case 'noteSchemaVersion':
 		case 'collections':
 		case 'parentItem':
 		case 'deleted':
@@ -4878,7 +4834,7 @@ Zotero.Item.prototype.fromJSON = function (json, options = {}) {
 		
 		if (!this.isAnnotation()) {
 			let note = json.note;
-			this.setNote(note !== undefined ? note : "", json.noteSchemaVersion);
+			this.setNote(note !== undefined ? note : "");
 		}
 	}
 	
@@ -4965,10 +4921,9 @@ Zotero.Item.prototype.toJSON = function (options = {}) {
 		
 		// Notes and embedded attachment notes
 		if (this.isAttachment() || this.isNote()) {
-			let note = this.note;
+			let note = this.getNote();
 			if (note !== "" || mode == 'full' || (mode == 'new' && this.isNote())) {
 				obj.note = note;
-				obj.noteSchemaVersion = this.noteSchemaVersion || 0;
 			}
 		}
 		

chrome/content/zotero/xpcom/data/items.js

@@ -404,7 +404,7 @@ Zotero.Items = function() {
 	this._loadNotes = Zotero.Promise.coroutine(function* (libraryID, ids, idSQL) {
 		var notesToUpdate = [];
 		
-		var sql = "SELECT itemID, note, schemaVersion FROM items "
+		var sql = "SELECT itemID, note FROM items "
 			+ "JOIN itemNotes USING (itemID) "
 			+ "WHERE libraryID=?" + idSQL;
 		var params = [libraryID];
@@ -420,7 +420,6 @@ Zotero.Items = function() {
 						throw new Error("Item " + itemID + " not found");
 					}
 					let note = row.getResultByIndex(1);
-					let schemaVersion = row.getResultByIndex(2);
 					
 					// Convert non-HTML notes on-the-fly
 					if (note !== "") {
@@ -451,10 +450,7 @@ Zotero.Items = function() {
 						}
 					}
 					
-					item._note.data = note || '';
+					item._noteText = note ? note : '';
-					item._note.schemaVersion = schemaVersion;
-					// Lazily loaded
-					item._note.title = null;
 					item._loaded.note = true;
 					item._clearChanged('note');
 				}.bind(this)
@@ -487,8 +483,7 @@ Zotero.Items = function() {
 						throw new Error("Item " + itemID + " not loaded");
 					}
 					
-					item._note.data = '';
+					item._noteText = '';
-					item._note.schemaVersion = 0;
 					item._loaded.note = true;
 					item._clearChanged('note');
 				}.bind(this)
@@ -747,9 +742,7 @@ Zotero.Items = function() {
 		
 		// Mark all top-level items as having child items loaded
 		sql = "SELECT itemID FROM items I WHERE libraryID=?" + idSQL + " AND itemID NOT IN "
-			+ "(SELECT itemID FROM itemAttachments "
+			+ "(SELECT itemID FROM itemAttachments UNION SELECT itemID FROM itemNotes)";
-			+ "UNION SELECT itemID FROM itemNotes "
-			+ "UNION SELECT itemID FROM itemAnnotations)";
 		yield Zotero.DB.queryAsync(
 			sql,
 			params,

chrome/content/zotero/xpcom/data/notes.js

@@ -32,11 +32,6 @@ Zotero.Notes = new function() {
 	this.__defineGetter__("notePrefix", function () { return '<div class="zotero-note znv1">'; });
 	this.__defineGetter__("noteSuffix", function () { return '</div>'; });
 	
-	Zotero.defineProperty(this, 'schemaVersion', {
-		value: 1,
-		writable: false
-	});
-	
 	/**
 	* Return first line (or first MAX_LENGTH characters) of note content
 	**/

chrome/content/zotero/xpcom/schema.js

@@ -3238,8 +3238,6 @@ Zotero.Schema = new function(){
 				
 				yield Zotero.DB.queryAsync("CREATE TABLE itemAnnotations (\n    itemID INTEGER PRIMARY KEY,\n    parentItemID INT NOT NULL,\n    type INTEGER NOT NULL,\n    text TEXT,\n    comment TEXT,\n    color TEXT,\n    pageLabel TEXT,\n    sortIndex TEXT NOT NULL,\n    position TEXT NOT NULL,\n    FOREIGN KEY (itemID) REFERENCES items(itemID) ON DELETE CASCADE,\n    FOREIGN KEY (parentItemID) REFERENCES itemAttachments(itemID) ON DELETE CASCADE\n)");
 				yield Zotero.DB.queryAsync("CREATE INDEX itemAnnotations_parentItemID ON itemAnnotations(parentItemID)");
-				
-				yield Zotero.DB.queryAsync("ALTER TABLE itemNotes ADD COLUMN schemaVersion INT NOT NULL DEFAULT 0");
 			}
 			
 			// If breaking compatibility or doing anything dangerous, clear minorUpdateFrom

resource/schema/userdata.sql

@@ -88,7 +88,6 @@ CREATE TABLE itemNotes (
     parentItemID INT,
     note TEXT,
     title TEXT,
-    schemaVersion INT NOT NULL DEFAULT 0,
     FOREIGN KEY (itemID) REFERENCES items(itemID) ON DELETE CASCADE,
     FOREIGN KEY (parentItemID) REFERENCES items(itemID) ON DELETE CASCADE
 );

test/content/support.js

@@ -468,10 +468,7 @@ function createUnsavedDataObject(objectType, params = {}) {
 			obj.setTags(params.tags);
 		}
 		if (params.note !== undefined) {
-			obj.setNote(params.note, params.noteSchemaVersion);
+			obj.setNote(params.note);
-		}
-		else if (itemType == 'note') {
-			obj.setNote(`<p>${Zotero.Utilities.randomString()}</p>`, params.noteSchemaVersion);
 		}
 		break;
 	

test/tests/itemTest.js

@@ -854,30 +854,6 @@ describe("Zotero.Item", function () {
 		});
 	})
 	
-	describe("#noteSchemaVersion", function () {
-		it("should be set to current schema version", async function () {
-			var note = await createDataObject('item', { itemType: 'note' });
-			assert.equal(note.noteSchemaVersion, Zotero.Notes.schemaVersion);
-		});
-		
-		it("should be set to an explicit value with setNote()", async function () {
-			var note = createUnsavedDataObject('item', { itemType: 'note' });
-			note.setNote('<div>Foo</div>', 2);
-			await note.saveTx();
-			assert.equal(note.noteSchemaVersion, 2);
-		});
-		
-		it("shouldn't be settable to null", async function () {
-			var note = createUnsavedDataObject('item', { itemType: 'note' });
-			assert.throws(() => note.setNote('<div>Foo</div>', null));
-		});
-		
-		it("shouldn't be settable to a numeric string", async function () {
-			var note = createUnsavedDataObject('item', { itemType: 'note' });
-			assert.throws(() => note.setNote('<div>Foo</div>', "2"));
-		});
-	});
-	
 	describe("#attachmentCharset", function () {
 		it("should get and set a value", function* () {
 			var charset = 'utf-8';
@@ -1583,13 +1559,6 @@ describe("Zotero.Item", function () {
 			var newItem = item.clone();
 			assert.isEmpty(Object.keys(newItem.toJSON().relations));
 		});
-		
-		it("should preserve noteSchemaVersion when set to a different version from the current version", async function () {
-			var oldVersion = Zotero.Notes.schemaVersion - 1;
-			var note = await createDataObject('item', { itemType: 'note', noteSchemaVersion: oldVersion });
-			var newNote = note.clone();
-			assert.equal(newNote.noteSchemaVersion, oldVersion);
-		});
 	})
 	
 	describe("#moveToLibrary()", function () {
@@ -1829,12 +1798,6 @@ describe("Zotero.Item", function () {
 				var json = item.toJSON({ mode: 'full' });
 				assert.notProperty(json, "inPublications");
 			});
-			
-			it("should include noteSchemaVersion", function () {
-				var note = createUnsavedDataObject('item', { itemType: 'note', noteSchemaVersion: 3 });
-				var json = note.toJSON();
-				assert.propertyVal(json, 'noteSchemaVersion', 3);
-			});
 		})
 		
 		describe("'full' mode", function () {
@@ -2380,17 +2343,6 @@ describe("Zotero.Item", function () {
 			assert.equal(item.getField("bookTitle"), "Publication Title");
 		});
 		
-		it("should set note and noteSchemaField", function () {
-			var item = new Zotero.Item;
-			item.fromJSON({
-				itemType: "note",
-				note: "<p>Foo</p>",
-				noteSchemaVersion: 3
-			});
-			assert.equal(item.note, "<p>Foo</p>");
-			assert.equal(item.noteSchemaVersion, 3);
-		});
-		
 		it("should import annotation fields", async function () {
 			var attachment = await importPDFAttachment();
 			

test/tests/syncLocalTest.js

@@ -2560,29 +2560,6 @@ describe("Zotero.Sync.Data.Local", function() {
 			assert.lengthOf(result.conflicts, 1);
 		});
 		
-		it("should ignore noteSchemaVersion=0 if no note", function () {
-			var json1 = {
-				key: "AAAAAAAA",
-				version: 1234,
-				title: "Link",
-				dateModified: "2017-04-02 12:34:56"
-			};
-			var json2 = {
-				key: "AAAAAAAA",
-				version: 1235,
-				title: "Link",
-				note: "",
-				noteSchemaVersion: 0,
-				dateModified: "2017-04-02 12:34:56"
-			};
-			var ignoreFields = ['dateAdded', 'dateModified'];
-			var result = Zotero.Sync.Data.Local._reconcileChangesWithoutCache(
-				'item', json1, json2, ignoreFields
-			);
-			assert.lengthOf(result.changes, 0);
-			assert.lengthOf(result.conflicts, 0);
-		});
-		
 		it("should automatically use remote version for conflicting fields when both sides are in trash", function () {
 			var json1 = {
 				key: "AAAAAAAA",