Skip to content
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

Add skip and retry option for failed file uploads #30

Merged
merged 102 commits into from
Jun 14, 2024
Merged

Conversation

lehecht
Copy link
Contributor

@lehecht lehecht commented May 13, 2024

  • Add button and functionality to retry upload
  • Add button and functionality to skip failed files
  • Prevent outdated delete requests to be executed
  • Handle duplicated files in different storage requests
  • Handle exceeded storage quota
  • Add/Change tests for controller
  • Add/Change tests for delete job
  • Create PR to update fileBrowser (Update file browser core#835)
  • Fix editable fileBrowser after successfull upload
  • Fix 404 not found error for failed deletions
  • Run and commit npm run prod

Closes #11.

Remove error throwing, because it is handled later.
Add promise parts together
Don't submit storage request as long as there are failed files.
Variable finished was false even when upload was complete.
Set finish on true after the upload is complete and
set editable on false after file browser view was changed.
Remove finishing storage request.
When delete request ist outdated (retry counts don't match), don't delete chunked files
If retryChunk is true, the retry counter will be increased on backend side.
File is removed at the end of the job.
Check for uploaded chunks is done in the controller.
File paths were compared instead to look for identical file names.
Show message when duplicates files are detected.
Previous check only contained size of selected files,
but did not consider already used storage.
@lehecht
Copy link
Contributor Author

lehecht commented Jun 3, 2024

@mzur
How should a storage request be handled if all files have been uploaded successfully, but the request couldn't be updated in the end?

Currently, only the handleErrorResponse method is used. Should I add there also a retry button?

@mzur
Copy link
Member

mzur commented Jun 3, 2024

No, just keep it like that. The user should be able to click submit again (right?). Also, they could reload the page and find the storage request still pending. Then they can click submit again, too.

@lehecht lehecht requested a review from mzur June 4, 2024 06:00
src/Http/Controllers/Api/StorageRequestFileController.php Outdated Show resolved Hide resolved
src/Http/Controllers/Api/StorageRequestFileController.php Outdated Show resolved Hide resolved
src/resources/assets/js/createContainer.vue Outdated Show resolved Hide resolved
src/resources/views/create.blade.php Outdated Show resolved Hide resolved
src/resources/assets/js/createContainer.vue Outdated Show resolved Hide resolved
src/resources/assets/js/createContainer.vue Show resolved Hide resolved
src/resources/assets/js/createContainer.vue Outdated Show resolved Hide resolved
Copy link
Member

@mzur mzur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some previous comments are not resolved yet. Also, I don't see the failed/duplicate file states in the UI any more (i.e. changed color and icon):

image

@lehecht
Copy link
Contributor Author

lehecht commented Jun 7, 2024

@mzur

Some previous comments are not resolved yet. Also, I don't see the failed/duplicate file states in the UI any more (i.e. changed color and icon):

I don't know how to reproduce this bug. For me everything seems to work. Can you describe what you did?

@mzur
Copy link
Member

mzur commented Jun 10, 2024

I don't know how to reproduce this bug. For me everything seems to work. Can you describe what you did?

You are right. I pulled the recent changes and recompiled the assets. Now it works again. Maybe my biigle/core assets haven't been up to date...

@lehecht lehecht requested a review from mzur June 11, 2024 07:25
Copy link
Member

@mzur mzur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost finished! Only one comment now. And maybe it's a good idea to add a second test case for a chunked file upload that is retried now. Finally, could you clarify the discussion about this.finished above?

src/resources/views/create.blade.php Outdated Show resolved Hide resolved
@lehecht lehecht requested a review from mzur June 13, 2024 10:46
@mzur mzur merged commit e8302d1 into master Jun 14, 2024
2 checks passed
@mzur mzur deleted the skip-err-files branch June 14, 2024 08:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve upload error handling
2 participants