Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions .changeset/7561-filter-builder-operator-trigger-label.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,3 +47,16 @@ marker:

Which of the two vocabularies should win remains open and is deliberately not
answered here (objectui#7561).

Superseded in this release by objectui#9306: the mounted `SelectItem` ids are
no longer camelCase. The dropdown now draws the protocol's own ids — the twenty
`VIEW_FILTER_OPERATORS` members (`greater_than`, `not_in`, …) plus the opt-in
`exists` / `notExists` — so picking **Less than** makes `onChange` hand back
`less_than`: still never `lt`, and no longer `lessThan`. The vocabulary question
this entry leaves open is answered by that change: the protocol's spelling is
the one the dropdown emits, and a retired camelCase id a stored filter still
carries is read as the deprecated alias it is and written back canonical on the
author's next edit. The fold this entry added is unchanged, and the builder now
also folds a row's spelling when the group arrives, so the alias table (`gt` /
`lt` / `eq`) and the retired camelCase ids both still draw their operator's
label.
11 changes: 11 additions & 0 deletions .changeset/8748-icontains-empty-comparand.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,3 +53,14 @@ they did; only what the builder WRITES from now on changes.
⚠️ The builder ROW is unaffected by the drop: it is held as local state and stays on
screen with its value box empty, so the five text operators stay reachable — the criteria
is what the rows emit, not what they are.

Superseded in this release by objectui#9306, for the `@object-ui/fields` half:
the builder's operator ids are now the protocol's canonical spellings, so the
text operators `condToMongo` drops on an empty comparand are `contains`,
`icontains`, `not_contains`, `starts_with` and `ends_with` (the case-insensitive
contains is `icontains` now, and no longer opt-in), and the value-less operators
it leaves untouched are `is_null` / `exists` / `is_empty` and their negations
`is_not_null` / `notExists` / `is_not_empty`. The drop itself, `equals ''`, and
every emitted `$`-token (`$contains`, `$icontains`, `$notContains`,
`$startsWith`, `$endsWith`) are unchanged — objectui#9306's census measured the
same stored predicate for all 22 former ids and the ids they became.
16 changes: 16 additions & 0 deletions .changeset/9302-filter-builder-valueless-canonical-fold.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,3 +49,19 @@ and no row's stored `operator` is rewritten — rendering is not an edit.
Which operator vocabulary should WIN is a separate, still-open question and is
not decided here. The `contains` / `icontains` boundary is untouched and pinned:
the fold this gate routes through maps neither onto the other.

Superseded in this release by objectui#9306: the dropdown's ids are now the
protocol's canonical spellings, so `VALUELESS_FILTER_BUILDER_OPERATORS` holds
`is_empty`, `is_not_empty`, `is_null`, `is_not_null`, `exists` and `notExists`.
The exported set's membership therefore did change in this release — by that
change, not this one, and it is still one id per operator with no second
spelling added. The gate this entry repaired is unchanged in shape: it folds the
row's spelling before the lookup (now through `normalizeFilterBuilderOperator`,
the spec's `normalizeFilterOperator` plus one local row for the
`containsCaseInsensitive` spelling the spec's table lacks), so the spellings the
gate answers "no value" for without their being members are now the retired
camelCase ids a filter stored before objectui#9306 carries, and the spec's other
alias rows for these operators. The vocabulary question this entry calls
open is answered: the dropdown speaks the protocol's ids, and camelCase is read
as the deprecated alias form and rewritten canonical on the next edit.
`contains` and `icontains` remain two operators.
60 changes: 60 additions & 0 deletions .changeset/9306-filter-builder-protocol-operator-ids.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
---
'@object-ui/components': minor
'@object-ui/fields': minor
'@object-ui/app-shell': minor
'@object-ui/plugin-view': minor
'@object-ui/i18n': minor
---

The `FilterBuilder` dropdown speaks the protocol's operator ids (objectui#9306).

`defaultOperators` now emits the twenty members of `@objectstack/spec`'s
`VIEW_FILTER_OPERATORS`, spelled as the spec spells them (`not_equals`,
`greater_than_or_equal`, `is_null`, `icontains`, …), plus the two opt-in
existence ids `exists` / `notExists`, which the protocol has no member for and
which stay unfolded (objectui#9559 ruling B). The camelCase ids the dropdown used
to emit (`notEquals`, `greaterOrEqual`, `isNull`, …) are the spec's deprecated
alias form (objectui#7993); `containsCaseInsensitive` is the one former id the
spec's alias table has no row for, and the builder reads it itself (below).

**Stored filters keep loading.** The builder folds a stored spelling at its read
boundary, through the spec's `normalizeFilterOperator` plus one local row the
spec's alias table lacks (`containsCaseInsensitive` → `icontains`,
objectstack-ai/objectstack#20092), and the author's next edit writes the
canonical id back. Opening a stored filter writes nothing. No row changes the
predicate it stores: the sharing-rule criteria, the dataset filter, the saved-view
fold, the live grid and the override recovery pass were each measured over all
22 former ids against the ids they became, and store the same predicate. The only
cells that differ are `icontains` on the three consumers that never offered the
case-insensitive contains (dataset filter, saved-view fold, live grid), where
the old id produced no storable filter at all and the new one does.

**Breaking, stated here because the group never takes a `major`:**

- `@object-ui/components`: the published `FilterBuilderOperator` type NARROWS
from the camelCase union to the spec's `ViewFilterOperator` plus `'exists' |
'notExists'` — a `'greaterOrEqual'` literal typed against it no longer
compiles. `FILTER_BUILDER_OPERATORS` and `VALUELESS_FILTER_BUILDER_OPERATORS`
hold the canonical ids, and a host's `onChange` receives them. New export:
`normalizeFilterBuilderOperator`, the builder's read-side fold.
- `@object-ui/components`: `icontains` ("Contains (ignore case)") is no longer
opt-in. Its old reason — only the Mongo criteria dialect could carry a
case-insensitive contains — stopped being true when `VIEW_FILTER_OPERATORS`
gained `icontains` (spec 17.1.0), and `OPT_IN_OPERATORS`' own docblock recorded
deleting the entry as the planned outcome. It is offered on the text bucket to
every consumer; `contains` and `icontains` stay two operators (objectui#7379).
- `@object-ui/i18n`: the `filterBuilder.operators.*` keys are re-keyed to the
canonical ids in all ten packs (`filterBuilder.operators.is_null`, …); every
translated value is unchanged. A host that overrides one of these keys must
re-key its override.
- `@object-ui/fields`: `FILTER_CONDITION_EXTRA_OPERATORS` is `['exists',
'notExists']` — the case-insensitive contains needs no grant any more.
`FilterConditionField` still writes `$icontains` for it, and its builder rows
(`kvToCondition`) carry the canonical ids.
- `@object-ui/plugin-view`: `toFilterGroup` emits the canonical ids.

Also in `@object-ui/app-shell`: the dataset inspector's bridge maps `icontains`
to `$icontains`; the drill-down "is null" chip reads the re-keyed label; and the
view-override recovery pass folds a row's operator before its value-less check,
so a stored `{ operator: 'isEmpty', value: '' }` row is kept rather than dropped
as unfinished.
15 changes: 15 additions & 0 deletions .changeset/9359-list-ast-valueless-canonical-fold.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,3 +60,18 @@ operator is rewritten: converting is not migrating.
Which operator vocabulary should WIN is a separate, still-open question and is
not decided here. The `contains` / `icontains` boundary is untouched and pinned:
the fold this reader routes through maps neither onto the other.

Superseded in this release by objectui#9306: the list toolbar's FilterBuilder
now emits the protocol's canonical ids, so the canonical spelling IS a measured
producer into this reader, and `VALUELESS_FILTER_BUILDER_OPERATORS` holds
`is_empty`, `is_not_empty`, `is_null`, `is_not_null`, `exists` and `notExists` —
its membership changed in this release by that change, still one id per
operator. A group restored per browser carries whatever ids it was written
with, so a group written before objectui#9306 can still reach this reader in
camelCase. This reader is unchanged and answers both spellings alike: over the
former dropdown ids and the ids they became (the list reader's leg of
objectui#9306's census), every pair emits the same node except
`containsCaseInsensitive`, which the toolbar never offered and which the builder
folds onto `icontains` before a row leaves it. The vocabulary
question this entry calls open is answered: the dropdown speaks the protocol's
ids, and camelCase is the deprecated alias form.
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@
* Three catalog entries carry the rows this measures — `product-search`,
* `with-conditions`, and the `filter-builder` nested inside `search-interface`.
* No `SelectItem` in the operator dropdown carries a spelling from outside its
* own camelCase vocabulary, and the trigger used to match its value LITERALLY
* own vocabulary (camelCase then, the protocol's ids since objectui#9306), and
* the trigger used to match its value LITERALLY
* against the mounted items, so any other spelling of the same operator drew a
* blank operator cell over a row that filtered correctly.
*
Expand All @@ -41,13 +42,17 @@
* one canonical member and the trigger now resolves through it. Before that
* repair they differ — which is what makes this measurement able to fail.
*
* ⭐ The corrected column deliberately uses the dropdown's OWN camelCase ids
* (`lessThan`), not the spec's canonical `less_than`: those ids are the ones
* mounted, so the corrected column is the "what it would have looked like if
* authored in the renderer's dialect" arm the card described. Both arms
* rendering the same text is the claim; neither arm is a recommendation about
* which vocabulary an author SHOULD use — that is objectui#7561's separate
* ruling and is not decided here.
* ⭐ The corrected column deliberately uses the camelCase ids (`lessThan`),
* not the spec's canonical `less_than`. When objectui#7561 landed those were
* the dropdown's OWN ids, the ones mounted, so that column was the "what it
* would have looked like if authored in the renderer's dialect" arm the card
* described. Since objectui#9306 the dropdown mounts the canonical ids and the
* camelCase ones are the spec's DEPRECATED alias form, which the builder folds
* at its read boundary — so the same column now measures the other direction:
* a filter stored before that change still draws its label. Both arms
* rendering the same text is the claim either way; neither arm is a
* recommendation about which vocabulary an author SHOULD use — the canonical
* one, per objectui#9306's ruling, which this control does not re-decide.
*
* ⚠️ The two vocabularies OVERLAP on three members (`equals`, `contains`,
* `in` are spelled identically in both), so "the arms are two dialects" cannot
Expand Down Expand Up @@ -89,10 +94,11 @@ const EXPECTED: Record<(typeof AFFECTED)[number], string[]> = {
};

/**
* The declared spellings these entries author → the dropdown id each folds
* onto. Covers the spec's alias table too, so an entry re-authored in EITHER
* off-dropdown dialect is still carried by the control arm rather than
* silently passed through as an identity.
* The declared spellings these entries author → the camelCase id each folds
* from: the dropdown's own id before objectui#9306, the deprecated alias form
* after it (see this file's header). Covers the spec's alias table too, so an
* entry re-authored in EITHER off-canonical dialect is still carried by the
* control arm rather than silently passed through as an identity.
*/
const CORRECTION: Record<string, string> = {
// what the catalog authors today — `@objectstack/spec`'s canonical members
Expand Down
48 changes: 46 additions & 2 deletions packages/app-shell/src/views/ObjectView.overlayPairValue.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -164,8 +164,9 @@ describe('the arities this change must not touch (#5025)', () => {
});

it('still keeps a value-less operator whose value slot is empty', () => {
// Both dialects: the builder id and the canonical spec spelling this
// layer additionally sees.
// Both spellings a stored overlay can carry: the canonical one (the
// builder's own id since objectui#9306) and the deprecated camelCase
// one a row stored before that change still holds.
const row = overlay([
{ field: 'closed_at', operator: 'isEmpty', value: '' },
{ field: 'owner', operator: 'is_null', value: '' },
Expand All @@ -174,6 +175,49 @@ describe('the arities this change must not touch (#5025)', () => {
});
});

/**
* objectui#9306 — the recovery pass FOLDS a row's operator before asking
* whether it takes a value.
*
* The value-less set it asks (`VALUELESS_FILTER_OPERATORS`) is written in the
* protocol's spellings, and since objectui#9306 so is the builder half of it:
* the deprecated camelCase ids are no longer members. A stored overlay row
* `{ operator: 'isEmpty', value: '' }` — written before that change — would
* then miss a RAW lookup, fall to the value test, have its `''` read as
* unfilled, and be DROPPED: the stored filter loses a condition on read, and
* the list widens to rows the author excluded. Every spelling below is one the
* spec's alias table folds onto a value-less member.
*/
describe('a stored deprecated-spelling value-less row survives the recovery pass (objectui#9306)', () => {
it.each([
['isEmpty', 'is_empty'],
['isNotEmpty', 'is_not_empty'],
['isNull', 'is_null'],
['isNotNull', 'is_not_null'],
['isnull', 'is_null'],
])('`%s` (folds to `%s`) with an empty value slot is KEPT', (operator) => {
const row = overlay([{ field: 'closed_at', operator, value: '' }]);
expect(sanitizeViewOverride(row)).toBe(row);
});

it('…in the legacy runtime triple shape too', () => {
const row = overlay([['closed_at', 'isEmpty', '']]);
expect(sanitizeViewOverride(row)).toBe(row);
});

it('CONTROL: a value-TAKING deprecated spelling with no value is still dropped', () => {
// The fold must not make every camelCase row look finished: `notEquals`
// takes a value, so an empty one is an unfinished row exactly as its
// canonical twin's is.
expect(
sanitizeViewOverride(overlay([{ field: 'name', operator: 'notEquals', value: '' }])).filter,
).toBeUndefined();
expect(
sanitizeViewOverride(overlay([{ field: 'name', operator: 'not_equals', value: '' }])).filter,
).toBeUndefined();
});
});

describe('why the read path is the last guard (#5025)', () => {
// NON-DISCRIMINATING by construction: this asserts a fact about
// `@objectstack/spec`, not about `sanitizeViewOverride`, so it is green in
Expand Down
24 changes: 19 additions & 5 deletions packages/app-shell/src/views/ObjectView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ import { MetadataPanel, useMetadataInspector } from './MetadataInspector.js';
import { ViewConfigPanel } from './ViewConfigPanel.js';
import { useMetadataClient } from './metadata-admin/useMetadata.js';
import { persistRuntimeMetadata, createRuntimeMetadata, viewEnvelope, type ViewEnvelope } from './runtime-metadata-persistence.js';
import { ListViewSchema as SpecListViewSchema } from '@objectstack/spec/ui';
import { ListViewSchema as SpecListViewSchema, normalizeFilterOperator } from '@objectstack/spec/ui';
import { CreateViewDialog } from './CreateViewDialog.js';
import {
usePreviewDrafts,
Expand Down Expand Up @@ -696,8 +696,19 @@ export function defaultListColumnsFromObject(
*
* The split of duties is the helper's own: it answers the VALUE question only,
* while which operators want no value at all stays with
* {@link VALUELESS_FILTER_OPERATORS} above — this layer additionally sees the
* canonical spec spellings (`is_null`) that never reach the dropdown.
* {@link VALUELESS_FILTER_OPERATORS} above.
*
* That membership is asked of the row's spelling FOLDED through the spec's own
* `normalizeFilterOperator` (objectui#9306), never of the raw spelling — the
* same both-sides fold objectui#9302 / #9359 put on the builder's gate and the
* live grid, and that {@link isFilterValueComplete} already applies to the
* arity question. The set's members are protocol ids (plus `exists` /
* `notExists`), on which the fold is the identity, so folding the row is
* folding both sides. It became load-bearing when the builder's ids became the
* protocol's: a row stored under the deprecated camelCase id —
* `{ operator: 'isEmpty', value: '' }` — no longer matches the set raw, and a
* raw lookup would hand it to the value test, which reads `''` as unfilled and
* DROPS it, so the stored filter would lose a condition on read.
*/
export function sanitizeViewOverride(override: any): any {
if (!override || typeof override !== 'object') return override;
Expand All @@ -715,17 +726,20 @@ export function sanitizeViewOverride(override: any): any {
if (narrowed !== override) return narrowed;
if (!Array.isArray(override.filter)) return override;

// Folded before the lookup — see the docblock (objectui#9306).
const isValueless = (operator: unknown): boolean =>
VALUELESS_FILTER_OPERATORS.has(normalizeFilterOperator(String(operator)));
const kept = override.filter.filter((entry: any) => {
if (Array.isArray(entry)) {
// Legacy runtime triple: [field, operator, value]
if (entry.length < 2) return false;
const [, operator, value] = entry;
if (VALUELESS_FILTER_OPERATORS.has(String(operator))) return true;
if (isValueless(operator)) return true;
return isFilterValueComplete(String(operator), value);
}
if (!entry || typeof entry !== 'object') return false;
if (typeof entry.field !== 'string' || entry.field === '') return false;
if (VALUELESS_FILTER_OPERATORS.has(String(entry.operator))) return true;
if (isValueless(entry.operator)) return true;
const value = entry.value;
return isFilterValueComplete(String(entry.operator), value);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ describe('the drill escape hatch and the empty bucket (objectui#9159)', () => {
expect(NULL_FILTER.flag).toBe('true');
expect(NULL_FILTER.op).toBe('is_null');
expect(NULL_FILTER.key).toBe('$null');
expect(NULL_FILTER.labelKey).toBe('filterBuilder.operators.isNull');
expect(NULL_FILTER.labelKey).toBe('filterBuilder.operators.is_null');
});

it('the chip the list renders names the condition instead of showing a bare `true`', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ describe('drill escape hatch vs the empty bucket (objectui#9085, repaired by obj
expect(NULL_FILTER.flag).toBe('true');
expect(NULL_FILTER.op).toBe('is_null');
expect(NULL_FILTER.key).toBe('$null');
expect(NULL_FILTER.labelKey).toBe('filterBuilder.operators.isNull');
expect(NULL_FILTER.labelKey).toBe('filterBuilder.operators.is_null');
});

it('the new spelling NO LONGER serializes identically to the bare null it replaced', () => {
Expand Down
4 changes: 2 additions & 2 deletions packages/app-shell/src/views/drillUrlFilters.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ describe('the is-null operator: `filter[<field>][null]=true`', () => {
// existing operator key and the render site resolves it — pinned against a
// real non-English render in `ObjectDataPage.filterChipI18n-9159.test.tsx`.
expect(groupFilterChips([['owner', 'is_null', true]])).toEqual([
{ field: 'owner', textKey: 'filterBuilder.operators.isNull' },
{ field: 'owner', textKey: 'filterBuilder.operators.is_null' },
]);
// And it finishes NO text of its own, so nothing can render that bare
// `true` even if the render site forgot the key.
Expand Down Expand Up @@ -418,7 +418,7 @@ describe('the is-not-null operator and its synonyms (objectui#9508)', () => {

it('renders a chip carrying the is-not-null operator KEY, not `= true`', () => {
expect(groupFilterChips([['owner', 'is_not_null', true]])).toEqual([
{ field: 'owner', textKey: 'filterBuilder.operators.isNotNull' },
{ field: 'owner', textKey: 'filterBuilder.operators.is_not_null' },
]);
expect(groupFilterChips([['owner', 'is_not_null', true]])[0].text).toBeUndefined();
// And it is a DIFFERENT key from the is-null chip's — a single shared key
Expand Down
Loading
Loading