From abcd219714bf3b7484bdeb3b3a2eecc92ecedc6c Mon Sep 17 00:00:00 2001 From: Mimansa Jaiswal Date: Thu, 9 Jan 2025 17:34:57 -0800 Subject: [PATCH 1/3] Add type mismatch handling in merge process and update preferences --- addon/chrome/content/preferences.xhtml | 128 +++++++++++++------------ addon/locale/en-US/addon.ftl | 6 +- addon/locale/en-US/preferences.ftl | 12 ++- addon/locale/zh-CN/addon.ftl | 6 +- addon/locale/zh-CN/preferences.ftl | 12 ++- addon/prefs.js | 1 + package-lock.json | 7 +- src/modules/merger.ts | 86 ++++++++++++++++- src/utils/prefs.ts | 9 ++ 9 files changed, 195 insertions(+), 72 deletions(-) diff --git a/addon/chrome/content/preferences.xhtml b/addon/chrome/content/preferences.xhtml index c159137..3669f11 100644 --- a/addon/chrome/content/preferences.xhtml +++ b/addon/chrome/content/preferences.xhtml @@ -3,80 +3,88 @@ - - - - - - - - + + + + + + + + - + - - + + + + + + + + - - - - - - + + + + + + - - + + + - - - + + + - - + + - - + \ No newline at end of file diff --git a/addon/locale/en-US/addon.ftl b/addon/locale/en-US/addon.ftl index 03485a6..8f82180 100644 --- a/addon/locale/en-US/addon.ftl +++ b/addon/locale/en-US/addon.ftl @@ -61,6 +61,10 @@ bulk-merge-popup-restore = Restoring: { $item } duplicate-tooltip = Found { $total } duplicates from { $unique } unique { $items }. duplicate-not-found-tooltip = No duplicate items found. +type-mismatch-title = Item Types Are Different +type-mismatch-message = Some items have different types. Would you like to convert them to match or skip merging? +type-mismatch-convert = Convert Types +type-mismatch-skip = Skip Merging ## Menus menuitem-refresh-duplicates = Refresh @@ -72,4 +76,4 @@ menuitem-is-duplicate = They are duplicates # Add non-duplicate add-not-duplicates-alert-error-duplicates = The selected items are not identified as Duplicates. add-not-duplicates-alert-error-exist = The selected items already exist. -add-not-duplicates-alert-error-diff-library = Cannot add items from different libraries. +add-not-duplicates-alert-error-diff-library = Cannot add items from different libraries. \ No newline at end of file diff --git a/addon/locale/en-US/preferences.ftl b/addon/locale/en-US/preferences.ftl index 295ceda..b09df44 100644 --- a/addon/locale/en-US/preferences.ftl +++ b/addon/locale/en-US/preferences.ftl @@ -27,9 +27,19 @@ pref-default-master-item-detailed = pref-default-master-item-always-ask = .label = [Always Ask]: Ask for master item every time +pref-type-mismatch-title = Type Mismatch Preferences +pref-type-mismatch-description = How to handle when merging items of different types + +pref-type-mismatch-skip = + .label = [Skip]: Skip merging items of different types +pref-type-mismatch-convert = + .label = [Convert]: Convert items to match the master item type +pref-type-mismatch-ask = + .label = [Ask]: Ask what to do when types don't match + pref-view-title = View Preferences pref-view-description = Choose the widget to display in the main window pref-view-duplicate-stats = .label = Append duplicate counts to the "Duplicate Items" entry, e.g., Duplicate Items 2/6 -pref-help = { $name } Build { $version } { $time } +pref-help = { $name } Build { $version } { $time } \ No newline at end of file diff --git a/addon/locale/zh-CN/addon.ftl b/addon/locale/zh-CN/addon.ftl index 0a4285a..aba2421 100644 --- a/addon/locale/zh-CN/addon.ftl +++ b/addon/locale/zh-CN/addon.ftl @@ -62,6 +62,10 @@ bulk-merge-popup-restore = 正在恢复: { $item } duplicate-tooltip = 检测到 { $total } 个重复项，源自 { $unique } 个唯一条目。 duplicate-not-found-tooltip = 未找到重复条目 +type-mismatch-title = 条目类型不匹配 +type-mismatch-message = 部分条目类型不一致。您想转换它们的类型还是跳过合并？ +type-mismatch-convert = 转换类型 +type-mismatch-skip = 跳过合并 ## Menus menuitem-refresh-duplicates = 刷新 @@ -73,4 +77,4 @@ menuitem-is-duplicate = 标记为重复条目 # Add non-duplicate add-not-duplicates-alert-error-duplicates = 所选条目没有被识别为重复条目。 add-not-duplicates-alert-error-exist = 所选条目已存在。 -add-not-duplicates-alert-error-diff-library = 无法添加不同库中的条目。 +add-not-duplicates-alert-error-diff-library = 无法添加不同库中的条目。 \ No newline at end of file diff --git a/addon/locale/zh-CN/preferences.ftl b/addon/locale/zh-CN/preferences.ftl index 2987847..48a1f89 100644 --- a/addon/locale/zh-CN/preferences.ftl +++ b/addon/locale/zh-CN/preferences.ftl @@ -27,9 +27,19 @@ pref-default-master-item-detailed = pref-default-master-item-always-ask = .label = [始终询问]: 每次都询问我该如何选择主条目 +pref-type-mismatch-title = 条目类型不匹配设置 +pref-type-mismatch-description = 如何处理不同类型条目的合并 + +pref-type-mismatch-skip = + .label = [跳过]: 跳过不同类型条目的合并 +pref-type-mismatch-convert = + .label = [转换]: 将条目转换为主条目类型 +pref-type-mismatch-ask = + .label = [询问]: 类型不匹配时询问如何处理 + pref-view-title = 视图设置 pref-view-description = 选择重复条目的显示方式 pref-view-duplicate-stats = .label = 在"重复条目"标签后显示重复条目数量。如，重复条目 2/6 -pref-help = { $name } Build { $version } { $time } +pref-help = { $name } Build { $version } { $time } \ No newline at end of file diff --git a/addon/prefs.js b/addon/prefs.js index aed0e64..c83cb49 100644 --- a/addon/prefs.js +++ b/addon/prefs.js @@ -2,3 +2,4 @@ pref("__prefsPrefix__.duplicate.default.action", "ask"); pref("__prefsPrefix__.bulk.master.item", "oldest"); pref("__prefsPrefix__.duplicate.stats.enable", true); +pref("__prefsPrefix__.duplicate.type.mismatch", "skip"); // Options: skip, convert, ask \ No newline at end of file diff --git a/package-lock.json b/package-lock.json index a7d9018..c0f4129 100644 --- a/package-lock.json +++ b/package-lock.json @@ -8873,10 +8873,11 @@ } }, "node_modules/micromatch": { - "version": "4.0.7", - "resolved": "https://registry.npmjs.org/micromatch/-/micromatch-4.0.7.tgz", - "integrity": "sha512-LPP/3KorzCwBxfeUuZmaR6bG2kdeHSbe0P2tY3FLRU4vYrjYz5hI4QZwV0njUx3jeuKe67YukQ1LSPZBKDqO/Q==", + "version": "4.0.8", + "resolved": "https://registry.npmjs.org/micromatch/-/micromatch-4.0.8.tgz", + "integrity": "sha512-PXwfBhYu0hBCPw8Dn0E+WDYb7af3dSLVWKi3HGv84IdF4TyFoC0ysxFd0Goxw7nSv4T/PzEJQxsYsEiFCKo2BA==", "dev": true, + "license": "MIT", "dependencies": { "braces": "^3.0.3", "picomatch": "^2.3.1" diff --git a/src/modules/merger.ts b/src/modules/merger.ts index 4bf9acd..495248e 100644 --- a/src/modules/merger.ts +++ b/src/modules/merger.ts @@ -1,3 +1,6 @@ +import { getPref, TypeMismatch, setPref } from "../utils/prefs"; +import { getString } from "../utils/locale"; + export async function merge( masterItem: Zotero.Item, otherItems: Zotero.Item[], // Already sorted @@ -5,16 +8,89 @@ export async function merge( Zotero.CollectionTreeCache.clear(); const masterItemType = masterItem.itemTypeID; - otherItems = otherItems.filter((item) => item.itemTypeID === masterItemType); - if (otherItems.length === 0) { - return; + const mismatchedItems = otherItems.filter(item => item.itemTypeID !== masterItemType); + + if (mismatchedItems.length > 0) { + const typeMismatchPref = getPref("duplicate.type.mismatch") as TypeMismatch; + + if (typeMismatchPref === TypeMismatch.ASK) { + const dialog = new ztoolkit.Dialog(3, 1) + .setDialogData({ + action: TypeMismatch.SKIP, + savePreference: false // This controls if we save preference permanently + }) + .addCell(0, 0, { + tag: "h2", + properties: { innerHTML: getString("type-mismatch-message") } + }) + .addCell(1, 0, { + tag: "div", + children: [ + { + tag: "input", + id: "save_pref", + attributes: { + type: "checkbox", + "data-bind": "savePreference", + "data-prop": "checked" + } + }, + { + tag: "label", + attributes: { for: "save_pref" }, + properties: { innerHTML: getString("du-dialog-as-default") } + } + ] + }) + .addButton(getString("type-mismatch-convert"), "btn_convert", { + callback: () => { + dialog.dialogData.action = TypeMismatch.CONVERT; + // Only save preference if user checked the box + if (dialog.dialogData.savePreference) { + setPref("duplicate.type.mismatch", TypeMismatch.CONVERT); + } + } + }) + .addButton(getString("type-mismatch-skip"), "btn_skip", { + callback: () => { + dialog.dialogData.action = TypeMismatch.SKIP; + // Only save preference if user checked the box + if (dialog.dialogData.savePreference) { + setPref("duplicate.type.mismatch", TypeMismatch.SKIP); + } + } + }); + + // Remove loadCallback and unloadCallback since we handle preference in button callbacks + + dialog.open(getString("type-mismatch-title"), { + centerscreen: true, + resizable: true + }); + + await dialog.dialogData.loadLock?.promise; + + if (dialog.dialogData.action === TypeMismatch.CONVERT) { + await Promise.all(mismatchedItems.map(item => item.setType(masterItemType))); + } else { + otherItems = otherItems.filter(item => item.itemTypeID === masterItemType); + } + } + else if (typeMismatchPref === TypeMismatch.CONVERT) { + await Promise.all(mismatchedItems.map(item => item.setType(masterItemType))); + } + else { // TypeMismatch.SKIP + otherItems = otherItems.filter(item => item.itemTypeID === masterItemType); + } } + if (otherItems.length === 0) return; + const masterJSON = masterItem.toJSON(); const candidateJSON: { + //[field in Zotero.Item.DataType]?: string | unknown; [field in _ZoteroTypes.Item.DataType]?: string | unknown; } = otherItems.reduce((acc, obj) => ({ ...acc, ...obj.toJSON() }), {}); - // Refer to https://github.com/zotero/zotero/blob/main/chrome/content/zotero/duplicatesMerge.js#L151 // New link since 02/02/2024: https://github.com/zotero/zotero/blob/main/chrome/content/zotero/elements/duplicatesMergePane.js#L172 // Exclude certain properties that are empty in the cloned object, so we don't clobber them @@ -22,4 +98,4 @@ export async function merge( masterItem.fromJSON({ ...keep, ...masterJSON }); return await Zotero.Items.merge(masterItem, otherItems); -} +} \ No newline at end of file diff --git a/src/utils/prefs.ts b/src/utils/prefs.ts index 14b5c39..8e96d9f 100644 --- a/src/utils/prefs.ts +++ b/src/utils/prefs.ts @@ -51,3 +51,12 @@ export enum MasterItem { MODIFIED = "modified", DETAILED = "detailed", } + +/** + * NOTE: Corresponding to radio values in addon/chrome/content/preferences.xhtml. + */ +export enum TypeMismatch { + SKIP = "skip", + CONVERT = "convert", + ASK = "ask" +} \ No newline at end of file From 36de581ddcbb8c2a38c3cdf4b61df270d711fb3e Mon Sep 17 00:00:00 2001 From: Mimansa Jaiswal Date: Thu, 9 Jan 2025 18:11:46 -0800 Subject: [PATCH 2/3] Refactor type mismatch handling in merge process to improve clarity and efficiency --- src/modules/merger.ts | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/src/modules/merger.ts b/src/modules/merger.ts index 495248e..e1951be 100644 --- a/src/modules/merger.ts +++ b/src/modules/merger.ts @@ -8,9 +8,10 @@ export async function merge( Zotero.CollectionTreeCache.clear(); const masterItemType = masterItem.itemTypeID; - const mismatchedItems = otherItems.filter(item => item.itemTypeID !== masterItemType); + // Check if any items need type conversion + const hasMismatch = otherItems.some(item => item.itemTypeID !== masterItemType); - if (mismatchedItems.length > 0) { + if (hasMismatch) { const typeMismatchPref = getPref("duplicate.type.mismatch") as TypeMismatch; if (typeMismatchPref === TypeMismatch.ASK) { @@ -71,13 +72,24 @@ export async function merge( await dialog.dialogData.loadLock?.promise; if (dialog.dialogData.action === TypeMismatch.CONVERT) { - await Promise.all(mismatchedItems.map(item => item.setType(masterItemType))); + // Convert all mismatched items in place + await Promise.all(otherItems.map(async item => { + if (item.itemTypeID !== masterItemType) { + item.setType(masterItemType); // Remove await since setType handles its own state + } + })); } else { otherItems = otherItems.filter(item => item.itemTypeID === masterItemType); } } else if (typeMismatchPref === TypeMismatch.CONVERT) { - await Promise.all(mismatchedItems.map(item => item.setType(masterItemType))); + // Convert all mismatched items in place + await Promise.all(otherItems.map(async item => { + if (item.itemTypeID !== masterItemType) { + item.setType(masterItemType); // Remove await since setType handles its own state + } + })); + otherItems = otherItems.filter(item => item.itemTypeID === masterItemType); } else { // TypeMismatch.SKIP otherItems = otherItems.filter(item => item.itemTypeID === masterItemType); From 4e37d61711c406a30c701459f1aed83bfee6bf77 Mon Sep 17 00:00:00 2001 From: Mimansa Jaiswal Date: Thu, 9 Jan 2025 22:33:41 -0800 Subject: [PATCH 3/3] Refactor type conversion handling in merge process for improved clarity and efficiency --- src/modules/merger.ts | 39 +++++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 14 deletions(-) diff --git a/src/modules/merger.ts b/src/modules/merger.ts index e1951be..e59fd55 100644 --- a/src/modules/merger.ts +++ b/src/modules/merger.ts @@ -1,6 +1,15 @@ import { getPref, TypeMismatch, setPref } from "../utils/prefs"; import { getString } from "../utils/locale"; +async function convertItemType(item: Zotero.Item, targetTypeID: number) { + await Zotero.DB.executeTransaction(async () => { + item.setType(targetTypeID); + await item.save(); + // Small delay to ensure DB operations complete + await Zotero.Promise.delay(50); + }); +} + export async function merge( masterItem: Zotero.Item, otherItems: Zotero.Item[], // Already sorted @@ -18,10 +27,12 @@ export async function merge( const dialog = new ztoolkit.Dialog(3, 1) .setDialogData({ action: TypeMismatch.SKIP, - savePreference: false // This controls if we save preference permanently + savePreference: false, + // Add promise to track dialog completion + dialogPromise: Zotero.Promise.defer() }) .addCell(0, 0, { - tag: "h2", + tag: "p", properties: { innerHTML: getString("type-mismatch-message") } }) .addCell(1, 0, { @@ -46,49 +57,49 @@ export async function merge( .addButton(getString("type-mismatch-convert"), "btn_convert", { callback: () => { dialog.dialogData.action = TypeMismatch.CONVERT; - // Only save preference if user checked the box if (dialog.dialogData.savePreference) { setPref("duplicate.type.mismatch", TypeMismatch.CONVERT); } + dialog.dialogData.dialogPromise.resolve(); } }) .addButton(getString("type-mismatch-skip"), "btn_skip", { callback: () => { dialog.dialogData.action = TypeMismatch.SKIP; - // Only save preference if user checked the box if (dialog.dialogData.savePreference) { setPref("duplicate.type.mismatch", TypeMismatch.SKIP); } + dialog.dialogData.dialogPromise.resolve(); } }); - // Remove loadCallback and unloadCallback since we handle preference in button callbacks - dialog.open(getString("type-mismatch-title"), { centerscreen: true, resizable: true }); + // Wait for both dialog load and user action await dialog.dialogData.loadLock?.promise; + await dialog.dialogData.dialogPromise.promise; if (dialog.dialogData.action === TypeMismatch.CONVERT) { - // Convert all mismatched items in place - await Promise.all(otherItems.map(async item => { + // Convert items one by one + for (const item of otherItems) { if (item.itemTypeID !== masterItemType) { - item.setType(masterItemType); // Remove await since setType handles its own state + await convertItemType(item, masterItemType); } - })); + } } else { otherItems = otherItems.filter(item => item.itemTypeID === masterItemType); } } else if (typeMismatchPref === TypeMismatch.CONVERT) { - // Convert all mismatched items in place - await Promise.all(otherItems.map(async item => { + // Convert items one by one + for (const item of otherItems) { if (item.itemTypeID !== masterItemType) { - item.setType(masterItemType); // Remove await since setType handles its own state + await convertItemType(item, masterItemType); } - })); + } otherItems = otherItems.filter(item => item.itemTypeID === masterItemType); } else { // TypeMismatch.SKIP