Repository navigation
Commit a7df45f
fix(fields): the lookup cell names the referenced record from the fields the viewer may read (objectui#10501) (#10592)
Fixes #10501
Clause-②: no
## What changes
`LookupCellRenderer` names the record a lookup points at through the
referenced object's schema: `displayField`, then `nameField`, then
`titleFormat`, then the type-aware derivation. It did that on the row as
served. On a backend that does not strip policy-denied keys, a
`titleFormat` or `nameField` naming a denied field printed that field in
every lookup cell.
- `resolveLookupRecordName` now resolves from the row with the fields
the loaded policy denies on the REFERENCED object removed. `id` and
`_id` are kept. This is the objectui#10411 / objectui#10434 rule, so the
ladder falls through exactly as it does for the row a stripping backend
(`FieldMasker`) serves.
- The policy is a **required** parameter of `resolveLookupRecordName`.
So every caller gets the gated row, and the compiler refuses a new
caller that leaves it out. The callers are the expanded record, the
JSON-encoded reference, each multi-value chip and the overflow chip's
`title`, and the fetch-on-demand path.
- The gate is one module-private copy, `withoutDeniedFields(record,
policy, objectName)`, in `packages/fields/src/index.tsx`.
`LookupField.tsx`'s helpers are not in reach: they are module-private in
a file the package entry re-exports whole (`export * from
'./widgets/LookupField.js'`). Reaching them would mean editing
`LookupField.tsx` and adding a new internal module, and both are outside
the claim's file surface. See the census below for the consolidation
question.
- No new public export. `pnpm check:unreferenced-sources` and
`check:phantom-deps` are green. `@object-ui/permissions` was already a
declared dependency, and it is already in the entry's static graph
through `LookupField.tsx`.
## Route change, measured: "`perms` in the effect's deps" would not
relabel
The triage and the claim put `perms` in the resolving effect's deps. I
measured that mechanism before building it, and it does not relabel. The
fetch-on-demand hook cached the resolved NAME in a module-level map, and
its effect returns early on a settled entry. A policy change re-runs the
effect, the effect returns at once, and the stale name stays.
So the cache now holds the fetched **record and schema**, and the name
is resolved on every render with the render's policy. That is the shape
objectui#10487 (PR objectui#10557) gave the lookup editor's hydrated
chip. A policy that loads or changes after the record arrived relabels
the mounted cell with no second read. The effect only fetches, so it has
no `perms` dependency to carry. The ablation M2b below is the evidence:
I put the name back in the effect WITH `policy` in its deps, and the
relabel pin went red.
The cache key still carries `displayField` (objectui#2926 ⑧). It is no
longer needed now that the entry holds a record, but dropping it would
change how many reads a screen makes, and this card does not make that
change.
## Reproduction (before the fix)
I wrote the new pin first and ran it on base `8740e86ce`: **7 failed, 2
passed**. The 2 that passed are the controls. Each denied-field row
failed on the served value, for example:
```text
AssertionError: expected '...' not to contain 'ada@example.com'
Received: "SPAN class="block max-w-full truncate" title="ada@example.com"...ada@example.com"
```
(The tag in the received line is spelled as a word so the text survives
this page.)
## The pin —
`packages/fields/src/__tests__/lookupCell.readGate-10501.test.tsx`
The tests use the real `PermissionProvider` and a backend that does not
strip. Each test uses its own referenced object, because the caches are
module-level.
- An expanded record whose `titleFormat` names a denied field prints the
name, and the text is the same as the text a stripping backend's row
gives.
- An expanded record whose `nameField` is denied prints the name.
- A bare id fetched on demand: the fetched record is named from its
readable fields.
- A multi-value cell: no chip and no overflow `title` prints a denied
value.
- A JSON-encoded reference prints the name.
- **Lit controls:** a field the policy does not deny still prints, on
the expanded path and the fetched path. With no provider (`isLoaded`
false), the row is named as served.
- **A policy that arrives after mount relabels the SAME mounted label
node** (identity asserted), on the expanded path and the fetched path.
The fetched path asserts `findOne` was called once: relabelling reads
the policy again, not the backend.
## Ablations: mutated on disk, restored by blob
Each leg ran on committed `HEAD` `4c1bad224`. The mutation went through
`ablation-replace.mjs` (anchor must hit, blob must change), was checked
with an on-disk `grep -c` marker, and was restored from `HEAD`. The
restore was proven by blob `9e540c8cffee` == `HEAD` blob and an empty
`git diff HEAD`. The test imports `../index` (source), so no `dist` is
involved.
| leg | mutation | marker | result |
|---|---|---|---|
| M1 | gate removed (`if (true` or the rest) | 1 | **7 failed / 2
passed**: every denied-field and relabel row is red, both controls green
|
| M2a | name resolved inside the fetch effect, policy NOT in deps | 1 |
**1 failed / 8 passed**: only "relabels … a bare id fetched on demand" |
| M2b | the same, WITH `policy` in the effect deps (the triage's
mechanism) | 1 + 1 | **1 failed / 8 passed**: same row, same stale
`title="ada@example.com"` |
| restored | none | 0 | 9 passed |
## Verification
All the checks below ran at final `HEAD` **`a0885627b`**, from the repo
root. The closure was built first with `turbo run build
--filter=@object-ui/fields^...`.
- `pnpm exec vitest run packages/fields/`: **201 files passed, 1
skipped; 3315 tests passed, 7 skipped**. The pin alone: 9/9.
- `pnpm --filter @object-ui/fields run type-check` (`tsc --noEmit && tsc
-p tsconfig.test.json`): exit 0. `tsc -p tsconfig.test.json --listFiles`
includes the new test file (count 1).
- eslint over the two touched files, with `--no-inline-config --format
json` (2 files linted, 0 errors):
- `index.tsx`: the per-rule counts are identical to base `8740e86ce`:
`react-refresh/only-export-components` 93, `no-explicit-any` 44,
`no-unused-vars` 1. **`react-hooks/exhaustive-deps` is 0 before and 0
after.**
- The new test file has 0 messages.
- The config enables no type-aware linting
(`tseslint.configs.recommended`, no `parserOptions.project`), so this
diff cannot move the verdict on any untouched file.
- These gates exit 0: `check:control-bytes`; `check:new-line-citations`
(0 new); `check-changeset-presence` (1 changeset, `@object-ui/fields`
patch); `check-changeset-no-major`; `check-changeset-overwrite`;
`check-type-check-coverage`; `check:phantom-deps`;
`check:esm-specifiers` (specifiers only); `check-test-path-roots`;
`check-vi-mock-override-shape`; `check:self-import`;
`check:unreferenced-sources`.
- `check:changeset-claims` flags 4 pending changesets that name
`packages/fields/src/index.tsx`: 6625, 6694, 6837 and 7166. I read each
paragraph. None is falsified: this diff adds no field-meta read and does
not touch `decimals`.
- NOT MEASURED locally: the eager-closure budget (it needs built
artifacts, so CI runs it). `@object-ui/permissions` was already in the
entry's static graph through `LookupField.tsx`.
## Census: every copy of "the row as the viewer may read it" (for the
seat's decision)
The claim asks for this census and bars an export on this card. Five
copies are on `main` after this PR, and a sixth is in open PR
objectui#10570.
| file | symbol | exported? | keeps | gate source | shape |
|---|---|---|---|---|---|
| `packages/fields/src/widgets/LookupField.tsx` | `fieldReadGate` +
`withoutDeniedFields(record, readable)` | module-private (the file is
`export *`'d by the entry) | `id`, `_id`, declared `idField` |
`usePermissions()` | a gate closure plus a row filter (two functions) |
| `packages/fields/src/widgets/RecordPickerDialog.tsx` |
`withoutDeniedFields(record, perms, objectName, idField)` |
module-private (the file is `export *`'d by the entry) | `id`, `_id`,
`idField` | `usePermissions()` | row filter |
| `packages/fields/src/index.tsx` (this PR) |
`withoutDeniedFields(record, policy, objectName)` | module-private |
`id`, `_id` | `usePermissions()`, typed as the Pick of
`isLoaded`/`checkField` | row filter |
| `packages/plugin-detail/src/withoutDeniedFields.ts` |
`withoutDeniedFields(record, perms, objectName)` | a module export, not
a package export | `id`, `_id` | `usePermissions()`, the same Pick | row
filter |
| `packages/app-shell/src/views/RecordDetailView.tsx` |
`withoutDeniedFields(record, perms, objectName)` | module-private |
`id`, `_id` | `usePermissions()`, the same Pick | row filter |
| `packages/react/src/hooks/useRecordSearch.ts` (open PR objectui#10570,
not on `main`) | `readableRow(record, objectName, policy)` |
module-private | `id`, `_id` | the caller passes a structural
`fieldReadPolicy` (react does not depend on permissions) | row filter,
argument order differs |
The six share one body: return the row unchanged unless the policy is
loaded and an object is named. Keep `id`, `_id` and any extra identity
key. Drop every key for which `checkField(object, key, 'read')` is
false. Return the SAME object when nothing is withheld.
One pure export would replace all six. Its signature would be `(record,
policy, objectName, extraKeep?)`, where `policy` is structural and needs
only `isLoaded` and `checkField`. The differences it has to absorb are
the `idField` extra key in two fields copies, the closure split in
`LookupField`, and the argument order in react.
The dependency facts that decide where the export lives:
- `@object-ui/permissions` is a runtime dependency of app-shell, fields,
plugin-calendar, plugin-dashboard, plugin-detail, plugin-form,
plugin-gantt, plugin-grid, plugin-kanban, plugin-list, plugin-map,
plugin-timeline, plugin-tree and plugin-view. `apps/console` lists it
only as a devDependency.
- **`@object-ui/react` does not depend on it.** `@object-ui/permissions`
depends only on `@object-ui/types`, so adding it to react would not
create a cycle.
- `@object-ui/core` is already a dependency of fields, plugin-detail,
app-shell and react. A structural-policy export there would need no new
edge anywhere. An export from `@object-ui/permissions` would need react
to add one edge.
A different shape is **not** covered by a row export: the
column/field-LIST filters. These are `PeoplePicker`'s `$expand`,
subtitle and avatar filters, `LookupField`'s `readablePreviewColumns`
and expand list, and the `keepReadableColumns` family in `RelatedList`
and the grids. They filter a list of names, not a row.
## Acceptance notes
- These notes are not filed and are not in scope.
- `UserCellRenderer` names an embedded user from `name` / `username` /
`image` off the row as served. It does not go through a schema or a
policy. A policy that denies `name` on the user object would still print
it. I did not probe this: there is no repro and no claim that such a
policy exists. Terms a later search could use: `UserCellRenderer`
`denied name` and `user cell FLS`.
- The cache key keeps `displayField` (see above). Dropping it would save
one read when two columns point at the same record with different
display fields. That is a fetch-count change, not a correctness change.
## Overlap
- PR objectui#10568 edits the masked-cell region of the same file
(`MaskedCellRenderer` / `buildStandardCellRendererMap`). This PR touches
only the lookup resolver, `useLookupName` and `LookupCellRenderer`, so
the hunks are disjoint.
- The live claim objectui#10493 (`ImageCellRenderer`): this PR does not
touch it.
This body was written by
`https://claude.ai/code/session_01BA3nKVUwKQJf8DBxrSVtNC` (the
`domain:ui` seat 1 dispatch).
---
_Generated by [Claude
Code](https://claude.ai/code/session_01BA3nKVUwKQJf8DBxrSVtNC)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 1daf477 commit a7df45f
3 files changed
Lines changed: 385 additions & 21 deletions
File tree
- .changeset
- packages/fields/src
- __tests__
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
| 264 | + | |
| 265 | + | |
| 266 | + | |
| 267 | + | |
| 268 | + | |
| 269 | + | |
| 270 | + | |
| 271 | + | |
0 commit comments