diff --git a/.changeset/9159-drill-url-isnull-operator.md b/.changeset/9159-drill-url-isnull-operator.md new file mode 100644 index 0000000000..515fcd4826 --- /dev/null +++ b/.changeset/9159-drill-url-isnull-operator.md @@ -0,0 +1,51 @@ +--- +'@object-ui/app-shell': minor +--- + +The drill "escape hatch" can spell an empty bucket: the `filter[...]` URL dialect grows an +is-null operator on both sides plus a chip for it (objectui#9159). + +`buildDatasetDrillFilter`'s output has three consumers. Two lower it through +`convertFiltersToAST`, where objectui#9085's `{ field: { $null: true } }` becomes +`[field, 'is_null', true]` and the empty bucket selects its own rows. The third is this +one — `OpenInListButton` and `drillDown.target: navigate`, which reach the host's +`openRecordList` and serialize the drill filter into `filter[...]` search params for the +ADR-0055 bare data surface. That dialect had equality plus four range bounds and nothing +else, so the condition simply vanished: `{ stage: 'won', owner: { $null: true } }` and +`{ stage: 'won', owner: null }` produced the byte-identical `filter[stage]=won`, and an +empty-bucket-only drill produced an empty query string. The surviving condition was the +NON-empty one, so drilling into the empty bucket and escalating to the full list page +returned a **superset** — silently, with filter chips showing only the conditions that +survived, so the page looked correctly scoped. + +`drillUrlFilters` now carries the operator on both halves of its one module, as +`NULL_FILTER`: + +- **write** — `{ $null: true }` becomes `filter[][null]=true`, emitted beside any + range bound on the same object because `convertFiltersToAST` emits both conditions for + that input; +- **read** — that param becomes `[field, 'is_null', true]`, the same triple the other two + consumers already produce; +- **chip** — the grouper gives it its own arm, where it previously fell to the `= ` + default and displayed a bare `true`. That arm hands out an i18n KEY rather than finished + text (`FilterChip.textKey`), and `ObjectDataPage` resolves it at the same half-chip seam + that already draws the field name through `fieldLabel`. The key is the filter builder's + existing operator label, already defined and translated in all ten locale packs, so + nothing new is authored and no second spelling of one operator label is put at rest. The + range and equality arms are untouched and still finish their own text: a comparand is the + user's own data, which no catalogue can translate. + +The value is a FLAG, not a comparand. `filter[][null]=false` is **not** a second +operator: this dialect cannot write "is not null", so the read side drops that param the +way it drops an unknown suffix rather than inventing an operator with no producer — and +equality-to-empty-string remains no condition at all, since a param whose value is empty +was already skipped. `{ $null: false }` is dropped on the write side for the same reason, +degrading to a superset exactly as any other unspellable operator does. + +The range maps are deliberately untouched: `is_null` is already a canonical +`ViewFilterRule` word, and `ObjectDataPage` inverts `URL_FILTER_OPS` to bridge a triple's +operator to the spec's *alias* spelling. An entry there would have sent it through that +bridge as the alias `null`, which the rule schema refuses — a "Save as view" that silently +loses the condition. `ObjectDataPage.saveAsViewFilterFold.test.ts` pins the fold for the +new operator, and `drillEmptyBucketNavHost-9085.test.ts` — which recorded this boundary as +open — is updated to record it closed. diff --git a/packages/app-shell/src/views/ObjectDataPage.filterChipI18n-9159.test.tsx b/packages/app-shell/src/views/ObjectDataPage.filterChipI18n-9159.test.tsx new file mode 100644 index 0000000000..cfbed29051 --- /dev/null +++ b/packages/app-shell/src/views/ObjectDataPage.filterChipI18n-9159.test.tsx @@ -0,0 +1,222 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * objectui#9159 round 2 — the is-null filter chip is drawn from the locale + * packs, like every other user-visible string on this page. + * + * The operator landed with its chip text finished inside `groupFilterChips` as + * the literal `'is null'`. That breaks two rules at once and "make it English" + * answers only the first: a user-facing literal in a renderer is unlocalized for + * every reader on every locale, and the defect survives verbatim in whatever + * language it was written in. So the grouper now hands out the filter builder's + * existing operator key and this page resolves it at the same half-chip seam + * that already draws the field name through `fieldLabel`. + * + * ## Why this file renders the page instead of asserting the key + * + * A test on the grouper's return value can only pin that a key is handed OUT + * (`drillUrlFilters.test.ts` does that). It stays green if the render site never + * calls `t` — which is the whole defect, one layer down. So the observation + * point is the DOM of the real page, under a real `I18nProvider`, and the + * assertion is on the chip's own text node reached through the remove button + * beside it rather than on the chip row's whole `textContent`, which also holds + * the "Filtered by" lead-in and the translated field name. + * + * ## Why a NON-English locale, and what each control rules out + * + * English cannot tell the two worlds apart: a hardcoded `'is null'` and a + * correctly resolved `en` pack value render the same pixels. Under `zh` they + * differ, so the assertion has the power to fail for the reason it exists. The + * controls below rule out the two ways this could pass vacuously — the packs + * agreeing (they are asserted to differ, live) and the key failing to resolve + * (i18next would render the key itself, so the text is asserted not to contain + * the key's own prefix). ⇒ a pack that loses this key is VISIBLE here rather + * than silently falling back to English. + */ + +import * as React from 'react'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, cleanup } from '@testing-library/react'; +import { MemoryRouter, Routes, Route } from 'react-router-dom'; + +vi.mock('@object-ui/permissions', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + usePermissions: () => ({ + check: () => ({ allowed: true }), + checkField: () => true, + getFieldPermissions: () => [], + getRowFilter: () => undefined, + getObjectApiOperations: () => undefined, + roles: [], + isLoaded: false, + hasCapabilities: () => true, + can: () => true, + cannot: () => false, + }), + useFieldPermissions: () => ({ canRead: () => true, canWrite: () => true, permissions: [] }), + }; +}); + +vi.mock('@object-ui/auth', async (importOriginal) => ({ + ...(await importOriginal>()), + useAuth: () => ({ user: { id: 'u1', name: 'Ada' }, activeOrganization: null }), + useWorkspaceAdminStatus: () => ({ isAdmin: false, isResolved: true }), + createAuthenticatedFetch: () => vi.fn(), +})); + +// Heavy children, all orthogonal to the chip row under test and each dragging in +// a plugin bundle — the same posture as the sibling create-affordance test on +// this page. +vi.mock('@object-ui/plugin-list', async (importOriginal) => ({ + ...(await importOriginal()), + ListView: () => null, +})); +vi.mock('./RecordDetailView', () => ({ RecordDetailView: () => null })); +vi.mock('./CreateViewDialog', () => ({ CreateViewDialog: () => null })); +vi.mock('./metadata-admin/useMetadata', () => ({ useMetadataClient: () => ({}) })); + +import { I18nProvider } from '@object-ui/i18n'; +import { builtInLocales } from '@object-ui/i18n/locales'; +import { ObjectDataPage } from './ObjectDataPage'; +import { ExpressionProvider } from '../providers/ExpressionProvider'; +import { NULL_FILTER } from './drillUrlFilters'; + +const h = React.createElement; +const OBJECT_NAME = 'showcase_invoice'; +const FIELD = 'owner'; + +const OBJECTS = [ + { + name: OBJECT_NAME, + label: 'Invoice', + managedBy: 'platform', + fields: { + id: { type: 'text', label: 'Id' }, + owner: { type: 'text', label: 'Owner' }, + }, + }, +]; + +function makeDataSource() { + return { + find: vi.fn(async () => ({ data: [], total: 0 })), + findOne: vi.fn(async () => null), + create: vi.fn(async () => ({})), + update: vi.fn(async () => ({})), + delete: vi.fn(async () => ({})), + } as any; +} + +/** What the pack itself says this operator is called, in one language. */ +function packLabel(language: string): unknown { + const pack = (builtInLocales as Record)[language]; + return NULL_FILTER.labelKey.split('.').reduce((node, part) => node?.[part], pack); +} + +/** + * Render the page at one `filter[...]` query string, in one language. + * + * `children` rides in the PROPS object rather than in `createElement`'s third + * argument: both providers declare it required, and the third-argument overload + * does not satisfy that (objectui#4040, the same fix the sibling i18n render + * tests carry). + */ +function renderAt(language: string, search: string) { + const page = h( + MemoryRouter, + { initialEntries: [`/apps/demo/${OBJECT_NAME}/data?${search}`] }, + h( + Routes, + null, + h(Route, { + path: '/apps/:appName/:objectName/data', + element: h(ObjectDataPage, { dataSource: makeDataSource(), objects: OBJECTS }), + }), + ), + ); + return render( + h(I18nProvider, { + config: { defaultLanguage: language, detectBrowserLanguage: false }, + children: h(ExpressionProvider, { + user: { id: 'u1', name: 'Ada', profile: 'admin' }, + children: page, + }), + }), + ); +} + +/** The empty-bucket drill's own query string, spelled from the contract. */ +const EMPTY_BUCKET_SEARCH = `filter[${FIELD}][${NULL_FILTER.param}]=${NULL_FILTER.flag}`; + +/** + * The chip's operator half. Reached through the remove button that sits beside + * it, so this cannot drift onto the field-name half or the row's lead-in. + */ +function chipOperatorText(): string { + const remove = document.querySelector(`[data-testid="object-data-remove-filter-${FIELD}"]`); + expect(remove, 'the empty-bucket chip did not render at all').toBeTruthy(); + return remove!.previousElementSibling?.textContent ?? ''; +} + +beforeEach(() => { + cleanup(); + vi.stubGlobal( + 'fetch', + vi.fn( + async () => + new Response(JSON.stringify({ data: [] }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }), + ), + ); +}); + +afterEach(() => { + vi.unstubAllGlobals(); + vi.clearAllMocks(); +}); + +describe('the is-null filter chip is translated (objectui#9159)', () => { + it('renders the zh pack copy, not an English literal', () => { + renderAt('zh', EMPTY_BUCKET_SEARCH); + const rendered = chipOperatorText(); + + // The rendered copy IS the pack copy. Read from the pack rather than + // written out here: this card owns whether the chip is TRANSLATED, while + // the wording of the operator label belongs to the catalogue and to the + // filter builder that shares this key. + expect(rendered).toBe(packLabel('zh')); + // A key that failed to resolve renders as itself — the exact user-visible + // symptom the filter-builder operator family's locale-parity pin names. + expect(rendered).not.toContain('filterBuilder.operators'); + // And the two defects this arm replaced, neither of which can come back + // through a pack: the bare comparand, and the deleted English literal. + expect(rendered).not.toContain('true'); + expect(rendered).not.toBe('is null'); + }); + + it('CONTROL: the English pack renders too, and the two packs really differ', () => { + // Without this pair the zh assertion could pass on a page that never called + // `t` at all, if the packs happened to agree. + renderAt('en', EMPTY_BUCKET_SEARCH); + expect(chipOperatorText()).toBe(packLabel('en')); + expect(packLabel('zh')).not.toBe(packLabel('en')); + }); + + it('CONTROL: an equality chip is NOT translated, because its text is the user\'s own comparand', () => { + // The other half of the split: a comparand has no catalogue entry and must + // reach the DOM verbatim in every locale. A change that routed every chip + // through `t` would turn this red. + renderAt('zh', `filter[${FIELD}]=alice`); + expect(chipOperatorText()).toBe('= alice'); + }); +}); diff --git a/packages/app-shell/src/views/ObjectDataPage.saveAsViewFilterFold.test.ts b/packages/app-shell/src/views/ObjectDataPage.saveAsViewFilterFold.test.ts index 8dc7c089c4..091e5ce694 100644 --- a/packages/app-shell/src/views/ObjectDataPage.saveAsViewFilterFold.test.ts +++ b/packages/app-shell/src/views/ObjectDataPage.saveAsViewFilterFold.test.ts @@ -25,7 +25,7 @@ import { describe, it, expect, vi, afterEach } from 'vitest'; import { ViewItemSchema } from '@objectstack/spec/ui'; -import { URL_FILTER_OPS, type FilterTriple } from './drillUrlFilters'; +import { URL_FILTER_OPS, NULL_FILTER, type FilterTriple } from './drillUrlFilters'; import { viewEnvelope } from './runtime-metadata-persistence'; import { buildSaveAsViewSpec } from './ObjectDataPage'; @@ -93,11 +93,13 @@ describe('Save as view folds URL drill triples to spec rules (objectui#3419)', ( }); it('folds EVERY operator the URL contract can emit to a canonical spelling', () => { - // Derived from `URL_FILTER_OPS` (plus `=`, which has no `[op]` suffix form) - // so a range operator added to the URL contract fails HERE rather than at - // publish time. `parseUrlFilterTriples` emits nothing outside this set. - const emittable = ['=', ...Object.values(URL_FILTER_OPS)]; - expect(emittable).toEqual(['=', '>=', '<=', '>', '<']); + // Derived from `URL_FILTER_OPS` (plus `=`, which has no `[op]` suffix form, + // and `NULL_FILTER.op`, whose param carries a FLAG rather than a comparand + // and so is not in that range map) so an operator added to the URL contract + // fails HERE rather than at publish time. `parseUrlFilterTriples` emits + // nothing outside this set. + const emittable = ['=', ...Object.values(URL_FILTER_OPS), NULL_FILTER.op]; + expect(emittable).toEqual(['=', '>=', '<=', '>', '<', 'is_null']); const { spec, gate } = saveAsView( emittable.map((op, i) => ['f' + i, op, String(i)] as FilterTriple), @@ -108,10 +110,29 @@ describe('Save as view folds URL drill triples to spec rules (objectui#3419)', ( 'less_than_or_equal', 'greater_than', 'less_than', + // objectui#9159. `is_null` is ALREADY a canonical ViewFilterRule word, so + // it must reach `normalizeFilterOperator` unbridged; the symbol-to-alias + // table is keyed on the range symbols, which this operator is not one of. + // Were the flag added to `URL_FILTER_OPS` instead, the bridge would hand + // over the alias `null`, the rule schema would refuse it, and saving the + // view would silently drop the condition. + 'is_null', ]); expect(gate.success).toBe(true); }); + it('keeps the is-null flag intact through the fold, value and all', () => { + // The escape hatch's empty-bucket drill (objectui#9159) is savable as a + // view: `[field,'is_null',true]` is a canonical rule, and the value rides + // along exactly as `viewFilterFold` carries a value-less operator's value. + const { spec, gate } = saveAsView([['owner', NULL_FILTER.op, true]]); + expect(spec.filter).toEqual([{ field: 'owner', operator: 'is_null', value: true }]); + expect( + gate.success, + `ViewItem rejected by spec: ${JSON.stringify(gate.error?.issues)}`, + ).toBe(true); + }); + it('carries field and value through untouched', () => { // Placeholder resolution has already run upstream; the fold must not // re-interpret what it produced (`''` included — the spec accepts it). diff --git a/packages/app-shell/src/views/ObjectDataPage.tsx b/packages/app-shell/src/views/ObjectDataPage.tsx index 0dc960f459..e71642806a 100644 --- a/packages/app-shell/src/views/ObjectDataPage.tsx +++ b/packages/app-shell/src/views/ObjectDataPage.tsx @@ -570,13 +570,23 @@ export function ObjectDataPage({ dataSource, objects }: any) { {t('console.objectData.filteredBy', { defaultValue: 'Filtered by' })} - {filterChips.map(({ field, text }) => ( + {filterChips.map(({ field, text, textKey }) => ( {fieldLabel(objectDef.name, field, field)} - {text} + {/* + A chip carrying `textKey` is one whose text is PROSE rather than + the user's own comparand, so it is translated HERE — the same + half-chip seam that already draws the field name through + `fieldLabel` (objectui#9159). Passed bare, with no inline + `defaultValue`: all ten packs define this operator family and its + locale-parity pin holds them to it, so a default would only be an + unwatched second English spelling that hides a pack miss + (objectui#3469 deleted exactly that pattern from this console). + */} + {textKey ? t(textKey) : text}