-
-
Notifications
You must be signed in to change notification settings - Fork 530
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[5.x] Ability to use files fieldtype in forms for attaching temporary files #9084
[5.x] Ability to use files fieldtype in forms for attaching temporary files #9084
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was really well done, nice work. Sorry for the delay.
I think it's a of an awkward flow to have to upload and create assets only for them to be deleted.
We have a files
fieldtype that works similar to assets
and puts the files in a temporary location, leaving it there for you to do whatever you want. We use this in the "Reupload Asset" action.
I think instead of a "delete after upload" setting, we could let people use the files fieldtype to be able to upload files that will only be used as attachments. Then in the job we can delete those files.
Also if you want to you can target 5.x which might make tests and composer dependencies a bit simpler.
…elete-attachment-files-after-form-submission
…sion' of github.com:statamic/cms into add-option-to-delete-attachment-files-after-form-submission
…elete-attachment-files-after-form-submission
…sion' of github.com:statamic/cms into add-option-to-delete-attachment-files-after-form-submission
Laravel 10.33 includes this PR which we're using to test the form email jobs are being queued/chained succesfully. laravel/framework#49003
Good idea, I've re-worked this PR around the "Files" fieldtype instead. One less setting 😅 I've also re-targeted this PR at |
src/Forms/Email.php
Outdated
$fields = $this->getRenderableFieldData(Arr::except($augmented, ['id', 'date', 'form'])); | ||
|
||
if (array_has($this->config, 'attachments')) { | ||
$fields = $fields->reject(fn ($field) => $field['fieldtype'] === 'assets'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do asset fields need to get rejected?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When the "Attachments" toggle is enabled on a form email, this code hides the Assets fields from being displayed in the form data since the Asset is going to be attached anyway.
Otherwise, it'd show a URL to the asset (which when we were using Assets fields for uploads would 404 anyway).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In fact, that's made me realise we should be filtering out Files fields here too.
- 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
This pull request allows you to use the Files fieldtype in forms, enabling you to have file uploads which get deleted from the server after the form emails have sent.
This PR also makes a small improvement to the handling of assets in form submission emails. When the "Attachments" toggle is enabled on an email, we'll now filter out any Asset fields when displaying the submission data.
We figured that there's not much point in displaying the asset URLs if they're already attached to the email.
Closes #7711.