From 1ac264ee19578ca92ef2f65818331ec9da3135e6 Mon Sep 17 00:00:00 2001 From: Duncan McClean Date: Tue, 28 Nov 2023 16:21:42 +0000 Subject: [PATCH 01/35] When email has attachments enabled, filter out asset fields from email view --- src/Forms/Email.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/Forms/Email.php b/src/Forms/Email.php index ec4d3b2eda..4984b3a283 100644 --- a/src/Forms/Email.php +++ b/src/Forms/Email.php @@ -132,10 +132,15 @@ protected function addAttachments() protected function addData() { $augmented = $this->submission->toAugmentedArray(); + $fields = $this->getRenderableFieldData(Arr::except($augmented, ['id', 'date', 'form'])); + + if (array_has($this->config, 'attachments')) { + $fields = $fields->reject(fn ($field) => $field['fieldtype'] === 'assets'); + } $data = array_merge($augmented, $this->getGlobalsData(), [ 'config' => config()->all(), - 'fields' => $this->getRenderableFieldData(Arr::except($augmented, ['id', 'date', 'form'])), + 'fields' => $fields, 'site_url' => Config::getSiteUrl(), 'date' => now(), 'now' => now(), From a96703fa3f191746695daf7859b3bdf5bb66026f Mon Sep 17 00:00:00 2001 From: Duncan McClean Date: Tue, 28 Nov 2023 16:56:51 +0000 Subject: [PATCH 02/35] Add delete attachments toggle to forms --- resources/lang/en/messages.php | 1 + src/Forms/Form.php | 24 ++++++++++++++++++- .../Controllers/CP/Forms/FormsController.php | 9 +++++++ 3 files changed, 33 insertions(+), 1 deletion(-) diff --git a/resources/lang/en/messages.php b/resources/lang/en/messages.php index a33e865c0d..1741a284f7 100644 --- a/resources/lang/en/messages.php +++ b/resources/lang/en/messages.php @@ -117,6 +117,7 @@ 'form_configure_honeypot_instructions' => 'Field name to use as a honeypot. Honeypots are special fields used to reduce botspam.', 'form_configure_intro' => 'Forms are used to collect information from visitors and dispatch events and notifications when there are new submissions.', 'form_configure_store_instructions' => 'Disable to stop storing submissions. Events and email notifications will still be sent.', + 'form_configure_delete_attachments_instructions' => 'Enable to delete attachments automatically after the submission emails are sent.', 'form_configure_title_instructions' => 'Usually a call to action, like "Contact Us".', 'getting_started_widget_blueprints' => 'Blueprints define the custom fields used to create and store content.', 'getting_started_widget_collections' => 'Collections contain the different types of content in the site.', diff --git a/src/Forms/Form.php b/src/Forms/Form.php index 268eec1091..0240103697 100644 --- a/src/Forms/Form.php +++ b/src/Forms/Form.php @@ -23,6 +23,7 @@ use Statamic\Forms\Exporters\Exporter; use Statamic\Statamic; use Statamic\Support\Arr; +use Statamic\Support\Str; use Statamic\Support\Traits\FluentlyGetsAndSets; class Form implements Arrayable, Augmentable, FormContract @@ -34,6 +35,7 @@ class Form implements Arrayable, Augmentable, FormContract protected $blueprint; protected $honeypot; protected $store; + protected $deleteAttachments = false; protected $email; protected $metrics; protected $afterSaveCallbacks = []; @@ -112,6 +114,21 @@ public function store($store = null) ->args(func_get_args()); } + /** + * Get or set the deleteAttachments field. + * + * @param mixed $store + * @return mixed + */ + public function deleteAttachments($deleteAttachments = null) + { + return $this->fluentlyGetOrSet('deleteAttachments') + ->setter(function ($deleteAttachments) { + return $deleteAttachments === true ? true : false; + }) + ->args(func_get_args()); + } + /** * Get or set the email field. * @@ -200,6 +217,10 @@ public function save() $data['store'] = false; } + if ($this->deleteAttachments === true) { + $data['delete_attachments'] = true; + } + File::put($this->path(), YAML::dump($data)); foreach ($afterSaveCallbacks as $callback) { @@ -240,11 +261,12 @@ public function hydrate() 'title', 'honeypot', 'store', + 'delete_attachments', 'email', ]); }) ->each(function ($value, $property) { - $this->{$property}($value); + $this->{Str::camel($property)}($value); }); return $this; diff --git a/src/Http/Controllers/CP/Forms/FormsController.php b/src/Http/Controllers/CP/Forms/FormsController.php index c1156ce9c5..dd845a5614 100644 --- a/src/Http/Controllers/CP/Forms/FormsController.php +++ b/src/Http/Controllers/CP/Forms/FormsController.php @@ -144,6 +144,7 @@ public function edit($form) 'title' => $form->title(), 'honeypot' => $form->honeypot(), 'store' => $form->store(), + 'delete_attachments' => $form->deleteAttachments(), 'email' => $form->email(), ]; @@ -174,6 +175,7 @@ public function update($form, Request $request) ->title($values['title']) ->honeypot($values['honeypot']) ->store($values['store']) + ->deleteAttachments($values['delete_attachments']) ->email($values['email']); $form->save(); @@ -225,6 +227,13 @@ protected function editFormBlueprint($form) 'display' => __('Store Submissions'), 'type' => 'toggle', 'instructions' => __('statamic::messages.form_configure_store_instructions'), + 'width' => 50, + ], + 'delete_attachments' => [ + 'display' => __('Delete Attachments after Submission?'), + 'type' => 'toggle', + 'instructions' => __('statamic::messages.form_configure_delete_attachments_instructions'), + 'width' => 50, ], ], ], From e4505a58399eadecea5021fd17279f620812373d Mon Sep 17 00:00:00 2001 From: Duncan McClean Date: Tue, 28 Nov 2023 17:30:22 +0000 Subject: [PATCH 03/35] Delete attachments after submission emails are sent --- src/Forms/DeleteTemporaryAttachments.php | 24 +++++++++++++++ src/Forms/SendEmails.php | 20 ++++++++----- src/Forms/Submission.php | 37 ++++++++++++++++++++++++ 3 files changed, 74 insertions(+), 7 deletions(-) create mode 100644 src/Forms/DeleteTemporaryAttachments.php diff --git a/src/Forms/DeleteTemporaryAttachments.php b/src/Forms/DeleteTemporaryAttachments.php new file mode 100644 index 0000000000..b19f0b5aa2 --- /dev/null +++ b/src/Forms/DeleteTemporaryAttachments.php @@ -0,0 +1,24 @@ +submission->deleteAttachments(); + } +} diff --git a/src/Forms/SendEmails.php b/src/Forms/SendEmails.php index 9dc93479f1..03f3803f19 100644 --- a/src/Forms/SendEmails.php +++ b/src/Forms/SendEmails.php @@ -4,8 +4,10 @@ use Illuminate\Foundation\Bus\Dispatchable; use Illuminate\Queue\SerializesModels; +use Illuminate\Support\Collection; use Illuminate\Support\Facades\Bus; use Statamic\Contracts\Forms\Submission; +use Statamic\Forms\DeleteTemporaryAttachments; use Statamic\Sites\Site; class SendEmails @@ -23,16 +25,20 @@ public function __construct(Submission $submission, Site $site) public function handle() { - $this->jobs()->each(fn ($job) => Bus::dispatch($job)); + Bus::chain($this->jobs())->dispatch(); } - private function jobs() + private function jobs(): Collection { - return $this->emailConfigs($this->submission)->map(function ($config) { - $class = config('statamic.forms.send_email_job'); - - return new $class($this->submission, $this->site, $config); - }); + return $this->emailConfigs($this->submission) + ->map(function ($config) { + $class = config('statamic.forms.send_email_job'); + + return new $class($this->submission, $this->site, $config); + }) + ->when($this->submission->form()->deleteAttachments(), function ($jobs) { + $jobs->push(new DeleteTemporaryAttachments($this->submission)); + }); } private function emailConfigs($submission) diff --git a/src/Forms/Submission.php b/src/Forms/Submission.php index a4f1378ec2..9cebcaaeea 100644 --- a/src/Forms/Submission.php +++ b/src/Forms/Submission.php @@ -3,6 +3,7 @@ namespace Statamic\Forms; use Carbon\Carbon; +use Statamic\Contracts\Assets\AssetContainer as AssetContainerContract; use Statamic\Contracts\Data\Augmentable; use Statamic\Contracts\Forms\Submission as SubmissionContract; use Statamic\Data\ContainsData; @@ -12,8 +13,13 @@ use Statamic\Events\SubmissionDeleted; use Statamic\Events\SubmissionSaved; use Statamic\Events\SubmissionSaving; +use Statamic\Exceptions\AssetContainerNotFoundException; +use Statamic\Facades\Asset; +use Statamic\Facades\AssetContainer; use Statamic\Facades\File; use Statamic\Facades\YAML; +use Statamic\Fields\Field; +use Statamic\Fieldtypes\Assets\UndefinedContainerException; use Statamic\Forms\Uploaders\AssetsUploader; use Statamic\Support\Arr; use Statamic\Support\Traits\FluentlyGetsAndSets; @@ -124,6 +130,37 @@ public function uploadFiles($uploadedFiles) })->all(); } + public function deleteAttachments(): void + { + $this->blueprint()->fields()->all() + ->filter(fn (Field $field) => $field->type() === 'assets') + ->each(function (Field $field) { + $assets = Arr::wrap($this->get($field->handle(), [])); + $assetContainer = $this->resolveAssetContainerForField($field); + + collect($assets)->each(function ($path) use ($assetContainer) { + $assetContainer->asset($path)?->delete(); + }); + }); + } + + protected function resolveAssetContainerForField(Field $field): AssetContainerContract + { + if ($configured = $field->get('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 afterSave($callback) { $this->afterSaveCallbacks[] = $callback; From 23380f2d59bbfcff4e39b59f0e7b28c4383ac0fd Mon Sep 17 00:00:00 2001 From: Duncan McClean Date: Wed, 29 Nov 2023 10:55:06 +0000 Subject: [PATCH 04/35] Fix tests & handle no jobs --- src/Forms/SendEmails.php | 9 ++-- tests/Forms/SendEmailsTest.php | 76 ++++++++++++++-------------------- tests/__fixtures__/users/.yaml | 3 ++ 3 files changed, 40 insertions(+), 48 deletions(-) create mode 100644 tests/__fixtures__/users/.yaml diff --git a/src/Forms/SendEmails.php b/src/Forms/SendEmails.php index 03f3803f19..fb0f5c1456 100644 --- a/src/Forms/SendEmails.php +++ b/src/Forms/SendEmails.php @@ -7,7 +7,6 @@ use Illuminate\Support\Collection; use Illuminate\Support\Facades\Bus; use Statamic\Contracts\Forms\Submission; -use Statamic\Forms\DeleteTemporaryAttachments; use Statamic\Sites\Site; class SendEmails @@ -23,9 +22,13 @@ public function __construct(Submission $submission, Site $site) $this->site = $site; } - public function handle() + public function handle(): void { - Bus::chain($this->jobs())->dispatch(); + $jobs = $this->jobs(); + + if ($jobs->isNotEmpty()) { + Bus::chain($jobs)->dispatch(); + } } private function jobs(): Collection diff --git a/tests/Forms/SendEmailsTest.php b/tests/Forms/SendEmailsTest.php index 4acf0c5323..0d4f21ae03 100644 --- a/tests/Forms/SendEmailsTest.php +++ b/tests/Forms/SendEmailsTest.php @@ -2,12 +2,11 @@ namespace Tests\Forms; -use Illuminate\Support\Facades\Queue; -use Mockery; +use Illuminate\Support\Facades\Bus; use Statamic\Facades\Form as FacadesForm; +use Statamic\Facades\Site; use Statamic\Forms\SendEmail; use Statamic\Forms\SendEmails; -use Statamic\Sites\Site; use Tests\PreventSavingStacheItemsToDisk; use Tests\TestCase; @@ -18,7 +17,7 @@ class SendEmailsTest extends TestCase /** @test */ public function it_queues_email_jobs() { - Queue::fake(); + Bus::fake(); $form = tap(FacadesForm::make('test')->email([ [ @@ -35,33 +34,24 @@ public function it_queues_email_jobs() (new SendEmails( $submission = $form->makeSubmission(), - $site = Mockery::mock(Site::class) + $site = Site::default(), ))->handle(); - Queue::assertPushed(SendEmail::class, 2); - - Queue::assertPushed(function (SendEmail $job) use ($submission, $site) { - return $job->submission === $submission - && $job->site === $site - && $job->config === [ - 'from' => 'first@sender.com', - 'to' => 'first@recipient.com', - 'foo' => 'bar', - // test that the config is passed along unparsed. - // the email class will handle that. we don't want to double parse. - 'unparsed' => '{{ test }}', - ]; - }); - - Queue::assertPushed(function (SendEmail $job) use ($submission, $site) { - return $job->submission === $submission - && $job->site === $site - && $job->config === [ - 'from' => 'second@sender.com', - 'to' => 'second@recipient.com', - 'baz' => 'qux', - ]; - }); + Bus::assertChained([ + new SendEmail($submission, $site, [ + 'from' => 'first@sender.com', + 'to' => 'first@recipient.com', + 'foo' => 'bar', + // test that the config is passed along unparsed. + // the email class will handle that. we don't want to double parse. + 'unparsed' => '{{ test }}', + ]), + new SendEmail($submission, $site, [ + 'from' => 'second@sender.com', + 'to' => 'second@recipient.com', + 'baz' => 'qux', + ]), + ]); } /** @test */ @@ -72,7 +62,7 @@ public function it_queues_email_jobs_when_config_contains_single_email() // but it's possible that a user may have only one email config. // e.g. [to,from,...] - Queue::fake(); + Bus::fake(); $form = tap(FacadesForm::make('test')->email([ 'from' => 'first@sender.com', @@ -82,20 +72,16 @@ public function it_queues_email_jobs_when_config_contains_single_email() (new SendEmails( $submission = $form->makeSubmission(), - $site = Mockery::mock(Site::class) + $site = Site::default(), ))->handle(); - Queue::assertPushed(SendEmail::class, 1); - - Queue::assertPushed(function (SendEmail $job) use ($submission, $site) { - return $job->submission === $submission - && $job->site === $site - && $job->config === [ - 'from' => 'first@sender.com', - 'to' => 'first@recipient.com', - 'foo' => 'bar', - ]; - }); + Bus::assertChained([ + new SendEmail($submission, $site, [ + 'from' => 'first@sender.com', + 'to' => 'first@recipient.com', + 'foo' => 'bar', + ]), + ]); } /** @@ -105,16 +91,16 @@ public function it_queues_email_jobs_when_config_contains_single_email() */ public function no_email_jobs_are_queued_if_none_are_configured($emailConfig) { - Queue::fake(); + Bus::fake(); $form = tap(FacadesForm::make('test')->email($emailConfig))->save(); (new SendEmails( $form->makeSubmission(), - Mockery::mock(Site::class) + Site::default(), ))->handle(); - Queue::assertNotPushed(SendEmail::class); + Bus::assertNothingDispatched(); } public function noEmailsProvider() diff --git a/tests/__fixtures__/users/.yaml b/tests/__fixtures__/users/.yaml new file mode 100644 index 0000000000..2f235617f9 --- /dev/null +++ b/tests/__fixtures__/users/.yaml @@ -0,0 +1,3 @@ +roles: + - noaccess +id: 3784807f-fa14-4e1d-af5a-0c0a425976fc From 30e0e01404b7bae56e1734bb56b7705b5b0ca976 Mon Sep 17 00:00:00 2001 From: Duncan McClean Date: Wed, 29 Nov 2023 10:58:01 +0000 Subject: [PATCH 05/35] Add test to ensure DeleteTemporaryAttachments job is dispatched --- tests/Forms/SendEmailsTest.php | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests/Forms/SendEmailsTest.php b/tests/Forms/SendEmailsTest.php index 0d4f21ae03..6e8f790b51 100644 --- a/tests/Forms/SendEmailsTest.php +++ b/tests/Forms/SendEmailsTest.php @@ -5,6 +5,7 @@ use Illuminate\Support\Facades\Bus; use Statamic\Facades\Form as FacadesForm; use Statamic\Facades\Site; +use Statamic\Forms\DeleteTemporaryAttachments; use Statamic\Forms\SendEmail; use Statamic\Forms\SendEmails; use Tests\PreventSavingStacheItemsToDisk; @@ -84,6 +85,32 @@ public function it_queues_email_jobs_when_config_contains_single_email() ]); } + /** @test */ + public function it_dispatches_delete_attachments_job_after_dispatching_email_jobs() + { + Bus::fake(); + + $form = tap(FacadesForm::make('test')->email([ + 'from' => 'first@sender.com', + 'to' => 'first@recipient.com', + 'foo' => 'bar', + ])->deleteAttachments(true))->save(); + + (new SendEmails( + $submission = $form->makeSubmission(), + $site = Site::default(), + ))->handle(); + + Bus::assertChained([ + new SendEmail($submission, $site, [ + 'from' => 'first@sender.com', + 'to' => 'first@recipient.com', + 'foo' => 'bar', + ]), + new DeleteTemporaryAttachments($submission), + ]); + } + /** * @test * From 82ea8f00bdec4688396fd4a77a91e838379c3708 Mon Sep 17 00:00:00 2001 From: Duncan McClean Date: Wed, 29 Nov 2023 11:11:38 +0000 Subject: [PATCH 06/35] Ensure attachments are deleted --- tests/Forms/SubmissionTest.php | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/tests/Forms/SubmissionTest.php b/tests/Forms/SubmissionTest.php index 70a5279d3b..350253b4f2 100644 --- a/tests/Forms/SubmissionTest.php +++ b/tests/Forms/SubmissionTest.php @@ -4,16 +4,21 @@ use Illuminate\Support\Collection; use Illuminate\Support\Facades\Event; +use Illuminate\Support\Facades\Storage; use Statamic\Events\SubmissionCreated; use Statamic\Events\SubmissionCreating; use Statamic\Events\SubmissionSaved; use Statamic\Events\SubmissionSaving; +use Statamic\Facades\AssetContainer; use Statamic\Facades\Blueprint; use Statamic\Facades\Form; +use Tests\PreventSavingStacheItemsToDisk; use Tests\TestCase; class SubmissionTest extends TestCase { + use PreventSavingStacheItemsToDisk; + /** @test */ public function the_id_is_generated_the_first_time_but_can_be_overridden() { @@ -187,4 +192,29 @@ public function if_saving_event_returns_false_the_submission_doesnt_save() Event::assertNotDispatched(SubmissionSaved::class); } + + /** @test */ + public function it_deletes_attachments() + { + Storage::fake('uploads'); + AssetContainer::make('uploads')->disk('uploads')->save(); + + $form = tap(Form::make('contact_us'))->save(); + $form->blueprint()->ensureField('attachments', ['type' => 'assets', 'container' => 'uploads'])->save(); + + Storage::disk('uploads')->put('foo.jpg', ''); + Storage::disk('uploads')->put('bar.pdf', ''); + Storage::disk('uploads')->put('baz.txt', ''); + + Storage::disk('uploads')->assertExists(['foo.jpg', 'bar.pdf', 'baz.txt']); + + $submission = tap($form->makeSubmission()->data([ + 'attachments' => ['foo.jpg', 'bar.pdf'], + ]))->save(); + + $submission->deleteAttachments(); + + Storage::disk('uploads')->assertMissing(['foo.jpg', 'bar.pdf']); + Storage::disk('uploads')->assertExists(['baz.txt']); + } } From e7133ba9b09cbe13b1a0344b4a8162636e04bb4a Mon Sep 17 00:00:00 2001 From: Duncan McClean Date: Wed, 29 Nov 2023 11:13:10 +0000 Subject: [PATCH 07/35] Clear assets field value --- src/Forms/Submission.php | 2 ++ tests/Forms/SubmissionTest.php | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/src/Forms/Submission.php b/src/Forms/Submission.php index 9cebcaaeea..813ea80950 100644 --- a/src/Forms/Submission.php +++ b/src/Forms/Submission.php @@ -141,6 +141,8 @@ public function deleteAttachments(): void collect($assets)->each(function ($path) use ($assetContainer) { $assetContainer->asset($path)?->delete(); }); + + $this->set($field->handle(), null)->saveQuietly(); }); } diff --git a/tests/Forms/SubmissionTest.php b/tests/Forms/SubmissionTest.php index 350253b4f2..0623dad07c 100644 --- a/tests/Forms/SubmissionTest.php +++ b/tests/Forms/SubmissionTest.php @@ -212,8 +212,12 @@ public function it_deletes_attachments() 'attachments' => ['foo.jpg', 'bar.pdf'], ]))->save(); + $this->assertCount(2, $submission->get('attachments')); + $submission->deleteAttachments(); + $this->assertNull($submission->get('attachments')); + Storage::disk('uploads')->assertMissing(['foo.jpg', 'bar.pdf']); Storage::disk('uploads')->assertExists(['baz.txt']); } From db216d164fb7c712124b5ba678ce7366d4cf9cbb Mon Sep 17 00:00:00 2001 From: Duncan McClean Date: Wed, 29 Nov 2023 11:19:39 +0000 Subject: [PATCH 08/35] whoops, didn't mean to commit a test user --- tests/__fixtures__/users/.yaml | 3 --- 1 file changed, 3 deletions(-) delete mode 100644 tests/__fixtures__/users/.yaml diff --git a/tests/__fixtures__/users/.yaml b/tests/__fixtures__/users/.yaml deleted file mode 100644 index 2f235617f9..0000000000 --- a/tests/__fixtures__/users/.yaml +++ /dev/null @@ -1,3 +0,0 @@ -roles: - - noaccess -id: 3784807f-fa14-4e1d-af5a-0c0a425976fc From ac97b84febdf199b0704d735419ea812532f62cd Mon Sep 17 00:00:00 2001 From: duncanmcclean Date: Wed, 29 Nov 2023 11:21:21 +0000 Subject: [PATCH 09/35] Fix styling --- src/Forms/Form.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Forms/Form.php b/src/Forms/Form.php index 0240103697..3e3f52ee88 100644 --- a/src/Forms/Form.php +++ b/src/Forms/Form.php @@ -114,7 +114,7 @@ public function store($store = null) ->args(func_get_args()); } - /** + /** * Get or set the deleteAttachments field. * * @param mixed $store From 7d894f47787281cd197bf7b8a6389cab9fd10055 Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Wed, 29 Nov 2023 13:05:24 -0500 Subject: [PATCH 10/35] dont fail fast --- .github/workflows/tests.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 67d8b7ae83..407c4241de 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -12,6 +12,7 @@ jobs: if: "!contains(github.event.head_commit.message, '[ci skip]')" strategy: + fail-fast: false matrix: php: [8.0, 8.1, 8.2, 8.3] laravel: [9.*, 10.*] From 3f169f6a212f1eed2d350df8c598399ff1975a48 Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Wed, 29 Nov 2023 13:09:20 -0500 Subject: [PATCH 11/35] skip test --- tests/Forms/SendEmailsTest.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/Forms/SendEmailsTest.php b/tests/Forms/SendEmailsTest.php index 6e8f790b51..bcab6ff874 100644 --- a/tests/Forms/SendEmailsTest.php +++ b/tests/Forms/SendEmailsTest.php @@ -58,6 +58,12 @@ public function it_queues_email_jobs() /** @test */ public function it_queues_email_jobs_when_config_contains_single_email() { + if (version_compare(app()->version(), '10.0.0', '<')) { + // The test breaks but it technically works. + // See: https://github.com/laravel/framework/pull/47832 + $this->markTestSkipped(); + } + // The email config should be an array of email configs. // e.g. [ [to,from,...], [to,from,...], ... ] // but it's possible that a user may have only one email config. From 232276cfd5c99a75211c5086549b6913a30b9062 Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Wed, 29 Nov 2023 13:09:27 -0500 Subject: [PATCH 12/35] bump to fixed version --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 1bbf69cc6b..afd637b2ec 100644 --- a/composer.json +++ b/composer.json @@ -15,7 +15,7 @@ "facade/ignition-contracts": "^1.0", "guzzlehttp/guzzle": "^6.3 || ^7.0", "james-heinrich/getid3": "^1.9.21", - "laravel/framework": "^9.50.0 || ^10.0", + "laravel/framework": "^9.50.0 || ^10.16.1", "laravel/helpers": "^1.1", "league/commonmark": "^2.2", "league/csv": "^9.0", From 24fe8dae581319ed78c11e79b42d1ebdf7f7612b Mon Sep 17 00:00:00 2001 From: Duncan McClean Date: Wed, 29 Nov 2023 18:12:36 +0000 Subject: [PATCH 13/35] do fail fast --- .github/workflows/tests.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 407c4241de..67d8b7ae83 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -12,7 +12,6 @@ jobs: if: "!contains(github.event.head_commit.message, '[ci skip]')" strategy: - fail-fast: false matrix: php: [8.0, 8.1, 8.2, 8.3] laravel: [9.*, 10.*] From 62d7b9131b57d0a90fa93a3acb122f1e1f83e806 Mon Sep 17 00:00:00 2001 From: Duncan McClean Date: Mon, 25 Mar 2024 17:29:41 +0000 Subject: [PATCH 14/35] Make it possible to use the Files fieldtype on forms --- resources/views/extend/forms/fields/files.antlers.html | 7 +++++++ src/Fieldtypes/Files.php | 2 ++ 2 files changed, 9 insertions(+) create mode 100644 resources/views/extend/forms/fields/files.antlers.html diff --git a/resources/views/extend/forms/fields/files.antlers.html b/resources/views/extend/forms/fields/files.antlers.html new file mode 100644 index 0000000000..0f0aa3c0c4 --- /dev/null +++ b/resources/views/extend/forms/fields/files.antlers.html @@ -0,0 +1,7 @@ + diff --git a/src/Fieldtypes/Files.php b/src/Fieldtypes/Files.php index 9dbf7c14fe..3945e9df32 100644 --- a/src/Fieldtypes/Files.php +++ b/src/Fieldtypes/Files.php @@ -8,6 +8,8 @@ class Files extends Fieldtype { protected $defaultValue = []; protected $selectable = false; + protected $selectableInForms = true; + protected $categories = ['media']; protected function configFieldItems(): array { From 3a88a1e6947adfcf809675fd732a5b73ab165922 Mon Sep 17 00:00:00 2001 From: Duncan McClean Date: Mon, 25 Mar 2024 17:30:38 +0000 Subject: [PATCH 15/35] Get uploads working with the Files fieldtype --- src/Forms/Email.php | 6 ++++-- src/Forms/SendEmails.php | 2 +- src/Forms/Submission.php | 15 ++++++++++++++- src/Http/Requests/FrontendFormRequest.php | 2 +- 4 files changed, 20 insertions(+), 5 deletions(-) diff --git a/src/Forms/Email.php b/src/Forms/Email.php index a9565a6fa3..9d883ed32a 100644 --- a/src/Forms/Email.php +++ b/src/Forms/Email.php @@ -112,7 +112,7 @@ protected function addAttachments() $this->getRenderableFieldData(Arr::except($this->submissionData, ['id', 'date', 'form'])) ->filter(function ($field) { - return $field['fieldtype'] === 'assets'; + return $field['fieldtype'] === 'assets' || $field['fieldtype'] === 'files'; }) ->each(function ($field) { $value = $field['value']->value(); @@ -122,7 +122,9 @@ protected function addAttachments() : $value->get(); foreach ($value as $file) { - $this->attachFromStorageDisk($file->container()->diskHandle(), $file->path()); + $field['fieldtype'] === 'files' + ? $this->attachFromStorageDisk('local', 'statamic/file-uploads/'.$file) + : $this->attachFromStorageDisk($file->container()->diskHandle(), $file->path()); } }); diff --git a/src/Forms/SendEmails.php b/src/Forms/SendEmails.php index fb0f5c1456..1f085d08ab 100644 --- a/src/Forms/SendEmails.php +++ b/src/Forms/SendEmails.php @@ -39,7 +39,7 @@ private function jobs(): Collection return new $class($this->submission, $this->site, $config); }) - ->when($this->submission->form()->deleteAttachments(), function ($jobs) { + ->when(true, function ($jobs) { $jobs->push(new DeleteTemporaryAttachments($this->submission)); }); } diff --git a/src/Forms/Submission.php b/src/Forms/Submission.php index 813ea80950..9cc549062f 100644 --- a/src/Forms/Submission.php +++ b/src/Forms/Submission.php @@ -3,6 +3,7 @@ namespace Statamic\Forms; use Carbon\Carbon; +use Statamic\Assets\FileUploader; use Statamic\Contracts\Assets\AssetContainer as AssetContainerContract; use Statamic\Contracts\Data\Augmentable; use Statamic\Contracts\Forms\Submission as SubmissionContract; @@ -126,7 +127,19 @@ public function formattedDate() public function uploadFiles($uploadedFiles) { return collect($uploadedFiles)->map(function ($files, $handle) { - return AssetsUploader::field($this->fields()->get($handle))->upload($files); + $field = $this->fields()->get($handle); + + if ($field['type'] === 'files') { + $paths = collect(Arr::wrap($files))->map(function ($file) { + return FileUploader::container()->upload($file); + }); + + return $field['max_files'] === 1 + ? $paths->first() + : $paths->all(); + } + + return AssetsUploader::field($field)->upload($files); })->all(); } diff --git a/src/Http/Requests/FrontendFormRequest.php b/src/Http/Requests/FrontendFormRequest.php index 728b398780..b3d13c4c01 100644 --- a/src/Http/Requests/FrontendFormRequest.php +++ b/src/Http/Requests/FrontendFormRequest.php @@ -110,7 +110,7 @@ private function normalizeAssetsValues($fields) { // The assets fieldtype is expecting an array, even for `max_files: 1`, but we don't want to force that on the front end. return $fields->all() - ->filter(fn ($field) => $field->fieldtype()->handle() === 'assets' && $this->hasFile($field->handle())) + ->filter(fn ($field) => in_array($field->fieldtype()->handle(), ['assets', 'files']) && $this->hasFile($field->handle())) ->map(fn ($field) => Arr::wrap($this->file($field->handle()))) ->all(); } From dbd44e0bd3945f3f305672836416fe019079b525 Mon Sep 17 00:00:00 2001 From: Duncan McClean Date: Mon, 25 Mar 2024 17:31:14 +0000 Subject: [PATCH 16/35] Ensure files are deleted after submission --- src/Forms/DeleteTemporaryAttachments.php | 15 ++++++++++- src/Forms/Submission.php | 33 ------------------------ 2 files changed, 14 insertions(+), 34 deletions(-) diff --git a/src/Forms/DeleteTemporaryAttachments.php b/src/Forms/DeleteTemporaryAttachments.php index b19f0b5aa2..7f2b7d10c9 100644 --- a/src/Forms/DeleteTemporaryAttachments.php +++ b/src/Forms/DeleteTemporaryAttachments.php @@ -7,7 +7,10 @@ use Illuminate\Foundation\Bus\Dispatchable; use Illuminate\Queue\InteractsWithQueue; use Illuminate\Queue\SerializesModels; +use Illuminate\Support\Facades\Storage; use Statamic\Contracts\Forms\Submission; +use Statamic\Fields\Field; +use Statamic\Support\Arr; class DeleteTemporaryAttachments implements ShouldQueue { @@ -19,6 +22,16 @@ public function __construct(public Submission $submission) public function handle() { - $this->submission->deleteAttachments(); + $this->submission->form()->blueprint()->fields()->all() + ->filter(fn (Field $field) => $field->type() === 'files') + ->each(function (Field $field) { + $files = collect(Arr::wrap($this->submission->get($field->handle(), []))); + + $files->each(function ($path) { + Storage::disk('local')->delete('statamic/file-uploads/' . $path); + }); + + $this->submission->set($field->handle(), null)->saveQuietly(); + }); } } diff --git a/src/Forms/Submission.php b/src/Forms/Submission.php index 9cc549062f..65fe05ee90 100644 --- a/src/Forms/Submission.php +++ b/src/Forms/Submission.php @@ -143,39 +143,6 @@ public function uploadFiles($uploadedFiles) })->all(); } - public function deleteAttachments(): void - { - $this->blueprint()->fields()->all() - ->filter(fn (Field $field) => $field->type() === 'assets') - ->each(function (Field $field) { - $assets = Arr::wrap($this->get($field->handle(), [])); - $assetContainer = $this->resolveAssetContainerForField($field); - - collect($assets)->each(function ($path) use ($assetContainer) { - $assetContainer->asset($path)?->delete(); - }); - - $this->set($field->handle(), null)->saveQuietly(); - }); - } - - protected function resolveAssetContainerForField(Field $field): AssetContainerContract - { - if ($configured = $field->get('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 afterSave($callback) { $this->afterSaveCallbacks[] = $callback; From 94db637fc515dd675510e7e78e4cb20474040f1a Mon Sep 17 00:00:00 2001 From: duncanmcclean Date: Mon, 25 Mar 2024 17:32:58 +0000 Subject: [PATCH 17/35] Fix styling --- src/Forms/DeleteTemporaryAttachments.php | 2 +- src/Forms/Submission.php | 5 ----- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/src/Forms/DeleteTemporaryAttachments.php b/src/Forms/DeleteTemporaryAttachments.php index 7f2b7d10c9..ec9820d6c5 100644 --- a/src/Forms/DeleteTemporaryAttachments.php +++ b/src/Forms/DeleteTemporaryAttachments.php @@ -28,7 +28,7 @@ public function handle() $files = collect(Arr::wrap($this->submission->get($field->handle(), []))); $files->each(function ($path) { - Storage::disk('local')->delete('statamic/file-uploads/' . $path); + Storage::disk('local')->delete('statamic/file-uploads/'.$path); }); $this->submission->set($field->handle(), null)->saveQuietly(); diff --git a/src/Forms/Submission.php b/src/Forms/Submission.php index 65fe05ee90..dd33fd73f3 100644 --- a/src/Forms/Submission.php +++ b/src/Forms/Submission.php @@ -4,7 +4,6 @@ use Carbon\Carbon; use Statamic\Assets\FileUploader; -use Statamic\Contracts\Assets\AssetContainer as AssetContainerContract; use Statamic\Contracts\Data\Augmentable; use Statamic\Contracts\Forms\Submission as SubmissionContract; use Statamic\Data\ContainsData; @@ -14,13 +13,9 @@ use Statamic\Events\SubmissionDeleted; use Statamic\Events\SubmissionSaved; use Statamic\Events\SubmissionSaving; -use Statamic\Exceptions\AssetContainerNotFoundException; use Statamic\Facades\Asset; -use Statamic\Facades\AssetContainer; use Statamic\Facades\File; use Statamic\Facades\YAML; -use Statamic\Fields\Field; -use Statamic\Fieldtypes\Assets\UndefinedContainerException; use Statamic\Forms\Uploaders\AssetsUploader; use Statamic\Support\Arr; use Statamic\Support\Traits\FluentlyGetsAndSets; From fab258605648dce42b4f17ecfc97c75979a37eed Mon Sep 17 00:00:00 2001 From: Duncan McClean Date: Mon, 25 Mar 2024 17:42:49 +0000 Subject: [PATCH 18/35] Drop the "Delete attachments after submission" toggle --- resources/lang/en/messages.php | 1 - src/Forms/Form.php | 23 +------------------ src/Forms/SendEmails.php | 1 + .../Controllers/CP/Forms/FormsController.php | 9 -------- tests/Forms/SendEmailsTest.php | 6 ----- 5 files changed, 2 insertions(+), 38 deletions(-) diff --git a/resources/lang/en/messages.php b/resources/lang/en/messages.php index 2589aa36c4..d2d4b906f7 100644 --- a/resources/lang/en/messages.php +++ b/resources/lang/en/messages.php @@ -120,7 +120,6 @@ 'form_configure_intro' => 'Forms are used to collect information from visitors and dispatch events and notifications when there are new submissions.', 'form_configure_mailer_instructions' => 'Choose the mailer for sending this email. Leave blank to fall back to the default mailer.', 'form_configure_store_instructions' => 'Disable to stop storing submissions. Events and email notifications will still be sent.', - 'form_configure_delete_attachments_instructions' => 'Enable to delete attachments automatically after the submission emails are sent.', 'form_configure_title_instructions' => 'Usually a call to action, like "Contact Us".', 'getting_started_widget_blueprints' => 'Blueprints define the custom fields used to create and store content.', 'getting_started_widget_collections' => 'Collections contain the different types of content in the site.', diff --git a/src/Forms/Form.php b/src/Forms/Form.php index f1547eae8b..b3fc38da7c 100644 --- a/src/Forms/Form.php +++ b/src/Forms/Form.php @@ -38,7 +38,6 @@ class Form implements Arrayable, Augmentable, FormContract protected $blueprint; protected $honeypot; protected $store; - protected $deleteAttachments = false; protected $email; protected $metrics; protected $afterSaveCallbacks = []; @@ -117,21 +116,6 @@ public function store($store = null) ->args(func_get_args()); } - /** - * Get or set the deleteAttachments field. - * - * @param mixed $store - * @return mixed - */ - public function deleteAttachments($deleteAttachments = null) - { - return $this->fluentlyGetOrSet('deleteAttachments') - ->setter(function ($deleteAttachments) { - return $deleteAttachments === true ? true : false; - }) - ->args(func_get_args()); - } - /** * Get or set the email field. * @@ -220,10 +204,6 @@ public function save() $data['store'] = false; } - if ($this->deleteAttachments === true) { - $data['delete_attachments'] = true; - } - File::put($this->path(), YAML::dump($data)); foreach ($afterSaveCallbacks as $callback) { @@ -268,12 +248,11 @@ public function hydrate() 'title', 'honeypot', 'store', - 'delete_attachments', 'email', ]); }) ->each(function ($value, $property) { - $this->{Str::camel($property)}($value); + $this->{$property}($value); }); return $this; diff --git a/src/Forms/SendEmails.php b/src/Forms/SendEmails.php index 1f085d08ab..e835a754a9 100644 --- a/src/Forms/SendEmails.php +++ b/src/Forms/SendEmails.php @@ -39,6 +39,7 @@ private function jobs(): Collection return new $class($this->submission, $this->site, $config); }) + // TODO: only dispatch this job when the form blueprint contains a Files field. ->when(true, function ($jobs) { $jobs->push(new DeleteTemporaryAttachments($this->submission)); }); diff --git a/src/Http/Controllers/CP/Forms/FormsController.php b/src/Http/Controllers/CP/Forms/FormsController.php index 643f9848c8..6ac63cdeec 100644 --- a/src/Http/Controllers/CP/Forms/FormsController.php +++ b/src/Http/Controllers/CP/Forms/FormsController.php @@ -147,7 +147,6 @@ public function edit($form) 'title' => __($form->title()), 'honeypot' => $form->honeypot(), 'store' => $form->store(), - 'delete_attachments' => $form->deleteAttachments(), 'email' => $form->email(), ]; @@ -178,7 +177,6 @@ public function update($form, Request $request) ->title($values['title']) ->honeypot($values['honeypot']) ->store($values['store']) - ->deleteAttachments($values['delete_attachments']) ->email($values['email']); $form->save(); @@ -230,13 +228,6 @@ protected function editFormBlueprint($form) 'display' => __('Store Submissions'), 'type' => 'toggle', 'instructions' => __('statamic::messages.form_configure_store_instructions'), - 'width' => 50, - ], - 'delete_attachments' => [ - 'display' => __('Delete Attachments after Submission?'), - 'type' => 'toggle', - 'instructions' => __('statamic::messages.form_configure_delete_attachments_instructions'), - 'width' => 50, ], ], ], diff --git a/tests/Forms/SendEmailsTest.php b/tests/Forms/SendEmailsTest.php index 7c7b8daf9a..0a87ecd286 100644 --- a/tests/Forms/SendEmailsTest.php +++ b/tests/Forms/SendEmailsTest.php @@ -58,12 +58,6 @@ public function it_queues_email_jobs() /** @test */ public function it_queues_email_jobs_when_config_contains_single_email() { - if (version_compare(app()->version(), '10.0.0', '<')) { - // The test breaks but it technically works. - // See: https://github.com/laravel/framework/pull/47832 - $this->markTestSkipped(); - } - // The email config should be an array of email configs. // e.g. [ [to,from,...], [to,from,...], ... ] // but it's possible that a user may have only one email config. From a0201f8cbb652439f50cdb4a7c8afd910c8bd130 Mon Sep 17 00:00:00 2001 From: Duncan McClean Date: Mon, 25 Mar 2024 17:46:19 +0000 Subject: [PATCH 19/35] remove test - attachment deleting isn't done on the `Submission` anymore --- tests/Forms/SubmissionTest.php | 29 ----------------------------- 1 file changed, 29 deletions(-) diff --git a/tests/Forms/SubmissionTest.php b/tests/Forms/SubmissionTest.php index 0623dad07c..8b7a29ac60 100644 --- a/tests/Forms/SubmissionTest.php +++ b/tests/Forms/SubmissionTest.php @@ -192,33 +192,4 @@ public function if_saving_event_returns_false_the_submission_doesnt_save() Event::assertNotDispatched(SubmissionSaved::class); } - - /** @test */ - public function it_deletes_attachments() - { - Storage::fake('uploads'); - AssetContainer::make('uploads')->disk('uploads')->save(); - - $form = tap(Form::make('contact_us'))->save(); - $form->blueprint()->ensureField('attachments', ['type' => 'assets', 'container' => 'uploads'])->save(); - - Storage::disk('uploads')->put('foo.jpg', ''); - Storage::disk('uploads')->put('bar.pdf', ''); - Storage::disk('uploads')->put('baz.txt', ''); - - Storage::disk('uploads')->assertExists(['foo.jpg', 'bar.pdf', 'baz.txt']); - - $submission = tap($form->makeSubmission()->data([ - 'attachments' => ['foo.jpg', 'bar.pdf'], - ]))->save(); - - $this->assertCount(2, $submission->get('attachments')); - - $submission->deleteAttachments(); - - $this->assertNull($submission->get('attachments')); - - Storage::disk('uploads')->assertMissing(['foo.jpg', 'bar.pdf']); - Storage::disk('uploads')->assertExists(['baz.txt']); - } } From 3b1565bc54cf576588e1ee1cb72c56d65ef90021 Mon Sep 17 00:00:00 2001 From: Duncan McClean Date: Mon, 25 Mar 2024 17:46:22 +0000 Subject: [PATCH 20/35] wip --- tests/Forms/SendEmailsTest.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/Forms/SendEmailsTest.php b/tests/Forms/SendEmailsTest.php index 0a87ecd286..796fe77820 100644 --- a/tests/Forms/SendEmailsTest.php +++ b/tests/Forms/SendEmailsTest.php @@ -90,11 +90,13 @@ public function it_dispatches_delete_attachments_job_after_dispatching_email_job { Bus::fake(); + // TODO: add a Files field to the form blueprint to trigger this behaviour. + $form = tap(FacadesForm::make('test')->email([ 'from' => 'first@sender.com', 'to' => 'first@recipient.com', 'foo' => 'bar', - ])->deleteAttachments(true))->save(); + ]))->save(); (new SendEmails( $submission = $form->makeSubmission(), From 1a599e4b64f13d8dd5a662c4375ab45cd25c9dd8 Mon Sep 17 00:00:00 2001 From: Duncan McClean Date: Mon, 25 Mar 2024 17:53:57 +0000 Subject: [PATCH 21/35] only dispatch `DeleteTemporaryAttachments` when there are files to be deleted --- src/Forms/SendEmails.php | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/Forms/SendEmails.php b/src/Forms/SendEmails.php index e835a754a9..f03aa1e4f8 100644 --- a/src/Forms/SendEmails.php +++ b/src/Forms/SendEmails.php @@ -7,6 +7,7 @@ use Illuminate\Support\Collection; use Illuminate\Support\Facades\Bus; use Statamic\Contracts\Forms\Submission; +use Statamic\Fields\Field; use Statamic\Sites\Site; class SendEmails @@ -39,8 +40,7 @@ private function jobs(): Collection return new $class($this->submission, $this->site, $config); }) - // TODO: only dispatch this job when the form blueprint contains a Files field. - ->when(true, function ($jobs) { + ->when($this->shouldDeleteTemporaryAttachments(), function ($jobs) { $jobs->push(new DeleteTemporaryAttachments($this->submission)); }); } @@ -53,4 +53,14 @@ private function emailConfigs($submission) return collect($config); } + + protected function shouldDeleteTemporaryAttachments(): bool + { + $fields = $this->submission->form()->blueprint()->fields()->all(); + + return $fields + ->filter(fn (Field $field) => $field->fieldtype()->handle() === 'files') + ->filter() + ->count() > 0; + } } From 31c8f834e8e7ac8d62fd5015e417bb14d9e55063 Mon Sep 17 00:00:00 2001 From: duncanmcclean Date: Mon, 25 Mar 2024 17:56:01 +0000 Subject: [PATCH 22/35] Fix styling --- tests/Forms/SubmissionTest.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/Forms/SubmissionTest.php b/tests/Forms/SubmissionTest.php index 8b7a29ac60..730d3a4c7f 100644 --- a/tests/Forms/SubmissionTest.php +++ b/tests/Forms/SubmissionTest.php @@ -4,12 +4,10 @@ use Illuminate\Support\Collection; use Illuminate\Support\Facades\Event; -use Illuminate\Support\Facades\Storage; use Statamic\Events\SubmissionCreated; use Statamic\Events\SubmissionCreating; use Statamic\Events\SubmissionSaved; use Statamic\Events\SubmissionSaving; -use Statamic\Facades\AssetContainer; use Statamic\Facades\Blueprint; use Statamic\Facades\Form; use Tests\PreventSavingStacheItemsToDisk; From 0f4d88891fea733610a104026f9143b1252c3cfe Mon Sep 17 00:00:00 2001 From: Duncan McClean Date: Mon, 25 Mar 2024 18:15:09 +0000 Subject: [PATCH 23/35] Fix test --- tests/Forms/SendEmailsTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Forms/SendEmailsTest.php b/tests/Forms/SendEmailsTest.php index 796fe77820..864c38a49f 100644 --- a/tests/Forms/SendEmailsTest.php +++ b/tests/Forms/SendEmailsTest.php @@ -90,14 +90,14 @@ public function it_dispatches_delete_attachments_job_after_dispatching_email_job { Bus::fake(); - // TODO: add a Files field to the form blueprint to trigger this behaviour. - $form = tap(FacadesForm::make('test')->email([ 'from' => 'first@sender.com', 'to' => 'first@recipient.com', 'foo' => 'bar', ]))->save(); + $form->blueprint()->ensureField('attachments', ['type' => 'files'])->save(); + (new SendEmails( $submission = $form->makeSubmission(), $site = Site::default(), From 91520e03581c3ddf51d20912c98fb608f836fd32 Mon Sep 17 00:00:00 2001 From: Duncan McClean Date: Tue, 26 Mar 2024 14:03:16 +0000 Subject: [PATCH 24/35] change handle of form in test --- tests/Forms/SendEmailsTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Forms/SendEmailsTest.php b/tests/Forms/SendEmailsTest.php index 864c38a49f..150ecd4657 100644 --- a/tests/Forms/SendEmailsTest.php +++ b/tests/Forms/SendEmailsTest.php @@ -90,7 +90,7 @@ public function it_dispatches_delete_attachments_job_after_dispatching_email_job { Bus::fake(); - $form = tap(FacadesForm::make('test')->email([ + $form = tap(FacadesForm::make('attachments_test')->email([ 'from' => 'first@sender.com', 'to' => 'first@recipient.com', 'foo' => 'bar', From d14716acc01cb3036f38249a520a5da7f8510e29 Mon Sep 17 00:00:00 2001 From: Duncan McClean Date: Tue, 26 Mar 2024 14:04:13 +0000 Subject: [PATCH 25/35] Specify Laravel 10.33 as a minimum version Laravel 10.33 includes this PR which we're using to test the form email jobs are being queued/chained succesfully. https://github.com/laravel/framework/pull/49003 --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 9bc42d31c3..52aa5feb26 100644 --- a/composer.json +++ b/composer.json @@ -14,7 +14,7 @@ "composer/composer": "^2.2.22", "guzzlehttp/guzzle": "^6.3 || ^7.0", "james-heinrich/getid3": "^1.9.21", - "laravel/framework": "^10.0 || ^11.0", + "laravel/framework": "^10.33 || ^11.0", "laravel/helpers": "^1.1", "league/commonmark": "^2.2", "league/csv": "^9.0", From 6a99b41a355e34e4b17d387f29be2afe17a00269 Mon Sep 17 00:00:00 2001 From: Duncan McClean Date: Tue, 26 Mar 2024 14:06:17 +0000 Subject: [PATCH 26/35] wip --- src/Forms/Form.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Forms/Form.php b/src/Forms/Form.php index b3fc38da7c..2ab0d6ee1a 100644 --- a/src/Forms/Form.php +++ b/src/Forms/Form.php @@ -25,7 +25,6 @@ use Statamic\Forms\Exporters\Exporter; use Statamic\Statamic; use Statamic\Support\Arr; -use Statamic\Support\Str; use Statamic\Support\Traits\FluentlyGetsAndSets; use Statamic\Yaml\ParseException; From 1d576279c70b6e72abcda4f52e3f40ee9c256fec Mon Sep 17 00:00:00 2001 From: Duncan McClean Date: Tue, 26 Mar 2024 14:07:35 +0000 Subject: [PATCH 27/35] wip --- src/Forms/SendEmails.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Forms/SendEmails.php b/src/Forms/SendEmails.php index f03aa1e4f8..3398d56691 100644 --- a/src/Forms/SendEmails.php +++ b/src/Forms/SendEmails.php @@ -56,9 +56,7 @@ private function emailConfigs($submission) protected function shouldDeleteTemporaryAttachments(): bool { - $fields = $this->submission->form()->blueprint()->fields()->all(); - - return $fields + return $this->submission->form()->blueprint()->fields()->all() ->filter(fn (Field $field) => $field->fieldtype()->handle() === 'files') ->filter() ->count() > 0; From b2a615345a5578f2c88ce28bc26ff998007fec0f Mon Sep 17 00:00:00 2001 From: Duncan McClean Date: Tue, 26 Mar 2024 14:18:27 +0000 Subject: [PATCH 28/35] Upload the same way as assets --- src/Forms/Submission.php | 16 ++---- src/Forms/Uploaders/FilesUploader.php | 76 +++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 12 deletions(-) create mode 100644 src/Forms/Uploaders/FilesUploader.php diff --git a/src/Forms/Submission.php b/src/Forms/Submission.php index dd33fd73f3..d3088689c2 100644 --- a/src/Forms/Submission.php +++ b/src/Forms/Submission.php @@ -3,7 +3,6 @@ namespace Statamic\Forms; use Carbon\Carbon; -use Statamic\Assets\FileUploader; use Statamic\Contracts\Data\Augmentable; use Statamic\Contracts\Forms\Submission as SubmissionContract; use Statamic\Data\ContainsData; @@ -17,6 +16,7 @@ use Statamic\Facades\File; use Statamic\Facades\YAML; use Statamic\Forms\Uploaders\AssetsUploader; +use Statamic\Forms\Uploaders\FilesUploader; use Statamic\Support\Arr; use Statamic\Support\Traits\FluentlyGetsAndSets; @@ -124,17 +124,9 @@ public function uploadFiles($uploadedFiles) return collect($uploadedFiles)->map(function ($files, $handle) { $field = $this->fields()->get($handle); - if ($field['type'] === 'files') { - $paths = collect(Arr::wrap($files))->map(function ($file) { - return FileUploader::container()->upload($file); - }); - - return $field['max_files'] === 1 - ? $paths->first() - : $paths->all(); - } - - return AssetsUploader::field($field)->upload($files); + return $field['type'] === 'files' + ? FilesUploader::field($field)->upload($files) + : AssetsUploader::field($field)->upload($files); })->all(); } diff --git a/src/Forms/Uploaders/FilesUploader.php b/src/Forms/Uploaders/FilesUploader.php new file mode 100644 index 0000000000..5a575b39e0 --- /dev/null +++ b/src/Forms/Uploaders/FilesUploader.php @@ -0,0 +1,76 @@ +config = collect($config); + } + + /** + * Instantiate files uploader. + * + * @param array $config + * @return static + */ + public static function field($config) + { + return new static($config); + } + + /** + * Upload the files and return their IDs. + * + * @param mixed $files + * @return array|string + */ + public function upload($files) + { + $ids = $this->getUploadableFiles($files)->map(function ($file) { + return FileUploader::container()->upload($file); + }); + + return $this->isSingleFile() + ? $ids->first() + : $ids->all(); + } + + /** + * Get uploadable files. + * + * @param mixed $files + * @return \Illuminate\Support\Collection + */ + protected function getUploadableFiles($files) + { + $files = collect(Arr::wrap($files))->filter(); + + // If multiple uploads is not enabled for this field, we will + // simply take the first uploaded file and ignore the rest. + 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; + } +} From a22941ab468118501169efbc3ac878ca5ee61e9d Mon Sep 17 00:00:00 2001 From: Duncan McClean Date: Tue, 26 Mar 2024 14:20:17 +0000 Subject: [PATCH 29/35] wip --- tests/Forms/SubmissionTest.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/Forms/SubmissionTest.php b/tests/Forms/SubmissionTest.php index 730d3a4c7f..70a5279d3b 100644 --- a/tests/Forms/SubmissionTest.php +++ b/tests/Forms/SubmissionTest.php @@ -10,13 +10,10 @@ use Statamic\Events\SubmissionSaving; use Statamic\Facades\Blueprint; use Statamic\Facades\Form; -use Tests\PreventSavingStacheItemsToDisk; use Tests\TestCase; class SubmissionTest extends TestCase { - use PreventSavingStacheItemsToDisk; - /** @test */ public function the_id_is_generated_the_first_time_but_can_be_overridden() { From e6391192e71f9981ac79e336413121b20cf51290 Mon Sep 17 00:00:00 2001 From: Duncan McClean Date: Tue, 26 Mar 2024 15:52:26 +0000 Subject: [PATCH 30/35] in fact, we should be filtering out files fields here too --- src/Forms/Email.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Forms/Email.php b/src/Forms/Email.php index 9d883ed32a..a25df94400 100644 --- a/src/Forms/Email.php +++ b/src/Forms/Email.php @@ -137,7 +137,7 @@ protected function addData() $fields = $this->getRenderableFieldData(Arr::except($augmented, ['id', 'date', 'form'])); if (array_has($this->config, 'attachments')) { - $fields = $fields->reject(fn ($field) => $field['fieldtype'] === 'assets'); + $fields = $fields->reject(fn ($field) => $field['fieldtype'] === 'assets' || $field['fieldtype'] === 'files'); } $data = array_merge($augmented, $this->getGlobalsData(), [ From d2dd4fb7c3cb0303956bcbca40c942e37cce4080 Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Wed, 27 Mar 2024 16:36:42 -0400 Subject: [PATCH 31/35] check for the files fieldtype so files="true" gets added automatically to the form --- src/Forms/Form.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Forms/Form.php b/src/Forms/Form.php index 2ab0d6ee1a..1824315ccf 100644 --- a/src/Forms/Form.php +++ b/src/Forms/Form.php @@ -381,7 +381,7 @@ public function dateFormat() public function hasFiles() { return $this->fields()->filter(function ($field) { - return $field->fieldtype()->handle() === 'assets'; + return in_array($field->fieldtype()->handle(), ['assets', 'files']); })->isNotEmpty(); } From bfd7414cfe8d366e01681d5c55323d1bc341ff4f Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Wed, 27 Mar 2024 16:38:48 -0400 Subject: [PATCH 32/35] simplify ... - arrow closures etc - use Collection::wrap() which is identical to collect(Arr::wrap()) - use submission remove instead of set null - save once at the end rather than in each iteration --- src/Forms/DeleteTemporaryAttachments.php | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/Forms/DeleteTemporaryAttachments.php b/src/Forms/DeleteTemporaryAttachments.php index ec9820d6c5..9d49785f52 100644 --- a/src/Forms/DeleteTemporaryAttachments.php +++ b/src/Forms/DeleteTemporaryAttachments.php @@ -7,10 +7,10 @@ use Illuminate\Foundation\Bus\Dispatchable; use Illuminate\Queue\InteractsWithQueue; use Illuminate\Queue\SerializesModels; +use Illuminate\Support\Collection; use Illuminate\Support\Facades\Storage; use Statamic\Contracts\Forms\Submission; use Statamic\Fields\Field; -use Statamic\Support\Arr; class DeleteTemporaryAttachments implements ShouldQueue { @@ -25,13 +25,12 @@ public function handle() $this->submission->form()->blueprint()->fields()->all() ->filter(fn (Field $field) => $field->type() === 'files') ->each(function (Field $field) { - $files = collect(Arr::wrap($this->submission->get($field->handle(), []))); + Collection::wrap($this->submission->get($field->handle(), [])) + ->each(fn ($path) => Storage::disk('local')->delete('statamic/file-uploads/'.$path)); - $files->each(function ($path) { - Storage::disk('local')->delete('statamic/file-uploads/'.$path); - }); - - $this->submission->set($field->handle(), null)->saveQuietly(); + $this->submission->remove($field->handle()); }); + + $this->submission->saveQuietly(); } } From a4dda5ccc8cb79c058baaae6f0691af46889da88 Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Wed, 27 Mar 2024 17:01:32 -0400 Subject: [PATCH 33/35] in_array is shorter here --- src/Forms/Email.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Forms/Email.php b/src/Forms/Email.php index a25df94400..a881175767 100644 --- a/src/Forms/Email.php +++ b/src/Forms/Email.php @@ -137,7 +137,7 @@ protected function addData() $fields = $this->getRenderableFieldData(Arr::except($augmented, ['id', 'date', 'form'])); if (array_has($this->config, 'attachments')) { - $fields = $fields->reject(fn ($field) => $field['fieldtype'] === 'assets' || $field['fieldtype'] === 'files'); + $fields = $fields->reject(fn ($field) => in_array($field['fieldtype'], ['assets', 'files'])); } $data = array_merge($augmented, $this->getGlobalsData(), [ From 950ea37cee2aa6f16a29fa068708a390f3771c70 Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Wed, 27 Mar 2024 17:20:15 -0400 Subject: [PATCH 34/35] arrow func and in_array --- src/Forms/Email.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Forms/Email.php b/src/Forms/Email.php index a881175767..f84a3e2af0 100644 --- a/src/Forms/Email.php +++ b/src/Forms/Email.php @@ -111,9 +111,7 @@ protected function addAttachments() } $this->getRenderableFieldData(Arr::except($this->submissionData, ['id', 'date', 'form'])) - ->filter(function ($field) { - return $field['fieldtype'] === 'assets' || $field['fieldtype'] === 'files'; - }) + ->filter(fn ($field) => in_array($field['fieldtype'], ['assets', 'files'])) ->each(function ($field) { $value = $field['value']->value(); From f1ea05b722044853786d8b63e9134dc4bbce710b Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Wed, 27 Mar 2024 17:21:05 -0400 Subject: [PATCH 35/35] split into methods and fix files fieldtype not working when max_files > 1 --- src/Forms/Email.php | 39 ++++++++++++++++++++++++++++----------- 1 file changed, 28 insertions(+), 11 deletions(-) diff --git a/src/Forms/Email.php b/src/Forms/Email.php index f84a3e2af0..48dc816daf 100644 --- a/src/Forms/Email.php +++ b/src/Forms/Email.php @@ -113,22 +113,39 @@ protected function addAttachments() $this->getRenderableFieldData(Arr::except($this->submissionData, ['id', 'date', 'form'])) ->filter(fn ($field) => in_array($field['fieldtype'], ['assets', 'files'])) ->each(function ($field) { - $value = $field['value']->value(); - - $value = array_get($field, 'config.max_files') === 1 - ? collect([$value])->filter() - : $value->get(); - - foreach ($value as $file) { - $field['fieldtype'] === 'files' - ? $this->attachFromStorageDisk('local', 'statamic/file-uploads/'.$file) - : $this->attachFromStorageDisk($file->container()->diskHandle(), $file->path()); - } + $field['value'] = $field['value']->value(); + $field['fieldtype'] === 'assets' ? $this->attachAssets($field) : $this->attachFiles($field); }); return $this; } + private function attachAssets($field) + { + $value = $field['value']; + + $value = array_get($field, 'config.max_files') === 1 + ? collect([$value])->filter() + : $value->get(); + + foreach ($value as $asset) { + $this->attachFromStorageDisk($asset->container()->diskHandle(), $asset->path()); + } + } + + private function attachFiles($field) + { + $value = $field['value']; + + $value = array_get($field, 'config.max_files') === 1 + ? collect([$value])->filter() + : $value; + + foreach ($value as $file) { + $this->attachFromStorageDisk('local', 'statamic/file-uploads/'.$file); + } + } + protected function addData() { $augmented = $this->submission->toAugmentedArray();