[6.x] Forms 2: Various file upload improvements#15006
Merged
Merged
Conversation
duncanmcclean
changed the title
duncanmcclean
changed the title
duncanmcclean
marked this pull request as ready for review
July 15, 2026 13:35
…oads... For example: 1. Upload files 2. Submit the page 3. Go back to the previous page (the one w/ files) 4. Attempt to resubmit the page Before this commit, a validation error would be thrown complaining that you haven't uploaded a valid file. After this commit, you don't see any validation errors because the array/min/max rules are removed before validation happens if it detects existing file uploads. Obviously, if you upload a new file, it will be re-validated as you'd expect.
merges stuff from `extraRules` and the `withoutRulesForAlreadyUploadedFiles` method i added. should make things _slightly_ easier to read and understand.
DeleteTemporaryFiles bailed early when a form had no `files` or `store: false` upload fields, so a form with only `store: true` fields never had its `form-uploads/{id}` directory removed. For abandoned partial submissions - which are never promoted to real assets - this left their temporary uploads orphaned on disk.
The directory is now always deleted regardless of each field's `store` setting; only `files`/`store: false` field values are removed from the submission, so promoted asset paths are preserved. SendEmails also dispatches the cleanup for any upload field, so `store: true`-only forms get their temp directory cleaned on finalize too.
Carried-over upload values were validated with a byte-for-byte equality check against what's stored, so removing one of several already-uploaded files (resending a subset) was rejected as a forged value. Only clearing the field entirely or resending it unchanged was allowed. Values not present in the request now just have to be a subset of what's already stored against the field. Removing files validates normally, while paths that were never uploaded - which could be used to read or delete arbitrary files at finalize - are still prohibited.
and add config option to change form file uploads path
CreateAssetsFromFileUploads was ShouldQueue, so finalize()'s dispatchSync serialized it and the asset-path rewrite landed on a detached copy. The original submission kept the temp path, which DeleteTemporaryFiles then persisted, leaving store:true uploads pointing at a non-existent temp path (augmentation returned null). Run the job in-process so the rewrite sticks. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…e CP preload() looked up the asset with Asset::find($value) using the bare path that submissions store, but Asset::find needs a container-qualified id, so it returned null and the CP submission view showed an un-clickable filename with no download link. Qualify the path with the container, mirroring augment(). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… a fieldset Fields imported into a form via a fieldset import keep their raw assets/files type instead of being normalized to upload, unlike inline form fields. Since the dedicated assets/files form field views were removed, these fields resolved to the default text input instead of a file upload input. Fallback::equivalentFormFieldtype() now maps assets/files to the upload form fieldtype so the view falls back correctly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
FormUpload::preload() typehinted its map closure as string $value, so a non-string element in the field value (e.g. a null slot from a cleared or partial submission) threw a TypeError while rendering the submission in the CP. augment() already iterates the same values untyped; drop the hint to match and let the existing basename/asset-not-found path handle it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CreateAssetsFromFileUploads::uploadAsset() built the Symfony UploadedFile directly from $disk->path($diskPath). That method just concatenates the disk's configured root with the path — it doesn't guarantee a real local filesystem location. For the default local disk that happens to work, but now that file_uploads_disk is configurable, a cloud disk (S3 etc.) returns a bare object key, so the UploadedFile pointed nowhere and asset creation silently failed downstream in Uploader::upload()'s getRealPath() call. Stream the temp upload down to a real local temp file (matching the pattern used in ReplacementFile::writeTo()) and build the UploadedFile from that instead, cleaning up both the local temp file and the remote temp object afterwards. Added a test using a minimal in-memory Flysystem adapter to simulate a non-local disk, since Storage::fake() is itself a local driver and wouldn't have caught this. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CreateAssetsFromFileUploads::uploadAsset() passed the readStream() resource straight into file_put_contents() without closing it afterwards, matching the fclose pattern already used for readStream in Imaging/ResponseFactory. Close it explicitly. Replaced the 111-line hand-rolled InMemoryFlysystemAdapter with a ~10-line NonLocalPathFilesystemAdapter that wraps a real local Flysystem backend and only overrides path(), which is the one thing that needs to lie to reproduce a cloud-disk-style bug. exists(), readStream(), mimeType(), and delete() all hit real files instead of being reimplemented. Confirmed it still fails against the pre-fix code (FileNotFoundException from the bogus path()). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Member
|
Built on top of the original work with runtime verification in a live sandbox, which surfaced and fixed several issues: Fixes
Verified end-to-end
(Yes, I had Claude write that. Can you tell?) |
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
2 participants
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.
This pull request makes various improvements to how File Uploads are handled in Forms.
Upload fieldtype
This PR adds a new Upload fieldtype, designed to better facilitate file uploads on forms.
Previously, the Upload form fieldtype resolved to either the
assets/filesfieldtypes, depending on whether the uploaded file should be stored or not.This worked great — until we started building automagic form pages in Forms Pro, which reuses fieldtype Vue components. The
assetsandfilesfieldtypes had different UIs, which was a little confusing, but they also hit/cpURLs which require authentication (something a visitor filling out a form obviously doesn't have).FormUploadFieldtype.vuehas two states:filesfieldtype. Instead of uploading files the moment they're added, they're uploaded when the page is submitted. This is the state used on the Automagic Form page.Deferring asset creation until the submission finalises
Previously, a
store: trueupload field created a real Asset as soon as that page of a form was submitted. On multi-page forms, this meant an Asset could be created before the submission was actually complete — if the visitor abandoned the form partway through, that Asset was left orphaned.Uploads now land in temporary storage first (
storage/statamic/form-uploads/{submission}/{field}), and are only turned into real Assets once the submission is finalized, via a newCreateAssetsFromFileUploadsjob.Submission::finalize()runs through this order: create assets, send emails (with attachments), then delete anything left over in temporary storage viaDeleteTemporaryFiles, which also cleans up old-stylefilesfield uploads.This timing only really matters for multi-page forms. On a single-page form, submission and finalization happen in the same request, so the behavior is effectively unchanged.
Support for
assets&filesfieldsForms on
6.xcurrently use theassetsfieldtype for fields which should be retained after submission, or thefilesfieldtype for files which should be deleted once the email's sent.Form blueprint fields will be converted to their "form fieldtype" equivalents when updating to Forms 2. As part of this,
assetsandfilesfields are migrated to the new Upload fieldtype w/ the relevant config options.However, fields imported from a fieldset are not converted as field sets continue to use "normal" fieldtypes. This means we still need to maintain support for
assetsandfilesfields as they work right now.