Save createdByUserID and lastModifiedByUserID for group items

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; },
+for (let name of ['libraryID', 'key', 'dateAdded', 'dateModified', 'version', 'synced',
-	set: function(val) { return this.setField('libraryID', val); }
+		'createdByUserID', 'lastModifiedByUserID']) {
-});
+	let prop = '_' + name;
-Zotero.defineProperty(Zotero.Item.prototype, 'key', {
+	Zotero.defineProperty(Zotero.Item.prototype, name, {
-	get: function() { return this._key; },
+		get: function () { return this[prop]; },
-	set: function(val) { return this.setField('key', val); }
+		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

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";
 	

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
 		}
 		

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);

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()) {
+			if (obj.objectType == 'item') {
-				yield this._checkAttachmentForDownload(obj, json.data.mtime, options.isNewObject);
+				// 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) {

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<rows.length; i++) {
@@ -56,11 +57,16 @@ Zotero.Users = new function () {
 			let key = Zotero.randomString(8);
 			
 			sql = "INSERT INTO settings VALUES ('account', 'localUserKey', ?)";
-			yield Zotero.DB.queryAsync(sql, key);
+			await Zotero.DB.queryAsync(sql, key);
 			
 			_localUserKey = key;
 		}
-	});
+		
+		rows = await Zotero.DB.queryAsync("SELECT userID, name FROM users");
+		for (let row of rows) {
+			_users[row.userID] = row.name;
+		}
+	};
 	
 	
 	this.getCurrentUserID = function() { return _userID };
@@ -87,4 +93,18 @@ Zotero.Users = new function () {
 	this.getLocalUserKey = function () {
 		return _localUserKey;
 	};
+	
+	
+	this.getName = function (userID) {
+		return _users[userID] || '';
+	};
+	
+	
+	this.setName = async function (userID, name) {
+		if (this.getName(userID) == name) {
+			return;
+		}
+		await Zotero.DB.queryAsync("REPLACE INTO users VALUES (?, ?)", [userID, name]);
+		_users[userID] = name;
+	}
 };

resource/schema/userdata.sql

@@ -1,4 +1,4 @@
--- 111
+-- 112
 
 -- Copyright (c) 2009 Center for History and New Media
 --                    George Mason University, Fairfax, Virginia, USA
@@ -275,7 +275,7 @@ CREATE TABLE libraries (
 
 CREATE TABLE users (
     userID INTEGER PRIMARY KEY,
-    username TEXT NOT NULL
+    name TEXT NOT NULL
 );
 
 CREATE TABLE groups (

test/tests/syncEngineTest.js

@@ -4046,6 +4046,85 @@ describe("Zotero.Sync.Data.Engine", function () {
 	});
 	
 	
+	describe("#_updateGroupItemUsers()", function () {
+		it("should update createdByUserID and lastModifiedByUserID", async function () {
+			var { id: groupID, libraryID } = await createGroup();
+			({ engine, client, caller } = await setup({ libraryID }));
+			
+			var item1 = await createDataObject('item', { libraryID });
+			var item1DateModified = item1.dateModified;
+			var item2 = await createDataObject('item', { libraryID });
+			var responseJSON = [
+				item1.toResponseJSON(),
+				item2.toResponseJSON()
+			];
+			responseJSON[0].meta.createdByUser = {
+				id: 152315,
+				username: "user152315",
+				name: "User 152315"
+			};
+			responseJSON[0].meta.lastModifiedByUser = {
+				id: 352352,
+				username: "user352352",
+				name: "User 352352"
+			};
+			responseJSON[1].meta.createdByUser = {
+				id: 346534,
+				username: "user346534",
+				name: "User 346534"
+			};
+			
+			setResponse({
+				method: "GET",
+				url: `groups/${groupID}/items?itemKey=${item1.key}%2C${item2.key}&includeTrashed=1`,
+				status: 200,
+				headers: {
+					"Last-Modified-Version": 5
+				},
+				json: responseJSON
+			});
+			
+			await engine._updateGroupItemUsers();
+			
+			assert.equal(item1.createdByUserID, 152315);
+			assert.equal(item1.lastModifiedByUserID, 352352);
+			assert.equal(item1.dateModified, item1DateModified);
+			assert.equal(item2.createdByUserID, 346534);
+		});
+		
+		
+		it("should use username if no name", async function () {
+			var { id: groupID, libraryID } = await createGroup();
+			({ engine, client, caller } = await setup({ libraryID }));
+			
+			var item = await createDataObject('item', { libraryID });
+			var responseJSON = [
+				item.toResponseJSON()
+			];
+			responseJSON[0].meta.createdByUser = {
+				id: 235235,
+				username: "user235235",
+				name: ""
+			};
+			
+			setResponse({
+				method: "GET",
+				url: `groups/${groupID}/items?itemKey=${item.key}&includeTrashed=1`,
+				status: 200,
+				headers: {
+					"Last-Modified-Version": 6
+				},
+				json: responseJSON
+			});
+			
+			await engine._updateGroupItemUsers();
+			
+			assert.equal(item.createdByUserID, 235235);
+			assert.equal(Zotero.Users.getName(235235), 'user235235');
+		});
+	});
+	
+	
 	describe("#_upgradeCheck()", function () {
 		it("should upgrade a library last synced with the classic sync architecture", function* () {
 			var userLibraryID = Zotero.Libraries.userLibraryID;

test/tests/syncLocalTest.js

@@ -930,6 +930,69 @@ describe("Zotero.Sync.Data.Local", function() {
 				"SELECT COUNT(*) FROM items WHERE libraryID=? AND key=?", [libraryID, key2]
 			), 0);
 		});
+		
+		it("should update createdByUser and lastModifiedBy when saving group item", async function () {
+			var { libraryID } = await getGroup();
+			let item = await createDataObject('item', { libraryID });
+			let data = item.toJSON();
+			data.key = item.key;
+			data.version = 10;
+			let json = {
+				key: item.key,
+				version: 10,
+				meta: {
+					createdByUser: {
+						id: 12345,
+						username: 'foo',
+						name: 'Foo Foo'
+					},
+					lastModifiedByUser: {
+						id: 23456,
+						username: 'bar',
+						name: 'Bar Bar'
+					}
+				},
+				data
+			};
+			await Zotero.Sync.Data.Local.processObjectsFromJSON(
+				'item', libraryID, [json], { stopOnError: true }
+			);
+			let localItem = Zotero.Items.getByLibraryAndKey(libraryID, item.key);
+			assert.isTrue(localItem.synced);
+			
+			assert.equal(localItem.createdByUserID, 12345);
+			assert.equal(localItem.lastModifiedByUserID, 23456);
+			assert.equal(Zotero.Users.getName(12345), 'Foo Foo');
+			assert.equal(Zotero.Users.getName(23456), 'Bar Bar');
+		});
+		
+		it("should use username if empty name for createdByUser when saving group item", async function () {
+			var { libraryID } = await getGroup();
+			let item = await createDataObject('item', { libraryID });
+			let data = item.toJSON();
+			data.key = item.key;
+			data.version = 10;
+			let json = {
+				key: item.key,
+				version: 10,
+				meta: {
+					createdByUser: {
+						id: 12345,
+						username: 'foo',
+						name: ''
+					},
+				},
+				data
+			};
+			await Zotero.Sync.Data.Local.processObjectsFromJSON(
+				'item', libraryID, [json], { stopOnError: true }
+			);
+			let localItem = Zotero.Items.getByLibraryAndKey(libraryID, item.key);
+			assert.isTrue(localItem.synced);
+			
+			assert.equal(localItem.createdByUserID, 12345);
+			assert.equal(Zotero.Users.getName(12345), 'foo');
+		});
 	})
 	
 	describe("Sync Queue", function () {