From e8ee4ee6874ce4180c817278fc68b24673240ad6 Mon Sep 17 00:00:00 2001 From: Mostafa Sadeghi <205455727+mostafasadeghidev@users.noreply.github.com> Date: Thu, 6 Aug 2026 10:05:49 +0200 Subject: [PATCH 1/3] feat(loops): filter data-row loops by a cell value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A loop could pick a table and an order but not WHICH rows, so a section that should list the three featured articles listed the three newest ones. Migrated Webflow sites hit this immediately: their lists are curated by a boolean field the loop had no way to read. - New `@core/loops/cellFilter`: parse one condition out of the loop's filter bag and render it as SQL. Six operators (is / is not / checked / unchecked / has any value / empty), a closed set — never interpolated. - Both query paths apply it (post-type version join and data-kind direct read) and so do their COUNT queries, or pagination advertises rows the page query drops. - The canvas preview endpoint takes the same condition, so the editor shows what the published page will emit. - Properties panel renders a field picker from the selected table, and hides the value box for the operators that ignore it. Dialect notes, both learned the hard way and now pinned by tests: SQLite binds `?` by position in the TEXT, so the condition's parameters sit between tableId and limit/offset; and its json_extract returns INTEGER 1/0 for booleans, which never equals '1' across storage classes — hence the cast. The JSON read appears exactly once per fragment so the field name binds once, with coalesce folding in rows that lack the field. The field NAME binds as a parameter like the value, so no part of a filter reaches the statement text. Co-Authored-By: Claude Fable 5 --- server/handlers/cms/data/tables.ts | 10 ++ src/__tests__/loops/cellFilter.test.ts | 165 ++++++++++++++++++ .../loops/dataRowsCellFilter.test.ts | 165 ++++++++++++++++++ .../pages/site/canvas/useLoopPreviewItems.ts | 9 +- .../PropertiesPanel/LoopPropertiesView.tsx | 25 ++- src/core/loops/cellFilter.ts | 148 ++++++++++++++++ src/core/loops/sources/dataRows.ts | 101 ++++++++--- src/core/persistence/cmsData.ts | 7 + 8 files changed, 608 insertions(+), 22 deletions(-) create mode 100644 src/__tests__/loops/cellFilter.test.ts create mode 100644 src/__tests__/loops/dataRowsCellFilter.test.ts create mode 100644 src/core/loops/cellFilter.ts diff --git a/server/handlers/cms/data/tables.ts b/server/handlers/cms/data/tables.ts index 521a87339..083968058 100644 --- a/server/handlers/cms/data/tables.ts +++ b/server/handlers/cms/data/tables.ts @@ -40,6 +40,7 @@ import { normalizeDataTableFields } from '@core/data/fields' import { slugForTable } from '@core/data/cells' import { slugFromTitle } from '@core/utils/slug' import { fetchPublishedDataRowItems } from '@core/loops/sources/dataRows' +import { parseCellFilter } from '@core/loops/cellFilter' import { badRequest, jsonResponse, methodNotAllowed, readValidatedBody } from '../../../http' import { CMS_API_PREFIX, requestAuditContext } from '../shared' import { @@ -382,12 +383,21 @@ async function handleTableLoopPreview( const rawOffset = Number.parseInt(url.searchParams.get('offset') ?? '0', 10) const offset = Math.max(Number.isFinite(rawOffset) ? rawOffset : 0, 0) + // The canvas preview must apply the loop's cell condition too, or the + // editor shows rows the published page will not. + const cellFilter = parseCellFilter({ + cellField: url.searchParams.get('cellField') ?? '', + cellOperator: url.searchParams.get('cellOperator') ?? '', + cellValue: url.searchParams.get('cellValue') ?? '', + }) + const result = await fetchPublishedDataRowItems(db, { tableId, orderBy, direction, limit, offset, + cellFilter, }) return jsonResponse(result) } diff --git a/src/__tests__/loops/cellFilter.test.ts b/src/__tests__/loops/cellFilter.test.ts new file mode 100644 index 000000000..ea874295e --- /dev/null +++ b/src/__tests__/loops/cellFilter.test.ts @@ -0,0 +1,165 @@ +/** + * Unit tests for the loop cell filter — the pure half. + * + * Two properties matter and both are easy to get wrong: + * - a half-configured filter must never silently empty a list, and + * - the SQL must bind BOTH the field name and the value, so a field id + * can never reach the statement text. + * + * The SQL/TypeScript predicates are also checked against each other: they + * are two spellings of one rule, and the canvas uses one while the + * publisher uses the other. + */ +import { describe, expect, test } from 'bun:test' +import { + cellFilterMatches, + cellFilterSql, + parseCellFilter, + CELL_FILTER_OPERATORS, + type CellFilter, +} from '@core/loops/cellFilter' + +describe('parseCellFilter', () => { + test('returns null when no field is chosen', () => { + expect(parseCellFilter({})).toBeNull() + expect(parseCellFilter({ cellField: ' ' })).toBeNull() + }) + + test('a comparison without a value is treated as not-yet-configured', () => { + expect(parseCellFilter({ cellField: 'featured', cellOperator: 'is' })).toBeNull() + expect(parseCellFilter({ cellField: 'featured', cellOperator: 'isNot', cellValue: '' })).toBeNull() + }) + + test('valueless operators need no value', () => { + expect(parseCellFilter({ cellField: 'featured', cellOperator: 'isTrue' })) + .toEqual({ field: 'featured', operator: 'isTrue', value: '' }) + }) + + test('defaults to `is` and coerces non-string values', () => { + expect(parseCellFilter({ cellField: 'rank', cellValue: 3 })) + .toEqual({ field: 'rank', operator: 'is', value: '3' }) + expect(parseCellFilter({ cellField: 'live', cellValue: true })) + .toEqual({ field: 'live', operator: 'is', value: 'true' }) + }) + + test('an unknown operator falls back to `is` rather than breaking the query', () => { + expect(parseCellFilter({ cellField: 'a', cellOperator: 'DROP TABLE', cellValue: 'x' })) + .toEqual({ field: 'a', operator: 'is', value: 'x' }) + }) +}) + +describe('cellFilterSql', () => { + const filter: CellFilter = { field: 'team-on-about-page', operator: 'isTrue', value: '' } + + test('binds the field name as a parameter — never as SQL text', () => { + for (const dialect of ['postgres', 'sqlite'] as const) { + const { sql, params } = cellFilterSql({ filter, dialect, column: 'data_rows.cells_json', nextParamIndex: 4 }) + expect(sql).not.toContain('team-on-about-page') + expect(params[0]).toBe('team-on-about-page') + } + }) + + test('a hostile field id cannot escape into the statement', () => { + const hostile: CellFilter = { field: "x'); drop table data_rows; --", operator: 'is', value: 'y' } + const { sql, params } = cellFilterSql({ filter: hostile, dialect: 'sqlite', column: 'c', nextParamIndex: 2 }) + expect(sql).not.toContain('drop table') + expect(params).toEqual([hostile.field, 'y']) + }) + + test('placeholders follow the dialect and start at the given index', () => { + const pg = cellFilterSql({ filter: { field: 'f', operator: 'is', value: 'v' }, dialect: 'postgres', column: 'c', nextParamIndex: 4 }) + expect(pg.sql).toContain('$4') + expect(pg.sql).toContain('$5') + const sqlite = cellFilterSql({ filter: { field: 'f', operator: 'is', value: 'v' }, dialect: 'sqlite', column: 'c', nextParamIndex: 4 }) + expect(sqlite.sql).toContain('?') + expect(sqlite.sql).not.toContain('$4') + }) + + test('every operator produces a fragment with the right parameter count', () => { + const cases: Array<[CellFilter['operator'], number]> = [ + ['is', 2], ['isNot', 2], ['isTrue', 1], ['isFalse', 1], ['isSet', 1], ['isEmpty', 1], + ] + for (const [operator, paramCount] of cases) { + const { sql, params } = cellFilterSql({ + filter: { field: 'f', operator, value: 'v' }, + dialect: 'sqlite', + column: 'c', + nextParamIndex: 1, + }) + expect(sql.length).toBeGreaterThan(0) + expect(params).toHaveLength(paramCount) + } + }) + + test('the JSON read appears once per fragment, so the field binds once', () => { + // Repeating the expression would repeat its placeholder while the caller + // binds the field name a single time — the bug that made SQLite reject + // the statement with "expected 3 values, received 2". + for (const operator of CELL_FILTER_OPERATORS) { + const { sql, params } = cellFilterSql({ + filter: { field: 'f', operator, value: 'v' }, + dialect: 'sqlite', + column: 'c', + nextParamIndex: 1, + }) + expect(sql.match(/json_extract/g) ?? []).toHaveLength(1) + expect(sql.match(/\?/g) ?? []).toHaveLength(params.length) + } + }) + + test('missing cells fold into the comparison instead of vanishing', () => { + // `coalesce(…, '')` is what keeps a row that never set the field inside + // "is not X" and "is unchecked". + for (const operator of ['isNot', 'isFalse', 'isEmpty'] as const) { + const { sql } = cellFilterSql({ + filter: { field: 'f', operator, value: 'v' }, + dialect: 'sqlite', + column: 'c', + nextParamIndex: 1, + }) + expect(sql).toContain('coalesce') + } + }) + + test('SQLite casts the JSON read so boolean cells compare as text', () => { + const { sql } = cellFilterSql({ filter: { field: 'f', operator: 'isTrue', value: '' }, dialect: 'sqlite', column: 'c', nextParamIndex: 1 }) + // Without the cast, json_extract returns INTEGER 1 and `1 = '1'` is false. + expect(sql).toContain('cast(') + expect(sql).toContain("'1'") + }) +}) + +describe('cellFilterMatches mirrors the SQL semantics', () => { + const rows = { + featuredTrue: { featured: true, name: 'A' }, + featuredFalse: { featured: false, name: 'B' }, + missing: { name: 'C' }, + empty: { featured: '', name: 'D' }, + } + + test('isTrue only matches a true value', () => { + const f: CellFilter = { field: 'featured', operator: 'isTrue', value: '' } + expect(cellFilterMatches(f, rows.featuredTrue)).toBe(true) + expect(cellFilterMatches(f, rows.featuredFalse)).toBe(false) + expect(cellFilterMatches(f, rows.missing)).toBe(false) + }) + + test('isFalse matches false AND a missing field', () => { + const f: CellFilter = { field: 'featured', operator: 'isFalse', value: '' } + expect(cellFilterMatches(f, rows.featuredFalse)).toBe(true) + expect(cellFilterMatches(f, rows.missing)).toBe(true) + expect(cellFilterMatches(f, rows.featuredTrue)).toBe(false) + }) + + test('is / isNot compare as text', () => { + expect(cellFilterMatches({ field: 'name', operator: 'is', value: 'A' }, rows.featuredTrue)).toBe(true) + expect(cellFilterMatches({ field: 'name', operator: 'isNot', value: 'A' }, rows.featuredTrue)).toBe(false) + expect(cellFilterMatches({ field: 'name', operator: 'isNot', value: 'A' }, rows.featuredFalse)).toBe(true) + }) + + test('isSet / isEmpty treat an empty string as empty', () => { + expect(cellFilterMatches({ field: 'featured', operator: 'isSet', value: '' }, rows.empty)).toBe(false) + expect(cellFilterMatches({ field: 'featured', operator: 'isEmpty', value: '' }, rows.empty)).toBe(true) + expect(cellFilterMatches({ field: 'featured', operator: 'isEmpty', value: '' }, rows.missing)).toBe(true) + }) +}) diff --git a/src/__tests__/loops/dataRowsCellFilter.test.ts b/src/__tests__/loops/dataRowsCellFilter.test.ts new file mode 100644 index 000000000..f06aafa74 --- /dev/null +++ b/src/__tests__/loops/dataRowsCellFilter.test.ts @@ -0,0 +1,165 @@ +/** + * Behavior tests for the `data.rows` loop cell filter against a real + * migrated SQLite database. + * + * The pure half (parsing, SQL assembly) is covered in `cellFilter.test.ts`; + * what matters here is that the condition actually reaches the query on + * BOTH table kinds, that `totalItems` counts the filtered set (otherwise + * pagination advertises rows the page query drops), and that a filter on a + * field some rows lack behaves the way an author expects. + */ + +import { describe, expect, it, beforeAll, afterAll } from 'bun:test' +import { createTestDb, type TestDb } from '../helpers/createTestDb' +import { fetchPublishedDataRowItems } from '@core/loops/sources/dataRows' +import type { CellFilter } from '@core/loops/cellFilter' + +type Db = TestDb['db'] + +let testDb: TestDb +let db: Db + +async function seedPost( + rowId: string, + slug: string, + cells: Record, + publishedAt: string, +): Promise { + await db` + insert into data_rows (id, table_id, cells_json, slug, status, updated_at) + values (${rowId}, ${'posts'}, ${JSON.stringify(cells)}, ${slug}, ${'published'}, ${publishedAt}) + ` + await db` + insert into data_row_versions (id, row_id, version_number, cells_json, slug, published_at, created_at) + values (${`${rowId}-v1`}, ${rowId}, ${1}, ${JSON.stringify(cells)}, ${slug}, ${publishedAt}, ${publishedAt}) + ` + await db`update data_rows set active_version_id = ${`${rowId}-v1`} where id = ${rowId}` +} + +async function seedDataRow( + tableId: string, + rowId: string, + slug: string, + cells: Record, +): Promise { + await db` + insert into data_rows (id, table_id, cells_json, slug, status, created_at, updated_at) + values (${rowId}, ${tableId}, ${JSON.stringify(cells)}, ${slug}, ${'draft'}, ${'2024-01-01T00:00:00Z'}, ${'2024-01-01T00:00:00Z'}) + ` +} + +async function slugsWith(tableId: string, cellFilter: CellFilter | null): Promise { + const { items } = await fetchPublishedDataRowItems(db, { + tableId, + orderBy: 'slug', + direction: 'asc', + limit: 50, + offset: 0, + cellFilter, + }) + return items.map((item) => String(item.fields['slug'])) +} + +beforeAll(async () => { + testDb = await createTestDb() + db = testDb.db + + // Post-type rows: two featured, one not, one missing the field entirely — + // the exact shape that made a real migration list the wrong three items. + await seedPost('p-a', 'alpha', { title: 'Alpha', featured: true, tag: 'news' }, '2024-01-01T00:00:00Z') + await seedPost('p-b', 'bravo', { title: 'Bravo', featured: false, tag: 'news' }, '2024-01-02T00:00:00Z') + await seedPost('p-c', 'charlie', { title: 'Charlie', featured: true, tag: 'guide' }, '2024-01-03T00:00:00Z') + await seedPost('p-d', 'delta', { title: 'Delta' }, '2024-01-04T00:00:00Z') + + await db` + insert into data_tables (id, name, slug, kind, route_base, singular_label, plural_label, fields_json, system) + values ('logos', 'Logos', 'logos', 'data', '/logos', 'Logo', 'Logos', ${JSON.stringify([])}, 0) + ` + await seedDataRow('logos', 'l-a', 'acme', { name: 'Acme', member: true }) + await seedDataRow('logos', 'l-b', 'globex', { name: 'Globex', member: false }) + await seedDataRow('logos', 'l-c', 'initech', { name: 'Initech' }) +}) + +afterAll(async () => { + await testDb.cleanup() +}) + +describe('data.rows cell filter — post-type tables', () => { + it('no filter lists every published row', async () => { + expect(await slugsWith('posts', null)).toEqual(['alpha', 'bravo', 'charlie', 'delta']) + }) + + it('isTrue keeps only the marked rows', async () => { + expect(await slugsWith('posts', { field: 'featured', operator: 'isTrue', value: '' })) + .toEqual(['alpha', 'charlie']) + }) + + it('isFalse includes rows that lack the field', async () => { + expect(await slugsWith('posts', { field: 'featured', operator: 'isFalse', value: '' })) + .toEqual(['bravo', 'delta']) + }) + + it('is matches a text cell exactly', async () => { + expect(await slugsWith('posts', { field: 'tag', operator: 'is', value: 'news' })) + .toEqual(['alpha', 'bravo']) + }) + + it('isNot also returns rows missing the field', async () => { + expect(await slugsWith('posts', { field: 'tag', operator: 'isNot', value: 'news' })) + .toEqual(['charlie', 'delta']) + }) + + it('isSet / isEmpty split on presence', async () => { + expect(await slugsWith('posts', { field: 'tag', operator: 'isSet', value: '' })) + .toEqual(['alpha', 'bravo', 'charlie']) + expect(await slugsWith('posts', { field: 'tag', operator: 'isEmpty', value: '' })) + .toEqual(['delta']) + }) + + it('totalItems counts the filtered set, not the table', async () => { + const { items, totalItems } = await fetchPublishedDataRowItems(db, { + tableId: 'posts', + orderBy: 'slug', + direction: 'asc', + limit: 1, + offset: 0, + cellFilter: { field: 'featured', operator: 'isTrue', value: '' }, + }) + expect(items).toHaveLength(1) + expect(totalItems).toBe(2) + }) + + it('paginates within the filtered set', async () => { + const { items } = await fetchPublishedDataRowItems(db, { + tableId: 'posts', + orderBy: 'slug', + direction: 'asc', + limit: 5, + offset: 1, + cellFilter: { field: 'featured', operator: 'isTrue', value: '' }, + }) + expect(items.map((i) => String(i.fields['slug']))).toEqual(['charlie']) + }) + + it('an unknown field matches nothing rather than everything', async () => { + expect(await slugsWith('posts', { field: 'nope', operator: 'isTrue', value: '' })).toEqual([]) + }) +}) + +describe('data.rows cell filter — data-kind tables', () => { + it('applies on the direct-read path too', async () => { + expect(await slugsWith('logos', { field: 'member', operator: 'isTrue', value: '' })).toEqual(['acme']) + }) + + it('counts the filtered set on the data-kind path', async () => { + const { totalItems } = await fetchPublishedDataRowItems(db, { + tableId: 'logos', + orderBy: 'slug', + direction: 'asc', + limit: 50, + offset: 0, + cellFilter: { field: 'member', operator: 'isFalse', value: '' }, + }) + expect(totalItems).toBe(2) + }) +}) diff --git a/src/admin/pages/site/canvas/useLoopPreviewItems.ts b/src/admin/pages/site/canvas/useLoopPreviewItems.ts index 8b920d7ef..ef70c11ba 100644 --- a/src/admin/pages/site/canvas/useLoopPreviewItems.ts +++ b/src/admin/pages/site/canvas/useLoopPreviewItems.ts @@ -254,6 +254,10 @@ export function useLoopPreviewItems( const { sourceId, filters, orderBy, direction, offset, limit } = readLoopProps(node) const tableId = typeof filters.tableId === 'string' ? filters.tableId : '' const mimePrefix = typeof filters.mimePrefix === 'string' ? filters.mimePrefix : '' + // Read as primitives so the fetch effect's dependency list stays stable. + const cellField = typeof filters.cellField === 'string' ? filters.cellField : '' + const cellOperator = typeof filters.cellOperator === 'string' ? filters.cellOperator : '' + const cellValue = typeof filters.cellValue === 'string' ? filters.cellValue : '' const isPluginSource = sourceId !== '' && !BUILT_IN_SOURCE_IDS.has(sourceId) // Narrow, identity-stable subscriptions (see module header). Inactive @@ -300,6 +304,9 @@ export function useLoopPreviewItems( direction, limit, offset, + cellField, + cellOperator, + cellValue, }) .then((result) => { if (!cancelled) setAsyncDataRowItems(result.items) @@ -311,7 +318,7 @@ export function useLoopPreviewItems( return () => { cancelled = true } - }, [sourceId, tableId, orderBy, direction, limit, offset, previewReadiness]) + }, [sourceId, tableId, orderBy, direction, limit, offset, cellField, cellOperator, cellValue, previewReadiness]) // ── Async fetch: site.media ───────────────────────────────────────── useEffect(() => { diff --git a/src/admin/pages/site/panels/PropertiesPanel/LoopPropertiesView.tsx b/src/admin/pages/site/panels/PropertiesPanel/LoopPropertiesView.tsx index 2859c4dfb..6b77edb4f 100644 --- a/src/admin/pages/site/panels/PropertiesPanel/LoopPropertiesView.tsx +++ b/src/admin/pages/site/panels/PropertiesPanel/LoopPropertiesView.tsx @@ -65,7 +65,13 @@ export function LoopPropertiesView({ nodeId, props, activePage }: LoopProperties if (source.id === 'data.rows' && tables) { const tableField = source.filterSchema.tableId if (tableField && tableField.type === 'select') { - return { + const selectedTable = tables.find((t) => t.id === filters.tableId) + const cellFieldControl = source.filterSchema.cellField + const operator = typeof filters.cellOperator === 'string' ? filters.cellOperator : 'is' + // The value box is meaningless for the checkbox / emptiness operators, + // and a stale value in it would read as a live condition. + const valuelessOperator = ['isTrue', 'isFalse', 'isSet', 'isEmpty'].includes(operator) + const schema: PropertySchema = { ...source.filterSchema, tableId: { ...tableField, @@ -75,6 +81,23 @@ export function LoopPropertiesView({ nodeId, props, activePage }: LoopProperties ], }, } + if (cellFieldControl?.type === 'select') { + schema.cellField = { + ...cellFieldControl, + options: [ + { label: '— every row —', value: '' }, + ...(selectedTable?.fields ?? []).map((f) => ({ label: f.label || f.id, value: f.id })), + ], + } + } + // Condition + value only matter once a field is picked. + if (!filters.cellField) { + delete schema.cellOperator + delete schema.cellValue + } else if (valuelessOperator) { + delete schema.cellValue + } + return schema } } if (source.id === ENTRY_FIELD_SOURCE_ID && tables) { diff --git a/src/core/loops/cellFilter.ts b/src/core/loops/cellFilter.ts new file mode 100644 index 000000000..51f1f1d7c --- /dev/null +++ b/src/core/loops/cellFilter.ts @@ -0,0 +1,148 @@ +/** + * Cell filtering for data-row loops. + * + * A loop could pick a table and an order, but not *which* rows — so a page + * that should list three featured articles listed the three most recent + * ones instead. This adds one condition on a row's own cell, which is what + * "featured", "show on homepage" or "category = X" style lists need. + * + * The value lives inside `cells_json`, so the comparison needs JSON access — + * the one place the two dialects genuinely differ. `cellFilterSql` isolates + * that behind the same `db.dialect` switch `positionalParam` already uses; + * everything else (parsing, validation, the closed operator set) is pure and + * unit-tested here. + * + * Deliberately ONE condition, not a query builder: it covers the real cases + * without inventing an AND/OR grammar the editor cannot express and future + * maintainers would have to keep sound. + */ + +/** Operators a loop filter can use. Closed set — never interpolated raw. */ +export const CELL_FILTER_OPERATORS = ['is', 'isNot', 'isTrue', 'isFalse', 'isSet', 'isEmpty'] as const + +export type CellFilterOperator = (typeof CELL_FILTER_OPERATORS)[number] + +export interface CellFilter { + /** Field id as stored in `cells_json` (a data-table field id). */ + field: string + operator: CellFilterOperator + /** Compared value for `is` / `isNot`; ignored by the other operators. */ + value: string +} + +/** Operators that ignore the comparison value. */ +const VALUELESS: ReadonlySet = new Set(['isTrue', 'isFalse', 'isSet', 'isEmpty']) + +export function isCellFilterOperator(value: unknown): value is CellFilterOperator { + return typeof value === 'string' && (CELL_FILTER_OPERATORS as readonly string[]).includes(value) +} + +/** + * Read a filter out of a loop's free-form `filters` bag. + * + * Returns null whenever the filter is absent or unusable, so a half-configured + * loop (field picked, operator not yet) keeps listing everything instead of + * silently returning nothing. + */ +export function parseCellFilter(filters: Record): CellFilter | null { + const field = typeof filters.cellField === 'string' ? filters.cellField.trim() : '' + if (!field) return null + + const operator: CellFilterOperator = isCellFilterOperator(filters.cellOperator) + ? filters.cellOperator + : 'is' + + const rawValue = filters.cellValue + const value = typeof rawValue === 'string' + ? rawValue.trim() + : typeof rawValue === 'number' || typeof rawValue === 'boolean' + ? String(rawValue) + : '' + + // `is` / `isNot` without a value would filter on the empty string, which is + // never what an author means — treat it as "not configured yet". + if (!VALUELESS.has(operator) && !value) return null + + return { field, operator, value } +} + +/** + * SQL fragment + parameters for a cell filter. + * + * `column` is the qualified JSON column (`data_rows.cells_json` or + * `data_row_versions.cells_json`). `nextParamIndex` is the 1-based index the + * first parameter of this fragment takes in the statement's parameter list; + * `placeholder` renders it in the dialect's own style. + * + * The field NAME is a parameter too — never string-concatenated into the SQL — + * so a crafted field id cannot escape into the statement. + */ +export function cellFilterSql(input: { + filter: CellFilter + dialect: 'postgres' | 'sqlite' + column: string + nextParamIndex: number +}): { sql: string; params: unknown[] } { + const { filter, dialect, column, nextParamIndex } = input + const placeholder = (offset: number) => + dialect === 'postgres' ? `$${nextParamIndex + offset}` : '?' + + // Postgres: `cells_json #>> array[key]` reads a text value at a dynamic key. + // SQLite: `json_extract(cells_json, '$.' || key)` does the same. Both take + // the key as a bound parameter. + // + // Two shapes matter here: + // - The expression appears EXACTLY ONCE per fragment. Repeating it would + // repeat its placeholder, and the caller binds the field name once. + // `coalesce(…, '')` folds the missing-field case into the comparison + // instead of needing a second `is null` branch. + // - Booleans do not read back identically: Postgres yields 'true'/'false' + // text, SQLite's json_extract yields the INTEGERS 1/0. SQLite compares + // across storage classes by class first, so `1 = '1'` is false — hence + // the cast, and hence the operators accepting both spellings. + const rawValue = dialect === 'postgres' + ? `(${column} #>> array[${placeholder(0)}])` + : `cast(json_extract(${column}, '$.' || ${placeholder(0)}) as text)` + const textValue = `coalesce(${rawValue}, '')` + + switch (filter.operator) { + case 'is': + return { sql: `${textValue} = ${placeholder(1)}`, params: [filter.field, filter.value] } + case 'isNot': + // A row missing the field is "not X" — the coalesce keeps it in. + return { sql: `${textValue} <> ${placeholder(1)}`, params: [filter.field, filter.value] } + case 'isTrue': + return { sql: `${textValue} in ('true', '1')`, params: [filter.field] } + case 'isFalse': + // Unchecked includes rows where the field was never set. + return { sql: `${textValue} in ('false', '0', '')`, params: [filter.field] } + case 'isSet': + return { sql: `${textValue} <> ''`, params: [filter.field] } + case 'isEmpty': + return { sql: `${textValue} = ''`, params: [filter.field] } + } +} + +/** + * The same predicate in TypeScript, for callers holding rows rather than a + * query — the canvas preview and any future in-memory path. Keeping it beside + * the SQL keeps the two definitions honest about each other. + */ +export function cellFilterMatches(filter: CellFilter, cells: Record): boolean { + const raw = cells[filter.field] + const text = raw === null || raw === undefined + ? null + : typeof raw === 'string' ? raw : typeof raw === 'number' || typeof raw === 'boolean' ? String(raw) : JSON.stringify(raw) + + // Mirrors the SQL exactly: a missing cell reads as the empty string, and + // the checked/unchecked operators accept both boolean spellings. + const value = text ?? '' + switch (filter.operator) { + case 'is': return value === filter.value + case 'isNot': return value !== filter.value + case 'isTrue': return value === 'true' || value === '1' + case 'isFalse': return value === 'false' || value === '0' || value === '' + case 'isSet': return value !== '' + case 'isEmpty': return value === '' + } +} diff --git a/src/core/loops/sources/dataRows.ts b/src/core/loops/sources/dataRows.ts index ab96d8151..4bccaacc6 100644 --- a/src/core/loops/sources/dataRows.ts +++ b/src/core/loops/sources/dataRows.ts @@ -20,6 +20,7 @@ */ import type { LoopEntitySource, LoopFetchResult, LoopItem, LoopSourceDb } from '@core/loops/types' +import { cellFilterSql, parseCellFilter, type CellFilter } from '../cellFilter' import { isoDate } from '../../utils/isoDate' import { firstImagePathFromMarkdown } from '@core/markdown/renderMarkdown' import { normalizeRouteBase } from '@core/templates/templateMatching' @@ -218,8 +219,18 @@ async function fetchPage( direction: 'asc' | 'desc', limit: number, offset: number, + filter: CellFilter | null, ): Promise { const orderColumn = POST_TYPE_ORDER_COLUMN[orderBy] + // SQLite binds `?` by POSITION IN THE TEXT, so the parameter list must follow + // the clause order: tableId, then the cell condition (WHERE), then + // limit/offset. Postgres indices are numbered to match. + const cell = filter + ? cellFilterSql({ filter, dialect: db.dialect, column: 'data_row_versions.cells_json', nextParamIndex: 2 }) + : null + const cellParams = cell?.params ?? [] + const limitParam = positionalParam(db, 2 + cellParams.length) + const offsetParam = positionalParam(db, 3 + cellParams.length) const { rows } = await db.unsafe( `select data_row_versions.id as version_id, data_rows.id as row_id, @@ -252,9 +263,10 @@ async function fetchPage( and data_rows.status = 'published' and data_rows.deleted_at is null and data_tables.deleted_at is null + ${cell ? `and ${cell.sql}` : ''} order by ${orderColumn} ${direction}, data_row_versions.id ${direction} - limit ${positionalParam(db, 2)} offset ${positionalParam(db, 3)}`, - [tableId, limit, offset], + limit ${limitParam} offset ${offsetParam}`, + [tableId, ...cellParams, limit, offset], ) return rows } @@ -357,10 +369,18 @@ async function fetchDataKindPage( direction: 'asc' | 'desc', limit: number, offset: number, + filter: CellFilter | null, ): Promise { const sortKey: 'createdAt' | 'updatedAt' | 'slug' = orderBy === 'updatedAt' ? 'updatedAt' : orderBy === 'slug' ? 'slug' : 'createdAt' const orderColumn = DATA_KIND_ORDER_COLUMN[sortKey] + // Parameter order follows the clause order — see `fetchPage`. + const cell = filter + ? cellFilterSql({ filter, dialect: db.dialect, column: 'data_rows.cells_json', nextParamIndex: 2 }) + : null + const cellParams = cell?.params ?? [] + const limitParam = positionalParam(db, 2 + cellParams.length) + const offsetParam = positionalParam(db, 3 + cellParams.length) // Same safety contract as `fetchPage`: the ORDER BY text comes only from // the closed map above; every runtime value is a positional parameter. @@ -384,9 +404,10 @@ async function fetchDataKindPage( where data_rows.table_id = ${positionalParam(db, 1)} and data_rows.deleted_at is null and data_tables.deleted_at is null + ${cell ? `and ${cell.sql}` : ''} order by ${orderColumn} ${direction}, data_rows.id ${direction} - limit ${positionalParam(db, 2)} offset ${positionalParam(db, 3)}`, - [tableId, limit, offset], + limit ${limitParam} offset ${offsetParam}`, + [tableId, ...cellParams, limit, offset], ) return rows } @@ -413,9 +434,12 @@ export async function fetchPublishedDataRowItems( direction: 'asc' | 'desc' limit: number offset: number + /** Optional condition on one of the row's own cells. */ + cellFilter?: CellFilter | null }, ): Promise { if (!opts.tableId) return { items: [], totalItems: 0 } + const cellFilter = opts.cellFilter ?? null const { rows: kindRows } = await db<{ kind: string }>` select kind @@ -433,17 +457,24 @@ export async function fetchPublishedDataRowItems( const direction: 'asc' | 'desc' = opts.direction === 'asc' ? 'asc' : 'desc' if (tableKind === 'data') { - const { rows: countRows } = await db<{ total: number }>` - select count(*) as total - from data_rows - where table_id = ${opts.tableId} - and deleted_at is null - ` + // The count must apply the same condition, or pagination advertises rows + // the page query filters out. + const dataCountCell = cellFilter + ? cellFilterSql({ filter: cellFilter, dialect: db.dialect, column: 'data_rows.cells_json', nextParamIndex: 2 }) + : null + const { rows: countRows } = await db.unsafe<{ total: number }>( + `select count(*) as total + from data_rows + where data_rows.table_id = ${positionalParam(db, 1)} + and data_rows.deleted_at is null + ${dataCountCell ? `and ${dataCountCell.sql}` : ''}`, + [opts.tableId, ...(dataCountCell?.params ?? [])], + ) const totalItems = Number(countRows[0]?.total ?? 0) if (totalItems === 0) return { items: [], totalItems: 0 } const sqlRows = await fetchDataKindPage( - db, opts.tableId, orderBy, direction, opts.limit, opts.offset, + db, opts.tableId, orderBy, direction, opts.limit, opts.offset, cellFilter, ) const mediaPathMap = await resolveMediaIdsToPaths(db, extractFeaturedMediaIds(sqlRows)) return { @@ -453,18 +484,23 @@ export async function fetchPublishedDataRowItems( } // Post-type path (default): only published rows, joined to active version. - const { rows: countRows } = await db<{ total: number }>` - select count(*) as total - from data_rows - join data_row_versions on data_row_versions.id = data_rows.active_version_id - where data_rows.table_id = ${opts.tableId} - and data_rows.status = 'published' - and data_rows.deleted_at is null - ` + const postCountCell = cellFilter + ? cellFilterSql({ filter: cellFilter, dialect: db.dialect, column: 'data_row_versions.cells_json', nextParamIndex: 2 }) + : null + const { rows: countRows } = await db.unsafe<{ total: number }>( + `select count(*) as total + from data_rows + join data_row_versions on data_row_versions.id = data_rows.active_version_id + where data_rows.table_id = ${positionalParam(db, 1)} + and data_rows.status = 'published' + and data_rows.deleted_at is null + ${postCountCell ? `and ${postCountCell.sql}` : ''}`, + [opts.tableId, ...(postCountCell?.params ?? [])], + ) const totalItems = Number(countRows[0]?.total ?? 0) if (totalItems === 0) return { items: [], totalItems: 0 } - const sqlRows = await fetchPage(db, opts.tableId, orderBy, direction, opts.limit, opts.offset) + const sqlRows = await fetchPage(db, opts.tableId, orderBy, direction, opts.limit, opts.offset, cellFilter) const mediaPathMap = await resolveMediaIdsToPaths(db, extractFeaturedMediaIds(sqlRows)) return { @@ -491,6 +527,30 @@ export const DataRowsSource: LoopEntitySource = { // valid when the source is registered before the table list is loaded. options: [], }, + // Optional condition on one of the row's own cells: the difference + // between "the newest three" and "the three marked featured". Field + // options are populated per selected table by the Properties Panel. + cellField: { + type: 'select', + label: 'Only rows where', + options: [], + }, + cellOperator: { + type: 'select', + label: 'Condition', + options: [ + { label: 'is', value: 'is' }, + { label: 'is not', value: 'isNot' }, + { label: 'is checked', value: 'isTrue' }, + { label: 'is unchecked', value: 'isFalse' }, + { label: 'has any value', value: 'isSet' }, + { label: 'is empty', value: 'isEmpty' }, + ], + }, + cellValue: { + type: 'text', + label: 'Value', + }, }, orderByOptions: [ @@ -526,6 +586,7 @@ export const DataRowsSource: LoopEntitySource = { direction: ctx.direction, limit: ctx.limit, offset: ctx.offset, + cellFilter: parseCellFilter(ctx.filters), }) }, diff --git a/src/core/persistence/cmsData.ts b/src/core/persistence/cmsData.ts index b7626b44b..7d755df86 100644 --- a/src/core/persistence/cmsData.ts +++ b/src/core/persistence/cmsData.ts @@ -365,6 +365,10 @@ interface DataLoopPreviewOptions { direction?: 'asc' | 'desc' limit?: number offset?: number + /** Cell condition, so the canvas previews the rows the page will publish. */ + cellField?: string + cellOperator?: string + cellValue?: string } interface DataLoopPreviewResult { @@ -386,6 +390,9 @@ export async function previewCmsDataLoopItems( direction: options.direction, limit: options.limit, offset: options.offset, + cellField: options.cellField, + cellOperator: options.cellOperator, + cellValue: options.cellValue, }, schema: LoopPreviewEnvelope, fetchImpl, From b3509e88dc64833c50c13554b0b29102f5194ed3 Mon Sep 17 00:00:00 2001 From: Mostafa Sadeghi <205455727+mostafasadeghidev@users.noreply.github.com> Date: Thu, 6 Aug 2026 12:36:03 +0200 Subject: [PATCH 2/3] feat(loops): sort data-row loops by a cell value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ordering was limited to the row's own SQL columns, so a list could not be sorted by a real date, title, or rank that lives in cells_json. A migrated site could only approximate 'newest first' with import arrival order, which drifts the moment anything is re-imported. - `orderBy` now also accepts `cell:`; `parseCellOrder` reads it and `cellOrderSql` renders the expression with the field name bound as a parameter, so nothing reaches the SQL text. - Both query paths order by it, values compare as TEXT in both dialects (ISO dates sort chronologically; documented that numbers sort lexicographically — one predictable rule beats two engine-specific ones), and coalesce gives rows lacking the field a defined position. - The Loop panel lists the selected table's fields as order options. Riding on `orderBy` rather than a new prop means every caller that already threads it — publisher, canvas preview, imported data-order-by attributes — supports this without further plumbing. Co-Authored-By: Claude Fable 5 --- src/__tests__/loops/cellFilter.test.ts | 35 +++++++++ .../loops/dataRowsCellFilter.test.ts | 71 +++++++++++++++++- .../PropertiesPanel/LoopPropertiesView.tsx | 18 +++-- src/core/loops/cellFilter.ts | 47 +++++++++++- src/core/loops/sources/dataRows.ts | 73 ++++++++++++------- 5 files changed, 209 insertions(+), 35 deletions(-) diff --git a/src/__tests__/loops/cellFilter.test.ts b/src/__tests__/loops/cellFilter.test.ts index ea874295e..50a7f3953 100644 --- a/src/__tests__/loops/cellFilter.test.ts +++ b/src/__tests__/loops/cellFilter.test.ts @@ -14,7 +14,9 @@ import { describe, expect, test } from 'bun:test' import { cellFilterMatches, cellFilterSql, + cellOrderSql, parseCellFilter, + parseCellOrder, CELL_FILTER_OPERATORS, type CellFilter, } from '@core/loops/cellFilter' @@ -129,6 +131,39 @@ describe('cellFilterSql', () => { }) }) +describe('parseCellOrder', () => { + test('only `cell:` values mean a cell sort', () => { + expect(parseCellOrder('publishedAt')).toBeNull() + expect(parseCellOrder('')).toBeNull() + expect(parseCellOrder('cell:published-on')).toEqual({ field: 'published-on' }) + }) + + test('a prefix with no field is not a sort', () => { + expect(parseCellOrder('cell:')).toBeNull() + expect(parseCellOrder('cell: ')).toBeNull() + }) +}) + +describe('cellOrderSql', () => { + test('binds the field name and never writes it into the SQL', () => { + for (const dialect of ['postgres', 'sqlite'] as const) { + const { sql, params } = cellOrderSql({ field: 'published-on', dialect, column: 'c', paramIndex: 2 }) + expect(sql).not.toContain('published-on') + expect(params).toEqual(['published-on']) + } + }) + + test('rows without the field get a defined sort position', () => { + const { sql } = cellOrderSql({ field: 'f', dialect: 'sqlite', column: 'c', paramIndex: 1 }) + expect(sql).toContain('coalesce') + }) + + test('placeholder style follows the dialect', () => { + expect(cellOrderSql({ field: 'f', dialect: 'postgres', column: 'c', paramIndex: 3 }).sql).toContain('$3') + expect(cellOrderSql({ field: 'f', dialect: 'sqlite', column: 'c', paramIndex: 3 }).sql).toContain('?') + }) +}) + describe('cellFilterMatches mirrors the SQL semantics', () => { const rows = { featuredTrue: { featured: true, name: 'A' }, diff --git a/src/__tests__/loops/dataRowsCellFilter.test.ts b/src/__tests__/loops/dataRowsCellFilter.test.ts index f06aafa74..3be672969 100644 --- a/src/__tests__/loops/dataRowsCellFilter.test.ts +++ b/src/__tests__/loops/dataRowsCellFilter.test.ts @@ -66,10 +66,12 @@ beforeAll(async () => { // Post-type rows: two featured, one not, one missing the field entirely — // the exact shape that made a real migration list the wrong three items. - await seedPost('p-a', 'alpha', { title: 'Alpha', featured: true, tag: 'news' }, '2024-01-01T00:00:00Z') - await seedPost('p-b', 'bravo', { title: 'Bravo', featured: false, tag: 'news' }, '2024-01-02T00:00:00Z') - await seedPost('p-c', 'charlie', { title: 'Charlie', featured: true, tag: 'guide' }, '2024-01-03T00:00:00Z') - await seedPost('p-d', 'delta', { title: 'Delta' }, '2024-01-04T00:00:00Z') + // `published-on` deliberately disagrees with the row's own publish column, + // so a cell sort cannot be mistaken for a column sort. + await seedPost('p-a', 'alpha', { title: 'Alpha', featured: true, tag: 'news', 'published-on': '2023-05-02' }, '2024-01-01T00:00:00Z') + await seedPost('p-b', 'bravo', { title: 'Bravo', featured: false, tag: 'news', 'published-on': '2023-09-30' }, '2024-01-02T00:00:00Z') + await seedPost('p-c', 'charlie', { title: 'Charlie', featured: true, tag: 'guide', 'published-on': '2023-01-15' }, '2024-01-03T00:00:00Z') + await seedPost('p-d', 'delta', { title: 'Delta', 'published-on': '2023-07-11' }, '2024-01-04T00:00:00Z') await db` insert into data_tables (id, name, slug, kind, route_base, singular_label, plural_label, fields_json, system) @@ -146,6 +148,67 @@ describe('data.rows cell filter — post-type tables', () => { }) }) +describe('data.rows ordering by a cell', () => { + it('sorts by the cell, not by the row columns', async () => { + // Seed order is alpha, bravo, charlie, delta; the dates deliberately + // disagree with it so a column sort cannot produce this result. + const { items } = await fetchPublishedDataRowItems(db, { + tableId: 'posts', + orderBy: 'cell:published-on', + direction: 'desc', + limit: 10, + offset: 0, + }) + expect(items.map((i) => String(i.fields['slug']))).toEqual(['bravo', 'delta', 'alpha', 'charlie']) + }) + + it('reverses cleanly', async () => { + const { items } = await fetchPublishedDataRowItems(db, { + tableId: 'posts', + orderBy: 'cell:published-on', + direction: 'asc', + limit: 10, + offset: 0, + }) + expect(items.map((i) => String(i.fields['slug']))).toEqual(['charlie', 'alpha', 'delta', 'bravo']) + }) + + it('combines with a filter and keeps the filtered count', async () => { + const { items, totalItems } = await fetchPublishedDataRowItems(db, { + tableId: 'posts', + orderBy: 'cell:published-on', + direction: 'desc', + limit: 10, + offset: 0, + cellFilter: { field: 'featured', operator: 'isTrue', value: '' }, + }) + expect(items.map((i) => String(i.fields['slug']))).toEqual(['alpha', 'charlie']) + expect(totalItems).toBe(2) + }) + + it('works on the data-kind path too', async () => { + const { items } = await fetchPublishedDataRowItems(db, { + tableId: 'logos', + orderBy: 'cell:name', + direction: 'desc', + limit: 10, + offset: 0, + }) + expect(items.map((i) => String(i.fields['slug']))).toEqual(['initech', 'globex', 'acme']) + }) + + it('an unknown sort field leaves every row present', async () => { + const { items } = await fetchPublishedDataRowItems(db, { + tableId: 'posts', + orderBy: 'cell:does-not-exist', + direction: 'desc', + limit: 10, + offset: 0, + }) + expect(items).toHaveLength(4) + }) +}) + describe('data.rows cell filter — data-kind tables', () => { it('applies on the direct-read path too', async () => { expect(await slugsWith('logos', { field: 'member', operator: 'isTrue', value: '' })).toEqual(['acme']) diff --git a/src/admin/pages/site/panels/PropertiesPanel/LoopPropertiesView.tsx b/src/admin/pages/site/panels/PropertiesPanel/LoopPropertiesView.tsx index 6b77edb4f..a9d77c2a0 100644 --- a/src/admin/pages/site/panels/PropertiesPanel/LoopPropertiesView.tsx +++ b/src/admin/pages/site/panels/PropertiesPanel/LoopPropertiesView.tsx @@ -17,6 +17,7 @@ import { useAsyncResource } from '@admin/lib/useAsyncResource' import { useEditorStore } from '@site/store/store' import { loopSourceRegistry } from '@core/loops/registry' import { ENTRY_FIELD_FILTER_KEY, ENTRY_FIELD_SOURCE_ID } from '@core/loops' +import { CELL_ORDER_PREFIX } from '@core/loops/cellFilter' import type { LoopEntitySource } from '@core/loops/types' import type { DataTableListItem } from '@core/data/schemas' import type { PropertyControl, PropertySchema } from '@core/module-engine' @@ -119,14 +120,21 @@ export function LoopPropertiesView({ nodeId, props, activePage }: LoopProperties } const filterSchema = buildFilterSchema() - // Order options reactive to source change. + // Order options reactive to source change. For data rows the selected + // table's own fields are offered too (`cell:`), so a list can sort by a + // real date or title instead of only by the row's SQL columns. const orderOptions: PropertyControl = { type: 'select', label: 'Order by', - options: - source?.orderByOptions.map((o) => ({ label: o.label, value: o.id })) ?? [ - { label: 'Default', value: '' }, - ], + options: source + ? [ + ...source.orderByOptions.map((o) => ({ label: o.label, value: o.id })), + ...(source.id === 'data.rows' + ? (tables?.find((t) => t.id === filters.tableId)?.fields ?? []) + .map((f) => ({ label: `${f.label || f.id} (field)`, value: `${CELL_ORDER_PREFIX}${f.id}` })) + : []), + ] + : [{ label: 'Default', value: '' }], } function handleSourceChange(_key: string, value: unknown) { diff --git a/src/core/loops/cellFilter.ts b/src/core/loops/cellFilter.ts index 51f1f1d7c..aba88a29d 100644 --- a/src/core/loops/cellFilter.ts +++ b/src/core/loops/cellFilter.ts @@ -1,5 +1,6 @@ /** - * Cell filtering for data-row loops. + * Cell access for data-row loops — filtering and ordering by a row's own + * cell rather than only by the table's SQL columns. * * A loop could pick a table and an order, but not *which* rows — so a page * that should list three featured articles listed the three most recent @@ -17,6 +18,50 @@ * maintainers would have to keep sound. */ +// --------------------------------------------------------------------------- +// Ordering by a cell +// --------------------------------------------------------------------------- + +/** `orderBy` values of this shape sort by a cell instead of a column. */ +export const CELL_ORDER_PREFIX = 'cell:' + +/** + * Read a cell-ordering request out of a loop's `orderBy`. + * + * Riding on `orderBy` (rather than a second prop) keeps ordering in one + * place: callers that already thread `orderBy` — the publisher, the canvas + * preview endpoint, imported `data-order-by` attributes — get this for free. + */ +export function parseCellOrder(orderBy: string): { field: string } | null { + if (!orderBy.startsWith(CELL_ORDER_PREFIX)) return null + const field = orderBy.slice(CELL_ORDER_PREFIX.length).trim() + return field ? { field } : null +} + +/** + * `ORDER BY` expression for a cell, with the field name bound as a parameter. + * + * Values are compared as TEXT in both dialects. ISO dates — the reason this + * exists — sort chronologically that way, and text sorts naturally. Numbers + * sort lexicographically (`'10' < '9'`), which is the price of one predictable + * rule across Postgres and SQLite instead of two subtly different ones. + */ +export function cellOrderSql(input: { + field: string + dialect: 'postgres' | 'sqlite' + column: string + paramIndex: number +}): { sql: string; params: unknown[] } { + const { field, dialect, column, paramIndex } = input + const placeholder = dialect === 'postgres' ? `$${paramIndex}` : '?' + const raw = dialect === 'postgres' + ? `(${column} #>> array[${placeholder}])` + : `cast(json_extract(${column}, '$.' || ${placeholder}) as text)` + // `coalesce` keeps rows that lack the field in one predictable place instead + // of relying on NULL ordering, which differs between the engines. + return { sql: `coalesce(${raw}, '')`, params: [field] } +} + /** Operators a loop filter can use. Closed set — never interpolated raw. */ export const CELL_FILTER_OPERATORS = ['is', 'isNot', 'isTrue', 'isFalse', 'isSet', 'isEmpty'] as const diff --git a/src/core/loops/sources/dataRows.ts b/src/core/loops/sources/dataRows.ts index 4bccaacc6..41d7083f6 100644 --- a/src/core/loops/sources/dataRows.ts +++ b/src/core/loops/sources/dataRows.ts @@ -20,7 +20,7 @@ */ import type { LoopEntitySource, LoopFetchResult, LoopItem, LoopSourceDb } from '@core/loops/types' -import { cellFilterSql, parseCellFilter, type CellFilter } from '../cellFilter' +import { cellFilterSql, cellOrderSql, parseCellFilter, parseCellOrder, type CellFilter } from '../cellFilter' import { isoDate } from '../../utils/isoDate' import { firstImagePathFromMarkdown } from '@core/markdown/renderMarkdown' import { normalizeRouteBase } from '@core/templates/templateMatching' @@ -214,23 +214,27 @@ const POST_TYPE_ORDER_COLUMN: Record = { async function fetchPage( db: LoopSourceDb, - tableId: string, orderBy: OrderColumn, direction: 'asc' | 'desc', - limit: number, - offset: number, - filter: CellFilter | null, + opts: { tableId: string; limit: number; offset: number; filter: CellFilter | null; orderCellField: string | null }, ): Promise { - const orderColumn = POST_TYPE_ORDER_COLUMN[orderBy] + const { tableId, limit, offset, filter, orderCellField } = opts + const column = 'data_row_versions.cells_json' // SQLite binds `?` by POSITION IN THE TEXT, so the parameter list must follow - // the clause order: tableId, then the cell condition (WHERE), then - // limit/offset. Postgres indices are numbered to match. + // the clause order: tableId, the cell condition (WHERE), the ordering cell + // (ORDER BY), then limit/offset. Postgres indices are numbered to match. const cell = filter - ? cellFilterSql({ filter, dialect: db.dialect, column: 'data_row_versions.cells_json', nextParamIndex: 2 }) + ? cellFilterSql({ filter, dialect: db.dialect, column, nextParamIndex: 2 }) : null const cellParams = cell?.params ?? [] - const limitParam = positionalParam(db, 2 + cellParams.length) - const offsetParam = positionalParam(db, 3 + cellParams.length) + const order = orderCellField + ? cellOrderSql({ field: orderCellField, dialect: db.dialect, column, paramIndex: 2 + cellParams.length }) + : null + const orderColumn = order ? order.sql : POST_TYPE_ORDER_COLUMN[orderBy] + const orderParams = order?.params ?? [] + const before = cellParams.length + orderParams.length + const limitParam = positionalParam(db, 2 + before) + const offsetParam = positionalParam(db, 3 + before) const { rows } = await db.unsafe( `select data_row_versions.id as version_id, data_rows.id as row_id, @@ -266,7 +270,7 @@ async function fetchPage( ${cell ? `and ${cell.sql}` : ''} order by ${orderColumn} ${direction}, data_row_versions.id ${direction} limit ${limitParam} offset ${offsetParam}`, - [tableId, ...cellParams, limit, offset], + [tableId, ...cellParams, ...orderParams, limit, offset], ) return rows } @@ -364,23 +368,27 @@ const DATA_KIND_ORDER_COLUMN: Record<'createdAt' | 'updatedAt' | 'slug', string> async function fetchDataKindPage( db: LoopSourceDb, - tableId: string, orderBy: OrderColumn, direction: 'asc' | 'desc', - limit: number, - offset: number, - filter: CellFilter | null, + opts: { tableId: string; limit: number; offset: number; filter: CellFilter | null; orderCellField: string | null }, ): Promise { + const { tableId, limit, offset, filter, orderCellField } = opts const sortKey: 'createdAt' | 'updatedAt' | 'slug' = orderBy === 'updatedAt' ? 'updatedAt' : orderBy === 'slug' ? 'slug' : 'createdAt' - const orderColumn = DATA_KIND_ORDER_COLUMN[sortKey] + const column = 'data_rows.cells_json' // Parameter order follows the clause order — see `fetchPage`. const cell = filter - ? cellFilterSql({ filter, dialect: db.dialect, column: 'data_rows.cells_json', nextParamIndex: 2 }) + ? cellFilterSql({ filter, dialect: db.dialect, column, nextParamIndex: 2 }) : null const cellParams = cell?.params ?? [] - const limitParam = positionalParam(db, 2 + cellParams.length) - const offsetParam = positionalParam(db, 3 + cellParams.length) + const order = orderCellField + ? cellOrderSql({ field: orderCellField, dialect: db.dialect, column, paramIndex: 2 + cellParams.length }) + : null + const orderColumn = order ? order.sql : DATA_KIND_ORDER_COLUMN[sortKey] + const orderParams = order?.params ?? [] + const before = cellParams.length + orderParams.length + const limitParam = positionalParam(db, 2 + before) + const offsetParam = positionalParam(db, 3 + before) // Same safety contract as `fetchPage`: the ORDER BY text comes only from // the closed map above; every runtime value is a positional parameter. @@ -407,7 +415,7 @@ async function fetchDataKindPage( ${cell ? `and ${cell.sql}` : ''} order by ${orderColumn} ${direction}, data_rows.id ${direction} limit ${limitParam} offset ${offsetParam}`, - [tableId, ...cellParams, limit, offset], + [tableId, ...cellParams, ...orderParams, limit, offset], ) return rows } @@ -451,6 +459,11 @@ export async function fetchPublishedDataRowItems( const tableKind = kindRows[0]?.kind if (!tableKind) return { items: [], totalItems: 0 } + // `orderBy` is either one of the whitelisted columns or `cell:`, + // in which case the sort runs on the row's own cell (the field name binds + // as a parameter, so nothing reaches the SQL text). + const cellOrder = parseCellOrder(opts.orderBy) + const orderCellField = cellOrder?.field ?? null const orderBy: OrderColumn = ALLOWED_ORDER_BY.has(opts.orderBy as OrderColumn) ? (opts.orderBy as OrderColumn) : 'publishedAt' @@ -473,9 +486,13 @@ export async function fetchPublishedDataRowItems( const totalItems = Number(countRows[0]?.total ?? 0) if (totalItems === 0) return { items: [], totalItems: 0 } - const sqlRows = await fetchDataKindPage( - db, opts.tableId, orderBy, direction, opts.limit, opts.offset, cellFilter, - ) + const sqlRows = await fetchDataKindPage(db, orderBy, direction, { + tableId: opts.tableId, + limit: opts.limit, + offset: opts.offset, + filter: cellFilter, + orderCellField, + }) const mediaPathMap = await resolveMediaIdsToPaths(db, extractFeaturedMediaIds(sqlRows)) return { items: sqlRows.map((row) => dataKindRowToLoopItem(row, mediaPathMap)), @@ -500,7 +517,13 @@ export async function fetchPublishedDataRowItems( const totalItems = Number(countRows[0]?.total ?? 0) if (totalItems === 0) return { items: [], totalItems: 0 } - const sqlRows = await fetchPage(db, opts.tableId, orderBy, direction, opts.limit, opts.offset, cellFilter) + const sqlRows = await fetchPage(db, orderBy, direction, { + tableId: opts.tableId, + limit: opts.limit, + offset: opts.offset, + filter: cellFilter, + orderCellField, + }) const mediaPathMap = await resolveMediaIdsToPaths(db, extractFeaturedMediaIds(sqlRows)) return { From c0f932fd34d8429b4dfabe853652ee43ab75479d Mon Sep 17 00:00:00 2001 From: Mostafa Sadeghi <205455727+mostafasadeghidev@users.noreply.github.com> Date: Thu, 6 Aug 2026 13:03:05 +0200 Subject: [PATCH 3/3] fix(loops): label the cell filter's presence operator "is set" "has any value" described the same condition in more words. The operator id was already `isSet`; the label now says so too. Co-Authored-By: Claude Fable 5 --- src/core/loops/sources/dataRows.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/loops/sources/dataRows.ts b/src/core/loops/sources/dataRows.ts index 41d7083f6..e2e93e763 100644 --- a/src/core/loops/sources/dataRows.ts +++ b/src/core/loops/sources/dataRows.ts @@ -566,7 +566,7 @@ export const DataRowsSource: LoopEntitySource = { { label: 'is not', value: 'isNot' }, { label: 'is checked', value: 'isTrue' }, { label: 'is unchecked', value: 'isFalse' }, - { label: 'has any value', value: 'isSet' }, + { label: 'is set', value: 'isSet' }, { label: 'is empty', value: 'isEmpty' }, ], },