stephenmcd
Add FileStorage abstraction for handling remote file storage
Instead of using Python's open() and assuming that the files are stored locally, use the file storage abstraction to open the file.
This also allows the specification of a separate file storage class.
Thanks for this, it definitely needs fixing but I think a much simpler approach would be to just use the default storage engine configured. Would you like to look at that?
This does do that. If you don't specify a storage class, it uses the default. Why limit the options when it is just as easy to allow both?
Also I just realised on reviewing this a bit further - the whole idea of FORMS_BUILDER_UPLOAD_ROOT and using filesystem storage is to protect the files from being publicly accessible, so I'm not sure how good an idea it would be to bypass that.
Actually, your current solution only works if you have one application server and don't store your uploads on remote media, like Amazon S3. As soon as you have to scale to multiple app servers or want to store your uploads remotely, your solution completely fails, and people must fork your project, as we did.
In order to keep things safe, and flexible, the better option is to use Django's Storage API to allow the users to set up the storage as they need to. A custom Storage class only need set a new place to save the files that isn't part of MEDIA_ROOT. The Storage API can do everything your existing FORMS_BUILDER_UPLOAD_ROOT can do and more.
You're right. So I guess the approach is on the right track, it probably needs a bit more work though.
We should probably have one location in the code that returns the file storage object, which is either file system storage when FORMS_BUILDER_UPLOAD_ROOT is defined, or the custom storage.
I think defaulting back to the default storage is probably not what we want, and that if either of the settings aren't defined, we raise some kind of error. (I realise this problem existed already.)
I guess we should point out in the docs as well that one of the settings is required, and why (so that the files aren't publicly accessible).
Also some work to be done in https://github.com/stephenmcd/django-forms-builder/blob/master/forms_builder/forms/admin.py#L187 where (if I recall) admin users can download uploaded files from links with emails they're sent.