From 33eaaffd832ad84e4305b8e25a5ff20232cff890 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Wed, 6 May 2015 01:07:40 -0400 Subject: [PATCH] Fix Date Modified handling when saving items --- chrome/content/zotero/xpcom/data/item.js | 41 ++++++------- test/tests/itemTest.js | 77 ++++++++++++++++++++++++ 2 files changed, 97 insertions(+), 21 deletions(-) diff --git a/chrome/content/zotero/xpcom/data/item.js b/chrome/content/zotero/xpcom/data/item.js index 0042b84f8..136c85631 100644 --- a/chrome/content/zotero/xpcom/data/item.js +++ b/chrome/content/zotero/xpcom/data/item.js @@ -727,7 +727,7 @@ Zotero.Item.prototype.setField = function(field, value, loadIn) { let date = Zotero.Date.sqlToDate(value, true); if (!date) throw new Error("Invalid SQL date: " + value); - value = Zotero.Date.dateToSQL(date); + value = Zotero.Date.dateToSQL(date, true); break; case 'version': @@ -1205,27 +1205,26 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) { this.synced ? 1 : 0 ); - if (this._changed.primaryData && this._changed.primaryData._dateModified) { - sqlColumns.push('dateModified', 'clientDateModified'); - sqlValues.push(this.dateModified, Zotero.DB.transactionDateTime); + // If a new item and Date Modified hasn't been provided, or an existing item and + // Date Modified hasn't changed from its previous value and skipDateModifiedUpdate wasn't + // passed, use the current timestamp + if (!this.dateModified + || ((!this._changed.primaryData || !this._changed.primaryData.dateModified) + && !options.skipDateModifiedUpdate)) { + sqlColumns.push('dateModified'); + sqlValues.push(Zotero.DB.transactionDateTime); } - else if (isNew) { - sqlColumns.push('dateModified', 'clientDateModified'); - sqlValues.push(Zotero.DB.transactionDateTime, Zotero.DB.transactionDateTime); + // Otherwise, if a new Date Modified was provided, use that. (This would also work when + // skipDateModifiedUpdate was passed and there's an existing value, but in that case we + // can just not change the field at all.) + else if (this._changed.primaryData && this._changed.primaryData.dateModified) { + sqlColumns.push('dateModified'); + sqlValues.push(this.dateModified); } - else { - for each (let field in ['dateModified', 'clientDateModified']) { - switch (field) { - case 'dateModified': - case 'clientDateModified': - let skipFlag = "skip" + field[0].toUpperCase() + field.substr(1) + "Update"; - if (!options[skipFlag]) { - sqlColumns.push(field); - sqlValues.push(Zotero.DB.transactionDateTime); - } - break; - } - } + + if (isNew || !options.skipClientDateModifiedUpdate) { + sqlColumns.push('clientDateModified'); + sqlValues.push(Zotero.DB.transactionDateTime); } if (isNew) { @@ -4029,7 +4028,7 @@ Zotero.Item.prototype.fromJSON = Zotero.Promise.coroutine(function* (json) { case 'dateAdded': case 'dateModified': - this['_' + field] = Zotero.Date.dateToSQL(Zotero.Date.isoToDate(val), true); + this[field] = Zotero.Date.dateToSQL(Zotero.Date.isoToDate(val), true); break; case 'parentItem': diff --git a/test/tests/itemTest.js b/test/tests/itemTest.js index 2919d1e92..d02d50aae 100644 --- a/test/tests/itemTest.js +++ b/test/tests/itemTest.js @@ -79,6 +79,83 @@ describe("Zotero.Item", function () { }); }) + describe("#dateModified", function () { + it("should use given Date Modified for a new item", function* () { + var dateModified = "2015-05-05 17:18:12"; + var item = new Zotero.Item('book'); + item.dateModified = dateModified; + var id = yield item.save(); + item = yield Zotero.Items.getAsync(id); + assert.equal(item.dateModified, dateModified); + }) + + it("should use given Date Modified when skipDateModifiedUpdate is set for a new item", function* () { + var dateModified = "2015-05-05 17:18:12"; + var item = new Zotero.Item('book'); + item.dateModified = dateModified; + var id = yield item.save({ + skipDateModifiedUpdate: true + }); + item = yield Zotero.Items.getAsync(id); + assert.equal(item.dateModified, dateModified); + }) + + it("should use current time if Date Modified was not given for an existing item", function* () { + var dateModified = "2015-05-05 17:18:12"; + var item = new Zotero.Item('book'); + item.dateModified = dateModified; + var id = yield item.save(); + item = yield Zotero.Items.getAsync(id); + + // Save again without changing Date Modified + yield item.loadItemData(); + item.setField('title', 'Test'); + yield item.save() + + assert.closeTo(Zotero.Date.sqlToDate(item.dateModified, true).getTime(), Date.now(), 1000); + }) + + it("should use current time if the existing Date Modified value was given for an existing item", function* () { + var dateModified = "2015-05-05 17:18:12"; + var item = new Zotero.Item('book'); + item.dateModified = dateModified; + var id = yield item.save(); + item = yield Zotero.Items.getAsync(id); + + // Set Date Modified to existing value + yield item.loadItemData(); + item.setField('title', 'Test'); + item.dateModified = dateModified; + yield item.save() + assert.closeTo(Zotero.Date.sqlToDate(item.dateModified, true).getTime(), Date.now(), 1000); + }) + + it("should use current time if Date Modified is not given when skipDateModifiedUpdate is set for a new item", function* () { + var item = new Zotero.Item('book'); + var id = yield item.save({ + skipDateModifiedUpdate: true + }); + item = yield Zotero.Items.getAsync(id); + assert.closeTo(Zotero.Date.sqlToDate(item.dateModified, true).getTime(), Date.now(), 1000); + }) + + it("should keep original Date Modified when skipDateModifiedUpdate is set for an existing item", function* () { + var dateModified = "2015-05-05 17:18:12"; + var item = new Zotero.Item('book'); + item.dateModified = dateModified; + var id = yield item.save(); + item = yield Zotero.Items.getAsync(id); + + // Resave with skipDateModifiedUpdate + yield item.loadItemData(); + item.setField('title', 'Test'); + yield item.save({ + skipDateModifiedUpdate: true + }) + assert.equal(item.dateModified, dateModified); + }) + }) + describe("#parentID", function () { it("should create a child note", function* () { var item = new Zotero.Item('book');