Fix/promote legacy images#39
Merged
Merged
Conversation
The image section of the page editor used to render two parallel sources side-by-side: a `<ul class="image-list--uploaded">` block above the FilePond widget for new uploads (with delete button, dimensions, size, and the title-PATCH endpoint), and a `<details>` block below for migrated 1.x entries that lived in `Item.data.images`. Legacy rows had no delete button, no dimensions, and a different title-save flow. The dual layout was also visually jarring on migrated pages. Collapse the two paths into one. A one-shot migration walks every Pages item, promotes each `data.images` entry into a proper `File` row pointing at the same on-disk asset (no file movement necessary — the migrator left the files where FileStorage expects them under `data/uploads-2.0/`), generates the editor's `300x300_<name>` thumbnail, and clears the entry from `data.images`. After that, only the unified upload row renders, and the legacy code paths can come out. Changes: - bin/promote-legacy-images.php: idempotent migration (skips on existing file rows by name), `--dry-run` and `--db=` flags, reports promoted / skipped / missing per item. - PagesModule: drop `renderLegacyImageRow()`, the `<details>` legacy block in `renderImagesSection()`, the `legacy_image_titles[]` POST splice in `saveAction()`, and the `images` key from `existingDataMap()`. - BasicTheme::headlineImage(): drop the `Page::$images` fallback branch — `findByItemAndField()` is the single source now. - Page DTO: remove the `$images` property and its constructor population. No consumer remains in the bundled theme. Smoke: - Migration on live db: 3 entries promoted (items #1/#3/#4), thumbnails generated, `data.images` cleared. - Frontend pages render headline images via the unified path (1200x0 / 800x350 thumbnails resolve over HTTP, sizes match). - Editor renders only the upload row block; FilePond widget sits alone at the bottom.
…can upload)
Two image-section issues going into one PR because they touch the same
render row + save flow:
1) Remove the per-image "save title" button + status span. Captions
travel with the page form as `image_titles[<fileId>]` inputs and
apply onto the matching FileRepository rows in `saveAction`,
scoped to files the page actually owns. The dedicated PATCH
endpoint and its JS click handler are gone.
2) New pages can now stage images. The image-section emits the
FilePond widget with `data-itemid="0"` instead of the cryptic
"Save the page first" message; the JS picks deferred mode
(`instantUpload: false`), hooks the form submit, and runs a
two-step flow:
- POST the page form via fetch with `X-Requested-With:
XMLHttpRequest`. saveAction returns JSON
`{status, pageId, redirect}` instead of 302 when XHR is
detected.
- Swap the new pageId into the FilePond `ondata` closure and
call `processFiles()` so each staged file uploads against
the freshly created page.
- Navigate to the redirect URL the server returned.
Existing-page edits keep their immediate-upload behaviour
(instantUpload stays on, no XHR submit interception).
Changes:
- PagesModule:
- renderImagesSection: drop the new-page early-return; emit the
widget with itemId=0 when no page yet.
- renderUploadedFileRow: drop save-title button + status span;
add `name="image_titles[<id>]"` to the title input; drop the
XHR-only data-* attributes.
- saveAction: detect XHR, route validation/save errors through
a new `saveError()` helper that emits JSON 400 in XHR mode
and falls back to the existing addMsg flow otherwise. On
success, return JSON with pageId+redirect for XHR or 302 for
HTML navigation.
- applyImageTitles(): persist captions posted with the form,
scoped to files owned by the saved page.
- jsonResponse: take an optional HTTP status (default 200).
- UploadEndpoint: drop `handlePatch()` and the PATCH branch in
`handle()`. PATCH now responds 405. Class header refreshed.
- filepond-init.js: drop the `.image-list__title-save` click
handler. Bail-out check on init now requires fieldId/csrf/url
but tolerates itemId=0 (deferred mode). New
`attachDeferredSubmit()` runs the page-save→process-files→
navigate sequence for new pages.
FilePond.create() replaces the original `<input class="filepond">`
with its own root element and detaches the input node from the DOM.
The deferred-submit handler called `input.closest('form')` *after*
that, so it always saw a detached node and got null back — meaning
the submit listener was never attached, the form fell through to a
plain HTTP POST, and any FilePond-staged files were lost when the
browser navigated to the redirect.
Net symptom: creating a new page with images "saved the page but
threw the images away."
- Capture the parent form once, before FilePond.create() runs.
- Pass it into attachDeferredSubmit() instead of re-resolving from a
detached input.
- Strip the FilePond `name="file"` field out of the FormData before
the page-save fetch, so the staged file isn't double-sent (the
page-save POST is now purely page fields; processFiles() handles
the upload afterwards).
…cleanup Two bugs in the on-disk cleanup path showed up after a page-delete: 1) Empty `<itemId>/<fieldId>/...` directories were left behind. The asset bytes and thumbnails were removed cleanly, but the directory chain itself (including the now-empty `thumbnail/` subdir) stayed on disk indefinitely, so deleting many pages slowly accumulated stale folders. 2) Thumbnail purge matched against `File.name` instead of the path basename. After a collision-free upload (e.g. a second `foo.png` lands at `<dir>/foo-2.png` while keeping `name = "foo.png"`), thumbnails are written as `300x300_foo-2.png` (path stem). The old purge looked for `*_foo.png` and would have wiped the *first* upload's thumbnail while leaving the actual `_foo-2.png` thumbnail orphaned. Item #17 in the live db reproduced both: directories left over, and `300x300_auz-3.png` orphaned in `13/7/thumbnail/`. Both call sites — the page-delete listener and the per-file DELETE endpoint — now funnel through a single `Scriptor\Boot\Files\DirectoryCleanup` helper that: - deletes the asset bytes, - walks `<dir>/thumbnail/` and removes every `<W>x<H>_<basename>` entry where the basename is taken from the path (matches what's actually written to disk), - rmdir's the thumbnail directory once it goes empty, - walks up the parent chain rmdir'ing each empty dir, stopping before the storage root so siblings with their own files are untouched. The endpoint's existing `if ($storage->exists($thumbDir))` guard was also wrong on its own — `exists()` checks `is_file()`, so the guard was always false for a directory and the thumbnail loop never ran. Replacing the inline logic with the helper drops that dead branch and gives DELETE the same scandir-based purge the listener already used for ItemDeleted. Smoke (against the live storage): - Synthetic page + 3-way collision upload (foo.png, foo-2.png, foo-3.png with 300x300 thumbnails for each) → page delete → itemDir, all assets, all thumbnails, all parents removed. - Stale empty dirs from item #17 / test-15 / item #16 swept by hand in the same session.
When the page doesn't exist yet (`itemId=0`), FilePond's default UI shows a per-staged-file "process" button next to the X. Clicking it would try to upload immediately against `itemId=0` and the upload endpoint would 400 because the page isn't saved yet. `allowProcess: !deferred` hides that button when we're staging for a new page. The X (remove from staging) button stays — users can still take a file back out before the form is submitted. Existing pages keep both buttons but the process button is rarely visible there since `instantUpload: true` auto-processes on drop.
The page editor now binds jQuery-UI sortable on `.image-list--uploaded`. Each image row carries a hidden `image_positions[<fileId>]` input — initialised to its render index, rewritten to the new index by the sortable's `update` callback whenever the user drops a row. The values travel with the next page-save, exactly like `image_titles[]`, and `applyImageMetadata()` writes them onto the matching `File.position` columns via `File::withPosition()`. `findByItemAndField()` already orders by `position, id`, so the next render reflects the saved order without further work. - PagesModule: rename applyImageTitles → applyImageMetadata (reads both arrays in one pass, single `findByItemAndField` lookup); renderUploadedFileRow takes a row index and emits the hidden position input. - filepond-init.js: bind sortable on the image list (jQuery-UI is already loaded via editor.js for the page-list reorder). On drag-end, walk siblings and rewrite each `.image-list__position` value to the new index. Note: depends on the iManager-side `File::withPosition()` addition — the `imanager-2.0` Composer path repo at `../imanager` already carries it locally; will land upstream in iManager via a separate small PR.
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.
No description provided.