Skip to content

Commit 6181493

Browse files
myasnikovdaniillexfrei
myasnikovdaniil
and
lexfrei
committed
refactor(dashboard): validate the schema form before page-level checks
In the four backup create pages (Backup, BackupJob, BackupPlan, BackupRestoreJob), run the RJSF SchemaForm.validate() call right after the name check and before the page-level required-field alerts. The submit button lives outside RJSF and bypasses its validation, so each page triggers validate() explicitly; it previously ran only after the manual alert checks, so a schema-required field left empty exited via an alert without RJSF ever rendering its inline errors. Validating first makes inline errors show consistently; the page-level alerts then cover only page-only requirements. Re-applied from cozystack/cozystack-ui#26 onto the in-tree console after the UI was vendored (#2963); the source repo is being archived. Scope is the validation-ordering change only — the original PR's VMDisk/StorageClass parts were obsoleted by the DynamicOptionsWidget refactor (#30) already in the vendored snapshot. Includes the original per-page tests. Co-authored-by: Aleksei Sviridkin <[email protected]> Assisted-By: Claude <[email protected]> Signed-off-by: Myasnikov Daniil <[email protected]>
1 parent 71c0fc1 commit 6181493

8 files changed

Lines changed: 92 additions & 20 deletions

File tree

‎packages/system/dashboard/images/console/apps/console/src/routes/BackupCreatePage.test.tsx‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ const h = vi.hoisted(() => ({
1515
strategyRef: { kind: "Strategy", name: "s3" },
1616
takenAt: "2024-01-01T00:00:00Z",
1717
},
18+
// Spec the mocked SchemaForm emits into the page on mount; reset per test.
19+
emitSpec: {} as unknown,
1820
}))
1921

2022
vi.mock("../lib/tenant-context.tsx", () => ({
@@ -38,7 +40,7 @@ vi.mock("../components/SchemaForm.tsx", () => ({
3840
function MockSchemaForm({ onChange }, ref) {
3941
useImperativeHandle(ref, () => ({ validate: () => h.validateReturn }))
4042
useEffect(() => {
41-
onChange(h.validSpec)
43+
onChange(h.emitSpec)
4244
}, [onChange])
4345
return null
4446
},
@@ -60,6 +62,7 @@ describe("BackupCreatePage submit validation gate", () => {
6062
h.createMutateAsync.mockReset()
6163
h.createMutateAsync.mockResolvedValue({})
6264
h.validateReturn = true
65+
h.emitSpec = h.validSpec
6366
})
6467

6568
it("does not POST when the form fails RJSF validation", async () => {
@@ -83,4 +86,22 @@ describe("BackupCreatePage submit validation gate", () => {
8386

8487
expect(h.createMutateAsync).toHaveBeenCalledTimes(1)
8588
})
89+
90+
it("runs RJSF validation before the page-level required-field alerts", async () => {
91+
// Form is RJSF-invalid AND a page-required field is missing. The gate must
92+
// fire first, so no alert() is shown — proving validate() runs before the
93+
// manual checks (under the old ordering the applicationRef alert fired).
94+
h.validateReturn = false
95+
h.emitSpec = {}
96+
const alertSpy = vi.spyOn(window, "alert").mockImplementation(() => {})
97+
const user = userEvent.setup()
98+
renderPage()
99+
100+
await user.type(screen.getByRole("textbox"), "my-backup")
101+
await user.click(screen.getByRole("button", { name: /create/i }))
102+
103+
expect(h.createMutateAsync).not.toHaveBeenCalled()
104+
expect(alertSpy).not.toHaveBeenCalled()
105+
alertSpy.mockRestore()
106+
})
86107
})

‎packages/system/dashboard/images/console/apps/console/src/routes/BackupCreatePage.tsx‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,10 @@ export function BackupCreatePage() {
8080
return
8181
}
8282

83+
// Run RJSF validation before the page-level checks so schema-required
84+
// fields render inline errors instead of being masked by the alerts below.
85+
if (schemaFormRef.current && !schemaFormRef.current.validate()) return
86+
8387
if (!formData.applicationRef?.kind || !formData.applicationRef?.name) {
8488
alert("Application reference is required")
8589
return
@@ -95,10 +99,6 @@ export function BackupCreatePage() {
9599
return
96100
}
97101

98-
// The submit button lives outside RJSF and bypasses its validation, so
99-
// trigger it explicitly; an invalid spec renders errors inline and aborts.
100-
if (schemaFormRef.current && !schemaFormRef.current.validate()) return
101-
102102
const resource = {
103103
apiVersion: "backups.cozystack.io/v1alpha1",
104104
kind: "Backup",

‎packages/system/dashboard/images/console/apps/console/src/routes/BackupJobCreatePage.test.tsx‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ const h = vi.hoisted(() => ({
1414
applicationRef: { kind: "VirtualMachine", name: "demo" },
1515
backupClassName: "s3",
1616
},
17+
emitSpec: {} as unknown,
1718
}))
1819

1920
vi.mock("../lib/tenant-context.tsx", () => ({
@@ -37,7 +38,7 @@ vi.mock("../components/SchemaForm.tsx", () => ({
3738
function MockSchemaForm({ onChange }, ref) {
3839
useImperativeHandle(ref, () => ({ validate: () => h.validateReturn }))
3940
useEffect(() => {
40-
onChange(h.validSpec)
41+
onChange(h.emitSpec)
4142
}, [onChange])
4243
return null
4344
},
@@ -59,6 +60,7 @@ describe("BackupJobCreatePage submit validation gate", () => {
5960
h.createMutateAsync.mockReset()
6061
h.createMutateAsync.mockResolvedValue({})
6162
h.validateReturn = true
63+
h.emitSpec = h.validSpec
6264
})
6365

6466
it("does not POST when the form fails RJSF validation", async () => {
@@ -82,4 +84,19 @@ describe("BackupJobCreatePage submit validation gate", () => {
8284

8385
expect(h.createMutateAsync).toHaveBeenCalledTimes(1)
8486
})
87+
88+
it("runs RJSF validation before the page-level required-field alerts", async () => {
89+
h.validateReturn = false
90+
h.emitSpec = {}
91+
const alertSpy = vi.spyOn(window, "alert").mockImplementation(() => {})
92+
const user = userEvent.setup()
93+
renderPage()
94+
95+
await user.type(screen.getByRole("textbox"), "my-job")
96+
await user.click(screen.getByRole("button", { name: /create/i }))
97+
98+
expect(h.createMutateAsync).not.toHaveBeenCalled()
99+
expect(alertSpy).not.toHaveBeenCalled()
100+
alertSpy.mockRestore()
101+
})
85102
})

‎packages/system/dashboard/images/console/apps/console/src/routes/BackupJobCreatePage.tsx‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,10 @@ export function BackupJobCreatePage() {
9797
return
9898
}
9999

100+
// Run RJSF validation before the page-level checks so schema-required
101+
// fields render inline errors instead of being masked by the alerts below.
102+
if (schemaFormRef.current && !schemaFormRef.current.validate()) return
103+
100104
if (!formData.applicationRef?.kind || !formData.applicationRef?.name) {
101105
alert("Application reference is required")
102106
return
@@ -107,10 +111,6 @@ export function BackupJobCreatePage() {
107111
return
108112
}
109113

110-
// The submit button lives outside RJSF and bypasses its validation, so
111-
// trigger it explicitly; an invalid spec renders errors inline and aborts.
112-
if (schemaFormRef.current && !schemaFormRef.current.validate()) return
113-
114114
// planRef is optional metadata recording which Plan triggered the job. The
115115
// dropdown ships an empty sentinel; strip it so the API never receives
116116
// `planRef: { name: "" }`, which would otherwise round-trip as a malformed

‎packages/system/dashboard/images/console/apps/console/src/routes/BackupPlanCreatePage.test.tsx‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ const h = vi.hoisted(() => ({
1414
applicationRef: { kind: "VirtualMachine", name: "demo" },
1515
backupClassName: "s3",
1616
},
17+
emitSpec: {} as unknown,
1718
}))
1819

1920
vi.mock("../lib/tenant-context.tsx", () => ({
@@ -37,7 +38,7 @@ vi.mock("../components/SchemaForm.tsx", () => ({
3738
function MockSchemaForm({ onChange }, ref) {
3839
useImperativeHandle(ref, () => ({ validate: () => h.validateReturn }))
3940
useEffect(() => {
40-
onChange(h.validSpec)
41+
onChange(h.emitSpec)
4142
}, [onChange])
4243
return null
4344
},
@@ -59,6 +60,7 @@ describe("BackupPlanCreatePage submit validation gate", () => {
5960
h.createMutateAsync.mockReset()
6061
h.createMutateAsync.mockResolvedValue({})
6162
h.validateReturn = true
63+
h.emitSpec = h.validSpec
6264
})
6365

6466
it("does not POST when the form fails RJSF validation", async () => {
@@ -82,4 +84,19 @@ describe("BackupPlanCreatePage submit validation gate", () => {
8284

8385
expect(h.createMutateAsync).toHaveBeenCalledTimes(1)
8486
})
87+
88+
it("runs RJSF validation before the page-level required-field alerts", async () => {
89+
h.validateReturn = false
90+
h.emitSpec = {}
91+
const alertSpy = vi.spyOn(window, "alert").mockImplementation(() => {})
92+
const user = userEvent.setup()
93+
renderPage()
94+
95+
await user.type(screen.getByRole("textbox"), "my-plan")
96+
await user.click(screen.getByRole("button", { name: /create/i }))
97+
98+
expect(h.createMutateAsync).not.toHaveBeenCalled()
99+
expect(alertSpy).not.toHaveBeenCalled()
100+
alertSpy.mockRestore()
101+
})
85102
})

‎packages/system/dashboard/images/console/apps/console/src/routes/BackupPlanCreatePage.tsx‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,10 @@ export function BackupPlanCreatePage() {
102102
return
103103
}
104104

105+
// Run RJSF validation before the page-level checks so schema-required
106+
// fields render inline errors instead of being masked by the alerts below.
107+
if (schemaFormRef.current && !schemaFormRef.current.validate()) return
108+
105109
if (!formData.applicationRef?.kind || !formData.applicationRef?.name) {
106110
alert("Application reference is required")
107111
return
@@ -112,10 +116,6 @@ export function BackupPlanCreatePage() {
112116
return
113117
}
114118

115-
// The submit button lives outside RJSF and bypasses its validation, so
116-
// trigger it explicitly; an invalid spec renders errors inline and aborts.
117-
if (schemaFormRef.current && !schemaFormRef.current.validate()) return
118-
119119
const resource = {
120120
apiVersion: "backups.cozystack.io/v1alpha1",
121121
kind: "Plan",

‎packages/system/dashboard/images/console/apps/console/src/routes/BackupRestoreJobCreatePage.test.tsx‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ const h = vi.hoisted(() => ({
1313
validSpec: {
1414
backupRef: { name: "backup-1" },
1515
},
16+
emitSpec: {} as unknown,
1617
}))
1718

1819
vi.mock("../lib/tenant-context.tsx", () => ({
@@ -36,7 +37,7 @@ vi.mock("../components/SchemaForm.tsx", () => ({
3637
function MockSchemaForm({ onChange }, ref) {
3738
useImperativeHandle(ref, () => ({ validate: () => h.validateReturn }))
3839
useEffect(() => {
39-
onChange(h.validSpec)
40+
onChange(h.emitSpec)
4041
}, [onChange])
4142
return null
4243
},
@@ -58,6 +59,7 @@ describe("BackupRestoreJobCreatePage submit validation gate", () => {
5859
h.createMutateAsync.mockReset()
5960
h.createMutateAsync.mockResolvedValue({})
6061
h.validateReturn = true
62+
h.emitSpec = h.validSpec
6163
})
6264

6365
it("does not POST when the form fails RJSF validation", async () => {
@@ -81,4 +83,19 @@ describe("BackupRestoreJobCreatePage submit validation gate", () => {
8183

8284
expect(h.createMutateAsync).toHaveBeenCalledTimes(1)
8385
})
86+
87+
it("runs RJSF validation before the page-level required-field alerts", async () => {
88+
h.validateReturn = false
89+
h.emitSpec = {}
90+
const alertSpy = vi.spyOn(window, "alert").mockImplementation(() => {})
91+
const user = userEvent.setup()
92+
renderPage()
93+
94+
await user.type(screen.getByRole("textbox"), "my-restore")
95+
await user.click(screen.getByRole("button", { name: /create/i }))
96+
97+
expect(h.createMutateAsync).not.toHaveBeenCalled()
98+
expect(alertSpy).not.toHaveBeenCalled()
99+
alertSpy.mockRestore()
100+
})
84101
})

‎packages/system/dashboard/images/console/apps/console/src/routes/BackupRestoreJobCreatePage.tsx‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,10 @@ export function BackupRestoreJobCreatePage() {
118118
return
119119
}
120120

121+
// Run RJSF validation before the page-level checks so schema-required
122+
// fields render inline errors instead of being masked by the alerts below.
123+
if (schemaFormRef.current && !schemaFormRef.current.validate()) return
124+
121125
if (!formData.backupRef?.name) {
122126
alert("Backup reference is required")
123127
return
@@ -134,10 +138,6 @@ export function BackupRestoreJobCreatePage() {
134138
return
135139
}
136140

137-
// The submit button lives outside RJSF and bypasses its validation, so
138-
// trigger it explicitly; an invalid spec renders errors inline and aborts.
139-
if (schemaFormRef.current && !schemaFormRef.current.validate()) return
140-
141141
// Strip an empty targetApplicationRef so the API does not receive an empty
142142
// object that the API server would reject as malformed.
143143
const spec = { ...formData }

0 commit comments

Comments
 (0)