From bd75ad042a152c5ebba7c822301a2f8dc5c2b1b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20Sz=C3=BCcs?= <127139797+balazs-szucs@users.noreply.github.com> Date: Tue, 20 Jan 2026 15:57:44 +0100 Subject: [PATCH] [V2] fix(automation): enhance parameter handling and default values across operations, fix error in ManyToOne tools (#5123) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description of Changes This pull request focuses on improving the handling of tool parameters in the frontend automation and tool operation system. The main changes ensure that default parameter values are consistently applied, missing or undefined values are handled gracefully, and custom tools like Automate and Merge correctly track input file consumption. These updates enhance reliability and reduce errors due to missing or improperly formatted parameters. **Parameter Handling and Defaults:** * Tool configuration and automation forms now merge user-provided parameters with tool-specific default parameters, guaranteeing all required fields are present during initialization and execution (`ToolConfigurationModal.tsx`, `useAutomationForm.ts`, `automationExecutor.ts`). * All tool operation hooks (e.g., Add Watermark, Merge, Sanitize, Split, Compress, Change Permissions, OCR) have been updated to safely handle missing or undefined parameter values by providing sensible defaults, preventing runtime errors and backend issues (`useAddWatermarkOperation.ts`, `useMergeOperation.ts`, `useSanitizeOperation.ts`, `useSplitOperation.ts`, `useCompressOperation.ts`, `useChangePermissionsOperation.ts`, `useOCROperation.ts`). **Tool Operation Configuration Enhancements:** * The `BaseToolOperationConfig` interface now supports a `consumesAllInputs` flag, allowing tools with complex input-output relationships (like Automate and Merge) to mark all input files as successfully processed when appropriate (`useToolOperation.ts`). * The Automate tool operation is configured to use this new flag, and the core hook logic is updated to respect it when determining success (`useAutomateOperation.ts`, `useToolOperation.ts`). **Registry and Configuration Consistency:** * Default parameters are now explicitly included in tool operation configs (e.g., Merge), ensuring consistency between registry definitions and runtime behavior (`useMergeOperation.ts`). **Dependency Updates:** * React hooks and callbacks now correctly include dependencies related to parameter defaults to avoid stale values in forms and modals (`useAutomationForm.ts`). These changes collectively improve the robustness and maintainability of the automation and tool configuration system. --- ## Checklist ### General - [X] I have read the [Contribution Guidelines](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/CONTRIBUTING.md) - [ ] I have read the [Stirling-PDF Developer Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/DeveloperGuide.md) (if applicable) - [X] I have read the [How to add new languages to Stirling-PDF](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/HowToAddNewLanguage.md) (if applicable) - [X] I have performed a self-review of my own code - [X] My changes generate no new warnings ### Documentation - [ ] I have updated relevant docs on [Stirling-PDF's doc repo](https://github.com/Stirling-Tools/Stirling-Tools.github.io/blob/main/docs/) (if functionality has heavily changed) - [ ] I have read the section [Add New Translation Tags](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/HowToAddNewLanguage.md#add-new-translation-tags) (for new translation tags only) ### Translations (if applicable) - [ ] I ran [`scripts/counter_translation.py`](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/docs/counter_translation.md) ### UI Changes (if applicable) - [ ] Screenshots or videos demonstrating the UI changes are attached (e.g., as comments or direct attachments in the PR) ### Testing (if applicable) - [X] I have tested my changes locally. Refer to the [Testing Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/DeveloperGuide.md#6-testing) for more details. --------- Signed-off-by: Balázs Szücs Co-authored-by: Reece Browne <74901996+reecebrowne@users.noreply.github.com> --- .../tools/automate/ToolConfigurationModal.tsx | 11 +++++++---- .../tools/addWatermark/useAddWatermarkOperation.ts | 8 ++++---- .../hooks/tools/automate/useAutomateOperation.ts | 3 ++- .../core/hooks/tools/automate/useAutomationForm.ts | 4 ++-- .../useChangePermissionsOperation.ts | 11 ++++++----- .../hooks/tools/compress/useCompressOperation.ts | 2 +- .../src/core/hooks/tools/merge/useMergeOperation.ts | 7 ++++--- frontend/src/core/hooks/tools/ocr/useOCROperation.ts | 12 +++++++----- .../hooks/tools/sanitize/useSanitizeOperation.ts | 12 ++++++------ .../src/core/hooks/tools/shared/useToolOperation.ts | 7 +++++++ .../src/core/hooks/tools/split/useSplitOperation.ts | 8 ++++---- frontend/src/core/utils/automationExecutor.ts | 9 ++++++--- 12 files changed, 56 insertions(+), 38 deletions(-) diff --git a/frontend/src/core/components/tools/automate/ToolConfigurationModal.tsx b/frontend/src/core/components/tools/automate/ToolConfigurationModal.tsx index a4b415856..fbb51467a 100644 --- a/frontend/src/core/components/tools/automate/ToolConfigurationModal.tsx +++ b/frontend/src/core/components/tools/automate/ToolConfigurationModal.tsx @@ -41,13 +41,16 @@ export default function ToolConfigurationModal({ opened, tool, onSave, onCancel, // Initialize parameters from tool (which should contain defaults from registry) useEffect(() => { + const toolConfig = toolRegistry[tool.operation as ToolId]?.operationConfig; + const defaultParams = toolConfig?.defaultParameters || {}; + if (tool.parameters) { - setParameters(tool.parameters); + setParameters({ ...defaultParams, ...tool.parameters }); } else { - // Fallback to empty parameters if none provided - setParameters({}); + // Fallback to default parameters if none provided + setParameters({ ...defaultParams }); } - }, [tool.parameters, tool.operation]); + }, [tool.parameters, tool.operation, toolRegistry]); // Render the settings component const renderToolSettings = () => { diff --git a/frontend/src/core/hooks/tools/addWatermark/useAddWatermarkOperation.ts b/frontend/src/core/hooks/tools/addWatermark/useAddWatermarkOperation.ts index 6dcdec4eb..a02b95290 100644 --- a/frontend/src/core/hooks/tools/addWatermark/useAddWatermarkOperation.ts +++ b/frontend/src/core/hooks/tools/addWatermark/useAddWatermarkOperation.ts @@ -18,7 +18,7 @@ export const buildAddWatermarkFormData = (parameters: AddWatermarkParameters, fi formData.append("watermarkImage", parameters.watermarkImage); } - // Required parameters with correct formatting + // Required parameters with correct formatting (defaults merged in automationExecutor) formData.append("fontSize", parameters.fontSize.toString()); formData.append("rotation", parameters.rotation.toString()); formData.append("opacity", (parameters.opacity / 100).toString()); // Convert percentage to decimal @@ -26,9 +26,9 @@ export const buildAddWatermarkFormData = (parameters: AddWatermarkParameters, fi formData.append("heightSpacer", parameters.heightSpacer.toString()); // Backend-expected parameters from user input - formData.append("alphabet", parameters.alphabet); - formData.append("customColor", parameters.customColor); - formData.append("convertPDFToImage", parameters.convertPDFToImage.toString()); + formData.append("alphabet", parameters.alphabet || ""); + formData.append("customColor", parameters.customColor || ""); + formData.append("convertPDFToImage", (parameters.convertPDFToImage ?? false).toString()); return formData; }; diff --git a/frontend/src/core/hooks/tools/automate/useAutomateOperation.ts b/frontend/src/core/hooks/tools/automate/useAutomateOperation.ts index f6fbcacda..d88507636 100644 --- a/frontend/src/core/hooks/tools/automate/useAutomateOperation.ts +++ b/frontend/src/core/hooks/tools/automate/useAutomateOperation.ts @@ -38,7 +38,7 @@ export function useAutomateOperation() { console.log(`✅ Automation completed, returning ${finalResults.length} files`); return { files: finalResults, - consumedAllInputs: false, + consumedAllInputs: true, }; }, [toolRegistry]); @@ -46,5 +46,6 @@ export function useAutomateOperation() { toolType: ToolType.custom, operationType: 'automate', customProcessor, + consumesAllInputs: true, }); } diff --git a/frontend/src/core/hooks/tools/automate/useAutomationForm.ts b/frontend/src/core/hooks/tools/automate/useAutomationForm.ts index c86d112d6..3bcfad53b 100644 --- a/frontend/src/core/hooks/tools/automate/useAutomationForm.ts +++ b/frontend/src/core/hooks/tools/automate/useAutomationForm.ts @@ -52,7 +52,7 @@ export function useAutomationForm({ mode, existingAutomation, toolRegistry }: Us operation: operation, name: getToolName(operation), configured: isConfigured, - parameters: typeof op === 'object' ? op.parameters || {} : {} + parameters: typeof op === 'object' ? { ...getToolDefaultParameters(operation), ...(op.parameters || {}) } : getToolDefaultParameters(operation) }; }); @@ -68,7 +68,7 @@ export function useAutomationForm({ mode, existingAutomation, toolRegistry }: Us })); setSelectedTools(defaultTools); } - }, [mode, existingAutomation, t, getToolName]); + }, [mode, existingAutomation, t, getToolName, getToolDefaultParameters]); const addTool = (operation: string) => { const toolEntry = toolRegistry[operation as ToolId]; diff --git a/frontend/src/core/hooks/tools/changePermissions/useChangePermissionsOperation.ts b/frontend/src/core/hooks/tools/changePermissions/useChangePermissionsOperation.ts index 6e391f739..e854b5865 100644 --- a/frontend/src/core/hooks/tools/changePermissions/useChangePermissionsOperation.ts +++ b/frontend/src/core/hooks/tools/changePermissions/useChangePermissionsOperation.ts @@ -3,11 +3,12 @@ import { ToolType, useToolOperation } from '@app/hooks/tools/shared/useToolOpera import { createStandardErrorHandler } from '@app/utils/toolErrorHandler'; import { ChangePermissionsParameters, defaultParameters } from '@app/hooks/tools/changePermissions/useChangePermissionsParameters'; -export const getFormData = ((parameters: ChangePermissionsParameters) => - Object.entries(parameters).map(([key, value]) => - [key, value.toString()] - ) as string[][] -); +export const getFormData = ((parameters: ChangePermissionsParameters) => { + if (!parameters) return []; + return Object.entries(parameters).map(([key, value]) => + [key, (value ?? false).toString()] + ) as string[][]; +}); // Static function that can be used by both the hook and automation executor export const buildChangePermissionsFormData = (parameters: ChangePermissionsParameters, file: File): FormData => { diff --git a/frontend/src/core/hooks/tools/compress/useCompressOperation.ts b/frontend/src/core/hooks/tools/compress/useCompressOperation.ts index e1dda9d0f..c10bea535 100644 --- a/frontend/src/core/hooks/tools/compress/useCompressOperation.ts +++ b/frontend/src/core/hooks/tools/compress/useCompressOperation.ts @@ -18,7 +18,7 @@ export const buildCompressFormData = (parameters: CompressParameters, file: File } } - formData.append("grayscale", parameters.grayscale.toString()); + formData.append("grayscale", (parameters.grayscale ?? false).toString()); formData.append("lineArt", parameters.lineArt.toString()); formData.append("linearize", parameters.linearize.toString()); if (parameters.lineArt) { diff --git a/frontend/src/core/hooks/tools/merge/useMergeOperation.ts b/frontend/src/core/hooks/tools/merge/useMergeOperation.ts index e21814a4f..3bed21479 100644 --- a/frontend/src/core/hooks/tools/merge/useMergeOperation.ts +++ b/frontend/src/core/hooks/tools/merge/useMergeOperation.ts @@ -1,7 +1,7 @@ import { useTranslation } from 'react-i18next'; import { useToolOperation, ToolOperationConfig, ToolType } from '@app/hooks/tools/shared/useToolOperation'; import { createStandardErrorHandler } from '@app/utils/toolErrorHandler'; -import { MergeParameters } from '@app/hooks/tools/merge/useMergeParameters'; +import { MergeParameters, defaultParameters } from '@app/hooks/tools/merge/useMergeParameters'; const buildFormData = (parameters: MergeParameters, files: File[]): FormData => { const formData = new FormData(); @@ -13,8 +13,8 @@ const buildFormData = (parameters: MergeParameters, files: File[]): FormData => const clientIds: string[] = files.map((f: any) => String((f as any).fileId || f.name)); formData.append('clientFileIds', JSON.stringify(clientIds)); formData.append("sortType", "orderProvided"); // Always use orderProvided since UI handles sorting - formData.append("removeCertSign", parameters.removeDigitalSignature.toString()); - formData.append("generateToc", parameters.generateTableOfContents.toString()); + formData.append("removeCertSign", (parameters.removeDigitalSignature ?? false).toString()); + formData.append("generateToc", (parameters.generateTableOfContents ?? false).toString()); return formData; }; @@ -26,6 +26,7 @@ export const mergeOperationConfig: ToolOperationConfig = { operationType: 'merge', endpoint: '/api/v1/general/merge-pdfs', filePrefix: 'merged_', + defaultParameters, }; export const useMergeOperation = () => { diff --git a/frontend/src/core/hooks/tools/ocr/useOCROperation.ts b/frontend/src/core/hooks/tools/ocr/useOCROperation.ts index 6e86a083e..8b3950e7f 100644 --- a/frontend/src/core/hooks/tools/ocr/useOCROperation.ts +++ b/frontend/src/core/hooks/tools/ocr/useOCROperation.ts @@ -44,11 +44,13 @@ export const buildOCRFormData = (parameters: OCRParameters, file: File): FormDat parameters.languages.forEach((lang) => formData.append('languages', lang)); formData.append('ocrType', parameters.ocrType); formData.append('ocrRenderType', parameters.ocrRenderType); - formData.append('sidecar', parameters.additionalOptions.includes('sidecar').toString()); - formData.append('deskew', parameters.additionalOptions.includes('deskew').toString()); - formData.append('clean', parameters.additionalOptions.includes('clean').toString()); - formData.append('cleanFinal', parameters.additionalOptions.includes('cleanFinal').toString()); - formData.append('removeImagesAfter', parameters.additionalOptions.includes('removeImagesAfter').toString()); + + const options = parameters.additionalOptions || []; + formData.append('sidecar', options.includes('sidecar').toString()); + formData.append('deskew', options.includes('deskew').toString()); + formData.append('clean', options.includes('clean').toString()); + formData.append('cleanFinal', options.includes('cleanFinal').toString()); + formData.append('removeImagesAfter', options.includes('removeImagesAfter').toString()); return formData; }; diff --git a/frontend/src/core/hooks/tools/sanitize/useSanitizeOperation.ts b/frontend/src/core/hooks/tools/sanitize/useSanitizeOperation.ts index 580fabe31..b4de35601 100644 --- a/frontend/src/core/hooks/tools/sanitize/useSanitizeOperation.ts +++ b/frontend/src/core/hooks/tools/sanitize/useSanitizeOperation.ts @@ -9,12 +9,12 @@ export const buildSanitizeFormData = (parameters: SanitizeParameters, file: File formData.append('fileInput', file); // Add parameters - formData.append('removeJavaScript', parameters.removeJavaScript.toString()); - formData.append('removeEmbeddedFiles', parameters.removeEmbeddedFiles.toString()); - formData.append('removeXMPMetadata', parameters.removeXMPMetadata.toString()); - formData.append('removeMetadata', parameters.removeMetadata.toString()); - formData.append('removeLinks', parameters.removeLinks.toString()); - formData.append('removeFonts', parameters.removeFonts.toString()); + formData.append('removeJavaScript', (parameters.removeJavaScript ?? false).toString()); + formData.append('removeEmbeddedFiles', (parameters.removeEmbeddedFiles ?? false).toString()); + formData.append('removeXMPMetadata', (parameters.removeXMPMetadata ?? false).toString()); + formData.append('removeMetadata', (parameters.removeMetadata ?? false).toString()); + formData.append('removeLinks', (parameters.removeLinks ?? false).toString()); + formData.append('removeFonts', (parameters.removeFonts ?? false).toString()); return formData; }; diff --git a/frontend/src/core/hooks/tools/shared/useToolOperation.ts b/frontend/src/core/hooks/tools/shared/useToolOperation.ts index 4804032d0..b4881408a 100644 --- a/frontend/src/core/hooks/tools/shared/useToolOperation.ts +++ b/frontend/src/core/hooks/tools/shared/useToolOperation.ts @@ -71,6 +71,13 @@ interface BaseToolOperationConfig { /** Default parameter values for automation */ defaultParameters?: TParams; + + /** + * For custom tools: if true, success implies all input files were successfully processed. + * Use this for tools like Automate or Merge where Many-to-One relationships exist + * and exact input-output mapping is difficult. + */ + consumesAllInputs?: boolean; } export interface SingleFileToolOperationConfig extends BaseToolOperationConfig { diff --git a/frontend/src/core/hooks/tools/split/useSplitOperation.ts b/frontend/src/core/hooks/tools/split/useSplitOperation.ts index 1392900c9..425948683 100644 --- a/frontend/src/core/hooks/tools/split/useSplitOperation.ts +++ b/frontend/src/core/hooks/tools/split/useSplitOperation.ts @@ -19,7 +19,7 @@ export const buildSplitFormData = (parameters: SplitParameters, file: File): For case SPLIT_METHODS.BY_SECTIONS: formData.append("horizontalDivisions", parameters.hDiv); formData.append("verticalDivisions", parameters.vDiv); - formData.append("merge", parameters.merge.toString()); + formData.append("merge", (parameters.merge ?? false).toString()); formData.append("splitMode", parameters.splitMode || 'SPLIT_ALL'); if (parameters.splitMode === 'CUSTOM' && parameters.customPages) { formData.append("pageNumbers", parameters.customPages); @@ -39,11 +39,11 @@ export const buildSplitFormData = (parameters: SplitParameters, file: File): For break; case SPLIT_METHODS.BY_CHAPTERS: formData.append("bookmarkLevel", parameters.bookmarkLevel); - formData.append("includeMetadata", parameters.includeMetadata.toString()); - formData.append("allowDuplicates", parameters.allowDuplicates.toString()); + formData.append("includeMetadata", (parameters.includeMetadata ?? false).toString()); + formData.append("allowDuplicates", (parameters.allowDuplicates ?? false).toString()); break; case SPLIT_METHODS.BY_PAGE_DIVIDER: - formData.append("duplexMode", parameters.duplexMode.toString()); + formData.append("duplexMode", (parameters.duplexMode ?? false).toString()); break; default: throw new Error(`Unknown split method: ${parameters.method}`); diff --git a/frontend/src/core/utils/automationExecutor.ts b/frontend/src/core/utils/automationExecutor.ts index 2651784e1..7f8801559 100644 --- a/frontend/src/core/utils/automationExecutor.ts +++ b/frontend/src/core/utils/automationExecutor.ts @@ -155,18 +155,21 @@ export const executeToolOperationWithPrefix = async ( throw new Error(`Tool operation not supported: ${operationName}`); } + // Merge with default parameters to ensure all required fields are present + const mergedParameters = { ...config.defaultParameters, ...parameters }; + try { // Check if tool uses custom processor (like Convert tool) if (config.customProcessor) { - const result = await config.customProcessor(parameters, files); + const result = await config.customProcessor(mergedParameters, files); return result.files; } // Execute based on tool type if (config.toolType === ToolType.multiFile) { - return await executeMultiFileOperation(config, parameters, files, filePrefix); + return await executeMultiFileOperation(config, mergedParameters, files, filePrefix); } else { - return await executeSingleFileOperation(config, parameters, files, filePrefix); + return await executeSingleFileOperation(config, mergedParameters, files, filePrefix); } } catch (error: any) {