fix(fields): the criteria builder stops calling an empty criteria "All records" (#2962)

Renderer half of objectstack#3896 / objectstack-ai/objectstack#3929.

`FilterConditionField` renders `sys_sharing_rule.criteria_json`. With no criteria it displayed "All records" — describing a bug as a feature, since the server evaluated that shape as a match-all grant. The server now refuses to save such a rule; this is the renderer catching up.

The empty read-only state now says the rule shares nothing, in destructive styling, retranslated across all ten locales. A new `criteriaRequired` hint shows under the builder and the JSON editor while the criteria is empty, because the server's rejection only arrives as a toast after Save — the form does not map server `fields[]` onto per-field errors. `isMatchAllCriteria` is exported as a client-side mirror of the server predicate and only decides whether the hint shows; the server stays authoritative.

Author: os-zhuang

.changeset/sharing-criteria-not-match-all.md

@@ -0,0 +1,41 @@
+---
+"@object-ui/fields": minor
+"@object-ui/i18n": minor
+---
+
+fix(fields): the sharing-criteria builder stops calling an empty criteria "All records" (objectstack#3896)
+
+`FilterConditionField` renders `sys_sharing_rule.criteria_json`. With no
+criteria it displayed **"All records"**, and `filterGroupToMongo` carried a
+matching `// empty = match all` comment. That was describing a bug as a
+feature: a sharing rule with no predicate was stored as `criteria_json: null`
+and evaluated as `find(object, { filter: {} })` under the system context —
+every record of the object, granted to the recipient. `SharingRuleSchema` had
+always forbidden the shape ("never seeded as a permissive match-all",
+ADR-0049); the REST and data-API entries just never checked.
+
+objectstack#3896 closes those entries: the server now refuses to save a rule
+whose criteria would match everything, and one already stored shares nothing.
+This is the renderer catching up.
+
+- **The empty read-only state now says the rule shares nothing**, in
+  `destructive` styling — key renamed `fields.filterCondition.allRecords` →
+  `fields.filterCondition.noCriteria`, retranslated across all ten locales.
+  Nothing else read the old key.
+- **A new `fields.filterCondition.criteriaRequired` hint** renders under the
+  builder (and the JSON editor) while the criteria is empty. The server's
+  rejection is precise but only arrives as a toast *after* Save; this says it
+  while the admin is still looking at the empty builder.
+- **`isMatchAllCriteria` is exported** — a client-side mirror of the server
+  predicate covering `{}`, `[]`, and the vacuous combinators (`{ $and: [] }`,
+  `{ $or: [{}] }`), conservative in the same direction. The server stays
+  authoritative; this only decides whether to show the hint.
+
+Unparsable JSON keeps its own `invalidJson` message and does **not** also
+collect the empty-criteria hint.
+
+Note for anyone wiring this end-to-end: the Criteria field is not marked
+`required` in the object metadata, deliberately — `sys_sharing_rule.criteria_json`
+is nullable in deployed tenants, so `required: true` would only produce a
+destructive `NOT NULL` migration that those nulls block. The invariant lives in
+the server's write guards; this change makes the UI stop contradicting it.

packages/fields/src/widgets/FilterConditionField.tsx

@@ -144,7 +144,11 @@ function filterGroupToMongo(group: BuilderGroup, typeOf: (f: string) => string |
   const frags = (group?.conditions ?? [])
     .map((c) => condToMongo(c, typeOf))
     .filter((x): x is Record<string, any> => !!x);
-  if (frags.length === 0) return null; // empty = match all
+  // No conditions → no predicate. NOT "match all": a sharing rule with an
+  // empty criteria is refused on save, and one already stored shares nothing
+  // (objectstack#3896 / ADR-0049). `null` becomes an empty stored value, which
+  // is what the required-criteria messaging below keys off.
+  if (frags.length === 0) return null;
   if (group.logic === 'or') return { $or: frags };
   if (frags.length === 1) return frags[0];
   const keys = frags.flatMap((f) => Object.keys(f));
@@ -233,6 +237,37 @@ function mongoToFilterGroup(mongo: any): BuilderGroup | null {
   return { id: 'root', logic: 'and', conditions };
 }
 
+/**
+ * Would this criteria select EVERY record of the object?
+ *
+ * Mirrors the server's `isMatchAllCriteria` (objectstack `plugin-sharing`,
+ * #3896) closely enough to warn before the round-trip: blank, `{}`, `[]`, and
+ * the vacuous combinators. Deliberately conservative in the same direction —
+ * the cost of a false positive is one extra hint, the cost of a false negative
+ * is a save that fails with a toast. The server stays authoritative.
+ *
+ * @internal exported for tests
+ */
+export function isMatchAllCriteria(parsed: any): boolean {
+  if (parsed == null) return true;
+  if (Array.isArray(parsed)) return parsed.every(isMatchAllCriteria);
+  if (typeof parsed !== 'object') return true;
+  const entries = Object.entries(parsed);
+  if (entries.length === 0) return true;
+  for (const [key, value] of entries) {
+    if (key === '$and') {
+      if (!Array.isArray(value) || value.every(isMatchAllCriteria)) continue;
+      return false;
+    }
+    if (key === '$or') {
+      if (!Array.isArray(value) || value.length === 0 || value.some(isMatchAllCriteria)) continue;
+      return false;
+    }
+    return false;
+  }
+  return true;
+}
+
 function stringifyValue(value: string | object | undefined | null): string {
   if (value == null || value === '') return '';
   if (typeof value === 'string') return value;
@@ -291,6 +326,13 @@ export function FilterConditionField({
     [parsed],
   );
 
+  // Only flag a criteria that PARSES to match-all. Unparsable JSON has its own
+  // message (`invalidJson`) and must not collect a second, contradictory one.
+  const isEmptyCriteria = React.useMemo(
+    () => parsed.ok && isMatchAllCriteria(parsed.mongo),
+    [parsed],
+  );
+
   // Raw JSON mode: forced when the stored value can't be represented in the
   // builder; otherwise opt-in via the toggle.
   const representable = parsed.ok && group !== null;
@@ -319,9 +361,13 @@ export function FilterConditionField({
 
   if (readonly) {
     if (!rawValue.trim()) {
+      // Used to read "All records" — which was both wrong and the most
+      // dangerous thing this widget could say (objectstack#3896). A rule with
+      // no criteria has never usefully shared everything; it now shares
+      // nothing and is refused on save, so name that instead.
       return (
-        <span className={cn('text-sm text-muted-foreground', className)}>
-          {t('fields.filterCondition.allRecords')}
+        <span className={cn('text-sm text-destructive', className)}>
+          {t('fields.filterCondition.noCriteria')}
         </span>
       );
     }
@@ -353,6 +399,18 @@ export function FilterConditionField({
           onChange={handleBuilderChange as any}
         />
       )}
+      {/*
+        The server refuses to save a criteria that would select every record
+        (objectstack#3896), but that rejection only arrives as a toast after
+        the admin hits Save. Say it here, while they are still looking at the
+        empty builder — and never imply that leaving it empty means "share
+        everything", which is what this widget used to do.
+      */}
+      {isEmptyCriteria && (
+        <span className="text-xs text-destructive">
+          {t('fields.filterCondition.criteriaRequired')}
+        </span>
+      )}
       <button
         type="button"
         className="self-start text-xs text-muted-foreground underline-offset-2 hover:underline"

packages/fields/src/widgets/__tests__/FilterConditionField.matchAll.test.ts

@@ -0,0 +1,66 @@
+/**
+ * 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.
+ */
+
+/**
+ * Empty-criteria guard for the sharing-rule criteria builder
+ * (objectstack#3896).
+ *
+ * This widget used to tell an admin that an empty criteria meant "All
+ * records" — the most dangerous thing it could say, because it was describing
+ * a bug as a feature. A sharing rule with no predicate was stored as
+ * `criteria_json: null` and then evaluated as `find(object, { filter: {} })`,
+ * i.e. every record of the object granted to the recipient. `SharingRuleSchema`
+ * had always forbidden that shape ("never seeded as a permissive match-all",
+ * ADR-0049); the REST and data-API entries just never checked.
+ *
+ * The server now refuses to save such a rule and shares nothing for one
+ * already stored. These tests pin the client-side mirror of that predicate, so
+ * the widget warns while the admin is still looking at the empty builder
+ * rather than after a rejected save.
+ */
+import { describe, it, expect } from 'vitest';
+import { isMatchAllCriteria } from '../FilterConditionField';
+
+describe('isMatchAllCriteria — shapes that would share every record', () => {
+  it('flags the empty and vacuous shapes', () => {
+    for (const shape of [
+      undefined,
+      null,
+      {},           // what a blank builder / blank textarea parses to
+      [],
+      true,
+      42,
+      { $and: [] },
+      { $or: [] },
+      { $and: [{}] },
+      { $or: [{ stage: 'won' }, {}] }, // one unconstrained branch opens the OR
+    ]) {
+      expect(isMatchAllCriteria(shape), `${JSON.stringify(shape) ?? String(shape)}`).toBe(true);
+    }
+  });
+
+  it('passes anything that actually narrows the result set', () => {
+    for (const shape of [
+      { stage: 'won' },
+      { amount: { $gte: 100000 } },
+      [{ stage: 'won' }],
+      { $and: [{ stage: 'won' }] },
+      { $or: [{ a: 1 }, { b: 2 }] },
+      { health: 'red', budget: { $gt: 1 } },
+    ]) {
+      expect(isMatchAllCriteria(shape), JSON.stringify(shape)).toBe(false);
+    }
+  });
+
+  it('agrees with the server on the reported case: a rule saved with no criteria', () => {
+    // The builder emits '' for an empty group, which the widget parses to {}.
+    expect(isMatchAllCriteria({})).toBe(true);
+    // …and one real condition is enough to clear the gate.
+    expect(isMatchAllCriteria({ health: 'red' })).toBe(false);
+  });
+});

packages/fields/src/widgets/useFieldTranslation.ts

@@ -58,7 +58,13 @@ const FIELD_DEFAULTS: Record<string, string> = {
   'fields.recipient.selectPosition': 'Select a position',
   'fields.recipient.selectUnitAndSubordinates': 'Select a business unit',
   'fields.filterCondition.selectObjectFirst': 'Select an object first.',
-  'fields.filterCondition.allRecords': 'All records',
+  // objectstack#3896 — this used to be 'All records'. An empty criteria never
+  // meant "share everything"; it meant the predicate was missing, and the
+  // sharing evaluator failed open on it. Such a rule is now refused on save
+  // and shares nothing, so say that rather than advertise the old bug.
+  'fields.filterCondition.noCriteria': 'No criteria — this rule shares nothing',
+  'fields.filterCondition.criteriaRequired':
+    'Add at least one condition. A rule with no criteria would share every record, so it cannot be saved.',
   'fields.filterCondition.invalidJson': 'Invalid JSON — the rule will match no records until fixed.',
   'fields.filterCondition.jsonOnly': 'This criteria can only be edited as JSON',
   'fields.filterCondition.editAsJson': 'Edit as JSON',

packages/i18n/src/locales/ar.ts

@@ -190,7 +190,8 @@ const ar = {
     },
     filterCondition: {
       selectObjectFirst: "اختر كائناً أولاً.",
-      allRecords: "كل السجلات",
+      noCriteria: "لا توجد معايير — لا تشارك هذه القاعدة أي شيء",
+      criteriaRequired: "أضف شرطًا واحدًا على الأقل. القاعدة بدون معايير ستشارك كل السجلات، لذا لا يمكن حفظها.",
       invalidJson: "JSON غير صالح — لن تطابق القاعدة أي سجل حتى يتم إصلاحه.",
       jsonOnly: "لا يمكن تحرير هذا المعيار إلا بصيغة JSON",
       editAsJson: "التحرير بصيغة JSON",

packages/i18n/src/locales/de.ts

@@ -190,7 +190,8 @@ const de = {
     },
     filterCondition: {
       selectObjectFirst: "Wählen Sie zuerst ein Objekt.",
-      allRecords: "Alle Datensätze",
+      noCriteria: "Keine Kriterien – diese Regel teilt nichts",
+      criteriaRequired: "Fügen Sie mindestens eine Bedingung hinzu. Eine Regel ohne Kriterien würde jeden Datensatz teilen und kann daher nicht gespeichert werden.",
       invalidJson: "Ungültiges JSON — die Regel trifft auf keinen Datensatz zu, bis dies behoben ist.",
       jsonOnly: "Dieses Kriterium kann nur als JSON bearbeitet werden",
       editAsJson: "Als JSON bearbeiten",

packages/i18n/src/locales/en.ts

@@ -196,7 +196,8 @@ const en = {
     },
     filterCondition: {
       selectObjectFirst: 'Select an object first.',
-      allRecords: 'All records',
+      noCriteria: 'No criteria — this rule shares nothing',
+      criteriaRequired: 'Add at least one condition. A rule with no criteria would share every record, so it cannot be saved.',
       invalidJson: 'Invalid JSON — the rule will match no records until fixed.',
       jsonOnly: 'This criteria can only be edited as JSON',
       editAsJson: 'Edit as JSON',

packages/i18n/src/locales/es.ts

@@ -190,7 +190,8 @@ const es = {
     },
     filterCondition: {
       selectObjectFirst: "Selecciona primero un objeto.",
-      allRecords: "Todos los registros",
+      noCriteria: "Sin criterios: esta regla no comparte nada",
+      criteriaRequired: "Añade al menos una condición. Una regla sin criterios compartiría todos los registros, por lo que no se puede guardar.",
       invalidJson: "JSON no válido: la regla no coincidirá con ningún registro hasta que se corrija.",
       jsonOnly: "Este criterio solo se puede editar como JSON",
       editAsJson: "Editar como JSON",

packages/i18n/src/locales/fr.ts

@@ -190,7 +190,8 @@ const fr = {
     },
     filterCondition: {
       selectObjectFirst: "Sélectionnez d'abord un objet.",
-      allRecords: "Tous les enregistrements",
+      noCriteria: "Aucun critère — cette règle ne partage rien",
+      criteriaRequired: "Ajoutez au moins une condition. Une règle sans critères partagerait tous les enregistrements ; elle ne peut donc pas être enregistrée.",
       invalidJson: "JSON invalide — la règle ne correspondra à aucun enregistrement tant qu'elle n'est pas corrigée.",
       jsonOnly: "Ce critère ne peut être modifié qu'en JSON",
       editAsJson: "Modifier en JSON",

packages/i18n/src/locales/ja.ts

@@ -190,7 +190,8 @@ const ja = {
     },
     filterCondition: {
       selectObjectFirst: "先にオブジェクトを選択してください。",
-      allRecords: "すべてのレコード",
+      noCriteria: "条件が未設定です — このルールは何も共有しません",
+      criteriaRequired: "条件を1つ以上追加してください。条件のないルールは全レコードを共有してしまうため、保存できません。",
       invalidJson: "JSON が不正です — 修正するまでこのルールはどのレコードにも一致しません。",
       jsonOnly: "この条件は JSON でのみ編集できます",
       editAsJson: "JSON で編集",