Form support lacks file upload

[nerdoc]

While FormComponent works reasonably well, It completely lacks file upload support. The problem is separated in a few parts:

• HTML seems to need a form tag with an enctype="multipart/form-data" attribute to allow file uploads
• There must be a POST request. Subsequent GET requests loose the file again - it must be kept track of in between (it's deleted/GCed if the user does not finish the form)
• I think the relevant code parts are in tetra/js/tetra.core.js - this is the main part where all the Js magic of Tetra happens - note: if you change something there locally, call tetra/build_js.sh to update the static min files and debug maps automatically.
• I think the callServerMethod() and getStateWithChildren(...) methods closely resemble the code @waqasidrees07 wrote to upload the file in his PR Waqas/enhancement/form support #70. But it has to be done in tetra.core.js.
  In the test_tetra_forms repository, there should ideally only be changed some minimal things like an added <form enctype="multipart/form-data"></form> tag.

UPDATE:

upload partially works, but there are a few issues that keep it from working properly, they must be fixed in order to close this issue:

• FileField validates "ok", even when no file is attached and the field is required.
• after re-rendering component, Alpine component is destroyed and not rebuilt.
• destination file is not stored in the upload_to directory of the FileField, but in MEDIA_ROOT directly (fixed in 024a9c9)
• Rather use django.core.files.upladedfile.TemporaryUploadedFile than using own temp file storage in _upload_temp_file(). Maybe modify Django's upload handler on the fly in FormComponents to always use TemporaryFileUploadHandler - and keep track of the uploaded file name?
• The attachment is retrieved correctly and saved in the component attribute, but the ModelForm does not save it correctly into the model when form_valid() is doing the form.save().
• An easy way to garbage-collect old temp files would be: delete all that are older than session timeout

[nerdoc]

Maybe this is usable:
https://dev.to/code_rabbi/programmatically-setting-file-inputs-in-javascript-2p7i

[nerdoc]

@gsxdsm Basically, in the form_upload branch, Tetra creates a x-model attribute automatically when creating the form component, for each form field. I first really thought that it is an Alpine bug and filed an issue there, just to learn that this is normal HTML behaviour: file input fields can't have x-model attrs.

As far as I found out, like stated above, I think that it should go this way.

• when a user adds a file to an file input field, Tetra should capture that changed event and upload the file to a temporary place (after checking the FormField constraints like size, ending etc.
• Upload must be done by tetra.core.js using Js FormData() - and not the normal form upload process - this is where @waqasidrees07 did wrong). a <form> tag MUST NOT be necessary, as Tetra doesn't enforce it. In fact, <form> tags create more problems, as they cause a full site reload, if not prevented actively. AFAIK the callServerMethod() and getStateWithChildren(...) methods are the best places for that - but I for myself don't have the Js skills to work that out well.
• when the FormComponent is validated wrong and HTML gets updated, the filename should remain as "value" in the file input field, so that it is visible the field has a value.
• When the component is submit()ted, and validated ok, the file could be moved to it's permanent place.
• There should be some garbage collection of files in the temporary directory, to be deleted. CAVE: race slow conditions, when the "GC" deletes "old" temp files, and someone is in the upload process and just uploaded a file into the temp. folder. I think there could be a (Tetra internal) TempFile model helpful to mark the upload time into the tmp folder, and GC only deletes files that exceed a certain age, e.g. 24h.

[nerdoc]

i pushed what I have in my local form_support branch.

[gsxdsm]

Got it thanks for the detailed breakdown. Starting to dig in there!

[nerdoc]

@gsxdsm Oh, and in case you didn't see it: I started a small example project to test the file upload:
https://github.com/tetra-framework/test_tetra_forms
Maybe you can use that.

[gsxdsm]

Excellent thank you!

[nerdoc]

@gsxdsm Did you make any progress? I have time tomorrow to help out here/code - if you tell me what you are doing ATM, I could drop in and help a bit?

[gsxdsm]

Not yet, have been a bit swamped - no notable progress. I have been looking at Laravel Livewire to see how they approach this, but nothing concrete.

[gsxdsm]

Still looking into this, not making much progress but will keep you updated this weekend

[gsxdsm]

Have been thinking about the cleanup of the temporary files - don't want to take another dependency here, but I'm thinking we need some sort of job/task. Did you have thoughts on the mechanism for the cleanup?

[gsxdsm]

Maybe kicking off something asynchronously from runserver?

[gsxdsm]

Okay, a quick test of creating an async job spawned from runserver to run at a period interval to do the cleanup seems to work. With this in place, I'll work on the rest of the upload process.

[nerdoc]

You mean a regular job? Because runserver isn't called in production (when using gunicorn, Daphne etc).
I think I wouldn't bother too much ATM when this job is executed, but just make some cleanup procedure that actually does the job. It could be up to the Dev when this cleanup happens, starting from a poor-man's-cron to a celery task.
IMHO it's enough when the procedure is available...

[gsxdsm]

Sounds good, will take that approach and focus on getting the rest working.

[nerdoc]

please tell me if I can assist. the form_support branch has already some code in it towards that aim, but not very much. If you add a PR, please do against that branch.

[gsxdsm]

Yes will do - I am working off of the form_support branch and the form test project, thanks! It is fairly complex, will keep you posted if I get stuck further.

[nerdoc]

As @waqasidrees07 got stuck in a very early stage, please feel free to contact me at any stage, so you don't make the same errors again as I did ;-)

[gsxdsm]

Quick update:

I have the change event being fired in Javascript and testing the file upload using the FormData (rather than a form tag).

Took a while to get my head around the overall flow, but thanks for the pointers - I see a path now to implementing. At least for single files! (Multi will need to come later)

[gsxdsm]

Getting closer! I have the javascript posting the file to a server method for the temporary upload on change! Debugging a bit, but making progress.

[gsxdsm]

Here's where I am:

• when a user adds a file to an file input field, Tetra should capture that changed event and upload the file to a temporary place

• after checking the FormField constraints like size, ending etc.

• Upload must be done by tetra.core.js using Js FormData()

• when the FormComponent is validated wrong and HTML gets updated, the filename should remain as "value" in the file input field, so that it is visible the field has a value.

• When the component is submit()ted, and validated ok, the file could be moved to it's permanent place.

• There should be some garbage collection of files in the temporary directory, to be deleted. CAVE: race slow conditions, when the "GC" deletes "old" temp files, and someone is in the upload process and just uploaded a file into the temp. folder. I think there could be a (Tetra internal) TempFile model helpful to mark the upload time into the tmp folder, and GC only deletes files that exceed a certain age, e.g. 24h

---

The hardest part was getting the interplay between the server method and javascript working, but that's all done. The rest should be pretty straightforward.

One question - do we want:

1. A task that can be run via manage.py to cleanup stale temp files?
2. Stale temp files to be cleaned up whenever a new temp file upload happens (this is what livewire does)
3. Both 1&2 -> benefit, the user will never have temp files accumulate if they forget to run the cleanup job or fail to setup a scheduler

I'm inclined to do #3, but want to get your POV.

Also, I'm thinking the TTL for the temp files should probably be a default of maybe 15 minutes? But should be overridable in settings?

Finally - I was going to make this work for multiple files, but I now realize django doesn't really support multiple files in a single FormField, right?

[nerdoc]

That's great news, really.
About your question:

I think that a GC procedure should be available definitely as management command, on the long term it could be a subcommand of "tetra", like manage.py tetra init, manage.py tetra startcomponent, and here: manage.py tetra clean.
This could trigger the same procedure as the poor-mans-GC with e.g. during file upload. Attention, here the race condition could occur.

I think I would really leave it up to the user. There may be use cases where the upload-auto-GC makes sense, in some (more critical) environments this would be a no-go...

So it would be absolutely ok if you just implement the GC procedure (e.g. in tetra.utils), I (or you?) can make a makagement command later too.

[gsxdsm]

I'm thinking through the step of moving the file from temporary storage to its permanent location and I'm guessing we'd want to use the FileField.upload_to property, right? (https://docs.djangoproject.com/en/5.1/ref/models/fields/#django.db.models.FileField.upload_to). If this is the case, I'd assume we'd also use the storage method on the File (or Image) field as well, right? We'd probably want to use the storage method on the file (or default) for the temporary file storage too.

Or did you have something more customizable/user controllable in mind?

[gsxdsm]

Okay, have the file moving and deletion of temp file working now. Tomorrow I will cleanup and submit something in the branch to take a look - we can iterate on that while I work on validation and the garbage collection.

[gsxdsm]

Actually I just put together a quick commit now to show what's done so far so you can provide feedback @nerdoc - would love feedback/suggestions. I need to do a lot more testing and refinement, but the basic approach is here

[nerdoc]

That's great, exactly how I thought it should be. I just hat a few minutes to look over the code, did not test it - but AFAICT this is definitely the way to go. Thanks, really. That was a few months of agony that comes to an end now ;-)

[nerdoc]

2 Things came into my mind while reading the code:

• The callServerMethod() and callServerMethodWithFile() are mostly dup code and may be merged - but maybe you had a good reason to use a separate method "with files".
• could the _uploadFile() Js method be a security risk? by uploading files (to at least a temp dir?)

Just to be kept in mind.

[gsxdsm]

Yes - I can merge them, I started with them merged and debated splitting it up or not, but will merge back.

Yes, _uploadFile is definitely something that needs a bit of scrutiny. The csrf check helps a little, but I am going to take another look at livewire's approach to see if there are additional checks/holes there. One quick thing I was thinking was to have the method become a no-op if the component doesn't have a FileField...

[nerdoc]

Yes, that would be some additional security. Maybe you could add just a TODO in the callServerMethodWithFile() so we don't forget the security checks at the backend side.

[gsxdsm]

Updated with a todo for security checks and restricted the upload endpoint to only components with a file field

[nerdoc]

@gsxdsm I've tested the file upload a bit. The temporary upload works great. But if the field is marked as required, the form complains about the upload field not being filled. when the form is validated wrong.

[gsxdsm]

Ah! I'll fix that. And add the cleanup feature. I also want to make the temporary folder name something the user can override

[nerdoc]

Wait for later today. I'll push a commit that fixes some problems and merges in form support and dir-components. Update: pushed a commit, working still on fixing some edge cases with dir compoents. But the main part is online in main. Breaks a few things, especially libraries are not usable the old way now. Just use packages now.

[nerdoc]

I uploaded a few tweaks (+some doc) for file uploading. Only moves file to final destination when form is valid.
I still have problems after uploading the file. It is in place, but the form does not receive the file. and rerendering the component seems to be broken, as alpine doesn't get loaded with the rerendering?

[gsxdsm]

Will take a look tonight

[nerdoc]

I identified a few issues that break the workflow, and may not be related to each other, but need to be fixed before upload is working properly. I'll put them into the first post to get an overview.

[nerdoc]

Getting closer. instead of handlign the file copying using storage in the submit handler, the saving of the TemporaryUploadedFile or InMemoryUploadedFile Dajngo proviles (and @gsxdsm brought to the upload method) should rather be done when pickling the file within Tetra.
This way it would be done automatically, at the correct point in time - when the component state is saved.

I am experimenting with this, but it should work.
OTOH it must be ensured that the file is not copied around whenever the component is encoded again (with each click). The Pickler has no access to the Form, only to the UploadedFile it has to pickle, which is to few to cache it with its path.

[nerdoc]

@gsxdsm could you maybe contact me on Matrix, if you're there? @nerdoc:matrix.org

[gsxdsm]

Yes I will reach out asap / later today

[nerdoc]

I pushed my work so far - but it still does not work as intended. updated the open issues in the first post above.

[nerdoc]

I got it working finally. File upload does work as intended. @gsxdsm Thank you very much for helping, I wouldn't have done this without your help and code frame.

• The crashing of tetra after the uploading was only by this code.
• form.files was not restored when recreating the form in ready(). Fixed easily.
• The biggest part of submit() was not needed - after deleting the code, it worked 😄

There are still a few smaller issues, but the code mostly works. Any polishing more than welcome.

[nerdoc]

With 71604c1 the form is cleared after submit() is called. @gsxdsm maybe you could have a look at the Js function - I implemented this because I think it is the only way of clearing a file field. The browser does not clear a file field when the page is refreshed (hence when a component is reloaded), so after submitting the filename still was in the browsers input field memory, and visible in the field.
I dunno if there is a better method.

What is not working extremely well: When you add a file to the input field, it is uploaded instantly and saved as temp file, so far, so good. But if the user presses F5 in the browser now (which is not a good idea, I know), the file is lost, stays in the temp folder, but the component looses its handle to it. I don't know why this happens.

[gsxdsm]

Excellent, this looks great thank you!!!! Will take a look at 71604c1

[nerdoc]

Hello again, a happy new year to everyone!
I think that the current approach kind of works, but has a big drawback, maybe even a legal one.
There could be a "I agree to the T&C" checkbox a the end of the form, and the user uploads a file, and THEN does not comply to the terms and conditions he read in the link below, and doesn't want to upload that file - but it is already at the server. While the server could delete unused files after a while, the user legally never consented to uploading that file.
And that could be problematic.

In e.g. Europe, GDPR requires user consent before processing user data. And uploading + saving the file is processing user data. So I think the whole process of uploading with the change event may not be legal in some (most?) cases.

The two legal ways of circumventing this problem are:

• Inform the user (in the form) that the file is uploaded instantly when adding it to the field. This cannot be done by Tetra.
• Create a form that first collects user consent for T&C (including file upload), and then add another form, where the uploading is done, as second step. This could only be done by creating a Form wizard IMHO - which should be an easy task using Tetra.
• Create an "I agree to T&C" checkbox and only enable the file upload input field when the checkbox is checked. THis could be done using Tetra/Alpine (where "terms_conditions" is the field/attribute name of the boolean T&C field):

    <input name="some_file" type="file" :disabled="!terms_conditions">

Mind the ":" before disabled (or use x-bind:).

So, this must be definitly added to the docs, because Devs MUST know about it, to decide whether to inform the user before uploading, or add a Wizard.