From b54d4e78b718aa28ee8d3c6e2e9c7bb49fb497a0 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Sat, 20 Jun 2020 05:43:24 -0400 Subject: [PATCH] Save createdByUserID and lastModifiedByUserID for group items --- chrome/content/zotero/xpcom/data/item.js | 92 ++++++++++++------- chrome/content/zotero/xpcom/data/items.js | 6 +- chrome/content/zotero/xpcom/schema.js | 5 + .../content/zotero/xpcom/sync/syncEngine.js | 61 ++++++++++++ chrome/content/zotero/xpcom/sync/syncLocal.js | 19 +++- chrome/content/zotero/xpcom/users.js | 28 +++++- resource/schema/userdata.sql | 4 +- test/tests/syncEngineTest.js | 79 ++++++++++++++++ test/tests/syncLocalTest.js | 63 +++++++++++++ 9 files changed, 317 insertions(+), 40 deletions(-) diff --git a/chrome/content/zotero/xpcom/data/item.js b/chrome/content/zotero/xpcom/data/item.js index 601fe8b971..a7ff2ccc1b 100644 --- a/chrome/content/zotero/xpcom/data/item.js +++ b/chrome/content/zotero/xpcom/data/item.js @@ -38,6 +38,8 @@ Zotero.Item = function(itemTypeOrID) { // loadPrimaryData (additional properties in dataObject.js) this._itemTypeID = null; + this._createdByUserID = null; + this._lastModifiedByUserID = null; this._firstCreator = null; this._sortCreator = null; this._attachmentCharset = null; @@ -107,33 +109,19 @@ Zotero.defineProperty(Zotero.Item.prototype, 'itemID', { }, enumerable: false }); -Zotero.defineProperty(Zotero.Item.prototype, 'libraryID', { - get: function() { return this._libraryID; }, - set: function(val) { return this.setField('libraryID', val); } -}); -Zotero.defineProperty(Zotero.Item.prototype, 'key', { - get: function() { return this._key; }, - set: function(val) { return this.setField('key', val); } -}); + +for (let name of ['libraryID', 'key', 'dateAdded', 'dateModified', 'version', 'synced', + 'createdByUserID', 'lastModifiedByUserID']) { + let prop = '_' + name; + Zotero.defineProperty(Zotero.Item.prototype, name, { + get: function () { return this[prop]; }, + set: function (val) { return this.setField(name, val); } + }); +} + Zotero.defineProperty(Zotero.Item.prototype, 'itemTypeID', { get: function() { return this._itemTypeID; } }); -Zotero.defineProperty(Zotero.Item.prototype, 'dateAdded', { - get: function() { return this._dateAdded; }, - set: function(val) { return this.setField('dateAdded', val); } -}); -Zotero.defineProperty(Zotero.Item.prototype, 'dateModified', { - get: function() { return this._dateModified; }, - set: function(val) { return this.setField('dateModified', val); } -}); -Zotero.defineProperty(Zotero.Item.prototype, 'version', { - get: function() { return this._version; }, - set: function(val) { return this.setField('version', val); } -}); -Zotero.defineProperty(Zotero.Item.prototype, 'synced', { - get: function() { return this._synced; }, - set: function(val) { return this.setField('synced', val); } -}); // .parentKey and .parentID defined in dataObject.js, but create aliases Zotero.defineProperty(Zotero.Item.prototype, 'parentItemID', { @@ -335,6 +323,8 @@ Zotero.Item.prototype._parseRowData = function(row) { case 'attachmentSyncState': case 'attachmentSyncedHash': case 'attachmentSyncedModificationTime': + case 'createdByUserID': + case 'lastModifiedByUserID': break; case 'itemID': @@ -668,6 +658,19 @@ Zotero.Item.prototype.setField = function(field, value, loadIn) { value = !!value; break; + case 'createdByUserID': + case 'lastModifiedByUserID': + if (typeof value != 'number' || value != parseInt(value)) { + throw new Error(`${field} must be a number`); + } + if (!this._libraryID) { + throw new Error(`libraryID must be set before setting ${field}`); + } + if (Zotero.Libraries.get(this._libraryID).libraryType != 'group') { + throw new Error(`${field} is only valid for group library items`); + } + break; + default: throw new Error('Primary field ' + field + ' cannot be changed in Zotero.Item.setField()'); @@ -1297,6 +1300,12 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) { } } + if (this._changed.primaryData + && (this._changed.primaryData.createdByUserID || this._changed.primaryData.lastModifiedByUserID)) { + let sql = "REPLACE INTO groupItems VALUES (?, ?, ?)"; + yield Zotero.DB.queryAsync(sql, [itemID, this._createdByUserID || null, this._lastModifiedByUserID || null]); + } + // // ItemData // @@ -4783,13 +4792,6 @@ Zotero.Item.prototype.migrateExtraFields = function () { } -////////////////////////////////////////////////////////////////////////////// -// -// Asynchronous load methods -// -////////////////////////////////////////////////////////////////////////////// - - /** * Return an item in the specified library equivalent to this item * @@ -4816,6 +4818,34 @@ Zotero.Item.prototype.addLinkedItem = Zotero.Promise.coroutine(function* (item) }); + +/** + * Update createdByUserID/lastModifiedByUserID, efficiently + * + * Used by sync code + */ +Zotero.Item.prototype.updateCreatedByUser = async function (createdByUserID, lastModifiedByUserID) { + this._createdByUserID = createdByUserID || null; + this._lastModifiedByUserID = lastModifiedByUserID || null; + + var sql = "REPLACE INTO groupItems VALUES (?, ?, ?)"; + await Zotero.DB.queryAsync(sql, [this.id, this._createdByUserID, this._lastModifiedByUserID]); + + if (this._changed.primaryData) { + for (let x of ['createdByUserID', 'lastModifiedByUserID']) { + if (this._changed.primaryData[x]) { + if (Objects.keys(this._changed.primaryData).length == 1) { + delete this._changed.primaryData; + } + else { + delete this._changed.primaryData[x]; + } + } + } + } +}; + + ////////////////////////////////////////////////////////////////////////////// // // Private methods diff --git a/chrome/content/zotero/xpcom/data/items.js b/chrome/content/zotero/xpcom/data/items.js index bfbf478dd1..be3d050c3b 100644 --- a/chrome/content/zotero/xpcom/data/items.js +++ b/chrome/content/zotero/xpcom/data/items.js @@ -49,6 +49,9 @@ Zotero.Items = function() { version: "O.version", synced: "O.synced", + createdByUserID: "createdByUserID", + lastModifiedByUserID: "lastModifiedByUserID", + firstCreator: _getFirstCreatorSQL(), sortCreator: _getSortCreatorSQL(), @@ -79,7 +82,8 @@ Zotero.Items = function() { + "LEFT JOIN items INoP ON (INo.parentItemID=INoP.itemID) " + "LEFT JOIN deletedItems DI ON (O.itemID=DI.itemID) " + "LEFT JOIN publicationsItems PI ON (O.itemID=PI.itemID) " - + "LEFT JOIN charsets CS ON (IA.charsetID=CS.charsetID)"; + + "LEFT JOIN charsets CS ON (IA.charsetID=CS.charsetID)" + + "LEFT JOIN groupItems GI ON (O.itemID=GI.itemID)"; this._relationsTable = "itemRelations"; diff --git a/chrome/content/zotero/xpcom/schema.js b/chrome/content/zotero/xpcom/schema.js index 54912f009e..84c3e2f578 100644 --- a/chrome/content/zotero/xpcom/schema.js +++ b/chrome/content/zotero/xpcom/schema.js @@ -3218,6 +3218,11 @@ Zotero.Schema = new function(){ yield Zotero.DB.queryAsync("CREATE INDEX deletedSearches_dateDeleted ON deletedSearches(dateDeleted)"); } + else if (i == 112) { + yield Zotero.DB.queryAsync("DROP TABLE IF EXISTS users"); + yield Zotero.DB.queryAsync("CREATE TABLE users (\n userID INTEGER PRIMARY KEY,\n name TEXT NOT NULL\n)"); + } + // If breaking compatibility or doing anything dangerous, clear minorUpdateFrom } diff --git a/chrome/content/zotero/xpcom/sync/syncEngine.js b/chrome/content/zotero/xpcom/sync/syncEngine.js index 80a24a8c3e..ed55c2f492 100644 --- a/chrome/content/zotero/xpcom/sync/syncEngine.js +++ b/chrome/content/zotero/xpcom/sync/syncEngine.js @@ -223,6 +223,15 @@ Zotero.Sync.Data.Engine.prototype.start = Zotero.Promise.coroutine(function* () skipNotifier: true }); + if (this.library.libraryType == 'group') { + try { + yield this._updateGroupItemUsers(); + } + catch (e) { + Zotero.logError(e); + } + } + Zotero.debug("Done syncing " + this.library.name); }); @@ -1428,6 +1437,58 @@ Zotero.Sync.Data.Engine.prototype._uploadDeletions = Zotero.Promise.coroutine(fu }); +/** + * Update createdByUserID/lastModifiedByUserID for previously downloaded group items + * + * TEMP: Currently only processes one batch of items, but before we start displaying the names, + * we'll need to update it to fetch all + */ +Zotero.Sync.Data.Engine.prototype._updateGroupItemUsers = async function () { + // TODO: Do more at once when we actually start showing these names + var max = this.apiClient.MAX_OBJECTS_PER_REQUEST; + + var sql = "SELECT key FROM items LEFT JOIN groupItems GI USING (itemID) " + + `WHERE libraryID=? AND GI.itemID IS NULL ORDER BY itemID LIMIT ${max}`; + var keys = await Zotero.DB.columnQueryAsync(sql, this.libraryID); + if (!keys.length) { + return; + } + + Zotero.debug(`Updating item users in ${this.library.name}`); + + var jsonItems = await this.apiClient.downloadObjects( + this.library.libraryType, this.libraryTypeID, 'item', keys + )[0]; + + if (!Array.isArray(jsonItems)) { + Zotero.logError(e); + return; + } + + for (let jsonItem of jsonItems) { + let item = Zotero.Items.getByLibraryAndKey(this.libraryID, jsonItem.key); + let params = [null, null]; + + // This should almost always exist, but maybe doesn't for some old items? + if (jsonItem.meta.createdByUser) { + let { id: userID, username, name } = jsonItem.meta.createdByUser; + await Zotero.Users.setName(userID, name !== '' ? name : username); + params[0] = userID; + } + + if (jsonItem.meta.lastModifiedByUser) { + let { id: userID, username, name } = jsonItem.meta.lastModifiedByUser; + await Zotero.Users.setName(userID, name !== '' ? name : username); + params[1] = userID; + } + + await item.updateCreatedByUser.apply(item, params); + } + + return; +}; + + Zotero.Sync.Data.Engine.prototype._getJSONForObject = function (objectType, id, options = {}) { return Zotero.DB.executeTransaction(function* () { var objectsClass = Zotero.DataObjectUtilities.getObjectsClassForObjectType(objectType); diff --git a/chrome/content/zotero/xpcom/sync/syncLocal.js b/chrome/content/zotero/xpcom/sync/syncLocal.js index 42eea1a61b..ccc3aa419a 100644 --- a/chrome/content/zotero/xpcom/sync/syncLocal.js +++ b/chrome/content/zotero/xpcom/sync/syncLocal.js @@ -1459,8 +1459,23 @@ Zotero.Sync.Data.Local = { if (!options.skipData) { obj.fromJSON(json.data, { strict: true }); } - if (obj.objectType == 'item' && obj.isImportedAttachment()) { - yield this._checkAttachmentForDownload(obj, json.data.mtime, options.isNewObject); + if (obj.objectType == 'item') { + // Update createdByUserID and lastModifiedByUserID + for (let p of ['createdByUser', 'lastModifiedByUser']) { + if (json.meta && json.meta[p]) { + let { id: userID, username, name } = json.meta[p]; + obj[p + 'ID'] = userID; + name = name !== '' ? name : username; + // Update stored name if it changed + if (Zotero.Users.getName(userID) != name) { + yield Zotero.Users.setName(userID, name); + } + } + } + + if (obj.isImportedAttachment()) { + yield this._checkAttachmentForDownload(obj, json.data.mtime, options.isNewObject); + } } obj.version = json.data.version; if (!options.saveAsUnsynced) { diff --git a/chrome/content/zotero/xpcom/users.js b/chrome/content/zotero/xpcom/users.js index 8eda0c91fc..4f3cb9dbb7 100644 --- a/chrome/content/zotero/xpcom/users.js +++ b/chrome/content/zotero/xpcom/users.js @@ -28,10 +28,11 @@ Zotero.Users = new function () { var _libraryID; var _username; var _localUserKey; + var _users = {}; - this.init = Zotero.Promise.coroutine(function* () { + this.init = async function () { let sql = "SELECT key, value FROM settings WHERE setting='account'"; - let rows = yield Zotero.DB.queryAsync(sql); + let rows = await Zotero.DB.queryAsync(sql); let settings = {}; for (let i=0; i