Uh oh!
There was an error while loading. Please reload this page.
fix(fields): the criteria builder stops calling an empty criteria "All records" (objectstack#3896) - #2962
Merged
Conversation
…l records"
`FilterConditionField` renders `sys_sharing_rule.criteria_json`. With no
criteria it displayed "All records", and `filterGroupToMongo` carried a
matching `// empty = match all` comment. That described 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 such a
rule, and one already stored shares nothing. This is the renderer catching up,
because a UI that advertises the failure mode is how an admin ends up choosing
it on purpose.
- The empty read-only state says the rule shares nothing, in destructive
styling. Key renamed `fields.filterCondition.allRecords` →
`.noCriteria`, retranslated across all ten locales; nothing else read it.
- New `.criteriaRequired` hint 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` exported — a client mirror of the server predicate
covering `{}`, `[]`, and the vacuous combinators, conservative in the same
direction. The server stays authoritative; this only gates the hint.
Unparsable JSON keeps its own `invalidJson` message and does not also collect
the empty-criteria hint.
The field is deliberately NOT marked `required` in the object metadata:
`criteria_json` is nullable in deployed tenants, so `required: true` would only
produce a destructive NOT NULL migration those nulls block. The invariant lives
in the server's write guards; this stops the UI contradicting it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QuViRSR1j6GJjf9qGbnqFXThe latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
Uh oh!
There was an error while loading. Please reload this page.
os-zhuang added a commit
that referenced
this pull request
Jul 29, 2026
#2966) The server's VALIDATION_FAILED always carried fields[] — one entry per offending field with a human message — and every form dropped it, showing one undirected toast. On a long form the offending field is usually off-screen, so Save appeared to do nothing. Three layers, each of which was dropping the detail: @object-ui/react gains extractFieldErrors(err), normalising the three shapes the error arrives in (typed ValidationError, the raw client error whose details falls back to the whole response body, and the hand-rolled duck-typed shape); @object-ui/data-objectstack maps a 400 VALIDATION_FAILED onto the ValidationError class that had been exported and never once constructed, and `create` now normalises at all (only `update` did); the form renderer applies entries via form.setError — but only when every rejected field has a visible input, falling through to the banner otherwise so the part the user cannot see inline is still said out loud. The toast + scroll-and-focus of the first offender is shared with the client-side invalid handler (announceFieldErrors): to the person filling in the form these are the same event — only the referee differs. Removes the reason for client-side predicate mirroring (#2962): a form no longer has to guess what the server will reject. Non-field failures (403, permission denials) take exactly the path they took before. 14 + 6 new tests; 107 files / 1073 tests green; tsc clean on all three changed packages.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Renderer half of objectstack#3896 / objectstack-ai/objectstack#3929.
The problem
FilterConditionFieldrenderssys_sharing_rule.criteria_json. With no criteria it displayed "All records", andfilterGroupToMongocarried a matching// empty = match allcomment.That was describing a bug as a feature. A sharing rule with no predicate was stored as
criteria_json: nulland evaluated asfind(object, { filter: {} })under the system context — every record of the object, granted to the recipient.SharingRuleSchemahad 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 such a rule, and one already stored shares nothing. This is the renderer catching up — because a UI that advertises the failure mode is how an admin ends up choosing it deliberately.
Changes
destructivestyling. Key renamedfields.filterCondition.allRecords→fields.filterCondition.noCriteriaand retranslated across all ten locales. Nothing else read the old key (the otherallRecordskeys live underlist.*/console.objectView.*and are untouched).fields.filterCondition.criteriaRequiredhint under the builder and the JSON editor while the criteria is empty. The server's rejection message is precise, but it only arrives as a toast after Save — the form does not map serverfields[]onto per-field errors. This says it while the admin is still looking at the empty builder.isMatchAllCriteriaexported — 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 the hint shows.// empty = match allcomment is corrected.Unparsable JSON keeps its own
invalidJsonmessage and does not also collect the empty-criteria hint.Why the field is not marked
requiredWorth stating, since it is the obvious first instinct.
sys_sharing_rule.criteria_jsonis nullable in every deployed tenant (rows predating the server gate), sorequired: truein the object metadata would translate into a destructiveNOT NULLmigration that those very nulls block — objectstack'sschema-drift.tsclassifies itseverity: error/category: destructive.requiredWhenwas the other candidate and was rejected too: objectstack evaluates it server-side against the merged record on update, so it would block editing or backfilling a legacy criteria-less row — including thefull→editaccess-level backfill — a wider blast radius than the bug. The invariant is enforced in objectstack's write guards (defineRule+ asys_sharing_rulebeforeInserthook); this PR makes the UI stop contradicting it.Verification
FilterConditionField.matchAll.test.tspins the client predicate against the shapes the server rejects, and against the ones it must let through.packages/fields+packages/i18n: 51 files, 607 tests, all passing.type-checkclean; eslint reports 0 errors on the changed files (remaining warnings are pre-existing, in untouched lines).Merge order
Safe to merge independently — this PR only changes copy and adds a hint. It is most useful after objectstack#3929, since that is what makes "cannot be saved" true.
Generated by Claude Code