Commit 789d42b
authored
feat(tools): reference an existing file from a content element (#865)
Closes #834. The seventh purpose-built writer, and the first that
creates a `sys_file_reference`. Demo content with images stopped at the
upload: nothing could make an uploaded file part of the site, because
the `files` group is read-only by design (ADR-047) and the only FAL
write was `set_file_alternative_text` on a metadata row that already
existed.
## A control caught my own design, and the design lost
The tool wrote through the DataHandler **twice**, and I had a
measurement to justify it: a probe showed a single-pass write leaving
the element's counter one short and the new reference sorting *first*.
Both issue comments say so.
Then I disabled the second pass and **every test still passed**.
The probe had run without `$GLOBALS['LANG']`. In that environment the
defect is real; in a complete backend environment — the only one this
tool will run in, because `refuseWithoutBackendEnvironment()` checks — a
single datamap carrying the parent list and the `NEW_` placeholder
produces the right count *and* the right `sorting_foreign`. One pass
now, `substNEWwithIDs` only to identify the row for the read-back, and
the docblock says what was measured rather than what I remembered.
The issue comments are corrected rather than left standing.
## Three refusals the description did not name
Each came from the live TCA, not from reasoning about it:
| | |
|---|---|
| **The field must accept the file.** | `image` takes fourteen
extensions, `assets` twenty-seven, `media` anything. A `.docx` on
`image` is a relation the FormEngine would reject, so it is refused
rather than written. |
| **`sys_language_uid` must be in the payload.** | Without it a
non-admin's write dies inside `checkRecordEditAccess()` on a missing
`languageField`. Stated rather than defaulted, which also pins the tool
to the default language like the other FAL writer. |
| **`title`, `alternative`, `description` are `exclude` fields.** | The
DataHandler drops them for a user without the grant — silently, empty
`errorLog`. Without checking them the tool reports a caption it did not
write. |
That last one is also what made the read-back **testable**: remove the
grant and the guard fires.
## Discarding is a rollback, not a delete
When the read-back fails, removing the reference row is not enough — the
element's counter still names it, which is exactly the inconsistent
state this tool exists to avoid producing. `discard()` now writes the
parent field back to the list that was there before the call. A test
caught that too.
## Six controls, each observed
| defect injected | result |
|---|---|
| read-back stops checking the texts | fails |
| `discard()` stops restoring the counter | fails |
| extension check removed | fails |
| ambiguity branch removed | fails |
| storage gate removed | fails |
| page-permission check removed | fails |
Restored tree: 13 tests green. An earlier run of this same harness
reported two of these as *undetected*; both were failed `sed` anchors,
so the test had run against an unmodified file. The harness now compares
the checksum before and after and says "anchor missed" instead of
"undetected" — a control that cannot fail is worse than none.
## One measured fact worth recording
**No CType that ships with the core offers two of `image` / `assets` /
`media`.** `textpic` and `image` show `image`, `textmedia` shows
`assets`, nothing shows two. The "several fields → name the one you
mean" branch is therefore unreachable with core content types, so the
test registers a two-field type of its own rather than leaving the
branch an untested claim.
There is no `position` argument: the write appends by construction, and
an argument nothing reads is worse than none.
## Verification
| gate | result |
|---|---|
| `-s functional -d sqlite` (this class) | 13 tests, 86 assertions, exit
0 |
| `-s functional` (tool surfaces) | 32 tests, 193 assertions, exit 0 |
| `-s unit` (full) | 7289 tests, 24795 assertions, exit 0 |
| `-s phpstan` | No errors |
| `-s cgl -n` | SUCCESS |
| `-s rector -n` (pinned 8.2, run locally) | Rector is done |
| `composer ci:test:changelog` | exit 0 |
Every counting surface moves with it: **47 tools → 48**, **six writers →
seven**, across `Tools.rst`, `README.md`, seven landing-page data files
in both languages, and the four tests that pin those numbers.
_Assisted by claude-code:claude-opus-5 —
[Session](https://claude.ai/code/session_01MNg1MysJVugv1xo2husknU)_20 files changed
Lines changed: 1273 additions & 27 deletions
File tree
- Classes/Service/Tool/Builtin
- Configuration
- Documentation/Administration
- Resources
- Private/Language
- Public/Icons
- Tests
- Functional/Service/Tool
- Unit
- Service/Tool/Builtin
- landingpage/build/data
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
8 | 8 | | |
9 | 9 | | |
10 | 10 | | |
| 11 | + | |
| 12 | + | |
11 | 13 | | |
12 | 14 | | |
13 | 15 | | |
| |||
Lines changed: 759 additions & 0 deletions
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
96 | 96 | | |
97 | 97 | | |
98 | 98 | | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
99 | 103 | | |
100 | 104 | | |
101 | 105 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
43 | 43 | | |
44 | 44 | | |
45 | 45 | | |
46 | | - | |
| 46 | + | |
47 | 47 | | |
48 | 48 | | |
49 | | - | |
| 49 | + | |
| 50 | + | |
50 | 51 | | |
51 | 52 | | |
52 | 53 | | |
| |||
283 | 284 | | |
284 | 285 | | |
285 | 286 | | |
286 | | - | |
| 287 | + | |
287 | 288 | | |
288 | | - | |
289 | | - | |
| 289 | + | |
| 290 | + | |
| 291 | + | |
290 | 292 | | |
291 | 293 | | |
292 | 294 | | |
| |||
344 | 346 | | |
345 | 347 | | |
346 | 348 | | |
| 349 | + | |
| 350 | + | |
| 351 | + | |
| 352 | + | |
| 353 | + | |
| 354 | + | |
| 355 | + | |
| 356 | + | |
| 357 | + | |
| 358 | + | |
| 359 | + | |
| 360 | + | |
| 361 | + | |
| 362 | + | |
| 363 | + | |
| 364 | + | |
| 365 | + | |
| 366 | + | |
| 367 | + | |
| 368 | + | |
| 369 | + | |
| 370 | + | |
| 371 | + | |
| 372 | + | |
| 373 | + | |
| 374 | + | |
| 375 | + | |
| 376 | + | |
| 377 | + | |
| 378 | + | |
| 379 | + | |
347 | 380 | | |
348 | 381 | | |
349 | 382 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
152 | 152 | | |
153 | 153 | | |
154 | 154 | | |
155 | | - | |
| 155 | + | |
156 | 156 | | |
157 | 157 | | |
158 | 158 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1796 | 1796 | | |
1797 | 1797 | | |
1798 | 1798 | | |
| 1799 | + | |
| 1800 | + | |
| 1801 | + | |
| 1802 | + | |
| 1803 | + | |
| 1804 | + | |
| 1805 | + | |
| 1806 | + | |
1799 | 1807 | | |
1800 | 1808 | | |
1801 | 1809 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1356 | 1356 | | |
1357 | 1357 | | |
1358 | 1358 | | |
| 1359 | + | |
| 1360 | + | |
| 1361 | + | |
| 1362 | + | |
| 1363 | + | |
| 1364 | + | |
1359 | 1365 | | |
1360 | 1366 | | |
1361 | 1367 | | |
| |||
Loading
0 commit comments