add direct upload feature #40
Conversation
… uploads while enabled
…mp issue when uploading file
…rigger when file gets upload by using the main frappe on_upload function
…t longer file paths
|
Hi @Ahmedsam199 thank you very much for your PR :) !!! I am reviewing it locally. I will ask some questions here related to some lines :) Thanks thanks thanks for your time and effort doing this new great functionality and creating a PR! :) |
There was a problem hiding this comment.
@Ahmedsam199 for all "file_uploader" folder, do you think it is possible to reuse funcionality from Frappe and extend it here with custom props and methods?
There was a problem hiding this comment.
couldn't figure it out honestly
|
|
||
|
|
||
| @frappe.whitelist() | ||
| def generate_presigned_url(file_name, file_path="Record"): |
There was a problem hiding this comment.
Why not using the MinioConnection->presigned_get_object() within dfp_external_storage.py?
There was a problem hiding this comment.
i just want to keep the api separate from your logic
| } | ||
|
|
||
| @frappe.whitelist() | ||
| def create_file_record( |
There was a problem hiding this comment.
Whitelisted method create_file_record is open to anyone without system user access to upload files to your S3, is that right? I am wrong?
There was a problem hiding this comment.
That's because if you want to use the direct upload separated from frappe UI
There was a problem hiding this comment.
@Ahmedsam199 I installed locally your PR to do some checks.
At this moment I localized one security issue that should be resolved before merging the PR.
create_file_recordcould be used by a low permissions user to create almost any document in Frappe, because kwargs can override even the doctype parameter and document is created withignore_permissions=True:
-
We should be able to find a way for overriding
class FileUploaderto add our functionality without duplicate so many files fromfrappe. If Frappe team makes improvements to those files, we will be always affected. If not easy we could create only a bundle.js but importing all files that are the same, for example. Tell me if you want I think about it :) -
Using always te same: spaces or tabs, not mixed:
@Ahmedsam199 thanks again for your time, tell me if I can help you with something ;)
3 participants
New Features
dfp_external_storageto enable direct uploads, enhancing user control over the upload process.DFPExternalStorageto prevent configuration with multiple buckets, ensuring data integrity and preventing potential conflicts.dfp_external_storageto prevent multiple direct uploads while enabled, avoiding resource contention.get_presigned_urlconnection forMinioConnection, allowing external access to files via presigned URLs.Bug Fixes
on_uploadfunction.api.pyto prevent server timestamp issues during file uploads.dfp_external_storage_s3_keylength to 255 characters to accommodate longer file paths, preventing truncation issues.Documentation
READMEfile, providing comprehensive instructions for users.Refactor
storage_namedependency inapi.py, improving code maintainability.Testing