From ee12c8d2a76be7a5206278273953b2bc0699e2e5 Mon Sep 17 00:00:00 2001 From: Travis Lesicka Date: Sun, 17 Nov 2024 13:53:46 +1000 Subject: [PATCH 01/11] Initial Commit --- .../desktop-client/src/components/Modals.tsx | 11 + .../src/components/manager/BudgetList.tsx | 45 +++- .../modals/manager/DuplicateFileModal.tsx | 255 ++++++++++++++++++ .../loot-core/src/client/actions/budgets.ts | 55 ++++ .../src/client/state-types/modals.d.ts | 31 +++ .../src/platform/server/fs/index.web.ts | 93 +++++-- packages/loot-core/src/server/main.ts | 111 +++++++- .../loot-core/src/server/util/budget-name.ts | 7 +- .../loot-core/src/types/server-handlers.d.ts | 17 ++ 9 files changed, 598 insertions(+), 27 deletions(-) create mode 100644 packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx diff --git a/packages/desktop-client/src/components/Modals.tsx b/packages/desktop-client/src/components/Modals.tsx index a52f6fef71d..58ba6f051f4 100644 --- a/packages/desktop-client/src/components/Modals.tsx +++ b/packages/desktop-client/src/components/Modals.tsx @@ -45,6 +45,7 @@ import { KeyboardShortcutModal } from './modals/KeyboardShortcutModal'; import { LoadBackupModal } from './modals/LoadBackupModal'; import { ConfirmChangeDocumentDirModal } from './modals/manager/ConfirmChangeDocumentDir'; import { DeleteFileModal } from './modals/manager/DeleteFileModal'; +import { DuplicateFileModal } from './modals/manager/DuplicateFileModal'; import { FilesSettingsModal } from './modals/manager/FilesSettingsModal'; import { ImportActualModal } from './modals/manager/ImportActualModal'; import { ImportModal } from './modals/manager/ImportModal'; @@ -586,6 +587,16 @@ export function Modals() { return ; case 'delete-budget': return ; + case 'duplicate-budget': + return ( + + ); case 'import': return ; case 'files-settings': diff --git a/packages/desktop-client/src/components/manager/BudgetList.tsx b/packages/desktop-client/src/components/manager/BudgetList.tsx index 72a3ebab0c0..3fef3015d61 100644 --- a/packages/desktop-client/src/components/manager/BudgetList.tsx +++ b/packages/desktop-client/src/components/manager/BudgetList.tsx @@ -64,9 +64,11 @@ function getFileDescription(file: File, t: (key: string) => string) { function FileMenu({ onDelete, onClose, + onDuplicate, }: { onDelete: () => void; onClose: () => void; + onDuplicate?: () => void; }) { function onMenuSelect(type: string) { onClose(); @@ -75,18 +77,30 @@ function FileMenu({ case 'delete': onDelete(); break; + case 'duplicate': + if (onDuplicate) onDuplicate(); + break; default: } } const { t } = useTranslation(); - const items = [{ name: 'delete', text: t('Delete') }]; + const items = [ + ...(onDuplicate ? [{ name: 'duplicate', text: t('Duplicate') }] : []), + { name: 'delete', text: t('Delete') }, + ]; return ; } -function FileMenuButton({ onDelete }: { onDelete: () => void }) { +function FileMenuButton({ + onDelete, + onDuplicate, +}: { + onDelete: () => void; + onDuplicate?: () => void; +}) { const triggerRef = useRef(null); const [menuOpen, setMenuOpen] = useState(false); @@ -108,7 +122,11 @@ function FileMenuButton({ onDelete }: { onDelete: () => void }) { isOpen={menuOpen} onOpenChange={() => setMenuOpen(false)} > - setMenuOpen(false)} /> + setMenuOpen(false)} + onDuplicate={onDuplicate} + /> ); @@ -169,11 +187,13 @@ function FileItem({ quickSwitchMode, onSelect, onDelete, + onDuplicate, }: { file: File; quickSwitchMode: boolean; onSelect: (file: File) => void; onDelete: (file: File) => void; + onDuplicate: (file: File) => void; }) { const { t } = useTranslation(); @@ -239,7 +259,10 @@ function FileItem({ )} {!quickSwitchMode && ( - onDelete(file)} /> + onDelete(file)} + onDuplicate={'id' in file ? () => onDuplicate(file) : undefined} + /> )} @@ -252,11 +275,13 @@ function BudgetFiles({ quickSwitchMode, onSelect, onDelete, + onDuplicate, }: { files: File[]; quickSwitchMode: boolean; onSelect: (file: File) => void; onDelete: (file: File) => void; + onDuplicate: (file: File) => void; }) { function isLocalFile(file: File): file is LocalFile { return file.state === 'local'; @@ -292,6 +317,7 @@ function BudgetFiles({ quickSwitchMode={quickSwitchMode} onSelect={onSelect} onDelete={onDelete} + onDuplicate={onDuplicate} /> )) )} @@ -467,7 +493,16 @@ export function BudgetList({ showHeader = true, quickSwitchMode = false }) { files={files} quickSwitchMode={quickSwitchMode} onSelect={onSelect} - onDelete={file => dispatch(pushModal('delete-budget', { file }))} + onDelete={(file: File) => + dispatch(pushModal('delete-budget', { file })) + } + onDuplicate={(file: File) => { + if (file && 'id' in file) { + dispatch(pushModal('duplicate-budget', { file, managePage: true })); + } else { + console.error('Attempted to duplicate an invalid file:', file); + } + }} /> {!quickSwitchMode && ( void; +}; + +export function DuplicateFileModal({ + file, + managePage, + loadBudget = 'none', + onComplete, +}: DuplicateFileProps) { + const { t } = useTranslation(); + const [newName, setNewName] = useState(file.name + ' - copy'); + const [nameError, setNameError] = useState(null); + + // If the state is "broken" that means it was created by another user. + const isCloudFile = 'cloudFileId' in file && file.state !== 'broken'; + const dispatch = useDispatch(); + + const [loadingState, setLoadingState] = useState<'cloud' | 'local' | null>( + null, + ); + + const validateNewName = (name: string): string | null => { + const trimmedName = name.trim(); + if (trimmedName === '') return t('Budget name cannot be blank'); + if (trimmedName.length > 100) { + return t('Budget name is too long (max length 100)'); + } + if (!/^[a-zA-Z0-9 .\-_()]+$/.test(trimmedName)) { + return t('Budget name contains invalid characters'); + } + // Additional validation checks can go here + + return null; + }; + + const validateAndSetName = (name: string) => { + const trimmedName = name.trim(); + const error = validateNewName(trimmedName); + if (error) { + setNameError(error); + } else { + setNewName(trimmedName); + setNameError(null); + } + }; + + const handleDuplicate = async (sync: 'localOnly' | 'cloudSync') => { + const error = validateNewName(newName); + if (!error) { + setLoadingState(sync === 'cloudSync' ? 'cloud' : 'local'); + + try { + await dispatch( + duplicateBudget({ + id: 'id' in file ? file.id : undefined, + cloudId: + sync === 'cloudSync' && 'cloudFileId' in file + ? file.cloudFileId + : undefined, + oldName: file.name, + newName, + cloudSync: sync === 'cloudSync', + managePage, + loadBudget, + }), + ); + dispatch( + addNotification({ + type: 'message', + message: t('Duplicate file “{{newName}}” created.', { newName }), + }), + ); + if (onComplete) onComplete({ status: 'success' }); + } catch (e) { + const newError = new Error(t('Failed to duplicate budget')); + if (onComplete) onComplete({ status: 'failed', error: newError }); + else console.error('Failed to duplicate budget:', e); + dispatch( + addNotification({ + type: 'error', + message: t('Failed to duplicate budget file.'), + }), + ); + } finally { + setLoadingState(null); + } + } else { + const failError = new Error(error); + if (onComplete) onComplete({ status: 'failed', error: failError }); + } + }; + + return ( + + {({ state: { close } }) => ( + <> + { + close(); + if (onComplete) onComplete({ status: 'canceled' }); + }} + /> + } + /> + + + + setNewName(event.target.value)} + onBlur={event => validateAndSetName(event.target.value)} + style={{ flex: 1 }} + /> + + + {nameError && ( + + {nameError} + + )} + + {isCloudFile && ( + <> + + + Current budget is a hosted budget which + means it is stored on your server to make it available for + download on any device. Would you like to duplicate this + budget for all devices? + + + + handleDuplicate('cloudSync')} + > + Duplicate budget for all devices + + + )} + + {'id' in file && ( + <> + {isCloudFile ? ( + + + You can also duplicate to just the local copy. This will + leave the original on the server and create a duplicate on + only this device. + + + ) : ( + + + This is a local budget which is not + stored on a server. Only a local copy will be duplicated. + + + )} + + + + handleDuplicate('localOnly')} + > + {!isCloudFile && Duplicate budget} + {isCloudFile && ( + Duplicate budget locally only + )} + + + + )} + + + )} + + ); +} diff --git a/packages/loot-core/src/client/actions/budgets.ts b/packages/loot-core/src/client/actions/budgets.ts index 96dc92434a2..ed6bb7d1dbc 100644 --- a/packages/loot-core/src/client/actions/budgets.ts +++ b/packages/loot-core/src/client/actions/budgets.ts @@ -148,6 +148,61 @@ export function createBudget({ testMode = false, demoMode = false } = {}) { }; } +export function duplicateBudget({ + id, + cloudId, + oldName, + newName, + managePage, + loadBudget = 'none', + cloudSync, +}: { + id?: string; + cloudId?: string; + oldName: string; + newName: string; + managePage?: boolean; + loadBudget: 'none' | 'original' | 'copy'; + /** + * cloudSync is used to determine if the duplicate budget + * should be synced to the server + */ + cloudSync?: boolean; +}) { + return async (dispatch: Dispatch) => { + try { + dispatch( + setAppState({ + loadingText: t('Duplicating: {{oldName}} -- to: {{newName}}', { + oldName, + newName, + }), + }), + ); + + await send('duplicate-budget', { + id, + cloudId, + newName, + cloudSync, + open: loadBudget, + }); + + dispatch(closeModal()); + + if (managePage) { + await dispatch(loadAllFiles()); + } + } catch (error) { + console.error('Error duplicating budget:', error); + dispatch(setAppState({ loadingText: null })); + throw new Error('Error duplicating budget:'); + } finally { + dispatch(setAppState({ loadingText: null })); + } + }; +} + export function importBudget( filepath: string, type: Parameters[0]['type'], diff --git a/packages/loot-core/src/client/state-types/modals.d.ts b/packages/loot-core/src/client/state-types/modals.d.ts index 11450cf4775..9a415988fcf 100644 --- a/packages/loot-core/src/client/state-types/modals.d.ts +++ b/packages/loot-core/src/client/state-types/modals.d.ts @@ -78,6 +78,37 @@ type FinanceModals = { 'delete-budget': { file: File }; + 'duplicate-budget': { + /** The budget file to be duplicated */ + file: File; + /** + * Indicates whether the duplication is initiated from the budget + * management page. This may affect the behavior or UI of the + * duplication process. + */ + managePage?: boolean; + /** + * loadBudget indicates whether to open the 'original' budget, the + * new duplicated 'copy' budget, or no budget ('none'). If 'none' + * duplicate-budget stays on the same page. + */ + loadBudget?: 'none' | 'original' | 'copy'; + /** + * onComplete is called when the DuplicateFileModal is closed. + * @param event the event object will pass back the status of the + * duplicate process. + * 'success' if the budget was duplicated. + * 'failed' if the budget could not be duplicated. This will also + * pass an error on the event object. + * 'canceled' if the DuplicateFileModal was canceled. + * @returns + */ + onComplete?: (event: { + status: 'success' | 'failed' | 'canceled'; + error?: Error; + }) => void; + }; + import: null; 'import-ynab4': null; diff --git a/packages/loot-core/src/platform/server/fs/index.web.ts b/packages/loot-core/src/platform/server/fs/index.web.ts index 06eebb13ade..0d299888528 100644 --- a/packages/loot-core/src/platform/server/fs/index.web.ts +++ b/packages/loot-core/src/platform/server/fs/index.web.ts @@ -19,11 +19,11 @@ export { join }; export { getDocumentDir, getBudgetDir, _setDocumentDir } from './shared'; export const getDataDir = () => process.env.ACTUAL_DATA_DIR; -export const pathToId = function (filepath) { +export const pathToId = function (filepath: string): string { return filepath.replace(/^\//, '').replace(/\//g, '-'); }; -function _exists(filepath) { +function _exists(filepath: string): boolean { try { FS.readlink(filepath); return true; @@ -47,7 +47,7 @@ function _mkdirRecursively(dir) { } } -function _createFile(filepath) { +function _createFile(filepath: string) { // This can create the file. Check if it exists, if not create a // symlink if it's a sqlite file. Otherwise store in idb @@ -67,7 +67,7 @@ function _createFile(filepath) { return filepath; } -async function _readFile(filepath, opts?: { encoding?: string }) { +async function _readFile(filepath: string, opts?: { encoding?: string }) { // We persist stuff in /documents, but don't need to handle sqlite // file specifically because those are symlinked to a separate // filesystem and will be handled in the BlockedFS @@ -88,7 +88,7 @@ async function _readFile(filepath, opts?: { encoding?: string }) { throw new Error('File does not exist: ' + filepath); } - if (opts.encoding === 'utf8' && ArrayBuffer.isView(item.contents)) { + if (opts?.encoding === 'utf8' && ArrayBuffer.isView(item.contents)) { return String.fromCharCode.apply( null, new Uint16Array(item.contents.buffer), @@ -101,7 +101,7 @@ async function _readFile(filepath, opts?: { encoding?: string }) { } } -function resolveLink(path) { +function resolveLink(path: string): string { try { const { node } = FS.lookupPath(path, { follow: false }); return node.link ? FS.readlink(path) : path; @@ -110,7 +110,7 @@ function resolveLink(path) { } } -async function _writeFile(filepath, contents) { +async function _writeFile(filepath: string, contents): Promise { if (contents instanceof ArrayBuffer) { contents = new Uint8Array(contents); } else if (ArrayBuffer.isView(contents)) { @@ -146,9 +146,53 @@ async function _writeFile(filepath, contents) { } else { FS.writeFile(resolveLink(filepath), contents); } + return true; } -async function _removeFile(filepath) { +async function _copySqlFile( + frompath: string, + topath: string, +): Promise { + _createFile(topath); + + const { store } = await idb.getStore(await idb.getDatabase(), 'files'); + await idb.set(store, { filepath: topath, contents: '' }); + const fromitem = await idb.get(store, frompath); + const fromDbPath = pathToId(fromitem.filepath); + const toDbPath = pathToId(topath); + + const fromfile = BFS.backend.createFile(fromDbPath); + const tofile = BFS.backend.createFile(toDbPath); + + try { + fromfile.open(); + tofile.open(); + const fileSize = fromfile.meta.size; + const blockSize = fromfile.meta.blockSize; + + const buffer = new ArrayBuffer(blockSize); + const bufferView = new Uint8Array(buffer); + + for (let i = 0; i < fileSize; i += blockSize) { + const bytesToRead = Math.min(blockSize, fileSize - i); + fromfile.read(bufferView, 0, bytesToRead, i); + tofile.write(bufferView, 0, bytesToRead, i); + } + } catch (error) { + tofile.close(); + fromfile.close(); + _removeFile(toDbPath); + console.error('Failed to copy database file', error); + return false; + } finally { + tofile.close(); + fromfile.close(); + } + + return true; +} + +async function _removeFile(filepath: string) { if (!NO_PERSIST && filepath.startsWith('/documents')) { const isDb = filepath.endsWith('.sqlite'); @@ -272,22 +316,39 @@ export const size = async function (filepath) { return attrs.size; }; -export const copyFile = async function (frompath, topath) { - // TODO: This reads the whole file into memory, but that's probably - // not a problem. This could be optimized - const contents = await _readFile(frompath); - return _writeFile(topath, contents); +export const copyFile = async function ( + frompath: string, + topath: string, +): Promise { + let result = false; + try { + const contents = await _readFile(frompath); + result = await _writeFile(topath, contents); + } catch (error) { + if (frompath.endsWith('.sqlite') || topath.endsWith('.sqlite')) { + try { + result = await _copySqlFile(frompath, topath); + } catch (secondError) { + throw new Error( + `Failed to copy SQL file from ${frompath} to ${topath}: ${secondError.message}`, + ); + } + } else { + throw error; + } + } + return result; }; -export const readFile = async function (filepath, encoding = 'utf8') { +export const readFile = async function (filepath: string, encoding = 'utf8') { return _readFile(filepath, { encoding }); }; -export const writeFile = async function (filepath, contents) { +export const writeFile = async function (filepath: string, contents) { return _writeFile(filepath, contents); }; -export const removeFile = async function (filepath) { +export const removeFile = async function (filepath: string) { return _removeFile(filepath); }; diff --git a/packages/loot-core/src/server/main.ts b/packages/loot-core/src/server/main.ts index d2801148e4b..24e8cdf06f0 100644 --- a/packages/loot-core/src/server/main.ts +++ b/packages/loot-core/src/server/main.ts @@ -1870,7 +1870,7 @@ handlers['close-budget'] = async function () { } prefs.unloadPrefs(); - stopBackupService(); + await stopBackupService(); return 'ok'; }; @@ -1883,6 +1883,12 @@ handlers['delete-budget'] = async function ({ id, cloudFileId }) { // If a local file exists, you can delete it by passing its local id if (id) { + // loading and then closing the budget is a hack to be able to delete + // the budget file if it hasn't been opened yet. This needs a better + // way, but works for now. + await loadBudget(id); + await handlers['close-budget'](); + const budgetDir = fs.getBudgetDir(id); await fs.removeDirRecursively(budgetDir); } @@ -1890,6 +1896,103 @@ handlers['delete-budget'] = async function ({ id, cloudFileId }) { return 'ok'; }; +handlers['duplicate-budget'] = async function ({ + id, + newName, + cloudSync, + open, +}): Promise { + if (!id) throw new Error('Unable to duplicate a budget that is not local.'); + if (!newName?.trim()) { + throw new Error('Budget name is required and cannot be empty'); + } + if (!/^[a-zA-Z0-9 .\-_()]+$/.test(newName)) { + throw new Error('Budget name contains invalid characters'); + } + + const budgetDir = fs.getBudgetDir(id); + + let budgetName = newName; + let sameName = false; + + if (budgetName.indexOf(' - copy') !== -1) { + sameName = true; + budgetName = budgetName.replace(' - copy', ''); + } + + const newId = await idFromFileName(budgetName); + + const budgets = await handlers['get-budgets'](); + budgetName = await uniqueFileName( + budgets, + sameName ? budgetName + ' - copy' : budgetName, + ); + + // copy metadata from current budget + // replace id with new budget id and budgetName with new budget name + const metadataText = await fs.readFile(fs.join(budgetDir, 'metadata.json')); + const metadata = JSON.parse(metadataText); + metadata.id = newId; + metadata.budgetName = budgetName; + [ + 'cloudFileId', + 'groupId', + 'lastUploaded', + 'encryptKeyId', + 'lastSyncedTimestamp', + ].forEach(item => { + if (metadata[item]) delete metadata[item]; + }); + + try { + const newBudgetDir = fs.getBudgetDir(newId); + await fs.mkdir(newBudgetDir); + + // write metadata for new budget + await fs.writeFile( + fs.join(newBudgetDir, 'metadata.json'), + JSON.stringify(metadata), + ); + + await fs.copyFile( + fs.join(budgetDir, 'db.sqlite'), + fs.join(newBudgetDir, 'db.sqlite'), + ); + } catch (error) { + // Clean up any partially created files + try { + const newBudgetDir = fs.getBudgetDir(newId); + if (await fs.exists(newBudgetDir)) { + await fs.removeDirRecursively(newBudgetDir); + } + } catch {} // Ignore cleanup errors + throw new Error(`Failed to duplicate budget: ${error.message}`); + } + + // load in and validate + const { error } = await loadBudget(newId); + if (error) { + console.log('Error duplicating budget: ' + error); + return error; + } + + if (cloudSync) { + try { + await cloudStorage.upload(); + } catch (error) { + console.warn('Failed to sync duplicated budget to cloud:', error); + // Ignore any errors uploading. If they are offline they should + // still be able to create files. + } + } + + handlers['close-budget'](); + if (open === 'original') await loadBudget(id); + if (open === 'copy') await loadBudget(newId); + + return newId; +}; + handlers['create-budget'] = async function ({ budgetName, avoidUpload, @@ -1984,8 +2087,8 @@ handlers['export-budget'] = async function () { } }; -async function loadBudget(id) { - let dir; +async function loadBudget(id: string) { + let dir: string; try { dir = fs.getBudgetDir(id); } catch (e) { @@ -2062,7 +2165,7 @@ async function loadBudget(id) { !Platform.isMobile && process.env.NODE_ENV !== 'test' ) { - startBackupService(id); + await startBackupService(id); } try { diff --git a/packages/loot-core/src/server/util/budget-name.ts b/packages/loot-core/src/server/util/budget-name.ts index 3c94888f0da..87157717a14 100644 --- a/packages/loot-core/src/server/util/budget-name.ts +++ b/packages/loot-core/src/server/util/budget-name.ts @@ -2,9 +2,12 @@ import { v4 as uuidv4 } from 'uuid'; import * as fs from '../../platform/server/fs'; +import { type Budget } from '../../types/budget'; -export async function uniqueFileName(existingFiles) { - const initialName = 'My Finances'; +export async function uniqueFileName( + existingFiles: Budget[], + initialName: string = 'My Finances', +): Promise { let idx = 1; // If there is a conflict, keep appending an index until there is no diff --git a/packages/loot-core/src/types/server-handlers.d.ts b/packages/loot-core/src/types/server-handlers.d.ts index 338b5e2f3f1..173b23a4f1a 100644 --- a/packages/loot-core/src/types/server-handlers.d.ts +++ b/packages/loot-core/src/types/server-handlers.d.ts @@ -326,6 +326,23 @@ export interface ServerHandlers { cloudFileId?: string; }) => Promise<'ok'>; + /** + * Duplicates a budget file. + * @param {Object} arg - The arguments for duplicating a budget. + * @param {string} [arg.id] - The ID of the local budget to duplicate. + * @param {string} [arg.cloudId] - The ID of the cloud-synced budget to duplicate. + * @param {string} arg.newName - The name for the duplicated budget. + * @param {boolean} [arg.cloudSync] - Whether to sync the duplicated budget to the cloud. + * @returns {Promise} The ID of the newly created budget. + */ + 'duplicate-budget': (arg: { + id?: string; + cloudId?: string; + newName: string; + cloudSync?: boolean; + open: 'none' | 'original' | 'copy'; + }) => Promise; + 'create-budget': (arg: { budgetName?; avoidUpload?; From 02cc76c3c068b3d22434ebb9f23e614b2155476d Mon Sep 17 00:00:00 2001 From: Travis Lesicka Date: Sun, 17 Nov 2024 13:58:30 +1000 Subject: [PATCH 02/11] Create 3847.md --- upcoming-release-notes/3847.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 upcoming-release-notes/3847.md diff --git a/upcoming-release-notes/3847.md b/upcoming-release-notes/3847.md new file mode 100644 index 00000000000..785e81f7abe --- /dev/null +++ b/upcoming-release-notes/3847.md @@ -0,0 +1,6 @@ +--- +category: Features +authors: [tlesicka] +--- + +Added ability to duplicate budgets. From 89a9907082112209d66c3986046dd7d54d030a41 Mon Sep 17 00:00:00 2001 From: Travis Lesicka Date: Sun, 17 Nov 2024 21:47:54 +1000 Subject: [PATCH 03/11] Removed un-needed comment --- .../src/components/modals/manager/DuplicateFileModal.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx b/packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx index 0d2f82b184d..1d69f7d6347 100644 --- a/packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx +++ b/packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx @@ -57,7 +57,6 @@ export function DuplicateFileModal({ if (!/^[a-zA-Z0-9 .\-_()]+$/.test(trimmedName)) { return t('Budget name contains invalid characters'); } - // Additional validation checks can go here return null; }; From 882c14d58db1bd059bf3daa5c98379f680d6870a Mon Sep 17 00:00:00 2001 From: Travis Lesicka Date: Sun, 17 Nov 2024 21:48:12 +1000 Subject: [PATCH 04/11] Changed error log text --- .../desktop-client/src/components/manager/BudgetList.tsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/desktop-client/src/components/manager/BudgetList.tsx b/packages/desktop-client/src/components/manager/BudgetList.tsx index 3fef3015d61..12defd2e4dd 100644 --- a/packages/desktop-client/src/components/manager/BudgetList.tsx +++ b/packages/desktop-client/src/components/manager/BudgetList.tsx @@ -500,7 +500,10 @@ export function BudgetList({ showHeader = true, quickSwitchMode = false }) { if (file && 'id' in file) { dispatch(pushModal('duplicate-budget', { file, managePage: true })); } else { - console.error('Attempted to duplicate an invalid file:', file); + console.error( + 'Attempted to duplicate a cloud file - only local files are supported. Cloud file:', + file, + ); } }} /> From d5bba7aa8dd637d5642ad2469c9c03ab4c4568b2 Mon Sep 17 00:00:00 2001 From: Travis Lesicka Date: Tue, 19 Nov 2024 22:23:10 +1000 Subject: [PATCH 05/11] Moved budget name validation from DuplicateFileModal to loot-core/server --- .../modals/manager/DuplicateFileModal.tsx | 45 +++++++++-------- .../loot-core/src/client/actions/budgets.ts | 16 +++++- packages/loot-core/src/server/main.ts | 50 +++++++++---------- .../loot-core/src/server/util/budget-name.ts | 29 ++++++++--- .../loot-core/src/types/server-handlers.d.ts | 6 +++ 5 files changed, 89 insertions(+), 57 deletions(-) diff --git a/packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx b/packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx index 1d69f7d6347..d59a128d731 100644 --- a/packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx +++ b/packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx @@ -1,8 +1,13 @@ -import React, { useState } from 'react'; +import React, { useEffect, useState } from 'react'; import { Trans, useTranslation } from 'react-i18next'; import { useDispatch } from 'react-redux'; -import { addNotification, duplicateBudget } from 'loot-core/client/actions'; +import { + addNotification, + duplicateBudget, + uniqueBudgetName, + validateBudgetName, +} from 'loot-core/client/actions'; import { type File } from 'loot-core/src/types/file'; import { theme } from '../../../style'; @@ -48,33 +53,27 @@ export function DuplicateFileModal({ null, ); - const validateNewName = (name: string): string | null => { - const trimmedName = name.trim(); - if (trimmedName === '') return t('Budget name cannot be blank'); - if (trimmedName.length > 100) { - return t('Budget name is too long (max length 100)'); - } - if (!/^[a-zA-Z0-9 .\-_()]+$/.test(trimmedName)) { - return t('Budget name contains invalid characters'); - } - - return null; - }; + useEffect(() => { + (async () => { + setNewName(await uniqueBudgetName(file.name + ' - copy')); + })(); + }, [file.name]); - const validateAndSetName = (name: string) => { + const validateAndSetName = async (name: string) => { const trimmedName = name.trim(); - const error = validateNewName(trimmedName); - if (error) { - setNameError(error); - } else { + const { valid, message } = await validateBudgetName(trimmedName); + if (valid) { setNewName(trimmedName); setNameError(null); + } else { + // The "Unknown error" should never happen, but this satifies type checking + setNameError(message ?? t('Unknown error with budget name')); } }; const handleDuplicate = async (sync: 'localOnly' | 'cloudSync') => { - const error = validateNewName(newName); - if (!error) { + const { valid, message } = await validateBudgetName(newName); + if (valid) { setLoadingState(sync === 'cloudSync' ? 'cloud' : 'local'); try { @@ -113,7 +112,9 @@ export function DuplicateFileModal({ setLoadingState(null); } } else { - const failError = new Error(error); + const failError = new Error( + message ?? t('Unknown error with budget name'), + ); if (onComplete) onComplete({ status: 'failed', error: failError }); } }; diff --git a/packages/loot-core/src/client/actions/budgets.ts b/packages/loot-core/src/client/actions/budgets.ts index ed6bb7d1dbc..6c21eff0b09 100644 --- a/packages/loot-core/src/client/actions/budgets.ts +++ b/packages/loot-core/src/client/actions/budgets.ts @@ -148,6 +148,17 @@ export function createBudget({ testMode = false, demoMode = false } = {}) { }; } +export function validateBudgetName(name: string): { + valid: boolean; + message?: string; +} { + return send('validate-budget-name', { name }); +} + +export function uniqueBudgetName(name: string): string { + return send('unique-budget-name', { name }); +} + export function duplicateBudget({ id, cloudId, @@ -195,8 +206,9 @@ export function duplicateBudget({ } } catch (error) { console.error('Error duplicating budget:', error); - dispatch(setAppState({ loadingText: null })); - throw new Error('Error duplicating budget:'); + throw error instanceof Error + ? error + : new Error('Error duplicating budget: ' + String(error)); } finally { dispatch(setAppState({ loadingText: null })); } diff --git a/packages/loot-core/src/server/main.ts b/packages/loot-core/src/server/main.ts index 2b675cfb14f..8d765d27110 100644 --- a/packages/loot-core/src/server/main.ts +++ b/packages/loot-core/src/server/main.ts @@ -73,7 +73,11 @@ import * as syncMigrations from './sync/migrate'; import { app as toolsApp } from './tools/app'; import { withUndo, clearUndo, undo, redo } from './undo'; import { updateVersion } from './update'; -import { uniqueFileName, idFromFileName } from './util/budget-name'; +import { + uniqueBudgetName, + idFromBudgetName, + validateBudgetName, +} from './util/budget-name'; const DEMO_BUDGET_ID = '_demo-budget'; const TEST_BUDGET_ID = '_test-budget'; @@ -1710,6 +1714,14 @@ handlers['sync'] = async function () { return fullSync(); }; +handlers['validate-budget-name'] = async function ({ name }) { + return validateBudgetName(name); +}; + +handlers['unique-budget-name'] = async function ({ name }) { + return uniqueBudgetName(name); +}; + handlers['get-budgets'] = async function () { const paths = await fs.listDir(fs.getDocumentDir()); const budgets = ( @@ -1912,37 +1924,24 @@ handlers['duplicate-budget'] = async function ({ open, }): Promise { if (!id) throw new Error('Unable to duplicate a budget that is not local.'); - if (!newName?.trim()) { - throw new Error('Budget name is required and cannot be empty'); - } - if (!/^[a-zA-Z0-9 .\-_()]+$/.test(newName)) { - throw new Error('Budget name contains invalid characters'); - } - const budgetDir = fs.getBudgetDir(id); + const { valid, message } = await validateBudgetName(newName); + if (!valid) throw new Error(message); - let budgetName = newName; - let sameName = false; + const budgetDir = fs.getBudgetDir(id); - if (budgetName.indexOf(' - copy') !== -1) { - sameName = true; - budgetName = budgetName.replace(' - copy', ''); + let budgetFileName = newName; + if (budgetFileName.indexOf(' - copy') !== -1) { + budgetFileName = budgetFileName.replace(/\s-\scopy.*/, ''); } - - const newId = await idFromFileName(budgetName); - - const budgets = await handlers['get-budgets'](); - budgetName = await uniqueFileName( - budgets, - sameName ? budgetName + ' - copy' : budgetName, - ); + const newId = await idFromBudgetName(budgetFileName); // copy metadata from current budget // replace id with new budget id and budgetName with new budget name const metadataText = await fs.readFile(fs.join(budgetDir, 'metadata.json')); const metadata = JSON.parse(metadataText); metadata.id = newId; - metadata.budgetName = budgetName; + metadata.budgetName = newName; [ 'cloudFileId', 'groupId', @@ -2024,13 +2023,10 @@ handlers['create-budget'] = async function ({ } else { // Generate budget name if not given if (!budgetName) { - // Unfortunately we need to load all of the existing files first - // so we can detect conflicting names. - const files = await handlers['get-budgets'](); - budgetName = await uniqueFileName(files); + budgetName = await uniqueBudgetName(); } - id = await idFromFileName(budgetName); + id = await idFromBudgetName(budgetName); } const budgetDir = fs.getBudgetDir(id); diff --git a/packages/loot-core/src/server/util/budget-name.ts b/packages/loot-core/src/server/util/budget-name.ts index 87157717a14..dfe492e5c51 100644 --- a/packages/loot-core/src/server/util/budget-name.ts +++ b/packages/loot-core/src/server/util/budget-name.ts @@ -1,19 +1,18 @@ -// @ts-strict-ignore import { v4 as uuidv4 } from 'uuid'; import * as fs from '../../platform/server/fs'; -import { type Budget } from '../../types/budget'; +import { handlers } from '../main'; -export async function uniqueFileName( - existingFiles: Budget[], +export async function uniqueBudgetName( initialName: string = 'My Finances', ): Promise { + const budgets = await handlers['get-budgets'](); let idx = 1; // If there is a conflict, keep appending an index until there is no // conflict and we have a unique name let newName = initialName; - while (existingFiles.find(file => file.name === newName)) { + while (budgets.find(file => file.name === newName)) { newName = `${initialName} ${idx}`; idx++; } @@ -21,7 +20,25 @@ export async function uniqueFileName( return newName; } -export async function idFromFileName(name) { +export async function validateBudgetName( + name: string, +): Promise<{ valid: boolean; message?: string }> { + const trimmedName = name.trim(); + const uniqueName = await uniqueBudgetName(trimmedName); + let message: string | null = null; + + if (trimmedName === '') message = 'Budget name cannot be blank'; + if (trimmedName.length > 100) { + message = 'Budget name is too long (max length 100)'; + } + if (uniqueName !== trimmedName) { + message = `“${name}” already exists, try “${uniqueName}” instead`; + } + + return message ? { valid: false, message } : { valid: true }; +} + +export async function idFromBudgetName(name: string): Promise { let id = name.replace(/( |[^A-Za-z0-9])/g, '-') + '-' + uuidv4().slice(0, 7); // Make sure the id is unique. There's a chance one could already diff --git a/packages/loot-core/src/types/server-handlers.d.ts b/packages/loot-core/src/types/server-handlers.d.ts index 3b97143ea12..47f8f2c7a8c 100644 --- a/packages/loot-core/src/types/server-handlers.d.ts +++ b/packages/loot-core/src/types/server-handlers.d.ts @@ -303,6 +303,12 @@ export interface ServerHandlers { | { messages: Message[] } >; + 'validate-budget-name': (arg: { + name: string; + }) => Promise<{ valid: boolean; message?: string }>; + + 'unique-budget-name': (arg: { name: string }) => Promise; + 'get-budgets': () => Promise; 'get-remote-files': () => Promise; From a87570481a54dd97640040d3e6d077d44266613b Mon Sep 17 00:00:00 2001 From: Travis Lesicka Date: Sat, 7 Dec 2024 21:49:49 +1000 Subject: [PATCH 06/11] Added translation --- .../src/components/modals/manager/DuplicateFileModal.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx b/packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx index d59a128d731..b4a3063b1e2 100644 --- a/packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx +++ b/packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx @@ -42,7 +42,7 @@ export function DuplicateFileModal({ onComplete, }: DuplicateFileProps) { const { t } = useTranslation(); - const [newName, setNewName] = useState(file.name + ' - copy'); + const [newName, setNewName] = useState(file.name + t(' - copy')); const [nameError, setNameError] = useState(null); // If the state is "broken" that means it was created by another user. @@ -55,7 +55,7 @@ export function DuplicateFileModal({ useEffect(() => { (async () => { - setNewName(await uniqueBudgetName(file.name + ' - copy')); + setNewName(await uniqueBudgetName(file.name + t(' - copy'))); })(); }, [file.name]); From 0fb0bf7a965bad52f78bf978ae9f6e3b10fac370 Mon Sep 17 00:00:00 2001 From: Travis Lesicka Date: Sat, 7 Dec 2024 22:20:43 +1000 Subject: [PATCH 07/11] Fixed linting error --- .../src/components/modals/manager/DuplicateFileModal.tsx | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx b/packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx index b4a3063b1e2..b96562e10ee 100644 --- a/packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx +++ b/packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx @@ -42,7 +42,8 @@ export function DuplicateFileModal({ onComplete, }: DuplicateFileProps) { const { t } = useTranslation(); - const [newName, setNewName] = useState(file.name + t(' - copy')); + const fileEndingTranslation = t(' - copy'); + const [newName, setNewName] = useState(file.name + fileEndingTranslation); const [nameError, setNameError] = useState(null); // If the state is "broken" that means it was created by another user. @@ -55,9 +56,9 @@ export function DuplicateFileModal({ useEffect(() => { (async () => { - setNewName(await uniqueBudgetName(file.name + t(' - copy'))); + setNewName(await uniqueBudgetName(file.name + fileEndingTranslation)); })(); - }, [file.name]); + }, [file.name, fileEndingTranslation]); const validateAndSetName = async (name: string) => { const trimmedName = name.trim(); From 49697a0e4862805d28ce0bdbbf1a41fc9a841bdb Mon Sep 17 00:00:00 2001 From: Travis Lesicka Date: Sat, 7 Dec 2024 22:40:35 +1000 Subject: [PATCH 08/11] Changed delete file hack Changed from loading and closing the budget file to just opening and closing the database to be able to delete it. --- packages/loot-core/src/server/main.ts | 15 +++++++++------ packages/loot-core/src/types/server-handlers.d.ts | 2 +- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/packages/loot-core/src/server/main.ts b/packages/loot-core/src/server/main.ts index 8d765d27110..deb18cc5e47 100644 --- a/packages/loot-core/src/server/main.ts +++ b/packages/loot-core/src/server/main.ts @@ -1904,14 +1904,17 @@ handlers['delete-budget'] = async function ({ id, cloudFileId }) { // If a local file exists, you can delete it by passing its local id if (id) { - // loading and then closing the budget is a hack to be able to delete + // opening and then closing the database is a hack to be able to delete // the budget file if it hasn't been opened yet. This needs a better // way, but works for now. - await loadBudget(id); - await handlers['close-budget'](); - - const budgetDir = fs.getBudgetDir(id); - await fs.removeDirRecursively(budgetDir); + try { + await db.openDatabase(id); + await db.closeDatabase(); + const budgetDir = fs.getBudgetDir(id); + await fs.removeDirRecursively(budgetDir); + } catch (e) { + return 'fail'; + } } return 'ok'; diff --git a/packages/loot-core/src/types/server-handlers.d.ts b/packages/loot-core/src/types/server-handlers.d.ts index 84a2d2f75c3..e4f2bac30cd 100644 --- a/packages/loot-core/src/types/server-handlers.d.ts +++ b/packages/loot-core/src/types/server-handlers.d.ts @@ -333,7 +333,7 @@ export interface ServerHandlers { 'delete-budget': (arg: { id?: string; cloudFileId?: string; - }) => Promise<'ok'>; + }) => Promise<'ok' | 'fail'>; /** * Duplicates a budget file. From 968a437cc84ecde8cf7b28d50ee83467514012d0 Mon Sep 17 00:00:00 2001 From: Travis Lesicka Date: Sun, 8 Dec 2024 11:54:57 +1000 Subject: [PATCH 09/11] Removed hard coded english from loot-core server --- packages/loot-core/src/server/main.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/packages/loot-core/src/server/main.ts b/packages/loot-core/src/server/main.ts index deb18cc5e47..24fb626f487 100644 --- a/packages/loot-core/src/server/main.ts +++ b/packages/loot-core/src/server/main.ts @@ -1933,11 +1933,7 @@ handlers['duplicate-budget'] = async function ({ const budgetDir = fs.getBudgetDir(id); - let budgetFileName = newName; - if (budgetFileName.indexOf(' - copy') !== -1) { - budgetFileName = budgetFileName.replace(/\s-\scopy.*/, ''); - } - const newId = await idFromBudgetName(budgetFileName); + const newId = await idFromBudgetName(newName); // copy metadata from current budget // replace id with new budget id and budgetName with new budget name From b9419082f9c49475c90646523745a097b921a833 Mon Sep 17 00:00:00 2001 From: Travis Lesicka Date: Mon, 9 Dec 2024 09:12:25 +1000 Subject: [PATCH 10/11] Updated wording and style of Duplicate File Modal --- .../modals/manager/DuplicateFileModal.tsx | 133 ++++++++---------- 1 file changed, 62 insertions(+), 71 deletions(-) diff --git a/packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx b/packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx index b96562e10ee..3e7866a1e82 100644 --- a/packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx +++ b/packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx @@ -48,6 +48,7 @@ export function DuplicateFileModal({ // If the state is "broken" that means it was created by another user. const isCloudFile = 'cloudFileId' in file && file.state !== 'broken'; + const isLocalFile = 'id' in file; const dispatch = useDispatch(); const [loadingState, setLoadingState] = useState<'cloud' | 'local' | null>( @@ -123,7 +124,7 @@ export function DuplicateFileModal({ return ( {({ state: { close } }) => ( - <> + } /> + @@ -168,88 +169,78 @@ export function DuplicateFileModal({ )} - {isCloudFile && ( - <> + {isLocalFile ? ( + isCloudFile ? ( - Current budget is a hosted budget which - means it is stored on your server to make it available for - download on any device. Would you like to duplicate this - budget for all devices? + Your budget is hosted on a server, making it accessible for + download on your devices. +
+ Would you like to duplicate this budget for all your devices + or keep it stored locally on this device? - + ) : ( + + + Your budget is only located on your device. +
+ Only a local copy will be duplicated. + + + ) + ) : ( + + + Unable to duplicate a budget that is not located on your + device. +
+ Please download the budget from the server before duplicating. + + + )} + + + {isLocalFile && isCloudFile && ( handleDuplicate('cloudSync')} > - Duplicate budget for all devices + Duplicate for all devices - - )} - - {'id' in file && ( - <> - {isCloudFile ? ( - - - You can also duplicate to just the local copy. This will - leave the original on the server and create a duplicate on - only this device. - - - ) : ( - - - This is a local budget which is not - stored on a server. Only a local copy will be duplicated. - - - )} - - - - handleDuplicate('localOnly')} - > - {!isCloudFile && Duplicate budget} - {isCloudFile && ( - Duplicate budget locally only - )} - - - - )} + )} + {isLocalFile && ( + handleDuplicate('localOnly')} + > + Duplicate locally + + )} + - + )} ); From a39aa2912353537d55473ecbb77f37bfe0a2f04a Mon Sep 17 00:00:00 2001 From: Travis Lesicka Date: Mon, 9 Dec 2024 20:02:57 +1000 Subject: [PATCH 11/11] Simpler wording for Duplication text and buttons --- .../modals/manager/DuplicateFileModal.tsx | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx b/packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx index 3e7866a1e82..ed1d705521e 100644 --- a/packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx +++ b/packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx @@ -170,7 +170,7 @@ export function DuplicateFileModal({ )} {isLocalFile ? ( - isCloudFile ? ( + isCloudFile && ( Your budget is hosted on a server, making it accessible for @@ -180,14 +180,6 @@ export function DuplicateFileModal({ or keep it stored locally on this device? - ) : ( - - - Your budget is only located on your device. -
- Only a local copy will be duplicated. - - ) ) : ( @@ -235,7 +227,8 @@ export function DuplicateFileModal({ }} onPress={() => handleDuplicate('localOnly')} > - Duplicate locally + Duplicate + {isCloudFile && locally} )}