From 2ac2b94d313d591572ef4ffbe220b01b00b3e61b Mon Sep 17 00:00:00 2001 From: Aleksei Sviridkin Date: Mon, 1 Jun 2026 20:20:55 +0300 Subject: [PATCH] refactor(forms): validate the schema form before page-level checks The backup create pages ran SchemaForm.validate() only after their manual required-field alerts, so a schema-required field left empty exited via an alert without RJSF ever rendering its inline errors. Run validate() right after the name check, before the page-level alerts, so inline errors show consistently and the alerts only cover page-only requirements. Assisted-By: Claude Signed-off-by: Aleksei Sviridkin --- .../src/routes/BackupCreatePage.test.tsx | 23 ++++++++++++++++++- apps/console/src/routes/BackupCreatePage.tsx | 8 +++---- .../src/routes/BackupJobCreatePage.test.tsx | 19 ++++++++++++++- .../src/routes/BackupJobCreatePage.tsx | 8 +++---- .../src/routes/BackupPlanCreatePage.test.tsx | 19 ++++++++++++++- .../src/routes/BackupPlanCreatePage.tsx | 8 +++---- .../BackupRestoreJobCreatePage.test.tsx | 19 ++++++++++++++- .../src/routes/BackupRestoreJobCreatePage.tsx | 8 +++---- 8 files changed, 92 insertions(+), 20 deletions(-) diff --git a/apps/console/src/routes/BackupCreatePage.test.tsx b/apps/console/src/routes/BackupCreatePage.test.tsx index 0612d4d..7df6883 100644 --- a/apps/console/src/routes/BackupCreatePage.test.tsx +++ b/apps/console/src/routes/BackupCreatePage.test.tsx @@ -15,6 +15,8 @@ const h = vi.hoisted(() => ({ strategyRef: { kind: "Strategy", name: "s3" }, takenAt: "2024-01-01T00:00:00Z", }, + // Spec the mocked SchemaForm emits into the page on mount; reset per test. + emitSpec: {} as unknown, })) vi.mock("../lib/tenant-context.tsx", () => ({ @@ -38,7 +40,7 @@ vi.mock("../components/SchemaForm.tsx", () => ({ function MockSchemaForm({ onChange }, ref) { useImperativeHandle(ref, () => ({ validate: () => h.validateReturn })) useEffect(() => { - onChange(h.validSpec) + onChange(h.emitSpec) }, [onChange]) return null }, @@ -60,6 +62,7 @@ describe("BackupCreatePage submit validation gate", () => { h.createMutateAsync.mockReset() h.createMutateAsync.mockResolvedValue({}) h.validateReturn = true + h.emitSpec = h.validSpec }) it("does not POST when the form fails RJSF validation", async () => { @@ -83,4 +86,22 @@ describe("BackupCreatePage submit validation gate", () => { expect(h.createMutateAsync).toHaveBeenCalledTimes(1) }) + + it("runs RJSF validation before the page-level required-field alerts", async () => { + // Form is RJSF-invalid AND a page-required field is missing. The gate must + // fire first, so no alert() is shown — proving validate() runs before the + // manual checks (under the old ordering the applicationRef alert fired). + h.validateReturn = false + h.emitSpec = {} + const alertSpy = vi.spyOn(window, "alert").mockImplementation(() => {}) + const user = userEvent.setup() + renderPage() + + await user.type(screen.getByRole("textbox"), "my-backup") + await user.click(screen.getByRole("button", { name: /create/i })) + + expect(h.createMutateAsync).not.toHaveBeenCalled() + expect(alertSpy).not.toHaveBeenCalled() + alertSpy.mockRestore() + }) }) diff --git a/apps/console/src/routes/BackupCreatePage.tsx b/apps/console/src/routes/BackupCreatePage.tsx index 0be40f0..9cdb137 100644 --- a/apps/console/src/routes/BackupCreatePage.tsx +++ b/apps/console/src/routes/BackupCreatePage.tsx @@ -80,6 +80,10 @@ export function BackupCreatePage() { return } + // Run RJSF validation before the page-level checks so schema-required + // fields render inline errors instead of being masked by the alerts below. + if (schemaFormRef.current && !schemaFormRef.current.validate()) return + if (!formData.applicationRef?.kind || !formData.applicationRef?.name) { alert("Application reference is required") return @@ -95,10 +99,6 @@ export function BackupCreatePage() { return } - // The submit button lives outside RJSF and bypasses its validation, so - // trigger it explicitly; an invalid spec renders errors inline and aborts. - if (schemaFormRef.current && !schemaFormRef.current.validate()) return - const resource = { apiVersion: "backups.cozystack.io/v1alpha1", kind: "Backup", diff --git a/apps/console/src/routes/BackupJobCreatePage.test.tsx b/apps/console/src/routes/BackupJobCreatePage.test.tsx index f5f644f..e517713 100644 --- a/apps/console/src/routes/BackupJobCreatePage.test.tsx +++ b/apps/console/src/routes/BackupJobCreatePage.test.tsx @@ -14,6 +14,7 @@ const h = vi.hoisted(() => ({ applicationRef: { kind: "VirtualMachine", name: "demo" }, backupClassName: "s3", }, + emitSpec: {} as unknown, })) vi.mock("../lib/tenant-context.tsx", () => ({ @@ -37,7 +38,7 @@ vi.mock("../components/SchemaForm.tsx", () => ({ function MockSchemaForm({ onChange }, ref) { useImperativeHandle(ref, () => ({ validate: () => h.validateReturn })) useEffect(() => { - onChange(h.validSpec) + onChange(h.emitSpec) }, [onChange]) return null }, @@ -59,6 +60,7 @@ describe("BackupJobCreatePage submit validation gate", () => { h.createMutateAsync.mockReset() h.createMutateAsync.mockResolvedValue({}) h.validateReturn = true + h.emitSpec = h.validSpec }) it("does not POST when the form fails RJSF validation", async () => { @@ -82,4 +84,19 @@ describe("BackupJobCreatePage submit validation gate", () => { expect(h.createMutateAsync).toHaveBeenCalledTimes(1) }) + + it("runs RJSF validation before the page-level required-field alerts", async () => { + h.validateReturn = false + h.emitSpec = {} + const alertSpy = vi.spyOn(window, "alert").mockImplementation(() => {}) + const user = userEvent.setup() + renderPage() + + await user.type(screen.getByRole("textbox"), "my-job") + await user.click(screen.getByRole("button", { name: /create/i })) + + expect(h.createMutateAsync).not.toHaveBeenCalled() + expect(alertSpy).not.toHaveBeenCalled() + alertSpy.mockRestore() + }) }) diff --git a/apps/console/src/routes/BackupJobCreatePage.tsx b/apps/console/src/routes/BackupJobCreatePage.tsx index 3d3b814..46bec58 100644 --- a/apps/console/src/routes/BackupJobCreatePage.tsx +++ b/apps/console/src/routes/BackupJobCreatePage.tsx @@ -97,6 +97,10 @@ export function BackupJobCreatePage() { return } + // Run RJSF validation before the page-level checks so schema-required + // fields render inline errors instead of being masked by the alerts below. + if (schemaFormRef.current && !schemaFormRef.current.validate()) return + if (!formData.applicationRef?.kind || !formData.applicationRef?.name) { alert("Application reference is required") return @@ -107,10 +111,6 @@ export function BackupJobCreatePage() { return } - // The submit button lives outside RJSF and bypasses its validation, so - // trigger it explicitly; an invalid spec renders errors inline and aborts. - if (schemaFormRef.current && !schemaFormRef.current.validate()) return - // planRef is optional metadata recording which Plan triggered the job. The // dropdown ships an empty sentinel; strip it so the API never receives // `planRef: { name: "" }`, which would otherwise round-trip as a malformed diff --git a/apps/console/src/routes/BackupPlanCreatePage.test.tsx b/apps/console/src/routes/BackupPlanCreatePage.test.tsx index 83d67d4..3f2f1b0 100644 --- a/apps/console/src/routes/BackupPlanCreatePage.test.tsx +++ b/apps/console/src/routes/BackupPlanCreatePage.test.tsx @@ -14,6 +14,7 @@ const h = vi.hoisted(() => ({ applicationRef: { kind: "VirtualMachine", name: "demo" }, backupClassName: "s3", }, + emitSpec: {} as unknown, })) vi.mock("../lib/tenant-context.tsx", () => ({ @@ -37,7 +38,7 @@ vi.mock("../components/SchemaForm.tsx", () => ({ function MockSchemaForm({ onChange }, ref) { useImperativeHandle(ref, () => ({ validate: () => h.validateReturn })) useEffect(() => { - onChange(h.validSpec) + onChange(h.emitSpec) }, [onChange]) return null }, @@ -59,6 +60,7 @@ describe("BackupPlanCreatePage submit validation gate", () => { h.createMutateAsync.mockReset() h.createMutateAsync.mockResolvedValue({}) h.validateReturn = true + h.emitSpec = h.validSpec }) it("does not POST when the form fails RJSF validation", async () => { @@ -82,4 +84,19 @@ describe("BackupPlanCreatePage submit validation gate", () => { expect(h.createMutateAsync).toHaveBeenCalledTimes(1) }) + + it("runs RJSF validation before the page-level required-field alerts", async () => { + h.validateReturn = false + h.emitSpec = {} + const alertSpy = vi.spyOn(window, "alert").mockImplementation(() => {}) + const user = userEvent.setup() + renderPage() + + await user.type(screen.getByRole("textbox"), "my-plan") + await user.click(screen.getByRole("button", { name: /create/i })) + + expect(h.createMutateAsync).not.toHaveBeenCalled() + expect(alertSpy).not.toHaveBeenCalled() + alertSpy.mockRestore() + }) }) diff --git a/apps/console/src/routes/BackupPlanCreatePage.tsx b/apps/console/src/routes/BackupPlanCreatePage.tsx index ff6ac3d..7f0277e 100644 --- a/apps/console/src/routes/BackupPlanCreatePage.tsx +++ b/apps/console/src/routes/BackupPlanCreatePage.tsx @@ -102,6 +102,10 @@ export function BackupPlanCreatePage() { return } + // Run RJSF validation before the page-level checks so schema-required + // fields render inline errors instead of being masked by the alerts below. + if (schemaFormRef.current && !schemaFormRef.current.validate()) return + if (!formData.applicationRef?.kind || !formData.applicationRef?.name) { alert("Application reference is required") return @@ -112,10 +116,6 @@ export function BackupPlanCreatePage() { return } - // The submit button lives outside RJSF and bypasses its validation, so - // trigger it explicitly; an invalid spec renders errors inline and aborts. - if (schemaFormRef.current && !schemaFormRef.current.validate()) return - const resource = { apiVersion: "backups.cozystack.io/v1alpha1", kind: "Plan", diff --git a/apps/console/src/routes/BackupRestoreJobCreatePage.test.tsx b/apps/console/src/routes/BackupRestoreJobCreatePage.test.tsx index dcb144e..770f32e 100644 --- a/apps/console/src/routes/BackupRestoreJobCreatePage.test.tsx +++ b/apps/console/src/routes/BackupRestoreJobCreatePage.test.tsx @@ -13,6 +13,7 @@ const h = vi.hoisted(() => ({ validSpec: { backupRef: { name: "backup-1" }, }, + emitSpec: {} as unknown, })) vi.mock("../lib/tenant-context.tsx", () => ({ @@ -36,7 +37,7 @@ vi.mock("../components/SchemaForm.tsx", () => ({ function MockSchemaForm({ onChange }, ref) { useImperativeHandle(ref, () => ({ validate: () => h.validateReturn })) useEffect(() => { - onChange(h.validSpec) + onChange(h.emitSpec) }, [onChange]) return null }, @@ -58,6 +59,7 @@ describe("BackupRestoreJobCreatePage submit validation gate", () => { h.createMutateAsync.mockReset() h.createMutateAsync.mockResolvedValue({}) h.validateReturn = true + h.emitSpec = h.validSpec }) it("does not POST when the form fails RJSF validation", async () => { @@ -81,4 +83,19 @@ describe("BackupRestoreJobCreatePage submit validation gate", () => { expect(h.createMutateAsync).toHaveBeenCalledTimes(1) }) + + it("runs RJSF validation before the page-level required-field alerts", async () => { + h.validateReturn = false + h.emitSpec = {} + const alertSpy = vi.spyOn(window, "alert").mockImplementation(() => {}) + const user = userEvent.setup() + renderPage() + + await user.type(screen.getByRole("textbox"), "my-restore") + await user.click(screen.getByRole("button", { name: /create/i })) + + expect(h.createMutateAsync).not.toHaveBeenCalled() + expect(alertSpy).not.toHaveBeenCalled() + alertSpy.mockRestore() + }) }) diff --git a/apps/console/src/routes/BackupRestoreJobCreatePage.tsx b/apps/console/src/routes/BackupRestoreJobCreatePage.tsx index 8bd2709..a386fbb 100644 --- a/apps/console/src/routes/BackupRestoreJobCreatePage.tsx +++ b/apps/console/src/routes/BackupRestoreJobCreatePage.tsx @@ -118,6 +118,10 @@ export function BackupRestoreJobCreatePage() { return } + // Run RJSF validation before the page-level checks so schema-required + // fields render inline errors instead of being masked by the alerts below. + if (schemaFormRef.current && !schemaFormRef.current.validate()) return + if (!formData.backupRef?.name) { alert("Backup reference is required") return @@ -134,10 +138,6 @@ export function BackupRestoreJobCreatePage() { return } - // The submit button lives outside RJSF and bypasses its validation, so - // trigger it explicitly; an invalid spec renders errors inline and aborts. - if (schemaFormRef.current && !schemaFormRef.current.validate()) return - // Strip an empty targetApplicationRef so the API does not receive an empty // object that the API server would reject as malformed. const spec = { ...formData }