From cabb0c302d8a7be2f491dcd94789118fda7aa84e Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 22 Aug 2026 10:54:28 +0000 Subject: [PATCH 1/2] fix(ui): curate admin default columns by declared flag, not field name/type (#1018) Replaces the hardcoded password/createdAt/updatedAt exclusion in the admin list view, related-list tables, and ListTable with one shared curation rule driven by each field's declared `ui.listView.defaultColumn` (core). A list's structural createdAt/updatedAt columns are identified from its own timestamp config rather than by name, password() sets the flag instead of being matched by type, and auth's read-denied credential fields now declare it too so they no longer render as permanently empty default columns. Closes #1018 Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Pydn7Pah2bUAZnKCDzhvXU --- .changeset/curly-badgers-column.md | 9 ++ .changeset/curly-badgers-credential.md | 5 + .changeset/curly-badgers-curate.md | 13 +++ docs/content/reference/config-api.md | 1 + docs/content/reference/fields-api.md | 1 + packages/auth/src/config/derive-auth-lists.ts | 12 +- packages/auth/tests/derive-auth-lists.test.ts | 23 ++++ packages/core/src/config/types.ts | 38 ++++++- packages/core/src/fields/index.ts | 7 ++ packages/core/tests/field-types.test.ts | 21 ++++ packages/ui/src/components/ListView.tsx | 12 +- packages/ui/src/components/ListViewClient.tsx | 12 +- .../src/components/standalone/ListTable.tsx | 19 +++- packages/ui/src/lib/defaultColumns.ts | 63 +++++++++++ packages/ui/src/lib/deriveItemView.ts | 27 ++--- packages/ui/src/lib/serializeFieldConfig.ts | 4 + .../ui/tests/components/ListTable.test.tsx | 46 +++++--- .../ui/tests/components/ListView.test.tsx | 42 ++++++- .../tests/components/ListViewClient.test.tsx | 34 +++++- packages/ui/tests/lib/defaultColumns.test.ts | 105 ++++++++++++++++++ packages/ui/tests/lib/deriveItemView.test.ts | 70 +++++++++++- 21 files changed, 513 insertions(+), 51 deletions(-) create mode 100644 .changeset/curly-badgers-column.md create mode 100644 .changeset/curly-badgers-credential.md create mode 100644 .changeset/curly-badgers-curate.md create mode 100644 packages/ui/src/lib/defaultColumns.ts create mode 100644 packages/ui/tests/lib/defaultColumns.test.ts diff --git a/.changeset/curly-badgers-column.md b/.changeset/curly-badgers-column.md new file mode 100644 index 00000000..b9486f8c --- /dev/null +++ b/.changeset/curly-badgers-column.md @@ -0,0 +1,9 @@ +--- +'@opensaas/stack-ui': minor +--- + +Replace the admin UI's hardcoded `password`/`createdAt`/`updatedAt` default-column exclusion with curation driven by each field's declared `ui.listView.defaultColumn` (issue #1018). The list view, related-list tables, and the `ListTable` standalone component now share one implementation (`computeDefaultColumns`) instead of three independent name/type-matching copies, and a list's structural `createdAt`/`updatedAt` timestamp columns are identified from its own timestamp configuration rather than by name. + +**Behavior change:** an application field literally named (or typed) `password`, `createdAt`, or `updatedAt` that does NOT declare `ui.listView.defaultColumn: false` — and isn't your list's actual auto-timestamp column — is no longer hidden from default admin columns purely by name/type match. Real password fields (built with `password()`) and real system timestamps are unaffected; they're excluded via the declared flag instead. + +`ListTable` gains an optional `fields?: Record` prop to supply this curation metadata; without it (as before), every `fieldTypes` column shows absent an explicit `columns` list. diff --git a/.changeset/curly-badgers-credential.md b/.changeset/curly-badgers-credential.md new file mode 100644 index 00000000..a8b2d38e --- /dev/null +++ b/.changeset/curly-badgers-credential.md @@ -0,0 +1,5 @@ +--- +'@opensaas/stack-auth': patch +--- + +Read-denied credential fields (ADR-0036) now also declare `ui.listView.defaultColumn: false`, so they're curated out of the admin's default table columns instead of rendering as permanently empty columns. diff --git a/.changeset/curly-badgers-curate.md b/.changeset/curly-badgers-curate.md new file mode 100644 index 00000000..73998980 --- /dev/null +++ b/.changeset/curly-badgers-curate.md @@ -0,0 +1,13 @@ +--- +'@opensaas/stack-core': minor +--- + +Add `ui.listView.defaultColumn` to field config — a declared, presentation-only flag (default `true`) controlling whether a field belongs in a list/related-list table's default column set. Naming a field explicitly in `ui.listView.initialColumns` or a relationship's `ui.itemView.columns` always shows it regardless of this flag. + +```typescript +fields: { + internalScore: integer({ ui: { listView: { defaultColumn: false } } }), +} +``` + +`password()` now sets this flag to `false` by default instead of the admin UI matching on field type — a password field can opt back into default columns with `ui: { listView: { defaultColumn: true } } }`. diff --git a/docs/content/reference/config-api.md b/docs/content/reference/config-api.md index a78d3a44..b5dfbefb 100644 --- a/docs/content/reference/config-api.md +++ b/docs/content/reference/config-api.md @@ -703,6 +703,7 @@ UI-specific configuration passed to field components. - `component?: React.Component` - Custom field component - `fieldType?: string` - Reference to globally registered field type - `valueForClientSerialization?: (args) => unknown` - Transform value before sending to browser +- `listView?: { defaultColumn?: boolean }` - Whether this field belongs in a list/related-list table's default column set (default `true`). Naming the field explicitly in `ui.listView.initialColumns` or a relationship's `ui.itemView.columns` always shows it regardless of this flag. **Presentation only** — it does not affect who can read the field; use `access.read` for that. --- diff --git a/docs/content/reference/fields-api.md b/docs/content/reference/fields-api.md index 1aa4269d..86acdab7 100644 --- a/docs/content/reference/fields-api.md +++ b/docs/content/reference/fields-api.md @@ -915,6 +915,7 @@ password(options?: { 2. **Idempotent**: Already-hashed passwords are not re-hashed 3. **Secure Output**: Query results return `HashedPassword` instances with a `compare()` method 4. **No Exposure**: Only sends `{ isSet: boolean }` to client (not the hash) +5. **Excluded from default admin columns**: sets `ui.listView.defaultColumn: false` by default (a presentation default, not an access control — see [`ui.listView`](/docs/reference/config-api#ui)); override with `ui: { listView: { defaultColumn: true } }` to show it anyway #### Options diff --git a/packages/auth/src/config/derive-auth-lists.ts b/packages/auth/src/config/derive-auth-lists.ts index 9b53676d..b0079a9e 100644 --- a/packages/auth/src/config/derive-auth-lists.ts +++ b/packages/auth/src/config/derive-auth-lists.ts @@ -195,7 +195,17 @@ function withCredentialAccess( field: FieldConfig, ): FieldConfig { if (!registry[modelKey]?.has(fieldKey)) return field - return { ...field, access: DENY_READ } + return { + ...field, + access: DENY_READ, + // Curated out of the admin's default table columns too (issue #1018), + // via the same declared flag as everything else — a read-denied column + // would otherwise render permanently empty rather than simply absent. + ui: { + ...field.ui, + listView: { ...field.ui?.listView, defaultColumn: false }, + }, + } } /** diff --git a/packages/auth/tests/derive-auth-lists.test.ts b/packages/auth/tests/derive-auth-lists.test.ts index 5ac88aff..1c001f45 100644 --- a/packages/auth/tests/derive-auth-lists.test.ts +++ b/packages/auth/tests/derive-auth-lists.test.ts @@ -480,6 +480,27 @@ describe('deriveAuthLists - credential fields ship read-denied (ADR-0036, issue } }) + it('also curates read-denied credential fields out of the admin default columns (issue #1018)', async () => { + const { lists } = deriveAuthLists(defaultModels) + + const denied: Array<[string, string]> = [ + ['Session', 'token'], + ['Verification', 'value'], + ['Account', 'password'], + ['Account', 'accessToken'], + ['Account', 'refreshToken'], + ['Account', 'idToken'], + ] + + for (const [listKey, fieldKey] of denied) { + const field = lists[listKey].fields[fieldKey] + expect(field.ui?.listView?.defaultColumn).toBe(false) + } + + // A non-credential field is left with no ui.listView declaration at all. + expect(lists.Session.fields.ipAddress.ui?.listView).toBeUndefined() + }) + it('leaves identifying fields open — session/account metadata and every User field', () => { const { lists } = deriveAuthLists(defaultModels) @@ -557,6 +578,7 @@ describe('deriveAuthLists - credential fields on plugin tables (issue #1014)', ( expect(field.access?.read).toBeTypeOf('function') // eslint-disable-next-line @typescript-eslint/no-explicit-any -- minimal read-access call fixture expect(await field.access!.read!({} as any)).toBe(false) + expect(field.ui?.listView?.defaultColumn).toBe(false) } }) @@ -578,6 +600,7 @@ describe('deriveAuthLists - credential fields on plugin tables (issue #1014)', ( expect(field.access?.read).toBeTypeOf('function') // eslint-disable-next-line @typescript-eslint/no-explicit-any -- minimal read-access call fixture expect(await field.access!.read!({} as any)).toBe(false) + expect(field.ui?.listView?.defaultColumn).toBe(false) } }) diff --git a/packages/core/src/config/types.ts b/packages/core/src/config/types.ts index e3f8de28..e508ff20 100644 --- a/packages/core/src/config/types.ts +++ b/packages/core/src/config/types.ts @@ -591,6 +591,32 @@ export type BaseFieldConfig = { * ``` */ description?: string + /** + * Whether this field belongs in a list/related-list table's DEFAULT + * column set (issue #1018) — the columns shown when nothing explicitly + * names them (`ui.listView.initialColumns` on the list, or a + * relationship's own `ui.itemView.columns`). Naming the field explicitly + * in either of those always shows it regardless of this flag; it governs + * only what appears absent an explicit column list. + * + * This is a PRESENTATION default, not an access control — a field can be + * read-denied and still default to `true` here (it simply renders empty + * for a viewer who can't read it), and setting this to `false` hides a + * column without restricting who can read the underlying value. The + * field-level `access.read` deny remains the only real boundary. + * + * @default true + * + * @example Hide an internal field from default table views without denying read access + * ```typescript + * fields: { + * internalScore: integer({ ui: { listView: { defaultColumn: false } } }), + * } + * ``` + */ + listView?: { + defaultColumn?: boolean + } /** * Transform field value before sending to client (browser) * Useful for sensitive fields (e.g., passwords) or complex data structures @@ -1018,8 +1044,10 @@ export type RelationshipItemViewConfig = { * The related list's fields to show as Relationship-table columns, in order. * * When omitted, the columns default to the related list's own column - * curation (`ui.listView.initialColumns`, else all non-system fields) minus - * the back-reference field that points at the parent record. + * curation (`ui.listView.initialColumns`, else every field whose own + * `ui.listView.defaultColumn` declaration holds — see {@link + * BaseFieldConfig.ui}) minus the back-reference field that points at the + * parent record. * * @example * ```typescript @@ -2277,14 +2305,16 @@ export interface BulkAction { * `ui.listView`. * * When omitted, the admin UI falls back to its existing defaults: every - * non-system field is shown as a column and no default sort is applied. + * field whose own `ui.listView.defaultColumn` declaration holds is shown as + * a column (see {@link BaseFieldConfig.ui}) and no default sort is applied. */ export type ListViewUIConfig = { /** * The fields to show as columns in the list table, in order. * * Drives both the column **selection** and their **order**. When omitted, - * all non-system fields are shown (current default behaviour). + * every field whose own `ui.listView.defaultColumn` declaration holds is + * shown (current default behaviour). * * @example * ```typescript diff --git a/packages/core/src/fields/index.ts b/packages/core/src/fields/index.ts index f0e7485e..cf8b172e 100644 --- a/packages/core/src/fields/index.ts +++ b/packages/core/src/fields/index.ts @@ -972,6 +972,13 @@ export function password ({ isSet: !!value }), }, hooks: { diff --git a/packages/core/tests/field-types.test.ts b/packages/core/tests/field-types.test.ts index 3161f4e0..d4dcdd0f 100644 --- a/packages/core/tests/field-types.test.ts +++ b/packages/core/tests/field-types.test.ts @@ -856,6 +856,27 @@ describe('Field Types', () => { ) }) }) + + describe('ui.listView.defaultColumn (issue #1018)', () => { + test('is excluded from default admin table columns by default', () => { + const field = password() + + expect(field.ui?.listView?.defaultColumn).toBe(false) + }) + + test('can be opted back into default columns explicitly', () => { + const field = password({ ui: { listView: { defaultColumn: true } } }) + + expect(field.ui?.listView?.defaultColumn).toBe(true) + }) + + test('preserves other caller-supplied ui options', () => { + const field = password({ ui: { description: 'Account password' } }) + + expect(field.ui?.description).toBe('Account password') + expect(field.ui?.listView?.defaultColumn).toBe(false) + }) + }) }) describe('select field', () => { diff --git a/packages/ui/src/components/ListView.tsx b/packages/ui/src/components/ListView.tsx index c5927107..a494bf41 100644 --- a/packages/ui/src/components/ListView.tsx +++ b/packages/ui/src/components/ListView.tsx @@ -4,6 +4,7 @@ import { ListViewClient } from './ListViewClient.js' import type { SerializedBulkAction } from './BulkActions.js' import { formatListName } from '../lib/utils.js' import { serializeFieldConfigs } from '../lib/serializeFieldConfig.js' +import { withStructuralTimestampDefaults } from '../lib/defaultColumns.js' import { jsonSafeClone } from '../lib/jsonSafeClone.js' import { PageHeader } from './PageHeader.js' import { Button } from '../primitives/button.js' @@ -339,6 +340,13 @@ export async function ListView({ listKey, ) + // Fold in the structural createdAt/updatedAt exclusion (issue #1018) before + // crossing the server/client boundary, so `ListViewClient`'s fallback (used + // when no explicit `columns` is configured) curates off the same declared + // `ui.listView.defaultColumn` flag as everything else — no timestamp-aware + // logic needed on the client. + const displayFields = withStructuralTimestampDefaults(listConfig.fields, listConfig, config.db) + return (
[ + Object.entries(displayFields).map(([key, field]) => [ key, (field as { type: string }).type, ]), )} - fields={serializeFieldConfigs(listConfig.fields)} + fields={serializeFieldConfigs(displayFields)} relationshipRefs={relationshipRefs} columns={columns} initialSort={activeSort} diff --git a/packages/ui/src/components/ListViewClient.tsx b/packages/ui/src/components/ListViewClient.tsx index 5fb4784e..293e202e 100644 --- a/packages/ui/src/components/ListViewClient.tsx +++ b/packages/ui/src/components/ListViewClient.tsx @@ -24,6 +24,7 @@ import { BulkActions, type SerializedBulkAction } from './BulkActions.js' import { useRowSelection, getPageCheckboxState } from '../lib/useRowSelection.js' import { useBulkStatus } from '../lib/useBulkStatus.js' import type { SerializableFieldConfig } from '../lib/serializeFieldConfig.js' +import { computeDefaultColumns } from '../lib/defaultColumns.js' import type { ServerActionInput } from '../server/types.js' import type { FilterFieldSuggestion } from '@opensaas/stack-core' @@ -183,11 +184,14 @@ export function ListViewClient({ selection.togglePage(pageIds) } + // Absent an explicit `columns` list, curate off each field's own declared + // `ui.listView.defaultColumn` (issue #1018) via the same shared function + // `deriveItemView.ts`'s server-side layout helper uses — no field name/type + // matching here. When `fields` metadata isn't supplied at all (a caller + // with only `fieldTypes`), there is nothing to curate by, so every column + // shows. const displayColumns = - columns || - Object.keys(fieldTypes).filter( - (key) => fieldTypes[key] !== 'password' && !['createdAt', 'updatedAt'].includes(key), - ) + columns || (fields ? computeDefaultColumns(fields) : Object.keys(fieldTypes)) // Items are already sorted by the server via orderBy; no in-memory sort needed. diff --git a/packages/ui/src/components/standalone/ListTable.tsx b/packages/ui/src/components/standalone/ListTable.tsx index 7e964cbc..9e69d4c7 100644 --- a/packages/ui/src/components/standalone/ListTable.tsx +++ b/packages/ui/src/components/standalone/ListTable.tsx @@ -15,6 +15,7 @@ import { import { EmptyState } from '../EmptyState.js' import { CellRenderer } from '../cells/CellRenderer.js' import type { SerializableFieldConfig } from '../../lib/serializeFieldConfig.js' +import { computeDefaultColumns } from '../../lib/defaultColumns.js' /** * Per-part `classNames` slots for `ListTable` (issue #709). Each slot is merged @@ -59,6 +60,14 @@ export interface ListTableProps { * neutral badge, same as before this option existed. */ fieldOptions?: Record> + /** + * Serialised per-field config, keyed by field name (issue #1018). When + * supplied, an explicit `columns` list absent, the default columns are + * curated off each field's own `ui.listView.defaultColumn` declaration + * (see `computeDefaultColumns`) instead of showing every `fieldTypes` key. + * Omit when you have no field config to hand — every column shows. + */ + fields?: Record basePath?: string columns?: string[] onRowClick?: (item: Record) => void @@ -90,6 +99,7 @@ export function ListTable({ fieldTypes, relationshipRefs, fieldOptions, + fields, basePath = '/admin', columns, onRowClick, @@ -117,11 +127,12 @@ export function ListTable({ options: fieldOptions?.[fieldName], }) + // Absent an explicit `columns` list, curate off each field's own declared + // `ui.listView.defaultColumn` (issue #1018) when `fields` metadata was + // supplied; with no `fields` at all there's nothing to curate by, so every + // column shows. const displayColumns = - columns || - Object.keys(fieldTypes).filter( - (key) => fieldTypes[key] !== 'password' && !['createdAt', 'updatedAt'].includes(key), - ) + columns || (fields ? computeDefaultColumns(fields) : Object.keys(fieldTypes)) const sortedItems = [...items] if (sortBy && sortable) { diff --git a/packages/ui/src/lib/defaultColumns.ts b/packages/ui/src/lib/defaultColumns.ts new file mode 100644 index 00000000..10a6128f --- /dev/null +++ b/packages/ui/src/lib/defaultColumns.ts @@ -0,0 +1,63 @@ +import type { DatabaseConfig, ListConfig } from '@opensaas/stack-core' + +/** The subset of a field config `computeDefaultColumns` needs — satisfied by both `FieldConfig` (server) and `SerializableFieldConfig` (client), since both carry `ui` intact. */ +export interface DefaultColumnFieldLike { + ui?: { + listView?: { + defaultColumn?: boolean + } + } +} + +/** Whether a field belongs in a list/related-list table's DEFAULT column set (issue #1018) — `field.ui.listView.defaultColumn`, defaulting to `true`. Does not affect a field named explicitly via `initialColumns`/`itemView.columns`, which always wins over this default. */ +export function isDefaultColumnField(field: DefaultColumnFieldLike | undefined): boolean { + return field?.ui?.listView?.defaultColumn !== false +} + +/** + * The default column set for a fields map, in declaration order — every + * field whose {@link isDefaultColumnField} holds. This is the single + * curation rule shared by the server-side item-view layout helper + * (`deriveItemView.ts`, for a related list's default table columns) and the + * client-side list-view fallback (`ListViewClient`/`ListTable`, when no + * explicit `columns` is supplied) — both curate off the same declared flag + * instead of independently matching field names or types. + */ +export function computeDefaultColumns( + fields: Record, +): string[] { + return Object.keys(fields).filter((key) => isDefaultColumnField(fields[key])) +} + +/** + * Marks a list's `createdAt`/`updatedAt` fields as excluded from default + * columns when they are this list's structural, system-managed timestamp + * columns — identified by the list's own timestamp configuration + * (`db.timestamps`, per-list or global), not by the field names alone. A + * field's own explicit `ui.listView.defaultColumn` always wins over this. + * + * Only meaningful server-side, where the list's `db` config is available; + * the client fallback never calls this and instead reads whatever flag + * already reached it on the field (see `computeDefaultColumns`). + */ +export function withStructuralTimestampDefaults( + fields: Record, + // eslint-disable-next-line @typescript-eslint/no-explicit-any -- ListConfig is generic over TypeInfo + listConfig: Pick, 'db'>, + dbConfig: DatabaseConfig | undefined, +): Record { + const timestampsEnabled = listConfig.db?.timestamps ?? dbConfig?.timestamps ?? false + if (!timestampsEnabled) return fields + + let result = fields + for (const name of ['createdAt', 'updatedAt']) { + const field = result[name] + if (!field || field.ui?.listView?.defaultColumn !== undefined) continue + if (result === fields) result = { ...fields } + result[name] = { + ...field, + ui: { ...field.ui, listView: { ...field.ui?.listView, defaultColumn: false } }, + } + } + return result +} diff --git a/packages/ui/src/lib/deriveItemView.ts b/packages/ui/src/lib/deriveItemView.ts index 6462311d..2c8bd0af 100644 --- a/packages/ui/src/lib/deriveItemView.ts +++ b/packages/ui/src/lib/deriveItemView.ts @@ -1,4 +1,5 @@ -import type { FieldConfig, ListConfig, OpenSaasConfig } from '@opensaas/stack-core' +import type { DatabaseConfig, FieldConfig, ListConfig, OpenSaasConfig } from '@opensaas/stack-core' +import { computeDefaultColumns, withStructuralTimestampDefaults } from './defaultColumns.js' /** * The container arrangement of an item view, DERIVED from the number of @@ -79,13 +80,6 @@ export interface ItemViewLayout { arrangement: ItemViewArrangement } -/** - * System timestamp columns never shown by default, mirroring the list view's - * own default curation (`ListViewClient`). Password fields are excluded - * separately, by type rather than name (see {@link defaultColumnsFor}). - */ -const DEFAULT_EXCLUDED_COLUMNS = new Set(['createdAt', 'updatedAt']) - function readStringArray(value: unknown): string[] | undefined { return Array.isArray(value) && value.every((entry) => typeof entry === 'string') ? (value as string[]) @@ -145,18 +139,24 @@ function isToManyRelationship(field: FieldConfig): boolean { return field.type === 'relationship' && 'many' in field && field.many === true } -/** The related list's own column curation (`ui.listView.initialColumns`, else all non-system fields), minus the back-reference to the parent. */ +/** + * The related list's own column curation, minus the back-reference to the + * parent: `ui.listView.initialColumns` when set, else every field whose own + * `ui.listView.defaultColumn` declaration holds (issue #1018) — including + * the list's structural `createdAt`/`updatedAt` timestamp columns, excluded + * via {@link withStructuralTimestampDefaults} rather than by name. + */ function defaultColumnsFor( // eslint-disable-next-line @typescript-eslint/no-explicit-any -- ListConfig is generic over TypeInfo relatedListConfig: ListConfig | undefined, backReferenceField: string | undefined, + dbConfig: DatabaseConfig | undefined, ): string[] { if (!relatedListConfig) return [] const curated = relatedListConfig.ui?.listView?.initialColumns ?? - Object.keys(relatedListConfig.fields).filter( - (key) => - relatedListConfig.fields[key]?.type !== 'password' && !DEFAULT_EXCLUDED_COLUMNS.has(key), + computeDefaultColumns( + withStructuralTimestampDefaults(relatedListConfig.fields, relatedListConfig, dbConfig), ) return curated.filter((column) => column !== backReferenceField) } @@ -214,7 +214,8 @@ export function deriveItemViewLayout(config: OpenSaasConfig, listKey: string): I ref, relatedListKey, backReferenceField, - columns: overrides.columns ?? defaultColumnsFor(relatedListConfig, backReferenceField), + columns: + overrides.columns ?? defaultColumnsFor(relatedListConfig, backReferenceField, config.db), take: overrides.take, sumColumns: overrides.sum ?? [], removeAction: overrides.removeAction, diff --git a/packages/ui/src/lib/serializeFieldConfig.ts b/packages/ui/src/lib/serializeFieldConfig.ts index 0d7ae8f4..b15069a6 100644 --- a/packages/ui/src/lib/serializeFieldConfig.ts +++ b/packages/ui/src/lib/serializeFieldConfig.ts @@ -37,6 +37,10 @@ export type SerializableFieldConfig = { fieldType?: string /** Help / description text surfaced to the field component as `helpText`. */ description?: string + /** Whether this field belongs in a list's DEFAULT column set (issue #1018) — see `BaseFieldConfig.ui.listView`. */ + listView?: { + defaultColumn?: boolean + } [key: string]: unknown } } diff --git a/packages/ui/tests/components/ListTable.test.tsx b/packages/ui/tests/components/ListTable.test.tsx index c4a3acb2..b5357af9 100644 --- a/packages/ui/tests/components/ListTable.test.tsx +++ b/packages/ui/tests/components/ListTable.test.tsx @@ -439,34 +439,49 @@ describe('ListTable', () => { }) }) - describe('column filtering', () => { - it('should exclude password fields by default', () => { + describe('column filtering (issue #1018)', () => { + it('excludes a field whose fields metadata declares ui.listView.defaultColumn: false', () => { const items = [{ id: '1', username: 'john', password: 'secret123' }] - render() + render( + , + ) expect(screen.getByText('Username')).toBeInTheDocument() expect(screen.queryByText('Password')).not.toBeInTheDocument() }) - it('should exclude a password-typed field regardless of its name', () => { - const items = [{ id: '1', username: 'john', secret: 'hash...' }] + it('shows a field with no declaration, even one named or typed password', () => { + const items = [{ id: '1', password: 'plain text field' }] - render() + render( + , + ) - expect(screen.getByText('Username')).toBeInTheDocument() - expect(screen.queryByText('Secret')).not.toBeInTheDocument() + expect(screen.getByText('Password')).toBeInTheDocument() }) - it('should not exclude a field merely named password if it is not password-typed', () => { - const items = [{ id: '1', password: 'plain text field' }] + it('shows every column when no fields metadata is supplied at all', () => { + const items = [{ id: '1', username: 'john', secret: 'hash...' }] - render() + render() - expect(screen.getByText('Password')).toBeInTheDocument() + expect(screen.getByText('Username')).toBeInTheDocument() + expect(screen.getByText('Secret')).toBeInTheDocument() }) - it('should exclude createdAt and updatedAt by default', () => { + it('excludes createdAt/updatedAt when the fields metadata declares them out', () => { const items = [ { id: '1', @@ -484,6 +499,11 @@ describe('ListTable', () => { createdAt: 'timestamp', updatedAt: 'timestamp', }} + fields={{ + title: { type: 'text' }, + createdAt: { type: 'timestamp', ui: { listView: { defaultColumn: false } } }, + updatedAt: { type: 'timestamp', ui: { listView: { defaultColumn: false } } }, + }} />, ) diff --git a/packages/ui/tests/components/ListView.test.tsx b/packages/ui/tests/components/ListView.test.tsx index a19e07f0..872e7034 100644 --- a/packages/ui/tests/components/ListView.test.tsx +++ b/packages/ui/tests/components/ListView.test.tsx @@ -2,7 +2,7 @@ import { describe, it, expect, vi } from 'vitest' import * as React from 'react' import type { AccessContext, OpenSaasConfig } from '@opensaas/stack-core' import { list } from '@opensaas/stack-core' -import { text, relationship, select, virtual } from '@opensaas/stack-core/fields' +import { text, relationship, select, virtual, timestamp } from '@opensaas/stack-core/fields' import { ListView } from '../../src/components/ListView.js' import { ListViewClient, type ListViewClientProps } from '../../src/components/ListViewClient.js' @@ -496,3 +496,43 @@ describe('ListView to-one relationship label filter (issue #749 / #916)', () => expect(count).toHaveBeenCalledWith({ where: expectedWhere }) }) }) + +describe('ListView default-column curation (issue #1018)', () => { + it("bakes ui.listView.defaultColumn: false into createdAt/updatedAt when the list's timestamps resolve enabled", async () => { + const config: OpenSaasConfig = { + db: { provider: 'sqlite', url: 'file:./test.db', timestamps: true }, + lists: { + Post: list({ + fields: { title: text(), createdAt: timestamp(), updatedAt: timestamp() }, + }), + }, + } + const context = makeContext({ + post: { findMany: vi.fn(async () => []), count: vi.fn(async () => 0) }, + }) + + const tree = await ListView({ context, config, listKey: 'Post', basePath: '/admin' }) + const props = findListViewClientProps(tree) + + expect(props.fields?.createdAt?.ui?.listView?.defaultColumn).toBe(false) + expect(props.fields?.updatedAt?.ui?.listView?.defaultColumn).toBe(false) + expect(props.fields?.title?.ui?.listView?.defaultColumn).toBeUndefined() + }) + + it("leaves a field literally named createdAt alone when the list's timestamps are not enabled", async () => { + const config: OpenSaasConfig = { + db: { provider: 'sqlite', url: 'file:./test.db' }, + lists: { + Post: list({ fields: { title: text(), createdAt: text() } }), + }, + } + const context = makeContext({ + post: { findMany: vi.fn(async () => []), count: vi.fn(async () => 0) }, + }) + + const tree = await ListView({ context, config, listKey: 'Post', basePath: '/admin' }) + const props = findListViewClientProps(tree) + + expect(props.fields?.createdAt?.ui?.listView?.defaultColumn).toBeUndefined() + }) +}) diff --git a/packages/ui/tests/components/ListViewClient.test.tsx b/packages/ui/tests/components/ListViewClient.test.tsx index da53961d..57199e2e 100644 --- a/packages/ui/tests/components/ListViewClient.test.tsx +++ b/packages/ui/tests/components/ListViewClient.test.tsx @@ -505,8 +505,8 @@ describe('ListViewClient', () => { }) }) - describe('column filtering', () => { - it('should exclude password-typed columns by default regardless of field name', () => { + describe('column filtering (issue #1018)', () => { + it('excludes a field whose fields metadata declares ui.listView.defaultColumn: false', () => { const items = [{ id: '1', username: 'john', secret: 'hash...' }] render( @@ -514,6 +514,10 @@ describe('ListViewClient', () => { {...defaultProps} items={items} fieldTypes={{ username: 'text', secret: 'password' }} + fields={{ + username: { type: 'text' }, + secret: { type: 'password', ui: { listView: { defaultColumn: false } } }, + }} />, ) @@ -521,13 +525,35 @@ describe('ListViewClient', () => { expect(screen.queryByText('Secret')).not.toBeInTheDocument() }) - it('should not exclude a field merely named password if it is not password-typed', () => { + it('shows a field with no declaration, even one named or typed password', () => { const items = [{ id: '1', password: 'plain text field' }] - render() + render( + , + ) expect(screen.getByText('Password')).toBeInTheDocument() }) + + it('shows every column when no fields metadata is supplied at all', () => { + const items = [{ id: '1', username: 'john', secret: 'hash...' }] + + render( + , + ) + + expect(screen.getByText('Username')).toBeInTheDocument() + expect(screen.getByText('Secret')).toBeInTheDocument() + }) }) describe('edit links', () => { diff --git a/packages/ui/tests/lib/defaultColumns.test.ts b/packages/ui/tests/lib/defaultColumns.test.ts new file mode 100644 index 00000000..b672c6a3 --- /dev/null +++ b/packages/ui/tests/lib/defaultColumns.test.ts @@ -0,0 +1,105 @@ +import { describe, it, expect } from 'vitest' +import { + computeDefaultColumns, + isDefaultColumnField, + withStructuralTimestampDefaults, +} from '../../src/lib/defaultColumns.js' + +describe('isDefaultColumnField', () => { + it('is true for a field with no declaration', () => { + expect(isDefaultColumnField({ type: 'text' } as never)).toBe(true) + expect(isDefaultColumnField(undefined)).toBe(true) + }) + + it('is false only when explicitly declared false', () => { + expect(isDefaultColumnField({ ui: { listView: { defaultColumn: false } } })).toBe(false) + expect(isDefaultColumnField({ ui: { listView: { defaultColumn: true } } })).toBe(true) + expect(isDefaultColumnField({ ui: {} })).toBe(true) + }) +}) + +describe('computeDefaultColumns', () => { + it('includes every field whose declaration holds, in declaration order', () => { + const fields = { + title: { type: 'text' }, + secret: { type: 'password', ui: { listView: { defaultColumn: false } } }, + status: { type: 'select' }, + } + + expect(computeDefaultColumns(fields)).toEqual(['title', 'status']) + }) + + it('does not exclude a field merely named password or createdAt with no declaration', () => { + const fields = { + password: { type: 'text' }, + createdAt: { type: 'text' }, + } + + expect(computeDefaultColumns(fields)).toEqual(['password', 'createdAt']) + }) +}) + +describe('withStructuralTimestampDefaults', () => { + it('leaves fields untouched when timestamps are not enabled for the list', () => { + const fields = { createdAt: { type: 'timestamp' }, title: { type: 'text' } } + const result = withStructuralTimestampDefaults( + fields, + { db: undefined }, + { provider: 'sqlite' }, + ) + + expect(result).toBe(fields) + expect(computeDefaultColumns(result)).toEqual(['createdAt', 'title']) + }) + + it('excludes createdAt/updatedAt when the list resolves timestamps enabled', () => { + const fields = { + createdAt: { type: 'timestamp' }, + updatedAt: { type: 'timestamp' }, + title: { type: 'text' }, + } + const result = withStructuralTimestampDefaults( + fields, + { db: undefined }, + { provider: 'sqlite', timestamps: true }, + ) + + expect(computeDefaultColumns(result)).toEqual(['title']) + // Original map is not mutated. + expect(fields.createdAt.ui).toBeUndefined() + }) + + it('honours a per-list db.timestamps override', () => { + const fields = { createdAt: { type: 'timestamp' }, title: { type: 'text' } } + const result = withStructuralTimestampDefaults( + fields, + { db: { timestamps: true } }, + { provider: 'sqlite' }, + ) + + expect(computeDefaultColumns(result)).toEqual(['title']) + }) + + it("does not exclude a field literally named createdAt when the list's timestamps are off", () => { + // An application field that just happens to be named createdAt/updatedAt, + // unrelated to the list's own auto-timestamp column. + const fields = { createdAt: { type: 'text' }, title: { type: 'text' } } + const result = withStructuralTimestampDefaults(fields, { db: undefined }, undefined) + + expect(computeDefaultColumns(result)).toEqual(['createdAt', 'title']) + }) + + it("respects the field's own explicit declaration over the structural default", () => { + const fields = { + createdAt: { type: 'timestamp', ui: { listView: { defaultColumn: true } } }, + title: { type: 'text' }, + } + const result = withStructuralTimestampDefaults( + fields, + { db: undefined }, + { provider: 'sqlite', timestamps: true }, + ) + + expect(computeDefaultColumns(result)).toEqual(['createdAt', 'title']) + }) +}) diff --git a/packages/ui/tests/lib/deriveItemView.test.ts b/packages/ui/tests/lib/deriveItemView.test.ts index f2ac085e..cddcc4b0 100644 --- a/packages/ui/tests/lib/deriveItemView.test.ts +++ b/packages/ui/tests/lib/deriveItemView.test.ts @@ -1,6 +1,7 @@ import { describe, it, expect } from 'vitest' import { deriveItemViewLayout, DEFAULT_ITEM_VIEW_TAKE } from '../../src/lib/deriveItemView.js' import type { OpenSaasConfig } from '@opensaas/stack-core' +import { password } from '@opensaas/stack-core/fields' /** * Build a minimal config whose lists/fields carry just enough shape for the @@ -356,13 +357,14 @@ describe('deriveItemViewLayout', () => { expect(section.columns).toEqual(['name']) }) - it('excludes a password-typed related-list column by type, regardless of its name', () => { + it('excludes a real password() field from related-list default columns via its declared flag (#1018)', () => { const config = makeConfig({ Team: { fields: { members: { type: 'relationship', ref: 'Member', many: true } }, }, Member: { - fields: { name: { type: 'text' }, secret: { type: 'password' } }, + // eslint-disable-next-line @typescript-eslint/no-explicit-any -- test fixture, not typed against Member's TypeInfo + fields: { name: { type: 'text' }, secret: password() as any }, }, }) @@ -370,17 +372,75 @@ describe('deriveItemViewLayout', () => { expect(section.columns).toEqual(['name']) }) - it('does not exclude a related-list column merely named password if it is not password-typed', () => { + it('does not exclude a related-list column merely named or typed password with no declared flag', () => { const config = makeConfig({ Team: { fields: { members: { type: 'relationship', ref: 'Member', many: true } }, }, Member: { - fields: { name: { type: 'text' }, password: { type: 'text' } }, + // A hand-rolled 'password'-typed field with no ui.listView declaration + // is no longer excluded by type alone — curation is flag-based only. + fields: { + name: { type: 'text' }, + password: { type: 'text' }, + secret: { type: 'password' }, + }, + }, + }) + + const [section] = deriveItemViewLayout(config, 'Team').sections + expect(section.columns).toEqual(['name', 'password', 'secret']) + }) + + it('excludes a related-list field declaring ui.listView.defaultColumn: false, whatever its name', () => { + const config = makeConfig({ + Team: { + fields: { members: { type: 'relationship', ref: 'Member', many: true } }, + }, + Member: { + fields: { + name: { type: 'text' }, + internalScore: { type: 'integer', ui: { listView: { defaultColumn: false } } }, + }, + }, + }) + + const [section] = deriveItemViewLayout(config, 'Team').sections + expect(section.columns).toEqual(['name']) + }) + + it("excludes the related list's structural createdAt/updatedAt columns, identified by its own timestamp config", () => { + const config: OpenSaasConfig = { + db: { provider: 'sqlite', url: 'file:./test.db', timestamps: true }, + lists: { + Team: { + fields: { members: { type: 'relationship', ref: 'Member', many: true } }, + }, + Member: { + fields: { + name: { type: 'text' }, + createdAt: { type: 'timestamp' }, + updatedAt: { type: 'timestamp' }, + }, + }, + }, + } + + const [section] = deriveItemViewLayout(config, 'Team').sections + expect(section.columns).toEqual(['name']) + }) + + it("does not exclude a related-list field literally named createdAt when the list's timestamps are off", () => { + const config = makeConfig({ + Team: { + fields: { members: { type: 'relationship', ref: 'Member', many: true } }, + }, + Member: { + fields: { name: { type: 'text' }, createdAt: { type: 'text' } }, }, }) const [section] = deriveItemViewLayout(config, 'Team').sections - expect(section.columns).toEqual(['name', 'password']) + expect(section.columns).toEqual(['name', 'createdAt']) }) }) From 10f9b509229de86545283c20e098cc0b6a29f687 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 22 Aug 2026 11:00:57 +0000 Subject: [PATCH 2/2] fix(ui): don't drop fieldTypes columns absent from a partial fields map ListViewClient/ListTable derived the default column set from Object.keys(fields), but fields may legitimately cover only a subset of fieldTypes (columnField/getFieldConfig already synthesize a fallback for any column missing an entry). A column absent from fields was silently dropped instead of defaulting to shown. Curate off fieldTypes' own key set instead, consulting fields[key] only for the declaration. Found in code review of #1020 (chatgpt-codex-connector). Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Pydn7Pah2bUAZnKCDzhvXU --- packages/ui/src/components/ListViewClient.tsx | 27 ++++++++++++------- .../src/components/standalone/ListTable.tsx | 24 ++++++++++------- .../ui/tests/components/ListTable.test.tsx | 17 ++++++++++++ .../tests/components/ListViewClient.test.tsx | 20 ++++++++++++++ 4 files changed, 68 insertions(+), 20 deletions(-) diff --git a/packages/ui/src/components/ListViewClient.tsx b/packages/ui/src/components/ListViewClient.tsx index 293e202e..72b201c9 100644 --- a/packages/ui/src/components/ListViewClient.tsx +++ b/packages/ui/src/components/ListViewClient.tsx @@ -24,7 +24,7 @@ import { BulkActions, type SerializedBulkAction } from './BulkActions.js' import { useRowSelection, getPageCheckboxState } from '../lib/useRowSelection.js' import { useBulkStatus } from '../lib/useBulkStatus.js' import type { SerializableFieldConfig } from '../lib/serializeFieldConfig.js' -import { computeDefaultColumns } from '../lib/defaultColumns.js' +import { isDefaultColumnField } from '../lib/defaultColumns.js' import type { ServerActionInput } from '../server/types.js' import type { FilterFieldSuggestion } from '@opensaas/stack-core' @@ -70,9 +70,13 @@ export interface ListViewClientProps { fieldTypes: Record /** * Serialised per-field config, keyed by field name. Drives Cell resolution - * (per-field override, select option variants, relationship ref). Optional so - * callers that only have `fieldTypes` still render via the field-type - * registry; a minimal config is synthesised from `fieldTypes` when absent. + * (per-field override, select option variants, relationship ref) and, absent + * an explicit `columns` list, default-column curation (issue #1018, see + * `isDefaultColumnField`). Optional, and may cover only a subset of + * `fieldTypes`' keys — `columnField` below synthesises a fallback per-column + * for cell rendering, and a column missing an entry here has no + * `ui.listView.defaultColumn` declaration to curate by, so it defaults to + * shown, the same as when `fields` is omitted entirely. */ fields?: Record relationshipRefs: Record @@ -185,13 +189,16 @@ export function ListViewClient({ } // Absent an explicit `columns` list, curate off each field's own declared - // `ui.listView.defaultColumn` (issue #1018) via the same shared function - // `deriveItemView.ts`'s server-side layout helper uses — no field name/type - // matching here. When `fields` metadata isn't supplied at all (a caller - // with only `fieldTypes`), there is nothing to curate by, so every column - // shows. + // `ui.listView.defaultColumn` (issue #1018) — the same declaration + // `deriveItemView.ts`'s server-side layout helper reads, no field + // name/type matching here. The column set itself still comes from + // `fieldTypes` (not `Object.keys(fields)`): `fields` may cover only a + // subset of columns (`columnField` above synthesises a fallback for any + // column missing from it), and a column absent from `fields` has no + // declaration to curate by, so it defaults to shown — same as when + // `fields` is omitted entirely. const displayColumns = - columns || (fields ? computeDefaultColumns(fields) : Object.keys(fieldTypes)) + columns || Object.keys(fieldTypes).filter((key) => isDefaultColumnField(fields?.[key])) // Items are already sorted by the server via orderBy; no in-memory sort needed. diff --git a/packages/ui/src/components/standalone/ListTable.tsx b/packages/ui/src/components/standalone/ListTable.tsx index 9e69d4c7..2ed33005 100644 --- a/packages/ui/src/components/standalone/ListTable.tsx +++ b/packages/ui/src/components/standalone/ListTable.tsx @@ -15,7 +15,7 @@ import { import { EmptyState } from '../EmptyState.js' import { CellRenderer } from '../cells/CellRenderer.js' import type { SerializableFieldConfig } from '../../lib/serializeFieldConfig.js' -import { computeDefaultColumns } from '../../lib/defaultColumns.js' +import { isDefaultColumnField } from '../../lib/defaultColumns.js' /** * Per-part `classNames` slots for `ListTable` (issue #709). Each slot is merged @@ -61,11 +61,14 @@ export interface ListTableProps { */ fieldOptions?: Record> /** - * Serialised per-field config, keyed by field name (issue #1018). When - * supplied, an explicit `columns` list absent, the default columns are - * curated off each field's own `ui.listView.defaultColumn` declaration - * (see `computeDefaultColumns`) instead of showing every `fieldTypes` key. - * Omit when you have no field config to hand — every column shows. + * Serialised per-field config, keyed by field name (issue #1018), used only + * for default-column curation — not for cell rendering, which always + * synthesises from `fieldTypes`/`relationshipRefs`/`fieldOptions` (see + * `getFieldConfig`). When supplied, an explicit `columns` list absent, a + * column is excluded only if its entry here declares + * `ui.listView.defaultColumn: false` (see `isDefaultColumnField`); a column + * missing an entry — including every column when `fields` is omitted + * entirely — has nothing to curate by, so it defaults to shown. */ fields?: Record basePath?: string @@ -128,11 +131,12 @@ export function ListTable({ }) // Absent an explicit `columns` list, curate off each field's own declared - // `ui.listView.defaultColumn` (issue #1018) when `fields` metadata was - // supplied; with no `fields` at all there's nothing to curate by, so every - // column shows. + // `ui.listView.defaultColumn` (issue #1018). The column set comes from + // `fieldTypes` (not `Object.keys(fields)`), since `fields` may cover only + // a subset of columns — one missing from it has no declaration to curate + // by, so it defaults to shown, same as when `fields` is omitted entirely. const displayColumns = - columns || (fields ? computeDefaultColumns(fields) : Object.keys(fieldTypes)) + columns || Object.keys(fieldTypes).filter((key) => isDefaultColumnField(fields?.[key])) const sortedItems = [...items] if (sortBy && sortable) { diff --git a/packages/ui/tests/components/ListTable.test.tsx b/packages/ui/tests/components/ListTable.test.tsx index b5357af9..0410ff25 100644 --- a/packages/ui/tests/components/ListTable.test.tsx +++ b/packages/ui/tests/components/ListTable.test.tsx @@ -481,6 +481,23 @@ describe('ListTable', () => { expect(screen.getByText('Secret')).toBeInTheDocument() }) + it('shows a fieldTypes column with no entry in a partial fields map (does not drop it)', () => { + const items = [{ id: '1', username: 'john', views: 100, secret: 'hash...' }] + + render( + , + ) + + expect(screen.getByText('Username')).toBeInTheDocument() + expect(screen.getByText('Views')).toBeInTheDocument() + expect(screen.queryByText('Secret')).not.toBeInTheDocument() + }) + it('excludes createdAt/updatedAt when the fields metadata declares them out', () => { const items = [ { diff --git a/packages/ui/tests/components/ListViewClient.test.tsx b/packages/ui/tests/components/ListViewClient.test.tsx index 57199e2e..b7de8efe 100644 --- a/packages/ui/tests/components/ListViewClient.test.tsx +++ b/packages/ui/tests/components/ListViewClient.test.tsx @@ -554,6 +554,26 @@ describe('ListViewClient', () => { expect(screen.getByText('Username')).toBeInTheDocument() expect(screen.getByText('Secret')).toBeInTheDocument() }) + + it('shows a fieldTypes column with no entry in a partial fields map (does not drop it)', () => { + const items = [{ id: '1', username: 'john', views: 100, secret: 'hash...' }] + + render( + , + ) + + expect(screen.getByText('Username')).toBeInTheDocument() + expect(screen.getByText('Views')).toBeInTheDocument() + expect(screen.queryByText('Secret')).not.toBeInTheDocument() + }) }) describe('edit links', () => {