Fix raised feedback

This commit is contained in:
Connor Yoh 2025-09-12 18:08:41 +01:00
parent be31da217b
commit 18097a6d9b
10 changed files with 21 additions and 195 deletions

View File

@ -14,9 +14,9 @@ interface FileManagerContextValue {
selectedFiles: StirlingFileStub[];
filteredFiles: StirlingFileStub[];
fileInputRef: React.RefObject<HTMLInputElement | null>;
selectedFilesSet: Set<string>;
expandedFileIds: Set<string>;
fileGroups: Map<string, StirlingFileStub[]>;
selectedFilesSet: Set<FileId>;
expandedFileIds: Set<FileId>;
fileGroups: Map<FileId, StirlingFileStub[]>;
loadedHistoryFiles: Map<FileId, StirlingFileStub[]>;
// Handlers
@ -76,7 +76,7 @@ export const FileManagerProvider: React.FC<FileManagerProviderProps> = ({
const [selectedFileIds, setSelectedFileIds] = useState<FileId[]>([]);
const [searchTerm, setSearchTerm] = useState('');
const [lastClickedIndex, setLastClickedIndex] = useState<number | null>(null);
const [expandedFileIds, setExpandedFileIds] = useState<Set<string>>(new Set());
const [expandedFileIds, setExpandedFileIds] = useState<Set<FileId>>(new Set());
const [loadedHistoryFiles, setLoadedHistoryFiles] = useState<Map<FileId, StirlingFileStub[]>>(new Map()); // Cache for loaded history
const fileInputRef = useRef<HTMLInputElement>(null);
@ -173,12 +173,12 @@ export const FileManagerProvider: React.FC<FileManagerProviderProps> = ({
// 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<string>();
const filesToPreserve = new Set<string>();
): FileId[] => {
const fileMap = new Map(allStoredStubs.map(f => [f.id, f]));
const filesToDelete = new Set<FileId>();
const filesToPreserve = new Set<FileId>();
// First, identify all files in the lineages of the leaf files being deleted
for (const leafFileId of fileIds) {
@ -222,8 +222,8 @@ export const FileManagerProvider: React.FC<FileManagerProviderProps> = ({
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)
@ -243,7 +243,7 @@ export const FileManagerProvider: React.FC<FileManagerProviderProps> = ({
});
if (!hasLivingDescendants) {
orphanedNonLeafFiles.push(file.id as string);
orphanedNonLeafFiles.push(file.id);
}
}
}
@ -251,13 +251,6 @@ export const FileManagerProvider: React.FC<FileManagerProviderProps> = ({
// 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<FileManagerProviderProps> = ({
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<FileManagerProviderProps> = ({
// 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);
}

View File

@ -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';
@ -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(),

View File

@ -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());

View File

@ -16,7 +16,6 @@ export const adjustPageScaleOperationConfig = {
buildFormData: buildAdjustPageScaleFormData,
operationType: 'adjustPageScale',
endpoint: '/api/v1/general/scale-pages',
filePrefix: 'scaled_',
defaultParameters,
} as const;

View File

@ -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;

View File

@ -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());

View File

@ -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<FlattenParameters>({
...flattenOperationConfig,
filePrefix: t('flatten.filenamePrefix', 'flattened') + '_',
getErrorMessage: createStandardErrorHandler(t('flatten.error.failed', 'An error occurred while flattening the PDF.'))
});
};

View File

@ -37,7 +37,6 @@ export const redactOperationConfig = {
throw new Error('Manual redaction not yet implemented');
}
},
filePrefix: 'redacted_',
defaultParameters,
} as const;

View File

@ -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());

View File

@ -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<string, any>;
}>;
}
interface UseFileHistoryResult {
historyData: FileHistoryState | null;
isLoading: boolean;
error: string | null;
loadHistory: (file: File, fileId: FileId, updateFileStub?: (id: FileId, updates: Partial<StirlingFileStub>) => void) => Promise<void>;
clearHistory: () => void;
}
export function useFileHistory(): UseFileHistoryResult {
const [historyData, setHistoryData] = useState<FileHistoryState | null>(null);
const [isLoading, setIsLoading] = useState(false);
const [error, setError] = useState<string | null>(null);
const loadHistory = useCallback(async (
_file: File,
_fileId: FileId,
_updateFileStub?: (id: FileId, updates: Partial<StirlingFileStub>) => 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<Map<FileId, FileHistoryState>>(new Map());
const [loadingFiles, setLoadingFiles] = useState<Set<FileId>>(new Set());
const [errors, setErrors] = useState<Map<FileId, string>>(new Map());
const loadFileHistory = useCallback(async (
_file: File,
fileId: FileId,
_updateFileStub?: (id: FileId, updates: Partial<StirlingFileStub>) => 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
};
}