From 5ed6cc3cfc61bfc604a5bd28c301abbcb8102b78 Mon Sep 17 00:00:00 2001 From: James Brunton Date: Fri, 22 Aug 2025 16:59:01 +0100 Subject: [PATCH] Redesign Merge around new major merges into V2 --- frontend/src/contexts/file/FileReducer.ts | 74 ++++++++++--------- frontend/src/contexts/file/fileHooks.ts | 40 +++++----- .../src/data/useTranslatedToolRegistry.tsx | 4 + .../tools/merge/useMergeOperation.test.ts | 6 +- .../hooks/tools/merge/useMergeOperation.ts | 19 +++-- frontend/src/tools/Merge.tsx | 49 ++++++------ 6 files changed, 103 insertions(+), 89 deletions(-) diff --git a/frontend/src/contexts/file/FileReducer.ts b/frontend/src/contexts/file/FileReducer.ts index 0fbec6da0..3fcc99248 100644 --- a/frontend/src/contexts/file/FileReducer.ts +++ b/frontend/src/contexts/file/FileReducer.ts @@ -2,10 +2,10 @@ * FileContext reducer - Pure state management for file operations */ -import { - FileContextState, - FileContextAction, - FileId, +import { + FileContextState, + FileContextAction, + FileId, FileRecord } from '../../types/fileContext'; @@ -32,7 +32,7 @@ export function fileContextReducer(state: FileContextState, action: FileContextA const { fileRecords } = action.payload; const newIds: FileId[] = []; const newById: Record = { ...state.files.byId }; - + fileRecords.forEach(record => { // Only add if not already present (dedupe by stable ID) if (!newById[record.id]) { @@ -40,7 +40,7 @@ export function fileContextReducer(state: FileContextState, action: FileContextA newById[record.id] = record; } }); - + return { ...state, files: { @@ -49,20 +49,20 @@ export function fileContextReducer(state: FileContextState, action: FileContextA } }; } - + case 'REMOVE_FILES': { const { fileIds } = action.payload; const remainingIds = state.files.ids.filter(id => !fileIds.includes(id)); const newById = { ...state.files.byId }; - + // Remove files from state (resource cleanup handled by lifecycle manager) fileIds.forEach(id => { delete newById[id]; }); - + // Clear selections that reference removed files const validSelectedFileIds = state.ui.selectedFileIds.filter(id => !fileIds.includes(id)); - + return { ...state, files: { @@ -75,15 +75,15 @@ export function fileContextReducer(state: FileContextState, action: FileContextA } }; } - + case 'UPDATE_FILE_RECORD': { const { id, updates } = action.payload; const existingRecord = state.files.byId[id]; - + if (!existingRecord) { return state; // File doesn't exist, no-op } - + return { ...state, files: { @@ -98,22 +98,28 @@ export function fileContextReducer(state: FileContextState, action: FileContextA } }; } - + case 'REORDER_FILES': { const { orderedFileIds } = action.payload; - + // Validate that all IDs exist in current state const validIds = orderedFileIds.filter(id => state.files.byId[id]); - + // Reorder selected files by passed order + const selectedFileIds = orderedFileIds.filter(id => state.ui.selectedFileIds.includes(id)); + return { ...state, files: { ...state.files, ids: validIds + }, + ui: { + ...state.ui, + selectedFileIds, } }; } - + case 'SET_SELECTED_FILES': { const { fileIds } = action.payload; return { @@ -124,7 +130,7 @@ export function fileContextReducer(state: FileContextState, action: FileContextA } }; } - + case 'SET_SELECTED_PAGES': { const { pageNumbers } = action.payload; return { @@ -135,7 +141,7 @@ export function fileContextReducer(state: FileContextState, action: FileContextA } }; } - + case 'CLEAR_SELECTIONS': { return { ...state, @@ -146,7 +152,7 @@ export function fileContextReducer(state: FileContextState, action: FileContextA } }; } - + case 'SET_PROCESSING': { const { isProcessing, progress } = action.payload; return { @@ -158,7 +164,7 @@ export function fileContextReducer(state: FileContextState, action: FileContextA } }; } - + case 'SET_UNSAVED_CHANGES': { return { ...state, @@ -168,42 +174,42 @@ export function fileContextReducer(state: FileContextState, action: FileContextA } }; } - + case 'PIN_FILE': { const { fileId } = action.payload; const newPinnedFiles = new Set(state.pinnedFiles); newPinnedFiles.add(fileId); - + return { ...state, pinnedFiles: newPinnedFiles }; } - + case 'UNPIN_FILE': { const { fileId } = action.payload; const newPinnedFiles = new Set(state.pinnedFiles); newPinnedFiles.delete(fileId); - + return { ...state, pinnedFiles: newPinnedFiles }; } - + case 'CONSUME_FILES': { const { inputFileIds, outputFileRecords } = action.payload; - + // Only remove unpinned input files const unpinnedInputIds = inputFileIds.filter(id => !state.pinnedFiles.has(id)); const remainingIds = state.files.ids.filter(id => !unpinnedInputIds.includes(id)); - + // Remove unpinned files from state const newById = { ...state.files.byId }; unpinnedInputIds.forEach(id => { delete newById[id]; }); - + // Add output files const outputIds: FileId[] = []; outputFileRecords.forEach(record => { @@ -212,10 +218,10 @@ export function fileContextReducer(state: FileContextState, action: FileContextA newById[record.id] = record; } }); - + // Clear selections that reference removed files const validSelectedFileIds = state.ui.selectedFileIds.filter(id => !unpinnedInputIds.includes(id)); - + return { ...state, files: { @@ -228,13 +234,13 @@ export function fileContextReducer(state: FileContextState, action: FileContextA } }; } - + case 'RESET_CONTEXT': { // Reset UI state to clean slate (resource cleanup handled by lifecycle manager) return { ...initialFileContextState }; } - + default: return state; } -} \ No newline at end of file +} diff --git a/frontend/src/contexts/file/fileHooks.ts b/frontend/src/contexts/file/fileHooks.ts index 19452958f..f9c1ffc63 100644 --- a/frontend/src/contexts/file/fileHooks.ts +++ b/frontend/src/contexts/file/fileHooks.ts @@ -3,11 +3,11 @@ */ import { useContext, useMemo } from 'react'; -import { - FileStateContext, +import { + FileStateContext, FileActionsContext, FileContextStateValue, - FileContextActionsValue + FileContextActionsValue } from './contexts'; import { FileId, FileRecord } from '../../types/fileContext'; @@ -39,7 +39,7 @@ export function useFileActions(): FileContextActionsValue { */ export function useCurrentFile(): { file?: File; record?: FileRecord } { const { state, selectors } = useFileState(); - + const primaryFileId = state.files.ids[0]; return useMemo(() => ({ file: primaryFileId ? selectors.getFile(primaryFileId) : undefined, @@ -81,7 +81,7 @@ export function useFileSelection() { */ export function useFileManagement() { const { actions } = useFileActions(); - + return useMemo(() => ({ addFiles: actions.addFiles, removeFiles: actions.removeFiles, @@ -112,7 +112,7 @@ export function useFileUI() { */ export function useFileRecord(fileId: FileId): { file?: File; record?: FileRecord } { const { selectors } = useFileState(); - + return useMemo(() => ({ file: selectors.getFile(fileId), record: selectors.getFileRecord(fileId) @@ -124,7 +124,7 @@ export function useFileRecord(fileId: FileId): { file?: File; record?: FileRecor */ export function useAllFiles(): { files: File[]; records: FileRecord[]; fileIds: FileId[] } { const { state, selectors } = useFileState(); - + return useMemo(() => ({ files: selectors.getFiles(), records: selectors.getFileRecords(), @@ -135,13 +135,13 @@ export function useAllFiles(): { files: File[]; records: FileRecord[]; fileIds: /** * Hook for selected files (optimized for selection-based UI) */ -export function useSelectedFiles(): { files: File[]; records: FileRecord[]; fileIds: FileId[] } { +export function useSelectedFiles(): { selectedFiles: File[]; selectedRecords: FileRecord[]; selectedFileIds: FileId[] } { const { state, selectors } = useFileState(); - + return useMemo(() => ({ - files: selectors.getSelectedFiles(), - records: selectors.getSelectedFileRecords(), - fileIds: state.ui.selectedFileIds + selectedFiles: selectors.getSelectedFiles(), + selectedRecords: selectors.getSelectedFileRecords(), + selectedFileIds: state.ui.selectedFileIds }), [state.ui.selectedFileIds, selectors]); } @@ -160,31 +160,31 @@ export function useFileContext() { trackBlobUrl: actions.trackBlobUrl, scheduleCleanup: actions.scheduleCleanup, setUnsavedChanges: actions.setHasUnsavedChanges, - + // File management addFiles: actions.addFiles, consumeFiles: actions.consumeFiles, recordOperation: (fileId: string, operation: any) => {}, // Operation tracking not implemented - markOperationApplied: (fileId: string, operationId: string) => {}, // Operation tracking not implemented + markOperationApplied: (fileId: string, operationId: string) => {}, // Operation tracking not implemented markOperationFailed: (fileId: string, operationId: string, error: string) => {}, // Operation tracking not implemented - + // File ID lookup findFileId: (file: File) => { return state.files.ids.find(id => { const record = state.files.byId[id]; - return record && - record.name === file.name && - record.size === file.size && + return record && + record.name === file.name && + record.size === file.size && record.lastModified === file.lastModified; }); }, - + // Pinned files pinnedFiles: state.pinnedFiles, pinFile: actions.pinFile, unpinFile: actions.unpinFile, isFilePinned: selectors.isFilePinned, - + // Active files activeFiles: selectors.getFiles() }), [state, selectors, actions]); diff --git a/frontend/src/data/useTranslatedToolRegistry.tsx b/frontend/src/data/useTranslatedToolRegistry.tsx index de1b152a9..71531c5f1 100644 --- a/frontend/src/data/useTranslatedToolRegistry.tsx +++ b/frontend/src/data/useTranslatedToolRegistry.tsx @@ -28,6 +28,7 @@ import { ocrOperationConfig } from '../hooks/tools/ocr/useOCROperation'; import { convertOperationConfig } from '../hooks/tools/convert/useConvertOperation'; import { removeCertificateSignOperationConfig } from '../hooks/tools/removeCertificateSign/useRemoveCertificateSignOperation'; import { changePermissionsOperationConfig } from '../hooks/tools/changePermissions/useChangePermissionsOperation'; +import { mergeOperationConfig } from '../hooks/tools/merge/useMergeOperation'; import CompressSettings from '../components/tools/compress/CompressSettings'; import SplitSettings from '../components/tools/split/SplitSettings'; import AddPasswordSettings from '../components/tools/addPassword/AddPasswordSettings'; @@ -39,6 +40,7 @@ import AddWatermarkSingleStepSettings from '../components/tools/addWatermark/Add import OCRSettings from '../components/tools/ocr/OCRSettings'; import ConvertSettings from '../components/tools/convert/ConvertSettings'; import ChangePermissionsSettings from '../components/tools/changePermissions/ChangePermissionsSettings'; +import MergeSettings from '../components/tools/merge/MergeSettings'; const showPlaceholderTools = false; // For development purposes. Allows seeing the full list of tools, even if they're unimplemented @@ -636,6 +638,8 @@ export function useFlatToolRegistry(): ToolRegistry { subcategoryId: SubcategoryId.GENERAL, maxFiles: -1, endpoints: ["merge-pdfs"], + operationConfig: mergeOperationConfig, + settingsComponent: MergeSettings }, "multi-tool": { icon: dashboard_customize, diff --git a/frontend/src/hooks/tools/merge/useMergeOperation.test.ts b/frontend/src/hooks/tools/merge/useMergeOperation.test.ts index 9f23b8c6c..0612f784d 100644 --- a/frontend/src/hooks/tools/merge/useMergeOperation.test.ts +++ b/frontend/src/hooks/tools/merge/useMergeOperation.test.ts @@ -20,12 +20,12 @@ vi.mock('../../../utils/toolErrorHandler', () => ({ })); // Import the mocked function -import { ToolOperationConfig, ToolOperationHook, useToolOperation } from '../shared/useToolOperation'; +import { ToolOperationHook, useToolOperation } from '../shared/useToolOperation'; describe('useMergeOperation', () => { - const mockUseToolOperation = vi.mocked(useToolOperation); + const mockUseToolOperation = vi.mocked(useToolOperation); - const getToolConfig = (): ToolOperationConfig => mockUseToolOperation.mock.calls[0][0]; + const getToolConfig = () => mockUseToolOperation.mock.calls[0][0]; const mockToolOperationReturn: ToolOperationHook = { files: [], diff --git a/frontend/src/hooks/tools/merge/useMergeOperation.ts b/frontend/src/hooks/tools/merge/useMergeOperation.ts index bf103359e..ec59cd129 100644 --- a/frontend/src/hooks/tools/merge/useMergeOperation.ts +++ b/frontend/src/hooks/tools/merge/useMergeOperation.ts @@ -1,5 +1,5 @@ import { useTranslation } from 'react-i18next'; -import { useToolOperation, ResponseHandler } from '../shared/useToolOperation'; +import { useToolOperation, ResponseHandler, ToolOperationConfig } from '../shared/useToolOperation'; import { createStandardErrorHandler } from '../../../utils/toolErrorHandler'; import { MergeParameters } from './useMergeParameters'; @@ -21,16 +21,21 @@ const mergeResponseHandler: ResponseHandler = (blob: Blob, originalFiles: File[] return [new File([blob], filename, { type: 'application/pdf' })]; }; +// Operation configuration for automation +export const mergeOperationConfig: ToolOperationConfig = { + operationType: 'merge', + endpoint: '/api/v1/general/merge-pdfs', + buildFormData, + filePrefix: 'merged_', + multiFileEndpoint: true, + responseHandler: mergeResponseHandler, +}; + export const useMergeOperation = () => { const { t } = useTranslation(); return useToolOperation({ - operationType: 'merge', - endpoint: '/api/v1/general/merge-pdfs', - buildFormData, - filePrefix: 'merged_', - multiFileEndpoint: true, // Single API call with all files - responseHandler: mergeResponseHandler, // Handle single PDF response + ...mergeOperationConfig, getErrorMessage: createStandardErrorHandler(t('merge.error.failed', 'An error occurred while merging the PDFs.')) }); }; diff --git a/frontend/src/tools/Merge.tsx b/frontend/src/tools/Merge.tsx index d5362cd90..84be9e2a3 100644 --- a/frontend/src/tools/Merge.tsx +++ b/frontend/src/tools/Merge.tsx @@ -1,8 +1,7 @@ -import React, { useCallback, useEffect } from "react"; +import { useCallback, useEffect } from "react"; import { useTranslation } from "react-i18next"; import { useEndpointEnabled } from "../hooks/useEndpointConfig"; -import { useFileContext } from "../contexts/FileContext"; -import { useToolFileSelection, useFileSelectionActions } from "../contexts/FileSelectionContext"; +import { useFileSelection, useFileManagement, useSelectedFiles, useAllFiles } from "../contexts/FileContext"; import { createToolFlow } from "../components/tools/shared/createToolFlow"; import MergeSettings from "../components/tools/merge/MergeSettings"; @@ -14,9 +13,10 @@ import { BaseToolProps } from "../types/tool"; const Merge = ({ onPreviewFile, onComplete, onError }: BaseToolProps) => { const { t } = useTranslation(); - const { setCurrentMode } = useFileContext(); - const { selectedFiles } = useToolFileSelection(); - const { setSelectedFiles } = useFileSelectionActions(); + const { selectedFiles, selectedFileIds } = useFileSelection(); + const { fileIds } = useAllFiles() + const { selectedRecords } = useSelectedFiles() + const { reorderFiles } = useFileManagement(); const mergeParams = useMergeParameters(); const mergeOperation = useMergeOperation(); @@ -45,36 +45,35 @@ const Merge = ({ onPreviewFile, onComplete, onError }: BaseToolProps) => { const handleThumbnailClick = (file: File) => { onPreviewFile?.(file); sessionStorage.setItem("previousMode", "merge"); - setCurrentMode("viewer"); }; const handleSettingsReset = () => { mergeOperation.resetResults(); onPreviewFile?.(null); - setCurrentMode("merge"); }; // TODO: Move to more general place so other tools can use it const sortFiles = useCallback((sortType: 'filename' | 'dateModified', ascending: boolean = true) => { - setSelectedFiles(((prevFiles: File[]) => { - const sortedFiles = [...prevFiles].sort((a, b) => { - let comparison = 0; + // Sort the FileIds based on their corresponding File properties + const sortedRecords = [...selectedRecords].sort((recordA, recordB) => { + let comparison = 0; + switch (sortType) { + case 'filename': + comparison = recordA.name.localeCompare(recordB.name); + break; + case 'dateModified': + comparison = recordA.lastModified - recordB.lastModified; + break; + } - switch (sortType) { - case 'filename': - comparison = a.name.localeCompare(b.name); - break; - case 'dateModified': - comparison = a.lastModified - b.lastModified; - break; - } + return ascending ? comparison : -comparison; + }); - return ascending ? comparison : -comparison; - }); + const selectedIds = sortedRecords.map(record => record.id); + const deselectedIds = fileIds.filter(id => !selectedIds.includes(id)); - return sortedFiles; - }) as any /* FIX ME: Parameter type is wrong on setSelectedFiles */); - }, []); + reorderFiles([...selectedIds, ...deselectedIds]); // Move all sorted IDs to the front of the workbench + }, [selectedFiles, selectedFileIds, reorderFiles]); const minFiles = 2; // Merging one file doesn't make sense const hasFiles = selectedFiles.length >= minFiles; @@ -83,7 +82,7 @@ const Merge = ({ onPreviewFile, onComplete, onError }: BaseToolProps) => { return createToolFlow({ files: { - selectedFiles, + selectedFiles: selectedFiles, isCollapsed: hasFiles && !hasResults, placeholder: "Select multiple PDF files to merge", minFiles: minFiles,