Add Added By and Modified By columns for group libraries

- Add columns as options in item tree, visible only in group libraries
- Fall back to createdByUserID for Modified By when lastModifiedByUserID
  is not set
- Update lastModifiedByUserID on local saves when dateModified changes
- Fix backfill in _updateGroupItemUsers() to process all batches
- Update formatColumnName() to support Fluent string keys

Closes #233

chrome/content/zotero/components/virtualized-table.jsx

@@ -1864,7 +1864,8 @@ function formatColumnName(column) {
 	if (column.label in Zotero.Intl.strings) {
 		return Zotero.getString(column.label);
 	}
-	else if (/^[^\s]+\w\.\w[^\s]+$/.test(column.label)) {
+	// Dotted keys (.properties) or hyphenated keys with 3+ segments (Fluent)
+	else if (/^[^\s]+\w\.\w[^\s]+$/.test(column.label) || /^\w+(-\w+){2,}$/.test(column.label)) {
 		try {
 			let labelString = Zotero.getString(column.label);
 			if (labelString !== column.label) {

chrome/content/zotero/elements/itemBox.js

@@ -674,6 +674,39 @@
 					
 					rowData.appendChild(button);
 				}
+
+				// Insert user row after the corresponding date row
+				if (Zotero.Libraries.get(this.item.libraryID).libraryType === 'group') {
+					let userID;
+					let userFieldName;
+					let labelKey;
+					if (fieldName === 'dateAdded') {
+						userID = this.item.createdByUserID;
+						userFieldName = 'addedBy';
+						labelKey = 'items-column-added-by';
+					}
+					else if (fieldName === 'dateModified') {
+						userID = this.item.lastModifiedByUserID
+							|| this.item.createdByUserID;
+						userFieldName = 'lastModifiedBy';
+						labelKey = 'items-column-modified-by';
+					}
+					if (userID) {
+						let userLabel = document.createElement("div");
+						userLabel.className = "meta-label";
+						userLabel.setAttribute("fieldname", userFieldName);
+						userLabel.appendChild(this.createLabelElement({
+							text: Zotero.getString(labelKey),
+							id: `itembox-field-${userFieldName}-label`,
+						}));
+						let userData = document.createElement("div");
+						userData.className = "meta-data";
+						userData.appendChild(this.createValueElement({
+							text: Zotero.Users.getName(userID),
+						}));
+						this.addDynamicRow(userLabel, userData);
+					}
+				}
 			}
 			
 			//

chrome/content/zotero/itemTree.jsx

@@ -1514,6 +1514,15 @@ var ItemTree = class ItemTree extends LibraryTree {
 			case 'lastRead':
 				return item.getItemLastRead();
 
+			case 'addedBy':
+				return item.createdByUserID
+					? Zotero.Users.getName(item.createdByUserID) : '';
+
+			case 'lastModifiedBy': {
+				let userID = item.lastModifiedByUserID || item.createdByUserID;
+				return userID ? Zotero.Users.getName(userID) : '';
+			}
+
 			default:
 				let extraField = this.props.getExtraField(row.ref, field);
 				if (extraField !== undefined) return extraField;
@@ -3479,6 +3488,12 @@ var ItemTree = class ItemTree extends LibraryTree {
 		row.numNotes = treeRow.numNotes() || "";
 		row.feed = (treeRow.ref.isFeedItem && Zotero.Feeds.get(treeRow.ref.libraryID).name) || "";
 		row.lastRead = row.isItem ? treeRow.ref.getItemLastRead() : "";
+		row.addedBy = row.isItem && treeRow.ref.createdByUserID
+			? Zotero.Users.getName(treeRow.ref.createdByUserID) : "";
+		row.lastModifiedBy = row.isItem
+			&& (treeRow.ref.lastModifiedByUserID || treeRow.ref.createdByUserID)
+			? Zotero.Users.getName(treeRow.ref.lastModifiedByUserID
+				|| treeRow.ref.createdByUserID) : "";
 
 		if (treeRow.ref.isFileAttachment()
 				// TODO: Adjust this if we localize "Snapshot"
@@ -3563,6 +3578,15 @@ var ItemTree = class ItemTree extends LibraryTree {
 		
 		if (!this.props.persistColumns) return;
 		Zotero.debug(`Storing itemTree ${this.id} column prefs`, 2);
+		// Preserve prefs for columns not active in the current view (e.g.,
+		// group-only columns when viewing a personal library)
+		if (this._columnPrefs) {
+			for (let [key, val] of Object.entries(this._columnPrefs)) {
+				if (!(key in prefs) && COLUMNS.some(c => c.dataKey === key)) {
+					prefs[key] = val;
+				}
+			}
+		}
 		this._columnPrefs = prefs;
 		if (!this._columns) {
 			Zotero.debug(new Error(), 1);
@@ -3672,6 +3696,7 @@ var ItemTree = class ItemTree extends LibraryTree {
 		for (let column of columns) {
 			if (this.props.persistColumns) {
 				if (column.disabledIn && column.disabledIn.includes(visibilityGroup)) continue;
+				if (column.groupLibrariesOnly && !this.collectionTreeRow.isWithinGroup()) continue;
 				const columnSettings = columnsSettings[column.dataKey];
 				if (!columnSettings && this.id === 'main') {
 					column = this._setLegacyColumnSettings(column);

chrome/content/zotero/itemTreeColumns.jsx

@@ -372,6 +372,24 @@ const COLUMNS = [
 		staticWidth: true,
 		zoteroPersist: ["width", "hidden", "sortDirection"]
 	},
+	{
+		dataKey: "addedBy",
+		groupLibrariesOnly: true,
+		showInColumnPicker: true,
+		columnPickerSubMenu: true,
+		label: "items-column-added-by",
+		flex: 1,
+		zoteroPersist: ["width", "hidden", "sortDirection"]
+	},
+	{
+		dataKey: "lastModifiedBy",
+		groupLibrariesOnly: true,
+		showInColumnPicker: true,
+		columnPickerSubMenu: true,
+		label: "items-column-modified-by",
+		flex: 1,
+		zoteroPersist: ["width", "hidden", "sortDirection"]
+	},
 	{
 		dataKey: "feed",
 		disabledIn: ["default", "feed"],

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

@@ -1470,14 +1470,9 @@ Zotero.Item.prototype._saveData = async function (env) {
 			if (!createdByUserID && isNew) {
 				createdByUserID = Zotero.Users.getCurrentUserID();
 			}
-			// TEMP: For now, don't update lastModifiedByUserID -- we may want to start doing this
+			if (!lastModifiedByUserID && !isNew && !options.skipDateModifiedUpdate) {
-			// before we start showing a last-modified-by name in the UI so that it updates
+				lastModifiedByUserID = Zotero.Users.getCurrentUserID();
-			// immediately rather than waiting until a sync happens, but we should figure out if we
+			}
-			// want all changes to count and make sure the dataserver follows the same behavior.
-			//
-			//if (!lastModifiedByUserID && !isNew) {
-			//	lastModifiedByUserID = Zotero.Users.getCurrentUserID();
-			//}
 		}
 		if (createdByUserID || lastModifiedByUserID) {
 			try {

chrome/content/zotero/xpcom/sync/syncEngine.js

@@ -1465,53 +1465,59 @@ Zotero.Sync.Data.Engine.prototype._uploadDeletions = async function (objectType,
 
 /**
  * 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
+	let max = this.apiClient.MAX_OBJECTS_PER_REQUEST;
-	var max = this.apiClient.MAX_OBJECTS_PER_REQUEST;
 
-	var sql = "SELECT key FROM items LEFT JOIN groupItems GI USING (itemID) "
+	let 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);
+	let keys = await Zotero.DB.columnQueryAsync(sql, this.libraryID);
 	if (!keys.length) {
 		return;
 	}
 
 	Zotero.debug(`Updating item users in ${this.library.name}`);
 
-	var { json: jsonItems, error } = await this.apiClient.downloadObjects(
+	let lastCount;
-		this.library.libraryType, this.libraryTypeID, 'item', keys
+	while (keys.length) {
-	)[0];
+		// If no progress was made, bail
+		if (keys.length === lastCount) {
+			Zotero.debug(`${keys.length} items remaining without user data -- stopping`);
+			break;
+		}
+		lastCount = keys.length;
 
-	if (error) {
+		let { json: jsonItems, error } = await this.apiClient.downloadObjects(
-		Zotero.logError(error);
+			this.library.libraryType, this.libraryTypeID, 'item', keys
-		return;
+		)[0];
-	}
 
-	for (let jsonItem of jsonItems) {
+		if (error) {
-		let item = Zotero.Items.getByLibraryAndKey(this.libraryID, jsonItem.key);
+			Zotero.logError(error);
-		let params = [null, null];
+			return;
-		
-		// 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) {
+		for (let jsonItem of jsonItems) {
-			let { id: userID, username, name } = jsonItem.meta.lastModifiedByUser;
+			let item = Zotero.Items.getByLibraryAndKey(this.libraryID, jsonItem.key);
-			await Zotero.Users.setName(userID, name !== '' ? name : username);
+			let params = [null, null];
-			params[1] = userID;
+
+			// 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);
 		}
 
-		await item.updateCreatedByUser.apply(item, params);
+		keys = await Zotero.DB.columnQueryAsync(sql, this.libraryID);
 	}
-	
-	return;
 };
 
 

chrome/locale/en-US/zotero/zotero.ftl

@@ -329,6 +329,9 @@ items-table-cell-notes =
         *[other] { $count } Notes
     }
 
+items-column-added-by = Added By
+items-column-modified-by = Modified By
+
 report-error =
     .label = Report Error…
 

test/tests/itemTest.js

@@ -3322,4 +3322,38 @@ describe("Zotero.Item", function () {
 			assert.isUndefined((await item.toResponseJSONAsync()).links.attachment.attachmentSize);
 		});
 	});
+
+	describe("Group Item Users", function () {
+		let group;
+
+		before(async function () {
+			group = await createGroup();
+		});
+
+		it("should set createdByUserID for new group item", async function () {
+			let item = createUnsavedDataObject('item', { libraryID: group.libraryID });
+			await item.saveTx();
+			assert.equal(item.createdByUserID, Zotero.Users.getCurrentUserID());
+		});
+
+		it("should set lastModifiedByUserID when modifying group item", async function () {
+			let item = createUnsavedDataObject('item', { libraryID: group.libraryID });
+			await item.saveTx();
+
+			item.setField('title', 'Modified');
+			await item.saveTx();
+			assert.equal(item.lastModifiedByUserID, Zotero.Users.getCurrentUserID());
+		});
+
+		it("should not set lastModifiedByUserID with skipDateModifiedUpdate", async function () {
+			let item = createUnsavedDataObject('item', { libraryID: group.libraryID });
+			await item.saveTx();
+
+			item.addToCollection(
+				(await createDataObject('collection', { libraryID: group.libraryID })).id
+			);
+			await item.saveTx({ skipDateModifiedUpdate: true });
+			assert.isNull(item.lastModifiedByUserID);
+		});
+	});
 });