From 665a2e71778a4bd3a9927507c563e2a280cfad1d Mon Sep 17 00:00:00 2001 From: Armando Anaya Date: Tue, 25 Aug 2026 09:18:46 -0700 Subject: [PATCH] fix(ui): keep an unnamed class out of the schema removal preview, one at a time Deleting a class previews the remaining draft through POST /schema/preview, whose body is publish-shaped and refuses a blank name. With two unnamed classes the leftover one turned every delete into a 422 alert. A class with no name was never published and cannot carry annotations, so removing it is a local edit; removing a named class previews the named classes only. Add class now adds nothing while a class is still unnamed: it selects that class and says so, the remedy Save already uses for the same blank. --- docs/content/ui.md | 9 +++++ frontend/ui-core/src/screens/SchemaEditor.tsx | 21 +++++++++++- .../ui-core/src/screens/schemaDraft.test.tsx | 30 ++++++++++++++++ frontend/ui-core/src/screens/screens.test.tsx | 34 +++++++++++++++++++ 4 files changed, 93 insertions(+), 1 deletion(-) diff --git a/docs/content/ui.md b/docs/content/ui.md index 019e1111..90272490 100644 --- a/docs/content/ui.md +++ b/docs/content/ui.md @@ -986,6 +986,15 @@ how many classes narrow and states that nothing already labeled is invalidated. It also names what the narrowing defers: a batch still open on the outgoing version keeps writing the class that was dropped, and those labels block a release rather than this save. +**A class that has no name yet takes neither path.** The draft may hold one - naming a +class is typed work, and the draft exists so typed work survives - but the published +contract refuses a blank name, so the preview refuses it too. The editor therefore keeps +an unnamed class out of every preview body: removing one is a local edit with no request, +and removing a *named* class while one waits for a name previews the named classes only. +The same rule bounds how many can wait: **Add class** adds nothing while a class is still +unnamed, and instead selects that class and says so - the remedy Save already uses for the +same blank, not a greyed button. + That does not demote the 409. Nothing is locked between a preview and the publish, so somebody can label a class in the gap and turn a preview that looked safe into a refusal - which is why the publish's own refusal stays authoritative, and why it renders through the diff --git a/frontend/ui-core/src/screens/SchemaEditor.tsx b/frontend/ui-core/src/screens/SchemaEditor.tsx index 0f7b7fbe..069f718e 100644 --- a/frontend/ui-core/src/screens/SchemaEditor.tsx +++ b/frontend/ui-core/src/screens/SchemaEditor.tsx @@ -640,6 +640,16 @@ export function SchemaEditor({ function addClass(): void { if (saveInFlight.current || preview.isPending) return; + // One unnamed class at a time: a second one is indistinguishable from the + // first, and `save` would refuse the draft anyway. Same remedy as there — + // land on the class that still needs a name rather than grey the button. + const blank = classes.findIndex((declared) => declared.name.trim() === ""); + if (blank !== -1) { + setSelected(blank); + setFilter(""); + toast("Name the new class first"); + return; + } edit([...classes, { name: "", geometries: ["bbox"], color: null, attributes: [] }]); // Selected, and the filter cleared — a new class has an empty name, so any // filter at all would hide the row that was just created. @@ -658,8 +668,17 @@ export function SchemaEditor({ async function requestRemoveClass(index: number): Promise { if (saveInFlight.current || preview.isPending) return; + // A class still unnamed was never published, so nothing can carry it and no + // preview is owed; it is also not a class `POST /preview` accepts, so it is + // left out of the candidate the way `save` would refuse to send it. + if (classes[index]?.name.trim() === "") { + removeClass(index); + return; + } setFlow({ kind: "checking-removal" }); - const candidate = classes.filter((_, position) => position !== index); + const candidate = classes.filter( + (declared, position) => position !== index && declared.name.trim() !== "", + ); try { const previewed = await preview.mutateAsync({ classes: candidate }); if (previewed.is_refused) { diff --git a/frontend/ui-core/src/screens/schemaDraft.test.tsx b/frontend/ui-core/src/screens/schemaDraft.test.tsx index 7a358e08..417e3061 100644 --- a/frontend/ui-core/src/screens/schemaDraft.test.tsx +++ b/frontend/ui-core/src/screens/schemaDraft.test.tsx @@ -663,6 +663,36 @@ describe("saving twice with nothing edited in between", () => { * merged, and a publish always carries what was actually typed, including the * keystroke still inside the debounce window when Save was pressed. */ +describe("one unnamed class at a time", () => { + it("refuses a second Add and lands on the class still waiting for a name", async () => { + render( + mount( + <> + + + , + ), + ); + await screen.findByTestId("schema-editor"); + await userEvent.click(screen.getByTestId("add-class")); + await userEvent.click(screen.getByTestId("class-list").querySelectorAll("button")[0]); + + await userEvent.click(screen.getByTestId("add-class")); + + expect(screen.getByTestId("class-list").querySelectorAll("button")).toHaveLength( + CLASSES.length + 1, + ); + expect(await screen.findByText("Name the new class first")).toBeDefined(); + expect(screen.getByTestId(`class-name-${CLASSES.length}`)).toHaveProperty("value", ""); + + await userEvent.type(screen.getByTestId(`class-name-${CLASSES.length}`), "pedestrian"); + await userEvent.click(screen.getByTestId("add-class")); + expect(screen.getByTestId("class-list").querySelectorAll("button")).toHaveLength( + CLASSES.length + 2, + ); + }); +}); + describe("the draft lives on the server", () => { it("seeds from the server draft when there is no local one", async () => { curatedDrafts.set(PROJECT, { diff --git a/frontend/ui-core/src/screens/screens.test.tsx b/frontend/ui-core/src/screens/screens.test.tsx index aec019b5..27454753 100644 --- a/frontend/ui-core/src/screens/screens.test.tsx +++ b/frontend/ui-core/src/screens/screens.test.tsx @@ -2239,6 +2239,40 @@ describe("the schema editor's two panels", () => { expect(screen.getByTestId("class-list").querySelectorAll("button")).toHaveLength(2); expect(screen.queryByTestId("class-name-1")).not.toBeNull(); }); + + it("removes a class that was never named without asking the server", async () => { + withClasses(CLASSES); + render(mount()); + await screen.findByTestId("class-list"); + await userEvent.click(screen.getByTestId("add-class")); + const before = sent.filter((request) => request.url.endsWith("/schema/preview")).length; + + await userEvent.click(screen.getByTestId("remove-class-2")); + + await waitFor(() => + expect(screen.getByTestId("class-list").querySelectorAll("button")).toHaveLength(2), + ); + expect(sent.filter((request) => request.url.endsWith("/schema/preview"))).toHaveLength(before); + expect(screen.queryByTestId("schema-preview-error")).toBeNull(); + }); + + it("leaves an unnamed class out of the removal preview it cannot be part of", async () => { + withClasses(CLASSES); + render(mount()); + await screen.findByTestId("class-list"); + await userEvent.click(screen.getByTestId("add-class")); + + await userEvent.click(screen.getByTestId("class-list").querySelectorAll("button")[1]); + await userEvent.click(screen.getByTestId("remove-class-1")); + + await waitFor(() => + expect(screen.getByTestId("class-list").querySelectorAll("button")).toHaveLength(2), + ); + const request = sent.find((sentRequest) => sentRequest.url.endsWith("/schema/preview")); + if (request === undefined) throw new Error("Expected a schema preview request"); + expect(JSON.parse(bodies.get(request) ?? "")).toEqual({ classes: [CLASSES[0]] }); + expect(screen.queryByTestId("schema-preview-error")).toBeNull(); + }); }); describe("the project view's sections", () => {