Add noteSchemaVersion, and replace item.getNote() with .note

```
var noteContents = item.note; // was item.getNote()
var schemaVersion = item.noteSchemaVersion;

item.setNote(contents) // default to Zotero.Notes.schemaVersion
item.setNote(contents, schemaVersion) - explicit version
```
This commit is contained in:
Dan Stillman 2020-08-25 13:07:14 -04:00
parent 2543a695e8
commit ebc53a2bbc
8 changed files with 175 additions and 57 deletions

View file

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

View file

@ -56,13 +56,18 @@ Zotero.Item = function(itemTypeOrID) {
// loadItemData // loadItemData
this._itemData = null; this._itemData = null;
this._noteTitle = null;
this._noteText = null;
this._displayTitle = null; this._displayTitle = null;
this._note = {
data: null,
title: null,
schemaVersion: Zotero.Notes.schemaVersion
};
// loadChildItems // loadChildItems
this._attachments = null; this._attachments = null;
this._notes = null; this._notes = null;
this._annotations = null;
// loadAnnotation // loadAnnotation
this._annotationType = null; this._annotationType = null;
@ -1673,34 +1678,44 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
} }
// Note // Note
if ((isNew && this.isNote()) || this._changed.note) { if ((isNew && this.isNote()) || this._hasFieldChanged('note')) {
if (!isNew) { let note = this._getLatestField('note');
if (this._noteText === null || this._noteTitle === null) {
throw new Error("Cached note values not set with " // Since we override change detection for new notes and always save data, we have to mark
+ "this._changed.note set to true"); // the data type as loaded so it gets reloaded by Zotero.DataObject.reload()
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 parent = this.isNote() ? this.parentID : null;
let noteText = this._noteText ? this._noteText : ''; let noteData = note.data || '';
// Add <div> wrapper if not present // Add <div> wrapper if not present
if (!noteText.match(/^<div class="zotero-note znv[0-9]+">[\s\S]*<\/div>$/)) { if (!noteData.match(/^<div class="zotero-note znv[0-9]+">[\s\S]*<\/div>$/)) {
noteText = Zotero.Notes.notePrefix + noteText + Zotero.Notes.noteSuffix; noteData = Zotero.Notes.notePrefix + noteData + Zotero.Notes.noteSuffix;
} }
let params = [ let params = [
parent ? parent : null, parent ? parent : null,
noteText, noteData,
this._noteTitle ? this._noteTitle : '' note.title || '',
note.schemaVersion
]; ];
this._clearChanged('note');
this._markForReload('note');
let sql = "SELECT COUNT(*) FROM itemNotes WHERE itemID=?"; let sql = "SELECT COUNT(*) FROM itemNotes WHERE itemID=?";
if (yield Zotero.DB.valueQueryAsync(sql, itemID)) { if (yield Zotero.DB.valueQueryAsync(sql, itemID)) {
sql = "UPDATE itemNotes SET parentItemID=?, note=?, title=? WHERE itemID=?"; sql = "UPDATE itemNotes SET parentItemID=?, note=?, title=?, schemaVersion=? WHERE itemID=?";
params.push(itemID); params.push(itemID);
} }
else { else {
sql = "INSERT INTO itemNotes " sql = "INSERT INTO itemNotes "
+ "(itemID, parentItemID, note, title) VALUES (?,?,?,?)"; + "(itemID, parentItemID, note, title, schemaVersion) VALUES (?,?,?,?,?)";
params.unshift(itemID); params.unshift(itemID);
} }
yield Zotero.DB.queryAsync(sql, params); yield Zotero.DB.queryAsync(sql, params);
@ -2017,10 +2032,16 @@ Zotero.Item.prototype.numNotes = function(includeTrashed, includeEmbedded) {
*/ */
Zotero.Item.prototype.getNoteTitle = function() { Zotero.Item.prototype.getNoteTitle = function() {
if (!this.isNote() && !this.isAttachment()) { if (!this.isNote() && !this.isAttachment()) {
throw ("getNoteTitle() can only be called on notes and attachments"); throw new Error("getNoteTitle() can only be called on notes and attachments");
} }
if (this._noteTitle !== null) {
return this._noteTitle; var note = this._getLatestField('note');
if (note.title !== null) {
return note.title;
}
if (note.data !== null) {
note.title = Zotero.Notes.noteToTitle(note.data);
return note.title;
} }
this._requireData('itemData'); this._requireData('itemData');
return ""; return "";
@ -2049,10 +2070,8 @@ Zotero.Item.prototype.hasNote = Zotero.Promise.coroutine(function* () {
}); });
/** Zotero.defineProperty(Zotero.Item.prototype, 'note', {
* Get the text of an item note get: function () {
**/
Zotero.Item.prototype.getNote = function() {
if (!this.isNote() && !this.isAttachment()) { if (!this.isNote() && !this.isAttachment()) {
throw new Error("getNote() can only be called 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)})`); + `(${this.libraryID}/${this.key} is a ${Zotero.ItemTypes.getName(this.itemTypeID)})`);
@ -2061,49 +2080,81 @@ Zotero.Item.prototype.getNote = function() {
// Store access time for later garbage collection // Store access time for later garbage collection
this._noteAccessTime = new Date(); this._noteAccessTime = new Date();
if (this._noteText !== null) { var note = this._getLatestField('note');
return this._noteText;
if (note.data !== null) {
return note.data;
} }
this._requireData('note'); this._requireData('note');
return ""; 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 * Set an item note
* *
* Note: This can only be called on notes and attachments * Note: This can only be called on notes and attachments
**/ *
Zotero.Item.prototype.setNote = function(text) { * @param {String} data - Note contents
* @param {Number} [schemaVersion = Zotero.Notes.schemaVersion]
*/
Zotero.Item.prototype.setNote = function (data, schemaVersion) {
if (!this.isNote() && !this.isAttachment()) { if (!this.isNote() && !this.isAttachment()) {
throw ("updateNote() can only be called on notes and attachments"); throw ("updateNote() can only be called on notes and attachments");
} }
if (typeof text != 'string') { if (typeof data != 'string') {
throw ("text must be a string in Zotero.Item.setNote() (was " + typeof text + ")"); throw new Error("data must be a string (was " + typeof data + ")");
} }
text = text if (schemaVersion === undefined) {
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 // Strip control characters
.replace(/[\u0000-\u0008\u000B\u000C\u000E-\u001F\u007F]/g, "") .replace(/[\u0000-\u0008\u000B\u000C\u000E-\u001F\u007F]/g, "")
.trim(); .trim();
var oldText = this.getNote(); var { data: oldData, schemaVersion: oldSchemaVersion } = this._getLatestField('note');
if (text === oldText) { if (data === oldData && schemaVersion === oldSchemaVersion) {
Zotero.debug("Note hasn't changed", 4); Zotero.debug("Note hasn't changed", 4);
return false; return false;
} }
this._hasNote = text !== ''; this._hasNote = data !== '';
this._noteText = text; var title = Zotero.Notes.noteToTitle(data);
this._noteTitle = Zotero.Notes.noteToTitle(text);
// This isn't quite correct because the save could fail
if (this.isNote()) { if (this.isNote()) {
this._displayTitle = this._noteTitle; this._displayTitle = title;
} }
this._markFieldChange('note', oldText); this._markFieldChange('note', { data, title, schemaVersion });
this._changed.note = true;
return true; return true;
} }
@ -4253,7 +4304,7 @@ Zotero.Item.prototype.clone = function (libraryID, options = {}) {
newItem.setCreators(this.getCreators()); newItem.setCreators(this.getCreators());
} }
else { else {
newItem.setNote(this.getNote()); newItem.setNote(this.note, this.noteSchemaVersion);
if (sameLibrary) { if (sameLibrary) {
var parent = this.parentKey; var parent = this.parentKey;
if (parent) { if (parent) {
@ -4542,6 +4593,7 @@ Zotero.Item.prototype.fromJSON = function (json, options = {}) {
case 'version': case 'version':
case 'itemType': case 'itemType':
case 'note': case 'note':
case 'noteSchemaVersion':
// Use? // Use?
case 'md5': case 'md5':
case 'mtime': case 'mtime':
@ -4796,7 +4848,7 @@ Zotero.Item.prototype.fromJSON = function (json, options = {}) {
this.parentKey = parentKey ? parentKey : false; this.parentKey = parentKey ? parentKey : false;
let note = json.note; let note = json.note;
this.setNote(note !== undefined ? note : ""); this.setNote(note !== undefined ? note : "", json.noteSchemaVersion);
} }
// Update boolean fields that might not be present in JSON // Update boolean fields that might not be present in JSON
@ -4877,9 +4929,10 @@ Zotero.Item.prototype.toJSON = function (options = {}) {
} }
// Notes and embedded attachment notes // Notes and embedded attachment notes
let note = this.getNote(); let note = this.note;
if (note !== "" || mode == 'full' || (mode == 'new' && this.isNote())) { if (note !== "" || mode == 'full' || (mode == 'new' && this.isNote())) {
obj.note = note; obj.note = note;
obj.noteSchemaVersion = this.noteSchemaVersion || 0;
} }
} }

View file

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

View file

@ -35,6 +35,11 @@ Zotero.Notes = new function() {
this.__defineGetter__("notePrefix", function () { return '<div class="zotero-note znv1">'; }); this.__defineGetter__("notePrefix", function () { return '<div class="zotero-note znv1">'; });
this.__defineGetter__("noteSuffix", function () { return '</div>'; }); 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 * Return first line (or first MAX_LENGTH characters) of note content
**/ **/

View file

@ -3236,6 +3236,8 @@ 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 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("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 // If breaking compatibility or doing anything dangerous, clear minorUpdateFrom

View file

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

View file

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

View file

@ -854,6 +854,30 @@ 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 () { describe("#attachmentCharset", function () {
it("should get and set a value", function* () { it("should get and set a value", function* () {
var charset = 'utf-8'; var charset = 'utf-8';
@ -1559,6 +1583,13 @@ describe("Zotero.Item", function () {
var newItem = item.clone(); var newItem = item.clone();
assert.isEmpty(Object.keys(newItem.toJSON().relations)); 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 () { describe("#moveToLibrary()", function () {
@ -1742,6 +1773,12 @@ describe("Zotero.Item", function () {
var json = item.toJSON({ mode: 'full' }); var json = item.toJSON({ mode: 'full' });
assert.notProperty(json, "inPublications"); 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 () { describe("'full' mode", function () {
@ -2286,5 +2323,15 @@ describe("Zotero.Item", function () {
}); });
assert.equal(item.getField("bookTitle"), "Publication Title"); assert.equal(item.getField("bookTitle"), "Publication Title");
}); });
it("should set noteSchemaField", function () {
var item = new Zotero.Item;
item.fromJSON({
itemType: "note",
note: "<p>Foo</p>",
noteSchemaVersion: 3
});
assert.equal(item.noteSchemaVersion, 3);
});
}); });
}); });