Improve the Template Manager uploader - #1693
Draft
jakejackson1 wants to merge 1 commit into
Draft
Conversation
|
Coverage report for commit: 25d9eb8 Summary - Lines: 93.19% | Methods: 88.55% | Branches: 81.51%
🤖 Jest coverage report |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Coverage report for commit: 25d9eb8 Summary - Lines: 83.13% | Methods: 90.06%
🤖 PHPUnit coverage report |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Five changes to how PDF templates get installed. Multiple zips can now be selected or dropped together. Previously only the first survived: the saga used takeLatest, which cancelled every in-flight upload but the last, and the reducer held a single success/error object that concurrent results overwrote. Uploads now use takeEvery, results carry the filename they belong to and are appended to a templateUploadResults array, and the component drains that array with a batch counter so results arriving in one React render can't be lost. Each file reports its own outcome. Zips with the templates nested inside a folder now install. Safari auto-extracts a template zip on download; users then re-zip the folder, which buries the PHP files one level deep where the non-recursive get_all_templates_in_folder() couldn't see them. Helper_Templates now descends through single-directory wrappers to find the templates. Multiple directories in the root stay invalid, as #1336 specified. The upload limit goes from 10MB to 32MB, clamped by wp_max_upload_size(). The clamp matters because a POST over post_max_size is discarded by PHP before the request reaches us, which surfaced as a nonce failure rather than a size error. One constant now feeds the server-side validator, the JS pre-flight check and the error message, which reports the real limit via size_format(). The drop target is the whole Template Manager window instead of the tile at the foot of the list. TemplateUploader wraps the manager with noClick/noKeyboard and shares the file picker with the "Add New Template" tile through context. Dragging anywhere shows a full-viewport overlay, and progress and results appear in a toast pinned to the modal so they're visible wherever the list is scrolled. The bundled upload library moves to 4.0, a security-hardening release. Storage stages each upload and moves it into place rather than writing to the destination directly, refuses traversal, dotfiles, control characters and symlinked destinations, applies a default extension deny-list, and stores files as 0640. Template zips move from Extension + Mimetype to the new Validation\FileType: the old pair checked two independent allow-lists, so the extension and the sniffed contents never had to describe the same format. The octet-stream allowance is kept, because plenty of servers report a zip that way. GFPDF\Helper\Fonts\LocalFile overrides isValid() wholesale to skip the is-uploaded-file check, so it did not inherit 4.0's reset of the error list -- without it, upload() calling isValid() again reported every font validation error twice. Note for review: composer.json points at dev-hardening while GravityPDF/Upload#20 is open. It needs repointing at ^4.0 once that is tagged, before this can merge. Closes #1336 Closes #1337 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
jakejackson1
force-pushed
the
feat/template-manager-uploader-development
branch
from
August 14, 2026 07:02
8a9f410 to
25d9eb8
Compare
jakejackson1
marked this pull request as draft
August 14, 2026 07:02
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
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.
Summary
Four fixes to how PDF templates get installed via the Template Manager.
Multiple zips can be selected or dropped together. Only the first one used to install. The saga used
takeLatest, which cancelled every in-flight upload but the last, and the reducer kept a single success/error object that concurrent results overwrote. Uploads now usetakeEvery, each result carries the filename it belongs to, and the component drains an append-only results array with a batch counter so nothing is lost when several results land in the same React render. Every file reports its own outcome, so one bad zip no longer hides the rest.Zips with the templates inside a folder now install. Safari auto-extracts a template zip on download. People then re-zip that folder, which buries the PHP files a level deeper than the installer looked, and the upload failed with "No valid PDF template found in Zip archive." The installer now descends through single-directory wrappers to find the templates. Multiple directories in the root of the archive are still invalid, as #1336 asked for.
The upload limit goes from 10MB to 32MB. It is clamped by
wp_max_upload_size()so we never accept a file PHP will reject. That clamp matters: a POST overpost_max_sizegets discarded before the request reaches us, and the user saw a nonce failure instead of a size error. The message now names the real limit, so a host capped at 8MB says so.The drop target is the whole window. It used to be the small "Add New Template" box at the foot of the list. Dragging a zip anywhere over the Template Manager now shows a full-window drop overlay, and progress and results appear in a toast pinned to the modal so you see them wherever the list is scrolled.
Closes #1336 — Allow template zip to contain folder in root of archive
Closes #1337 — Process template zips up to 20MB (this raises it to 32MB)
Try it
yarn wp-env start && yarn dev:buildOpen a form, add a PDF, and click Manage PDF Templates:
Test plan
upload_max_filesize, the error names that limit rather than 32MBMore info
Where the changes live
The nested-folder resolution and the size constant both live on
Helper_Templates, which already owns template discovery, rather than onModel_Templates. That keepsHelper_Data(which needs the size for the localised script data) from reaching into a Model, and letsget_template_root_dir()reuse the same directory-listing helper asget_all_templates_in_folder()instead of hand-rolling a secondFilesystemIteratorwalk.unzip_and_verify_templates()now returns the resolved directory so the copy step reads from the right place. Hidden directories (.git,.idea) are skipped during the descent. Root-level__MACOSXneeds no special handling because core'sunzip_file()already refuses to extract it.New
gfpdf_template_max_upload_sizefilter for anyone who wants a different ceiling.On the front end,
TemplateUploadermoved from being a leaf inside the template list to wrapping the whole manager (noClick/noKeyboard), sharing the react-dropzoneopen()handle and upload status with the newTemplateUploaderTilethroughTemplateUploaderContext. The composition stays insideTemplateListrather than moving up to the router soTemplateUploaderandreact-dropzoneremain in the lazily-loaded chunk.Note on concurrency
takeEveryfires the uploads in parallel, and browsers cap concurrent connections to roughly six. Combined with the higher size limit that is up to six large multipart POSTs, each holding a PHP worker through an unzip and a recursive copy. PreviouslytakeLatestmeant effectively one at a time. Templates are usually small so this is left parallel, but a boundedactionChannelpool drops in without touching the batch accounting if it turns out to matter.Testing
Groupand their own slugs, since the whole suite shares one WordPress instance and the existing upload test asserts on the number of templates in theCustomgroup🤖 Generated with AI