Repository navigation
fix(plugin-list): the Sort picker's field list asks the column read check (objectui#11943) - #11962
Merged
Merged
Conversation
…heck (objectui#11943) The Sort picker built its list from the object definition with no field-level read check, so it offered a field the grid had dropped, and choosing it sent a sort the server refuses, blanking the list. - objectui#11925's read predicate moves out of the filterFields memo into one module-level function, canReadField, which the Filter panel's list and sortFields both call. The filter list's behaviour is unchanged. - sortFields drops every unreadable field the current sort does not name. A field the current sort already names stays listed as before; making it removable only needs a SortBuilder change and is the card's follow-up. - Before the permission answer loads nothing is filtered, as the columns defer. - The effectiveFields comment now states what is true: which lists are built from it, and which ask the read themselves. Claude-Session: https://claude.ai/code/session_01MgfduSkFrfM3eorB3UGfAU Co-authored-by: Claude <noreply@anthropic.com>
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
This was referenced Oct 8, 2026
akarma-synetal
pushed a commit
to akarma-synetal/objectui
that referenced
this pull request
Oct 9, 2026
…1) and the Interfaces page-create (objectui#11932) (objectstack-ai#11967) Part of objectstack-ai#11861 Re-lands objectui#11823 step 3: objectui#11932, the Studio Interfaces page-create (seat `domain:ui#3`). That card stays open. Clause-②: no Held as a draft until objectui#11949 (the +5,120 B allowance, ruling `6056819248` step 2) lands. When this PR was opened, objectui#11949 was already on `main` as `1c51e973b`, and this branch has merged it. ## What this is A plain `git revert 3c888c6`, followed by merges of `main`. `3c888c6` is the rollback PR objectui#11956, a single-parent squash, so `-m 1` does not apply. It brings back: - objectui#11931: Studio's validation "New" menu opens on common rules, with the rule types under "Advanced". This is the validation-rule entry of objectui#11861. - objectui#11932: objectui#11823 step 3. The Interfaces pillar creates a page and opens it on its source beside a live preview. Claim `6057967120` on objectui#11861 · review `6057399892` on objectui#11937 · ruling `6056819248` on objectui#11942. ## Tree proof - The revert commit `7eeffd29b` has the tree of `f3a0488` (`011fe521e`), so `git diff f3a0488 7eeffd2 --stat` prints nothing. - `.changeset/11937-eager-budget-rollback.md` was added by `3c888c6` and never existed at `f3a0488`. Its removal therefore shows against `3c888c6`, not against `f3a0488`. - Against `main`, the branch changes the 15 paths `3c888c6` changed, and no other path. - Twelve of the 15 hold their `f3a0488` blob byte for byte. The other three were also changed on `main` after `3c888c6`, and they carry both intents (next section). ## Merges of main, and the conflicts The branch merged `main` five times: at `1c51e973b`, `d73d98770`, `70e3d7721`, `ccc2824cf` and `247f50349`. Only two of the PRs these brought in touch a restored file. **objectui#11945** (objectui#11923, the *Runs on* row), merged at `70e3d7721`. - One conflicting hunk, in the import block of `ObjectValidationsPanel.tsx`. This side imports `validationPresets.js`; `main` imports `ScriptValidationSchema` / `ScriptValidationParsed` from `@objectstack/spec/data`. Both lines are kept. - Two files merged without conflict: - `metadata-admin/i18n.ts`: `main` removes the `engine.studio.rules.event.delete` row, en and zh. It stays removed. - the `newRuleWaits-11820` pin: `main` asserts there is no Delete box. That assertion stays. - Check, per file: the changed lines of the merge against `main` equal the re-land's own changed lines (`3c888c6..f3a0488`). The changed lines of the merge against this branch's previous head equal objectui#11945's own diff. Both comparisons were identical for all three files. **objectui#11935** (objectui#11894, typed is-empty operators), merged at `247f50349`. - Two conflicting hunks, both in `ObjectValidationsPanel.tsx`. Each side added the same `FieldOpt` member, `type`. - The interface keeps one `type`, `main`'s `multiple` and the presets' `system`. One doc comment names both readers of `type`. - The field mapping keeps one `type` line, `main`'s `multiple` line with its comment, and the presets' `system` line. - `i18n.ts` merged without conflict, and both sides' rows are kept. - The same check, run on this merge, leaves only the shared `type` member and the merged comments as residual lines. `main` moved on after `247f50349` (objectui#11961, objectstack-ai#11962, objectstack-ai#11944). None of those touches any of the 15 paths, so they are not merged here. ## Changesets - Restored: `.changeset/11861-validation-presets.md` and `.changeset/11823-page-create.md`, both `@object-ui/app-shell: patch`. - Removed: `.changeset/11937-eager-budget-rollback.md` (`@object-ui/app-shell: patch`). This re-land supersedes it. - `check-changeset-overwrite` is report-only and exits 0. It reports one pre-existing changeset deleted, with `@object-ui/app-shell` "GONE from the declaration" of that file. - The package still bumps, because both restored changesets declare it. This is the gate's case 3: a changeset superseded by a replacement in the same change. ## Gates, on head `fb0c91d45` (every merge above included) Tests use the repo-root `pnpm exec vitest run` and run under the shared verify lock. - **Named set:** 98 files and 631 tests, all passed. It covers: - all eight `ObjectValidationsPanel*` suites, including objectui#11945's `runsOn-11923` and objectui#11935's `typedEmptyOps-11894`; - every test file that imports `interfaceCreate` or `StudioDesignSurface`; - `column-identity.ratchet.test.ts`; - the `main` pins next to the conflicts: `ConditionBuilder.typedEmptyOps-11894`, `PagePreview.pageKind-11933` and `SourcePageEditor.pageKind-11933`. - **Widened set:** 87 files and 1,103 tests, all passed. It covers every other test that imports `validationPresets`, `ObjectValidationsPanel` or the designer string table `metadata-admin/i18n.ts`. - `pnpm --filter @object-ui/app-shell type-check` passes, after a turbo build of the app-shell dependency closure (28 of 28 tasks). - Each of these gates exits 0: - `check:i18n-keys`, `check:i18n-designer-parity`, `check:i18n-drift`; - `check-changeset-presence`, `check-changeset-fixed`, `check-changeset-no-major`, `check:changeset-claims`, `check:pending-changeset-literals`, `check-changeset-overwrite` (report-only, see above); - `check:unreferenced-sources`. - eslint on the 11 touched source and test files reports 0 errors and 22 warnings. The repo sets no `--max-warnings`. The warnings are the ones already in these files at `f3a0488`, and the merge adds none. - `check-governed-queue-guard --test` reports NOT GOVERNED: none of the 15 paths is on a governed surface. - Not run: the console build. CI's `Bundle Analysis` measures the first-load bytes against the ceiling that objectui#11949 raised. `scripts/check-eager-closure-budget.mjs` is not touched. - Reference point: on its push run, `f3a0488` (the tree this re-land restores) had 66 check runs succeed, 6 skipped and 1 fail, `Bundle Analysis`. That is 73 of 73 runs read. --- _Generated by [Claude Code](https://claude.ai/code/session_01MgfduSkFrfM3eorB3UGfAU)_ Co-authored-by: Claude <noreply@anthropic.com>
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Part of #11943
Clause-②: no
The Sort picker no longer offers a field the caller may not read. Choosing such a field sent a sort the server refuses with 403, and the list went blank. Left for the follow-up: an unreadable field that the current sort already names stays listed exactly as before, unmarked and still choosable in the picker's other rows, because listing it as removable only needs a new
SortBuilderprop (objectui#11943's ruling B; the seat returns that half to triage).What changes
filterFieldsmemo into a module-level function inListView.tsx,canReadField(perms, objectName, field). It isperms.checkField(objectName, field, 'read')behind the sameisLoadedgate as the column list. The Filter panel's list and the Sort picker's list both call it. The filter list behaves exactly as before: its seven objectui#11925 pins run unchanged and stay green.sortFieldsasks it first. A field the user may not read is dropped unless the current sort names it. Before the permission answer loads nothing is filtered, matchingeffectiveFields. A dropped lookup no longer raises the relational hint, which explains a missing relation the user could read.effectiveFieldscomment states what is true. The old one said unreadable columns also disappear from the hide-fields popover, the filter and sort builders and$selectbecause they are filtered there. None of those lists is built fromeffectiveFields. The new comment names which lists are built from it, which ask the read themselves, and the Sort picker's in-use exception with its follow-up.Route note. The seat asked for the predicate at component scope inside
ListView. It lives at module scope in the same file instead. A component-scope function would be either auseCallbackidentity in twouseMemodependency lists, which AGENTS.md #10 forbids, or a per-render closure thatreact-hooks/exhaustive-depsflags in both memos. As a plain function ofpermsandschema.objectName, both of which the memos already list, it needs neither.Pins
packages/plugin-list/src/__tests__/ListView.sortFieldRead-11943.test.tsxreads the realSortBuilderdropdown throughMePermissionsProvider:isLoaded: a provider refetching with the restricted answer still held (checkFieldwould deny,isLoadedis false) lists everything, as the columns do;Reverse check, on the committed fix
985ec3c, each mutation landed and restored throughscripts/ablation-replace.mjs(anchor 1 to 0, blob0edf2348fddechanged, restore blob equal to HEAD andgit diff HEADempty), running the new file plus the objectui#11925 file:Tests 3 failed / 9 passed (12): pins 1, 3, 5isLoadedleg ofcanReadFieldTests 1 failed / 11 passed (12): pin 3Tests 1 failed / 11 passed (12): pin 4The seven objectui#11925 pins stayed green under all three mutations.
Gates
On HEAD
985ec3c, through the container's verify lock where heavy:pnpm --workspace-concurrency=2 --filter '@object-ui/plugin-list^...' build:VERDICT command-exit 0(13 of 47 workspace projects).pnpm --filter @object-ui/plugin-list type-check(tsc --noEmit && tsc -p tsconfig.test.json): exit 0.--listFilesOnlylists the new test file.pnpm exec vitest run --maxWorkers=2 packages/plugin-list/src/__tests__/ListView packages/core/src/utils/__tests__/column-identity.ratchet.test.ts, which covers everyListView*file in plugin-list (the objectui#11925 pins among them), the new file and the ratchet:Test Files 83 passed (83),Tests 789 passed (789).node scripts/check-changeset-presence.mjs: exit 0, "2 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s)".node scripts/check-changeset-no-major.mjs: exit 0.check:new-line-citations:VERDICT new-cross-file-line-citations: 0 new citation(s).check:control-bytes: OK.check:changeset-claims,check:pending-changeset-literals,check:test-path-roots,check:vi-mock-specifiers,check:vi-mock-inherit,check:vi-mock-override-shape,check:phantom-deps,check:unreferenced-sources,check:i18n-keys,check:shell-escape-residue.eslint --format jsonover the 2 touched TS files (the config's**/*.{ts,tsx}population) found 2 files and 0 errors.ListView.tsxhas 188 warnings, equal to its base blob linted over stdin; the new test has 0.eslint.config.jsenables no type-aware linting and noeslint-rulesrule reads other files, so untouched files cannot move. Repo-wide lint is CI's.Acceptance notes
allFields) lists every declared column with no read check, so a restricted user can see an unreadable column's label there. Toggling it changes nothing, because the column is already gone. The oldeffectiveFieldscomment claimed the opposite. The new comment makes no claim about that popover.maind92b2a1before the change. Changeset:.changeset/11943-sort-field-read.md, a patch for@object-ui/plugin-list.Session:
https://claude.ai/code/session_01MgfduSkFrfM3eorB3UGfAUGenerated by Claude Code