From 8150d16b6f89a20b1ead03aa4b4dde4b3ce8a4dc Mon Sep 17 00:00:00 2001 From: James Brunton Date: Wed, 8 Jul 2026 13:47:11 +0100 Subject: [PATCH] Support bidirectional mapping for Change Metadata (#6906) # Description of Changes The Change Metadata tool was missed from the bidirectional mappings added in #6867. This PR adds it to the list of supported tools. --- .../useChangeMetadataOperation.test.ts | 86 ++++++++- .../useChangeMetadataOperation.ts | 172 +++++++++++++----- .../tools/shared/migratedToolMappers.test.ts | 32 +++- 3 files changed, 234 insertions(+), 56 deletions(-) diff --git a/frontend/editor/src/core/hooks/tools/changeMetadata/useChangeMetadataOperation.test.ts b/frontend/editor/src/core/hooks/tools/changeMetadata/useChangeMetadataOperation.test.ts index 6444f5f05d..0a3bf0e78a 100644 --- a/frontend/editor/src/core/hooks/tools/changeMetadata/useChangeMetadataOperation.test.ts +++ b/frontend/editor/src/core/hooks/tools/changeMetadata/useChangeMetadataOperation.test.ts @@ -1,5 +1,12 @@ -import { buildChangeMetadataFormData } from "@app/hooks/tools/changeMetadata/useChangeMetadataOperation"; -import { ChangeMetadataParameters } from "@app/hooks/tools/changeMetadata/useChangeMetadataParameters"; +import { + buildChangeMetadataFormData, + changeMetadataToApiParams, + changeMetadataFromApiParams, +} from "@app/hooks/tools/changeMetadata/useChangeMetadataOperation"; +import { + ChangeMetadataParameters, + defaultParameters, +} from "@app/hooks/tools/changeMetadata/useChangeMetadataParameters"; import { TrappedStatus } from "@app/types/metadata"; import { describe, expect, test } from "vitest"; @@ -142,3 +149,78 @@ describe("buildChangeMetadataFormData", () => { expect(formData.get("allRequestParams[customValue1]")).toBe("Engineering"); }); }); + +describe("changeMetadata mappers", () => { + const fullApi: Parameters[0] = { + title: "Title", + author: "Author", + subject: "Subject", + keywords: "a, b", + creator: "Creator", + producer: "Producer", + creationDate: "2024/01/15 10:30:00", + modificationDate: "2024/02/20 14:05:09", + trapped: "True", + deleteAll: false, + allRequestParams: { + customKey1: "Department", + customValue1: "Engineering", + customKey2: "Project", + customValue2: "Falcon", + }, + }; + + test("round-trips a full request through fromApiParams and back", () => { + const roundTripped = changeMetadataToApiParams({ + ...defaultParameters, + ...changeMetadataFromApiParams(fullApi), + }); + + expect(roundTripped).toEqual(fullApi); + }); + + test("reconstructs dates in local time and clears absent ones", () => { + const params = changeMetadataFromApiParams(fullApi); + expect(params.creationDate).toEqual(new Date(2024, 0, 15, 10, 30, 0)); + expect(params.modificationDate).toEqual(new Date(2024, 1, 20, 14, 5, 9)); + + const cleared = changeMetadataFromApiParams({ + creationDate: "", + modificationDate: undefined, + }); + expect(cleared.creationDate).toBeNull(); + expect(cleared.modificationDate).toBeNull(); + }); + + test.each([TrappedStatus.TRUE, TrappedStatus.FALSE, TrappedStatus.UNKNOWN])( + "round-trips trapped=%s", + (trapped) => { + const api = changeMetadataToApiParams({ ...defaultParameters, trapped }); + expect(api.trapped).toBe(trapped); + expect(changeMetadataFromApiParams(api).trapped).toBe(trapped); + }, + ); + + test("falls back to the default for an unrecognised trapped value", () => { + const params = changeMetadataFromApiParams({ + trapped: "Bogus", + } as unknown as Parameters[0]); + expect(params.trapped).toBe(defaultParameters.trapped); + }); + + test("reconstructs custom metadata, tolerating non-contiguous indices", () => { + const params = changeMetadataFromApiParams({ + allRequestParams: { + customKey1: "Department", + customValue1: "Engineering", + customKey3: "Project", + customValue3: "Falcon", + }, + }); + + expect(params.customMetadata).toEqual([ + { key: "Department", value: "Engineering", id: "custom1" }, + { key: "Project", value: "Falcon", id: "custom3" }, + ]); + }); +}); diff --git a/frontend/editor/src/core/hooks/tools/changeMetadata/useChangeMetadataOperation.ts b/frontend/editor/src/core/hooks/tools/changeMetadata/useChangeMetadataOperation.ts index 6b0cd8b5d5..97a8285194 100644 --- a/frontend/editor/src/core/hooks/tools/changeMetadata/useChangeMetadataOperation.ts +++ b/frontend/editor/src/core/hooks/tools/changeMetadata/useChangeMetadataOperation.ts @@ -3,13 +3,22 @@ import { useToolOperation, defineSingleFileTool, } from "@app/hooks/tools/shared/useToolOperation"; +import { + objectToFormData, + type ToolApiParams, + type ToolEndpoint, +} from "@app/hooks/tools/shared/toolApiMapping"; import { createStandardErrorHandler } from "@app/utils/toolErrorHandler"; +import { TrappedStatus, CustomMetadataEntry } from "@app/types/metadata"; import { ChangeMetadataParameters, defaultParameters, } from "@app/hooks/tools/changeMetadata/useChangeMetadataParameters"; -// Helper function to format Date object to string +const ENDPOINT = "/api/v1/misc/update-metadata" satisfies ToolEndpoint; +type ChangeMetadataApiParams = ToolApiParams[typeof ENDPOINT]; + +// Backend date format (yyyy/MM/dd HH:mm:ss), in local time to mirror the parser. const formatDateForBackend = (date: Date | null): string => { if (!date) return ""; const year = date.getFullYear(); @@ -21,64 +30,135 @@ const formatDateForBackend = (date: Date | null): string => { return `${year}/${month}/${day} ${hours}:${minutes}:${seconds}`; }; +// Inverse of formatDateForBackend; returns null for empty or unparseable input. +const parseDateFromBackend = (value: string | undefined): Date | null => { + if (!value) return null; + const match = /^(\d{4})\/(\d{2})\/(\d{2}) (\d{2}):(\d{2}):(\d{2})$/.exec( + value, + ); + if (!match) return null; + const [, year, month, day, hours, minutes, seconds] = match; + return new Date( + Number(year), + Number(month) - 1, + Number(day), + Number(hours), + Number(minutes), + Number(seconds), + ); +}; + +// Custom metadata is carried in the request's allRequestParams map as paired +// customKey/customValue entries; the backend rejoins them by the shared +// index N (see MetadataController). Only entries with a non-blank key and value +// are sent, and they are re-numbered from 1 so the indices stay contiguous. +const buildCustomMetadataMap = ( + customMetadata: CustomMetadataEntry[], +): Record | undefined => { + const validEntries = customMetadata.filter( + (entry) => entry.key.trim() && entry.value.trim(), + ); + if (validEntries.length === 0) return undefined; + + const map: Record = {}; + validEntries.forEach((entry, index) => { + const n = index + 1; + map[`customKey${n}`] = entry.key.trim(); + map[`customValue${n}`] = entry.value.trim(); + }); + return map; +}; + +// Rebuild the UI's custom metadata list from the allRequestParams map by pairing +// customKey/customValue on their shared index N. Mirrors the backend +// (MetadataController), which pairs by index across all entries and does not +// assume the indices are contiguous, so a non-contiguous stored map round-trips. +const parseCustomMetadataMap = ( + allRequestParams: ChangeMetadataApiParams["allRequestParams"], +): CustomMetadataEntry[] => { + if (!allRequestParams) return []; + return Object.keys(allRequestParams) + .map((key) => /^customKey(\d+)$/.exec(key)?.[1]) + .filter((n): n is string => n !== undefined) + .map(Number) + .sort((a, b) => a - b) + .map((n) => ({ + key: allRequestParams[`customKey${n}`] ?? "", + value: allRequestParams[`customValue${n}`] ?? "", + id: `custom${n}`, + })); +}; + +// Map the backend's trapped string onto the UI enum, validating against the +// actual enum values so an unrecognised value falls back to the default instead +// of being force-cast. +const parseTrapped = (value: string | undefined): TrappedStatus => + Object.values(TrappedStatus).find((status) => status === value) ?? + defaultParameters.trapped; + +// Convert the tool's UI parameters into the update-metadata request body. The +// return type is the generated backend model, so a spec change that renames or +// drops a field breaks the build here. +export const changeMetadataToApiParams = ( + parameters: ChangeMetadataParameters, +): ChangeMetadataApiParams => ({ + title: parameters.title, + author: parameters.author, + subject: parameters.subject, + keywords: parameters.keywords, + creator: parameters.creator, + producer: parameters.producer, + creationDate: formatDateForBackend(parameters.creationDate), + modificationDate: formatDateForBackend(parameters.modificationDate), + trapped: parameters.trapped, + deleteAll: parameters.deleteAll, + allRequestParams: buildCustomMetadataMap(parameters.customMetadata), +}); + +// Reconstruct the tool's UI parameters from an update-metadata request body, so +// a stored or AI-authored step can be re-rendered in the settings UI. +export const changeMetadataFromApiParams = ( + apiParams: ChangeMetadataApiParams, +): Partial => ({ + title: apiParams.title ?? defaultParameters.title, + author: apiParams.author ?? defaultParameters.author, + subject: apiParams.subject ?? defaultParameters.subject, + keywords: apiParams.keywords ?? defaultParameters.keywords, + creator: apiParams.creator ?? defaultParameters.creator, + producer: apiParams.producer ?? defaultParameters.producer, + creationDate: parseDateFromBackend(apiParams.creationDate), + modificationDate: parseDateFromBackend(apiParams.modificationDate), + trapped: parseTrapped(apiParams.trapped), + deleteAll: apiParams.deleteAll ?? defaultParameters.deleteAll, + customMetadata: parseCustomMetadataMap(apiParams.allRequestParams), +}); + // Static function that can be used by both the hook and automation executor export const buildChangeMetadataFormData = ( parameters: ChangeMetadataParameters, file: File, ): FormData => { - const formData = new FormData(); - formData.append("fileInput", file); - - // Standard metadata fields - formData.append("title", parameters.title || ""); - formData.append("author", parameters.author || ""); - formData.append("subject", parameters.subject || ""); - formData.append("keywords", parameters.keywords || ""); - formData.append("creator", parameters.creator || ""); - formData.append("producer", parameters.producer || ""); - - // Date fields - convert Date objects to strings - formData.append( - "creationDate", - formatDateForBackend(parameters.creationDate), - ); - formData.append( - "modificationDate", - formatDateForBackend(parameters.modificationDate), - ); - - // Trapped status - formData.append("trapped", parameters.trapped || ""); - - // Delete all metadata flag - formData.append("deleteAll", parameters.deleteAll.toString()); - - // Custom metadata - backend expects them as values to 'allRequestParams[customKeyX/customValueX]' - let keyNumber = 0; - if (parameters.customMetadata && Array.isArray(parameters.customMetadata)) { - parameters.customMetadata.forEach((entry) => { - if (entry.key.trim() && entry.value.trim()) { - keyNumber += 1; - formData.append( - `allRequestParams[customKey${keyNumber}]`, - entry.key.trim(), - ); - formData.append( - `allRequestParams[customValue${keyNumber}]`, - entry.value.trim(), - ); - } - }); + // allRequestParams is a Spring-bound map: objectToFormData only serializes + // primitives, so the scalar fields go through it and the map is flattened into + // allRequestParams[] form fields separately. + const { allRequestParams, ...scalarParams } = + changeMetadataToApiParams(parameters); + const formData = objectToFormData(scalarParams, { fileInput: file }); + for (const [key, value] of Object.entries(allRequestParams ?? {})) { + if (value !== undefined) { + formData.append(`allRequestParams[${key}]`, value); + } } - return formData; }; // Static configuration object export const changeMetadataOperationConfig = defineSingleFileTool({ buildFormData: buildChangeMetadataFormData, + toApiParams: changeMetadataToApiParams, + fromApiParams: changeMetadataFromApiParams, operationType: "changeMetadata", - endpoint: "/api/v1/misc/update-metadata", + endpoint: ENDPOINT, defaultParameters, }); diff --git a/frontend/editor/src/core/hooks/tools/shared/migratedToolMappers.test.ts b/frontend/editor/src/core/hooks/tools/shared/migratedToolMappers.test.ts index be81fd1884..d562384921 100644 --- a/frontend/editor/src/core/hooks/tools/shared/migratedToolMappers.test.ts +++ b/frontend/editor/src/core/hooks/tools/shared/migratedToolMappers.test.ts @@ -1,5 +1,8 @@ import { describe, expect, test } from "vitest"; -import { type RegistryToolOperationConfig } from "@app/hooks/tools/shared/toolOperationTypes"; +import { + ToolType, + type RegistryToolOperationConfig, +} from "@app/hooks/tools/shared/toolOperationTypes"; import { objectToFormData } from "@app/hooks/tools/shared/toolApiMapping"; // Pilot tools. @@ -17,6 +20,7 @@ import { adjustPageScaleOperationConfig } from "@app/hooks/tools/adjustPageScale import { autoRenameOperationConfig } from "@app/hooks/tools/autoRename/useAutoRenameOperation"; import { bookletImpositionOperationConfig } from "@app/hooks/tools/bookletImposition/useBookletImpositionOperation"; import { certSignOperationConfig } from "@app/hooks/tools/certSign/useCertSignOperation"; +import { changeMetadataOperationConfig } from "@app/hooks/tools/changeMetadata/useChangeMetadataOperation"; import { changePermissionsOperationConfig } from "@app/hooks/tools/changePermissions/useChangePermissionsOperation"; import { cropOperationConfig } from "@app/hooks/tools/crop/useCropOperation"; import { editTableOfContentsOperationConfig } from "@app/hooks/tools/editTableOfContents/useEditTableOfContentsOperation"; @@ -57,6 +61,7 @@ const MIGRATED_CONFIGS = [ autoRenameOperationConfig, bookletImpositionOperationConfig, certSignOperationConfig, + changeMetadataOperationConfig, changePermissionsOperationConfig, cropOperationConfig, editTableOfContentsOperationConfig, @@ -102,15 +107,26 @@ describe("migrated tool mappers (sweep)", () => { expect(config.toApiParams).toBeDefined(); expect(config.fromApiParams).toBeDefined(); - // toApiParams(defaults) must produce a body objectToFormData can serialize - // (i.e. only primitives / arrays of primitives). A mapper that leaked a - // structured value would throw here. + // Serialize the defaults through the tool's own buildFormData - the real + // path the executor uses - so a tool whose toApiParams carries a structured + // field that buildFormData flattens itself (e.g. changeMetadata's + // allRequestParams map) is exercised too, not just tools whose mapper + // output is directly objectToFormData-able. Custom tools have no + // buildFormData, so fall back to serializing the mapper output directly. const params = config.defaultParameters ?? FALLBACK_PARAMS[config.operationType] ?? {}; - const apiParams = config.toApiParams!(params); - expect(() => - objectToFormData(apiParams, { fileInput: file }), - ).not.toThrow(); + if (config.toolType === ToolType.multiFile) { + const build = config.buildFormData; + expect(() => build(params, [file])).not.toThrow(); + } else if (config.toolType === ToolType.singleFile) { + const build = config.buildFormData; + expect(() => build(params, file)).not.toThrow(); + } else { + const toApiParams = config.toApiParams!; + expect(() => + objectToFormData(toApiParams(params), { fileInput: file }), + ).not.toThrow(); + } }, ); });