From 18097a6d9b88a70399598ae384024a447a277196 Mon Sep 17 00:00:00 2001 From: Connor Yoh Date: Fri, 12 Sep 2025 18:08:41 +0100 Subject: [PATCH] Fix raised feedback --- frontend/src/contexts/FileManagerContext.tsx | 41 ++--- frontend/src/contexts/IndexedDBContext.tsx | 8 +- .../useAddPasswordOperation.test.ts | 1 - .../useAdjustPageScaleOperation.ts | 1 - .../autoRename/useAutoRenameOperation.ts | 1 - .../useChangePermissionsOperation.test.ts | 1 - .../tools/flatten/useFlattenOperation.ts | 4 +- .../hooks/tools/redact/useRedactOperation.ts | 1 - .../useRemovePasswordOperation.test.ts | 1 - frontend/src/hooks/useFileHistory.ts | 157 ------------------ 10 files changed, 21 insertions(+), 195 deletions(-) delete mode 100644 frontend/src/hooks/useFileHistory.ts diff --git a/frontend/src/contexts/FileManagerContext.tsx b/frontend/src/contexts/FileManagerContext.tsx index 4e1c88a9c..6f3afe21b 100644 --- a/frontend/src/contexts/FileManagerContext.tsx +++ b/frontend/src/contexts/FileManagerContext.tsx @@ -14,9 +14,9 @@ interface FileManagerContextValue { selectedFiles: StirlingFileStub[]; filteredFiles: StirlingFileStub[]; fileInputRef: React.RefObject; - selectedFilesSet: Set; - expandedFileIds: Set; - fileGroups: Map; + selectedFilesSet: Set; + expandedFileIds: Set; + fileGroups: Map; loadedHistoryFiles: Map; // Handlers @@ -76,7 +76,7 @@ export const FileManagerProvider: React.FC = ({ const [selectedFileIds, setSelectedFileIds] = useState([]); const [searchTerm, setSearchTerm] = useState(''); const [lastClickedIndex, setLastClickedIndex] = useState(null); - const [expandedFileIds, setExpandedFileIds] = useState>(new Set()); + const [expandedFileIds, setExpandedFileIds] = useState>(new Set()); const [loadedHistoryFiles, setLoadedHistoryFiles] = useState>(new Map()); // Cache for loaded history const fileInputRef = useRef(null); @@ -173,12 +173,12 @@ export const FileManagerProvider: React.FC = ({ // Helper function to safely determine which files can be deleted const getSafeFilesToDelete = useCallback(( - fileIds: string[], + fileIds: FileId[], allStoredStubs: StirlingFileStub[] - ): string[] => { - const fileMap = new Map(allStoredStubs.map(f => [f.id as string, f])); - const filesToDelete = new Set(); - const filesToPreserve = new Set(); + ): FileId[] => { + const fileMap = new Map(allStoredStubs.map(f => [f.id, f])); + const filesToDelete = new Set(); + const filesToPreserve = new Set(); // First, identify all files in the lineages of the leaf files being deleted for (const leafFileId of fileIds) { @@ -222,14 +222,14 @@ export const FileManagerProvider: React.FC = ({ let safeToDelete = Array.from(filesToDelete).filter(fileId => !filesToPreserve.has(fileId)); // Check for orphaned non-leaf files after main deletion - const remainingFiles = allStoredStubs.filter(file => !safeToDelete.includes(file.id as string)); - const orphanedNonLeafFiles: string[] = []; + const remainingFiles = allStoredStubs.filter(file => !safeToDelete.includes(file.id)); + const orphanedNonLeafFiles: FileId[] = []; for (const file of remainingFiles) { // Only check non-leaf files (files that have been processed and have children) if (file.isLeaf === false) { const fileOriginalId = file.originalFileId || file.id; - + // Check if this non-leaf file has any living descendants const hasLivingDescendants = remainingFiles.some(otherFile => { // Check if otherFile is a descendant of this file @@ -243,7 +243,7 @@ export const FileManagerProvider: React.FC = ({ }); if (!hasLivingDescendants) { - orphanedNonLeafFiles.push(file.id as string); + orphanedNonLeafFiles.push(file.id); } } } @@ -251,13 +251,6 @@ export const FileManagerProvider: React.FC = ({ // Add orphaned non-leaf files to deletion list safeToDelete = [...safeToDelete, ...orphanedNonLeafFiles]; - console.log('Deletion analysis:', { - candidatesForDeletion: Array.from(filesToDelete), - mustPreserve: Array.from(filesToPreserve), - orphanedNonLeafFiles, - safeToDelete - }); - return safeToDelete; }, []); @@ -269,9 +262,7 @@ export const FileManagerProvider: React.FC = ({ const allStoredStubs = await fileStorage.getAllStirlingFileStubs(); // Get safe files to delete (respecting shared lineages) - const filesToDelete = getSafeFilesToDelete([deletedFileId as string], allStoredStubs); - - console.log(`Safely deleting files for ${fileToRemove.name}:`, filesToDelete); + const filesToDelete = getSafeFilesToDelete([deletedFileId], allStoredStubs); // Clear from selection immediately setSelectedFileIds(prev => prev.filter(id => !filesToDelete.includes(id))); @@ -292,7 +283,7 @@ export const FileManagerProvider: React.FC = ({ // Also remove deleted files from any other file's history cache for (const [mainFileId, historyFiles] of newCache.entries()) { - const filteredHistory = historyFiles.filter(histFile => !filesToDelete.includes(histFile.id as string)); + const filteredHistory = historyFiles.filter(histFile => !filesToDelete.includes(histFile.id)); if (filteredHistory.length !== historyFiles.length) { newCache.set(mainFileId, filteredHistory); } @@ -329,7 +320,7 @@ export const FileManagerProvider: React.FC = ({ // Find the file and its index in filteredFiles const fileIndex = filteredFiles.findIndex(file => file.id === fileId); const fileToRemove = filteredFiles[fileIndex]; - + if (fileToRemove && fileIndex !== -1) { await performFileDelete(fileToRemove, fileIndex); } diff --git a/frontend/src/contexts/IndexedDBContext.tsx b/frontend/src/contexts/IndexedDBContext.tsx index b916b247c..6c9130f00 100644 --- a/frontend/src/contexts/IndexedDBContext.tsx +++ b/frontend/src/contexts/IndexedDBContext.tsx @@ -6,7 +6,7 @@ import React, { createContext, useContext, useCallback, useRef } from 'react'; import { fileStorage } from '../services/fileStorage'; import { FileId } from '../types/file'; -import { StirlingFileStub, createStirlingFile } from '../types/fileContext'; +import { StirlingFileStub, createStirlingFile, createQuickKey } from '../types/fileContext'; import { generateThumbnailForFile } from '../utils/thumbnailUtils'; const DEBUG = process.env.NODE_ENV === 'development'; @@ -64,7 +64,7 @@ export function IndexedDBProvider({ children }: IndexedDBProviderProps) { // Store in IndexedDB (no history data - that's handled by direct fileStorage calls now) const stirlingFile = createStirlingFile(file, fileId); - + // Create minimal stub for storage const stub: StirlingFileStub = { id: fileId, @@ -72,7 +72,7 @@ export function IndexedDBProvider({ children }: IndexedDBProviderProps) { size: file.size, type: file.type, lastModified: file.lastModified, - quickKey: `${file.name}|${file.size}|${file.lastModified}`, + quickKey: createQuickKey(file), thumbnailUrl: thumbnail, isLeaf: true, createdAt: Date.now(), @@ -80,7 +80,7 @@ export function IndexedDBProvider({ children }: IndexedDBProviderProps) { originalFileId: fileId, toolHistory: [] }; - + await fileStorage.storeStirlingFile(stirlingFile, stub); const storedFile = await fileStorage.getStirlingFileStub(fileId); diff --git a/frontend/src/hooks/tools/addPassword/useAddPasswordOperation.test.ts b/frontend/src/hooks/tools/addPassword/useAddPasswordOperation.test.ts index b92da0033..fe254aac5 100644 --- a/frontend/src/hooks/tools/addPassword/useAddPasswordOperation.test.ts +++ b/frontend/src/hooks/tools/addPassword/useAddPasswordOperation.test.ts @@ -119,7 +119,6 @@ describe('useAddPasswordOperation', () => { test.each([ { property: 'toolType' as const, expectedValue: ToolType.singleFile }, { property: 'endpoint' as const, expectedValue: '/api/v1/security/add-password' }, - { property: 'filePrefix' as const, expectedValue: undefined }, { property: 'operationType' as const, expectedValue: 'addPassword' } ])('should configure $property correctly', ({ property, expectedValue }) => { renderHook(() => useAddPasswordOperation()); diff --git a/frontend/src/hooks/tools/adjustPageScale/useAdjustPageScaleOperation.ts b/frontend/src/hooks/tools/adjustPageScale/useAdjustPageScaleOperation.ts index 1728e5e1d..3d3ae3280 100644 --- a/frontend/src/hooks/tools/adjustPageScale/useAdjustPageScaleOperation.ts +++ b/frontend/src/hooks/tools/adjustPageScale/useAdjustPageScaleOperation.ts @@ -16,7 +16,6 @@ export const adjustPageScaleOperationConfig = { buildFormData: buildAdjustPageScaleFormData, operationType: 'adjustPageScale', endpoint: '/api/v1/general/scale-pages', - filePrefix: 'scaled_', defaultParameters, } as const; diff --git a/frontend/src/hooks/tools/autoRename/useAutoRenameOperation.ts b/frontend/src/hooks/tools/autoRename/useAutoRenameOperation.ts index e0d868a7d..237cc7cf6 100644 --- a/frontend/src/hooks/tools/autoRename/useAutoRenameOperation.ts +++ b/frontend/src/hooks/tools/autoRename/useAutoRenameOperation.ts @@ -28,7 +28,6 @@ export const autoRenameOperationConfig = { buildFormData: buildAutoRenameFormData, operationType: 'autoRename', endpoint: '/api/v1/misc/auto-rename', - filePrefix: 'autoRename_', preserveBackendFilename: true, // Use filename from backend response headers defaultParameters, } as const; diff --git a/frontend/src/hooks/tools/changePermissions/useChangePermissionsOperation.test.ts b/frontend/src/hooks/tools/changePermissions/useChangePermissionsOperation.test.ts index 6748b5547..fa942d2aa 100644 --- a/frontend/src/hooks/tools/changePermissions/useChangePermissionsOperation.test.ts +++ b/frontend/src/hooks/tools/changePermissions/useChangePermissionsOperation.test.ts @@ -113,7 +113,6 @@ describe('useChangePermissionsOperation', () => { test.each([ { property: 'toolType' as const, expectedValue: ToolType.singleFile }, { property: 'endpoint' as const, expectedValue: '/api/v1/security/add-password' }, - { property: 'filePrefix' as const, expectedValue: undefined }, { property: 'operationType' as const, expectedValue: 'change-permissions' } ])('should configure $property correctly', ({ property, expectedValue }) => { renderHook(() => useChangePermissionsOperation()); diff --git a/frontend/src/hooks/tools/flatten/useFlattenOperation.ts b/frontend/src/hooks/tools/flatten/useFlattenOperation.ts index 82eaba258..e2b687434 100644 --- a/frontend/src/hooks/tools/flatten/useFlattenOperation.ts +++ b/frontend/src/hooks/tools/flatten/useFlattenOperation.ts @@ -17,7 +17,6 @@ export const flattenOperationConfig = { buildFormData: buildFlattenFormData, operationType: 'flatten', endpoint: '/api/v1/misc/flatten', - filePrefix: 'flattened_', // Will be overridden in hook with translation multiFileEndpoint: false, defaultParameters, } as const; @@ -27,7 +26,6 @@ export const useFlattenOperation = () => { return useToolOperation({ ...flattenOperationConfig, - filePrefix: t('flatten.filenamePrefix', 'flattened') + '_', getErrorMessage: createStandardErrorHandler(t('flatten.error.failed', 'An error occurred while flattening the PDF.')) }); -}; \ No newline at end of file +}; diff --git a/frontend/src/hooks/tools/redact/useRedactOperation.ts b/frontend/src/hooks/tools/redact/useRedactOperation.ts index d4da5530d..407716395 100644 --- a/frontend/src/hooks/tools/redact/useRedactOperation.ts +++ b/frontend/src/hooks/tools/redact/useRedactOperation.ts @@ -37,7 +37,6 @@ export const redactOperationConfig = { throw new Error('Manual redaction not yet implemented'); } }, - filePrefix: 'redacted_', defaultParameters, } as const; diff --git a/frontend/src/hooks/tools/removePassword/useRemovePasswordOperation.test.ts b/frontend/src/hooks/tools/removePassword/useRemovePasswordOperation.test.ts index 0bb12920f..d3dc93f7b 100644 --- a/frontend/src/hooks/tools/removePassword/useRemovePasswordOperation.test.ts +++ b/frontend/src/hooks/tools/removePassword/useRemovePasswordOperation.test.ts @@ -97,7 +97,6 @@ describe('useRemovePasswordOperation', () => { test.each([ { property: 'toolType' as const, expectedValue: ToolType.singleFile }, { property: 'endpoint' as const, expectedValue: '/api/v1/security/remove-password' }, - { property: 'filePrefix' as const, expectedValue: undefined }, { property: 'operationType' as const, expectedValue: 'removePassword' } ])('should configure $property correctly', ({ property, expectedValue }) => { renderHook(() => useRemovePasswordOperation()); diff --git a/frontend/src/hooks/useFileHistory.ts b/frontend/src/hooks/useFileHistory.ts deleted file mode 100644 index 0a1f5c1cc..000000000 --- a/frontend/src/hooks/useFileHistory.ts +++ /dev/null @@ -1,157 +0,0 @@ -/** - * Custom hook for on-demand file history loading - * Replaces automatic history extraction during file loading - */ - -import { useState, useCallback } from 'react'; -import { FileId } from '../types/file'; -import { StirlingFileStub } from '../types/fileContext'; -// loadFileHistoryOnDemand removed - history now comes from IndexedDB directly - -interface FileHistoryState { - originalFileId?: string; - versionNumber?: number; - parentFileId?: FileId; - toolHistory?: Array<{ - toolName: string; - timestamp: number; - parameters?: Record; - }>; -} - -interface UseFileHistoryResult { - historyData: FileHistoryState | null; - isLoading: boolean; - error: string | null; - loadHistory: (file: File, fileId: FileId, updateFileStub?: (id: FileId, updates: Partial) => void) => Promise; - clearHistory: () => void; -} - -export function useFileHistory(): UseFileHistoryResult { - const [historyData, setHistoryData] = useState(null); - const [isLoading, setIsLoading] = useState(false); - const [error, setError] = useState(null); - - const loadHistory = useCallback(async ( - _file: File, - _fileId: FileId, - _updateFileStub?: (id: FileId, updates: Partial) => void - ) => { - setIsLoading(true); - setError(null); - - try { - // History is now loaded from IndexedDB, not PDF metadata - // This function is deprecated - throw new Error('loadFileHistoryOnDemand is deprecated - use IndexedDB history directly'); - } catch (err) { - const errorMessage = err instanceof Error ? err.message : 'Failed to load file history'; - setError(errorMessage); - setHistoryData(null); - } finally { - setIsLoading(false); - } - }, []); - - const clearHistory = useCallback(() => { - setHistoryData(null); - setError(null); - setIsLoading(false); - }, []); - - return { - historyData, - isLoading, - error, - loadHistory, - clearHistory - }; -} - -/** - * Hook for managing history state of multiple files - */ -export function useMultiFileHistory() { - const [historyCache, setHistoryCache] = useState>(new Map()); - const [loadingFiles, setLoadingFiles] = useState>(new Set()); - const [errors, setErrors] = useState>(new Map()); - - const loadFileHistory = useCallback(async ( - _file: File, - fileId: FileId, - _updateFileStub?: (id: FileId, updates: Partial) => void - ) => { - // Don't reload if already loaded or currently loading - if (historyCache.has(fileId) || loadingFiles.has(fileId)) { - return historyCache.get(fileId) || null; - } - - setLoadingFiles(prev => new Set(prev).add(fileId)); - setErrors(prev => { - const newErrors = new Map(prev); - newErrors.delete(fileId); - return newErrors; - }); - - try { - // History is now loaded from IndexedDB, not PDF metadata - // This function is deprecated - throw new Error('loadFileHistoryOnDemand is deprecated - use IndexedDB history directly'); - } catch (err) { - const errorMessage = err instanceof Error ? err.message : 'Failed to load file history'; - setErrors(prev => new Map(prev).set(fileId, errorMessage)); - return null; - } finally { - setLoadingFiles(prev => { - const newSet = new Set(prev); - newSet.delete(fileId); - return newSet; - }); - } - }, [historyCache, loadingFiles]); - - const getHistory = useCallback((fileId: FileId) => { - return historyCache.get(fileId) || null; - }, [historyCache]); - - const isLoadingHistory = useCallback((fileId: FileId) => { - return loadingFiles.has(fileId); - }, [loadingFiles]); - - const getError = useCallback((fileId: FileId) => { - return errors.get(fileId) || null; - }, [errors]); - - const clearHistory = useCallback((fileId: FileId) => { - setHistoryCache(prev => { - const newCache = new Map(prev); - newCache.delete(fileId); - return newCache; - }); - setErrors(prev => { - const newErrors = new Map(prev); - newErrors.delete(fileId); - return newErrors; - }); - setLoadingFiles(prev => { - const newSet = new Set(prev); - newSet.delete(fileId); - return newSet; - }); - }, []); - - const clearAllHistory = useCallback(() => { - setHistoryCache(new Map()); - setLoadingFiles(new Set()); - setErrors(new Map()); - }, []); - - return { - loadFileHistory, - getHistory, - isLoadingHistory, - getError, - clearHistory, - clearAllHistory - }; -}