From aaed7ddfc0260f31b344adf28ca38ebdde111db4 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Wed, 18 Mar 2026 22:35:58 -0400 Subject: [PATCH] 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 --- .../zotero/components/virtualized-table.jsx | 3 +- chrome/content/zotero/elements/itemBox.js | 33 ++++++++ chrome/content/zotero/itemTree.jsx | 27 ++++++- chrome/content/zotero/itemTreeColumns.jsx | 18 +++++ chrome/content/zotero/xpcom/data/item.js | 11 +-- .../content/zotero/xpcom/sync/syncEngine.js | 80 ++++++++++--------- chrome/locale/en-US/zotero/zotero.ftl | 3 + test/tests/itemTest.js | 34 ++++++++ 8 files changed, 162 insertions(+), 47 deletions(-) diff --git a/chrome/content/zotero/components/virtualized-table.jsx b/chrome/content/zotero/components/virtualized-table.jsx index 2b2949c33d..5f833c417c 100644 --- a/chrome/content/zotero/components/virtualized-table.jsx +++ b/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) { diff --git a/chrome/content/zotero/elements/itemBox.js b/chrome/content/zotero/elements/itemBox.js index 28c36de908..40d791e29b 100644 --- a/chrome/content/zotero/elements/itemBox.js +++ b/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); + } + } } // diff --git a/chrome/content/zotero/itemTree.jsx b/chrome/content/zotero/itemTree.jsx index d8553efa60..63308cbc01 100644 --- a/chrome/content/zotero/itemTree.jsx +++ b/chrome/content/zotero/itemTree.jsx @@ -1513,7 +1513,16 @@ 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); diff --git a/chrome/content/zotero/itemTreeColumns.jsx b/chrome/content/zotero/itemTreeColumns.jsx index 4833c5f2ba..db1ae7aec0 100644 --- a/chrome/content/zotero/itemTreeColumns.jsx +++ b/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"], diff --git a/chrome/content/zotero/xpcom/data/item.js b/chrome/content/zotero/xpcom/data/item.js index 9ae0b4f302..ee5f9f2bde 100644 --- a/chrome/content/zotero/xpcom/data/item.js +++ b/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 - // before we start showing a last-modified-by name in the UI so that it updates - // 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 (!lastModifiedByUserID && !isNew && !options.skipDateModifiedUpdate) { + lastModifiedByUserID = Zotero.Users.getCurrentUserID(); + } } if (createdByUserID || lastModifiedByUserID) { try { diff --git a/chrome/content/zotero/xpcom/sync/syncEngine.js b/chrome/content/zotero/xpcom/sync/syncEngine.js index f23c6350a5..9ce56af45f 100644 --- a/chrome/content/zotero/xpcom/sync/syncEngine.js +++ b/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 - var max = this.apiClient.MAX_OBJECTS_PER_REQUEST; - - var sql = "SELECT key FROM items LEFT JOIN groupItems GI USING (itemID) " + let max = this.apiClient.MAX_OBJECTS_PER_REQUEST; + + 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( - this.library.libraryType, this.libraryTypeID, 'item', keys - )[0]; - - if (error) { - Zotero.logError(error); - 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; + + let lastCount; + while (keys.length) { + // If no progress was made, bail + if (keys.length === lastCount) { + Zotero.debug(`${keys.length} items remaining without user data -- stopping`); + break; } - - if (jsonItem.meta.lastModifiedByUser) { - let { id: userID, username, name } = jsonItem.meta.lastModifiedByUser; - await Zotero.Users.setName(userID, name !== '' ? name : username); - params[1] = userID; + lastCount = keys.length; + + let { json: jsonItems, error } = await this.apiClient.downloadObjects( + this.library.libraryType, this.libraryTypeID, 'item', keys + )[0]; + + if (error) { + Zotero.logError(error); + return; } - - await item.updateCreatedByUser.apply(item, params); + + 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); + } + + keys = await Zotero.DB.columnQueryAsync(sql, this.libraryID); } - - return; }; diff --git a/chrome/locale/en-US/zotero/zotero.ftl b/chrome/locale/en-US/zotero/zotero.ftl index 9b0e3535d6..b1e441fb23 100644 --- a/chrome/locale/en-US/zotero/zotero.ftl +++ b/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… diff --git a/test/tests/itemTest.js b/test/tests/itemTest.js index 99d0aec0b9..50bb0efad9 100644 --- a/test/tests/itemTest.js +++ b/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); + }); + }); });