[PB-6600]:refactor/upload managers to only handle networks context#2030
[PB-6600]:refactor/upload managers to only handle networks context#2030jaaaaavier wants to merge 2 commits into
Conversation
Deploying drive-web with
|
| Latest commit: |
6fb8f95
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://eac62ae8.drive-web.pages.dev |
| Branch Preview URL: | https://refactor-uploadmanager-and-u.drive-web.pages.dev |
|
| progress: uploadProgress, | ||
| }, | ||
| }); | ||
| this.taskLifecycle.updateTaskProgress(taskId, uploadProgress); |
There was a problem hiding this comment.
Again, it's not the responsibility of the upload manager to manage the tasks lifecycle. The upload manager may have events/callbacks like onFileUploaded or onFolderUploaded that could be used to manage tasks by injecting specific callbacks, but that should happen outside of here.
Right now, the control flow is
The component starts the upload
-> The upload service (say folder or file, whatever)
-> Upload Manager (handles the data transfer)
-> Tasks Management (handles UI)
The control flow should be
The component starts the upload
-> The upload service (say folder or file, whatever)
-> Tasks Management (handles UI)
-> Upload Manager (handles the data transfer)
There is still a dependency between two unrelated things, making the code coupled and far less maintainable (and testable): the tasks management and the upload manager.
Think it like this: if you had to change something related to the tasks management, does it have sense to go to the upload manager to make changes? It sounds like something strange is going on there. If you want to change something related to tasks, you go to the tasks management related-code only.
Consider this reading @jaaaaavier, as although for this specific PR and topic seems trivial, it is worth a lot in the medium and long term for our growing codebase
2 participants
Description
The aim of this PR is to relieve the UploadManager and UploadManagerFolder files of the responsibility for managing the lifecycle of tasks, trackings, loggings and other matters unrelated to upload management which is, after all, what they should be doing.
Related Issues
Related Pull Requests
Checklist
Testing Process
I’ve tested the preview version for uploading new files and folders, and there were no problems.
Additional Notes