Fix tools which crash in the Pipelines page (#7538)

# Description of Changes
Overlay PDFs and Change Metadata both crashed in the Processor because
they required `FilesModalContext` and `ViewerContext` respectively.
Neither of those contexts make sense to provide in the Processor because
there are no files in context and there is no Viewer, so redesign both
tool settings to only optionally require these contexts. Their behaviour
is unchanged in the Editor but they now work in the Processor (just
without the extra info about the active files, since there are none).

Also hooks up the Reorganise Pages settings so that it can be used from
Automate. The component already existed but just wasn't being used,
which just looks like an oversight.
This commit is contained in:
James Brunton
2026-08-18 13:08:23 +00:00
committed by GitHub
parent a14eec94ec
commit fb70fc13da
6 changed files with 323 additions and 79 deletions
@@ -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 (
<Stack gap="md">
{/* Delete All */}
<Stack gap="md">
<Text size="sm" fw={500}>
{t("changeMetadata.deleteAll.label", "Delete All Metadata")}
</Text>
<DeleteAllStep
parameters={parameters}
<>
{hasViewerContext && (
<MetadataPrefill
onParameterChange={onParameterChange}
disabled={disabled}
onExtractingChange={setIsExtractingMetadata}
/>
</Stack>
<Divider />
{/* Standard Metadata Fields */}
)}
<Stack gap="md">
<Text size="sm" fw={500}>
{t("changeMetadata.standardFields.title", "Standard Metadata")}
</Text>
<StandardMetadataStep
parameters={parameters}
onParameterChange={onParameterChange}
disabled={fieldsDisabled}
/>
{/* Delete All */}
<Stack gap="md">
<Text size="sm" fw={500}>
{t("changeMetadata.deleteAll.label", "Delete All Metadata")}
</Text>
<DeleteAllStep
parameters={parameters}
onParameterChange={onParameterChange}
disabled={disabled}
/>
</Stack>
<Divider />
{/* Standard Metadata Fields */}
<Stack gap="md">
<Text size="sm" fw={500}>
{t("changeMetadata.standardFields.title", "Standard Metadata")}
</Text>
<StandardMetadataStep
parameters={parameters}
onParameterChange={onParameterChange}
disabled={fieldsDisabled}
/>
</Stack>
<Divider />
{/* Document Dates */}
<Stack gap="md">
<Text size="sm" fw={500}>
{t("changeMetadata.dates.title", "Document Dates")}
</Text>
<DocumentDatesStep
parameters={parameters}
onParameterChange={onParameterChange}
disabled={fieldsDisabled}
/>
</Stack>
<Divider />
{/* Advanced Options */}
<Stack gap="md">
<Text size="sm" fw={500}>
{t("changeMetadata.advanced.title", "Advanced Options")}
</Text>
<AdvancedOptionsStep
parameters={parameters}
onParameterChange={onParameterChange}
disabled={fieldsDisabled}
addCustomMetadata={addCustomMetadata}
removeCustomMetadata={removeCustomMetadata}
updateCustomMetadata={updateCustomMetadata}
/>
</Stack>
</Stack>
<Divider />
{/* Document Dates */}
<Stack gap="md">
<Text size="sm" fw={500}>
{t("changeMetadata.dates.title", "Document Dates")}
</Text>
<DocumentDatesStep
parameters={parameters}
onParameterChange={onParameterChange}
disabled={fieldsDisabled}
/>
</Stack>
<Divider />
{/* Advanced Options */}
<Stack gap="md">
<Text size="sm" fw={500}>
{t("changeMetadata.advanced.title", "Advanced Options")}
</Text>
<AdvancedOptionsStep
parameters={parameters}
onParameterChange={onParameterChange}
disabled={fieldsDisabled}
addCustomMetadata={addCustomMetadata}
removeCustomMetadata={removeCustomMetadata}
updateCustomMetadata={updateCustomMetadata}
/>
</Stack>
</Stack>
</>
);
};
@@ -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 (
<Stack gap="md">
<Stack gap="xs">
@@ -183,17 +201,30 @@ export default function OverlayPdfsSettings({
<Text size="sm" fw={500}>
{t("overlay-pdfs.overlayFiles.label", "Overlay Files")}
</Text>
<Button
size="sm"
onClick={handleOpenOverlayFilesModal}
disabled={disabled}
leftSection={<LocalIcon icon="add" width="14" height="14" />}
fullWidth
>
{parameters.overlayFiles?.length > 0
? t("overlay-pdfs.overlayFiles.addMore", "Add more PDFs...")
: t("overlay-pdfs.overlayFiles.placeholder", "Choose PDF(s)...")}
</Button>
{filesModal ? (
<Button
size="sm"
onClick={handleOpenOverlayFilesModal}
disabled={disabled}
leftSection={<LocalIcon icon="add" width="14" height="14" />}
fullWidth
>
{overlayFilesButtonLabel}
</Button>
) : (
<FilePicker
multiple
accept="application/pdf"
onChange={appendOverlayFiles}
resetRef={resetOverlayPicker}
size="sm"
disabled={disabled}
leftSection={<LocalIcon icon="add" width="14" height="14" />}
fullWidth
>
{overlayFilesButtonLabel}
</FilePicker>
)}
{parameters.overlayFiles?.length > 0 &&
(() => {
@@ -41,7 +41,9 @@ interface FilesModalContextType {
setOnModalClose: (callback: () => void) => void;
}
const FilesModalContext = createContext<FilesModalContextType | null>(null);
export const FilesModalContext = createContext<FilesModalContextType | null>(
null,
);
export const FilesModalProvider: React.FC<{ children: React.ReactNode }> = ({
children,
@@ -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();
});
});
@@ -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: (
@@ -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<typeof import("react-i18next")>()),
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<ToolRegistry>;
// 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<ToolRegistry>;
// 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<ToolRegistry>;
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(
<PortalTestProviders>
<PipelineStepSettings
step={changeMetadataStep}
registry={changeMetadataRegistry}
onChange={() => {}}
/>
</PortalTestProviders>,
),
).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(
<PortalTestProviders>
<PipelineStepSettings
step={overlayStep}
registry={overlayRegistry}
onChange={() => {}}
/>
</PortalTestProviders>,
),
).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<ErasedToolParams>
>;
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(
<PortalTestProviders>
<PreferencesProvider>
<SidebarProvider>
<CaptureBoundary
onError={(error) => {
caught.error = error;
}}
>
<Suspense fallback={null}>
<Settings
parameters={params}
onParameterChange={() => {}}
disabled={false}
/>
<span data-testid={`rendered-${toolId}`} />
</Suspense>
</CaptureBoundary>
</SidebarProvider>
</PreferencesProvider>
</PortalTestProviders>,
);
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);
});