diff --git a/.changeset/13459-refuse-invalid-gantt-block.md b/.changeset/13459-refuse-invalid-gantt-block.md new file mode 100644 index 0000000000..57910f5830 --- /dev/null +++ b/.changeset/13459-refuse-invalid-gantt-block.md @@ -0,0 +1,11 @@ +--- +"@object-ui/plugin-gantt": patch +"@object-ui/app-shell": patch +--- + +Clause-②: no +Metadata-protocol: narrowed + +An invalid `gantt` block is refused instead of drawn (objectui#13459). `ObjectGantt` used to log `Invalid gantt configuration` and render the block anyway, so a block with a misspelt or missing key still drew a chart. It now renders a refusal that names each `GanttConfigSchema` issue by its path in the block (`gantt.endDateField: …`); a valid block and the flat ObjectView / ListView spelling render as before. + +The Studio designers' spec derivations (`dashboard`, `package`, `page`, `report` and `view` schemas in `metadata-admin`) no longer catch a failed derivation and return `undefined`, which rendered a designer form silently missing fields. Their only input is the bundled spec, so a failure is a build-time fact that fails their tests; at runtime it reaches the route's error boundary. diff --git a/packages/app-shell/src/views/metadata-admin/dashboard-schema.ts b/packages/app-shell/src/views/metadata-admin/dashboard-schema.ts index 7cf9b63a38..22fb2a5153 100644 --- a/packages/app-shell/src/views/metadata-admin/dashboard-schema.ts +++ b/packages/app-shell/src/views/metadata-admin/dashboard-schema.ts @@ -39,20 +39,19 @@ const FORM_FIELDS_OWNED_ELSEWHERE = new Set([ 'name', // record identity — not user-editable here ]); +/** + * No `try`/`catch` around the spec derivations in this file (objectui#13459). + * Their only input is the bundled `@objectstack/spec`, so a derivation that + * fails is a build-time fact: the tests beside this file go red, and at runtime + * the throw reaches the route's error boundary. It used to be caught, warned + * about in the console and turned into `undefined`, which rendered a designer + * form silently missing fields that the author could then save past. + */ let _dashboardDocSchema: JsonSchema | undefined; -let _dashboardDocFailed = false; /** JSONSchema for the whole (flat) Dashboard document. */ -export function getDashboardSchema(): JsonSchema | undefined { - if (_dashboardDocSchema || _dashboardDocFailed) return _dashboardDocSchema; - try { - _dashboardDocSchema = z.toJSONSchema(DashboardSchema, TO_JSON_OPTS) as JsonSchema; - } catch (err) { - _dashboardDocFailed = true; - if (typeof console !== 'undefined') { - console.warn('[dashboard-schema] failed to derive Dashboard JSONSchema from spec', err); - } - } +export function getDashboardSchema(): JsonSchema { + _dashboardDocSchema ??= z.toJSONSchema(DashboardSchema, TO_JSON_OPTS) as JsonSchema; return _dashboardDocSchema; } @@ -79,31 +78,24 @@ export function getDashboardForm(locale?: SupportedLocale | string): FormViewSpe const cacheKey = String(locale ?? ''); if (_dashboardForm.has(cacheKey)) return _dashboardForm.get(cacheKey); if (!specDashboardForm || typeof specDashboardForm !== 'object') return undefined; - try { - const clone = JSON.parse(JSON.stringify(specDashboardForm)) as FormViewSpec; - for (const section of clone.sections ?? []) { - section.fields = (section.fields ?? []).filter((f: any) => { - const name = typeof f === 'string' ? f : f?.field; - return !FORM_FIELDS_OWNED_ELSEWHERE.has(name); - }); - } - // Drop now-empty sections so we don't render bare headers. - clone.sections = (clone.sections ?? []).filter( - (s: any) => (s.fields ?? []).length > 0, - ); - // Localize AFTER pruning: the overlay only has to reach what renders, and - // pruning cannot drop a section the overlay just renamed. - const localized = localizeMetadataForm( - clone as unknown as Record, - 'dashboard', - locale, - ) as unknown as FormViewSpec; - _dashboardForm.set(cacheKey, localized); - return localized; - } catch (err) { - if (typeof console !== 'undefined') { - console.warn('[dashboard-schema] failed to prepare dashboardForm from spec', err); - } - return undefined; + const clone = JSON.parse(JSON.stringify(specDashboardForm)) as FormViewSpec; + for (const section of clone.sections ?? []) { + section.fields = (section.fields ?? []).filter((f: any) => { + const name = typeof f === 'string' ? f : f?.field; + return !FORM_FIELDS_OWNED_ELSEWHERE.has(name); + }); } + // Drop now-empty sections so we don't render bare headers. + clone.sections = (clone.sections ?? []).filter( + (s: any) => (s.fields ?? []).length > 0, + ); + // Localize AFTER pruning: the overlay only has to reach what renders, and + // pruning cannot drop a section the overlay just renamed. + const localized = localizeMetadataForm( + clone as unknown as Record, + 'dashboard', + locale, + ) as unknown as FormViewSpec; + _dashboardForm.set(cacheKey, localized); + return localized; } diff --git a/packages/app-shell/src/views/metadata-admin/package-schema.ts b/packages/app-shell/src/views/metadata-admin/package-schema.ts index 9b717ed095..d5f538fbf1 100644 --- a/packages/app-shell/src/views/metadata-admin/package-schema.ts +++ b/packages/app-shell/src/views/metadata-admin/package-schema.ts @@ -50,20 +50,19 @@ export const PACKAGE_FORM_FIELDS = [ 'dependencies', ] as const; +/** + * No `try`/`catch` around the spec derivations in this file (objectui#13459). + * Their only input is the bundled `@objectstack/spec`, so a derivation that + * fails is a build-time fact: the tests beside this file go red, and at runtime + * the throw reaches the route's error boundary. It used to be caught, warned + * about in the console and turned into `undefined`, which rendered a designer + * form silently missing fields that the author could then save past. + */ let _schema: JsonSchema | undefined; -let _schemaFailed = false; /** JSONSchema for the whole package manifest (memoised, spec-derived). */ -export function getPackageSchema(): JsonSchema | undefined { - if (_schema || _schemaFailed) return _schema; - try { - _schema = z.toJSONSchema(ManifestSchema, TO_JSON_OPTS) as JsonSchema; - } catch (err) { - _schemaFailed = true; - if (typeof console !== 'undefined') { - console.warn('[package-schema] failed to derive manifest JSONSchema from spec', err); - } - } +export function getPackageSchema(): JsonSchema { + _schema ??= z.toJSONSchema(ManifestSchema, TO_JSON_OPTS) as JsonSchema; return _schema; } diff --git a/packages/app-shell/src/views/metadata-admin/page-schema.ts b/packages/app-shell/src/views/metadata-admin/page-schema.ts index 89da29282d..3a45e8582c 100644 --- a/packages/app-shell/src/views/metadata-admin/page-schema.ts +++ b/packages/app-shell/src/views/metadata-admin/page-schema.ts @@ -39,20 +39,19 @@ const TO_JSON_OPTS = { io: 'input', unrepresentable: 'any' } as const; */ export const PAGE_FIELDS_OWNED_ELSEWHERE = new Set(['name', 'regions', 'children']); +/** + * No `try`/`catch` around the spec derivations in this file (objectui#13459). + * Their only input is the bundled `@objectstack/spec`, so a derivation that + * fails is a build-time fact: the tests beside this file go red, and at runtime + * the throw reaches the route's error boundary. It used to be caught, warned + * about in the console and turned into `undefined`, which rendered a designer + * form silently missing fields that the author could then save past. + */ let _pageSchema: JsonSchema | undefined; -let _pageSchemaFailed = false; /** JSONSchema for the whole Page document (memoised, spec-derived). */ -export function getPageSchema(): JsonSchema | undefined { - if (_pageSchema || _pageSchemaFailed) return _pageSchema; - try { - _pageSchema = z.toJSONSchema(PageSchema, TO_JSON_OPTS) as JsonSchema; - } catch (err) { - _pageSchemaFailed = true; - if (typeof console !== 'undefined') { - console.warn('[page-schema] failed to derive Page JSONSchema from spec', err); - } - } +export function getPageSchema(): JsonSchema { + _pageSchema ??= z.toJSONSchema(PageSchema, TO_JSON_OPTS) as JsonSchema; return _pageSchema; } @@ -67,21 +66,14 @@ let _pageForm: FormViewSpec | undefined; export function getPageForm(): FormViewSpec | undefined { if (_pageForm) return _pageForm; if (!specPageForm || typeof specPageForm !== 'object') return undefined; - try { - const clone = JSON.parse(JSON.stringify(specPageForm)) as FormViewSpec; - for (const section of clone.sections ?? []) { - section.fields = (section.fields ?? []).filter((f: any) => { - const name = typeof f === 'string' ? f : f?.field; - return !PAGE_FIELDS_OWNED_ELSEWHERE.has(name); - }); - } - clone.sections = (clone.sections ?? []).filter((s) => (s.fields ?? []).length > 0); - _pageForm = clone; - } catch (err) { - if (typeof console !== 'undefined') { - console.warn('[page-schema] failed to prune Page authoring form from spec', err); - } - _pageForm = specPageForm as FormViewSpec; + const clone = JSON.parse(JSON.stringify(specPageForm)) as FormViewSpec; + for (const section of clone.sections ?? []) { + section.fields = (section.fields ?? []).filter((f: any) => { + const name = typeof f === 'string' ? f : f?.field; + return !PAGE_FIELDS_OWNED_ELSEWHERE.has(name); + }); } + clone.sections = (clone.sections ?? []).filter((s) => (s.fields ?? []).length > 0); + _pageForm = clone; return _pageForm; } diff --git a/packages/app-shell/src/views/metadata-admin/report-schema.ts b/packages/app-shell/src/views/metadata-admin/report-schema.ts index 6422915143..e9540bc9c8 100644 --- a/packages/app-shell/src/views/metadata-admin/report-schema.ts +++ b/packages/app-shell/src/views/metadata-admin/report-schema.ts @@ -38,20 +38,19 @@ const FORM_FIELDS_OWNED_ELSEWHERE = new Set([ 'name', // record identity — not user-editable here ]); +/** + * No `try`/`catch` around the spec derivations in this file (objectui#13459). + * Their only input is the bundled `@objectstack/spec`, so a derivation that + * fails is a build-time fact: the tests beside this file go red, and at runtime + * the throw reaches the route's error boundary. It used to be caught, warned + * about in the console and turned into `undefined`, which rendered a designer + * form silently missing fields that the author could then save past. + */ let _reportDocSchema: JsonSchema | undefined; -let _reportDocFailed = false; /** JSONSchema for the whole (flat) Report document. */ -export function getReportSchema(): JsonSchema | undefined { - if (_reportDocSchema || _reportDocFailed) return _reportDocSchema; - try { - _reportDocSchema = z.toJSONSchema(ReportSchema, TO_JSON_OPTS) as JsonSchema; - } catch (err) { - _reportDocFailed = true; - if (typeof console !== 'undefined') { - console.warn('[report-schema] failed to derive Report JSONSchema from spec', err); - } - } +export function getReportSchema(): JsonSchema { + _reportDocSchema ??= z.toJSONSchema(ReportSchema, TO_JSON_OPTS) as JsonSchema; return _reportDocSchema; } @@ -80,48 +79,41 @@ let _reportForm: FormViewSpec | undefined; export function getReportForm(): FormViewSpec | undefined { if (_reportForm) return _reportForm; if (!specReportForm || typeof specReportForm !== 'object') return undefined; - try { - const clone = JSON.parse(JSON.stringify(specReportForm)) as FormViewSpec; - const schemaProps = getReportSchema()?.properties as Record | undefined; - for (const section of clone.sections ?? []) { - section.fields = (section.fields ?? []).filter((f: any) => { - const name = typeof f === 'string' ? f : f?.field; - if (FORM_FIELDS_OWNED_ELSEWHERE.has(name)) return false; - // Schema-subset pass: drop form fields the schema no longer carries. - if (schemaProps && !(name in schemaProps)) return false; - return true; - }); - } - if (schemaProps && 'runtimeFilter' in schemaProps) { - const declared = new Set(); - for (const s of clone.sections ?? []) { - for (const f of s.fields ?? []) { - declared.add(typeof f === 'string' ? f : (f as any)?.field); - } - } - if (!declared.has('runtimeFilter')) { - clone.sections = [ - ...(clone.sections ?? []), - { - label: 'Filter', - description: 'Render-time scope filter, ANDed at query time.', - collapsible: true, - collapsed: true, - fields: [{ field: 'runtimeFilter', widget: 'json' }], - }, - ]; + const clone = JSON.parse(JSON.stringify(specReportForm)) as FormViewSpec; + const schemaProps = getReportSchema().properties as Record | undefined; + for (const section of clone.sections ?? []) { + section.fields = (section.fields ?? []).filter((f: any) => { + const name = typeof f === 'string' ? f : f?.field; + if (FORM_FIELDS_OWNED_ELSEWHERE.has(name)) return false; + // Schema-subset pass: drop form fields the schema no longer carries. + if (schemaProps && !(name in schemaProps)) return false; + return true; + }); + } + if (schemaProps && 'runtimeFilter' in schemaProps) { + const declared = new Set(); + for (const s of clone.sections ?? []) { + for (const f of s.fields ?? []) { + declared.add(typeof f === 'string' ? f : (f as any)?.field); } } - // Drop now-empty sections so we don't render bare headers. - clone.sections = (clone.sections ?? []).filter( - (s: any) => (s.fields ?? []).length > 0, - ); - _reportForm = clone; - } catch (err) { - if (typeof console !== 'undefined') { - console.warn('[report-schema] failed to prepare reportForm from spec', err); + if (!declared.has('runtimeFilter')) { + clone.sections = [ + ...(clone.sections ?? []), + { + label: 'Filter', + description: 'Render-time scope filter, ANDed at query time.', + collapsible: true, + collapsed: true, + fields: [{ field: 'runtimeFilter', widget: 'json' }], + }, + ]; } - return undefined; } + // Drop now-empty sections so we don't render bare headers. + clone.sections = (clone.sections ?? []).filter( + (s: any) => (s.fields ?? []).length > 0, + ); + _reportForm = clone; return _reportForm; } diff --git a/packages/app-shell/src/views/metadata-admin/spec-derivation-13459.test.ts b/packages/app-shell/src/views/metadata-admin/spec-derivation-13459.test.ts new file mode 100644 index 0000000000..7f579c8c6d --- /dev/null +++ b/packages/app-shell/src/views/metadata-admin/spec-derivation-13459.test.ts @@ -0,0 +1,73 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#13459 — the designer forms' spec derivations fail LOUDLY. + * + * The five `*-schema.ts` modules derive their JSONSchema from the bundled + * `@objectstack/spec`. That input is fixed at build time, so a derivation that + * fails is a build-time fact: this file goes red for it. At runtime the throw + * reaches the route's error boundary instead of being caught, warned about and + * turned into `undefined` — which used to render a designer form silently + * missing fields. + * + * Two halves: every derivation succeeds against the bundled spec, and a + * derivation that throws reaches the caller instead of becoming `undefined`. + */ + +import { describe, it, expect, vi, afterEach } from 'vitest'; + +const DERIVATIONS = [ + ['dashboard-schema', 'getDashboardSchema'], + ['package-schema', 'getPackageSchema'], + ['page-schema', 'getPageSchema'], + ['report-schema', 'getReportSchema'], + ['view-schema', 'getListVariantSchema'], + ['view-schema', 'getFormVariantSchema'], + ['view-schema', 'getListColumnSchema'], +] as const; + +const LOADERS: Record<(typeof DERIVATIONS)[number][0], () => Promise> = { + 'dashboard-schema': () => import('./dashboard-schema'), + 'package-schema': () => import('./package-schema'), + 'page-schema': () => import('./page-schema'), + 'report-schema': () => import('./report-schema'), + 'view-schema': () => import('./view-schema'), +}; + +/** A fresh module instance per case, so a memoised result cannot answer. */ +async function derive(file: (typeof DERIVATIONS)[number][0], getter: string): Promise { + vi.resetModules(); + const mod = (await LOADERS[file]()) as Record unknown>; + return mod[getter](); +} + +// `vi.doMock`, not a hoisted `vi.mock`: the failing `toJSONSchema` is scoped to +// the cases that ask for it, and undone after each, so it cannot reach another +// test file sharing this worker's module registry. +afterEach(() => { + vi.doUnmock('zod'); + vi.resetModules(); +}); + +describe('designer spec derivations (objectui#13459)', () => { + it.each(DERIVATIONS)('%s.%s derives an object schema from the bundled spec', async (file, getter) => { + const schema = (await derive(file, getter)) as { type?: unknown; properties?: unknown } | undefined; + expect(schema).toBeDefined(); + expect(schema?.type).toBe('object'); + expect(schema?.properties).toBeTypeOf('object'); + }); + + it.each(DERIVATIONS)('%s.%s throws when the derivation fails, never returns undefined', async (file, getter) => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + vi.doMock('zod', async (importOriginal) => { + const actual = await importOriginal(); + const toJSONSchema = (): never => { + throw new Error('derivation failed (test)'); + }; + return { ...actual, z: { ...actual.z, toJSONSchema } }; + }); + await expect(derive(file, getter)).rejects.toThrow('derivation failed (test)'); + expect(warn).not.toHaveBeenCalled(); + warn.mockRestore(); + }); +}); diff --git a/packages/app-shell/src/views/metadata-admin/view-schema.ts b/packages/app-shell/src/views/metadata-admin/view-schema.ts index df81623624..2ab90258e8 100644 --- a/packages/app-shell/src/views/metadata-admin/view-schema.ts +++ b/packages/app-shell/src/views/metadata-admin/view-schema.ts @@ -44,46 +44,36 @@ const FORM_FIELDS_OWNED_ELSEWHERE = new Set([ 'name', // record identity — not user-editable here ]); +/** + * No `try`/`catch` around the spec derivations in this file (objectui#13459). + * Their only input is the bundled `@objectstack/spec`, so a derivation that + * fails is a build-time fact: the tests beside this file go red, and at runtime + * the throw reaches the route's error boundary. It used to be caught, warned + * about in the console and turned into `undefined`, which rendered a designer + * form silently missing fields that the author could then save past. + */ let _viewDocSchema: JsonSchema | undefined; -let _viewDocFailed = false; -function viewDocSchema(): JsonSchema | undefined { - if (_viewDocSchema || _viewDocFailed) return _viewDocSchema; - try { - _viewDocSchema = z.toJSONSchema(ViewSchema, TO_JSON_OPTS) as JsonSchema; - } catch (err) { - _viewDocFailed = true; - if (typeof console !== 'undefined') { - console.warn('[view-schema] failed to derive View JSONSchema from spec', err); - } - } +function viewDocSchema(): JsonSchema { + _viewDocSchema ??= z.toJSONSchema(ViewSchema, TO_JSON_OPTS) as JsonSchema; return _viewDocSchema; } /** JSONSchema for a single LIST-family view (grid/kanban/calendar/…). */ export function getListVariantSchema(): JsonSchema | undefined { - return viewDocSchema()?.properties?.list; + return viewDocSchema().properties?.list; } /** JSONSchema for a single FORM-family view. */ export function getFormVariantSchema(): JsonSchema | undefined { - return viewDocSchema()?.properties?.form; + return viewDocSchema().properties?.form; } let _listColumnSchema: JsonSchema | undefined; -let _listColumnFailed = false; /** JSONSchema for a single list column (ListColumn). */ -export function getListColumnSchema(): JsonSchema | undefined { - if (_listColumnSchema || _listColumnFailed) return _listColumnSchema; - try { - _listColumnSchema = z.toJSONSchema(ListColumnSchema, TO_JSON_OPTS) as JsonSchema; - } catch (err) { - _listColumnFailed = true; - if (typeof console !== 'undefined') { - console.warn('[view-schema] failed to derive ListColumn JSONSchema from spec', err); - } - } +export function getListColumnSchema(): JsonSchema { + _listColumnSchema ??= z.toJSONSchema(ListColumnSchema, TO_JSON_OPTS) as JsonSchema; return _listColumnSchema; } @@ -98,24 +88,17 @@ let _viewForm: FormViewSpec | undefined; export function getViewForm(): FormViewSpec | undefined { if (_viewForm) return _viewForm; if (!specViewForm || typeof specViewForm !== 'object') return undefined; - try { - const clone = JSON.parse(JSON.stringify(specViewForm)) as FormViewSpec; - for (const section of clone.sections ?? []) { - section.fields = (section.fields ?? []).filter((f: any) => { - const name = typeof f === 'string' ? f : f?.field; - return !FORM_FIELDS_OWNED_ELSEWHERE.has(name); - }); - } - // Drop now-empty sections so we don't render bare headers. - clone.sections = (clone.sections ?? []).filter( - (s: any) => (s.fields ?? []).length > 0, - ); - _viewForm = clone; - } catch (err) { - if (typeof console !== 'undefined') { - console.warn('[view-schema] failed to prepare viewForm from spec', err); - } - return undefined; + const clone = JSON.parse(JSON.stringify(specViewForm)) as FormViewSpec; + for (const section of clone.sections ?? []) { + section.fields = (section.fields ?? []).filter((f: any) => { + const name = typeof f === 'string' ? f : f?.field; + return !FORM_FIELDS_OWNED_ELSEWHERE.has(name); + }); } + // Drop now-empty sections so we don't render bare headers. + clone.sections = (clone.sections ?? []).filter( + (s: any) => (s.fields ?? []).length > 0, + ); + _viewForm = clone; return _viewForm; } diff --git a/packages/plugin-gantt/src/ObjectGantt.invalidBlockRefusal-13459.test.tsx b/packages/plugin-gantt/src/ObjectGantt.invalidBlockRefusal-13459.test.tsx new file mode 100644 index 0000000000..562f83a58a --- /dev/null +++ b/packages/plugin-gantt/src/ObjectGantt.invalidBlockRefusal-13459.test.tsx @@ -0,0 +1,102 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * objectui#13459 — a `gantt` block that fails `GanttConfigSchema` is REFUSED, + * never drawn. + * + * Before this card the block's `safeParse` failure was a `console.warn`, and the + * unvalidated block was returned and rendered anyway: a misspelt key or a + * missing date field still drew a plausible chart, so the author never learnt + * the block was wrong. The refusal names each parse issue by its path inside + * the block. + * + * The CONTROL case pins the other direction: a valid block still renders, so a + * fix that refused every block would not pass this file. + */ + +import React from 'react'; +import { describe, it, expect, vi, afterEach, beforeEach } from 'vitest'; +import { render, screen, waitFor, cleanup } from '@testing-library/react'; +import { ObjectGantt } from './ObjectGantt'; + +vi.mock('./GanttView', () => ({ + GanttView: ({ tasks }: any) => ( +
+ {tasks.map((t: any) => ( +
{t.title}
+ ))} +
+ ), +})); + +const ROWS = [ + { id: 'r1', name: 'Ada onboarding', start_date: '2024-01-01', end_date: '2024-01-05' }, +]; + +const inline = (gantt: Record) => + ({ + type: 'object-gantt', + objectName: 'crm_leave_request', + gantt, + data: { provider: 'value', items: ROWS }, + }) as any; + +let warn: ReturnType; +beforeEach(() => { + warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); +}); +afterEach(() => { + warn.mockRestore(); + cleanup(); +}); + +describe('ObjectGantt — an invalid `gantt` block is refused (objectui#13459)', () => { + it('REFUSES a block carrying a key the spec does not declare, and names it', async () => { + render( + , + ); + const refusal = await screen.findByTestId('gantt-invalid-config'); + expect(refusal.getAttribute('role')).toBe('alert'); + expect(refusal.textContent).toContain('Invalid gantt configuration'); + expect(refusal.textContent).toContain('startField'); + expect(screen.queryByTestId('gantt-view')).toBeNull(); + expect(screen.queryByTestId('gantt-task')).toBeNull(); + }); + + it('REFUSES a block missing a required field, naming its path in the block', async () => { + render(); + const refusal = await screen.findByTestId('gantt-invalid-config'); + expect(refusal.textContent).toContain('gantt.endDateField'); + expect(screen.queryByTestId('gantt-view')).toBeNull(); + }); + + it('keeps the console diagnostic beside the refusal', async () => { + render(); + await screen.findByTestId('gantt-invalid-config'); + expect(warn.mock.calls.some((c: unknown[]) => String(c[0]).includes('Invalid gantt configuration'))).toBe(true); + }); + + it('CONTROL: a valid block still renders its tasks, with no refusal', async () => { + render( + , + ); + await waitFor(() => expect(screen.getByTestId('gantt-view')).toBeTruthy()); + expect(screen.getByText('Ada onboarding')).toBeTruthy(); + expect(screen.queryByTestId('gantt-invalid-config')).toBeNull(); + }); +}); diff --git a/packages/plugin-gantt/src/ObjectGantt.tsx b/packages/plugin-gantt/src/ObjectGantt.tsx index 51ab2a7840..c92aca31a9 100644 --- a/packages/plugin-gantt/src/ObjectGantt.tsx +++ b/packages/plugin-gantt/src/ObjectGantt.tsx @@ -604,21 +604,32 @@ function warnOnShadowedFlatGanttKeys(schema: ObjectGanttSchema): void { * and worth stating, because the map case cannot produce it: the spec's * `GanttConfigSchema` REQUIRES `startDateField` / `endDateField` / `titleField` * (`ObjectMapConfigSchema` requires nothing), so an INCOMPLETE block now - * outranks a complete flat spelling and yields an incomplete config. Both + * outranks a complete flat spelling, and that block is REFUSED (below). Both * diagnostics fire on that node — `Invalid gantt configuration` from the * `safeParse` below, and the shadow warning naming the flat keys that lost. + * + * A block that fails `GanttConfigSchema` is refused, never drawn (objectui#13459). + * It used to be warned about in the console and then returned as-is, so a block + * with a misspelt or missing key still rendered something plausible and the + * author never learnt it was wrong. `invalidBlock` carries the parse issues to + * the refusal screen instead; `config` is `null` on that branch, so nothing + * downstream can read the unvalidated block. Same posture as `ObjectTimeline`'s + * refusal: the renderer says what is wrong rather than guessing what was meant. */ -function getGanttConfig(schema: ObjectGanttSchema): GanttConfigEx | null { +function getGanttConfig(schema: ObjectGanttSchema): { + config: GanttConfigEx | null; + invalidBlock: readonly string[] | null; +} { // 1. The `gantt` block (the ObjectGridSchema-style shape) — the authoring // face, and the winner whenever it is present. if (schema.gantt) { - const config = schema.gantt as GanttConfigEx; - const result = GanttConfigSchema.safeParse(config); + const result = GanttConfigSchema.safeParse(schema.gantt); + warnOnShadowedFlatGanttKeys(schema); if (!result.success) { console.warn(`[ObjectGantt] Invalid gantt configuration:`, result.error.format()); + return { config: null, invalidBlock: describeGanttBlockIssues(result.error.issues) }; } - warnOnShadowedFlatGanttKeys(schema); - return config; + return { config: schema.gantt as GanttConfigEx, invalidBlock: null }; } // 2. The internal flat form — the ObjectView / ListView flatten product. @@ -627,7 +638,7 @@ function getGanttConfig(schema: ObjectGanttSchema): GanttConfigEx | null { // `safeParse`d, also unchanged: this branch never validated, and adding // validation to it is a separate question from precedence. if (schema.startDateField && schema.endDateField) { - return { + const flat: GanttConfigEx = { startDateField: schema.startDateField, endDateField: schema.endDateField, titleField: schema.titleField || 'name', @@ -658,9 +669,21 @@ function getGanttConfig(schema: ObjectGanttSchema): GanttConfigEx | null { timeZone: schema.timeZone, dependencyTypes: schema.dependencyTypes, }; + return { config: flat, invalidBlock: null }; } - return null; + return { config: null, invalidBlock: null }; +} + +/** + * One line per `GanttConfigSchema` issue, `path: message`, for the refusal + * screen. The path is spelled from the block (`gantt.quickFilters.0.field`) so + * the author can find the key; an issue on the block itself names `gantt`. + */ +function describeGanttBlockIssues( + issues: readonly { path: readonly PropertyKey[]; message: string }[], +): string[] { + return issues.map((issue) => `${['gantt', ...issue.path.map(String)].join('.')}: ${issue.message}`); } export const ObjectGantt: React.FC = ({ @@ -734,7 +757,7 @@ export const ObjectGantt: React.FC = ({ return rawDataConfig; }, [dataConfigKey]); - const ganttConfig = getGanttConfig(schema); + const { config: ganttConfig, invalidBlock: invalidGanttBlock } = getGanttConfig(schema); const dataProvider = dataConfig?.provider; const hasInlineData = dataProvider === 'value'; /* @@ -2317,6 +2340,21 @@ export const ObjectGantt: React.FC = ({ ); } + if (invalidGanttBlock) { + return ( +
+
+ {/* English, like the "configuration required" refusal below: both + address the view's author, not the end user. */} +
+ Invalid gantt configuration — the `gantt` block does not match the spec, so the + chart is not drawn. Fix: {invalidGanttBlock.join('; ')} +
+
+
+ ); + } + if (!ganttConfig) { return (