mirror of
https://github.com/snapotter-hq/SnapOtter.git
synced 2026-08-03 07:46:42 +02:00
fix(image): image-to-pdf presets no longer 404 on 2+ files (#633)
jpg-to-pdf and its six image-to-pdf-group siblings share the base tool's registerImageToPdfRoute, which never registers into the toolRegistry the generic /batch endpoint reads from. The shared conversion-preset settings component routed any 2+-file submission to /batch regardless of tool, so these presets 404'd with `Tool "<id>" not found` past the first file, while the base image-to-pdf tool stayed unaffected because it bypasses that dispatch entirely with its own settings component. MULTI_FILE_TOOLS now includes every image-to-pdf-group preset, derived from BASE_CONFIG instead of hardcoded, and the preset settings component checks that set before choosing batch vs. a single combined request. Fixes #627
This commit is contained in:
@@ -6,6 +6,7 @@ import { ProgressCard } from "@/components/common/progress-card";
|
||||
import { useTranslation } from "@/contexts/i18n-context";
|
||||
import { useToolProcessor } from "@/hooks/use-tool-processor";
|
||||
import { format } from "@/lib/format";
|
||||
import { MULTI_FILE_TOOLS } from "@/lib/tool-display-modes";
|
||||
import { useFileStore } from "@/stores/file-store";
|
||||
|
||||
/** Target formats that honor a quality knob (lossy raster encodings). */
|
||||
@@ -46,8 +47,14 @@ export function ConversionPresetSettings() {
|
||||
|
||||
const handleProcess = () => {
|
||||
const settings = buildSettings();
|
||||
if (files.length > 1) processAllFiles(files, settings);
|
||||
else processFiles(files, settings);
|
||||
// image-to-pdf-group presets combine every file into one request (same as
|
||||
// the base tool), so they must skip the generic per-file /batch endpoint
|
||||
// that MULTI_FILE_TOOLS-gated tools never register into (issue #627).
|
||||
if (MULTI_FILE_TOOLS.has(toolId) || files.length <= 1) {
|
||||
processFiles(files, settings);
|
||||
} else {
|
||||
processAllFiles(files, settings);
|
||||
}
|
||||
};
|
||||
|
||||
const handleSubmit = (e: React.FormEvent) => {
|
||||
|
||||
@@ -227,9 +227,15 @@ for (const preset of CONVERSION_PRESETS) {
|
||||
|
||||
/**
|
||||
* Tools whose selected files all post in ONE request as repeated "file" parts.
|
||||
* Consumed by use-tool-processor; backend routes declare maxInputs.
|
||||
* Consumed by use-tool-processor and ConversionPresetSettings; backend routes
|
||||
* declare maxInputs, or (image-to-pdf group) loop over every uploaded file
|
||||
* with no cap. Conversion presets built on a combining base are added below
|
||||
* from BASE_CONFIG so a future image-to-pdf-group preset can't drift out of
|
||||
* sync the way jpg-to-pdf did (issue #627): registerImageToPdfRoute never
|
||||
* registers into the createToolRoute/registerToolProcessFn registry that the
|
||||
* generic per-file /batch route depends on, so those tools 404 there.
|
||||
*/
|
||||
export const MULTI_FILE_TOOLS: ReadonlySet<string> = new Set([
|
||||
const multiFileTools = new Set<string>([
|
||||
"create-zip",
|
||||
"merge-audio",
|
||||
"merge-csvs",
|
||||
@@ -241,3 +247,9 @@ export const MULTI_FILE_TOOLS: ReadonlySet<string> = new Set([
|
||||
"images-to-video",
|
||||
"sprite-sheet",
|
||||
]);
|
||||
for (const preset of CONVERSION_PRESETS) {
|
||||
if (BASE_CONFIG[preset.base].group === "image-to-pdf") {
|
||||
multiFileTools.add(preset.id);
|
||||
}
|
||||
}
|
||||
export const MULTI_FILE_TOOLS: ReadonlySet<string> = multiFileTools;
|
||||
|
||||
@@ -0,0 +1,75 @@
|
||||
import path from "node:path";
|
||||
import type { Page } from "@playwright/test";
|
||||
import { expect, test } from "./helpers";
|
||||
|
||||
const FIXTURES_DIR = path.join(process.cwd(), "tests", "fixtures", "image", "valid");
|
||||
|
||||
async function uploadFiles(page: Page, filenames: string[]): Promise<void> {
|
||||
const fileChooserPromise = page.waitForEvent("filechooser");
|
||||
await page
|
||||
.getByRole("button", { name: /upload from computer/i })
|
||||
.first()
|
||||
.click();
|
||||
const fileChooser = await fileChooserPromise;
|
||||
await fileChooser.setFiles(filenames.map((f) => path.join(FIXTURES_DIR, f)));
|
||||
await page.waitForTimeout(500);
|
||||
}
|
||||
|
||||
/**
|
||||
* Issue #627: image-to-pdf-group presets (jpg-to-pdf, png-to-pdf, ...) combine
|
||||
* every uploaded file into one PDF, the same as the base image-to-pdf tool.
|
||||
* Submitting 2+ files used to route to the generic per-file /batch endpoint,
|
||||
* which 404s with `Tool "<id>" not found` for these tools (they are
|
||||
* registered via registerImageToPdfRoute, never added to the
|
||||
* createToolRoute/registerToolProcessFn registry the batch route reads).
|
||||
*/
|
||||
test.describe("image-to-pdf-group conversion presets with multiple files (issue #627)", () => {
|
||||
test("jpg-to-pdf combines 2 JPGs into one PDF instead of failing", async ({
|
||||
loggedInPage: page,
|
||||
}) => {
|
||||
await page.goto("/image/jpg-to-pdf");
|
||||
await uploadFiles(page, ["portrait-color.jpg", "sample-photo.jpg"]);
|
||||
|
||||
await expect(page.getByText("Files (2)")).toBeVisible();
|
||||
|
||||
await page.getByTestId("preset-submit").click();
|
||||
|
||||
const download = page.getByTestId("preset-download");
|
||||
await expect(download).toBeVisible({ timeout: 15_000 });
|
||||
await expect(download).toHaveAttribute("href", /\/api\/v1\/download\/[^/]+\/images\.pdf$/);
|
||||
});
|
||||
|
||||
test("png-to-pdf combines 2 PNGs into one PDF instead of failing", async ({
|
||||
loggedInPage: page,
|
||||
}) => {
|
||||
await page.goto("/image/png-to-pdf");
|
||||
await uploadFiles(page, ["barcode.png", "portrait-isolated.png"]);
|
||||
|
||||
await expect(page.getByText("Files (2)")).toBeVisible();
|
||||
|
||||
await page.getByTestId("preset-submit").click();
|
||||
|
||||
const download = page.getByTestId("preset-download");
|
||||
await expect(download).toBeVisible({ timeout: 15_000 });
|
||||
await expect(download).toHaveAttribute("href", /\/api\/v1\/download\/[^/]+\/images\.pdf$/);
|
||||
});
|
||||
|
||||
test("jpg-to-png still batches 2 files independently (not a combine preset)", async ({
|
||||
loggedInPage: page,
|
||||
}) => {
|
||||
const batchRequest = page.waitForResponse(
|
||||
(res) => res.url().includes("/jpg-to-png/batch") && res.request().method() === "POST",
|
||||
);
|
||||
|
||||
await page.goto("/image/jpg-to-png");
|
||||
await uploadFiles(page, ["portrait-color.jpg", "sample-photo.jpg"]);
|
||||
|
||||
await expect(page.getByText("Files (2)")).toBeVisible();
|
||||
|
||||
await page.getByTestId("preset-submit").click();
|
||||
|
||||
const response = await batchRequest;
|
||||
expect(response.status()).toBe(200);
|
||||
await expect(page.getByText("Conversion complete")).toBeVisible({ timeout: 15_000 });
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,150 @@
|
||||
// @vitest-environment jsdom
|
||||
|
||||
import { cleanup, fireEvent, render, screen } from "@testing-library/react";
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
vi.mock("@/lib/image-preview", () => ({
|
||||
needsServerPreview: vi.fn(() => false),
|
||||
fetchDecodedPreview: vi.fn(() => Promise.resolve(null)),
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/analytics", () => ({
|
||||
track: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/api", () => ({
|
||||
formatHeaders: () => new Map<string, string>(),
|
||||
parseApiError: () => "error",
|
||||
}));
|
||||
|
||||
let mockToolId = "jpg-to-pdf";
|
||||
vi.mock("react-router-dom", async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import("react-router-dom")>();
|
||||
return { ...actual, useParams: () => ({ toolId: mockToolId }) };
|
||||
});
|
||||
|
||||
import { ConversionPresetSettings } from "@/components/tools/conversion-preset-settings";
|
||||
import { useFileStore } from "@/stores/file-store";
|
||||
|
||||
interface MockXhr {
|
||||
open: ReturnType<typeof vi.fn>;
|
||||
send: ReturnType<typeof vi.fn>;
|
||||
setRequestHeader: ReturnType<typeof vi.fn>;
|
||||
abort: ReturnType<typeof vi.fn>;
|
||||
upload: Record<string, unknown>;
|
||||
status: number;
|
||||
responseText: string;
|
||||
timeout: number;
|
||||
}
|
||||
|
||||
class MockEventSource {
|
||||
onmessage: ((event: MessageEvent) => void) | null = null;
|
||||
onerror: (() => void) | null = null;
|
||||
close = vi.fn();
|
||||
constructor(readonly url: string) {}
|
||||
}
|
||||
|
||||
function makeFile(name: string, type = "image/jpeg"): File {
|
||||
return new File([new ArrayBuffer(64)], name, { type });
|
||||
}
|
||||
|
||||
let xhrInstances: MockXhr[];
|
||||
let fetchMock: ReturnType<typeof vi.fn>;
|
||||
|
||||
beforeEach(() => {
|
||||
vi.stubGlobal("URL", {
|
||||
...globalThis.URL,
|
||||
createObjectURL: vi.fn(() => "blob:fake-url"),
|
||||
revokeObjectURL: vi.fn(),
|
||||
});
|
||||
useFileStore.getState().reset();
|
||||
xhrInstances = [];
|
||||
|
||||
vi.stubGlobal("EventSource", MockEventSource);
|
||||
vi.stubGlobal(
|
||||
"XMLHttpRequest",
|
||||
vi.fn(() => {
|
||||
const xhr: MockXhr = {
|
||||
status: 0,
|
||||
responseText: "",
|
||||
timeout: 0,
|
||||
upload: {},
|
||||
open: vi.fn(),
|
||||
send: vi.fn(),
|
||||
setRequestHeader: vi.fn(),
|
||||
abort: vi.fn(),
|
||||
};
|
||||
xhrInstances.push(xhr);
|
||||
return xhr;
|
||||
}),
|
||||
);
|
||||
|
||||
// Never resolves; these tests only assert the call was (or wasn't) made.
|
||||
fetchMock = vi.fn(() => new Promise(() => {}));
|
||||
vi.stubGlobal("fetch", fetchMock);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
cleanup();
|
||||
vi.unstubAllGlobals();
|
||||
});
|
||||
|
||||
/**
|
||||
* Issue #627: the jpg-to-pdf preset (and its image-to-pdf-group siblings)
|
||||
* combine every uploaded file into one PDF, the same as the base image-to-pdf
|
||||
* tool. Submitting 2+ files must stay on the single "attach all files" route,
|
||||
* never the generic per-file /batch endpoint (which 404s for these tools:
|
||||
* they are registered via registerImageToPdfRoute, not createToolRoute, so
|
||||
* they were never added to the batch-capable tool registry).
|
||||
*/
|
||||
describe("ConversionPresetSettings multi-file dispatch (issue #627)", () => {
|
||||
it("combines 2 files for jpg-to-pdf in one request instead of calling /batch", () => {
|
||||
mockToolId = "jpg-to-pdf";
|
||||
useFileStore.getState().setFiles([makeFile("a.jpg"), makeFile("b.jpg")]);
|
||||
|
||||
render(<ConversionPresetSettings />);
|
||||
fireEvent.click(screen.getByTestId("preset-submit"));
|
||||
|
||||
expect(xhrInstances).toHaveLength(1);
|
||||
expect(xhrInstances[0].open).toHaveBeenCalledWith("POST", "/api/v1/tools/image/jpg-to-pdf");
|
||||
expect(fetchMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("combines 2 files for every image-to-pdf-group preset, not just jpg-to-pdf", () => {
|
||||
mockToolId = "png-to-pdf";
|
||||
useFileStore
|
||||
.getState()
|
||||
.setFiles([makeFile("a.png", "image/png"), makeFile("b.png", "image/png")]);
|
||||
|
||||
render(<ConversionPresetSettings />);
|
||||
fireEvent.click(screen.getByTestId("preset-submit"));
|
||||
|
||||
expect(xhrInstances).toHaveLength(1);
|
||||
expect(xhrInstances[0].open).toHaveBeenCalledWith("POST", "/api/v1/tools/image/png-to-pdf");
|
||||
expect(fetchMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("still uses independent per-file batch processing for ordinary presets like jpg-to-png", () => {
|
||||
mockToolId = "jpg-to-png";
|
||||
useFileStore.getState().setFiles([makeFile("a.jpg"), makeFile("b.jpg")]);
|
||||
|
||||
render(<ConversionPresetSettings />);
|
||||
fireEvent.click(screen.getByTestId("preset-submit"));
|
||||
|
||||
expect(fetchMock).toHaveBeenCalledTimes(1);
|
||||
expect(fetchMock.mock.calls[0][0]).toBe("/api/v1/tools/image/jpg-to-png/batch");
|
||||
expect(xhrInstances).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("uses the single-file request for jpg-to-pdf when only 1 file is selected", () => {
|
||||
mockToolId = "jpg-to-pdf";
|
||||
useFileStore.getState().setFiles([makeFile("a.jpg")]);
|
||||
|
||||
render(<ConversionPresetSettings />);
|
||||
fireEvent.click(screen.getByTestId("preset-submit"));
|
||||
|
||||
expect(xhrInstances).toHaveLength(1);
|
||||
expect(xhrInstances[0].open).toHaveBeenCalledWith("POST", "/api/v1/tools/image/jpg-to-pdf");
|
||||
expect(fetchMock).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,23 @@
|
||||
import { BASE_CONFIG, CONVERSION_PRESETS } from "@snapotter/shared";
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { MULTI_FILE_TOOLS } from "@/lib/tool-display-modes";
|
||||
|
||||
/**
|
||||
* Issue #627: image-to-pdf-group presets (jpg-to-pdf, png-to-pdf, ...) combine
|
||||
* every uploaded file into one PDF via their own multi-file route, the same
|
||||
* as the base image-to-pdf tool. They must stay off the generic per-file
|
||||
* batch path, which never registers them (registerImageToPdfRoute bypasses
|
||||
* the createToolRoute/registerToolProcessFn registry the batch route depends
|
||||
* on) and 404s with `Tool "<id>" not found` if reached with 2+ files.
|
||||
*/
|
||||
describe("MULTI_FILE_TOOLS drift (issue #627)", () => {
|
||||
it("includes every conversion preset whose base combines inputs into one request", () => {
|
||||
for (const preset of CONVERSION_PRESETS) {
|
||||
if (BASE_CONFIG[preset.base].group !== "image-to-pdf") continue;
|
||||
expect(
|
||||
MULTI_FILE_TOOLS.has(preset.id),
|
||||
`preset "${preset.id}" (base "${preset.base}") combines inputs but is missing from MULTI_FILE_TOOLS`,
|
||||
).toBe(true);
|
||||
}
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user