diff --git a/frontend/editor/src/core/components/tools/changeMetadata/ChangeMetadataSingleStep.tsx b/frontend/editor/src/core/components/tools/changeMetadata/ChangeMetadataSingleStep.tsx index 2eff20b23b..07ba4e7e05 100644 --- a/frontend/editor/src/core/components/tools/changeMetadata/ChangeMetadataSingleStep.tsx +++ b/frontend/editor/src/core/components/tools/changeMetadata/ChangeMetadataSingleStep.tsx @@ -1,5 +1,7 @@ +import { useContext, useEffect, useState } from "react"; import { Stack, Divider, Text } from "@mantine/core"; import { useTranslation } from "react-i18next"; +import { ViewerContext } from "@app/contexts/ViewerContext"; import { ChangeMetadataParameters, createCustomMetadataFunctions, @@ -19,6 +21,31 @@ interface ChangeMetadataSingleStepProps { disabled?: boolean; } +/** + * Pre-fills the form from the currently open document's existing metadata. + * Isolated in its own component so it only mounts where a ViewerProvider exists + * (the editor and the in-editor Automate modal). The pipeline builder has no + * viewer and no single "current document", so it is skipped there rather than + * crashing on useViewer. + */ +const MetadataPrefill = ({ + onParameterChange, + onExtractingChange, +}: { + onParameterChange: ChangeMetadataSingleStepProps["onParameterChange"]; + onExtractingChange: (extracting: boolean) => void; +}) => { + const { isExtractingMetadata } = useMetadataExtraction({ + updateParameter: onParameterChange, + }); + + useEffect(() => { + onExtractingChange(isExtractingMetadata); + }, [isExtractingMetadata, onExtractingChange]); + + return null; +}; + const ChangeMetadataSingleStep = ({ parameters, onParameterChange, @@ -26,77 +53,85 @@ const ChangeMetadataSingleStep = ({ }: ChangeMetadataSingleStepProps) => { const { t } = useTranslation(); + // Auto-prefill reads the viewer/file contexts, which only exist in the editor. + // Gate on the viewer so the pipeline builder renders the fields without it. + const hasViewerContext = useContext(ViewerContext) !== null; + const [isExtractingMetadata, setIsExtractingMetadata] = useState(false); + // Get custom metadata functions using the utility const { addCustomMetadata, removeCustomMetadata, updateCustomMetadata } = createCustomMetadataFunctions(parameters, onParameterChange); - // Extract metadata from uploaded files - const { isExtractingMetadata } = useMetadataExtraction({ - updateParameter: onParameterChange, - }); - const isDeleteAllEnabled = parameters.deleteAll; const fieldsDisabled = disabled || isDeleteAllEnabled || isExtractingMetadata; return ( - - {/* Delete All */} - - - {t("changeMetadata.deleteAll.label", "Delete All Metadata")} - - + {hasViewerContext && ( + - - - - - {/* Standard Metadata Fields */} + )} - - {t("changeMetadata.standardFields.title", "Standard Metadata")} - - + {/* Delete All */} + + + {t("changeMetadata.deleteAll.label", "Delete All Metadata")} + + + + + + + {/* Standard Metadata Fields */} + + + {t("changeMetadata.standardFields.title", "Standard Metadata")} + + + + + + + {/* Document Dates */} + + + {t("changeMetadata.dates.title", "Document Dates")} + + + + + + + {/* Advanced Options */} + + + {t("changeMetadata.advanced.title", "Advanced Options")} + + + - - - - {/* Document Dates */} - - - {t("changeMetadata.dates.title", "Document Dates")} - - - - - - - {/* Advanced Options */} - - - {t("changeMetadata.advanced.title", "Advanced Options")} - - - - + ); }; diff --git a/frontend/editor/src/core/components/tools/overlayPdfs/OverlayPdfsSettings.tsx b/frontend/editor/src/core/components/tools/overlayPdfs/OverlayPdfsSettings.tsx index 2648990248..e7246bb4e3 100644 --- a/frontend/editor/src/core/components/tools/overlayPdfs/OverlayPdfsSettings.tsx +++ b/frontend/editor/src/core/components/tools/overlayPdfs/OverlayPdfsSettings.tsx @@ -1,3 +1,4 @@ +import { useContext, useRef } from "react"; import { Stack, Text, @@ -7,6 +8,7 @@ import { Divider, } from "@mantine/core"; import { Button } from "@app/ui/Button"; +import { FilePicker } from "@app/ui/FilePicker"; import { ActionIcon } from "@app/ui/ActionIcon"; import { SegmentedControl } from "@app/ui/SegmentedControl"; import { useTranslation } from "react-i18next"; @@ -15,7 +17,7 @@ import { type OverlayMode, } from "@app/hooks/tools/overlayPdfs/useOverlayPdfsParameters"; import LocalIcon from "@app/components/shared/LocalIcon"; -import { useFilesModalContext } from "@app/contexts/FilesModalContext"; +import { FilesModalContext } from "@app/contexts/FilesModalContext"; import styles from "@app/components/tools/overlayPdfs/OverlayPdfsSettings.module.css"; import { Z_INDEX_AUTOMATE_DROPDOWN } from "@app/styles/zIndex"; @@ -34,7 +36,12 @@ export default function OverlayPdfsSettings({ disabled = false, }: OverlayPdfsSettingsProps) { const { t } = useTranslation(); - const { openFilesModal } = useFilesModalContext(); + // Read optionally: the portal pipeline builder mounts no FilesModalProvider. + // Present (editor tool + Automate modal) -> keep the workspace file picker; + // absent (portal) -> fall back to the plain file input below. + const filesModal = useContext(FilesModalContext); + // Clears the FilePicker so the same file can be re-selected (Mantine resetRef). + const resetOverlayPicker = useRef<() => void>(null); const handleOverlayFilesChange = (files: File[]) => { onParameterChange("overlayFiles", files); @@ -66,8 +73,8 @@ export default function OverlayPdfsSettings({ }; const handleOpenOverlayFilesModal = () => { - if (disabled) return; - openFilesModal({ + if (disabled || !filesModal) return; + filesModal.openFilesModal({ customHandler: (files: File[]) => { handleOverlayFilesChange([ ...(parameters.overlayFiles || []), @@ -77,6 +84,17 @@ export default function OverlayPdfsSettings({ }); }; + const appendOverlayFiles = (files: File[]) => { + if (files.length === 0) return; + handleOverlayFilesChange([...(parameters.overlayFiles || []), ...files]); + resetOverlayPicker.current?.(); + }; + + const overlayFilesButtonLabel = + parameters.overlayFiles?.length > 0 + ? t("overlay-pdfs.overlayFiles.addMore", "Add more PDFs...") + : t("overlay-pdfs.overlayFiles.placeholder", "Choose PDF(s)..."); + return ( @@ -183,17 +201,30 @@ export default function OverlayPdfsSettings({ {t("overlay-pdfs.overlayFiles.label", "Overlay Files")} - + {filesModal ? ( + + ) : ( + } + fullWidth + > + {overlayFilesButtonLabel} + + )} {parameters.overlayFiles?.length > 0 && (() => { diff --git a/frontend/editor/src/core/contexts/FilesModalContext.tsx b/frontend/editor/src/core/contexts/FilesModalContext.tsx index 73d1b0477f..17585ae7e8 100644 --- a/frontend/editor/src/core/contexts/FilesModalContext.tsx +++ b/frontend/editor/src/core/contexts/FilesModalContext.tsx @@ -41,7 +41,9 @@ interface FilesModalContextType { setOnModalClose: (callback: () => void) => void; } -const FilesModalContext = createContext(null); +export const FilesModalContext = createContext( + null, +); export const FilesModalProvider: React.FC<{ children: React.ReactNode }> = ({ children, diff --git a/frontend/editor/src/core/data/automatableToolsHaveOperationConfig.test.tsx b/frontend/editor/src/core/data/automatableToolsHaveOperationConfig.test.tsx index 461c07ebdf..c8942942df 100644 --- a/frontend/editor/src/core/data/automatableToolsHaveOperationConfig.test.tsx +++ b/frontend/editor/src/core/data/automatableToolsHaveOperationConfig.test.tsx @@ -30,4 +30,15 @@ describe("automatable tools", () => { expect(offeredWithoutConfig).toEqual([]); }); + + // Reorganize Pages has an automatable form (organization mode + page-order string) and a + // context-free settings component, but its registry entry once left automationSettings null, + // so both Automate and the pipeline builder showed "no configurable settings". Guard the wiring. + test("Reorganize Pages exposes automation settings so it is configurable, not no-settings", () => { + const { result } = renderHook(() => useTranslatedToolCatalog()); + + expect( + result.current.regularTools.reorganizePages?.automationSettings, + ).toBeTruthy(); + }); }); diff --git a/frontend/editor/src/core/data/useTranslatedToolRegistry.tsx b/frontend/editor/src/core/data/useTranslatedToolRegistry.tsx index 0a39da2fb9..5ae8075d0f 100644 --- a/frontend/editor/src/core/data/useTranslatedToolRegistry.tsx +++ b/frontend/editor/src/core/data/useTranslatedToolRegistry.tsx @@ -700,7 +700,10 @@ export function useTranslatedToolCatalog(): TranslatedToolCatalog { endpoints: ["rearrange-pages"], operationConfig: asRegistryConfig(reorganizePagesOperationConfig), synonyms: getSynonyms(t, "reorganizePages"), - automationSettings: null, + automationSettings: lazySettings( + () => + import("@app/components/tools/reorganizePages/ReorganizePagesSettings"), + ), }, scalePages: { icon: ( diff --git a/frontend/editor/src/portal/components/pipelines/PipelineStepSettings.test.tsx b/frontend/editor/src/portal/components/pipelines/PipelineStepSettings.test.tsx index 946e24b810..55a52b496e 100644 --- a/frontend/editor/src/portal/components/pipelines/PipelineStepSettings.test.tsx +++ b/frontend/editor/src/portal/components/pipelines/PipelineStepSettings.test.tsx @@ -1,10 +1,23 @@ import { describe, expect, it, vi } from "vitest"; -import { useEffect, useState } from "react"; -import { render, screen } from "@testing-library/react"; +import { + Component, + Suspense, + useEffect, + useState, + type ComponentType, + type ReactNode, +} from "react"; +import { render, renderHook, screen, waitFor } from "@testing-library/react"; import { PortalTestProviders } from "@portal/test/TestQueryProvider"; +import { useTranslatedToolCatalog } from "@app/data/useTranslatedToolRegistry"; +import { PreferencesProvider } from "@app/contexts/PreferencesContext"; +import { SidebarProvider } from "@app/contexts/SidebarContext"; import { Tooltip } from "@app/components/shared/Tooltip"; import type { ToolRegistry } from "@app/data/toolsTaxonomy"; -import type { WorkingToolStep } from "@app/hooks/tools/shared/toolAutomation"; +import { + getExecutableTools, + type WorkingToolStep, +} from "@app/hooks/tools/shared/toolAutomation"; import { asRegistryConfig, type ErasedToolParams, @@ -13,6 +26,10 @@ import { import ConvertSettings from "@app/components/tools/convert/ConvertSettings"; import { convertOperationConfig } from "@app/hooks/tools/convert/useConvertOperation"; import { defaultParameters as convertDefaults } from "@app/hooks/tools/convert/useConvertParameters"; +import ChangeMetadataSingleStep from "@app/components/tools/changeMetadata/ChangeMetadataSingleStep"; +import { defaultParameters as changeMetadataDefaults } from "@app/hooks/tools/changeMetadata/useChangeMetadataParameters"; +import OverlayPdfsSettings from "@app/components/tools/overlayPdfs/OverlayPdfsSettings"; +import { defaultParameters as overlayDefaults } from "@app/hooks/tools/overlayPdfs/useOverlayPdfsParameters"; import { PipelineStepSettings } from "@portal/components/pipelines/PipelineStepSettings"; // Override only useTranslation; keep the rest of react-i18next (initReactI18next et al.) real, so @@ -21,6 +38,7 @@ vi.mock("react-i18next", async (importOriginal) => ({ ...(await importOriginal()), useTranslation: () => ({ t: (key: string, fallback?: string) => fallback ?? key, + i18n: { language: "en-US", changeLanguage: vi.fn() }, }), })); @@ -63,6 +81,32 @@ const convertRegistry = { }, } as unknown as Partial; +// The real Change Metadata automation settings. Its editor variant auto-prefills the +// form from the open document via useViewer; that path is now gated on a ViewerProvider +// so it renders here (the portal mounts none) instead of crashing on useViewer. +const changeMetadataStep = { + support: "editable", + toolId: "changeMetadata", + params: changeMetadataDefaults, +} as unknown as WorkingToolStep; + +const changeMetadataRegistry = { + changeMetadata: { automationSettings: ChangeMetadataSingleStep }, +} as unknown as Partial; + +// The real Overlay PDFs automation settings. Its overlay-file picker uses the +// editor FilesModal when present; that read is now optional so the portal (which +// mounts no FilesModalProvider) renders a plain file input instead of crashing. +const overlayStep = { + support: "editable", + toolId: "overlayPdfs", + params: overlayDefaults, +} as unknown as WorkingToolStep; + +const overlayRegistry = { + overlayPdfs: { automationSettings: OverlayPdfsSettings }, +} as unknown as Partial; + describe("PipelineStepSettings", () => { it("renders reused editor tool settings (which use the shared Tooltip) without app-wide Preferences/Sidebar providers", () => { expect(() => @@ -94,6 +138,36 @@ describe("PipelineStepSettings", () => { expect(screen.getByText(/Convert from/)).toBeInTheDocument(); }); + it("renders the Change Metadata tool's fields in the portal, with no ViewerProvider mounted", () => { + expect(() => + render( + + {}} + /> + , + ), + ).not.toThrow(); + expect(screen.getByText("Standard Metadata")).toBeInTheDocument(); + }); + + it("renders the Overlay PDFs tool's fields in the portal, with no FilesModalProvider mounted", () => { + expect(() => + render( + + {}} + /> + , + ), + ).not.toThrow(); + expect(screen.getByText("Overlay Mode")).toBeInTheDocument(); + }); + // Reproduces the convert-in-pipeline bug: picking a source format fires several onParameterChange // calls in one tick (set fromExtension, auto-target, reset options). If each rebuilt from the // step snapshot captured at render they'd clobber each other and the earlier field would be lost. @@ -148,3 +222,91 @@ describe("PipelineStepSettings", () => { }); }); }); + +// Records a render crash and swallows it (renders nothing), so one broken tool is attributed by id +// instead of aborting the whole sweep - mirroring the portal's own ErrorBoundary around the builder. +class CaptureBoundary extends Component< + { onError: (error: Error) => void; children: ReactNode }, + { failed: boolean } +> { + state = { failed: false }; + static getDerivedStateFromError() { + return { failed: true }; + } + componentDidCatch(error: Error) { + this.props.onError(error); + } + render() { + return this.state.failed ? null : this.props.children; + } +} + +// Automated version of the manual "add every tool" sweep: render each tool's real automation +// settings in a portal-only context (the same Preferences + Sidebar + Suspense wrappers +// PipelineStepSettings uses, and NO editor providers) and fail listing any that throw. This is the +// guard that would have caught Change Metadata (useViewer) and Overlay PDFs (useFilesModalContext). +describe("PipelineStepSettings: every tool's settings render in the portal", () => { + it("renders each tool's automation settings without throwing", async () => { + const { result } = renderHook(() => useTranslatedToolCatalog()); + const catalog = result.current.allTools; + // getExecutableTools is exactly what PipelineBuilder feeds its "Add a tool" picker, so this + // sweeps precisely the tools a user can add. Narrow to "editable" (renders a settings + // component); "noSettings"/"unsupported" steps show a Banner instead and can't crash. + const editableTools = getExecutableTools(catalog) + .filter((tool) => tool.support === "editable") + .map((tool) => [tool.toolId, catalog[tool.toolId]] as const) + .filter(([, entry]) => Boolean(entry?.automationSettings)); + // Guard against the filter silently matching nothing (e.g. a registry-shape change). + expect(editableTools.length).toBeGreaterThan(10); + + const failures: { toolId: string; message: string }[] = []; + + for (const [toolId, entry] of editableTools) { + const Settings = entry.automationSettings as ComponentType< + ToolAutomationSettingsProps + >; + const params = (entry.operationConfig?.defaultParameters ?? + {}) as ErasedToolParams; + + const caught: { error: Error | null } = { error: null }; + // The sentinel sibling commits only once the lazy Settings actually renders, so we wait for a + // real render (or a caught throw) - not just the providers' wrapper DOM. + const { unmount } = render( + + + + { + caught.error = error; + }} + > + + {}} + disabled={false} + /> + + + + + + , + ); + + await waitFor(() => + expect( + caught.error !== null || + screen.queryByTestId(`rendered-${toolId}`) !== null, + ).toBe(true), + ); + + if (caught.error) { + failures.push({ toolId, message: caught.error.message }); + } + unmount(); + } + + expect(failures).toEqual([]); + }, 30000); +});