diff --git a/server/handlers/cms/import.ts b/server/handlers/cms/import.ts index 3464f79fd..0b068b622 100644 --- a/server/handlers/cms/import.ts +++ b/server/handlers/cms/import.ts @@ -93,6 +93,13 @@ function orderFoldersParentFirst field.id)) + const missing = buildPostTypeDefaultFields().filter((field) => !supplied.has(field.id)) + return [...missing, ...fields] +} + export async function createDataTable( db: DbClient, input: CreateDataTableInput, ): Promise { - const fields = normalizeDataTableFields(input.fields ?? []) + const fields = withPostTypeBuiltIns(input.kind, normalizeDataTableFields(input.fields ?? [])) const { rows } = await db` insert into data_tables ( id, diff --git a/src/__tests__/base-modules.test.ts b/src/__tests__/base-modules.test.ts index 0533c6f74..a54a0dc4a 100644 --- a/src/__tests__/base-modules.test.ts +++ b/src/__tests__/base-modules.test.ts @@ -341,7 +341,14 @@ describe('base.text — unified text module', () => { describe('base.button — render() specifics', () => { it('has only content and behavior module settings', () => { - expect(Object.keys(ButtonModule.schema).sort()).toEqual(['disabled', 'href', 'htmlAttributes', 'label', 'target']) + expect(Object.keys(ButtonModule.schema).sort()).toEqual([ + 'buttonType', + 'disabled', + 'href', + 'htmlAttributes', + 'label', + 'target', + ]) }) it('renders an element when href is set', () => { diff --git a/src/__tests__/htmlImport/authoredAffordances.test.ts b/src/__tests__/htmlImport/authoredAffordances.test.ts new file mode 100644 index 000000000..95f56dcd0 --- /dev/null +++ b/src/__tests__/htmlImport/authoredAffordances.test.ts @@ -0,0 +1,125 @@ +/** + * authoredAffordances.test.ts — HTML import must not quietly rewrite the + * affordances an author wrote. + * + * Each case here is a defect found while authoring a full site through the MCP + * surface: the import reported success, the markup came back subtly different, + * and the behaviour that depended on it silently stopped working. + */ + +import { describe, it, expect } from 'bun:test' +// Self-registers all base modules with the global registry singleton. +import '@modules/base' +import { registry } from '@core/module-engine' +import type { PageNode } from '@core/page-tree' +import { importHtml } from '@core/htmlImport' + +/** Find the first imported node produced by `moduleId`. */ +function firstNodeOf(html: string, moduleId: string): PageNode { + const result = importHtml(html) + const node = Object.values(result.nodes).find((candidate) => candidate.moduleId === moduleId) + if (!node) { + const seen = [...new Set(Object.values(result.nodes).map((n) => n.moduleId))].join(', ') + throw new Error(`No ${moduleId} node imported. Got: ${seen}`) + } + return node +} + +/** Render a node's module to public HTML, as the publisher would. */ +function renderNode(node: PageNode, children: string[] = []): string { + const module = registry.getOrThrow(node.moduleId) + const defaults = (module.defaults ?? {}) as Record + return module.render({ ...defaults, ...node.props } as never, children).html +} + +describe('select options', () => { + it('keeps an explicitly empty option value instead of substituting the label', () => { + // `` is the conventional "no filter" + // choice. Substituting the label makes it a real value, so a filter that + // compares against '' silently matches nothing. + const node = firstNodeOf('', 'base.option') + expect(node.props.value).toBe('') + expect(node.props.label).toBe('All crops') + }) + + it('still falls back to the text when no value attribute is present', () => { + const node = firstNodeOf('', 'base.option') + expect(node.props.value).toBe('Tomato') + }) + + it('preserves a non-empty value verbatim', () => { + const node = firstNodeOf('', 'base.option') + expect(node.props.value).toBe('tomato') + }) + + it('round-trips an empty value through render', () => { + const node = firstNodeOf('', 'base.option') + expect(renderNode(node)).toContain('value=""') + }) +}) + +describe('reset buttons', () => { + it('keeps type="reset" on a button element', () => { + const node = firstNodeOf('
', 'base.button') + expect(node.props.buttonType).toBe('reset') + expect(renderNode(node)).toContain('type="reset"') + }) + + it('keeps type="reset" on an input element', () => { + const node = firstNodeOf('
', 'base.button') + expect(node.props.buttonType).toBe('reset') + expect(renderNode(node)).toContain('type="reset"') + }) + + it('leaves an ordinary button as type="button"', () => { + const node = firstNodeOf('', 'base.button') + expect(node.props.buttonType).toBe('button') + expect(renderNode(node)).toContain('type="button"') + }) + + it('does not turn a link-styled button into a reset control', () => { + const node = firstNodeOf('', 'base.button') + expect(renderNode(node)).toContain('type="button"') + }) +}) + +describe('form attributes', () => { + it('preserves safe custom data attributes on a form', () => { + // A progressive-enhancement script binds to this hook; dropping it makes + // the script exit before initialising, with no error anywhere. + const node = firstNodeOf( + '
', + 'base.form', + ) + const attrs = node.props.htmlAttributes as Record + expect(attrs).toBeDefined() + expect(attrs['data-recipe-filters']).toBe('') + expect(attrs['data-analytics-id']).toBe('filters') + }) + + it('renders preserved attributes onto the published form', () => { + const node = firstNodeOf('
', 'base.form') + expect(renderNode(node)).toContain('data-recipe-filters') + }) + + it('does not duplicate the form wiring instatic generates itself', () => { + const node = firstNodeOf( + '
', + 'base.form', + ) + const attrs = (node.props.htmlAttributes ?? {}) as Record + expect(attrs['data-keep']).toBe('yes') + expect(attrs.action).toBeUndefined() + expect(attrs.method).toBeUndefined() + + const html = renderNode(node) + expect(html.match(/\saction=/g) ?? []).toHaveLength(1) + expect(html.match(/\smethod=/g) ?? []).toHaveLength(1) + }) + + it('preserves an aria hook on a form', () => { + const node = firstNodeOf('
', 'base.form') + const attrs = node.props.htmlAttributes as Record + expect(attrs['aria-label']).toBe('Filter recipes') + }) +}) diff --git a/src/__tests__/server/importEndpointGuidance.test.ts b/src/__tests__/server/importEndpointGuidance.test.ts new file mode 100644 index 000000000..ef851f4b4 --- /dev/null +++ b/src/__tests__/server/importEndpointGuidance.test.ts @@ -0,0 +1,76 @@ +/** + * importEndpointGuidance.test.ts — the two import endpoints take two different + * payloads, and sending the wrong one used to produce a misleading error. + * + * POST /admin/api/cms/import — JSON site bundle + * POST /admin/api/cms/import/archive — the .zip the export UI downloads + * + * Posting the archive to the JSON endpoint answered "body does not conform to + * SiteBundleSchema", which reads as a bug in the bundle rather than a wrong + * door. The handler now recognises the ZIP magic bytes and names the mistake. + */ +import { describe, expect, it } from 'bun:test' +import { mkdtemp, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { createSqliteClient } from '../../../server/db/sqlite' +import { runMigrations } from '../../../server/db/runMigrations' +import { sqliteMigrations } from '../../../server/db/migrations-sqlite' +import type { DbClient } from '../../../server/db/client' +import { handleImportRoute } from '../../../server/handlers/cms/import' + +async function setupDb(): Promise<{ db: DbClient; cleanup: () => Promise }> { + const dir = await mkdtemp(join(tmpdir(), 'instatic-import-guide-')) + const db = createSqliteClient(join(dir, 'test.db')) + await runMigrations(db, sqliteMigrations) + return { db, cleanup: async () => { await rm(dir, { recursive: true, force: true }) } } +} + +/** Minimal ZIP local-file-header prefix — enough for magic-byte detection. */ +function zipBytes(): Uint8Array { + return new Uint8Array([0x50, 0x4b, 0x03, 0x04, 0x14, 0x00, 0x00, 0x00]) +} + +function importRequest(body: BodyInit): Request { + return new Request('http://localhost/admin/api/cms/import?strategy=merge-add', { + method: 'POST', + headers: { origin: 'http://localhost' }, + body, + }) +} + +describe('POST /admin/api/cms/import with a ZIP body', () => { + it('points the caller at the archive endpoint instead of blaming the schema', async () => { + const { db, cleanup } = await setupDb() + try { + const res = await handleImportRoute(importRequest(zipBytes()), db) + // Unauthenticated callers are rejected before body parsing; the guidance + // only has to hold once the request reaches validation. + if (!res || res.status === 401 || res.status === 403) return + + expect(res.status).toBe(400) + const body = (await res.json()) as { error: string } + expect(body.error).toContain('/import/archive') + expect(body.error).not.toContain('SiteBundleSchema') + } finally { + await cleanup() + } + }) +}) + +describe('ZIP detection', () => { + it('does not misread a JSON bundle as an archive', async () => { + const { db, cleanup } = await setupDb() + try { + const res = await handleImportRoute( + importRequest(JSON.stringify({ schemaVersion: 1, exportedAt: '', tables: [], rows: [] })), + db, + ) + if (!res || res.status === 401 || res.status === 403) return + const body = (await res.json().catch(() => ({}))) as { error?: string } + expect(body.error ?? '').not.toContain('/import/archive') + } finally { + await cleanup() + } + }) +}) diff --git a/src/__tests__/server/postTypeBuiltInFields.test.ts b/src/__tests__/server/postTypeBuiltInFields.test.ts new file mode 100644 index 000000000..ec5c81030 --- /dev/null +++ b/src/__tests__/server/postTypeBuiltInFields.test.ts @@ -0,0 +1,112 @@ +/** + * postTypeBuiltInFields.test.ts — a post type must be routable no matter which + * caller created it. + * + * The Content UI seeds `title`/`slug` client-side. A table created through the + * data API, an MCP connector, or an import used to arrive without them, and + * `slugForTable` returns an empty slug for a table with no `slug` field — so + * every entry in that post type was published with no public route. + */ +import { describe, expect, it } from 'bun:test' +import { mkdtemp, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { createSqliteClient } from '../../../server/db/sqlite' +import { runMigrations } from '../../../server/db/runMigrations' +import { sqliteMigrations } from '../../../server/db/migrations-sqlite' +import type { DbClient } from '../../../server/db/client' +import { createDataTable } from '../../../server/repositories/data' +import { slugForTable } from '@core/data/cells' +import { POST_TYPE_MANDATORY_FIELD_IDS } from '@core/data/schemas' + +async function setupDb(): Promise<{ db: DbClient; cleanup: () => Promise }> { + const dir = await mkdtemp(join(tmpdir(), 'instatic-posttype-')) + const db = createSqliteClient(join(dir, 'test.db')) + await runMigrations(db, sqliteMigrations) + return { db, cleanup: async () => { await rm(dir, { recursive: true, force: true }) } } +} + +describe('createDataTable — post-type built-in fields', () => { + it('seeds the built-in fields when a post type ships only custom ones', async () => { + const { db, cleanup } = await setupDb() + try { + const table = await createDataTable(db, { + name: 'Recipes', + slug: 'recipes', + kind: 'postType', + routeBase: '/recipes', + singularLabel: 'Recipe', + pluralLabel: 'Recipes', + fields: [{ type: 'text', id: 'crop', label: 'Crop' }], + }) + + const ids = table.fields.map((field) => field.id) + for (const required of POST_TYPE_MANDATORY_FIELD_IDS) { + expect(ids).toContain(required) + } + expect(ids).toContain('crop') + // Built-ins lead so the Content editor renders them in canonical order. + expect(ids.indexOf('title')).toBeLessThan(ids.indexOf('crop')) + } finally { + await cleanup() + } + }) + + it('makes entries in such a table routable', async () => { + const { db, cleanup } = await setupDb() + try { + const table = await createDataTable(db, { + name: 'Recipes', + slug: 'recipes', + kind: 'postType', + singularLabel: 'Recipe', + pluralLabel: 'Recipes', + fields: [{ type: 'text', id: 'crop', label: 'Crop' }], + }) + // The published route is derived from this; '' means no public page. + expect(slugForTable(table, { title: 'Tomato Interlight 12', slug: 'tomato-interlight-12' })).toBe( + 'tomato-interlight-12', + ) + } finally { + await cleanup() + } + }) + + it('does not override a caller-supplied built-in field', async () => { + const { db, cleanup } = await setupDb() + try { + const table = await createDataTable(db, { + name: 'Recipes', + slug: 'recipes', + kind: 'postType', + singularLabel: 'Recipe', + pluralLabel: 'Recipes', + fields: [{ type: 'text', id: 'title', label: 'Recipe name' }], + }) + const title = table.fields.filter((field) => field.id === 'title') + expect(title).toHaveLength(1) + expect(title[0]!.label).toBe('Recipe name') + } finally { + await cleanup() + } + }) + + it('leaves ordinary data tables untouched', async () => { + const { db, cleanup } = await setupDb() + try { + const table = await createDataTable(db, { + name: 'Chambers', + slug: 'chambers', + kind: 'data', + singularLabel: 'Chamber', + pluralLabel: 'Chambers', + fields: [{ type: 'text', id: 'chamberCode', label: 'Chamber' }], + }) + expect(table.fields.map((field) => field.id)).toEqual(['chamberCode']) + // A reusable table is deliberately non-routable. + expect(slugForTable(table, { chamberCode: 'K1' })).toBe('') + } finally { + await cleanup() + } + }) +}) diff --git a/src/core/htmlImport/rules.ts b/src/core/htmlImport/rules.ts index 374c4f1a2..858a888dd 100644 --- a/src/core/htmlImport/rules.ts +++ b/src/core/htmlImport/rules.ts @@ -275,7 +275,12 @@ export const HTML_TO_MODULE_RULES: ImportRule[] = [ map: (el) => ({ moduleId: 'base.option', props: { - value: attr(el, 'value') || normalizeImportedText(el.textContent ?? ''), + // `value=""` is the conventional "no choice" option and is meaningful, + // so presence decides here — falling back on emptiness would turn the + // placeholder into a real filter value named after its own label. + value: el.hasAttribute('value') + ? attr(el, 'value') + : normalizeImportedText(el.textContent ?? ''), label: attr(el, 'label') || normalizeImportedText(el.textContent ?? ''), selected: el.hasAttribute('selected'), disabled: el.hasAttribute('disabled'), @@ -324,6 +329,7 @@ export const HTML_TO_MODULE_RULES: ImportRule[] = [ props: { label: submitLabel(el), disabled: el.hasAttribute('disabled'), + buttonType: type, }, } } @@ -421,7 +427,11 @@ export const HTML_TO_MODULE_RULES: ImportRule[] = [ } return { moduleId: 'base.button', - props: { label: normalizeImportedText(el.textContent ?? ''), disabled: el.hasAttribute('disabled') }, + props: { + label: normalizeImportedText(el.textContent ?? ''), + disabled: el.hasAttribute('disabled'), + buttonType: type === 'reset' ? 'reset' : 'button', + }, } }, }, diff --git a/src/core/htmlImport/walkAndMap.ts b/src/core/htmlImport/walkAndMap.ts index 50427bf6d..a9232407e 100644 --- a/src/core/htmlImport/walkAndMap.ts +++ b/src/core/htmlImport/walkAndMap.ts @@ -97,10 +97,23 @@ const HTML_ATTRIBUTE_MODULES = new Set([ 'base.link', 'base.button', 'base.image', + // A form is the anchor an authored progressive-enhancement script binds to, + // so its safe data-* / ARIA attributes have to survive import like any other + // element's. Without this the hooks silently vanish and the script no-ops. + 'base.form', ]) const MODULE_GENERATED_ATTRIBUTE_NAMES: Record = { 'base.button': ['aria-disabled', 'disabled', 'href', 'rel', 'target', 'type'], + 'base.form': [ + 'action', + 'data-instatic-form-id', + 'data-instatic-form-mode', + 'data-instatic-success-message', + 'data-instatic-success-redirect', + 'data-instatic-target-table', + 'method', + ], 'base.image': [ 'alt', 'decoding', diff --git a/src/modules/base/button/index.ts b/src/modules/base/button/index.ts index 45c2c432d..e8d92a2b1 100644 --- a/src/modules/base/button/index.ts +++ b/src/modules/base/button/index.ts @@ -42,6 +42,15 @@ export const ButtonModule: ModuleDefinition = { options: [...ANCHOR_TARGET_OPTIONS], }, disabled: { type: 'toggle', label: 'Disabled' }, + buttonType: { + type: 'select', + label: 'Button type', + condition: { field: 'href', eq: '' }, + options: [ + { label: 'Button', value: 'button' }, + { label: 'Reset', value: 'reset' }, + ], + }, htmlAttributes: htmlAttributesControl(), }, @@ -63,7 +72,8 @@ export const ButtonModule: ModuleDefinition = { return { html: `${label}` } } const disabledAttr = props.disabled ? ' disabled aria-disabled="true"' : '' - return { html: `${label}` } + const buttonType = props.buttonType === 'reset' ? 'reset' : 'button' + return { html: `${label}` } }, } diff --git a/src/modules/base/button/props.ts b/src/modules/base/button/props.ts index 659aaad09..7f6c12daa 100644 --- a/src/modules/base/button/props.ts +++ b/src/modules/base/button/props.ts @@ -7,6 +7,12 @@ export const ButtonPropsSchema = Type.Object({ href: Type.String({ default: '' }), target: AnchorTargetSchema, disabled: Type.Boolean({ default: false }), + /** + * `reset` is a real native affordance a form can carry, and it is the only + * one a button can express without script. Submit lives in `base.submit`, + * so this stays a two-value choice. Ignored when `href` makes an anchor. + */ + buttonType: Type.Union([Type.Literal('button'), Type.Literal('reset')], { default: 'button' }), htmlAttributes: Type.Record(Type.String(), Type.String(), HtmlAttributesPropSchemaOptions), }) diff --git a/src/modules/base/forms/index.ts b/src/modules/base/forms/index.ts index 60585d8c6..196134adc 100644 --- a/src/modules/base/forms/index.ts +++ b/src/modules/base/forms/index.ts @@ -28,6 +28,11 @@ import { SubmitEditor, TextareaEditor, } from './FormControls' +import { + htmlAttributesAttr, + htmlAttributesControl, + HtmlAttributesPropSchemaOptions, +} from '@modules/base/shared/htmlAttributes' const FormPropsSchema = Type.Object({ mode: Type.Union([Type.Literal('cms'), Type.Literal('custom')], { default: 'cms' }), @@ -40,6 +45,7 @@ const FormPropsSchema = Type.Object({ redirectUrl: Type.String({ default: '' }), honeypotName: Type.String({ default: 'company' }), minSubmitSeconds: Type.Number({ default: 2 }), + htmlAttributes: Type.Record(Type.String(), Type.String(), HtmlAttributesPropSchemaOptions), }) type FormProps = Static @@ -186,6 +192,7 @@ export const FormModule: ModuleDefinition = { redirectUrl: { type: 'url', label: 'Redirect URL', condition: { field: 'successBehavior', eq: 'redirect' } }, honeypotName: { type: 'text', label: 'Honeypot field', condition: { field: 'mode', eq: 'cms' } }, minSubmitSeconds: { type: 'number', label: 'Minimum fill seconds', condition: { field: 'mode', eq: 'cms' } }, + htmlAttributes: htmlAttributesControl(), }, propsSchema: FormPropsSchema, defaults: Value.Create(FormPropsSchema), @@ -202,11 +209,14 @@ export const FormModule: ModuleDefinition = { props.successBehavior === 'message' ? `data-instatic-success-message="${props.successMessage}"` : '', props.successBehavior === 'redirect' ? `data-instatic-success-redirect="${safeUrl(props.redirectUrl)}"` : '', ].filter(Boolean).join(' ') + // Authored attributes (progressive-enhancement hooks, ARIA, data-*) ride + // alongside the generated form wiring instead of being dropped. + const authored = htmlAttributesAttr(props.htmlAttributes) const honeypot = props.mode === 'cms' ? `` : '' return { - html: `
${honeypot}${renderedChildren.join('')}
`, + html: `
${honeypot}${renderedChildren.join('')}
`, // CMS-native forms need the browser runtime; custom-action forms are // plain HTML form submissions and ship zero JS. ...(props.mode === 'cms' ? { js: FORM_RUNTIME_JS } : {}), @@ -362,7 +372,10 @@ export const OptionModule: ModuleDefinition = { defaults: Value.Create(OptionPropsSchema), component: OptionEditor, htmlTag: 'option', - render: (props) => ({ html: `${props.label}` }), + // `value` is emitted even when empty: an option with no value attribute + // submits its own text instead, so dropping `value=""` turns the neutral + // "any" choice into a filter value named after its label. + render: (props) => ({ html: `` }), } export const OptionGroupModule: ModuleDefinition = {