From bc4c31fa5f2445065422faaf6975bbee14e276f4 Mon Sep 17 00:00:00 2001 From: Duncan McClean Date: Tue, 14 Jul 2026 12:35:07 +0100 Subject: [PATCH 01/17] Add Upload fieldtype to handle form file uploads --- resources/js/bootstrap/fieldtypes.js | 2 + .../fieldtypes/FormUploadFieldtype.vue | 201 ++++++++++++++++++ .../views/forms/automagic-email.antlers.html | 7 +- src/Fieldtypes/FormUpload.php | 177 +++++++++++++++ src/Forms/DeleteTemporaryAttachments.php | 2 +- src/Forms/Email.php | 10 +- src/Forms/Fieldtypes/Upload.php | 29 ++- src/Forms/Form.php | 2 +- src/Forms/SendEmails.php | 3 +- src/Forms/Submission.php | 10 +- src/Forms/SubmitForm.php | 17 +- src/Jobs/DeletePartialFormSubmissions.php | 3 +- src/Providers/ExtensionServiceProvider.php | 1 + tests/Forms/Fieldtypes/UploadTest.php | 71 +++++++ tests/Forms/SubmitFormTest.php | 31 +++ 15 files changed, 535 insertions(+), 31 deletions(-) create mode 100644 resources/js/components/fieldtypes/FormUploadFieldtype.vue create mode 100644 src/Fieldtypes/FormUpload.php create mode 100644 tests/Forms/Fieldtypes/UploadTest.php diff --git a/resources/js/bootstrap/fieldtypes.js b/resources/js/bootstrap/fieldtypes.js index 4c071416139..6f1c83b7936 100644 --- a/resources/js/bootstrap/fieldtypes.js +++ b/resources/js/bootstrap/fieldtypes.js @@ -34,6 +34,7 @@ import GroupFieldtype from '../components/fieldtypes/GroupFieldtype.vue'; import FormBannerFieldtype from '../components/fieldtypes/FormBannerFieldtype.vue'; import FormHeadingFieldtype from '../components/fieldtypes/FormHeadingFieldtype.vue'; import FormParagraphFieldtype from '@/components/fieldtypes/FormParagraphFieldtype.vue'; +import FormUploadFieldtype from '@/components/fieldtypes/FormUploadFieldtype.vue'; import HiddenFieldtype from '../components/fieldtypes/HiddenFieldtype.vue'; import HtmlFieldtype from '../components/fieldtypes/HtmlFieldtype.vue'; import IconFieldtype from '../components/fieldtypes/IconFieldtype.vue'; @@ -116,6 +117,7 @@ export default function registerFieldtypes(app) { app.component('form_banner-fieldtype', FormBannerFieldtype); app.component('form_heading-fieldtype', FormHeadingFieldtype); app.component('form_paragraph-fieldtype', FormParagraphFieldtype); + app.component('form_upload-fieldtype', FormUploadFieldtype); app.component('hidden-fieldtype', HiddenFieldtype); app.component('html-fieldtype', HtmlFieldtype); app.component('icon-fieldtype', IconFieldtype); diff --git a/resources/js/components/fieldtypes/FormUploadFieldtype.vue b/resources/js/components/fieldtypes/FormUploadFieldtype.vue new file mode 100644 index 00000000000..7eb7b6faf59 --- /dev/null +++ b/resources/js/components/fieldtypes/FormUploadFieldtype.vue @@ -0,0 +1,201 @@ + + + diff --git a/resources/views/forms/automagic-email.antlers.html b/resources/views/forms/automagic-email.antlers.html index 8211aa631f5..3ea9b66e343 100644 --- a/resources/views/forms/automagic-email.antlers.html +++ b/resources/views/forms/automagic-email.antlers.html @@ -1,11 +1,16 @@ {{ fields }} {{ display }}: {{ if value }} - {{ if fieldtype == "assets" }} + {{ if (fieldtype == "form_upload" && config:store) || fieldtype == "assets" }} {{ value }} {{ permalink }} {{ if !last }},{{ /if }} {{ /value }} + {{ elseif fieldtype == "form_upload" }} + {{ value }} + {{ value | pathinfo:basename | sanitize }} + {{ if !last }},{{ /if }} + {{ /value }} {{ elseif fieldtype == "select" && !config:multiple }} {{ (value:label ?? value) | sanitize }} {{ elseif fieldtype == "radio" }} diff --git a/src/Fieldtypes/FormUpload.php b/src/Fieldtypes/FormUpload.php new file mode 100644 index 00000000000..7493143cb25 --- /dev/null +++ b/src/Fieldtypes/FormUpload.php @@ -0,0 +1,177 @@ +field->value()); + + return [ + 'files' => collect($values)->map(fn ($value) => $this->fileData($value))->values()->all(), + ]; + } + + private function fileData(string $value): array + { + if (! $this->config('store')) { + return ['filename' => basename($value)]; + } + + if (! $asset = Asset::find($value)) { + return ['filename' => $value]; + } + + return [ + 'filename' => $asset->basename(), + 'size' => $asset->size(), + 'download_url' => $asset->cpDownloadUrl(), + ]; + } + + public function preProcess($values) + { + if (! $this->config('store')) { + return $values ?? []; + } + + if (is_null($values)) { + return []; + } + + return collect($values)->map(fn ($value) => $this->valueToId($value))->filter()->values()->all(); + } + + private function valueToId(string $value): string + { + if (Str::contains($value, '::')) { + return $value; + } + + return optional($this->container()->asset($value))->id(); + } + + public function process($values) + { + if (! $this->config('store')) { + return $this->config('max_files') === 1 ? collect($values)->first() : $values; + } + + // A value is either a fresh upload's asset id (from AssetsUploader), or it's already a + // stored path from an earlier submission of this page riding along unchanged — e.g. going + // back and resubmitting a page without re-selecting its file. Only the former needs resolving. + $values = collect($values)->map(fn ($value) => Str::contains($value, '::') ? Asset::findOrFail($value)->path() : $value); + + return $this->config('max_files') === 1 ? $values->first() : $values->all(); + } + + public function augment($values) + { + if (! $this->config('store')) { + return $values; + } + + $values = Arr::wrap($values); + + $ids = collect($values)->map(fn ($value) => $this->container()->handle().'::'.$value)->all(); + + $query = new OrderedQueryBuilder($this->container()->queryAssets()->whereIn('path', $values), $ids); + + return $this->config('max_files') === 1 ? $query->first() : $query; + } + + public function shallowAugment($values) + { + if (! $this->config('store')) { + return $values; + } + + $items = $this->augment($values); + $items = $this->config('max_files') === 1 ? collect([$items]) : $items->get(); + + $items = $items->filter()->map(fn ($item) => $item->toShallowAugmentedCollection()); + + return $this->config('max_files') === 1 ? $items->first() : $items; + } + + private function container(): ?AssetContainerContract + { + if ($configured = $this->config('container')) { + if ($container = AssetContainer::find($configured)) { + return $container; + } + + throw new AssetContainerNotFoundException($configured); + } + + if (($containers = AssetContainer::all())->count() === 1) { + return $containers->first(); + } + + throw new UndefinedContainerException; + } + + public function rules(): array + { + $rules = ['array']; + + if ($max = $this->config('max_files')) { + $rules[] = 'max:'.$max; + } + + if ($min = $this->config('min_files')) { + $rules[] = 'min:'.$min; + } + + return $rules; + } + + public function fieldRules() + { + $classes = [ + 'dimensions' => DimensionsRule::class, + 'image' => ImageRule::class, + 'max_filesize' => MaxRule::class, + 'mimes' => MimesRule::class, + 'mimetypes' => MimetypesRule::class, + 'min_filesize' => MinRule::class, + ]; + + return collect(parent::fieldRules())->map(function ($rule) use ($classes) { + if (! is_string($rule)) { + return $rule; + } + + $name = Str::before($rule, ':'); + + if ($class = Arr::get($classes, $name)) { + $parameters = explode(',', Str::after($rule, ':')); + + return new $class($parameters); + } + + return $rule; + })->all(); + } +} diff --git a/src/Forms/DeleteTemporaryAttachments.php b/src/Forms/DeleteTemporaryAttachments.php index 2905f270cd0..a053680ed16 100644 --- a/src/Forms/DeleteTemporaryAttachments.php +++ b/src/Forms/DeleteTemporaryAttachments.php @@ -23,7 +23,7 @@ public function __construct(public Submission $submission) public function handle() { $this->submission->form()->blueprint()->fields()->all() - ->filter(fn (Field $field) => $field->type() === 'files') + ->filter(fn (Field $field) => $field->type() === 'files' || ($field->type() === 'form_upload' && ! $field->fieldtype()->config('store'))) ->each(function (Field $field) { Collection::wrap($this->submission->get($field->handle(), [])) ->each(fn ($path) => Storage::disk('local')->delete('statamic/file-uploads/'.$path)); diff --git a/src/Forms/Email.php b/src/Forms/Email.php index 013266d1d52..249ec626810 100644 --- a/src/Forms/Email.php +++ b/src/Forms/Email.php @@ -116,10 +116,14 @@ protected function addAttachments() } $this->getRenderableFieldData(Arr::except($this->submissionData, ['id', 'date', 'form'])) - ->filter(fn ($field) => in_array($field['fieldtype'], ['assets', 'files'])) + ->filter(fn ($field) => in_array($field['fieldtype'], ['assets', 'files', 'form_upload'])) ->each(function ($field) { $field['value'] = $field['value']->value(); - $field['fieldtype'] === 'assets' ? $this->attachAssets($field) : $this->attachFiles($field); + + $isStoredAsAsset = $field['fieldtype'] === 'assets' + || ($field['fieldtype'] === 'form_upload' && Arr::get($field, 'config.store')); + + $isStoredAsAsset ? $this->attachAssets($field) : $this->attachFiles($field); }); return $this; @@ -162,7 +166,7 @@ protected function addData() $fields = $this->getRenderableFieldData(Arr::except($augmented, ['id', 'date', 'form'])) ->reject(fn ($field) => $field['fieldtype'] === 'spacer') ->when(Arr::has($this->config, 'attachments'), function ($fields) { - return $fields->reject(fn ($field) => in_array($field['fieldtype'], ['assets', 'files'])); + return $fields->reject(fn ($field) => in_array($field['fieldtype'], ['assets', 'files', 'form_upload'])); }); $formConfig = ($configFields = Form::extraConfigFor($form->handle())) ? Blueprint::makeFromTabs($configFields)->fields()->addValues($form->data()->all())->values()->all() diff --git a/src/Forms/Fieldtypes/Upload.php b/src/Forms/Fieldtypes/Upload.php index 24e3af11753..237628febb0 100644 --- a/src/Forms/Fieldtypes/Upload.php +++ b/src/Forms/Fieldtypes/Upload.php @@ -9,7 +9,7 @@ class Upload extends FormFieldtype { - protected static $fieldtype = 'files'; + protected static $fieldtype = 'form_upload'; protected $description = 'Upload files - store them permanently or use them temporarily.'; protected $icon = 'upload-cloud'; protected $categories = ['media']; @@ -39,6 +39,12 @@ public function configFieldItems(): array 'max_items' => 1, 'if' => ['store' => true], ], + 'min_files' => [ + 'display' => __('Min Files'), + 'instructions' => __('Set a minimum number of selectable assets.'), + 'type' => 'integer', + 'min' => 1, + ], 'max_files' => [ 'display' => __('Max Files'), 'instructions' => __('The maximum number of files that may be uploaded.'), @@ -50,20 +56,14 @@ public function configFieldItems(): array public function toFieldArray(): array { - if ($this->config('store')) { - return [ - 'type' => 'assets', - 'container' => $this->config('container'), - 'folder' => $this->config('folder'), - 'max_files' => $this->config('max_files'), - ...Arr::except($this->config(), ['type', 'store', 'container', 'folder', 'max_files']), - ]; - } - return [ - 'type' => 'files', + 'type' => 'form_upload', + 'store' => $this->config('store'), + 'container' => $this->config('container'), + 'folder' => $this->config('folder'), + 'min_files' => $this->config('min_files'), 'max_files' => $this->config('max_files'), - ...Arr::except($this->config(), ['type', 'store', 'max_files']), + ...Arr::except($this->config(), ['type', 'store', 'container', 'folder', 'min_files', 'max_files']), ]; } @@ -80,9 +80,8 @@ public function example(): ?array public function view(): string { $language = config('statamic.templates.language', 'antlers'); - - // Check for user's custom assets or files views first (for backwards compatibility) $legacyType = $this->config('store') ? 'assets' : 'files'; + $legacyViews = [ "statamic::forms.fields.{$legacyType}", "statamic::forms.{$language}.fields.{$legacyType}", diff --git a/src/Forms/Form.php b/src/Forms/Form.php index 30dba24ce52..4bf8541c57b 100644 --- a/src/Forms/Form.php +++ b/src/Forms/Form.php @@ -644,7 +644,7 @@ public function editBlueprintUrl() public function hasFiles() { return $this->fields()->filter(function ($field) { - return in_array($field->fieldtype()->handle(), ['assets', 'files']); + return in_array($field->fieldtype()->handle(), ['assets', 'files', 'form_upload']); })->isNotEmpty(); } diff --git a/src/Forms/SendEmails.php b/src/Forms/SendEmails.php index 3398d566914..ba63bf477fd 100644 --- a/src/Forms/SendEmails.php +++ b/src/Forms/SendEmails.php @@ -57,7 +57,8 @@ private function emailConfigs($submission) protected function shouldDeleteTemporaryAttachments(): bool { return $this->submission->form()->blueprint()->fields()->all() - ->filter(fn (Field $field) => $field->fieldtype()->handle() === 'files') + ->filter(fn (Field $field) => $field->fieldtype()->handle() === 'files' + || ($field->fieldtype()->handle() === 'form_upload' && ! $field->fieldtype()->config('store'))) ->filter() ->count() > 0; } diff --git a/src/Forms/Submission.php b/src/Forms/Submission.php index 2432d76ee66..90b2bf78312 100644 --- a/src/Forms/Submission.php +++ b/src/Forms/Submission.php @@ -25,6 +25,7 @@ use Statamic\Forms\Uploaders\AssetsUploader; use Statamic\Forms\Uploaders\FilesUploader; use Statamic\Sites\Site; +use Statamic\Support\Arr; use Statamic\Support\Str; use Statamic\Support\Traits\FluentlyGetsAndSets; @@ -186,9 +187,12 @@ public function uploadFiles($uploadedFiles) return collect($uploadedFiles)->map(function ($files, $handle) { $field = $this->fields()->get($handle); - return $field['type'] === 'files' - ? FilesUploader::field($field)->upload($files) - : AssetsUploader::field($field)->upload($files); + $isStoredAsAsset = $field['type'] === 'assets' + || ($field['type'] === 'form_upload' && Arr::get($field, 'store')); + + return $isStoredAsAsset + ? AssetsUploader::field($field)->upload($files) + : FilesUploader::field($field)->upload($files); })->all(); } diff --git a/src/Forms/SubmitForm.php b/src/Forms/SubmitForm.php index 792a77a6cfa..2b2f2b2f4bd 100644 --- a/src/Forms/SubmitForm.php +++ b/src/Forms/SubmitForm.php @@ -108,7 +108,7 @@ public function submit(array $data, array $files = []): SubmissionResult private function normalizeFiles(array $files): array { $assetFields = $this->form->blueprint()->fields()->all() - ->filter(fn ($field) => in_array($field->fieldtype()->handle(), ['assets', 'files'])) + ->filter(fn ($field) => in_array($field->fieldtype()->handle(), ['assets', 'files', 'form_upload'])) ->keys(); foreach ($assetFields as $handle) { @@ -198,7 +198,7 @@ public function validate(array $data, array $files = [], ?array $only = null): v $validator = $fields ->validator() - ->withRules($this->extraRules($fields)) + ->withRules($this->extraRules($fields, $files)) ->validator(); if (! $only && $this->page) { @@ -212,12 +212,19 @@ public function validate(array $data, array $files = [], ?array $only = null): v $this->withLocale($this->site()?->lang(), fn () => $validator->validate()); } - private function extraRules($fields): array + private function extraRules($fields, array $files): array { return $fields->all() - ->filter(fn ($field): bool => in_array($field->fieldtype()->handle(), ['assets', 'files'])) + ->filter(fn ($field): bool => in_array($field->fieldtype()->handle(), ['assets', 'files', 'form_upload'])) + // Only validate as a file when one was actually uploaded in this request. + // Resubmitting a page whose file was uploaded earlier just resends its path, + // which isn't a file and shouldn't be re-validated as though it were one. + ->filter(fn ($field): bool => array_key_exists($field->handle(), $files)) ->mapWithKeys(function ($field): array { - $rules = $field->fieldtype()->handle() === 'assets' + $isStoredAsAsset = $field->fieldtype()->handle() === 'assets' + || ($field->fieldtype()->handle() === 'form_upload' && $field->fieldtype()->config('store')); + + $rules = $isStoredAsAsset ? array_merge(['file', new AllowedFile], $this->assetContainerRules($field)) : ['file', new AllowedFile($field->fieldtype()->config('allowed_extensions'))]; diff --git a/src/Jobs/DeletePartialFormSubmissions.php b/src/Jobs/DeletePartialFormSubmissions.php index df116781cbe..eafffbb0073 100644 --- a/src/Jobs/DeletePartialFormSubmissions.php +++ b/src/Jobs/DeletePartialFormSubmissions.php @@ -38,7 +38,8 @@ public function handle(): void private function garbageCollectAssets(Submission $submission): void { $submission->form()->blueprint()->fields()->all() - ->filter(fn ($field) => $field->fieldtype()->handle() === 'assets') + ->filter(fn ($field) => $field->fieldtype()->handle() === 'assets' + || ($field->fieldtype()->handle() === 'form_upload' && $field->fieldtype()->config('store'))) ->each(function ($field) use ($submission) { $container = $field->get('container'); diff --git a/src/Providers/ExtensionServiceProvider.php b/src/Providers/ExtensionServiceProvider.php index f06b5fa4f16..964a4af6dc8 100644 --- a/src/Providers/ExtensionServiceProvider.php +++ b/src/Providers/ExtensionServiceProvider.php @@ -94,6 +94,7 @@ class ExtensionServiceProvider extends ServiceProvider Fieldtypes\FormBanner::class, Fieldtypes\FormHeading::class, Fieldtypes\FormParagraph::class, + Fieldtypes\FormUpload::class, Fieldtypes\Hidden::class, Fieldtypes\Html::class, Fieldtypes\Icon::class, diff --git a/tests/Forms/Fieldtypes/UploadTest.php b/tests/Forms/Fieldtypes/UploadTest.php new file mode 100644 index 00000000000..b455df2a727 --- /dev/null +++ b/tests/Forms/Fieldtypes/UploadTest.php @@ -0,0 +1,71 @@ +setField(new FormField('avatar', [ + 'type' => 'upload', + 'store' => true, + 'container' => 'avatars', + 'folder' => 'uploads', + 'max_files' => 1, + ])); + + $this->assertEquals([ + 'type' => 'form_upload', + 'store' => true, + 'container' => 'avatars', + 'folder' => 'uploads', + 'min_files' => null, + 'max_files' => 1, + ], $fieldtype->toFieldArray()); + } + + #[Test] + public function it_shows_the_real_filename_for_a_stored_asset() + { + Storage::fake('test'); + Storage::disk('test')->put('avatar.jpg', ''); + AssetContainer::make('avatars')->disk('test')->save(); + + $field = (new Field('avatar', ['type' => 'form_upload', 'store' => true, 'container' => 'avatars'])) + ->setValue(['avatars::avatar.jpg']); + + $fieldtype = (new FormUpload)->setField($field); + + $files = $fieldtype->preload()['files']; + + $this->assertEquals('avatar.jpg', $files[0]['filename']); + } + + #[Test] + public function it_strips_the_temporary_folder_from_a_temp_files_display_filename() + { + $field = (new Field('document', ['type' => 'form_upload', 'store' => false])) + ->setValue(['1784026119/resume.pdf']); + + $fieldtype = (new FormUpload)->setField($field); + + $this->assertEquals([ + 'files' => [ + ['filename' => 'resume.pdf'], + ], + ], $fieldtype->preload()); + } +} diff --git a/tests/Forms/SubmitFormTest.php b/tests/Forms/SubmitFormTest.php index c06f247406e..0e3da29b6e8 100644 --- a/tests/Forms/SubmitFormTest.php +++ b/tests/Forms/SubmitFormTest.php @@ -307,9 +307,39 @@ public function it_uploads_files() $form->submissions()->each->delete(); } + #[Test] + public function it_does_not_revalidate_an_already_uploaded_file_when_resubmitting_its_page() + { + Storage::fake('avatars'); + AssetContainer::make('avatars')->disk('avatars')->save(); + + $form = $this->uploadForm(); + + $first = app(SubmitForm::class) + ->form($form) + ->page('main') + ->submit( + data: ['email' => 'test@example.com'], + files: ['avatar' => [UploadedFile::fake()->image('avatar.jpg')]], + ); + + // Going back to the page and resubmitting resends the already-resolved asset path as a + // plain string rather than a fresh upload. That shouldn't be revalidated as a file. + $result = app(SubmitForm::class) + ->form($form) + ->page('main') + ->resume($first->submission) + ->submit(data: ['email' => 'test@example.com', 'avatar' => $first->submission->get('avatar')]); + + $this->assertTrue($result->isFinalized()); + + $form->submissions()->each->delete(); + } + #[Test] public function it_validates_the_extension_of_uploaded_files() { + Bus::fake(); // Otherwise the temp file is deleted by DeleteTemporaryAttachments right after submission. Storage::fake('local'); // store: false makes this a temporary "files" upload rather than a stored asset. @@ -343,6 +373,7 @@ public function it_validates_the_extension_of_uploaded_files() ->submit(data: [], files: ['document' => [UploadedFile::fake()->create('resume.pdf', 10)]]); $this->assertTrue($result->isFinalized()); + Storage::disk('local')->assertExists('statamic/file-uploads/'.$result->submission->get('document')[0]); $form->submissions()->each->delete(); } From cf33d12977c989518ec0f334c4385c8104b57b9e Mon Sep 17 00:00:00 2001 From: Duncan McClean Date: Tue, 14 Jul 2026 12:42:46 +0100 Subject: [PATCH 02/17] refine --- src/Fieldtypes/FormUpload.php | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/Fieldtypes/FormUpload.php b/src/Fieldtypes/FormUpload.php index 7493143cb25..cf49679fb6c 100644 --- a/src/Fieldtypes/FormUpload.php +++ b/src/Fieldtypes/FormUpload.php @@ -18,8 +18,6 @@ use Statamic\Support\Arr; use Statamic\Support\Str; -use function Statamic\trans as __; - class FormUpload extends Fieldtype { protected $selectable = false; @@ -35,7 +33,7 @@ public function preload() private function fileData(string $value): array { - if (! $this->config('store')) { + if (! $this->storesAsAsset()) { return ['filename' => basename($value)]; } @@ -52,7 +50,7 @@ private function fileData(string $value): array public function preProcess($values) { - if (! $this->config('store')) { + if (! $this->storesAsAsset()) { return $values ?? []; } @@ -74,7 +72,7 @@ private function valueToId(string $value): string public function process($values) { - if (! $this->config('store')) { + if (! $this->storesAsAsset()) { return $this->config('max_files') === 1 ? collect($values)->first() : $values; } @@ -88,7 +86,7 @@ public function process($values) public function augment($values) { - if (! $this->config('store')) { + if (! $this->storesAsAsset()) { return $values; } @@ -103,7 +101,7 @@ public function augment($values) public function shallowAugment($values) { - if (! $this->config('store')) { + if (! $this->storesAsAsset()) { return $values; } @@ -115,6 +113,11 @@ public function shallowAugment($values) return $this->config('max_files') === 1 ? $items->first() : $items; } + private function storesAsAsset(): bool + { + return (bool) $this->config('store'); + } + private function container(): ?AssetContainerContract { if ($configured = $this->config('container')) { From 358e6574b92c027375cc84c9d0d41f143e3c4d79 Mon Sep 17 00:00:00 2001 From: Duncan McClean Date: Wed, 15 Jul 2026 13:03:12 +0100 Subject: [PATCH 03/17] Upload files to separate `form-uploads` directory & only create assets when finalizing --- config/forms.php | 5 +- .../views/forms/automagic-email.antlers.html | 5 - src/Fieldtypes/FormUpload.php | 66 ++----- src/Forms/CreateAssetsFromFileUploads.php | 91 +++++++++ src/Forms/DeleteTemporaryAttachments.php | 38 ---- src/Forms/DeleteTemporaryFiles.php | 47 +++++ src/Forms/SendEmails.php | 11 +- src/Forms/Submission.php | 13 +- src/Forms/Uploaders/FormFileUpload.php | 76 ++++++++ src/Forms/Uploaders/FormFileUploader.php | 27 +++ src/Jobs/DeletePartialFormSubmissions.php | 20 +- .../Forms/CreateAssetsFromFileUploadsTest.php | 181 ++++++++++++++++++ .../DeletePartialFormSubmissionsTest.php | 66 +------ tests/Forms/DeleteTemporaryFilesTest.php | 149 ++++++++++++++ tests/Forms/SendEmailsTest.php | 37 ++-- tests/Forms/SubmissionTest.php | 20 ++ tests/Forms/SubmitFormTest.php | 64 ++++++- tests/Forms/Uploaders/FormFileUploadTest.php | 74 +++++++ 18 files changed, 783 insertions(+), 207 deletions(-) create mode 100644 src/Forms/CreateAssetsFromFileUploads.php delete mode 100644 src/Forms/DeleteTemporaryAttachments.php create mode 100644 src/Forms/DeleteTemporaryFiles.php create mode 100644 src/Forms/Uploaders/FormFileUpload.php create mode 100644 src/Forms/Uploaders/FormFileUploader.php create mode 100644 tests/Forms/CreateAssetsFromFileUploadsTest.php create mode 100644 tests/Forms/DeleteTemporaryFilesTest.php create mode 100644 tests/Forms/Uploaders/FormFileUploadTest.php diff --git a/config/forms.php b/config/forms.php index 03617f1b0df..dce48ea7c18 100644 --- a/config/forms.php +++ b/config/forms.php @@ -41,15 +41,12 @@ |-------------------------------------------------------------------------- | | Partial submissions are automatically deleted after a set number of days. - | Set this to null to prevent their automatic deletion. You may also enable - | garbage collection to delete related assets at the same time. + | Set this to null to prevent their automatic deletion. | */ 'delete_partial_submissions_after' => 7, - 'garbage_collect_assets' => false, - /* |-------------------------------------------------------------------------- | Exporters diff --git a/resources/views/forms/automagic-email.antlers.html b/resources/views/forms/automagic-email.antlers.html index 3ea9b66e343..83e3cec7d1b 100644 --- a/resources/views/forms/automagic-email.antlers.html +++ b/resources/views/forms/automagic-email.antlers.html @@ -6,11 +6,6 @@ {{ permalink }} {{ if !last }},{{ /if }} {{ /value }} - {{ elseif fieldtype == "form_upload" }} - {{ value }} - {{ value | pathinfo:basename | sanitize }} - {{ if !last }},{{ /if }} - {{ /value }} {{ elseif fieldtype == "select" && !config:multiple }} {{ (value:label ?? value) | sanitize }} {{ elseif fieldtype == "radio" }} diff --git a/src/Fieldtypes/FormUpload.php b/src/Fieldtypes/FormUpload.php index cf49679fb6c..e4e54479e82 100644 --- a/src/Fieldtypes/FormUpload.php +++ b/src/Fieldtypes/FormUpload.php @@ -27,61 +27,27 @@ public function preload() $values = Arr::wrap($this->field->value()); return [ - 'files' => collect($values)->map(fn ($value) => $this->fileData($value))->values()->all(), + 'files' => collect($values)->map(function (string $value): array { + if (! $this->storesAsAsset()) { + return ['filename' => basename($value)]; + } + + if (! $asset = Asset::find($value)) { + return ['filename' => $value]; + } + + return [ + 'filename' => $asset->basename(), + 'size' => $asset->size(), + 'download_url' => $asset->cpDownloadUrl(), + ]; + })->values()->all(), ]; } - private function fileData(string $value): array - { - if (! $this->storesAsAsset()) { - return ['filename' => basename($value)]; - } - - if (! $asset = Asset::find($value)) { - return ['filename' => $value]; - } - - return [ - 'filename' => $asset->basename(), - 'size' => $asset->size(), - 'download_url' => $asset->cpDownloadUrl(), - ]; - } - - public function preProcess($values) - { - if (! $this->storesAsAsset()) { - return $values ?? []; - } - - if (is_null($values)) { - return []; - } - - return collect($values)->map(fn ($value) => $this->valueToId($value))->filter()->values()->all(); - } - - private function valueToId(string $value): string - { - if (Str::contains($value, '::')) { - return $value; - } - - return optional($this->container()->asset($value))->id(); - } - public function process($values) { - if (! $this->storesAsAsset()) { - return $this->config('max_files') === 1 ? collect($values)->first() : $values; - } - - // A value is either a fresh upload's asset id (from AssetsUploader), or it's already a - // stored path from an earlier submission of this page riding along unchanged — e.g. going - // back and resubmitting a page without re-selecting its file. Only the former needs resolving. - $values = collect($values)->map(fn ($value) => Str::contains($value, '::') ? Asset::findOrFail($value)->path() : $value); - - return $this->config('max_files') === 1 ? $values->first() : $values->all(); + return $this->config('max_files') === 1 ? collect($values)->first() : $values; } public function augment($values) diff --git a/src/Forms/CreateAssetsFromFileUploads.php b/src/Forms/CreateAssetsFromFileUploads.php new file mode 100644 index 00000000000..ee07f33082e --- /dev/null +++ b/src/Forms/CreateAssetsFromFileUploads.php @@ -0,0 +1,91 @@ +submission->form()->blueprint()->fields()->all() + ->filter(fn (Field $field): bool => $field->type() === 'form_upload' && $field->fieldtype()->config('store')) + ->reduce(function (bool $created, Field $field): bool { + $paths = $this->createAssets($field); + + if (! $paths) { + return $created; + } + + $this->submission->set($field->handle(), $paths); + + return true; + }, false); + + if ($created && $this->submission->form()->store()) { + $this->submission->saveQuietly(); + } + } + + private function createAssets(Field $field): array|string|null + { + $value = $this->submission->get($field->handle()); + + if (! $value) { + return null; + } + + $assetPaths = Collection::wrap($value) + ->filter() + ->map(fn (string $path) => $this->uploadAsset($field, $path)) + ->filter() + ->values(); + + if ($assetPaths->isEmpty()) { + return null; + } + + return $field->get('max_files') === 1 ? $assetPaths->first() : $assetPaths->all(); + } + + private function uploadAsset(Field $field, string $path): ?string + { + $diskPath = "statamic/form-uploads/{$path}"; + + if (! Storage::disk('local')->exists($diskPath)) { + return null; + } + + $uploadedFile = new UploadedFile( + Storage::disk('local')->path($diskPath), + basename($path), + Storage::disk('local')->mimeType($diskPath), + null, + true + ); + + $assetId = AssetsUploader::field($field->toArray())->upload($uploadedFile); + $assetId = is_array($assetId) ? $assetId[0] : $assetId; + + Storage::disk('local')->delete($diskPath); + + return Asset::findOrFail($assetId)->path(); + } +} diff --git a/src/Forms/DeleteTemporaryAttachments.php b/src/Forms/DeleteTemporaryAttachments.php deleted file mode 100644 index a053680ed16..00000000000 --- a/src/Forms/DeleteTemporaryAttachments.php +++ /dev/null @@ -1,38 +0,0 @@ -submission->form()->blueprint()->fields()->all() - ->filter(fn (Field $field) => $field->type() === 'files' || ($field->type() === 'form_upload' && ! $field->fieldtype()->config('store'))) - ->each(function (Field $field) { - Collection::wrap($this->submission->get($field->handle(), [])) - ->each(fn ($path) => Storage::disk('local')->delete('statamic/file-uploads/'.$path)); - - $this->submission->remove($field->handle()); - }); - - if ($this->submission->form()->store()) { - $this->submission->saveQuietly(); - } - } -} diff --git a/src/Forms/DeleteTemporaryFiles.php b/src/Forms/DeleteTemporaryFiles.php new file mode 100644 index 00000000000..9ffff45abac --- /dev/null +++ b/src/Forms/DeleteTemporaryFiles.php @@ -0,0 +1,47 @@ +submission->form()->blueprint()->fields()->all() + ->filter(fn (Field $field) => $field->type() === 'files' || ($field->type() === 'form_upload' && ! $field->fieldtype()->config('store'))); + + if ($uploadFields->isEmpty()) { + return; + } + + $uploadFields->filter(fn (Field $field) => $field->type() === 'files')->each(function (Field $field): void { + Collection::wrap($this->submission->get($field->handle(), [])) + ->each(fn ($path) => Storage::disk('local')->delete('statamic/file-uploads/'.$path)); + }); + + if (Storage::disk('local')->exists($uploadsPath = "statamic/form-uploads/{$this->submission->id()}")) { + Storage::disk('local')->deleteDirectory($uploadsPath); + } + + $uploadFields->each(fn (Field $field) => $this->submission->remove($field->handle())); + + if ($this->submission->form()->store()) { + $this->submission->saveQuietly(); + } + } +} diff --git a/src/Forms/SendEmails.php b/src/Forms/SendEmails.php index ba63bf477fd..6f3e32c5b4a 100644 --- a/src/Forms/SendEmails.php +++ b/src/Forms/SendEmails.php @@ -40,8 +40,8 @@ private function jobs(): Collection return new $class($this->submission, $this->site, $config); }) - ->when($this->shouldDeleteTemporaryAttachments(), function ($jobs) { - $jobs->push(new DeleteTemporaryAttachments($this->submission)); + ->when($this->shouldDeleteTemporaryFiles(), function ($jobs) { + $jobs->push(new DeleteTemporaryFiles($this->submission)); }); } @@ -54,12 +54,9 @@ private function emailConfigs($submission) return collect($config); } - protected function shouldDeleteTemporaryAttachments(): bool + private function shouldDeleteTemporaryFiles(): bool { return $this->submission->form()->blueprint()->fields()->all() - ->filter(fn (Field $field) => $field->fieldtype()->handle() === 'files' - || ($field->fieldtype()->handle() === 'form_upload' && ! $field->fieldtype()->config('store'))) - ->filter() - ->count() > 0; + ->contains(fn (Field $field) => $field->type() === 'files' || ($field->type() === 'form_upload' && ! $field->fieldtype()->config('store'))); } } diff --git a/src/Forms/Submission.php b/src/Forms/Submission.php index 90b2bf78312..0999c583f5a 100644 --- a/src/Forms/Submission.php +++ b/src/Forms/Submission.php @@ -17,15 +17,14 @@ use Statamic\Events\SubmissionFinalized; use Statamic\Events\SubmissionSaved; use Statamic\Events\SubmissionSaving; -use Statamic\Facades\Asset; use Statamic\Facades\File; use Statamic\Facades\FormSubmission; use Statamic\Facades\Site as Sites; use Statamic\Facades\Stache; use Statamic\Forms\Uploaders\AssetsUploader; use Statamic\Forms\Uploaders\FilesUploader; +use Statamic\Forms\Uploaders\FormFileUpload; use Statamic\Sites\Site; -use Statamic\Support\Arr; use Statamic\Support\Str; use Statamic\Support\Traits\FluentlyGetsAndSets; @@ -177,7 +176,7 @@ public function status(): string } /** - * Upload files and return asset IDs. + * Upload files and return their storage references. * * @param array $uploadedFiles * @return array @@ -187,10 +186,11 @@ public function uploadFiles($uploadedFiles) return collect($uploadedFiles)->map(function ($files, $handle) { $field = $this->fields()->get($handle); - $isStoredAsAsset = $field['type'] === 'assets' - || ($field['type'] === 'form_upload' && Arr::get($field, 'store')); + if ($field['type'] === 'form_upload') { + return FormFileUpload::field($field, $this->id())->upload($files); + } - return $isStoredAsAsset + return $field['type'] === 'assets' ? AssetsUploader::field($field)->upload($files) : FilesUploader::field($field)->upload($files); })->all(); @@ -271,6 +271,7 @@ public function finalize() SubmissionFinalized::dispatch($this); + CreateAssetsFromFileUploads::dispatchSync($this); SendEmails::dispatch($this, $this->site()); return $this; diff --git a/src/Forms/Uploaders/FormFileUpload.php b/src/Forms/Uploaders/FormFileUpload.php new file mode 100644 index 00000000000..9516e2235e4 --- /dev/null +++ b/src/Forms/Uploaders/FormFileUpload.php @@ -0,0 +1,76 @@ +config = collect($config); + } + + /** + * Instantiate form file upload. + * + * @param array $config + * @return static + */ + public static function field($config, string $submissionId) + { + return new static($config, $submissionId); + } + + /** + * Upload the files and return their storage paths. + * + * @param mixed $files + * @return array|string + */ + public function upload($files) + { + $paths = $this->getUploadableFiles($files)->map(function ($file) { + return (new FormFileUploader($this->submissionId, $this->config->get('handle'), $this->config->get('container')))->upload($file); + }); + + return $this->isSingleFile() + ? $paths->first() + : $paths->all(); + } + + /** + * Get uploadable files. + * + * @param mixed $files + * @return \Illuminate\Support\Collection + */ + protected function getUploadableFiles($files) + { + $files = collect(Arr::wrap($files))->filter(); + + return $this->isSingleFile() + ? $files->take(1) + : $files; + } + + /** + * Determine if uploader should only upload a single file. + * + * @return bool + */ + protected function isSingleFile() + { + return $this->config->get('max_files') === 1; + } +} diff --git a/src/Forms/Uploaders/FormFileUploader.php b/src/Forms/Uploaders/FormFileUploader.php new file mode 100644 index 00000000000..223cef0cefd --- /dev/null +++ b/src/Forms/Uploaders/FormFileUploader.php @@ -0,0 +1,27 @@ +submissionId}/{$this->handle}/{$file->getClientOriginalName()}"; + } + + protected function uploadPathPrefix() + { + return 'statamic/form-uploads/'; + } +} diff --git a/src/Jobs/DeletePartialFormSubmissions.php b/src/Jobs/DeletePartialFormSubmissions.php index eafffbb0073..fefbea040a1 100644 --- a/src/Jobs/DeletePartialFormSubmissions.php +++ b/src/Jobs/DeletePartialFormSubmissions.php @@ -7,8 +7,8 @@ use Illuminate\Foundation\Bus\Dispatchable; use Illuminate\Queue\InteractsWithQueue; use Statamic\Contracts\Forms\Submission; -use Statamic\Facades\Asset; use Statamic\Facades\FormSubmission; +use Statamic\Forms\DeleteTemporaryFiles; class DeletePartialFormSubmissions implements ShouldQueue { @@ -27,25 +27,9 @@ public function handle(): void ->where('date', '<', $threshold) ->get() ->each(function (Submission $submission): void { - if (config('statamic.forms.garbage_collect_assets')) { - $this->garbageCollectAssets($submission); - } + DeleteTemporaryFiles::dispatchSync($submission); $submission->delete(); }); } - - private function garbageCollectAssets(Submission $submission): void - { - $submission->form()->blueprint()->fields()->all() - ->filter(fn ($field) => $field->fieldtype()->handle() === 'assets' - || ($field->fieldtype()->handle() === 'form_upload' && $field->fieldtype()->config('store'))) - ->each(function ($field) use ($submission) { - $container = $field->get('container'); - - collect($submission->get($field->handle())) - ->filter() - ->each(fn ($path) => Asset::find("{$container}::{$path}")?->delete()); - }); - } } diff --git a/tests/Forms/CreateAssetsFromFileUploadsTest.php b/tests/Forms/CreateAssetsFromFileUploadsTest.php new file mode 100644 index 00000000000..578408516fb --- /dev/null +++ b/tests/Forms/CreateAssetsFromFileUploadsTest.php @@ -0,0 +1,181 @@ +fakeAvatarsContainer(); + + $form = tap(Form::make('contact')->formFields([ + 'sections' => [ + ['fields' => [ + ['handle' => 'avatar', 'field' => ['type' => 'upload', 'store' => true, 'container' => 'avatars', 'max_files' => 1]], + ]], + ], + ]))->save(); + + $submission = tap($form->makeSubmission())->save(); + + $path = FormFileUpload::field(['handle' => 'avatar', 'max_files' => 1], $submission->id()) + ->upload([UploadedFile::fake()->image('avatar.jpg')]); + + $submission->set('avatar', $path)->save(); + + (new CreateAssetsFromFileUploads($submission))->handle(); + + Storage::disk('local')->assertMissing('statamic/form-uploads/'.$path); + Storage::disk('avatars')->assertExists('avatar.jpg'); + + $newValue = $form->submission($submission->id())->get('avatar'); + $this->assertIsString($newValue); + $this->assertNotEquals($path, $newValue); + $this->assertNotNull(Asset::find("avatars::{$newValue}")); + } + + #[Test] + public function it_creates_assets_from_multiple_temporary_files() + { + Storage::fake('local'); + $this->fakeAvatarsContainer(); + + $form = tap(Form::make('contact')->formFields([ + 'sections' => [ + ['fields' => [ + ['handle' => 'photos', 'field' => ['type' => 'upload', 'store' => true, 'container' => 'avatars', 'max_files' => 3]], + ]], + ], + ]))->save(); + + $submission = tap($form->makeSubmission())->save(); + + $paths = FormFileUpload::field(['handle' => 'photos', 'max_files' => 3], $submission->id()) + ->upload([UploadedFile::fake()->image('one.jpg'), UploadedFile::fake()->image('two.jpg')]); + + $submission->set('photos', $paths)->save(); + + (new CreateAssetsFromFileUploads($submission))->handle(); + + Storage::disk('avatars')->assertExists('one.jpg'); + Storage::disk('avatars')->assertExists('two.jpg'); + + $newValue = $form->submission($submission->id())->get('photos'); + $this->assertCount(2, $newValue); + $this->assertNotNull(Asset::find("avatars::{$newValue[0]}")); + $this->assertNotNull(Asset::find("avatars::{$newValue[1]}")); + } + + #[Test] + public function it_only_creates_assets_when_storage_is_enabled_on_upload_field() + { + Storage::fake('local'); + $this->fakeAvatarsContainer(); + + $form = tap(Form::make('contact')->formFields([ + 'sections' => [ + ['fields' => [ + ['handle' => 'avatar', 'field' => ['type' => 'upload', 'store' => true, 'container' => 'avatars', 'max_files' => 1]], + ['handle' => 'document', 'field' => ['type' => 'upload', 'store' => false, 'max_files' => 1]], + ]], + ], + ]))->save(); + + $submission = tap($form->makeSubmission())->save(); + + $avatarPath = FormFileUpload::field(['handle' => 'avatar', 'max_files' => 1], $submission->id()) + ->upload([UploadedFile::fake()->image('avatar.jpg')]); + $documentPath = FormFileUpload::field(['handle' => 'document', 'max_files' => 1], $submission->id()) + ->upload([UploadedFile::fake()->create('resume.pdf', 10)]); + + $submission->set('avatar', $avatarPath)->set('document', $documentPath)->save(); + + (new CreateAssetsFromFileUploads($submission))->handle(); + + // `avatar` (store: true) is promoted to a real asset... + Storage::disk('local')->assertMissing('statamic/form-uploads/'.$avatarPath); + Storage::disk('avatars')->assertExists('avatar.jpg'); + $newAvatarValue = $form->submission($submission->id())->get('avatar'); + $this->assertNotEquals($avatarPath, $newAvatarValue); + $this->assertNotNull(Asset::find("avatars::{$newAvatarValue}")); + + // ...but `document` (store: false) is left exactly as it was. + Storage::disk('local')->assertExists('statamic/form-uploads/'.$documentPath); + $this->assertEquals($documentPath, $form->submission($submission->id())->get('document')); + } + + #[Test] + public function it_skips_creating_an_asset_when_the_temporary_file_doesnt_exist() + { + Storage::fake('local'); + $this->fakeAvatarsContainer(); + + $form = tap(Form::make('contact')->formFields([ + 'sections' => [ + ['fields' => [ + ['handle' => 'avatar', 'field' => ['type' => 'upload', 'store' => true, 'container' => 'avatars', 'max_files' => 1]], + ]], + ], + ]))->save(); + + $submission = tap($form->makeSubmission()->set('avatar', 'never-existed.jpg'))->save(); + + (new CreateAssetsFromFileUploads($submission))->handle(); + + $this->assertEquals('never-existed.jpg', $form->submission($submission->id())->get('avatar')); + $this->assertEmpty(AssetContainer::find('avatars')->assets()); + } + + #[Test] + public function it_still_creates_the_asset_when_the_form_does_not_store_submissions() + { + Storage::fake('local'); + $this->fakeAvatarsContainer(); + + $form = tap(Form::make('contact')->store(false)->formFields([ + 'sections' => [ + ['fields' => [ + ['handle' => 'avatar', 'field' => ['type' => 'upload', 'store' => true, 'container' => 'avatars', 'max_files' => 1]], + ]], + ], + ]))->save(); + + $submission = $form->makeSubmission(); + + $path = FormFileUpload::field(['handle' => 'avatar', 'max_files' => 1], $submission->id()) + ->upload([UploadedFile::fake()->image('avatar.jpg')]); + + $submission->set('avatar', $path); + + (new CreateAssetsFromFileUploads($submission))->handle(); + + // The asset is still created and the temporary copy still cleaned up... + Storage::disk('avatars')->assertExists('avatar.jpg'); + Storage::disk('local')->assertMissing('statamic/form-uploads/'.$path); + + // ...but since this form never persists submissions, there's nothing to save it to. + $this->assertNull($form->submission($submission->id())); + $this->assertNotEquals($path, $submission->get('avatar')); + } + + private function fakeAvatarsContainer(): void + { + Storage::fake('avatars'); + AssetContainer::make('avatars')->disk('avatars')->save(); + } +} diff --git a/tests/Forms/DeletePartialFormSubmissionsTest.php b/tests/Forms/DeletePartialFormSubmissionsTest.php index bccddacc7d2..65c58a7cecc 100644 --- a/tests/Forms/DeletePartialFormSubmissionsTest.php +++ b/tests/Forms/DeletePartialFormSubmissionsTest.php @@ -3,11 +3,10 @@ namespace Tests\Forms; use Carbon\Carbon; -use Illuminate\Http\UploadedFile; -use Illuminate\Support\Facades\Storage; +use Illuminate\Support\Facades\Bus; use PHPUnit\Framework\Attributes\Test; -use Statamic\Facades\AssetContainer; use Statamic\Facades\Form; +use Statamic\Forms\DeleteTemporaryFiles; use Statamic\Jobs\DeletePartialFormSubmissions; use Tests\PreventSavingStacheItemsToDisk; use Tests\TestCase; @@ -87,66 +86,21 @@ public function it_does_not_delete_anything_when_disabled() } #[Test] - public function it_deletes_attached_assets_when_garbage_collection_is_enabled() + public function it_dispatches_delete_temporary_files_for_each_abandoned_partial_submission() { - config([ - 'statamic.forms.delete_partial_submissions_after' => 7, - 'statamic.forms.garbage_collect_assets' => true, - ]); + Bus::fake(); - $form = $this->formWithUploadField(); - $partial = $this->partialWithAsset($form); - - Storage::disk('avatars')->assertExists('avatar.jpg'); - - (new DeletePartialFormSubmissions)->handle(); - - $this->assertNull($form->submission($partial->id())); - Storage::disk('avatars')->assertMissing('avatar.jpg'); - } - - #[Test] - public function it_leaves_attached_assets_when_garbage_collection_is_disabled() - { - config([ - 'statamic.forms.delete_partial_submissions_after' => 7, - 'statamic.forms.garbage_collect_assets' => false, - ]); - - $form = $this->formWithUploadField(); - $partial = $this->partialWithAsset($form); - - (new DeletePartialFormSubmissions)->handle(); - - // The partial is still deleted, but its asset is left untouched. - $this->assertNull($form->submission($partial->id())); - Storage::disk('avatars')->assertExists('avatar.jpg'); - } - - private function formWithUploadField() - { - Storage::fake('avatars'); - tap(AssetContainer::make('avatars')->disk('avatars'))->save(); - - return tap(Form::make('contact')->formFields([ - 'sections' => [ - ['fields' => [ - ['handle' => 'avatar', 'field' => ['type' => 'upload', 'store' => true, 'container' => 'avatars']], - ]], - ], - ]))->save(); - } + config(['statamic.forms.delete_partial_submissions_after' => 7]); - private function partialWithAsset($form) - { - $asset = tap(AssetContainer::find('avatars')->makeAsset('avatar.jpg')) - ->upload(UploadedFile::fake()->image('avatar.jpg')); + $form = tap(Form::make('contact'))->save(); Carbon::setTestNow('2025-06-01 12:00:00'); - $partial = tap($form->makeSubmission()->set('partial', true)->set('avatar', [$asset->path()]))->save(); + tap($form->makeSubmission()->set('partial', true))->save(); Carbon::setTestNow('2025-06-30 12:00:00'); - return $partial; + (new DeletePartialFormSubmissions)->handle(); + + Bus::assertDispatchedSync(DeleteTemporaryFiles::class); } } diff --git a/tests/Forms/DeleteTemporaryFilesTest.php b/tests/Forms/DeleteTemporaryFilesTest.php new file mode 100644 index 00000000000..230fbb92078 --- /dev/null +++ b/tests/Forms/DeleteTemporaryFilesTest.php @@ -0,0 +1,149 @@ +formFields([ + 'sections' => [ + ['fields' => [ + ['handle' => 'avatar', 'field' => ['type' => 'upload', 'store' => true, 'max_files' => 1]], + ['handle' => 'document', 'field' => ['type' => 'upload', 'store' => false, 'max_files' => 1]], + ]], + ], + ]))->save(); + + $submission = tap($form->makeSubmission())->save(); + + $avatarPath = FormFileUpload::field(['handle' => 'avatar', 'max_files' => 1], $submission->id()) + ->upload([UploadedFile::fake()->image('avatar.jpg')]); + $documentPath = FormFileUpload::field(['handle' => 'document', 'max_files' => 1], $submission->id()) + ->upload([UploadedFile::fake()->create('resume.pdf', 10)]); + + $submission->set('avatar', $avatarPath)->set('document', $documentPath)->save(); + + (new DeleteTemporaryFiles($submission))->handle(); + + Storage::disk('local')->assertMissing('statamic/form-uploads/'.$avatarPath); + Storage::disk('local')->assertMissing('statamic/form-uploads/'.$documentPath); + Storage::disk('local')->assertDirectoryEmpty('statamic/form-uploads/'.$submission->id()); + } + + #[Test] + public function it_removes_store_false_field_values_but_keeps_store_true_ones() + { + Storage::fake('local'); + + $form = tap(Form::make('contact')->formFields([ + 'sections' => [ + ['fields' => [ + ['handle' => 'avatar', 'field' => ['type' => 'upload', 'store' => true, 'max_files' => 1]], + ['handle' => 'document', 'field' => ['type' => 'upload', 'store' => false, 'max_files' => 1]], + ]], + ], + ]))->save(); + + $submission = tap($form->makeSubmission())->save(); + + $documentPath = FormFileUpload::field(['handle' => 'document', 'max_files' => 1], $submission->id()) + ->upload([UploadedFile::fake()->create('resume.pdf', 10)]); + + // Simulate the normal finalize order: `avatar` already promoted to a real asset path by + // `CreateAssetsFromFileUploads` before this job runs; `document` is still temporary. + $submission->set('avatar', 'uploads/avatar.jpg')->set('document', $documentPath)->save(); + + (new DeleteTemporaryFiles($submission))->handle(); + + $this->assertEquals('uploads/avatar.jpg', $form->submission($submission->id())->get('avatar')); + $this->assertNull($form->submission($submission->id())->get('document')); + } + + #[Test] + public function it_does_not_save_when_the_form_does_not_store_submissions() + { + Storage::fake('local'); + + $form = tap(Form::make('contact')->store(false)->formFields([ + 'sections' => [ + ['fields' => [ + ['handle' => 'document', 'field' => ['type' => 'upload', 'store' => false, 'max_files' => 1]], + ]], + ], + ]))->save(); + + $submission = $form->makeSubmission(); + + $documentPath = FormFileUpload::field(['handle' => 'document', 'max_files' => 1], $submission->id()) + ->upload([UploadedFile::fake()->create('resume.pdf', 10)]); + + $submission->set('document', $documentPath); + + (new DeleteTemporaryFiles($submission))->handle(); + + $this->assertNull($form->submission($submission->id())); + } + + #[Test] + public function it_deletes_temp_files_created_by_the_files_fieldtype() + { + Storage::fake('local'); + + $form = tap(Form::make('contact')->formFields([ + 'sections' => [ + ['fields' => [ + ['handle' => 'document', 'field' => ['type' => 'files']], + ]], + ], + ]))->save(); + + $submission = tap($form->makeSubmission())->save(); + + $path = now()->timestamp.'/resume.pdf'; + Storage::disk('local')->put('statamic/file-uploads/'.$path, ''); + $submission->set('document', [$path])->save(); + + (new DeleteTemporaryFiles($submission))->handle(); + + Storage::disk('local')->assertMissing('statamic/file-uploads/'.$path); + $this->assertNull($form->submission($submission->id())->get('document')); + } + + #[Test] + public function it_does_not_delete_asset_files() + { + Storage::fake('local'); + Storage::fake('avatars'); + AssetContainer::make('avatars')->disk('avatars')->save(); + + $form = tap(Form::make('contact')->formFields([ + 'sections' => [ + ['fields' => [ + ['handle' => 'avatar', 'field' => ['type' => 'assets', 'container' => 'avatars']], + ]], + ], + ]))->save(); + + $submission = tap($form->makeSubmission()->set('avatar', ['avatar.jpg']))->save(); + + (new DeleteTemporaryFiles($submission))->handle(); + + $this->assertEquals(['avatar.jpg'], $form->submission($submission->id())->get('avatar')); + } +} diff --git a/tests/Forms/SendEmailsTest.php b/tests/Forms/SendEmailsTest.php index f144b5afd1d..1c239dbad36 100644 --- a/tests/Forms/SendEmailsTest.php +++ b/tests/Forms/SendEmailsTest.php @@ -5,10 +5,9 @@ use Illuminate\Support\Facades\Bus; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; -use Statamic\Contracts\Forms\SubmissionRepository; use Statamic\Facades\Form as FacadesForm; use Statamic\Facades\Site; -use Statamic\Forms\DeleteTemporaryAttachments; +use Statamic\Forms\DeleteTemporaryFiles; use Statamic\Forms\SendEmail; use Statamic\Forms\SendEmails; use Tests\PreventSavingStacheItemsToDisk; @@ -89,7 +88,7 @@ public function it_queues_email_jobs_when_config_contains_single_email() } #[Test] - public function it_dispatches_delete_attachments_job_after_dispatching_email_jobs() + public function it_dispatches_delete_temporary_files_job_after_dispatching_email_jobs() { Bus::fake(); @@ -99,7 +98,7 @@ public function it_dispatches_delete_attachments_job_after_dispatching_email_job 'foo' => 'bar', ])->formFields([ 'fields' => [ - ['handle' => 'attachments', 'field' => ['type' => 'files']], + ['handle' => 'document', 'field' => ['type' => 'form_upload', 'store' => false]], ], ]))->save(); @@ -114,31 +113,27 @@ public function it_dispatches_delete_attachments_job_after_dispatching_email_job 'to' => 'first@recipient.com', 'foo' => 'bar', ]), - new DeleteTemporaryAttachments($submission), + new DeleteTemporaryFiles($submission), ]); } #[Test] - public function delete_attachments_job_only_saves_submission_when_enabled() + public function it_dispatches_delete_temporary_files_job_even_without_any_emails_configured() { - $form = tap(FacadesForm::make('attachments_test')->email([ - 'from' => 'first@sender.com', - 'to' => 'first@recipient.com', - 'foo' => 'bar', - ]))->save(); - - $form - ->store(false) - ->blueprint() - ->ensureField('attachments', ['type' => 'files'])->save(); - - $submission = $form->makeSubmission(); + Bus::fake(); - (new DeleteTemporaryAttachments($submission))->handle(); + $form = tap(FacadesForm::make('attachments_test')->formFields([ + 'fields' => [ + ['handle' => 'document', 'field' => ['type' => 'form_upload', 'store' => false]], + ], + ]))->save(); - $submissions = app(SubmissionRepository::class)->all(); + (new SendEmails( + $form->makeSubmission(), + Site::default(), + ))->handle(); - $this->assertEmpty($submissions); + Bus::assertDispatched(DeleteTemporaryFiles::class); } #[Test] diff --git a/tests/Forms/SubmissionTest.php b/tests/Forms/SubmissionTest.php index c2229205e4f..b6a2200b7a2 100644 --- a/tests/Forms/SubmissionTest.php +++ b/tests/Forms/SubmissionTest.php @@ -16,6 +16,7 @@ use Statamic\Events\SubmissionSaving; use Statamic\Facades\Form; use Statamic\Facades\Site; +use Statamic\Forms\CreateAssetsFromFileUploads; use Statamic\Forms\SendEmails; use Tests\PreventSavingStacheItemsToDisk; use Tests\TestCase; @@ -381,11 +382,26 @@ public function finalizing_a_new_submission_dispatches_created_and_finalized_eve Event::assertDispatched(SubmissionCreated::class, 1); Event::assertDispatched(SubmissionFinalized::class, 1); + Bus::assertDispatched(CreateAssetsFromFileUploads::class, 1); Bus::assertDispatched(SendEmails::class, 1); $this->assertNotNull($form->submission($submission->id())); } + #[Test] + public function finalizing_dispatches_asset_creation_synchronously_then_sends_emails() + { + Bus::fake(); + + $form = tap(Form::make('contact_us'))->save(); + $submission = $form->makeSubmission()->asPartial(); + + $submission->finalize(); + + Bus::assertDispatchedSync(CreateAssetsFromFileUploads::class); + Bus::assertDispatched(SendEmails::class); + } + #[Test] public function finalizing_a_partial_submission_removes_the_status_key_and_dispatches_events() { @@ -403,6 +419,7 @@ public function finalizing_a_partial_submission_removes_the_status_key_and_dispa // existing submission won't dispatch it again, but it does finalize and email. Event::assertDispatched(SubmissionCreated::class, 1); Event::assertDispatched(SubmissionFinalized::class, 1); + Bus::assertDispatched(CreateAssetsFromFileUploads::class, 1); Bus::assertDispatched(SendEmails::class, 1); } @@ -419,6 +436,7 @@ public function finalizing_a_submission_for_a_non_storing_form_still_dispatches_ Event::assertDispatched(SubmissionCreated::class, 1); Event::assertDispatched(SubmissionFinalized::class, 1); + Bus::assertDispatched(CreateAssetsFromFileUploads::class, 1); Bus::assertDispatched(SendEmails::class, 1); $this->assertNull($form->submission($submission->id())); } @@ -440,6 +458,7 @@ public function finalizing_a_submission_for_a_non_storing_form_deletes_it() Event::assertDispatched(SubmissionCreated::class, 1); Event::assertDispatched(SubmissionFinalized::class, 1); + Bus::assertDispatched(CreateAssetsFromFileUploads::class, 1); Bus::assertDispatched(SendEmails::class, 1); Event::assertNotDispatched(SubmissionDeleted::class); } @@ -458,6 +477,7 @@ public function finalizing_is_idempotent() // The second call is a no-op because the submission is no longer partial. Event::assertDispatched(SubmissionFinalized::class, 1); + Bus::assertDispatched(CreateAssetsFromFileUploads::class, 1); Bus::assertDispatched(SendEmails::class, 1); } diff --git a/tests/Forms/SubmitFormTest.php b/tests/Forms/SubmitFormTest.php index 0e3da29b6e8..1390af7d843 100644 --- a/tests/Forms/SubmitFormTest.php +++ b/tests/Forms/SubmitFormTest.php @@ -17,6 +17,7 @@ use Statamic\Facades\AssetContainer; use Statamic\Facades\Fieldset; use Statamic\Facades\Form; +use Statamic\Forms\CreateAssetsFromFileUploads; use Statamic\Forms\SendEmails; use Statamic\Forms\SubmissionResult; use Statamic\Forms\SubmitForm; @@ -165,6 +166,7 @@ public function it_finalizes_without_storing_when_store_is_disabled() $this->assertEmpty($this->form->submissions()); Event::assertDispatched(SubmissionCreated::class); Event::assertDispatched(SubmissionFinalized::class); + Bus::assertDispatched(CreateAssetsFromFileUploads::class); Bus::assertDispatched(SendEmails::class); } @@ -307,6 +309,60 @@ public function it_uploads_files() $form->submissions()->each->delete(); } + #[Test] + public function it_uploads_files_via_assets_fieldtype() + { + Storage::fake('avatars'); + AssetContainer::make('avatars')->disk('avatars')->save(); + + $form = tap(Form::make('avatar.png')->formFields([ + 'fields' => [ + ['handle' => 'email', 'field' => ['type' => 'email']], + ['handle' => 'avatar', 'field' => ['type' => 'assets', 'container' => 'avatars', 'max_files' => 1]], + ], + ]))->save(); + + $result = app(SubmitForm::class) + ->form($form) + ->submit( + data: ['email' => 'test@example.com'], + files: ['avatar' => [UploadedFile::fake()->image('avatar.jpg')]], + ); + + Storage::disk('avatars')->assertExists('avatar.jpg'); + $this->assertEquals('avatar.jpg', $result->submission->get('avatar')); + + $form->submissions()->each->delete(); + } + + #[Test] + public function it_uploads_files_via_files_fieldtype() + { + Storage::fake('local'); + Bus::fake(); // Otherwise finalize's queued cleanup job removes it before we can look. + + $form = tap(Form::make('avatar.png')->formFields([ + 'fields' => [ + ['handle' => 'email', 'field' => ['type' => 'email']], + ['handle' => 'avatar', 'field' => ['type' => 'files', 'max_files' => 1]], + ], + ]))->save(); + + $result = app(SubmitForm::class) + ->form($form) + ->submit( + data: ['email' => 'test@example.com'], + files: ['avatar' => [UploadedFile::fake()->create('resume.pdf', 10)]], + ); + + $path = $result->submission->get('avatar'); + + $this->assertNotNull($path); + Storage::disk('local')->assertExists('statamic/file-uploads/'.$path); + + $form->submissions()->each->delete(); + } + #[Test] public function it_does_not_revalidate_an_already_uploaded_file_when_resubmitting_its_page() { @@ -339,7 +395,7 @@ public function it_does_not_revalidate_an_already_uploaded_file_when_resubmittin #[Test] public function it_validates_the_extension_of_uploaded_files() { - Bus::fake(); // Otherwise the temp file is deleted by DeleteTemporaryAttachments right after submission. + Bus::fake(); // Otherwise the temp file is deleted by DeleteTemporaryFiles right after submission. Storage::fake('local'); // store: false makes this a temporary "files" upload rather than a stored asset. @@ -373,7 +429,7 @@ public function it_validates_the_extension_of_uploaded_files() ->submit(data: [], files: ['document' => [UploadedFile::fake()->create('resume.pdf', 10)]]); $this->assertTrue($result->isFinalized()); - Storage::disk('local')->assertExists('statamic/file-uploads/'.$result->submission->get('document')[0]); + Storage::disk('local')->assertExists('statamic/form-uploads/'.$result->submission->get('document')[0]); $form->submissions()->each->delete(); } @@ -477,6 +533,7 @@ public function it_returns_the_next_page_and_saves_a_partial_submission_when_sub Event::assertDispatched(SubmissionCreated::class); Event::assertNotDispatched(FormSubmitted::class); Event::assertNotDispatched(SubmissionFinalized::class); + Bus::assertNotDispatched(CreateAssetsFromFileUploads::class); Bus::assertNotDispatched(SendEmails::class); $form->submissions()->each->delete(); @@ -689,6 +746,7 @@ public function it_resumes_a_partial_submission_on_a_later_page() $this->assertTrue($result->submission->isPartial()); Event::assertNotDispatched(FormSubmitted::class); Event::assertNotDispatched(SubmissionFinalized::class); + Bus::assertNotDispatched(CreateAssetsFromFileUploads::class); Bus::assertNotDispatched(SendEmails::class); // Earlier-page values are preserved while the new page's values are merged in. @@ -735,6 +793,7 @@ public function it_finalizes_the_partial_submission_on_the_final_page() // Finalizing fires the completion events once; it doesn't re-create the submission. Event::assertNotDispatched(SubmissionCreated::class); Event::assertDispatched(SubmissionFinalized::class, 1); + Bus::assertDispatched(CreateAssetsFromFileUploads::class, 1); Bus::assertDispatched(SendEmails::class, 1); $form->submissions()->each->delete(); @@ -761,6 +820,7 @@ public function it_returns_to_the_first_page_when_finalizing_without_completing_ Event::assertNotDispatched(FormSubmitted::class); Event::assertNotDispatched(SubmissionFinalized::class); + Bus::assertNotDispatched(CreateAssetsFromFileUploads::class); Bus::assertNotDispatched(SendEmails::class); $form->submissions()->each->delete(); diff --git a/tests/Forms/Uploaders/FormFileUploadTest.php b/tests/Forms/Uploaders/FormFileUploadTest.php new file mode 100644 index 00000000000..2a521629a7b --- /dev/null +++ b/tests/Forms/Uploaders/FormFileUploadTest.php @@ -0,0 +1,74 @@ + 'document', 'max_files' => 1], 'submission-123') + ->upload([UploadedFile::fake()->create('resume.pdf', 10)]); + + $this->assertEquals('submission-123/document/resume.pdf', $path); + Storage::disk('local')->assertExists('statamic/form-uploads/submission-123/document/resume.pdf'); + } + + #[Test] + public function it_writes_multiple_files_and_returns_an_array_of_paths() + { + Storage::fake('local'); + + $paths = FormFileUpload::field(['handle' => 'documents', 'max_files' => 3], 'submission-123') + ->upload([ + UploadedFile::fake()->create('one.pdf', 10), + UploadedFile::fake()->create('two.pdf', 10), + ]); + + $this->assertEquals([ + 'submission-123/documents/one.pdf', + 'submission-123/documents/two.pdf', + ], $paths); + + Storage::disk('local')->assertExists('statamic/form-uploads/submission-123/documents/one.pdf'); + Storage::disk('local')->assertExists('statamic/form-uploads/submission-123/documents/two.pdf'); + } + + #[Test] + public function it_only_takes_the_first_file_when_the_field_is_single_file() + { + Storage::fake('local'); + + $path = FormFileUpload::field(['handle' => 'document', 'max_files' => 1], 'submission-123') + ->upload([ + UploadedFile::fake()->create('one.pdf', 10), + UploadedFile::fake()->create('two.pdf', 10), + ]); + + $this->assertEquals('submission-123/document/one.pdf', $path); + Storage::disk('local')->assertMissing('statamic/form-uploads/submission-123/document/two.pdf'); + } + + #[Test] + public function different_fields_on_the_same_submission_get_their_own_folder() + { + Storage::fake('local'); + + FormFileUpload::field(['handle' => 'avatar', 'max_files' => 1], 'submission-123') + ->upload([UploadedFile::fake()->image('avatar.jpg')]); + + FormFileUpload::field(['handle' => 'document', 'max_files' => 1], 'submission-123') + ->upload([UploadedFile::fake()->create('resume.pdf', 10)]); + + Storage::disk('local')->assertExists('statamic/form-uploads/submission-123/avatar/avatar.jpg'); + Storage::disk('local')->assertExists('statamic/form-uploads/submission-123/document/resume.pdf'); + } +} From 10441f3882e3e4e440f0d753f428bcc547d51be3 Mon Sep 17 00:00:00 2001 From: Duncan McClean Date: Wed, 15 Jul 2026 13:08:45 +0100 Subject: [PATCH 04/17] make phpstan happy --- src/Forms/Uploaders/FormFileUpload.php | 2 +- src/Forms/Uploaders/FormFileUploader.php | 15 ++++++++++----- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/Forms/Uploaders/FormFileUpload.php b/src/Forms/Uploaders/FormFileUpload.php index 9516e2235e4..94f5788c582 100644 --- a/src/Forms/Uploaders/FormFileUpload.php +++ b/src/Forms/Uploaders/FormFileUpload.php @@ -41,7 +41,7 @@ public static function field($config, string $submissionId) public function upload($files) { $paths = $this->getUploadableFiles($files)->map(function ($file) { - return (new FormFileUploader($this->submissionId, $this->config->get('handle'), $this->config->get('container')))->upload($file); + return FormFileUploader::submission($this->submissionId, $this->config->get('handle'), $this->config->get('container'))->upload($file); }); return $this->isSingleFile() diff --git a/src/Forms/Uploaders/FormFileUploader.php b/src/Forms/Uploaders/FormFileUploader.php index 223cef0cefd..9b672684714 100644 --- a/src/Forms/Uploaders/FormFileUploader.php +++ b/src/Forms/Uploaders/FormFileUploader.php @@ -5,14 +5,19 @@ use Statamic\Assets\FileUploader; use Symfony\Component\HttpFoundation\File\UploadedFile; -/** - * @phpstan-consistent-constructor - */ class FormFileUploader extends FileUploader { - public function __construct(protected string $submissionId, protected string $handle, ?string $container = null) + protected string $submissionId; + + protected string $handle; + + public static function submission(string $submissionId, string $handle, ?string $container = null): static { - parent::__construct($container); + $uploader = new static($container); + $uploader->submissionId = $submissionId; + $uploader->handle = $handle; + + return $uploader; } protected function uploadPath(UploadedFile $file) From ee0f6e394f15352ec96175d03c0093f611e9b165 Mon Sep 17 00:00:00 2001 From: Duncan McClean Date: Wed, 15 Jul 2026 15:40:01 +0100 Subject: [PATCH 05/17] Avoid validation errors when submitting a page with existing file uploads... 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. --- src/Forms/SubmitForm.php | 23 ++++++++++ tests/Forms/SubmitFormTest.php | 81 +++++++++++++++++++++++++++++++--- 2 files changed, 98 insertions(+), 6 deletions(-) diff --git a/src/Forms/SubmitForm.php b/src/Forms/SubmitForm.php index 2b2f2b2f4bd..acfbcd6022d 100644 --- a/src/Forms/SubmitForm.php +++ b/src/Forms/SubmitForm.php @@ -201,6 +201,8 @@ public function validate(array $data, array $files = [], ?array $only = null): v ->withRules($this->extraRules($fields, $files)) ->validator(); + $validator->setRules($this->withoutRulesForAlreadyUploadedFiles($validator->getRulesWithoutPlaceholders(), $fields, $files)); + if (! $only && $this->page) { $only = $this->fieldHandles($this->page); } @@ -246,6 +248,27 @@ private function assetContainerRules($field): array ->all(); } + private function withoutRulesForAlreadyUploadedFiles(array $rules, $fields, array $files): array + { + return $fields->all() + ->filter(fn ($field): bool => in_array($field->fieldtype()->handle(), ['assets', 'files', 'form_upload'])) + ->filter(fn ($field): bool => ! array_key_exists($field->handle(), $files)) + ->reduce(function ($rules, $field) { + $handle = $field->handle(); + + if (! isset($rules[$handle])) { + return $rules; + } + + $rules[$handle] = collect($rules[$handle]) + ->reject(fn ($rule) => $rule === 'array' || str_starts_with($rule, 'max:') || str_starts_with($rule, 'min:')) + ->values() + ->all(); + + return $rules; + }, $rules); + } + private function filterRules(array $rules, array $only): array { return collect($rules) diff --git a/tests/Forms/SubmitFormTest.php b/tests/Forms/SubmitFormTest.php index 1390af7d843..019f6b8b9e0 100644 --- a/tests/Forms/SubmitFormTest.php +++ b/tests/Forms/SubmitFormTest.php @@ -364,34 +364,103 @@ public function it_uploads_files_via_files_fieldtype() } #[Test] - public function it_does_not_revalidate_an_already_uploaded_file_when_resubmitting_its_page() + public function it_does_not_revalidate_already_uploaded_files_when_resubmitting_their_page() { Storage::fake('avatars'); AssetContainer::make('avatars')->disk('avatars')->save(); - $form = $this->uploadForm(); + $form = tap(Form::make('uploads')->formFields([ + 'pages' => [ + [ + 'id' => 'main', + 'sections' => [ + ['fields' => [ + ['handle' => 'email', 'field' => ['type' => 'email']], + ['handle' => 'avatar', 'field' => ['type' => 'upload', 'store' => true, 'container' => 'avatars']], + ['handle' => 'headshot', 'field' => ['type' => 'upload', 'store' => true, 'container' => 'avatars', 'max_files' => 1]], + ]], + ], + ], + ], + ]))->save(); $first = app(SubmitForm::class) ->form($form) ->page('main') ->submit( data: ['email' => 'test@example.com'], - files: ['avatar' => [UploadedFile::fake()->image('avatar.jpg')]], + files: [ + 'avatar' => [UploadedFile::fake()->image('avatar.jpg')], + 'headshot' => [UploadedFile::fake()->image('headshot.jpg')], + ], ); - // Going back to the page and resubmitting resends the already-resolved asset path as a - // plain string rather than a fresh upload. That shouldn't be revalidated as a file. + // Going back to the page and resubmitting resends each field's already-resolved value: + // an array for `avatar` (no max_files), a plain string for `headshot` (max_files: 1). + // Neither should be revalidated as though it were a fresh upload. $result = app(SubmitForm::class) ->form($form) ->page('main') ->resume($first->submission) - ->submit(data: ['email' => 'test@example.com', 'avatar' => $first->submission->get('avatar')]); + ->submit(data: [ + 'email' => 'test@example.com', + 'avatar' => $first->submission->get('avatar'), + 'headshot' => $first->submission->get('headshot'), + ]); $this->assertTrue($result->isFinalized()); $form->submissions()->each->delete(); } + #[Test] + public function it_still_validates_a_freshly_uploaded_file_when_resubmitting_its_page() + { + Storage::fake('avatars'); + AssetContainer::make('avatars')->disk('avatars')->save(); + + $form = tap(Form::make('uploads')->formFields([ + 'pages' => [ + [ + 'id' => 'main', + 'sections' => [ + ['fields' => [ + ['handle' => 'email', 'field' => ['type' => 'email']], + ['handle' => 'avatar', 'field' => ['type' => 'upload', 'store' => true, 'container' => 'avatars', 'max_files' => 1]], + ]], + ], + ], + ], + ]))->save(); + + $first = app(SubmitForm::class) + ->form($form) + ->page('main') + ->submit( + data: ['email' => 'test@example.com'], + files: ['avatar' => [UploadedFile::fake()->image('avatar.jpg')]], + ); + + // Resubmitting with a genuinely new file should still be validated as a fresh upload, + // not skipped the way an already-uploaded field's carried-over value is. + try { + app(SubmitForm::class) + ->form($form) + ->page('main') + ->resume($first->submission) + ->submit( + data: ['email' => 'test@example.com'], + files: ['avatar' => [UploadedFile::fake()->create('virus.php', 10)]], + ); + + $this->fail('Expected ValidationException was not thrown'); + } catch (ValidationException $e) { + $this->assertArrayHasKey('avatar.0', $e->errors()); + } + + $form->submissions()->each->delete(); + } + #[Test] public function it_validates_the_extension_of_uploaded_files() { From 729f78595d3d48b996a9c1f5b3596042b3c70172 Mon Sep 17 00:00:00 2001 From: Duncan McClean Date: Wed, 15 Jul 2026 16:09:03 +0100 Subject: [PATCH 06/17] always display basename in fieldtype. not the full path --- src/Fieldtypes/FormUpload.php | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/Fieldtypes/FormUpload.php b/src/Fieldtypes/FormUpload.php index e4e54479e82..750622fb183 100644 --- a/src/Fieldtypes/FormUpload.php +++ b/src/Fieldtypes/FormUpload.php @@ -24,16 +24,12 @@ class FormUpload extends Fieldtype public function preload() { - $values = Arr::wrap($this->field->value()); - return [ - 'files' => collect($values)->map(function (string $value): array { - if (! $this->storesAsAsset()) { - return ['filename' => basename($value)]; - } + 'files' => collect(Arr::wrap($this->field->value()))->map(function (string $value): array { + $asset = $this->storesAsAsset() ? Asset::find($value) : null; - if (! $asset = Asset::find($value)) { - return ['filename' => $value]; + if (! $asset) { + return ['filename' => basename($value)]; } return [ From 47a9c7c1a91862913fc449a8f1d8c5cda329f8b4 Mon Sep 17 00:00:00 2001 From: Duncan McClean Date: Wed, 15 Jul 2026 16:24:15 +0100 Subject: [PATCH 07/17] simplify file upload validation logic merges stuff from `extraRules` and the `withoutRulesForAlreadyUploadedFiles` method i added. should make things _slightly_ easier to read and understand. --- src/Forms/SubmitForm.php | 77 +++++++++++++++++++--------------------- 1 file changed, 37 insertions(+), 40 deletions(-) diff --git a/src/Forms/SubmitForm.php b/src/Forms/SubmitForm.php index acfbcd6022d..bcbf1b05958 100644 --- a/src/Forms/SubmitForm.php +++ b/src/Forms/SubmitForm.php @@ -196,12 +196,9 @@ public function validate(array $data, array $files = [], ?array $only = null): v $files = $this->normalizeFiles($files); $fields = $this->form->blueprint()->fields()->addValues(array_merge($data, $files)); - $validator = $fields - ->validator() - ->withRules($this->extraRules($fields, $files)) - ->validator(); + $validator = $fields->validator()->validator(); - $validator->setRules($this->withoutRulesForAlreadyUploadedFiles($validator->getRulesWithoutPlaceholders(), $fields, $files)); + $validator->setRules($this->withFileUploadRules($validator->getRulesWithoutPlaceholders(), $fields, $files)); if (! $only && $this->page) { $only = $this->fieldHandles($this->page); @@ -214,49 +211,36 @@ public function validate(array $data, array $files = [], ?array $only = null): v $this->withLocale($this->site()?->lang(), fn () => $validator->validate()); } - private function extraRules($fields, array $files): array + private function withFileUploadRules(array $rules, $fields, array $files): array { return $fields->all() ->filter(fn ($field): bool => in_array($field->fieldtype()->handle(), ['assets', 'files', 'form_upload'])) - // Only validate as a file when one was actually uploaded in this request. - // Resubmitting a page whose file was uploaded earlier just resends its path, - // which isn't a file and shouldn't be re-validated as though it were one. - ->filter(fn ($field): bool => array_key_exists($field->handle(), $files)) - ->mapWithKeys(function ($field): array { - $isStoredAsAsset = $field->fieldtype()->handle() === 'assets' - || ($field->fieldtype()->handle() === 'form_upload' && $field->fieldtype()->config('store')); - - $rules = $isStoredAsAsset - ? array_merge(['file', new AllowedFile], $this->assetContainerRules($field)) - : ['file', new AllowedFile($field->fieldtype()->config('allowed_extensions'))]; - - return [$field->handle().'.*' => $rules]; - }) - ->all(); - } + ->reduce(function ($rules, $field) use ($files) { + $handle = $field->handle(); - private function assetContainerRules($field): array - { - $configured = $field->fieldtype()->config('container'); + // Freshly uploaded files should be validated as files. + if (array_key_exists($handle, $files)) { + $shouldBeStoredAsAsset = $field->fieldtype()->handle() === 'assets' + || ($field->fieldtype()->handle() === 'form_upload' && $field->fieldtype()->config('store')); - $container = $configured - ? AssetContainer::find($configured) - : (($containers = AssetContainer::all())->count() === 1 ? $containers->first() : null); + $fileRules = $shouldBeStoredAsAsset + ? array_merge(['file', new AllowedFile], $this->assetContainerRules($field)) + : ['file', new AllowedFile($field->fieldtype()->config('allowed_extensions'))]; - return collect($container?->validationRules()) - ->map(fn ($rule) => FieldValidator::parse($rule)) - ->all(); - } + $rules["{$handle}.*"] = collect($rules["{$handle}.*"] ?? []) + ->merge($fileRules) + ->unique() + ->values() + ->all(); - private function withoutRulesForAlreadyUploadedFiles(array $rules, $fields, array $files): array - { - return $fields->all() - ->filter(fn ($field): bool => in_array($field->fieldtype()->handle(), ['assets', 'files', 'form_upload'])) - ->filter(fn ($field): bool => ! array_key_exists($field->handle(), $files)) - ->reduce(function ($rules, $field) { - $handle = $field->handle(); + return $rules; + } - if (! isset($rules[$handle])) { + // A carried-over value that's still an array (eg. removing one of several files without + // uploading a new one) should keep being validated normally. Only a `max_files: 1` field's + // collapsed scalar value needs the array/max/min rules skipped, since it was never a fresh + // upload to begin with. + if (is_array($field->value()) || ! isset($rules[$handle])) { return $rules; } @@ -269,6 +253,19 @@ private function withoutRulesForAlreadyUploadedFiles(array $rules, $fields, arra }, $rules); } + private function assetContainerRules($field): array + { + $configured = $field->fieldtype()->config('container'); + + $container = $configured + ? AssetContainer::find($configured) + : (($containers = AssetContainer::all())->count() === 1 ? $containers->first() : null); + + return collect($container?->validationRules()) + ->map(fn ($rule) => FieldValidator::parse($rule)) + ->all(); + } + private function filterRules(array $rules, array $only): array { return collect($rules) From 5a39ce69114d9e762c81cb49c5e68b0143ee06e9 Mon Sep 17 00:00:00 2001 From: Duncan McClean Date: Wed, 15 Jul 2026 16:24:23 +0100 Subject: [PATCH 08/17] add test to cover min_files validation when a file is removed without a new upload --- tests/Forms/SubmitFormTest.php | 45 ++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/tests/Forms/SubmitFormTest.php b/tests/Forms/SubmitFormTest.php index 019f6b8b9e0..1d4b63ded8d 100644 --- a/tests/Forms/SubmitFormTest.php +++ b/tests/Forms/SubmitFormTest.php @@ -461,6 +461,51 @@ public function it_still_validates_a_freshly_uploaded_file_when_resubmitting_its $form->submissions()->each->delete(); } + #[Test] + public function it_still_validates_min_files_when_removing_files_without_uploading_a_new_one() + { + Storage::fake('avatars'); + AssetContainer::make('avatars')->disk('avatars')->save(); + + $form = tap(Form::make('uploads')->formFields([ + 'pages' => [ + [ + 'id' => 'main', + 'sections' => [ + ['fields' => [ + ['handle' => 'email', 'field' => ['type' => 'email']], + ['handle' => 'gallery', 'field' => ['type' => 'upload', 'store' => true, 'container' => 'avatars', 'min_files' => 2]], + ]], + ], + ], + ], + ]))->save(); + + $first = app(SubmitForm::class) + ->form($form) + ->page('main') + ->submit( + data: ['email' => 'test@example.com'], + files: ['gallery' => [UploadedFile::fake()->image('one.jpg'), UploadedFile::fake()->image('two.jpg')]], + ); + + // Dropping one of the two files, without uploading a replacement, still leaves an + // array-shaped value. It should keep being validated against min_files as normal. + try { + app(SubmitForm::class) + ->form($form) + ->page('main') + ->resume($first->submission) + ->submit(data: ['email' => 'test@example.com', 'gallery' => [$first->submission->get('gallery')[0]]]); + + $this->fail('Expected ValidationException was not thrown'); + } catch (ValidationException $e) { + $this->assertArrayHasKey('gallery', $e->errors()); + } + + $form->submissions()->each->delete(); + } + #[Test] public function it_validates_the_extension_of_uploaded_files() { From 1a1339a303b06b6fdb8aa01ca2b16ba89b2b61be Mon Sep 17 00:00:00 2001 From: Duncan McClean Date: Wed, 15 Jul 2026 17:05:22 +0100 Subject: [PATCH 09/17] ensure disk path is correct when adding attachments --- src/Forms/Email.php | 4 +- tests/Forms/EmailTest.php | 87 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 89 insertions(+), 2 deletions(-) diff --git a/src/Forms/Email.php b/src/Forms/Email.php index 249ec626810..4f77ab5ccf9 100644 --- a/src/Forms/Email.php +++ b/src/Forms/Email.php @@ -154,8 +154,10 @@ private function attachFiles($field) return; } + $prefix = $field['fieldtype'] === 'form_upload' ? 'statamic/form-uploads/' : 'statamic/file-uploads/'; + foreach ($value as $file) { - $this->attachFromStorageDisk('local', 'statamic/file-uploads/'.$file); + $this->attachFromStorageDisk('local', $prefix.$file); } } diff --git a/tests/Forms/EmailTest.php b/tests/Forms/EmailTest.php index a2a693c3e68..725b89ce24e 100644 --- a/tests/Forms/EmailTest.php +++ b/tests/Forms/EmailTest.php @@ -3,15 +3,20 @@ namespace Tests\Forms; use Facades\Statamic\Fields\BlueprintRepository; +use Illuminate\Http\UploadedFile; +use Illuminate\Support\Facades\Storage; use Mockery; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; +use Statamic\Facades\Asset; +use Statamic\Facades\AssetContainer; use Statamic\Facades\Blueprint; use Statamic\Facades\Form; use Statamic\Facades\GlobalSet; use Statamic\Facades\Site; use Statamic\Forms\Email; use Statamic\Forms\Submission; +use Statamic\Forms\Uploaders\FormFileUpload; use Tests\PreventSavingStacheItemsToDisk; use Tests\TestCase; @@ -236,7 +241,87 @@ public function it_escapes_submitted_values_in_the_automagic_email() #[Test] public function attachments_are_added() { - $this->markTestIncomplete(); + Storage::fake('avatars'); + AssetContainer::make('avatars')->disk('avatars')->save(); + + // "store: true" means that the uploaded file is now an asset. + $form = tap(Form::make('test')->formFields([ + 'fields' => [ + ['handle' => 'avatar', 'field' => ['type' => 'upload', 'store' => true, 'container' => 'avatars', 'max_files' => 1]], + ], + ]))->save(); + + tap(Asset::make()->container('avatars')->path('avatar.jpg'))->save(); + + $submission = $form->makeSubmission()->data(['avatar' => 'avatar.jpg']); + + $email = tap(new Email($submission, ['to' => 'test@test.com', 'attachments' => true], Site::default()))->build(); + + $this->assertTrue($email->hasAttachmentFromStorageDisk('avatars', 'avatar.jpg')); + } + + #[Test] + public function it_attaches_temporary_file_upload() + { + Storage::fake('local'); + + // "store: false" means that the uploaded file is temporary. + $form = tap(Form::make('test')->formFields([ + 'fields' => [ + ['handle' => 'document', 'field' => ['type' => 'upload', 'store' => false, 'max_files' => 1]], + ], + ]))->save(); + + $submission = $form->makeSubmission(); + $path = FormFileUpload::field(['handle' => 'document', 'max_files' => 1], $submission->id()) + ->upload([UploadedFile::fake()->create('resume.pdf', 10)]); + $submission->data(['document' => $path]); + + $email = tap(new Email($submission, ['to' => 'test@test.com', 'attachments' => true], Site::default()))->build(); + + $this->assertTrue($email->hasAttachmentFromStorageDisk('local', 'statamic/form-uploads/'.$path)); + } + + #[Test] + public function it_attaches_files_from_assets_field() + { + Storage::fake('avatars'); + AssetContainer::make('avatars')->disk('avatars')->save(); + + $form = tap(Form::make('test')->formFields([ + 'fields' => [ + ['handle' => 'avatar', 'field' => ['type' => 'assets', 'container' => 'avatars', 'max_files' => 1]], + ], + ]))->save(); + + tap(Asset::make()->container('avatars')->path('avatar.jpg'))->save(); + + $submission = $form->makeSubmission()->data(['avatar' => 'avatar.jpg']); + + $email = tap(new Email($submission, ['to' => 'test@test.com', 'attachments' => true], Site::default()))->build(); + + $this->assertTrue($email->hasAttachmentFromStorageDisk('avatars', 'avatar.jpg')); + } + + #[Test] + public function it_attaches_files_from_files_field() + { + Storage::fake('local'); + + $form = tap(Form::make('test')->formFields([ + 'fields' => [ + ['handle' => 'document', 'field' => ['type' => 'files', 'max_files' => 1]], + ], + ]))->save(); + + $documentPath = now()->timestamp.'/resume.pdf'; + Storage::disk('local')->put('statamic/file-uploads/'.$documentPath, 'contents'); + + $submission = $form->makeSubmission()->data(['document' => $documentPath]); + + $email = tap(new Email($submission, ['to' => 'test@test.com', 'attachments' => true], Site::default()))->build(); + + $this->assertTrue($email->hasAttachmentFromStorageDisk('local', 'statamic/file-uploads/'.$documentPath)); } #[Test] From f3c0c02d72ff501b95c154a7efe5d289c3b55706 Mon Sep 17 00:00:00 2001 From: Duncan McClean Date: Wed, 15 Jul 2026 17:05:39 +0100 Subject: [PATCH 10/17] prevent path traversal --- src/Forms/CreateAssetsFromFileUploads.php | 2 +- src/Forms/DeleteTemporaryFiles.php | 1 + src/Forms/SubmitForm.php | 13 +- .../Forms/CreateAssetsFromFileUploadsTest.php | 24 ++++ tests/Forms/SubmitFormTest.php | 111 ++++++++++++++++++ 5 files changed, 146 insertions(+), 5 deletions(-) diff --git a/src/Forms/CreateAssetsFromFileUploads.php b/src/Forms/CreateAssetsFromFileUploads.php index ee07f33082e..fc2b56b4109 100644 --- a/src/Forms/CreateAssetsFromFileUploads.php +++ b/src/Forms/CreateAssetsFromFileUploads.php @@ -67,7 +67,7 @@ private function createAssets(Field $field): array|string|null private function uploadAsset(Field $field, string $path): ?string { - $diskPath = "statamic/form-uploads/{$path}"; + $diskPath = "statamic/form-uploads/{$this->submission->id()}/{$field->handle()}/".basename($path); if (! Storage::disk('local')->exists($diskPath)) { return null; diff --git a/src/Forms/DeleteTemporaryFiles.php b/src/Forms/DeleteTemporaryFiles.php index 9ffff45abac..bd741534fdd 100644 --- a/src/Forms/DeleteTemporaryFiles.php +++ b/src/Forms/DeleteTemporaryFiles.php @@ -31,6 +31,7 @@ public function handle(): void $uploadFields->filter(fn (Field $field) => $field->type() === 'files')->each(function (Field $field): void { Collection::wrap($this->submission->get($field->handle(), [])) + ->reject(fn ($path) => str_contains($path, '..')) ->each(fn ($path) => Storage::disk('local')->delete('statamic/file-uploads/'.$path)); }); diff --git a/src/Forms/SubmitForm.php b/src/Forms/SubmitForm.php index bcbf1b05958..ccca8463d1e 100644 --- a/src/Forms/SubmitForm.php +++ b/src/Forms/SubmitForm.php @@ -236,10 +236,15 @@ private function withFileUploadRules(array $rules, $fields, array $files): array return $rules; } - // A carried-over value that's still an array (eg. removing one of several files without - // uploading a new one) should keep being validated normally. Only a `max_files: 1` field's - // collapsed scalar value needs the array/max/min rules skipped, since it was never a fresh - // upload to begin with. + // Anything not present in $files must match what's already stored against the field. + // Otherwise, it should be rejected. An unvalidated value could be used to read/delete + // arbitrary files when the submission is finalized. + if ($field->value() !== $this->submission?->get($handle)) { + $rules[$handle] = array_merge($rules[$handle] ?? [], ['prohibited']); + + return $rules; + } + if (is_array($field->value()) || ! isset($rules[$handle])) { return $rules; } diff --git a/tests/Forms/CreateAssetsFromFileUploadsTest.php b/tests/Forms/CreateAssetsFromFileUploadsTest.php index 578408516fb..4f4c992342f 100644 --- a/tests/Forms/CreateAssetsFromFileUploadsTest.php +++ b/tests/Forms/CreateAssetsFromFileUploadsTest.php @@ -141,6 +141,30 @@ public function it_skips_creating_an_asset_when_the_temporary_file_doesnt_exist( $this->assertEmpty(AssetContainer::find('avatars')->assets()); } + #[Test] + public function it_never_reads_or_deletes_outside_the_submissions_own_directory() + { + Storage::fake('local'); + $this->fakeAvatarsContainer(); + + $form = tap(Form::make('contact')->formFields([ + 'sections' => [ + ['fields' => [ + ['handle' => 'avatar', 'field' => ['type' => 'upload', 'store' => true, 'container' => 'avatars', 'max_files' => 1]], + ]], + ], + ]))->save(); + + Storage::disk('local')->put('framework/sessions/secret', 'sensitive'); + + $submission = tap($form->makeSubmission()->set('avatar', '../../framework/sessions/secret'))->save(); + + (new CreateAssetsFromFileUploads($submission))->handle(); + + Storage::disk('local')->assertExists('framework/sessions/secret'); + $this->assertEmpty(AssetContainer::find('avatars')->assets()); + } + #[Test] public function it_still_creates_the_asset_when_the_form_does_not_store_submissions() { diff --git a/tests/Forms/SubmitFormTest.php b/tests/Forms/SubmitFormTest.php index 1d4b63ded8d..018bd07ec6c 100644 --- a/tests/Forms/SubmitFormTest.php +++ b/tests/Forms/SubmitFormTest.php @@ -506,6 +506,117 @@ public function it_still_validates_min_files_when_removing_files_without_uploadi $form->submissions()->each->delete(); } + #[Test] + public function it_rejects_an_upload_value_that_was_never_actually_uploaded() + { + Storage::fake('avatars'); + AssetContainer::make('avatars')->disk('avatars')->save(); + + $form = tap(Form::make('uploads')->formFields([ + 'pages' => [ + [ + 'id' => 'main', + 'sections' => [ + ['fields' => [ + ['handle' => 'email', 'field' => ['type' => 'email']], + ['handle' => 'avatar', 'field' => ['type' => 'upload', 'store' => true, 'container' => 'avatars', 'max_files' => 1]], + ]], + ], + ], + ], + ]))->save(); + + try { + app(SubmitForm::class) + ->form($form) + ->page('main') + ->submit(data: ['email' => 'test@example.com', 'avatar' => '../../framework/sessions/x']); + + $this->fail('Expected ValidationException was not thrown'); + } catch (ValidationException $e) { + $this->assertArrayHasKey('avatar', $e->errors()); + } + + $form->submissions()->each->delete(); + } + + #[Test] + public function it_rejects_a_forged_multi_file_upload_value() + { + Storage::fake('avatars'); + AssetContainer::make('avatars')->disk('avatars')->save(); + + $form = tap(Form::make('uploads')->formFields([ + 'pages' => [ + [ + 'id' => 'main', + 'sections' => [ + ['fields' => [ + ['handle' => 'email', 'field' => ['type' => 'email']], + ['handle' => 'gallery', 'field' => ['type' => 'upload', 'store' => true, 'container' => 'avatars']], + ]], + ], + ], + ], + ]))->save(); + + try { + app(SubmitForm::class) + ->form($form) + ->page('main') + ->submit(data: ['email' => 'test@example.com', 'gallery' => ['../../framework/sessions/x']]); + + $this->fail('Expected ValidationException was not thrown'); + } catch (ValidationException $e) { + $this->assertArrayHasKey('gallery', $e->errors()); + } + + $form->submissions()->each->delete(); + } + + #[Test] + public function it_rejects_a_resubmitted_upload_value_that_doesnt_match_whats_stored() + { + Storage::fake('avatars'); + AssetContainer::make('avatars')->disk('avatars')->save(); + + $form = tap(Form::make('uploads')->formFields([ + 'pages' => [ + [ + 'id' => 'main', + 'sections' => [ + ['fields' => [ + ['handle' => 'email', 'field' => ['type' => 'email']], + ['handle' => 'avatar', 'field' => ['type' => 'upload', 'store' => true, 'container' => 'avatars', 'max_files' => 1]], + ]], + ], + ], + ], + ]))->save(); + + $first = app(SubmitForm::class) + ->form($form) + ->page('main') + ->submit( + data: ['email' => 'test@example.com'], + files: ['avatar' => [UploadedFile::fake()->image('avatar.jpg')]], + ); + + try { + app(SubmitForm::class) + ->form($form) + ->page('main') + ->resume($first->submission) + ->submit(data: ['email' => 'test@example.com', 'avatar' => '../../framework/sessions/x']); + + $this->fail('Expected ValidationException was not thrown'); + } catch (ValidationException $e) { + $this->assertArrayHasKey('avatar', $e->errors()); + } + + $form->submissions()->each->delete(); + } + #[Test] public function it_validates_the_extension_of_uploaded_files() { From 9dfb7eaa322591c0bca6cf909ca1768e27a455e7 Mon Sep 17 00:00:00 2001 From: Duncan McClean Date: Wed, 15 Jul 2026 17:21:10 +0100 Subject: [PATCH 11/17] tweak field instructions --- src/Forms/Fieldtypes/Upload.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Forms/Fieldtypes/Upload.php b/src/Forms/Fieldtypes/Upload.php index 237628febb0..3945e2cd54f 100644 --- a/src/Forms/Fieldtypes/Upload.php +++ b/src/Forms/Fieldtypes/Upload.php @@ -41,7 +41,7 @@ public function configFieldItems(): array ], 'min_files' => [ 'display' => __('Min Files'), - 'instructions' => __('Set a minimum number of selectable assets.'), + 'instructions' => __('The minimum number of files that may be uploaded.'), 'type' => 'integer', 'min' => 1, ], From 90a66f5ceef31ad09f303e6bea6c6651996df1ea Mon Sep 17 00:00:00 2001 From: Duncan McClean Date: Wed, 15 Jul 2026 17:22:03 +0100 Subject: [PATCH 12/17] prevent duplicate filenames --- src/Forms/Uploaders/FormFileUploader.php | 19 +++++++++++++++- tests/Forms/Uploaders/FormFileUploadTest.php | 23 ++++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/src/Forms/Uploaders/FormFileUploader.php b/src/Forms/Uploaders/FormFileUploader.php index 9b672684714..b7ff1351fbf 100644 --- a/src/Forms/Uploaders/FormFileUploader.php +++ b/src/Forms/Uploaders/FormFileUploader.php @@ -2,6 +2,7 @@ namespace Statamic\Forms\Uploaders; +use Illuminate\Support\Facades\Storage; use Statamic\Assets\FileUploader; use Symfony\Component\HttpFoundation\File\UploadedFile; @@ -22,7 +23,23 @@ public static function submission(string $submissionId, string $handle, ?string protected function uploadPath(UploadedFile $file) { - return "{$this->submissionId}/{$this->handle}/{$file->getClientOriginalName()}"; + $directory = "{$this->submissionId}/{$this->handle}"; + + return "{$directory}/".$this->uniqueFilename($directory, $file->getClientOriginalName()); + } + + private function uniqueFilename(string $directory, string $filename, int $count = 0): string + { + $extension = pathinfo($filename, PATHINFO_EXTENSION); + $basename = pathinfo($filename, PATHINFO_FILENAME); + $suffix = $count ? "-{$count}" : ''; + $candidate = $basename.$suffix.($extension ? ".{$extension}" : ''); + + if (Storage::disk('local')->exists($this->uploadPathPrefix()."{$directory}/{$candidate}")) { + return $this->uniqueFilename($directory, $filename, $count + 1); + } + + return $candidate; } protected function uploadPathPrefix() diff --git a/tests/Forms/Uploaders/FormFileUploadTest.php b/tests/Forms/Uploaders/FormFileUploadTest.php index 2a521629a7b..f00ae62da7e 100644 --- a/tests/Forms/Uploaders/FormFileUploadTest.php +++ b/tests/Forms/Uploaders/FormFileUploadTest.php @@ -71,4 +71,27 @@ public function different_fields_on_the_same_submission_get_their_own_folder() Storage::disk('local')->assertExists('statamic/form-uploads/submission-123/avatar/avatar.jpg'); Storage::disk('local')->assertExists('statamic/form-uploads/submission-123/document/resume.pdf'); } + + #[Test] + public function it_keeps_files_with_the_same_name_by_suffixing_duplicates() + { + Storage::fake('local'); + + $paths = FormFileUpload::field(['handle' => 'documents', 'max_files' => 3], 'submission-123') + ->upload([ + UploadedFile::fake()->create('resume.pdf', 10), + UploadedFile::fake()->create('resume.pdf', 10), + UploadedFile::fake()->create('resume.pdf', 10), + ]); + + $this->assertEquals([ + 'submission-123/documents/resume.pdf', + 'submission-123/documents/resume-1.pdf', + 'submission-123/documents/resume-2.pdf', + ], $paths); + + Storage::disk('local')->assertExists('statamic/form-uploads/submission-123/documents/resume.pdf'); + Storage::disk('local')->assertExists('statamic/form-uploads/submission-123/documents/resume-1.pdf'); + Storage::disk('local')->assertExists('statamic/form-uploads/submission-123/documents/resume-2.pdf'); + } } From 4be76d1987aacdba5b273acb0b4e28167e6a05c3 Mon Sep 17 00:00:00 2001 From: Duncan McClean Date: Wed, 15 Jul 2026 17:24:38 +0100 Subject: [PATCH 13/17] always clean up temporary upload storage 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. --- src/Forms/DeleteTemporaryFiles.php | 16 ++++++-------- src/Forms/SendEmails.php | 2 +- tests/Forms/DeleteTemporaryFilesTest.php | 28 ++++++++++++++++++++++++ 3 files changed, 36 insertions(+), 10 deletions(-) diff --git a/src/Forms/DeleteTemporaryFiles.php b/src/Forms/DeleteTemporaryFiles.php index bd741534fdd..16f648e035a 100644 --- a/src/Forms/DeleteTemporaryFiles.php +++ b/src/Forms/DeleteTemporaryFiles.php @@ -22,14 +22,9 @@ public function __construct(public Submission $submission) public function handle(): void { - $uploadFields = $this->submission->form()->blueprint()->fields()->all() - ->filter(fn (Field $field) => $field->type() === 'files' || ($field->type() === 'form_upload' && ! $field->fieldtype()->config('store'))); + $fields = $this->submission->form()->blueprint()->fields()->all(); - if ($uploadFields->isEmpty()) { - return; - } - - $uploadFields->filter(fn (Field $field) => $field->type() === 'files')->each(function (Field $field): void { + $fields->filter(fn (Field $field) => $field->type() === 'files')->each(function (Field $field): void { Collection::wrap($this->submission->get($field->handle(), [])) ->reject(fn ($path) => str_contains($path, '..')) ->each(fn ($path) => Storage::disk('local')->delete('statamic/file-uploads/'.$path)); @@ -39,9 +34,12 @@ public function handle(): void Storage::disk('local')->deleteDirectory($uploadsPath); } - $uploadFields->each(fn (Field $field) => $this->submission->remove($field->handle())); + $removed = $fields + ->filter(fn (Field $field) => $field->type() === 'files' || ($field->type() === 'form_upload' && ! $field->fieldtype()->config('store'))) + ->each(fn (Field $field) => $this->submission->remove($field->handle())) + ->isNotEmpty(); - if ($this->submission->form()->store()) { + if ($removed && $this->submission->form()->store()) { $this->submission->saveQuietly(); } } diff --git a/src/Forms/SendEmails.php b/src/Forms/SendEmails.php index 6f3e32c5b4a..c2d9b3dcc22 100644 --- a/src/Forms/SendEmails.php +++ b/src/Forms/SendEmails.php @@ -57,6 +57,6 @@ private function emailConfigs($submission) private function shouldDeleteTemporaryFiles(): bool { return $this->submission->form()->blueprint()->fields()->all() - ->contains(fn (Field $field) => $field->type() === 'files' || ($field->type() === 'form_upload' && ! $field->fieldtype()->config('store'))); + ->contains(fn (Field $field) => in_array($field->type(), ['files', 'form_upload'])); } } diff --git a/tests/Forms/DeleteTemporaryFilesTest.php b/tests/Forms/DeleteTemporaryFilesTest.php index 230fbb92078..26dbbeb7287 100644 --- a/tests/Forms/DeleteTemporaryFilesTest.php +++ b/tests/Forms/DeleteTemporaryFilesTest.php @@ -46,6 +46,34 @@ public function it_deletes_the_submissions_whole_temporary_storage_folder() Storage::disk('local')->assertDirectoryEmpty('statamic/form-uploads/'.$submission->id()); } + #[Test] + public function it_deletes_the_temporary_folder_for_a_form_with_only_store_true_fields() + { + Storage::fake('local'); + + $form = tap(Form::make('contact')->formFields([ + 'sections' => [ + ['fields' => [ + ['handle' => 'avatar', 'field' => ['type' => 'upload', 'store' => true, 'max_files' => 1]], + ]], + ], + ]))->save(); + + $submission = tap($form->makeSubmission())->save(); + + // Simulates an abandoned partial: the upload landed in temporary storage but was never + // promoted to a real asset because the submission was never finalized. + $avatarPath = FormFileUpload::field(['handle' => 'avatar', 'max_files' => 1], $submission->id()) + ->upload([UploadedFile::fake()->image('avatar.jpg')]); + + $submission->set('avatar', $avatarPath)->save(); + + (new DeleteTemporaryFiles($submission))->handle(); + + Storage::disk('local')->assertMissing('statamic/form-uploads/'.$avatarPath); + Storage::disk('local')->assertDirectoryEmpty('statamic/form-uploads/'.$submission->id()); + } + #[Test] public function it_removes_store_false_field_values_but_keeps_store_true_ones() { From b4aa07a9a2f659faabc737e77ac07e7b8f58652f Mon Sep 17 00:00:00 2001 From: Duncan McClean Date: Wed, 15 Jul 2026 17:25:55 +0100 Subject: [PATCH 14/17] allow removing files from a multi-file upload when resubmitting a page 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. --- src/Forms/SubmitForm.php | 11 ++++++--- tests/Forms/SubmitFormTest.php | 45 ++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 4 deletions(-) diff --git a/src/Forms/SubmitForm.php b/src/Forms/SubmitForm.php index ccca8463d1e..27fc1f443ca 100644 --- a/src/Forms/SubmitForm.php +++ b/src/Forms/SubmitForm.php @@ -236,10 +236,13 @@ private function withFileUploadRules(array $rules, $fields, array $files): array return $rules; } - // Anything not present in $files must match what's already stored against the field. - // Otherwise, it should be rejected. An unvalidated value could be used to read/delete - // arbitrary files when the submission is finalized. - if ($field->value() !== $this->submission?->get($handle)) { + // Anything not present in $files must already be stored against the field. Removing + // some of an existing multi-file value is fine, but a value that was never uploaded + // gets rejected - an unvalidated one could be used to read/delete arbitrary files + // when the submission is finalized. + $stored = Arr::wrap($this->submission?->get($handle)); + + if (collect(Arr::wrap($field->value()))->contains(fn ($value) => ! in_array($value, $stored, true))) { $rules[$handle] = array_merge($rules[$handle] ?? [], ['prohibited']); return $rules; diff --git a/tests/Forms/SubmitFormTest.php b/tests/Forms/SubmitFormTest.php index 018bd07ec6c..30496505407 100644 --- a/tests/Forms/SubmitFormTest.php +++ b/tests/Forms/SubmitFormTest.php @@ -506,6 +506,51 @@ public function it_still_validates_min_files_when_removing_files_without_uploadi $form->submissions()->each->delete(); } + #[Test] + public function it_allows_removing_a_file_from_a_multi_file_upload_when_resubmitting_its_page() + { + Storage::fake('avatars'); + AssetContainer::make('avatars')->disk('avatars')->save(); + + $form = tap(Form::make('uploads')->formFields([ + 'pages' => [ + [ + 'id' => 'main', + 'sections' => [ + ['fields' => [ + ['handle' => 'email', 'field' => ['type' => 'email']], + ['handle' => 'gallery', 'field' => ['type' => 'upload', 'store' => true, 'container' => 'avatars']], + ]], + ], + ], + ], + ]))->save(); + + $first = app(SubmitForm::class) + ->form($form) + ->page('main') + ->submit( + data: ['email' => 'test@example.com'], + files: ['gallery' => [UploadedFile::fake()->image('one.jpg'), UploadedFile::fake()->image('two.jpg')]], + ); + + // Dropping one of the two already-stored files, without uploading a replacement, is a valid + // subset of what's stored - it shouldn't be rejected as a forged value. + $result = app(SubmitForm::class) + ->form($form) + ->page('main') + ->resume($first->submission) + ->submit(data: [ + 'email' => 'test@example.com', + 'gallery' => [$first->submission->get('gallery')[0]], + ]); + + $this->assertTrue($result->isFinalized()); + $this->assertEquals([$first->submission->get('gallery')[0]], $result->submission->get('gallery')); + + $form->submissions()->each->delete(); + } + #[Test] public function it_rejects_an_upload_value_that_was_never_actually_uploaded() { From 5b83e81e1e72d0ecf7272191eb4ee21d5ceec343 Mon Sep 17 00:00:00 2001 From: Duncan McClean Date: Wed, 15 Jul 2026 18:08:02 +0100 Subject: [PATCH 15/17] tweak field instructions (again) --- src/Forms/Fieldtypes/Upload.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Forms/Fieldtypes/Upload.php b/src/Forms/Fieldtypes/Upload.php index 3945e2cd54f..73133d967a9 100644 --- a/src/Forms/Fieldtypes/Upload.php +++ b/src/Forms/Fieldtypes/Upload.php @@ -41,7 +41,7 @@ public function configFieldItems(): array ], 'min_files' => [ 'display' => __('Min Files'), - 'instructions' => __('The minimum number of files that may be uploaded.'), + 'instructions' => __('The minimum number of files that must be uploaded.'), 'type' => 'integer', 'min' => 1, ], From 811eaec7542211a67b65cea56d797ba55c1c6524 Mon Sep 17 00:00:00 2001 From: Duncan McClean Date: Thu, 16 Jul 2026 11:38:34 +0100 Subject: [PATCH 16/17] use configured file uploads disk and add config option to change form file uploads path --- config/forms.php | 12 ++++ src/Forms/CreateAssetsFromFileUploads.php | 12 ++-- src/Forms/DeleteTemporaryFiles.php | 12 ++-- src/Forms/Email.php | 8 ++- src/Forms/Uploaders/FormFileUploader.php | 5 +- .../Forms/CreateAssetsFromFileUploadsTest.php | 35 ++++++++++ tests/Forms/DeleteTemporaryFilesTest.php | 67 +++++++++++++++++++ tests/Forms/EmailTest.php | 54 +++++++++++++++ tests/Forms/SendEmailsTest.php | 2 +- tests/Forms/Uploaders/FormFileUploadTest.php | 57 ++++++++++++++++ 10 files changed, 249 insertions(+), 15 deletions(-) diff --git a/config/forms.php b/config/forms.php index dce48ea7c18..37f94e2d256 100644 --- a/config/forms.php +++ b/config/forms.php @@ -13,6 +13,18 @@ 'forms' => resource_path('forms'), + /* + |-------------------------------------------------------------------------- + | File Uploads Path + |-------------------------------------------------------------------------- + | + | The path (on the file uploads disk) where file uploads are stored + | before they're converted to an asset or deleted. + | + */ + + 'file_uploads_path' => env('STATAMIC_FORM_UPLOADS_PATH', 'statamic/form-uploads'), + /* |-------------------------------------------------------------------------- | Email View Folder diff --git a/src/Forms/CreateAssetsFromFileUploads.php b/src/Forms/CreateAssetsFromFileUploads.php index fc2b56b4109..5ea726f5dee 100644 --- a/src/Forms/CreateAssetsFromFileUploads.php +++ b/src/Forms/CreateAssetsFromFileUploads.php @@ -67,16 +67,18 @@ private function createAssets(Field $field): array|string|null private function uploadAsset(Field $field, string $path): ?string { - $diskPath = "statamic/form-uploads/{$this->submission->id()}/{$field->handle()}/".basename($path); + $disk = Storage::disk(config('statamic.system.file_uploads_disk', 'local')); + $basePath = config('statamic.forms.file_uploads_path', 'statamic/form-uploads'); + $diskPath = "{$basePath}/{$this->submission->id()}/{$field->handle()}/".basename($path); - if (! Storage::disk('local')->exists($diskPath)) { + if (! $disk->exists($diskPath)) { return null; } $uploadedFile = new UploadedFile( - Storage::disk('local')->path($diskPath), + $disk->path($diskPath), basename($path), - Storage::disk('local')->mimeType($diskPath), + $disk->mimeType($diskPath), null, true ); @@ -84,7 +86,7 @@ private function uploadAsset(Field $field, string $path): ?string $assetId = AssetsUploader::field($field->toArray())->upload($uploadedFile); $assetId = is_array($assetId) ? $assetId[0] : $assetId; - Storage::disk('local')->delete($diskPath); + $disk->delete($diskPath); return Asset::findOrFail($assetId)->path(); } diff --git a/src/Forms/DeleteTemporaryFiles.php b/src/Forms/DeleteTemporaryFiles.php index 16f648e035a..15e424944a9 100644 --- a/src/Forms/DeleteTemporaryFiles.php +++ b/src/Forms/DeleteTemporaryFiles.php @@ -23,15 +23,19 @@ public function __construct(public Submission $submission) public function handle(): void { $fields = $this->submission->form()->blueprint()->fields()->all(); + $disk = Storage::disk(config('statamic.system.file_uploads_disk', 'local')); + $basePath = config('statamic.forms.file_uploads_path', 'statamic/form-uploads'); + + $fields->filter(fn (Field $field) => $field->type() === 'files')->each(function (Field $field) use ($disk): void { + $fileUploadsPath = config('statamic.system.file_uploads_path', 'statamic/file-uploads'); - $fields->filter(fn (Field $field) => $field->type() === 'files')->each(function (Field $field): void { Collection::wrap($this->submission->get($field->handle(), [])) ->reject(fn ($path) => str_contains($path, '..')) - ->each(fn ($path) => Storage::disk('local')->delete('statamic/file-uploads/'.$path)); + ->each(fn ($path) => $disk->delete("{$fileUploadsPath}/".$path)); }); - if (Storage::disk('local')->exists($uploadsPath = "statamic/form-uploads/{$this->submission->id()}")) { - Storage::disk('local')->deleteDirectory($uploadsPath); + if ($disk->exists($uploadsPath = "{$basePath}/{$this->submission->id()}")) { + $disk->deleteDirectory($uploadsPath); } $removed = $fields diff --git a/src/Forms/Email.php b/src/Forms/Email.php index 4f77ab5ccf9..be52427b0ba 100644 --- a/src/Forms/Email.php +++ b/src/Forms/Email.php @@ -154,10 +154,14 @@ private function attachFiles($field) return; } - $prefix = $field['fieldtype'] === 'form_upload' ? 'statamic/form-uploads/' : 'statamic/file-uploads/'; + $disk = config('statamic.system.file_uploads_disk', 'local'); + + $basePath = $field['fieldtype'] === 'form_upload' + ? config('statamic.forms.file_uploads_path', 'statamic/form-uploads') + : config('statamic.system.file_uploads_path', 'statamic/file-uploads'); foreach ($value as $file) { - $this->attachFromStorageDisk('local', $prefix.$file); + $this->attachFromStorageDisk($disk, $basePath.'/'.$file); } } diff --git a/src/Forms/Uploaders/FormFileUploader.php b/src/Forms/Uploaders/FormFileUploader.php index b7ff1351fbf..56c371f1076 100644 --- a/src/Forms/Uploaders/FormFileUploader.php +++ b/src/Forms/Uploaders/FormFileUploader.php @@ -2,7 +2,6 @@ namespace Statamic\Forms\Uploaders; -use Illuminate\Support\Facades\Storage; use Statamic\Assets\FileUploader; use Symfony\Component\HttpFoundation\File\UploadedFile; @@ -35,7 +34,7 @@ private function uniqueFilename(string $directory, string $filename, int $count $suffix = $count ? "-{$count}" : ''; $candidate = $basename.$suffix.($extension ? ".{$extension}" : ''); - if (Storage::disk('local')->exists($this->uploadPathPrefix()."{$directory}/{$candidate}")) { + if ($this->disk()->exists($this->uploadPathPrefix()."{$directory}/{$candidate}")) { return $this->uniqueFilename($directory, $filename, $count + 1); } @@ -44,6 +43,6 @@ private function uniqueFilename(string $directory, string $filename, int $count protected function uploadPathPrefix() { - return 'statamic/form-uploads/'; + return config('statamic.forms.file_uploads_path', 'statamic/form-uploads').'/'; } } diff --git a/tests/Forms/CreateAssetsFromFileUploadsTest.php b/tests/Forms/CreateAssetsFromFileUploadsTest.php index 4f4c992342f..f9e2dbb6e47 100644 --- a/tests/Forms/CreateAssetsFromFileUploadsTest.php +++ b/tests/Forms/CreateAssetsFromFileUploadsTest.php @@ -49,6 +49,41 @@ public function it_creates_an_asset_from_a_temporary_file() $this->assertNotNull(Asset::find("avatars::{$newValue}")); } + #[Test] + public function it_creates_an_asset_from_a_temporary_file_on_the_configured_disk_and_path() + { + config([ + 'statamic.system.file_uploads_disk' => 'uploads', + 'statamic.forms.file_uploads_path' => 'temp-form-uploads', + ]); + + Storage::fake('local'); + $uploadsDisk = Storage::fake('uploads'); + $this->fakeAvatarsContainer(); + + $form = tap(Form::make('contact')->formFields([ + 'sections' => [ + ['fields' => [ + ['handle' => 'avatar', 'field' => ['type' => 'upload', 'store' => true, 'container' => 'avatars', 'max_files' => 1]], + ]], + ], + ]))->save(); + + $submission = tap($form->makeSubmission())->save(); + + $path = FormFileUpload::field(['handle' => 'avatar', 'max_files' => 1], $submission->id()) + ->upload([UploadedFile::fake()->image('avatar.jpg')]); + + $submission->set('avatar', $path)->save(); + + (new CreateAssetsFromFileUploads($submission))->handle(); + + $uploadsDisk->assertMissing('temp-form-uploads/'.$path); + Storage::disk('avatars')->assertExists('avatar.jpg'); + + $this->assertNotNull(Asset::find('avatars::'.$form->submission($submission->id())->get('avatar'))); + } + #[Test] public function it_creates_assets_from_multiple_temporary_files() { diff --git a/tests/Forms/DeleteTemporaryFilesTest.php b/tests/Forms/DeleteTemporaryFilesTest.php index 26dbbeb7287..8edcb885265 100644 --- a/tests/Forms/DeleteTemporaryFilesTest.php +++ b/tests/Forms/DeleteTemporaryFilesTest.php @@ -153,6 +153,73 @@ public function it_deletes_temp_files_created_by_the_files_fieldtype() $this->assertNull($form->submission($submission->id())->get('document')); } + #[Test] + public function it_deletes_the_temporary_storage_folder_from_the_configured_disk_and_path() + { + config([ + 'statamic.system.file_uploads_disk' => 'uploads', + 'statamic.forms.file_uploads_path' => 'temp-form-uploads', + ]); + + $localDisk = Storage::fake('local'); + $uploadsDisk = Storage::fake('uploads'); + + $form = tap(Form::make('contact')->formFields([ + 'sections' => [ + ['fields' => [ + ['handle' => 'document', 'field' => ['type' => 'upload', 'store' => false, 'max_files' => 1]], + ]], + ], + ]))->save(); + + $submission = tap($form->makeSubmission())->save(); + + $path = FormFileUpload::field(['handle' => 'document', 'max_files' => 1], $submission->id()) + ->upload([UploadedFile::fake()->create('resume.pdf', 10)]); + + $submission->set('document', $path)->save(); + + $localDisk->put('temp-form-uploads/'.$path, 'contents'); + + (new DeleteTemporaryFiles($submission))->handle(); + + $uploadsDisk->assertMissing('temp-form-uploads/'.$path); + $localDisk->assertExists('temp-form-uploads/'.$path); + } + + #[Test] + public function it_deletes_files_fieldtype_uploads_from_the_configured_disk_and_path() + { + config([ + 'statamic.system.file_uploads_disk' => 'uploads', + 'statamic.system.file_uploads_path' => 'temp-uploads', + ]); + + $localDisk = Storage::fake('local'); + $uploadsDisk = Storage::fake('uploads'); + + $form = tap(Form::make('contact')->formFields([ + 'sections' => [ + ['fields' => [ + ['handle' => 'document', 'field' => ['type' => 'files', 'max_files' => 1]], + ]], + ], + ]))->save(); + + $submission = tap($form->makeSubmission())->save(); + + $path = now()->timestamp.'/resume.pdf'; + $uploadsDisk->put('temp-uploads/'.$path, 'contents'); + $localDisk->put('temp-uploads/'.$path, 'contents'); + + $submission->set('document', [$path])->save(); + + (new DeleteTemporaryFiles($submission))->handle(); + + $uploadsDisk->assertMissing('temp-uploads/'.$path); + $localDisk->assertExists('temp-uploads/'.$path); + } + #[Test] public function it_does_not_delete_asset_files() { diff --git a/tests/Forms/EmailTest.php b/tests/Forms/EmailTest.php index 725b89ce24e..c9d40b82f35 100644 --- a/tests/Forms/EmailTest.php +++ b/tests/Forms/EmailTest.php @@ -282,6 +282,33 @@ public function it_attaches_temporary_file_upload() $this->assertTrue($email->hasAttachmentFromStorageDisk('local', 'statamic/form-uploads/'.$path)); } + #[Test] + public function it_attaches_temporary_file_upload_from_the_configured_disk_and_path() + { + config([ + 'statamic.system.file_uploads_disk' => 'uploads', + 'statamic.forms.file_uploads_path' => 'temp-form-uploads', + ]); + + Storage::fake('local'); + Storage::fake('uploads'); + + $form = tap(Form::make('test')->formFields([ + 'fields' => [ + ['handle' => 'document', 'field' => ['type' => 'upload', 'store' => false, 'max_files' => 1]], + ], + ]))->save(); + + $submission = $form->makeSubmission(); + $path = FormFileUpload::field(['handle' => 'document', 'max_files' => 1], $submission->id()) + ->upload([UploadedFile::fake()->create('resume.pdf', 10)]); + $submission->data(['document' => $path]); + + $email = tap(new Email($submission, ['to' => 'test@test.com', 'attachments' => true], Site::default()))->build(); + + $this->assertTrue($email->hasAttachmentFromStorageDisk('uploads', 'temp-form-uploads/'.$path)); + } + #[Test] public function it_attaches_files_from_assets_field() { @@ -324,6 +351,33 @@ public function it_attaches_files_from_files_field() $this->assertTrue($email->hasAttachmentFromStorageDisk('local', 'statamic/file-uploads/'.$documentPath)); } + #[Test] + public function it_attaches_files_from_files_field_on_the_configured_disk_and_path() + { + config([ + 'statamic.system.file_uploads_disk' => 'uploads', + 'statamic.system.file_uploads_path' => 'temp-uploads', + ]); + + Storage::fake('local'); + $uploadsDisk = Storage::fake('uploads'); + + $form = tap(Form::make('test')->formFields([ + 'fields' => [ + ['handle' => 'document', 'field' => ['type' => 'files', 'max_files' => 1]], + ], + ]))->save(); + + $documentPath = now()->timestamp.'/resume.pdf'; + $uploadsDisk->put('temp-uploads/'.$documentPath, 'contents'); + + $submission = $form->makeSubmission()->data(['document' => $documentPath]); + + $email = tap(new Email($submission, ['to' => 'test@test.com', 'attachments' => true], Site::default()))->build(); + + $this->assertTrue($email->hasAttachmentFromStorageDisk('uploads', 'temp-uploads/'.$documentPath)); + } + #[Test] public function it_adds_renderable_fields() { diff --git a/tests/Forms/SendEmailsTest.php b/tests/Forms/SendEmailsTest.php index 17837b50ecd..60e62577357 100644 --- a/tests/Forms/SendEmailsTest.php +++ b/tests/Forms/SendEmailsTest.php @@ -159,7 +159,7 @@ public function delete_attachments_job_deletes_files_from_the_configured_disk_an $submission = $form->makeSubmission()->data(['attachments' => ['1234567/file.txt']]); - (new DeleteTemporaryAttachments($submission))->handle(); + (new DeleteTemporaryFiles($submission))->handle(); $uploadsDisk->assertMissing('temp-uploads/1234567/file.txt'); $localDisk->assertExists('statamic/file-uploads/1234567/file.txt'); diff --git a/tests/Forms/Uploaders/FormFileUploadTest.php b/tests/Forms/Uploaders/FormFileUploadTest.php index f00ae62da7e..66a6c227342 100644 --- a/tests/Forms/Uploaders/FormFileUploadTest.php +++ b/tests/Forms/Uploaders/FormFileUploadTest.php @@ -94,4 +94,61 @@ public function it_keeps_files_with_the_same_name_by_suffixing_duplicates() Storage::disk('local')->assertExists('statamic/form-uploads/submission-123/documents/resume-1.pdf'); Storage::disk('local')->assertExists('statamic/form-uploads/submission-123/documents/resume-2.pdf'); } + + #[Test] + public function it_writes_to_the_configured_disk() + { + config(['statamic.system.file_uploads_disk' => 'uploads']); + + $localDisk = Storage::fake('local'); + $uploadsDisk = Storage::fake('uploads'); + + FormFileUpload::field(['handle' => 'document', 'max_files' => 1], 'submission-123') + ->upload([UploadedFile::fake()->create('resume.pdf', 10)]); + + $uploadsDisk->assertExists('statamic/form-uploads/submission-123/document/resume.pdf'); + $localDisk->assertMissing('statamic/form-uploads/submission-123/document/resume.pdf'); + } + + #[Test] + public function it_writes_to_the_configured_path() + { + config(['statamic.forms.file_uploads_path' => 'temp-form-uploads']); + + $localDisk = Storage::fake('local'); + + $path = FormFileUpload::field(['handle' => 'document', 'max_files' => 1], 'submission-123') + ->upload([UploadedFile::fake()->create('resume.pdf', 10)]); + + $this->assertEquals('submission-123/document/resume.pdf', $path); + + $localDisk->assertExists('temp-form-uploads/submission-123/document/resume.pdf'); + $localDisk->assertMissing('statamic/form-uploads/submission-123/document/resume.pdf'); + } + + #[Test] + public function it_suffixes_duplicates_on_the_configured_disk_and_path() + { + config([ + 'statamic.system.file_uploads_disk' => 'uploads', + 'statamic.forms.file_uploads_path' => 'temp-form-uploads', + ]); + + Storage::fake('local'); + $uploadsDisk = Storage::fake('uploads'); + + $paths = FormFileUpload::field(['handle' => 'documents', 'max_files' => 2], 'submission-123') + ->upload([ + UploadedFile::fake()->create('resume.pdf', 10), + UploadedFile::fake()->create('resume.pdf', 10), + ]); + + $this->assertEquals([ + 'submission-123/documents/resume.pdf', + 'submission-123/documents/resume-1.pdf', + ], $paths); + + $uploadsDisk->assertExists('temp-form-uploads/submission-123/documents/resume.pdf'); + $uploadsDisk->assertExists('temp-form-uploads/submission-123/documents/resume-1.pdf'); + } } From 0245c4bf6464cabced3efe722d7742668dbadb95 Mon Sep 17 00:00:00 2001 From: Duncan McClean Date: Thu, 16 Jul 2026 16:45:40 +0100 Subject: [PATCH 17/17] avoid pre-cognitively validating files and form upload fields --- src/Fields/Validator.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Fields/Validator.php b/src/Fields/Validator.php index 394d7593795..0593bb9a755 100644 --- a/src/Fields/Validator.php +++ b/src/Fields/Validator.php @@ -77,7 +77,7 @@ private function fieldRules() } return $this->preProcessedFields()->all()->reduce(function ($carry, $field) { - if (request()->isPrecognitive() && $field->type() == 'assets') { + if (request()->isPrecognitive() && in_array($field->type(), ['assets', 'files', 'form_upload'], true)) { return $carry; }